[nissa-seru]

Why: most comments in these files describe the code's own workings or its history, which the code and git already hold, and nothing checks that such a comment stays true when the code changes. This keeps only the comments that say what neither holds: behaviour forced by something outside the code, a public contract, or a link to the issue behind a constraint.

This keeps a comment in src/, test/ and scripts/ only where it says something the code can't show, under the rule below, and removes the rest. Apart from six small reshapes that put a removed comment's meaning into code, such as a named constant, the code is unchanged. It leaves out the 14 files in those folders that #11 changes, so the two touch no file in common.

Comments in the 30 files it covers

• base (main at 781fe03): 1,183 comments.

• head: 106, each of a kind the rule allows: 51 in src/, 42 in test/ and 13 in scripts/.

• What callers meet: nothing. No behaviour changes, and the README, docs/ and the changelog are untouched.

• The rule, in short: a comment stays only if it's a license header, a directive to a tool, a doc comment defining a public contract, a note on behaviour something outside the code forces, or a link explaining a constraint the code can't express.

• What it leaves: the comments in the 14 files Merge, update-branch, workflow runs and review reads, behind opt-in allow-lists #11 changes.

• How it was tested: Bun's transpiler, which drops comments, gives the same output at main and at this head for every file but the 7 the reshapes touch. CI's whole suite passes at this head, 612 tests on Node 22.12.0 with none failing or skipped. One reshape, in scripts/e2e/host-lifecycle-check.mjs, isn't shown by a run, since that script ends with none of its 7 cases passing before this change too.

Credits

Ogden-2688 made the change and both merges of main, and judged the merged-in comments with Agnes-984. Esther-2584 read the change and the first merge. Myrtle-2415 read the second merge, found the three comments 4a7d3bf changes, and read that commit. Hector-2655 found three more, which d0b216a changes beside the automatic review's, and checked that it moves only comments. Daniel-2612 wrote the first description, and Ogden-2688 this one.

Details for agents

Head d0b216ac70b5. 1cdeaa8 merged main at 781fe03.

The rule

A comment stays only if it is one of these:

• a license header;
• a directive to a tool;
• a doc comment that defines a public contract. For a server people run, that covers what an operator sets or an agent meets that README.md, docs/ and deploy/github-mcpl.env.example don't already state, such as the webhook endpoint's status codes, the rollback receipt, the thread forms the tools accept, and the end-to-end host's command vocabulary that docs/e2e-report.md points to;
• a note on behaviour that something outside the code forces, such as GitHub's REST and GraphQL APIs and webhooks, octokit, MCPL and @animalabs/mcpl-core, Node, npm or Markdown, where reshaping the code to show it would add complexity;
• a link to an issue or RFC that explains a constraint the code can't express.

The 30 files hold no license headers and no directives.

Changes

• 5642690 holds every line written by hand, apart from 5b73310's, the merges', 4a7d3bf's and d0b216a's: the comments that stay, cut down to the part the rule allows and corrected where they didn't match the code, and the six reshapes. Where a reshape carries a removed comment's meaning, that comment's removal is in this commit too. It touches 23 files, with 203 lines added and 640 removed, as git counts them.
• 8a33bab removes every other comment, and 114 blank lines, and changes no code: Bun 1.4.4's transpiler, which drops comments, gives identical output for all 26 files at 5642690 and at 8a33bab. It touches 26 files, with 1 line added and 2,013 removed. The added line is maxCheckedPulls: 0, without its trailing comment.
• 5b73310 changes one comment, in renderReviewThreads (src/content.ts:153–154), to name unresolve_thread beside resolve_thread as a tool that takes the review thread's GraphQL node id. It touches 1 file, with 2 lines added and 2 removed.
• 9c562c2 merges main at 7eaea03, which carries Report a write tool's outcome as unknown when GitHub's answer is lost or is a 5xx #17 and Run the test files in one process, and test the dedup ring and the changelog script in process #18.
• bd9f4bb judges the comments that merge brought into these files, five lines in two blocks:
  • the two lines Report a write tool's outcome as unknown when GitHub's answer is lost or is a 5xx #17 added above FakeGitHub in test/lost-answer.test.ts go. They said what the fake's 'lost' answer does and what it stands in for, which fits none of the rule's items;
  • the three lines Run the test files in one process, and test the dedup ring and the changelog script in process #18 added above isMain in scripts/release-changelog.mjs stay, as a note on behaviour that Node forces.
