You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The self-review loop in webjs-start-work is slow for a reason that has nothing to do with reviewing. A review round is a reading pass over the diff and costs seconds, but the skill never says what NOT to run around one, so a session ends up re-running multi-minute test suites between rounds and then waiting on CI on top of that.
Measured on PR #1156 (closed) from its own commit timestamps: six commits landed in five minutes, two of them 16 and 31 seconds apart, so committing and pushing cost nothing. The damage was a single 32-minute gap between two commits, spent running the full Node suite twice and the e2e suite twice, plus time watching gh pr checks --watch. None of it changed a review finding, and CI re-ran all of it anyway.
Two rules fix it.
Design / approach
Add exactly two rules to the self-review loop section. Keep the change small: this is a timing rule, not a coverage rule, and it must not weaken anything the Definition of done already mandates.
1. No test suite inside the loop. During a round, run only the specific test file(s) covering the line just changed (node --test <paths>), which also satisfies the repo's commit rule that a commit's own tests pass. Do not run the e2e suite, the full Node suite, the browser suite, the Bun matrix, or the four-app boot check between rounds. Those run once, after the last clean round.
2. Never wait on CI between rounds. Pushing is fire-and-forget: push, then start the next round immediately. No gh pr checks --watch mid-loop. Read CI once, after the last clean round. A round that finds something means another push anyway, which supersedes the run being waited on.
Explicitly NOT in scope, to keep this minimal: the required-vs-optional CI check list, subagent failure handling, and anything about what to do when a reviewer agent dies. Those are separate concerns and were deliberately dropped from this issue.
Implementation notes (for the implementing agent)
Where to edit:.claude/skills/webjs-start-work/SKILL.md (git-tracked). Add the two rules to the ### How the loop works section (starts L313). The natural anchor is right before that heading, or as bullets after step 1 (L317). Step 4 of that list ("If the round reports CLEAN, the loop is done", ~L343) is where the "suites run now" pointer belongs.
Landmine, this is the one that matters: an earlier attempt at this wording contradicted AGENTS.md "Code workflow" item 1 and SKILL.md L138, both of which REQUIRE running browser / e2e / Bun and reporting the result. Wording like "do not run a slow suite locally to confirm what CI checks anyway" forbids exactly what those mandate. The rule must change WHEN the suites run (once, at the end) and never WHETHER. State that explicitly in the text so a later reader cannot misread it as permission to skip a layer.
Second landmine: the same earlier attempt said "run NO test suite inside the loop, and not npm test either", which collides with the repo's commit rule (AGENTS.md "Git workflow" item 2: commit when the unit is complete and its tests pass). Word the carve-out so running the touched test file(s) per fix is clearly allowed.
Invariants: AGENTS.md "Code workflow" item 1 (tests at every applicable layer, Bun parity) and item 2 (docs) are untouched by this. Invariant 11 (no em-dash, no space-hyphen or space-semicolon as pause punctuation, WebJs capitalized in prose) applies to the new prose; the .claude/hooks/block-prose-punctuation.sh hook enforces it on write.
Tests / docs surfaces: none. This edits a Claude Code skill file, not shipped source, so no test layer applies and no docs-site or scaffold surface changes. test/hooks/route-skills.test.mjs asserts referenced skills are committed in-repo, so keep the file where it is.
Acceptance criteria
The loop section states that no full test suite runs between rounds, and names the excluded ones (e2e, full Node suite, browser, Bun matrix, four-app boot check)
It allows running the specific test file(s) for the fix just made, so the commit-when-tests-pass rule still holds
It states that the suites run once after the last clean round, and says plainly that this changes when they run, not whether
It states that CI is never waited on between rounds, and is read once after the last clean round
No contradiction with AGENTS.md "Code workflow" item 1 or SKILL.md L138 (grep both and confirm)
Both copies of the skill are identical afterwards (diff -q in-repo vs ~/.claude), with the in-repo copy treated as the newer one
Problem
The self-review loop in
webjs-start-workis slow for a reason that has nothing to do with reviewing. A review round is a reading pass over the diff and costs seconds, but the skill never says what NOT to run around one, so a session ends up re-running multi-minute test suites between rounds and then waiting on CI on top of that.Measured on PR #1156 (closed) from its own commit timestamps: six commits landed in five minutes, two of them 16 and 31 seconds apart, so committing and pushing cost nothing. The damage was a single 32-minute gap between two commits, spent running the full Node suite twice and the e2e suite twice, plus time watching
gh pr checks --watch. None of it changed a review finding, and CI re-ran all of it anyway.Two rules fix it.
Design / approach
Add exactly two rules to the self-review loop section. Keep the change small: this is a timing rule, not a coverage rule, and it must not weaken anything the Definition of done already mandates.
1. No test suite inside the loop. During a round, run only the specific test file(s) covering the line just changed (
node --test <paths>), which also satisfies the repo's commit rule that a commit's own tests pass. Do not run the e2e suite, the full Node suite, the browser suite, the Bun matrix, or the four-app boot check between rounds. Those run once, after the last clean round.2. Never wait on CI between rounds. Pushing is fire-and-forget: push, then start the next round immediately. No
gh pr checks --watchmid-loop. Read CI once, after the last clean round. A round that finds something means another push anyway, which supersedes the run being waited on.Explicitly NOT in scope, to keep this minimal: the required-vs-optional CI check list, subagent failure handling, and anything about what to do when a reviewer agent dies. Those are separate concerns and were deliberately dropped from this issue.
Implementation notes (for the implementing agent)
.claude/skills/webjs-start-work/SKILL.md(git-tracked). Add the two rules to the### How the loop workssection (starts L313). The natural anchor is right before that heading, or as bullets after step 1 (L317). Step 4 of that list ("If the round reportsCLEAN, the loop is done", ~L343) is where the "suites run now" pointer belongs.~/.claude/skills/webjs-start-work/SKILL.md. Two copies of this skill exist and they drift. The in-repo copy has historically been the NEWER one (it knowsdocsandpackages/ui/packages/websiteare redirect-only hosts, seo: serve the docs at webjs.dev/docs with the marketing site chrome #1098/seo: serve the UI gallery at webjs.dev/ui with the marketing site chrome #1099, which the global copy did not), so diff before copying and never blind-copy global over in-repo. Confirm identical afterwards withdiff -q.AGENTS.md"Code workflow" item 1 and SKILL.md L138, both of which REQUIRE running browser / e2e / Bun and reporting the result. Wording like "do not run a slow suite locally to confirm what CI checks anyway" forbids exactly what those mandate. The rule must change WHEN the suites run (once, at the end) and never WHETHER. State that explicitly in the text so a later reader cannot misread it as permission to skip a layer.npm testeither", which collides with the repo's commit rule (AGENTS.md"Git workflow" item 2: commit when the unit is complete and its tests pass). Word the carve-out so running the touched test file(s) per fix is clearly allowed.WebJscapitalized in prose) applies to the new prose; the.claude/hooks/block-prose-punctuation.shhook enforces it on write.test/hooks/route-skills.test.mjsasserts referenced skills are committed in-repo, so keep the file where it is.Acceptance criteria
AGENTS.md"Code workflow" item 1 or SKILL.md L138 (grep both and confirm)diff -qin-repo vs~/.claude), with the in-repo copy treated as the newer one