Problem
The webjs-start-work pre-merge review cycle blocks the "ready to merge" report on a CI read that buys nothing. Line 378 tells the agent to launch a background CI watch when the cycle finishes and collect it before reporting, line 240 makes "CI has been read green" part of the condition for saying "ready to merge", and line 390 lists the CI read as one of the three things that must precede the report.
That read is redundant. Merge is already gated on green CI at the branch level (line 400): main branch protection requires the five ci.yml checks, so a red or pending check cannot merge no matter what the report said. When the owner asks to merge, the agent has to read CI at that point anyway, and a red check gets fixed then. Reading it earlier does not change whether the check runs or whether a failure gets fixed. It only parks the cycle on a CI run at the exact moment the cycle is trying to end.
The cost is real: a full ci.yml run is minutes of wall clock added to every PR's critical path, on a cycle whose whole design goal (see #1180) is to economise on rounds and elapsed time.
Design / approach
Drop the CI read from the end of the review cycle. Line 400 becomes the ONLY place the skill reads CI, at merge time, where branch protection enforces it regardless.
What does NOT change: the deferred local suites still run at cycle end, all of them. The e2e, full Node, browser, and Bun matrix runs plus the two-app dogfood boot check prove the change itself, they are not a substitute for CI and CI is not a substitute for them. Only the CI read moves; every local layer still gates the ready signal exactly as it does today.
The tradeoff, stated so it is not rediscovered as a bug. CI covers three things the local suites do not: the conventions job running webjs doctor over examples/blog and website (#1257), a package-lock.json desync failing npm ci (SKILL.md L202), and the webjs.nvim vendored-intellisense drift guard (SKILL.md L151). Under this change those surface at merge time instead of before the report, which is a fix-then-merge round trip rather than a fix-before-report one. That is the accepted cost: the same work, later, off the critical path. The failure mode to avoid is an agent reading this and "restoring" the end-of-cycle read to close that gap. It was removed deliberately.
Alternative considered and rejected: keep a non-blocking CI read that reports whatever state CI happens to be in without waiting. Rejected because a report that says "CI is still running" is not information the owner acts on, and an agent that sees a pending check tends to wait on it anyway, which reintroduces exactly the block being removed.
Implementation notes (for the implementing agent)
Where to edit. One file, four lines, all in .claude/skills/webjs-start-work/SKILL.md:
- L240 (
## Pre-merge review cycle, opening paragraph): ends ... after the cycle has finished AND the suites it deferred have run AND CI has been read green. Drop the final AND CI has been read green clause. The deferred-suites condition stays.
- L347 (
**Keep the cycle fast.**): contains Never wait on CI between rounds; read it once at the end. Note this sentence uses a semicolon as pause-punctuation between clauses, which invariant 11 bans, so the replacement must not reuse that shape. Restate as: CI is not read during the cycle at all, it is read at merge time per the merge section. Keep the surrounding sentences about scoped test runs and the Both rules change WHEN work happens, never WHETHER. closer.
- L378 (
**When the cycle FINISHES, run everything it deferred:**): drop and only now read CI from the first sentence and plus a background CI watch from the second. The rest of the sentence is load-bearing and stays intact: the parallel background batch, collect EVERY result before reporting, the silently-skipped-layer warning, and the unfinished-cycle carve-out.
- L390 (
### Reporting after the cycle): reads After the final review (and its fix-check, if it had one), the deferred suites, and the CI read, report exactly this shape:. Drop and the CI read. The quoted report template underneath does not mention CI and needs no edit.
Add the rationale so it is not reverted. The merge-gate paragraph at L400 is where the reasoning belongs, since that paragraph becomes the sole CI touchpoint. Add a sentence there recording that CI is deliberately read only at merge, and that an end-of-cycle read is redundant against branch protection. Without this, the next agent to read L378 sees a cycle that never verifies CI and "fixes" it. The skill already uses this do-not-restore pattern in two places (the **Do not restore what this replaced.** paragraph at L349 and the two-review-minimum note under it), so match that voice rather than inventing a new one.
Landmines.
- The
**No machinery tells.** bullet at L256 already forbids a reviewer's PR prose from mentioning CI status. It is about review comment style, not about when CI is read, so it is unrelated and must not be touched. Do not fold the two.
- L400 and L402 are the merge gate and the
--admin prohibition. Both must survive this change completely unweakened. L402 exists because a red Unit-test job was once admin-merged and broke main. Removing the end-of-cycle read makes merge-time the only CI checkpoint, so anything that softens those two paragraphs turns a redundant check into no check.
- The skill is consumed through a symlink:
~/.claude/skills/webjs-start-work points at the repo copy. Edit the repo file only, never copy it into ~/.claude/skills/.
- Two stale agent worktrees under
.claude/worktrees/agent-*/ contain their own copies of this SKILL.md and will match a repo-wide grep for the old wording. They are not a doc surface. Leave them alone.
- Invariant 11 applies to the prose being written: no em-dash, no space-surrounded hyphen or semicolon as a pause between clauses, and
WebJs capitalized wherever it names the project. .claude/hooks/block-prose-punctuation.sh scans new content and will block the commit otherwise.
Invariants and rules to respect. AGENTS.md invariant 11 (prose punctuation and brand casing), the worktree rule (cut a worktree, this is a tracked file and .claude/hooks/require-worktree-for-edits.sh blocks tracked edits in the primary checkout), and the standing no-proactive-follow-up-issues rule.
Tests and doc surfaces. No test asserts this wording. test/hooks/review-loop-exit.test.mjs and test/hooks/route-skills.test.mjs were both checked and neither greps the CI sentences (route-skills.test.mjs only asserts every skill the router references is committed in-repo, which this change does not affect). No scaffold template, no packages/cli/templates/ rule file, and no docs-site page carries a copy of this text, so webjs-doc-sync has nothing to sync here. This is an agent-workflow doc, not public API surface, so .claude/hooks/require-docs-with-src.sh does not fire (no packages/*/src is staged).
Acceptance criteria
Problem
The
webjs-start-workpre-merge review cycle blocks the "ready to merge" report on a CI read that buys nothing. Line 378 tells the agent to launch a background CI watch when the cycle finishes and collect it before reporting, line 240 makes "CI has been read green" part of the condition for saying "ready to merge", and line 390 lists the CI read as one of the three things that must precede the report.That read is redundant. Merge is already gated on green CI at the branch level (line 400):
mainbranch protection requires the fiveci.ymlchecks, so a red or pending check cannot merge no matter what the report said. When the owner asks to merge, the agent has to read CI at that point anyway, and a red check gets fixed then. Reading it earlier does not change whether the check runs or whether a failure gets fixed. It only parks the cycle on a CI run at the exact moment the cycle is trying to end.The cost is real: a full
ci.ymlrun is minutes of wall clock added to every PR's critical path, on a cycle whose whole design goal (see #1180) is to economise on rounds and elapsed time.Design / approach
Drop the CI read from the end of the review cycle. Line 400 becomes the ONLY place the skill reads CI, at merge time, where branch protection enforces it regardless.
What does NOT change: the deferred local suites still run at cycle end, all of them. The e2e, full Node, browser, and Bun matrix runs plus the two-app dogfood boot check prove the change itself, they are not a substitute for CI and CI is not a substitute for them. Only the CI read moves; every local layer still gates the ready signal exactly as it does today.
The tradeoff, stated so it is not rediscovered as a bug. CI covers three things the local suites do not: the
conventionsjob runningwebjs doctoroverexamples/blogandwebsite(#1257), apackage-lock.jsondesync failingnpm ci(SKILL.md L202), and the webjs.nvim vendored-intellisense drift guard (SKILL.md L151). Under this change those surface at merge time instead of before the report, which is a fix-then-merge round trip rather than a fix-before-report one. That is the accepted cost: the same work, later, off the critical path. The failure mode to avoid is an agent reading this and "restoring" the end-of-cycle read to close that gap. It was removed deliberately.Alternative considered and rejected: keep a non-blocking CI read that reports whatever state CI happens to be in without waiting. Rejected because a report that says "CI is still running" is not information the owner acts on, and an agent that sees a pending check tends to wait on it anyway, which reintroduces exactly the block being removed.
Implementation notes (for the implementing agent)
Where to edit. One file, four lines, all in
.claude/skills/webjs-start-work/SKILL.md:## Pre-merge review cycle, opening paragraph): ends... after the cycle has finished AND the suites it deferred have run AND CI has been read green.Drop the finalAND CI has been read greenclause. The deferred-suites condition stays.**Keep the cycle fast.**): containsNever wait on CI between rounds; read it once at the end.Note this sentence uses a semicolon as pause-punctuation between clauses, which invariant 11 bans, so the replacement must not reuse that shape. Restate as: CI is not read during the cycle at all, it is read at merge time per the merge section. Keep the surrounding sentences about scoped test runs and theBoth rules change WHEN work happens, never WHETHER.closer.**When the cycle FINISHES, run everything it deferred:**): dropand only now read CIfrom the first sentence andplus a background CI watchfrom the second. The rest of the sentence is load-bearing and stays intact: the parallel background batch,collect EVERY result before reporting, the silently-skipped-layer warning, and the unfinished-cycle carve-out.### Reporting after the cycle): readsAfter the final review (and its fix-check, if it had one), the deferred suites, and the CI read, report exactly this shape:. Dropand the CI read. The quoted report template underneath does not mention CI and needs no edit.Add the rationale so it is not reverted. The merge-gate paragraph at L400 is where the reasoning belongs, since that paragraph becomes the sole CI touchpoint. Add a sentence there recording that CI is deliberately read only at merge, and that an end-of-cycle read is redundant against branch protection. Without this, the next agent to read L378 sees a cycle that never verifies CI and "fixes" it. The skill already uses this do-not-restore pattern in two places (the
**Do not restore what this replaced.**paragraph at L349 and the two-review-minimum note under it), so match that voice rather than inventing a new one.Landmines.
**No machinery tells.**bullet at L256 already forbids a reviewer's PR prose from mentioning CI status. It is about review comment style, not about when CI is read, so it is unrelated and must not be touched. Do not fold the two.--adminprohibition. Both must survive this change completely unweakened. L402 exists because a red Unit-test job was once admin-merged and brokemain. Removing the end-of-cycle read makes merge-time the only CI checkpoint, so anything that softens those two paragraphs turns a redundant check into no check.~/.claude/skills/webjs-start-workpoints at the repo copy. Edit the repo file only, never copy it into~/.claude/skills/..claude/worktrees/agent-*/contain their own copies of this SKILL.md and will match a repo-wide grep for the old wording. They are not a doc surface. Leave them alone.WebJscapitalized wherever it names the project..claude/hooks/block-prose-punctuation.shscans new content and will block the commit otherwise.Invariants and rules to respect. AGENTS.md invariant 11 (prose punctuation and brand casing), the worktree rule (cut a worktree, this is a tracked file and
.claude/hooks/require-worktree-for-edits.shblocks tracked edits in the primary checkout), and the standing no-proactive-follow-up-issues rule.Tests and doc surfaces. No test asserts this wording.
test/hooks/review-loop-exit.test.mjsandtest/hooks/route-skills.test.mjswere both checked and neither greps the CI sentences (route-skills.test.mjsonly asserts every skill the router references is committed in-repo, which this change does not affect). No scaffold template, nopackages/cli/templates/rule file, and no docs-site page carries a copy of this text, sowebjs-doc-synchas nothing to sync here. This is an agent-workflow doc, not public API surface, so.claude/hooks/require-docs-with-src.shdoes not fire (nopackages/*/srcis staged).Acceptance criteria
--adminprohibition) is untouchedgrep -n "CI" .claude/skills/webjs-start-work/SKILL.mdshows CI referenced in the review cycle only at the merge gate.claude/hooks/block-prose-punctuation.shon the new lines