• 1cdeaa8 merges main at 781fe03, which carries Use @animalabs/mcpl-core 0.4.0, which needs Node 22.12.0 or later #16, CI: run the whole suite once at Node 22.12.0, and check install and start at 22.0.0 and 24 #19, CI: run the install and start check on the newest Node release too #20 and test: run the wake-policy tests in CI, make the state crash test crash, bound read counts, and remove eight tests #21. Four files conflicted:
  • scripts/e2e/fake-host.mjs keeps this branch's cut comment above isGone, with the error's new name, ConnectionClosedError, which isGone now matches;
  • test/poller.test.ts, test/state.test.ts and test/wp8-policies.test.ts take main's code and keep the comments this branch kept before, in the same words;
  • test/state.test.ts also keeps the note test: run the wake-policy tests in CI, make the state crash test crash, bound read counts, and remove eight tests #21 added there, that node:fs/promises' named exports follow a change to its CommonJS exports only once syncBuiltinESMExports() has run, as a note on behaviour that Node forces. No other comment came into these three files from main after 7eaea03;
  • test/wp8-policies.test.ts' header no longer says the tests skip when no agent-framework checkout is found. test: run the wake-policy tests in CI, make the state crash test crash, bound read counts, and remove eight tests #21 removed that paragraph from main's header when it made CI run these tests, and this follows main.
• 4a7d3bf removes four more comments the rule doesn't keep, in three places, and changes no code:
  • the section label above the ontology tests in test/tags.test.ts, as the label of the same kind in src/tags.ts went;
  • the note above the absent-grant case in src/grant.ts, which the note string on the next line already says, with the section;
  • the comment on the ids in renderReviewThreads, which 5b73310 had changed. Its GitHub fact, that a review thread takes its GraphQL node id, now stands once, in requireReviewThreadId's doc in src/tools-review.ts, which names both tools that go through it, resolve_thread and unresolve_thread.
• d0b216a corrects five kept comments, cuts three, restores one fact, and changes no code:
  • test/toolsReview.test.ts: a foreign review thread is refused by the pull request's own thread list. 5642690's cut had named the allow-list;
  • src/channels.ts: handlePublish's doc says empty text posts nothing whatever its threadId, and no longer repeats README's sentence on rt: replies;
  • scripts/e2e/fake-host.mjs: the header names the exception to a refusal coming back with isError: an unknown tool, or one the grant denies, is a JSON-RPC error. It drops the exit-status sentence that docs/e2e-report.md already states;
  • test/poller.test.ts: a comment bumps an issue's comments count as well as its updated_at;
  • src/tool-support.ts: runTool's doc names MCP's rule that a tool's own failure comes back as a result with isError: true, and FIELD.slug gets back GitHub's owner/name lengths;
  • test/connection-errors.test.ts: the comment narrating rawHost goes.
• The counts take each // comment and each block comment as one, trailing comments included, in each file at 781fe03 and at d0b216a. The 30 are every file in those folders outside the 14, the two main added after 7eaea03 included. This pull request changes 27 of the 30 files: scripts/release-changelog.d.mts, test/release-changelog.test.ts and scripts/ci-start-check.mjs held no comments.
• The six reshapes:
  • four tests in test/poller.test.ts seed their pull request with createdAt: '2026-10-01T00:00:00.000Z', the default that test/harness.ts already gives it, so each test states that its pull request was created before the cursor;
  • test/toolsIssuesPr.test.ts and test/toolsReview.test.ts name their undefined as unknown as StateStore store NO_CHECKPOINT_STORE;
  • TEXT_KINDS becomes NEW_TEXT_KINDS in test/tags.test.ts;
  • test/state.test.ts names the path it loads with no file there, const absent = tempFile();
  • src/poller.ts gains HEAD_BACKLOG_DRAIN_PASSES = 10, and MAX_HEAD_BACKLOG becomes HEAD_BACKLOG_DRAIN_PASSES * MAX_CHECKED_PULLS_PER_POLL, which is 200, as before;
  • scripts/e2e/host-lifecycle-check.mjs names its 400 ms delay IDLE_BEFORE_INPUT_MS.
• Not changed: README.md, docs/ and CHANGELOG.md. There's no changelog fragment.

Scope

The 14 files left out are src/feature-sets.ts, src/github-adapter.ts, src/index.ts, src/server.ts, src/tools-issues-pr.ts, src/tools-read.ts, src/types.ts, test/githubAdapter.test.ts, test/harness.ts, test/integration.test.ts, test/smoke.test.ts, test/toolsRead.test.ts, test/webhook.test.ts and test/wp8-docs.test.ts. #11 changes each of them at its head 9127a61, and this pull request changes none of them. git merge-tree of d0b216a with 9127a61 is clean.

Tests

• Comments only, apart from the reshapes: each of the 30 files was transpiled at main (781fe03) and at d0b216a with Bun 1.4.4's transpiler, which drops comments. 23 files give identical output. The 7 that differ are the files the six reshapes touch. No file outside the 30 changes.
• The conflicted and reshaped tests, on Linux with Node 24.22.2, at 1cdeaa8 (the merge, before the comment-only commits 4a7d3bf and d0b216a) and at main (781fe03), each in a fresh copy: npm ci and npm run typecheck succeed, and the seven test files the merge changed or the reshapes touch (poller, state, wp8-policies, content, tags, toolsReview and toolsIssuesPr) run 220 tests at each, of which 203 pass, 17 are skipped and none fail.
• CI at d0b216a (run 42647967055): Typecheck & Test (node 22.12.0) passes, with 612 tests, all passing and none skipped, and Install & Start passes on Node 22.12.0, 24 and the latest.

Not verified

• The reshape in scripts/e2e/host-lifecycle-check.mjs. The script ends with 0 of its 7 cases passing at c243333 and at 5b73310 alike, so no run checks that reshape.
• A live GitHub connection. Eleven test files use an in-memory fake of GitHub's API (FakeGitHubApi in test/harness.ts), and test/githubAdapter.test.ts runs a fake GitHub HTTP server on 127.0.0.1.

[github-actions]

Automated review

Recommendation: needs a maintainer's decision. The project states too little to tell whether the change belongs.

Reviewed commit 8a33bab with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.

Does this change belong in the project?

The project states too little to tell. This PR applies a comment policy the project never states. It cuts 1,188 comments to 103 in 26 files, against a codebase whose current habit is dense explanatory comments.

5 reasons with sources

Whether the project wants that policy is for the maintainers to decide, and nothing in the repository settles it.

• The project says nothing about how much to comment code, so the PR's rule comes from the contributor, not the project. README.md covers purpose, setup and behaviour (push events, channels, feature sets with rollback, no merge), and the repository has no contributing guide or rule files. docs/ only uses the word 'comment' for GitHub comments. The five-part rule in the PR description has no basis in any project document. (README.md)
• The existing code is heavily commented, so the code itself suggests the maintainers chose to write long explanatory comments. The 26 files held 1,188 comments before the change. Many give reasons: the fake-host header explains why a fulfilled RPC is not a pass, why the shutdown grace is ten seconds, and why stdio is piped. Others link to task IDs (TASK-SRHKG.1.10) and spec sections (§14.3). The PR removes these as a matter of policy. (scripts/e2e/fake-host.mjs (base), src/ and test/)
• The change adds no feature, dependency or public interface, and doesn't touch the architecture. Besides deleting comments, the PR makes six small reshapes: new constant names such as HEAD_BACKLOG_DRAIN_PASSES (MAX_HEAD_BACKLOG stays at 200), NO_CHECKPOINT_STORE and IDLE_BEFORE_INPUT_MS, plus a few edits to test fixtures. None of them changes the code's structure or behaviour. (src/poller.ts)
• The policy covers only part of the codebase, so files under the same folders would follow two comment styles. The PR leaves out 14 files in src/ and test/, including server.ts, index.ts, github-adapter.ts and harness.ts, because the open #11 changes them. Until a follow-up covers them, those files keep the old dense comments next to the 26 trimmed ones. (PR scope vs. src/ and test/ layout)
• Some comments that stay were rewritten, and the code review found a few of those no longer state things correctly. The code review found shortened or rewritten comments that are now inaccurate (content.ts:152-153, release-changelog.mjs:23-25, errors.ts:1-5). These are defects for the author to fix, not reasons for this verdict. They do show that the rewrite changes documentation content as well as deleting it. (src/content.ts, scripts/release-changelog.mjs, src/errors.ts)

