fix: two records that disagreed with the code (issue #103)

Neither changes behaviour. Both make the written record lie to the next
reader, which is how the #99 class of bug got in.

session.reason was documented as a CLOSED enum

INT-RUN-HISTORY-FILE-CONTRACT listed nine tokens and omitted promote-failed,
which promoteSession's outer catch returns on any fs fault during the swap and
which mergeSession writes straight into the record on the completed path. A
full disk or a permissions change mid-promotion would therefore have produced
a token the spec called impossible, and a reader checking the record against
the spec would have concluded the worker invented it.

Added, with a producer table, because the flat list is what made this hard to
see: three code paths write this one field (resolve, runner, promote) and a
refused promotion WINS over the other two. The table also records what the
list cannot show, that expired never arrives from the promote path, and that
promoted is a return value reaching no record.

no-key stays and is deliberately NOT in the enum. It is unreachable in a wired
worker: sessionKeyFor is total and binary, so resolveSession returns null
rather than a keyless session, and promoteSession is only ever called with
what prepare handed over. Kept as the DI-seam backstop for the same reason the
store's no-sessionsDir return is kept, and now carrying the same explanation
instead of being a bare one-liner. A token no wired worker can emit does not
belong in the record's vocabulary.

processor.mjs's cold-start list named "no key" for the same reason, and a job
with no key gets no mount and no reason at all; promote-failed is the token
that actually belongs there. docs/sessions.md, the operator-facing mirror,
gains the promote-path pair, why a refused promotion wins, and the two values
that are not cold starts at all.

Pinned by a fault-injection test that faults renameSync, the one call only the
promote path makes, so the lock is already held when it blows up: the reason
is promote-failed, the canonical transcript is untouched, and the lock does
NOT outlive the failure. A leaked lock would cold-start every future run for
that key, quieter and worse than the fault itself.

A comment that contradicted its own code, three lines below

loader.mjs said staged pi packages "ride this same option" as
PI_GLOBAL_ALLOW_EXTENSIONS. The spread is unconditional and the rest of the
system agrees it should be: the worker emits PI_PACKAGES independently of the
opt-out, having already applied the per-trigger run.packages decision, so
re-gating here would withhold packages the operator did arm. The comment was
the defect, and it existed twice, mirrored verbatim into
INT-SDK-SESSION-OPTIONS. Both corrected to state the split and why, since this
is exactly what someone reads before answering "how do I stop all third-party
extension code loading in my containers" and the honest answer is BOTH
switches.

No test covered the false case: allowGlobalExtensions appeared once in the
whole loader suite, as true. Added, and verified by mutation. Gating
packagePaths on the option, the "fix" the old comment invited, now fails
instead of silently withholding every staged package from every job on every
deployment that sets the opt-out, on a clean exit 0.

INT-SESSION-STORE-CONTRACT UNCHANGED, checked: its write-path prose names no
reason tokens, so this drift was only ever visible from the record contract.

Verified with a live Valkey in the CI posture
(PI_DISPATCH_REQUIRE_{LOADER,WORKER,RECEIVER}_TESTS=1): 1959 tests, 0 failures,
0 skipped.

Signed-off-by: Rob Boerman <robboerman@live.nl>

Author: edgehero

docs/sessions.md

@@ -127,12 +127,27 @@ mid-run, the resume is refused. **Upgrading the job image costs every key one co
 nothing is deleted, each key simply cold-starts the first time its stamped version fails to match, and its
 next completed run rewrites both the transcript and the stamp.
 
-One further reason reaches `session.reason` without being a read-path outcome at all: `locked`. Only
-`promoteSession` produces it, on a **completed** run whose key was already held by another job's exclusive
-promotion lock. That run discards its own copy rather than clobbering the other's, and the reason is
-recorded to explain why the next run for the key will not see this run's work. Two jobs on one pull request
-inside one runtime is a real shape (`REQ-QUEUE-BURST-NO-DROP`), and last-write-wins there would interleave
-two agents' turns into one transcript.
+Two further reasons reach `session.reason` without being read-path outcomes at all. Both come from
+`promoteSession`, so both appear only on a **completed** run, and both describe the *write* back to the
+store rather than the read that started the job:
+
+| reason | meaning |
+|---|---|
+| `locked` | the key was already held by another job's exclusive promotion lock |
+| `promote-failed` | the write itself failed: a full disk, or a permissions change under the store mid-promotion |
+
+`locked` is the one with a design behind it. That run discards its own copy rather than clobbering the
+other's, and the reason is recorded to explain why the next run for the key will not see this run's work.
+Two jobs on one pull request inside one runtime is a real shape (`REQ-QUEUE-BURST-NO-DROP`), and
+last-write-wins there would interleave two agents' turns into one transcript. `promote-failed` is the
+disk telling you something: the run itself succeeded and its result is already on the forge, but its
+transcript did not persist, so the next run for that key cold-starts.
+
+A refused promotion **wins** over whatever the read path said, because on a completed run the more useful
+reason is the one that explains the *next* run's cold start rather than this one's.
+
+Two values are not cold starts at all and round out the enum: `resumed`, the transcript loaded and pi
+continued it, and `disabled`, which every job that did not arm `run.resume` records.
 
 ## Cost
 

image/runner/src/loader.mjs

@@ -266,7 +266,15 @@ export function buildResourceLoader({
 		// decision, so a second arming step was friction rather than safety. PI_GLOBAL_ALLOW_EXTENSIONS=0
 		// is the opt-out, and any other value is refused at config load so a typo cannot silently mean
 		// "load third-party code into every container" (worker/src/config.mjs, image/runner/src/config.mjs).
-		// Staged pi packages (INT-CONTAINER-JOB-INPUTS) ride this same option, LAST. One staged dir
+		// Staged pi packages (INT-CONTAINER-JOB-INPUTS) come LAST, and they do NOT ride that option: the
+		// packagePaths spread below is unconditional. Two switches, deliberately, because they withhold two
+		// different things. PI_GLOBAL_ALLOW_EXTENSIONS=0 makes the OVERLAY's own extensions/ dormant;
+		// `run.packages: false` on a trigger withholds the staged set from that trigger's jobs, and the
+		// worker has already applied it before emitting PI_PACKAGES (worker/src/env-allowlist.mjs) -- so an
+		// empty packagePaths here already means "this job loads none" and re-gating it on the overlay's
+		// opt-out would withhold packages the operator did arm. The honest answer to "how do I stop ALL
+		// third-party extension code loading in my containers" is therefore BOTH, which is what
+		// docs/global-pi-overlay.md's withhold table and docs/workflows.md tell the operator. One staged dir
 		// contributes extensions AND skills AND prompts AND themes through its package.json "pi"
 		// manifest: resolveExtensionSources reads the manifest and returns all four resource kinds,
 		// and reload() keeps cliEnabledExtensions/cliEnabledSkills REGARDLESS of noExtensions/noSkills

image/runner/test/loader.test.mjs

@@ -611,6 +611,35 @@ test("a hostile skill in the workspace tree is still NOT loaded with packages on
 	assert.ok(!surface.includes(HOSTILE_SENTINEL), "hostile skill content reached the loader");
 });
 
+test("PI_GLOBAL_ALLOW_EXTENSIONS=0 makes the OVERLAY dormant and leaves staged packages loading", { skip }, async () => {
+	// The two switches are separate on purpose, and nothing pinned the OFF side of this one: every other
+	// case in this file passes allowGlobalExtensions:true. Without this, someone reading the loader could
+	// "fix" the packagePaths spread to ride the same option and silently withhold every staged package
+	// from every job on every deployment that sets the opt-out -- a change that loses the operator tools
+	// they armed, on a clean exit 0, with nothing red anywhere.
+	//
+	// The worker already applied the per-trigger opt-out before emitting PI_PACKAGES
+	// (worker/src/env-allowlist.mjs), so a non-empty packagePaths here IS the operator's yes. Withholding
+	// all third-party extension code takes BOTH PI_GLOBAL_ALLOW_EXTENSIONS=0 and run.packages:false.
+	const jobPiDir = fixtureExtensionDir("job-pi-ext-", REPO_EXT_SENTINEL);
+	const globalPiDir = fixtureExtensionDir("pi-global-ext-", OVERLAY_EXT_SENTINEL);
+	const pkg = fixturePackage();
+	const { loader } = await load({ jobPiDir, globalPiDir, allowGlobalExtensions: false, packagePaths: [pkg] });
+
+	const paths = loader.getExtensions().extensions.map((e) => e.path);
+	assert.ok(paths.includes(join(pkg, "ext", "sentinel.js")), `the staged package's extension must still load: ${JSON.stringify(paths)}`);
+	assert.ok(!paths.includes(join(globalPiDir, "extensions")), `the overlay's extensions must be dormant: ${JSON.stringify(paths)}`);
+
+	// Asserted on the loaded SURFACE too, not just the paths: the opt-out has to withhold what the overlay
+	// contributes, and the package's own contribution has to survive it intact.
+	const surface = [JSON.stringify(loader.getSkills()), JSON.stringify(extensionCommands(loader)), loader.getAppendSystemPrompt().join("\n\n")].join("\n");
+	assert.ok(surface.includes(PKG_SKILL_SENTINEL), "the staged package's skill was withheld by the OVERLAY's opt-out");
+	assert.ok(!surface.includes(OVERLAY_EXT_SENTINEL), "an overlay extension registered a command while the opt-out was set");
+
+	// And the repo's own extensions are untouched by either switch.
+	assert.ok(paths.includes(join(jobPiDir, "extensions")), "the repo's extensions path must survive the overlay opt-out");
+});
+
 test("package extension paths come LAST -- repo, then overlay, then packages", { skip }, async () => {
 	// Extension resolution is first-path-wins, so ordering IS the trust ordering: nothing a staged
 	// package ships may shadow a repo or operator-overlay extension. Asserted on the loaded

specs/interfaces.md

@@ -57,8 +57,11 @@ Evidence convention as in `constitution.md`.
     // Repo path FIRST so a repo skill overrides a global one of the same name (pi is first-path-wins).
     additionalSkillPaths:     ["/job/pi/skills", ...(existsSync("/opt/pi-global/skills") ? ["/opt/pi-global/skills"] : [])],
     // Overlay extensions load unless the operator opted OUT (PI_GLOBAL_ALLOW_EXTENSIONS=0) AND the dir
-    // is present. Operator-staged pi packages (REQ-GLOBAL-PI-OVERLAY) ride this same option, LAST —
-    // extension resolution is first-path-wins, so nothing a package ships can shadow a repo or overlay
+    // is present. Operator-staged pi packages (REQ-GLOBAL-PI-OVERLAY) come LAST and do NOT ride that
+    // option — the spread is unconditional, because the worker already applied the per-trigger
+    // `run.packages` opt-out before emitting PI_PACKAGES. Two switches, withholding two different things:
+    // withholding ALL third-party extension code takes both. LAST is the trust ordering: extension
+    // resolution is first-path-wins, so nothing a package ships can shadow a repo or overlay
     // EXTENSION. That ordering fix does NOT extend to skills; skillsOverride below is where that is
     // settled. With noExtensions off, reload() merges the paths DISCOVERED under /workspace/.pi/
     // extensions AFTER this whole list, so a workspace extension is last of all and shadows nothing.
@@ -1569,14 +1572,34 @@ validator rather than a second copy of it.
     "replica":  <int> | null,       // this job's 1-based index within its replica set; null = an ordinary run
     "replicas": <int> | null,       // the set size, so `r2` is legible without finding the sibling row
     "session": { "resumed": <bool>,                                                             // what pi ACTUALLY did
-                 "reason": "<fixed enum: resumed|absent|expired|too-large|unparseable|not-a-regular-file|pi-version-changed|locked|disabled>" | null,
+                 "reason": "<fixed enum: resumed|absent|expired|too-large|unparseable|not-a-regular-file|pi-version-changed|locked|promote-failed|disabled>" | null,
                  "bytes": <int> | null } | null }   // null when the job had no session at all
   ```
   Field order is the serialisation order (`JSON.stringify` emits insertion order). The filename uses the
   **sanitized** id (`:` → `_`, because `repeat:<sched>:<millis>` is NTFS-illegal); the record **body**
   keeps the raw `jobId`. `reason` is a fixed enum passed through from the terminal outcome — never
   free-form and never payload text — and `turns` is `null` when the container died before emitting the
   runner `exit` line.
+
+  `session.reason` reads as one flat list but has **three producers**, which is why a token can look
+  unreachable from whichever half of the code you happen to be in:
+
+  | Producer | Tokens |
+  |---|---|
+  | **resolve path**, host-side, before the container (`readCanonical`) | `resumed`, `absent`, `expired`, `too-large`, `unparseable`, `not-a-regular-file`, `pi-version-changed` |
+  | **runner**, in the container (`image/runner/src/session.mjs`) | `disabled` (every unarmed job), `resumed`, `absent`, `unparseable` |
+  | **promote path**, only on a `completed` exit (`promoteSession`) | `absent`, `not-a-regular-file`, `too-large`, `locked`, `promote-failed` |
+
+  A refused promotion **wins** over the other two (`mergeSession`, `worker/src/processor.mjs`): on a
+  completed run it is the more useful reason, because it says why the NEXT run for this key will cold
+  start. Three things follow that the list cannot show. `expired` never arrives from the promote path,
+  which checks the file but not the TTL. `promoted` is a `promoteSession` return value that reaches no
+  record, because the merge reads a promotion's reason only when it refused. And `promote-failed` is the
+  one an operator meets in the wild: a full disk or a permissions change mid-promotion produces it.
+  `promoteSession`'s remaining return, `no-key`, is deliberately **absent from this enum** and cannot
+  reach a record — it is a DI-seam backstop, unreachable in a wired worker for the same reason the
+  store's own no-`sessionsDir` return is, since `sessionKeyFor` is total and binary and `resolveSession`
+  therefore returns `null` rather than a keyless session.
 - **Why**: The admin extension is a separate process (`DES-ADMIN-VIA-PI-EXTENSION`) that reads this as a
   read-model it does not share memory with — the worker writes the files, the admin extension reads them, and
   nothing crosses in RAM. The worker writes on both terminal paths: `worker/src/index.mjs` `makeProcessor`
@@ -1995,6 +2018,7 @@ recorded repair is re-running `/dispatch setup` (or editing the pointer by hand)
 
 | Date | Change |
 |---|---|
+| 2026-08-08 | Issue #103, two records that disagreed with the code. **INT-RUN-HISTORY-FILE-CONTRACT**: the nested `session.reason` enum was documented as CLOSED while omitting `promote-failed`, which `promoteSession`'s outer catch returns on any fs fault during the swap and which `mergeSession` writes straight into the record on the completed path — a full disk would have produced a token the spec called impossible. Added, together with a producer table, because the enum reads as one flat list while three separate code paths write it (resolve, runner, promote) and a refused promotion WINS over the other two. Recorded three things the list cannot show: `expired` never arrives from the promote path, `promoted` is a return value that reaches no record, and `no-key` is deliberately NOT in the enum — a DI-seam backstop unreachable in a wired worker, since `sessionKeyFor` is total and binary so `resolveSession` returns `null` rather than a keyless session. Pinned by a new fault-injection test in `worker/test/session-store.test.mjs`. **INT-SDK-SESSION-OPTIONS**: the `additionalExtensionPaths` comment claimed operator-staged pi packages "ride this same option" as `PI_GLOBAL_ALLOW_EXTENSIONS`; the spread is and remains UNCONDITIONAL, so the comment was the defect, in both this file and `image/runner/src/loader.mjs`. Corrected to state the split and why (the worker applies `run.packages` before emitting `PI_PACKAGES`, so re-gating here would withhold what the operator armed), and the previously untested `allowGlobalExtensions: false` case is now pinned in `image/runner/test/loader.test.mjs`. **INT-SESSION-STORE-CONTRACT UNCHANGED, checked** — its write-path prose names no reason tokens, so the drift could only ever have been visible from the record's own contract. |
 | 2026-08-07 | Issue #102: **INT-PI-PACKAGES-FILE-CONTRACT** records that `pi-packages.json` is now the override-and-addition layer rather than the only source (discovery reaches the same validator, so it adds candidates and never exemptions), and that the receipt gained `from` as a CLOSED enum with a default — which covers both compatibility directions at once, since a pre-#102 receipt carries no `from` and reads as declared while an older worker drops it as an unknown key. Also records that the receipt is now read at EACH job start rather than once at boot, why the boot read was right until discovery made re-staging routine, and why a failed read keeps last-known-good instead of degrading to none (an empty set emits no `PI_PACKAGES`, so the runner's path assertion would have nothing to refuse and the job would run toolless on a clean exit 0). **INT-CONTAINER-RUNTIME-CONTRACT, INT-TRIGGERS-FILE-CONTRACT, INT-CONTAINER-JOB-INPUTS UNCHANGED, checked** — the mount, `PI_PACKAGES`, `run.packages` and the pre-spend refusal are all untouched; only who fills the manifest, and how often it is read, moved. |
 | 2026-08-04 | Documentation audit fallout (issue #99). **INT-TRIGGERS-FILE-CONTRACT** amended on `run.resume`, which this file had described as carried on **ALL FOUR** kinds for a month while the wiring covered three: `resolveSession` is handed to the forge preparers only, so a cron job with the flag armed staged no transcript, mounted no `/session`, promoted nothing and exited `0` as though it had. That is the flag's own believed-on-while-off inversion reached through the wiring rather than through a truthy `"false"` string, so the fix is `run.replicas`' fix: **refused at load**, worded *not yet covered* rather than impossible, because `session-key.mjs` already derives the local key from the scheduler id and nothing reaches it. Only `true` is refused — `false` and absent still validate and still land in `data` byte-identically, since `false` is the documented default and refusing an operator for writing down present behaviour would also change a shape pinned as byte-identical; the asymmetry with `run.replicas` (which refuses ANY value on cron) is recorded rather than left to read as an oversight, `1` being a no-op flag where `false` is the truth. The same bullet gains the **pre-spend** half, which the triggers file cannot answer by construction: whether a session store exists is deployment state, not file content, so an armed trigger under a deployment with no `PI_SESSIONS_DIR` is refused per delivery and `doctor` keeps the load-time warning. **INT-RUN-HISTORY-FILE-CONTRACT**: the `reason` enum gains one token, **`sessions-dir-unset`**, on `job-image-missing`'s precedent — a policy outcome with `budgetReserved: false`, since it is answered from two values already in hand before the mint, the branch check, the clone, the token-cap read and the budget INCR. Its shape follows `settings-overlay-invalid`'s (`<config artifact>-<its bad state>`) and its words are the spec's own, so the token greps to the text that mandates it. The nested `session.reason` enum's **`disabled`** was audited as unimplemented and is **UNCHANGED, checked**: the runner produces it (`image/runner/src/session.mjs`) whenever no session file is mounted, which is every unarmed job, so the entry was right and the audit finding was wrong. **INT-SESSION-STORE-CONTRACT UNCHANGED, checked**: the store's own no-`sessionsDir` return is now unreachable in a wired worker and kept as the DI-seam backstop, which changes no byte of its contract. |
 | 2026-08-04 | The panel learns to find a deployment built elsewhere (issue #92). Added **INT-DEPLOYMENT-POINTER-CONTRACT**: `<agent dir>/pi-dispatch-deployment.json` (override `PI_DISPATCH_DEPLOYMENT_FILE`), an allowlisted absolute-paths-only env map layered UNDER the operator's env once at extension load — env wins key by key, the worker/receiver never read it, and `PI_DISPATCH_RUN_ROOTS`/credentials in the file have no effect by construction (a pointer that widened the AI-run allowlist would be a second unreviewed door to a gated capability). Deliberate divergence from INT-SUBSCRIPTIONS' loud version refusal, recorded in the entry: a broken/newer pointer degrades to exactly the pre-pointer behavior with a one-line surfaced notice, never a throw — the read-model's never-throw doctrine outranks fail-loud here because the pointer is an availability aid, not a data file. **INT-SUBSCRIPTIONS-FILE-CONTRACT / INT-CONFIG-OVERLAY-CONTRACT UNCHANGED, checked**: the pointer changes how their Locations are *found*, not what the files contain. |

worker/src/processor.mjs

@@ -156,8 +156,8 @@ export async function runJob(job, deps) {
 		}
 
 		// REQ-RESUMABLE-SESSION's one fail-CLOSED case. Everything else in that feature fails OPEN and
-		// NAMES itself -- absent, expired, too-large, unparseable, locked, no key -- because a cold start is
-		// a correct run. This one cannot be: with no `sessionsDir`, resolveSession returns null
+		// NAMES itself -- absent, expired, too-large, unparseable, locked, promote-failed -- because a cold
+		// start is a correct run. This one cannot be: with no `sessionsDir`, resolveSession returns null
 		// (session-store.mjs), so nothing is staged, no /session is mounted, the transcript dies with the
 		// container, and the NEXT job on that key cold-starts too. The job would exit 0 and look like the
 		// feature worked. That is an operator who believes a disclosure is on while it is off, with a green

worker/src/session-store.mjs

@@ -115,6 +115,12 @@ export function makeSessionStore({
 	 * agents' turns into one transcript.
 	 */
 	function promoteSession(session, { piVersion = null } = {}) {
+		// The second DI-seam backstop, and unreachable for the same reason as the `!sessionsDir` return
+		// above: sessionKeyFor is total and binary (null, or 32 hex chars), so resolveSession returns null
+		// rather than a keyless session, and processor.mjs only calls this when prepare handed it one. Kept
+		// because the store and the preparer are separately injected and neither can assume the other. It is
+		// NOT in INT-RUN-HISTORY-FILE-CONTRACT's session.reason enum, deliberately: a token no wired worker
+		// can emit does not belong in the record's vocabulary, and `promote-failed` below does.
 		if (!session?.key) return { promoted: false, reason: "no-key" };
 		try {
 			const staged = join(session.hostDir, SESSION_FILE_NAME);