Reconcile the two contradictory review-posting rules in the start-work skill

[vivek7405]

Problem

.claude/skills/webjs-start-work/SKILL.md states the review-posting rules twice, in two sections, and the two copies contradict each other on two separate points. An agent following the file top-down gets one answer; an agent that jumps to the self-review loop gets the opposite. Both were surfaced by a review round on PR #1159 (they predate that PR and are untouched by its diff).

Contradiction 1, which API to post findings through. L253 is emphatic that review content goes in ONE review object: "Both go in ONE review object, via the reviews API, never as plain issue comments", and "gh pr comment issue comments are reserved for NON-review context", closing with "If you catch yourself about to gh pr comment something that is really a review observation, fold it into the review summary instead." L349 instructs the opposite twice: post findings with gh api repos/webjsdev/webjs/pulls/<N>/comments --input comment.json, which creates a STANDALONE review comment not attached to any review object, and "For cross-cutting or round-summary notes, use gh pr comment <N> --body-file /tmp/pr-comment.md", which is exactly the issue-comment path L253 forbids for review content.

The difference is visible on the PR, which is why L253 cares: only a review object renders the reviewed these changes - N comments trail that groups a summary with its inline notes. Findings posted the L349 way appear as loose, ungrouped boxes.

Contradiction 2, whether a finding carries its resolution. L251 says each inline finding "states the PROBLEM only, the way a reviewer flags it before anyone has fixed it", and explicitly forbids the resolution: "Do NOT bake the resolution into the finding (ending a finding with '...Fixed.' is wrong). The resolution is recorded separately, as a threaded reply." L349 says "Each comment states the finding, its file:line, and its disposition (fixed in <sha> / rejected because <reason> / filed as #<n>)", which is precisely the baked-in resolution L251 bans.

Design / approach

L251 and L253 are the more specific and more recently written statements, they explain their own rationale (the review-object trail, the reviewer-then-programmer flow that mirrors GitHub's real UI), and they are what the loop actually needs. So the resolution direction is to make L349 defer to them rather than restate them.

Concretely: L349 should keep the parts that are genuinely its own (the trail must live on the PR and not only in the chat reply, findings must be recorded as part of the loop rather than as an afterthought, rejected findings and false positives get posted too, build the JSON with a real serializer never a shell string, invariant 11 applies to comment bodies) and replace its API-and-content instructions with a pointer to the "Every PR review is posted ON the PR" section.

Removing the duplicate is the point. Restating a rule in two places is what let these two drift, and PR #1159's review rounds found four more instances of exactly this pattern in the same file (the working-tree premise, the repo-health-check trigger, the Failure-handling entry, and the isolation bullet). A pointer cannot drift; a paraphrase will.

While there, check whether the filed as #<n> disposition still has a home. It is real and useful (the loop's step 2 requires an out-of-scope finding to be filed and its number captured), it just belongs in the threaded reply per L251, not in the finding.

Implementation notes (for the implementing agent)

• Where to edit: .claude/skills/webjs-start-work/SKILL.md, the paragraph beginning "Record every genuine finding as a comment ON THE PR" (currently L349, inside step 2 of ### How the loop works). The authoritative rules it must defer to are in ### Every PR review is posted ON the PR, currently L249 to L253, plus the reviewer/programmer flow at ### Follow the real review flow: reviewer, then programmer, both roles (L257 onward) which carries the working gh api -X POST repos/webjsdev/webjs/pulls/<N>/reviews --input review.json recipe and the GraphQL resolveReviewThread mutation. Line numbers will have shifted, so grep for the quoted phrases rather than trusting them.
• Mirror to ~/.claude/skills/webjs-start-work/SKILL.md. Two copies exist and they drift. The in-repo copy has historically been the newer one, so diff before copying and never blind-copy global over in-repo. Verify with diff -q afterwards. This is the same mirroring requirement Never let a dead review subagent stall or downgrade the loop #1158 carried.
• Landmine: do not "fix" this by deleting L251 or L253 and keeping L349's version. The L349 wording is the one that produces ungrouped comments, and a review posted that way on fix: stale hover prefetch degraded a click to a full page load #1115 lost every code reference and had to be deleted and reposted. The direction is L349 defers to L251/L253, not the reverse.
• Landmine: the gh api .../pulls/<N>/comments call at L349 is not simply wrong in isolation, it is the correct API for a standalone comment. What is wrong is using it for review-round findings. Keep that distinction clear so the rewrite does not overcorrect into forbidding a legitimate one-off inline comment.
• Landmine: PR docs: make the self-review loop fast, parallel, and stall-proof #1159 rewrote a neighbouring part of this same file (step 1 of the loop). Rebase on top of it, and re-read step 2 in the post-docs: make the self-review loop fast, parallel, and stall-proof #1159 state before editing, since docs: make the self-review loop fast, parallel, and stall-proof #1159 also touched what counts as a round and what gets posted for a round that did not happen (a round whose reviewer did not review posts nothing at all).
• Invariants: AGENTS.md invariant 11 applies to the new prose (no em-dash, no space-hyphen or space-semicolon as pause punctuation, WebJs capitalized when naming the project). .claude/hooks/block-prose-punctuation.sh enforces it on write.
• Tests / docs surfaces: none beyond the file itself. This edits a Claude Code skill, not shipped source. test/hooks/route-skills.test.mjs asserts every skill the router references is committed in-repo, so keep the file at its current path and run that test.

Acceptance criteria

• Only one place in the file states which API review findings are posted through, and it is the reviews-object rule
• Only one place states whether an inline finding carries its disposition, and it is the problem-only rule with the resolution in a threaded reply
• The step-2 paragraph points at the authoritative section instead of paraphrasing it
• The genuinely-unique content of the step-2 paragraph survives (trail on the PR not just the chat, record during the loop, post rejected and false-positive findings, serialize the JSON properly, invariant 11 applies to comment bodies)
• filed as #<n> still has a documented home for an out-of-scope finding
• grep finds no remaining instruction to use gh pr comment for review-round content
• Both copies of the skill are identical afterwards (diff -q in-repo vs ~/.claude), in-repo treated as newer
• node --test test/hooks/route-skills.test.mjs passes