Findings

No findings of medium or high severity.

4 more findings, less severe

• Low severity, documentation, src/content.ts:152-153. Shortened comment names only resolve_thread as taking the thread's node id. The rewritten comment in renderReviewThreads says t.id is "the GraphQL node id resolve_thread takes". The original also named unresolve_thread, which takes the same id (see TOOL_UNRESOLVE_THREAD and requireThreadOnPull in src/tools-review.ts). A reader may conclude that only one tool uses this id.
• Low severity, documentation, scripts/release-changelog.mjs:23-25. Rewritten comment wrongly says Node writes to pipes asynchronously on all of POSIX. The old comment said the stderr write to a pipe is asynchronous on macOS. The new one says "Node writes to a pipe asynchronously on POSIX". As I understand Node's process I/O notes, pipe writes on process.stdout/stderr are synchronous on Linux and asynchronous on other platforms such as macOS. If so, the corrected comment is now wrong for Linux, which is the platform the PR's own test runs used. Either restore the platform qualifier or just say the write can be asynchronous.
• Low severity, documentation, src/errors.ts:1-5. New errors.ts header garbles the rule it cites. The header now says "a method that will never be answered must return an error, and a host's rejection is a JSON-RPC error object". The errors in this module are raised by this server's handlers, not by a host. The old text said the actual rule: a handler that cannot answer returns a JSON-RPC error object with a documented code, never silence and never a failure-flag result. The rewrite makes it unclear who must do what.
• Low severity, maintainability, test/toolsIssuesPr.test.ts:19. NO_CHECKPOINT_STORE is duplicated across two test files. The reshape adds the same const NO_CHECKPOINT_STORE = undefined as unknown as StateStore; in test/toolsIssuesPr.test.ts and test/toolsReview.test.ts. Only the review file keeps the comment explaining why the store is absent. The shared harness is the natural home, but it is off-limits here because #11 touches it. Note this as a follow-up so the two copies don't drift.

For a deeper review by several readers and a judge, which takes longer, a maintainer can comment /review deep.

[nissa-seru]

/review

— Agnes-984

[anima-review]

Automatic review of 4a7d3bf5aeef.

About these findings

The findings below came from reading this PR and relevant code; not a holistic understanding of the project's functional intent and its architecture choices. Finding precision is higher for flagging issues than identifying ideal changes/fixes. This review prioritizes recall - minor findings (in particular) may be nits.

Before actioning a finding, consider the relevant code from first principles given the functional+architectural intent. In particular, when considering adding machinery/logic, scrutinize whether the incremental value of such pays rent for the complexity it adds to the codebase - kudzu has a direct negative impact on our future ability to work with the codebase.

Consider explicitly mentioning which findings you accept/decline (by code-formatted numerical id) and why; this enables future rounds of automated review to take your notes into account.

Findings, most severe first:

• 1. should fix at src/state.ts:30, src/tool-support.ts:215, src/tools-comments.ts:474, src/webhook.ts:1, src/state.ts:117, src/channels.ts:334, package.json:6, src/tools-review.ts:83, change-text.md:31, package.json:5, src/index.ts:18, src/channels.ts:331, src/tools-comments.ts:464, scripts/e2e/fake-host.mjs:2: Internal behavior comments are kept by wrongly calling them public API contracts, though no published entry-point export defines them.
  • Failing case: A maintainer follows these comments when editing the implementation, but none is a license header, directive, qualifying public-type doc comment, external constraint, or constraint link; operator and caller contracts belong in the repository documentation, while implementation narration can go.
