Skip to content

docs(pm): the isolated review subagent writes under a round-unique scratch path and never trusts a file it did not write this turn (#18061, half A) - #18094

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-18061-review-subagent-scratch-isolation
Sep 14, 2026
Merged

os-project-manager merged 1 commit into
mainfrom
claude/issue-18061-review-subagent-scratch-isolation

Conversation

@claude

@claude claude Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #18061

Half A only. Half B (the check-reference-carrier-shape blind spot) is excluded per triage 5657095122 — see Acceptance notes. Nothing here measures, asserts or touches it.

What the card measured

One session's concurrently dispatched subagents share ONE flat scratchpad directory. Generically-named files — pr.json, card.json, the card_comments.json / card-comments.json pair — written minutes apart by sibling rounds overwrite each other silently and well-formed: a reviewer that re-read pr.json would have reported a confident verdict about the wrong diff. The card corrects its own reporting reviewer: cross-session isolation held (exactly one session directory exists under the scratchpad root), so the scope is session-specific but not agent-specific — a seat collides with its own siblings.

The change — one rule line, in the review subagent's order

.claude/skills/pm-dispatch/references/contract-review.md, immediately after the two lines that already spell that order out (when to spin an isolated reviewer up; what to feed it):

  • 隔离复核子代理暂存全写按所审 PR 命名的 SCRATCHPAD/pr-N/,⛔ 不读非本轮自写的暂存。

Two clauses, both from the triage's ⛔ Scope for half A:

  1. a round-unique scratch directory named by what is being reviewed — the shape .claude/agents/os-dev.md already mandates dev-side as issue-N/, applied to the PR;
  2. ⛔ never read a scratch file you did not write yourself in the same turn — the half that survives any path scheme.

⚠️ The path is spelled SCRATCHPAD/pr-N/ in this body only. The file itself carries the repo's ordinary angle-bracket placeholder spelling, and every quotation below respells it the same way (issue-N/, batch:N, --test PATHS); angle-bracket fragments do not survive this surface intact (AGENTS.md, GitHub mutates body BYTES).

Paying for the line — the file was at its ceiling

check:pm-skill-ratchet before the change, 2026-09-14T01:52Z, at f27e86b08:

✓ check-skill-line-ratchet: .claude/skills/pm-dispatch/references/contract-review.md: widest table row is 0 bytes (pin 0; headroom 0).
✓ check-skill-line-ratchet: .claude/skills/pm-dispatch/references/contract-review.md is 60 lines (ceiling 60; headroom 0).

Headroom 0, so the line is paid for by a density merge, not by deleting a rule. The two adjacent per-item lines ② and ③ of the review record became one:

-- ② semver 定级:变更级别与 changeset 声明一致。
-- ③ 边界旗处置:dev 挂旗与 `open_questions` 逐旗答复或升级。
+- ② semver 定级与 changeset 声明一致;③ 边界旗:dev 挂旗与 `open_questions` 逐旗答复或升级。

Every clause survives. ② keeps semver 定级, and 变更级别 is the referent of 定级 — the grading IS the change level, so the agreement clause against the changeset declaration is intact. ③ loses only the head noun 处置, whose content is the body that follows it (逐旗答复或升级). ⛔ No rule is dropped, and no wrap is counted as a line.

check:pm-skill-ratchet after the change, 2026-09-14T02:03Z, at 7ded5bad7 — exit 0:

✓ check-skill-line-ratchet: .claude/skills/pm-dispatch/references/contract-review.md: widest table row is 0 bytes (pin 0; headroom 0).
✓ check-skill-line-ratchet: .claude/skills/pm-dispatch/references/contract-review.md is 60 lines (ceiling 60; headroom 0).

Widest line in the file is unchanged at 120 bytes (the 120-byte per-line rule the ratchet enforces for non-table lines); the new rule line measures 118 bytes and the merged line 115.

The concurrency-budget half of the subject already landed

The triage notes that a path scheme alone does not address the lock starvation the same wave caused. ⛔ No budget rule is added here, because two already exist and are quoted below as they stand today at f27e86b08 (the angle-bracket placeholder in the first is spelled N for this surface only):

  • .claude/skills/pm-dispatch/SKILL.md:60| batch:N| 同时在飞的 dev 上限 | 默认2;n的维护者天花板5 |
  • .claude/skills/pm-dispatch/SKILL.md:436- 第 N 单派发前读 scripts/pm/os-verify-lock.sh --status:到达深度 ≥ LOCK_DEPTH_HOLD(= 2)即等。

(references/core-rules.md:11 carries the same pair in one line.) Both landed in 7ef05f997 as write-identity lock 3.

Premise readings

Taken in the worktree at f27e86b08 before the first edit.

P1 — the dev half of the remedy is already landed. TRUE. 2026-09-14T01:50:36Z, sed -n '30,35p' .claude/agents/os-dev.md:

32:   - Scratchpad 按 issue 隔离:在 scratchpad 目录下建 `issue-N/` 子目录,临时文件全写进去。
33:   - 同批 agents 共用一个 scratchpad 目录,自然命名的文件会被彼此静默覆盖。

⇒ ⛔ os-dev.md is not touched by this PR.

P2 — the colliding rounds had no rule to follow. TRUE. Same timestamp, grep -c -i scratch:

.claude/skills/pm-dispatch/references/contract-review.md : 0
.claude/skills/pm-dispatch/SKILL.md : 0
.claude/skills/pm-dispatch/references/dispatch-runbook.md : 0
.claude/agents/os-dev.md : 2      ← lit control, hits at :32 and :33

The lit control rules out a broken matcher. And the runbook is not where the review subagent's order is templated: its only mention of this review points away, at :161无主阻塞项的契约面(含 packages/spec)走 contract-review.md 独立性件的隔离达档复核 — and its ## 派发词构造细则 section templates the dev dispatch word, not the reviewer's brief. ⇒ ⛔ dispatch-runbook.md is not touched either; contract-review.md is the whole surface.

P3 — the file is at its ceiling. TRUE. 2026-09-14T01:52Z; the two ratchet rows quoted above, headroom 0 on both. ⇒ the density merge is mandatory, not stylistic.

Verification

Gate families — derived in the worktree, run with exits captured by redirect-then-capture (⛔ never through a pipe), recorded and reconciled:

node scripts/pm/dispatch-gates.mjs --ran /tmp/ran-18061.txt at 7ded5bad7:

✓ dispatch-gates --ran: 15 derived famil(ies) accounted for — 15 run, 0 NOT-MEASURED
  (a DERIVED zero — all 15 recorded an exit code and none of them is 3).

All 16 commands (the 15 derived plus pnpm check:pm-governed-prose, recorded as outside the derivation) exited 0, including check:pm-skill-ratchet, check:skill-frame-sync, check:pm-governed-merges, check:pm-skill-id-lint, check:nul-bytes, check:doc-authoring.

⚠️ One of them needed a second run to produce a reading at all. pnpm --filter @objectstack/lint run check:doc-formula-expressions first exited 3PREREQUISITE NOT MET, its own text saying "Nothing was measured … It is NOT a finding". That is NOT MEASURED, ⛔ not a failure. Its prescribed fix (turbo run build --filter=@objectstack/formula --filter=@objectstack/lint) was run under the shared verify lock (os-verify-lock: VERDICT command-exit 0 · held the lock 2s · waited 0s), after which the gate exited 0.

Governed-surface tier. node scripts/pm/check-governed-merges.mjs --test .claude/skills/pm-dispatch/references/contract-review.md — exit 3 (the deliberate GOVERNED code, ⛔ not a finding):

governed-surface predicate: 1 of 1 path(s) hit the register (5 surfaces, repo-agnostic).
  ⛔  GOVERNED — a human merge is the review record for this PR (#9495 regime).
      .claude/** ×1 — the agent instruction tree (skills, agents, hooks, settings)

Landing tier, read from scripts/pm/check-governed-queue-guard.mjs's own exported predicate:

REFERENCES_TIER_PREFIX = .claude/skills/pm-dispatch/references/
governedTierFor(['.claude/skills/pm-dispatch/references/contract-review.md']) = references

⚠️ Note for the dispatching seat: the --test PATHS predicate lives on check-governed-merges.mjs, ⛔ not on check-governed-queue-guard.mjs — the latter reads GITHUB_EVENT_PATH and exits 1 with "could not look must never exit 0 here" when given a path. The tier above is computed from its exported governedTierFor, which is the same decision.

Lint — a proven narrowing, not a skipped run. The repo-wide pnpm lint is CI's run. The narrowing here is total, and all three readings are present:

  1. Population, read from eslint's own config (⛔ not guessed): ESLint#isPathIgnored('.claude/skills/pm-dispatch/references/contract-review.md')true; lit control isPathIgnored('scripts/pm/check-skill-line-ratchet.mjs')false.
  2. File count, from --format json: eslint THAT-FILE --no-inline-config --format json → exit 0, errorCount: 0, one warning reading "File ignored because no matching configuration was supplied."zero lintable files in this diff.
  3. Immutability for untouched files: the diff is one markdown file that eslint's flat config does not match at all, so no untouched file's verdict can move. Type-awareness does not enter — the file is never parsed.

Control characters. grep -naP over the changed file for the C0/DEL set: no hits (exit 1). pnpm check:nul-bytes exit 0.

Changeset: none owed, and the label is the declaration. changeset-check (.github/workflows/pr-automation.yml) declares exactly two exemptions — the skip-changeset label, and the changesets release PR pinned by branch and author. There is ⛔ no path exemption, so .claude/** earns no automatic pass: the label IS the declaration, and it is applied on this PR. The substantive test is satisfied independently — nothing published moves, .claude/** ships in no package's files[].

Acceptance notes

  • Half B is excluded per triage 5657095122 — 「⚠️Do not let half B ride along on half A's PR.」 Different lane, different surface; one half is measured and the other is a hypothesis. ⛔ Nothing here measures, asserts or touches packages/lint/scripts/check-reference-carrier-shape.mjs, and this PR's closing keyword is written as instructed by the dispatching seat.
  • The concurrency-budget half of the subject already landed and is quoted above rather than re-legislated: batch default 2 and the LOCK_DEPTH_HOLD (= 2) arrival-depth wait, both from 7ef05f997.
  • noted, not filed: the dispatch word named check-governed-queue-guard.mjs --test PATHS; that flag is on check-governed-merges.mjs. A tooling-usage note for the dispatching seat, ⛔ not a repo defect — neither script is wrong. Carrier: this PR's reviewing seat.
  • noted, not filed: this session's own scratchpad shows the card's exact shape — 499 flat entries at the root, and the name issue-18061 is already taken by a file another round wrote, so the per-issue subdirectory .claude/agents/os-dev.md mandates could not be created under that name. This round wrote under dev-18061/ instead and re-fetched the card body itself rather than trusting the sibling's file. Live confirmation of the finding, ⛔ not a second finding. Carrier: this card.

Clause-②: no.claude/skills/pm-dispatch/references/** is internal dispatch doctrine, not published skills/**; the diff makes no falsifiable operator or contract-semantics claim, adds no exported symbol and no key on a published payload.

维护者速读(草稿)

改了什么。 席内契约复核细则加一条:隔离复核子代理的临时文件必须写进按所审 PR 命名的独立目录,且不读本轮不是自己写的临时文件。文件正好卡在 60 行天花板上,这一行由把复核记录 ②③ 两条并成一条来支付——两条的每个子句都还在,⛔ 没有删规则。

为什么改。 一个席位同时派五个复核子代理时,它们共用同一个扁平暂存目录,pr.json 这种通名文件被兄弟轮次静默覆盖。失效形态是静默且格式完好:复核者拿到的是另一个 PR 的 payload,却会给出一个自信的裁决——受管面复核上的「自信的错答案」,⛔ 不是工具小毛病。跨会话隔离本身没坏,坏的是同一会话内部。

风险与代价(含回滚)。 代价是复核子代理多建一层目录。风险面只有一处:②③ 合并后阅读密度变高,但两条的操作性内容一字未少。回滚 = git revert 单个 commit,受管面无运行时影响,不发布任何包,无 changeset。

席位意见。 (留空,待席内定稿)

你要做的。 这是受管面(.claude/**),按 Prime Directive #14 只能由你手动合并:⛔ 不入队、⛔ 不挂 auto-merge、⛔ 不翻出 draft。落地档位为 references。请确认那条新规则的中文措辞与本文件的机读语域一致,以及 ②③ 合并是否可接受。


Generated by Claude Code

The in-session review subagent's order in `references/contract-review.md`
said nothing about where it writes its scratch files. One session's
concurrently dispatched subagents share ONE flat scratchpad directory, so
generically-named files (`pr.json`, `card.json`, …) written minutes apart
by sibling rounds overwrite each other silently and well-formed — a
reviewer re-reading such a file reports a confident verdict about the
wrong diff.

Add one rule line to that order: every scratch file goes under a
round-unique directory named by what is being reviewed
(`<scratchpad>/pr-<n>/`, the shape `.claude/agents/os-dev.md` already
mandates for the dev side as `issue-<n>/`), and never read a scratch file
you did not write yourself in the same turn.

The file is at its `check:pm-skill-ratchet` ceiling (60/60, headroom 0),
so the line is paid for by a density merge of the two adjacent per-item
lines ② and ③ of the review record; every clause of both survives.

Claude-Session: https://claude.ai/code/session_01DAcomhvR9kKizeYgg89Vo8
Co-authored-by: Claude <noreply@anthropic.com>
@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author
  • Served-tier: 1750/1750 CONTRACT_REVIEW_TIER — harness model stamp counted over this seat's own transcript (non-sidechain assistant messages a model served; <synthetic> harness notices excluded) at 2026-09-14T02:05Z and compared to the constant's value outside the repository; get_session external_metadata.last_served_model read equal to the constant at 2026-09-14T00:20Z.

Contract review

Head: 7ded5bad (PR #18094, card #18061 — HALF A) — reviewed at 2026-09-14T02:05Z by the skills seat at the contract-review tier. Governed references tier (references/contract-review.md only; governedTierFor → references) ⇒ this in-seat record lands it on the merge-group leg (PR #18036's tier): ready + auto-merge by this seat.

① derived judgments (seat-measured on the fetched head, ⛔ not taken from the report):

  1. The rule line carries both of triage's scope items: 「隔离复核子代理暂存全写按所审 PR 命名的 <scratchpad>/pr-<n>/,⛔ 不读非本轮自写的暂存。」 — the round-unique path named by what is reviewed (the os-dev.md issue-<n>/ shape applied to the PR) and the same-turn trust discipline; 118 bytes under the file's unchanged 120-byte widest.
  2. Paid for without losing a clause: ② and ③ merged into one line — 「② semver 定级与 changeset 声明一致;③ 边界旗:dev 挂旗与 open_questions 逐旗答复或升级。」 — both rules survive verbatim in substance (seat-read against the two removed lines); 60/60, check-skill-line-ratchet exit 0 on the head (archive with the governed tree present).
  3. Surface held to the file that lacked the rule: os-dev.md untouched (its :32–:33 already carry the dev half — P1); SKILL.md untouched (the concurrency-budget half of the subject is lock 3's batch default 2 and the verify-lock depth wait, quoted in the PR body); dispatch-runbook.md read and found to template the dev word only.
  4. Gates: 15 derived / 15 run / 0 NOT-MEASURED / 0 UNRUN + check:pm-governed-prose, all exit 0; one exit-3 prerequisite (check:doc-formula-expressions) built under the shared lock and re-run to 0. Checks on the head at 2026-09-14T02:05Z: 0 red at read time.
  5. --pair 18094 on origin/main's reader → exit 0 at 2026-09-14T02:05Z before this record; Clause-②: no holds — no contract path.
  6. Half B is not closed by Fixes #18061: the seat filed it as its own bare card [finding] check-reference-carrier-shape blind spot for a JSON-Schema properties map and its literal-only predicate — REPORTED, NOT VERIFIED; a reading is owed (half B of #18061) #18095 (reported-not-verified lead, the settle recipe, triage's domain:spec routing note) — nothing is lost when the card closes.

② semver: unchanged — nothing published; skip-changeset is the declaration.

③ boundary flags: none binding. Two readings for the seat, not the tree: (a) the dev measured this session's scratchpad live — 499 flat entries, and issue-18061 already taken by a sibling round's FILE, so the per-issue directory could not be created under that name; it wrote under dev-18061/ and re-fetched the card rather than trusting the sibling's copy — the card's hazard, confirmed in the act; (b) the --test PATHS predicate the dispatch named lives on check-governed-merges.mjs, not the queue guard (which reads GITHUB_EVENT_PATH) — a seat tooling-usage note.

Implemented-by: claude/issue-18061-review-subagent-scratch-isolation
Reviewed-by: session_01DAcomhvR9kKizeYgg89Vo8

Verdict: PASS — ready + auto-merge by this seat.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xs skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants