fix(pipeline): give every scratchpad write a per-run namespace (§SP) (#3718)

[usirin]

Fixes #3718

The defect

Two concurrent reviewers shared the scratchpad leaf prref.txt. The #3698 reviewer overwrote it mid-run, and the #3703 reviewer's git diff silently returned #3698's file list. Nothing errored — a clobbered file reads back successfully, just with another run's content — so a reviewer can post a verdict describing a diff it never looked at. Verdicts are the pipeline's routing gate, so that surfaces as a false PASS on unreviewed code. Silent by construction: there is no read error to catch, which is why a defensive "re-check after read" cannot cover it.

The fix — the class, not the two filenames

New §SP. The per-run scratchpad namespace in claude-plugins/kampus-pipeline/skills/gh-issue-intake-formats.md — the single-sourced convention, sitting alongside §CP/§DOC/§ZS/§RO.

The namespace must satisfy two requirements, and a design that meets only one is broken:

1. Per-run uniqueness — concurrent agents must not be able to name each other's files. (The point of Concurrent reviewer agents collide on shared scratchpad filenames — a diff silently returned the WRONG PR's files #3718.)
2. Deterministic re-derivability across Bash calls — an agent's shell state does not survive between Bash calls, so a later call must recompute the path from scratch.

So the namespace is keyed on $CLAUDE_CODE_SESSION_ID, not a mktemp -d:

RUN_SCRATCH="${TMPDIR:-/tmp}/kampus-run/$CLAUDE_CODE_SESSION_ID/<slug>"

A bare mktemp -d gives (1) and destroys (2) — re-running it in a later call yields a new empty directory and recovers nothing. The session id gives both: verified stable across Bash calls, and distinct per agent run (one pane's scratch tree holds many separate session dirs, so concurrent agents never share one). Skills open the namespace once, with a reset that clears a previous run of the same slug, and re-derive it with the same one-line recipe in every later call.

§SP states four numbered rules in preference order — (1) prefer no file at all, (2) derive from $RUN_SCRATCH, fail-closed, (3) never park the path in another file, recompute it, (4) the single-Bash-call mktemp carve-out — plus the never-leak-the-path corollary and the fail-open -nt hazard.

Rule 4 is deliberate, not a tolerated exception. A raw $(mktemp …XXXXXX) (or a throwaway $(mktemp -d)) is compliant by construction when allocated and consumed inside one Bash call — the kernel supplies the uniqueness and nothing has to survive. The line is lifetime, not file count. This also keeps §SP consistent with §RO, which mandates exactly that shape for a gate's throwaway head worktree.

The generative property is the point: uniqueness lives in the directory, so leaf names stay plain (deps.md) and a scratch write added later inherits the guarantee. Renaming prref.txt would not have done that.

Same-commit adoption

§SP is authored and cited by its consumers in the same commit. Re-rooted onto the deterministic namespace: plan-epic (the whole /tmp/plan-epic-<EPIC>-* family, across 5 blocks), write-code (progress, epic handoff, repair-progress), triage + close-not-planned, review-skill (WT_FILE, a genuine cross-call carrier). Collapsed to a §SP citation: review-code, review-doc, review-skill, report, ship-it. Standing invariant added to all 8 crew agent definitions — including what to do when state must cross a Bash call, which for an agent is the dominant case.

ship-it's two sites are rule-4 (allocate-and-consume in one block) and stay that way, renamed off the kampus-run prefix so the two forms stay tellable apart.

plan-epic's splice guard failed open — fixed

[ existing -nt missing ] is true in bash, so if ! [ "$deps" -nt "$current" ] passed silently when the base was gone — the guard read green precisely when its precondition was most broken, letting a stale block splice into an epic body. Step 5 now re-derives the namespace, asserts Step 1's snapshot is present, and checks -s before comparing mtimes.

Verification

• Cross-call recovery, end to end: call 1 opened the namespace and wrote state; a separate call 2 saw $RUN_SCRATCH empty (state loss confirmed) yet recovered every file via the re-derive recipe and evaluated the freshness guard correctly. The old mktemp -d form re-allocated an empty dir, recovering 0 files.
• $$ vs the session id: $$ changed between two Bash calls while $CLAUDE_CODE_SESSION_ID held — independently confirming the original $$ diagnosis.
• Splice guard table-tested across 7 cases: trips on the missing-base case that previously passed silently, and on stale/empty blocks; proceeds only on the two legitimate ones.
• validate-skills.sh — OK, 26 skills valid. Pre-commit leak-guard clean.

Follow-up

The pipeline-cli lint guard for fixed-name scratch writes is filed as #3735 — not re-filed here.

§CP — banks for human approval

This touches the control plane (pipeline skills + crew agent definitions), so it will not auto-merge and banks for human approval per ADR 0048's single-merge-authority. Not a candidate for the autonomous drain path.

[usirin]

review-skill: advisory — BLOCKING (§CP control plane, manual merge). Verdict: do not merge as-is.

Reviewed-head: @ d7dbe9c

Classified from the authoritative CONTROL_PLANE_RE on origin/main: all 17 changed files sit under claude-plugins/kampus-pipeline/agents/ or the named §CP skills, so this is control-plane work and lands via manual merge, not the auto-merge lane. Class fan: has-skills only (no path matches HAS_CODE_RE; every path is carved out of HAS_DOCS_*; no apps/web/src/ path, so no review-design).

The intent is right and the diagnosis in #3718 is correct — a clobbered scratch file reading back cleanly as another run's content is exactly the failure worth designing against. But this diff has two independent blockers, one of which is structural.

---

B1 — BLOCKER: §SP does not exist. All 39 citations dangle.

The PR's entire premise is a new shared section §SP in claude-plugins/kampus-pipeline/skills/gh-issue-intake-formats.md. That file is not in the diff, and the section is not at head.

$ git diff origin/main..d7dbe9c -- claude-plugins/kampus-pipeline/skills/gh-issue-intake-formats.md
(empty)
$ git show d7dbe9c:.../gh-issue-intake-formats.md | grep -c '§SP'
0

Meanwhile all 17 changed files cite it — 39 references, including 8 agent defs that end with "single-sourced in the skills' gh-issue-intake-formats.md §SP." Every one resolves to nothing.

This inverts the repo's same-commit adoption rule: the rule is that a new shared convention must be cited by its consumers in the commit that introduces it. Here the consumers landed and the convention did not. The net effect is worse than the status quo — the diff deleted working local rationale (plan-epic's epic-scoping note, review-doc's collision explanation) and replaced it with a pointer into the void. An agent following review-skill today is told to obey "§SP rule 3" and has no way to learn what rule 3 is.

Point 5 sub-finding, in the PR's favour: the three collapses in review-code / review-doc / review-skill did not lose rule content — they each kept their local mktemp/non-${PR}-keyed rule inline and added a §SP pointer plus the "reads back successfully" insight. Those three are well-formed and would be fine once §SP exists.

B2 — BLOCKER: the re-rooting breaks cross-Bash-call state in triage, plan-epic, and write-code.

This is the blast-radius risk, and it landed. plan-epic states the hazard and then contradicts itself:

"Open the run by allocating it, and re-derive it inside any later Bash call — shell state doesn't survive between calls"

Re-deriving RUN_SCRATCH="$(mktemp -d …)" in a later call yields a new empty directory. It does not recover the first call's files. The old paths (/tmp/plan-epic-<EPIC>-updated-at.txt) were deterministically re-derivable across calls — that was their function, and it was traded away for uniqueness with no replacement carrier.

Concrete breakage in plan-epic — Step 1 (L245-247) writes snap.json / current.md / updated-at.txt; Step 5 (L822-847, a separate fenced block) reads them:

• WAS=$(cat "$RUN_SCRATCH/updated-at.txt") → file absent → WAS empty → the TOCTOU freshness comparison against NOW is meaningless.
• if ! [ "$RUN_SCRATCH/deps.md" -nt "$RUN_SCRATCH/current.md" ] → fails open. Verified in bash: [ existing -nt missing ] is TRUE, so the negation is false and the guard silently passes.

$ bash -c 'if ! [ /tmp/sp-a.txt -nt /tmp/sp-missing.txt ]; then echo TRIPS; else echo "PASSES SILENTLY"; fi'
PASSES SILENTLY

That guard exists to refuse splicing a stale block into an epic body. This diff converts it from a working anti-clobber protection into a no-op — replacing a rare cross-run clobber with a deterministic every-run one on the same surface.

Same class elsewhere:

• triage L389-391 vs L~404 — RUN_SCRATCH allocated in one fenced block; the next block does BODY="$(cat "$RUN_SCRATCH/body.md")". Different Bash call → unset → reads /body.md.
• write-code L1636-1641 (handoff) and L1976-1981 (repair) — each allocates a fresh empty dir and on the very next line cats a file that was never written into it. As written these post an empty comment body. (The progress site at L1564-1576 is the one that got this right, via the # …write the four-section comment to "$RUN_SCRATCH/progress.md" note — that note is missing at the other two.)
• The 8 agent-def invariants tell agents to allocate RUN_SCRATCH but say nothing about the reset-between-calls problem, which for an agent is the dominant case. The guidance is unusable for any state spanning calls.

ship-it is the counter-example that works: both of its changes allocate and consume inside a single block. No cross-call exposure.

---

The other four questions

2. Fail-closed allocation — PASS. Every one of the 15 allocation sites is RUN_SCRATCH="$(mktemp -d …)" || { … exit 1; } (or || exit 1 in ship-it's rehearsal). No :- default, no || falling through to a literal path, no shared-path resurrection anywhere. The coder's claim holds.

3. The single-file mktemp carve-out — sound in principle, unverifiable here. mktemp /tmp/review-code-verdict.XXXXXX genuinely is kernel-unique; there is no correctness reason to force those under a dir. The re-breed risk is real but modest, and the right mitigation is exactly the one claimed — state it explicitly in §SP. Since §SP doesn't exist, the mitigation isn't there, so today this is a second undocumented convention. Resolves with B1.

4. The $$/PID replacement — PASS, reasoning verified. $$ in /tmp/shipit-flag-consts.$$ is the shell's PID, and two ship-it runs sharing an agent shell carry the same value; the file is written and read inside one $( … ) group, so a collision reads back clean and mis-decides the dark-ship branch. Replacing it with mktemp is strictly safer, and it's allocate-and-consume in one block so B2 doesn't touch it. Minor nit: CONSTS_FILE is a file named kampus-run.XXXXXX, the dir convention's name — worth a distinct prefix.

5. Same-commit adoption — FAIL (see B1). Collapses lost no content.

6. AC 4 — not a blocker on its own; I agree with the coder here, on narrower grounds than offered. AC 4 asks for a demonstration that a previously-clobbering scenario no longer reads back another run's content. These are agent-executed prose surfaces with no executable harness, so a live demo isn't available at this tier, and the honesty of the disclosure isn't what carries it — what carries it is that the AC is structurally discharged: a mktemp -d namespace makes the collision unrepresentable rather than unlikely, which is a stronger result than a passing scenario test. The proposed pipeline-cli lint guard is the right follow-up and should be filed as its own issue before merge, not left in a PR comment. ACs 1-3 are discharged in design; AC 2 is discharged in breadth. Note ACs 1-3 are undermined in practice by B2, not by AC 4.

---

To clear this

1. Author §SP in gh-issue-intake-formats.md in this same commit — the rule, the fail-closed allocation, the never-leak-the-path corollary, the numbered rules (review-skill cites "rule 3", write-code cites the "first rule"), and the explicit single-file-mktemp carve-out.
2. Resolve the cross-call carrier. The honest options: keep a deterministic re-derivable root for state that must span calls (e.g. a per-run root keyed on a token that does survive, like $CLAUDE_CODE_SESSION_ID — which write-code already used pre-diff and this PR removed), or restructure the affected steps so allocate-and-consume live in one Bash call. Re-derive-by-mktemp is not an option; it cannot work.
3. Specifically restore working behaviour for plan-epic's Step 1→Step 5 TOCTOU chain (the -nt fail-open is the sharpest edge), triage's two-block split, and write-code's handoff + repair sites.
4. Give the 8 agent-def invariants a sentence on what to do when state must cross a Bash call.
5. File the lint-guard follow-up issue.

I gate and verdict only — I don't merge, and §CP merge authority isn't mine.

[usirin]

review-skill: advisory — both prior blockers DISCHARGED. No blocking findings at this head. (§CP control plane → manual merge; not the auto-merge lane.)

Reviewed-head: @ bb21a70
Prior verdict: #3733 (comment) (@ d7dbe9c, BLOCKING)

Class fan re-resolved from live main: has-skills only — all 18 files sit under claude-plugins/kampus-pipeline/{skills,agents}/; nothing matches HAS_CODE_RE, every path is carved out of HAS_DOCS_*, no apps/web/src/ path so no review-design. One required namespace, review-skill, covered here. §CP per the authoritative CONTROL_PLANE_RE on origin/main (agents/, gh-issue-intake-formats.md, and the named §CP skills all match).

---

B1 — §SP exists and the citations resolve. DISCHARGED.

## SP. is at gh-issue-intake-formats.md:1852 and is substantive, not a stub. I re-walked every rule-number citation rather than checking the section's presence:

Citation	Cites	Resolves?
report/SKILL.md:156 "first and strongest rule … prefer no file at all"	rule 1	Yes — rule 1 is verbatim "Prefer no file at all", and names report (#2002) as its exemplar
plan-epic/SKILL.md:236 "re-derive … per §SP rule 3"	rule 3	Yes — rule 3's remedy clause mandates recompute
review-skill/SKILL.md:87 "§SP rules 2+3 apply, NOT the rule-4 carve-out"	rules 2+3, 4	Yes
ship-it/SKILL.md:1389, :1813 "§SP rule 4 / rule-4 carve-out"	rule 4	Yes — both are allocate-and-consume in one block

The named-in-the-prior-verdict "§SP rule 3" now lands on a real rule that says what the citing sites assert. No dangling references remain.

B2 — the re-key onto $CLAUDE_CODE_SESSION_ID. DISCHARGED, both claims verified first-hand.

I did not take the coder's word on either behavioral claim.

Stable across Bash calls — confirmed. Five separate Bash calls in this review returned the identical value while $$ moved (43261 → 59844 → 72010 → 72578). It is a real exported env var (env | grep finds it) and is inherited by child processes.

Distinct per agent run — confirmed empirically. Ten agent runs on this machine each hold a distinct session-keyed scratch namespace, including five sibling agents under one pane (cartographer, chief-of-staff, intake-desk, and two spawned subagents) with five different ids. My own id as a spawned subagent differs from both enclosing run ids. No transcript carries subagent entries sharing a parent's sessionId. Honest scoping: this is strong convergent evidence from the harness layout plus my own environment, not a spawn-two-siblings-and-diff experiment, which I cannot run from inside a gate.

Unset / empty → fails closed, verified by execution:

OPEN form  [ -n "${V:-}" ] || exit 1     UNSET → exit 1   EMPTY → exit 1   SET → 0
RE-DERIVE  "${V:?§SP: session id unset}" UNSET → exit 127  EMPTY → exit 127 SET → 0

:? (not ?) is the right operator — it rejects empty as well as unset, prints the diagnostic, and never reaches the path. The fail-open sibling ${V:-} would silently collapse to /tmp/kampus-run//slug, a shared path; I swept all 20 session-id interpolations in the diff and that form appears nowhere in a scratch path. Every bare $CLAUDE_CODE_SESSION_ID in a RUN_SCRATCH= assignment is immediately preceded by its -n guard (:1901→1903, plan-epic:240→242, :812→814, triage:392→394).

CI / non-agent contexts: the variable is absent there, so these blocks would hard-exit 1. That is the correct direction (ADR 0092) and is unreachable in practice — these are agent-executed prose blocks; the CI guards are pipeline-cli node jobs that touch none of them.

The previously fail-open splice guard — re-run, not accepted from the table

I rebuilt the guard from plan-epic/SKILL.md:877-880 and table-tested it myself:

1 deps-missing        → TRIP    (this is the case that PASSED SILENTLY at d7dbe9c8)
2 deps-empty          → TRIP
3 deps-older          → TRIP
4 deps-newer          → PASS
5 replan plan-missing → TRIP
6 replan plan-older   → TRIP
7 replan both-fresh   → PASS
8 no-replan, plan absent but deps fresh → PASS  (correct — plan.md is REPLAN-only)

All eight correct. The [ ! -s … ] short-circuit ahead of the -nt comparison genuinely closes the [ existing -nt missing ] fail-open. The { [ "${REPLAN:-0}" = 1 ] && { … }; } grouping binds correctly inside the || chain.

Rulings on the three judgement calls

1. Widening rule 4 to lifetime-scoping — UPHELD; the §RO conflict is real. §RO:1774-1775 mandates "Any materialized tree is an isolated throwaway worktree … git worktree add "$(mktemp -d)/…" "$PR_REF"". A worktree directory holds thousands of files, so a file-count-scoped rule 4 would render §RO's own mandated form §SP-violating — a live contradiction between two sections of one document. Lifetime-scoping is the correct resolution, and it does not reopen #3718: uniqueness under rule 4 comes from the kernel (a mktemp name is never shared), and the rule explicitly revokes the carve-out "the moment a path has to survive into a later Bash call". Widening the carve-out along an axis where the collision cannot occur is sound.

2. Fixing review-skill's pre-existing WT_FILE — JUSTIFIED scope expansion. wt.env is a genuine cross-call carrier (:337-339 re-sources it in a later step), so at d7dbe9c8 it was a live counter-example to rule 4 sitting inside the commit that defines rule 4 — and in a §CP document whose whole purpose is to be the single citable definition. Leaving it would have made the new rule immediately unteachable. It also does not itself violate rule 3: rule 3 forbids parking the recomputable $RUN_SCRATCH path in a file; $REVIEW_WT is tool-allocated and not recomputable, so it must be carried — and the fix makes the carrier's own path deterministic, which is exactly what rule 3 asks for. Correctly done, with an -s assert so a lost handle fails loud.

3. Inlined one-liner over a shell helper — VERIFIED, the reasoning holds. I defined a function in one Bash call and probed for it in the next: it was gone (type fails), while the session-derived one-liner recomputed byte-identically. A helper is exactly the shell state that does not survive, so it could not be called from the call that needs it. Mandating the inline form is right.

Do-not-churn list — honored

• Fail-closed allocation preserved at every site — swept all 20 interpolations; no :- fallback, no bare /tmp resurrection, no ||-through to a shared path.
• No demonstration test added for AC 4 — no test/spec file in the diff.
• pipeline-cli guard: lint for fixed-name scratch writes so §SP cannot silently regress #3735 referenced, not re-filed — open, status:triaged, p2, cited in the PR body (L49). No duplicate.

Residual /tmp/... string matches across the plugin are all prose counter-examples ("never stash state in /tmp/verdict-$PR.md"), not writes.

---

Non-blocking findings (follow-ups, not merge conditions)

1. Rule 4 lists an example that contradicts its own boundary. Rule 4 names "§RO's throwaway head worktree" as a rule-4 case, but that worktree demonstrably survives into later Bash calls in review-skill (which is why wt.env exists at all). By rule 4's stated "allocated and consumed inside one Bash call" test, it isn't a rule-4 case. Safety is unaffected — a mktemp path is never a shared name regardless of lifetime — but since §SP exists precisely to stop re-derivation, an example that reads as licence to re-run mktemp -d in a later call is the one misread worth pre-empting. Suggest a clause: a rule-4 resource may outlive the call provided its non-recomputable path is carried per rules 2+3.
2. plan-epic's splice guard still decides on an absent base. Case 9 (current.md missing, deps.md present) → PASS. Unreachable in the documented flow because the pre-loop -s assert at :818-822 covers current.md, but the guard's own comment says "Assert the file exists and is non-empty FIRST" while asserting only the derived blocks, not the base it compares against. Cheap to make the guard self-sufficient.
3. write-code has no OPEN step. All three sites (:1579, :1648, :1992) use the re-derive form (mkdir -p, no rm -rf), so §SP's "clears leftovers from an EARLIER run of this same slug in this same session" never fires there. A same-session re-run over the same <N> could read its own stale leaf and pass the -s assert. Self-stale, same-agent content — far milder than the cross-agent wrong-PR class Concurrent reviewer agents collide on shared scratchpad filenames — a diff silently returned the WRONG PR's files #3718 closes, but an asymmetry with the stated recipe.
4. Determinism depends on $TMPDIR too, not just the session id. On macOS ${TMPDIR:-/tmp} resolves to /var/folders/…/T/ (yielding a harmless //); if TMPDIR were set in one call and unset in another, the recomputed path would differ. It degrades safely — the -s assert fails loud rather than silently — but §SP claims the recipe is deterministic on the session id alone. One sentence would close it.
5. Stale wording at review-skill:87 — "the rule-4 single-file mktemp carve-out" is the pre-widening framing and contradicts rule 4's "lifetime, not file count". The next clause states the lifetime test correctly, so it self-corrects; the agent defs already say "single-Bash-call". Word-level nit.
6. plan-epic:811 attributes "the whole failure §SP rule 3 exists to prevent" to rule 3, but the failure at that site (destroying state a later call needs, via rm -rf) belongs to §SP's "Open the run once, re-derive freely" subsection; rule 3 is about not parking paths in files. The citation resolves to a real rule — the attribution is just loose.

None of the six is a merge condition. The two structural blockers are genuinely closed, and the fix is stronger than the pre-diff state: the assert-before-read discipline now makes a lost scratch file fail loud at every consuming site, where previously several read a silent absence.

I gate and verdict only — I don't merge, and §CP merge authority isn't mine.