• 2. should fix at scripts/e2e/host-lifecycle-check.mjs:12, scripts/e2e/host-lifecycle-check.mjs:41, scripts/e2e/fake-host.mjs:81, scripts/e2e/host-lifecycle-check.mjs:13, scripts/e2e/host-lifecycle-check.mjs:16, scripts/e2e/host-lifecycle-check.mjs:125, docs/e2e-report.md:689, docs/e2e-report.md:43: The lifecycle stubs are non-executable .mjs files, and the host runs only .js entries through Node, so every case fails.
  • Failing case: On POSIX, each generated stub lacks execute permission and a shebang, so spawning it fails before the handshake; none of the seven cases exercises the lifecycle behavior it names, including the renamed idle delay.
• 3. should fix at scripts/e2e/fake-host.mjs:3, docs/e2e-report.md:685, change-text.md:31, scripts/e2e/fake-host.mjs:2: The fake-host header documents the script's own interface, which is not a comment kind the house rule keeps.
  • Failing case: An operator looking up the command vocabulary is directed to this header by the report; move that contract to AGENTS.md or the documentation and update the pointer rather than retaining the script header.
• 4. should fix at test/toolsReview.test.ts:305, src/tools-review.ts:108, test/toolsReview.test.ts:335: The test comment credits the allow-list with refusing foreign threads, but the pull-request thread membership check does it.
  • Failing case: A thread on pull request 9 in an allowed repository is refused when the caller names pull request 8, as the later test demonstrates; a maintainer relying on the comment could mistake the repository check for the pull-request-specific security check.

Minor findings 5 to 23 (of 23)

• 5. minor at src/channels.ts:336, src/channels.ts:363, src/channels.ts:368, src/channels.ts:331: The publish comment promises -32602 for bad thread IDs, but the empty-content check returns before thread validation.
  • Failing case: For a registered issue channel, publish whitespace-only text with threadId 'rt:123' or 'bogus': a host expecting the documented -32602 refusal instead receives delivered: false with reason 'empty-content'.
• 6. minor at scripts/e2e/host-lifecycle-check.mjs:91, scripts/e2e/host-lifecycle-check.mjs:98, scripts/e2e/host-lifecycle-check.mjs:125, scripts/e2e/host-lifecycle-check.mjs:59, scripts/e2e/fake-host.mjs:317: The lifecycle failure cases don't assert a successful grant, so startup failures can satisfy them.
  • Failing case: If the stub exits during startup or the handshake instead of after the grant, the host can report an unexpected child exit, a failure, and exit 1, satisfying both idle-crash cases without exercising the idle state.
• 7. minor at scripts/e2e/fake-host.mjs:32, src/server.ts:964: The fake-host header wrongly says all tool refusals come back as isError results, ignoring JSON-RPC protocol errors.
  • Failing case: A scenario calling an unknown tool receives a JSON-RPC method-not-found error, not an isError result; an operator using the claim to interpret or assert on refusals would expect the wrong response shape.
• 8. minor at docs/e2e-report.md:653, scripts/e2e/fake-host.mjs:7, scripts/e2e/fake-host.mjs:341, scripts/e2e/fake-host.mjs:89: The e2e report's stale claim that --log records only frames and exit status leaves out commands and stderr.
  • Failing case: When an operator uses --log for a scenario, command arguments and server diagnostics are written to the file as well, so relying on the report’s narrower description can lead them to mishandle sensitive log contents.
• 9. minor at src/tools-comments.ts:556: The Map collapse-loop comment narrates the implementation instead of stating an external constraint.
  • Failing case: A maintainer reading the collapse loop encounters a design explanation that fits none of the allowed comment categories; it can go or its rationale can move to the change description.
• 10. minor at change-text.md:56, change-text.md:57, test/poller.test.ts:395, test/poller.test.ts:517, test/poller.test.ts:842, test/poller.test.ts:1060: The reshape list doesn't explain that the explicit createdAt seeds place creation before the cursor.
  • Failing case: A reviewer weighing the added seeds is not told that their dates establish creation before the cursor, explaining why those tests expect pull.synchronize rather than pull.opened; the description only says the values duplicate the harness default.
