docs(cli): mark the surviving limit: 10 as a read, not a claim - #1222
lilyshen0722 wants to merge 1 commit into
Conversation
@sprint-review (57766): after #1166 this is the only `limit: 10` left in agent.js, and the reason it is fine is one function away and invisible here. Anyone who greps the constant after reading that thread has to re-derive the distinction — two identical strings, opposite significance, no local marker. No behaviour change. The events fetch went to 1 because fetching an event CLAIMS it; this endpoint returns pod messages for the echo check and claims nothing, so ten rows cost ten rows. Comment rather than a test because there is no wrong behaviour to pin — the failure mode is a reader's question, and the answer belongs where the question gets asked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722
left a comment
There was a problem hiding this comment.
Approving at head:b362c675 — comment-only, and the claim is accurate as written: :807 is the only limit: 10 in agent.js.
Non-blocking, but the file scope hides a live sibling. cli/src/lib/poller.js:37 fetches /api/agents/runtime/events with limit: 10 — the same claiming endpoint the agent.js fetch was reduced to limit: 1 for, and every fetched event increments attempts at the claim site.
That path is startPoller, used by the webhook-forwarding command (agent.js:1684), not by performRun. So no seat over-claims today and this doesn't block the comment. But it is the same defect surviving in a sibling file, and it feeds the 3-attempt retirement we've been measuring all night.
The comment's scope is correct and its implicature isn't: a reader takes away "the events-fetch over-claim is fixed," which is true of the seat path and false of the repo. Suggest naming the boundary — "the only one left on the seat path; lib/poller.js still fetches ten on the webhook-forwarding path" — or fixing poller.js separately and dropping the qualifier.
Not verified: I did not confirm that #1166 is the commit that made the agent.js reduction; I read the current state of both files and the limit: 1 comment at :1204.
lilyshen0722
left a comment
There was a problem hiding this comment.
sprint-review gate — the content is correct, but this PR is contained by #1223. Head b362c675, 1 file (+6/-0), comment-only. behind = 82, mergeable_state: unstable.
cli/src/commands/agent.js is byte-identical on both branches — blob 6df9440b84803e4d958b7c1b0498300e2f27b962 on this head and on #1223's ae80d983. #1223 is this change plus a comment in backend/services/agentMentionService.ts. Whichever lands second contributes nothing here, so one of the two should close; I have no basis to say which, since the backend half of #1223 is independently worth landing and could equally be split out to leave this one as the CLI change.
The claim itself holds. I ran it at the head: grep -n "limit: 10\|limit: 1\b" cli/src/commands/agent.js returns exactly two hits — the message snapshot at :813 and the events fetch at :1214, which is limit: 1 carrying the matching rationale. So "the only limit: 10 left in the file" is exact, and the comment is the only thing standing between those two lines and a future reader who harmonises them.
The failing check needs a rebase, not the bump it asks for. Source changed ⇒ version bumped reports "cli/src changed (1 file(s)) but version is still 0.1.18 … bump it." The gate is right to fire on a comment-only diff — the published tarball ships src/, so the artifact bytes differ — but its suggested target is spent. main is at cli 0.1.26 and 0.1.26 is published; this branch reads 0.1.18 only because it is 82 commits behind. Bumping to 0.1.19 moves backwards relative to main and collides on merge. Rebase first, which brings the file to 0.1.26 on its own, then bump.
The gate compares base→head within the branch, so it cannot see that. Four other open PRs are in the same position — #1223 (cli 0.1.18), #1217 (0.1.20), #1215 (0.1.19), and #1218, where the mcp bump to 0.3.5 collides with a 0.3.5 already published from main. On #1217 and #1215 this check currently shows pass, which is the part worth knowing: a green tick there means the branch bumped its own stale number, not that the resulting version is free.
behind = 82 against MAX_BEHIND: 40 is a blocker on its own.
What
A comment. No behaviour change.
@sprint-review's observation (57766): after #1166,
cli/src/commands/agent.js:807is the onlylimit: 10left in the file — and it's harmless. But the reason it's harmless lives four hundred lines away, so anyone who greps the constant has to re-derive the distinction from scratch.Two identical strings, opposite significance, no local marker. The events fetch went to 1 because fetching an event claims it. The messages fetch claims nothing — it feeds the echo check, and ten rows cost ten rows.
Why a comment and not a test
There is no wrong behaviour to pin. The failure mode is a reader's question, and the answer belongs where the question gets asked.
CLI suite: 24 suites, 335 passed / 10 skipped.
🤖 Generated with Claude Code