fix: address PR #2685 review comments - #2687
Conversation
Two valid findings, both on code added in this release. 1. packrat_list_pack_items advertised a bogus nextOffset. Passing `limit: items.length` to withNextOffset made its `items.length >= limit` check true for every response — including an empty pack, which returned `nextOffset: 0`. A consumer following that value could call the tool repeatedly or duplicate items. The endpoint takes only pack_id and returns every item in one response, so there is never a next page: return `nextOffset: null` directly and don't route it through withNextOffset. Comment explains why, so this isn't 'simplified' back later. 2. /support hardcoded support@packratai.com, bypassing the canonical siteConfig.support contract (hello@packratai.com, asserted by __tests__/legal.pages.test.ts). Render siteConfig.support.email/.mailto instead. Also refreshes the now-stale site.ts comment that said we don't run a support web page. The remaining CodeRabbit findings are on the Swift outbox/APIClient work from other PRs in this release and are left to those authors. MCP tests 1273 pass; tsc + biome clean. The one failing landing test (terms-of-service robots metadata) is pre-existing on development and unrelated.
|
Warning Review limit reached
Next review available in: 30 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report for packages/units (./packages/units)
File CoverageNo changed files found. |
Coverage Report for packages/overpass (./packages/overpass)
File CoverageNo changed files found. |
Coverage Report for apps/expo (./apps/expo)
File CoverageNo changed files found. |
Coverage Report for packages/api (./packages/api)
File CoverageNo changed files found. |
Coverage Report for packages/analytics (./packages/analytics)
File CoverageNo changed files found. |
Coverage Report for packages/mcp (./packages/mcp)
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Coverage Report for packages/utils (./packages/utils)
File CoverageNo changed files found. |
1 participant
Addresses the CodeRabbit review comments on #2685 that land on code added in this release. Two findings, both valid.
1.
packrat_list_pack_itemsadvertised a bogusnextOffsetReal bug, not just a style nit. The handler passed
limit: items.lengthintowithNextOffset, whose continuation check isitems.length >= limit— true for every response, including an empty pack, which returnednextOffset: 0. A consumer that followsnextOffsetcould call the tool repeatedly or duplicate items.This endpoint accepts only
pack_idand returns every item in one response, so there is never a next page. It now returnsnextOffset: nulldirectly instead of routing throughwithNextOffset. The comment explains the reasoning so it doesn't get "simplified" back into the helper later.2.
/supportbypassed the canonical support contractThe page hardcoded
support@packratai.com, butapps/landing/config/site.tsdefinessiteConfig.support.emailashello@packratai.comand__tests__/legal.pages.test.tsasserts that value. The page now renderssiteConfig.support.email/.mailto.hello@packratai.com, matching the canonical config and the address given in the Anthropic submission.Also refreshes the now-stale comment in
site.tsthat said "we don't run a separate support web page yet."Not addressed here
The other 8 CodeRabbit findings on #2685 are on the Swift outbox / APIClient work from #2673, #2674 and #2676 — retry backoff and 429/408 classification,
local-legacy ID migration, removing unreachablethrows,displayMessagewhitespace trimming. Several involve real design calls, so they're left to those authors rather than guessed at.Testing
bun test:mcp— 1273 pass, 23 skippedterms-of-servicerobots metadata) is pre-existing ondevelopmentand unrelated — verified by re-running it with these changes stashed.