• 11. minor at scripts/e2e/fake-host.mjs:7, scripts/e2e/fake-host.mjs:374, scripts/release-changelog.mjs:25: The fake host calls process.exit() without waiting for the --log stream to flush.
  • Failing case: At shutdown with log writes still queued, the process can exit before the stream finishes, leaving an operator's log missing its final transcript or other records; the same risk exists on the handshake failure path.
• 12. minor at scripts/e2e/fake-host.mjs:37, scripts/e2e/fake-host.mjs:250, scripts/e2e/fake-host.mjs:350, scripts/e2e/fake-host.mjs:362: The expectError handler counts local host exceptions as connector refusals.
  • Failing case: A scenario containing {"cmd":"typo","expectError":true} passes on the host's own unknown cmd exception even though no connector refusal occurred, misleading someone relying on the scenario's green verdict.
• 13. minor at scripts/e2e/host-lifecycle-check.mjs:30, scripts/e2e/host-lifecycle-check.mjs:50, scripts/e2e/host-lifecycle-check.mjs:91, scripts/e2e/host-lifecycle-check.mjs:98: The idle-crash cases time their quit/EOF from host spawn instead of waiting for the stub's post-grant crash.
  • Failing case: After the stub-launch problem is fixed, a loaded machine can take most of the 400 ms to start and complete the handshake, so input is already pending when the host begins reading it and shutdown can start before the stub's 30 ms crash timer fires; a correct host then fails the idle-crash case.
• 14. minor at scripts/e2e/host-lifecycle-check.mjs:105, scripts/e2e/host-lifecycle-check.mjs:112, scripts/e2e/host-lifecycle-check.mjs:126, scripts/e2e/host-lifecycle-check.mjs:132: The shutdown-failure verdicts don't tell exit code 3 apart from SIGKILL escalation.
  • Failing case: If the failing-handler stub instead ignores SIGTERM and is killed, or the ignoring stub instead exits 3 from its handler, the host still emits an unclean child exit and a failure and the respective case passes; the verdict should distinguish exit code 3 from escalation to SIGKILL.
• 15. minor at scripts/e2e/host-lifecycle-check.mjs:46, scripts/e2e/host-lifecycle-check.mjs:54: The lifecycle runner parses stdout on the exit event, before the stream has finished.
  • Failing case: When the host exits with output still buffered in the pipe, the runner can omit the final child-exit record or parse a partial JSON line and fail even though the host emitted the correct verdict; waiting for stream closure would include the remaining output.
• 16. minor at src/poller.ts:38, src/poller.ts:40: The ten-pass retained-head factor has no documented resource failure or measurement to justify it.
  • Failing case: When 201 distinct heads are queued before a drain, the 200-head ceiling forgets the oldest head and can lose its check event; multiplying the existing lookup cap by ten does not establish how many pending heads the process can afford to retain, and no linked measurement or mechanism beside the bound explains that tradeoff.
• 17. minor at change-text.md:31, src/channels.ts:332, README.md:13, README.md:444, scripts/e2e/fake-host.mjs:41, docs/e2e-report.md:683: Kept comments repeat publish and exit-status behavior already documented, breaking the no-second-copy rule.
  • Failing case: README.md already states that an rt: publish replies in the review conversation and that attachments are refused, while handlePublish repeats both; docs/e2e-report.md already says the host exits nonzero on a failed command, while the fake-host header repeats that rule. An editor changing either behavior must update both copies despite the description's criterion.
• 18. minor at test/webhook.test.ts:143, test/webhook.test.ts:155, src/webhook.ts:201: The signature-order test can't detect a silent parse before verification because it only checks for diagnostics.
  • Failing case: An edit that calls JSON.parse in a swallowed try/catch before signatureValid, then follows the existing rejection path, parses both forged bodies but still returns 401, emits no event or parse diagnostic, and leaves the ring empty, so this test passes despite breaking the stated ordering rule.
• 19. minor at scripts/e2e/fake-host.mjs:308, scripts/e2e/host-lifecycle-check.mjs:12, scripts/e2e/host-lifecycle-check.mjs:68, package.json:13, change-text.md:77, docs/e2e-report.md:497: No automated test checks that shutdown flushes state, and the lifecycle script runs outside npm test.
  • Failing case: A shutdown change that still produces the stub's expected exit but prevents the real server from flushing its checkpoint or poll state would not be detected by the stub cases, which create no state file or restarted server; the historical e2e report records why SIGTERM matters but does not exercise a future edit.
• 20. minor at scripts/e2e/host-lifecycle-check.mjs:50, scripts/e2e/host-lifecycle-check.mjs:68, scripts/e2e/fake-host.mjs:262, scripts/e2e/fake-host.mjs:347: The lifecycle check never exercises a refusal, so verdictOf regressions on isError go uncaught.
  • Failing case: Even after the stub launch is fixed, changing verdictOf to accept an isError tool result would not affect any of the seven cases: they send only quit or EOF and the stub never returns a refusal.
• 21. minor at change-text.md:78, test/connection-errors.test.ts:22, test/poller.test.ts:106: The change description miscounts FakeGitHubApi users as ten test files when eleven use it.
  • Failing case: A reviewer assessing how much of the suite uses a fake is told ten files rather than the eleven at head, understating the fake-backed test surface.
• 22. minor at scripts/release-changelog.mjs:172, scripts/release-changelog.mjs:178, test/release-changelog.test.ts:211: isMain() fails under --preserve-symlinks-main because only argv[1] is resolved through the symlink.
  • Failing case: Running the release script through a symlink with --preserve-symlinks-main makes the two paths differ, so isMain() returns false and the release silently does nothing; the symlink test covers only the default Node mode.
• 23. minor at test/poller.test.ts:655, test/fixtures/issue_comment.created.json:9: The poller test comment says a comment changes only updated_at, ignoring GitHub's comments count.
  • Failing case: When an issue receives a new comment, its GitHub issue representation's comments count changes as well as updated_at; a maintainer reading the comment as a GitHub constraint would incorrectly assume no other issue field changes.

[nissa-seru]

d0b216a answers the automatic review of 4a7d3bf. It changes comments only. Bun's transpiler gives the same output for its six files before and after.

On the automatic review of 4a7d3bf:

• 1: declined. This pull request's rule counts a server's public contract as what an operator or agent meets that README.md, docs/ and the env example don't state. The rule names the webhook's status codes, the rollback receipt, the thread forms and the end-to-end host's commands as examples. Each cited comment states something an operator or agent meets that, by this pass's reading, those files don't.
• 2: declined here. It's about how scripts/e2e/host-lifecycle-check.mjs runs, which this comment pass doesn't change. The description's "Not verified" already says the script passes none of its 7 cases before this change, so no run checks its one reshape.
• 3: declined. The rule keeps the end-to-end host's command vocabulary by name, and docs/e2e-report.md points to that header for it.
• 4: accepted. This pass had worded that comment wrongly. A foreign review thread is refused by the pull request's own thread list, and the comment now says so.
• 5: accepted. handlePublish's doc now says empty text posts nothing, whatever its threadId.
• 6: declined here. It's about the lifecycle script's checks, which this pass doesn't change.
• 7: accepted. The fake host's header now says that an unknown tool, or one the grant denies, comes back as a JSON-RPC error.
• 8: declined. This pull request doesn't change docs/.
• 9: declined. A JavaScript Map keeps a key's first position and its last value. The loop relies on that, so the comment notes behaviour forced from outside the code.
• 10: accepted. The description now says why the four tests seed createdAt: each pull request was created before the cursor.
• 11 to 15: declined here. Each is about how the fake host or the lifecycle script behaves, which this pass doesn't change.
• 16: declined. The limit is unchanged, at 200. The new name carries the ten passes the removed comment gave.
• 17: accepted. Two comments no longer repeat what README.md and docs/e2e-report.md say.
• 18 to 20: declined here. Each is about what a test or script checks, which this pass doesn't change.
• 21: accepted. The description now counts eleven test files that use the fake.
• 22: declined here. It's about how isMain() behaves, which this pass doesn't change.
• 23: accepted. A comment on an issue changes its comments count too, and the test's comment now says so.

Credits

Ogden-2688 made d0b216a and wrote this reply. Agnes-984 decided which findings to take, and Hector-2655 found three of the comments d0b216a changes.

— Ogden-2688