From d94220475957c943a2f92289ef07c794cafd91f7 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Fri, 3 Jul 2026 01:01:58 +0800
Subject: [PATCH 1/4] docs(AGENTS): distill process lessons from the hooks
 stack (#118-#129)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Retrospective on the whole stacked-PR effort. Adds a "Landing changes cleanly:
gates, Codex, and scope" section capturing the workflow lessons the effort
surfaced, and two codebase-specific traps to Defensive patterns.

The through-line: a mechanical gate proves lines ran and types check, never
that a test guards anything or that prose is accurate — so layer the human/AI
judgment on top, in order, and keep each unit honestly scoped:
- prove every regression test RED on the unfixed code (top-billed, not a nit)
- run the FULL test:coverage, not an isolated -t filter (test-isolation bugs)
- spend Codex on what gates can't see (prose/RFC/comment drift, self-introduced
  fix bugs), scoped to ONE concern per review (a two-fix prompt timed out)
- a mid-review cleanup that exceeds the reviewed RFC scope goes in a NEW stacked
  PR; enumerate consumers + grill before deleting a seam
- regenerate a generated artifact as part of the invalidating edit, not as a
  gate to fail; lint:fix before hand-fixing
- read a failure before reacting: ENOSPC watcher exhaustion is environmental,
  not a code regression

Defensive-patterns additions (both bit us this cycle): spawn narrows non-null
stdout/stderr only from a literal stdio tuple; AgentLoop.create() drops
options.meta (only the async factory threads it).
---
 AGENTS.md | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/AGENTS.md b/AGENTS.md
index 8438295683..47109ebfbe 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -34,6 +34,19 @@ A wave of review comments lands across several PRs in a dependent stack (`A ←
 - **Delegated work is trust-but-verify.** When sub-agents implement fixes in parallel, their report describes what they INTENDED, not necessarily what landed. Re-run the gates yourself on the actual tree, and for a regression guard, **prove it FAILS on the unfixed code** (introduce the regression, watch the test go red, revert) — a guard that passes both ways guards nothing. A sub-agent that "reframes the problem as already-handled" instead of fixing it is a signal to dig in personally, not to accept the reframing.
 - **Triage on the merits, then reply in-thread.** Verify each comment against the code before acting (a reviewer flagging the right symptom can still mis-diagnose the cause — confirm both). Reply in the GitHub review thread (`gh api …/pulls/{pr}/comments/{id}/replies`), not as a top-level comment, stating the fix and the commit that carries it.
 
+## Landing changes cleanly: gates, Codex, and scope
+
+Hard-won from the hooks stack (#118–#129). The recurring theme: a mechanical gate proves lines ran and types check; it does NOT prove semantics, doc accuracy, or that a test guards anything. Layer the cheap human/AI judgment on top, in the right order, and keep each unit of work honestly scoped.
+
+- **Every regression test must be proven RED on the unfixed code, and this is the top-billed discipline, not a footnote.** Neuter the fix (comment out the one line, or revert the source), run the new test, watch it fail, then restore. A guard that passes both ways guards nothing — and a green 100%-coverage suite actively hides this (the line ran; it just asserted nothing load-bearing). This caught real bugs repeatedly here: a `structuredClone` aliasing fix, a bridge `expectedEventName` discriminator guard, a blocking-Stop-hook reason fallback. Do it for EVERY guard, every time; the proof takes thirty seconds and is the only thing that certifies the test.
+- **Run `pnpm run test:coverage` (the FULL suite), not an isolated `-t` filter, before trusting green.** Test-isolation bugs surface only in the full run: here twelve fixed-`setTimeout` waits raced under full-suite load and passed in isolation but flaked together — fixed by replacing every fixed sleep with a `waitFor(predicate)` poll (ties to [§ Defensive patterns](#defensive-patterns-hard-won) "Async state is not synchronous state"). A suite that is green under `-t <name>` but red under `test:coverage` is telling you about shared state, not a flake to rerun.
+- **Codex convergence is for the class of defect gates STRUCTURALLY cannot catch — spend it there.** `xhigh` Codex reliably finds what `typecheck`/`lint`/`coverage`/`doc-sync` are blind to: (a) **prose/RFC/comment drift** the doc-sync scope doesn't scan — e.g. two package READMEs still advertising a removed event, or an RFC claiming a `block` decision "carries context too" when that union has no such field; (b) **a bug you INTRODUCED while fixing** — the fix's own new branch, un-covered by the test you wrote for the original bug; (c) **dishonest test comments** blessing a wrong assertion. Treat a Codex finding as a claim to verify against the code, then re-bucket it yourself (its own (A)/(B)/(C) label is an input, not a verdict) — but know that "clean gates" is exactly when Codex earns its keep.
+- **Scope a Codex review to ONE fix or concern.** A convergence prompt bundling two independent fixes plus verification context timed out at the 850s cap with no verdict — a wasted ~14-minute run — then completed fine once split into two smaller serial reviews. One concern per review is faster AND yields a sharper verdict. (For the invocation: the prompt is a POSITIONAL arg to `ask-codex.sh`, not `--file`; the only flags are `--codex-model`, `--codex-timeout`. Multi-paragraph prompts go via `"$(cat file)"`.)
+- **A cleanup or removal discovered mid-review that exceeds the reviewed RFC's scope goes in a NEW stacked PR, even though pre-release churn is cheap.** Do not retroactively widen a diff a reviewer already signed off on, and do not fold a fresh decision into a converged PR. Before deleting an event/seam, first enumerate every consumer and prove redundancy (here: `agent/stream-chunk` was proven a pure mirror of the durable `assistant/chunk` — ACP already read the durable one, the stdio UI ignored the live-only args), then grill the removal ("am I deleting a seam someone will re-add?"). The removal became its own PR-G with its own RFC, not an amendment to the reviewed #118.
+- **Regenerate a generated artifact as PART of the edit that invalidates it, not as a gate to fail.** Any edit to a `types.ts` `interface Events`/`Context` block or a module doc the generator reads makes `docs/cordis-catalog/events-and-services.md` stale; run `pnpm run gen-cordis-catalog` (and `gen-module-graph`) in the same step rather than letting `doc-sync` discover it. Likewise run `pnpm run lint:fix` before hand-fixing a new test file — the auto-fixable churn (quotes, `max-len`) should never consume review attention meant for the real errors.
+- **Read a failure before reacting: environmental ≠ code.** `ENOSPC: file watchers` from many concurrent worktrees fails the `tsx`-based `demo:echo` smoke, but the label/output already rendered correctly before the watcher died and the published-artifact built-bin smoke (plain `node`, no watcher) is unaffected. Recognize the class on the FIRST occurrence — fall back to the watcher-free check or prune stale worktrees — rather than burning retry cycles on a transient the code never caused.
+
+
 ## Architecture
 
 This codebase is based on the **Cordis** framework, built microkernel-style: **everything is a plugin**. All necessary Cordis dependencies are copied into this monorepo as vendored source (under `vendor/`) instead of being depended on via npm.
@@ -275,6 +288,8 @@ Each bullet is a bug class that bit us; the rule prevents the reoccurrence.
   - **A real-load-path test only GUARDS the export shape if a broken shape actually FAILS it.** The original crash (`cannot get property … without inject`) fired because that plugin HAS `inject`. A plugin with NO `inject` (a composition/bundle plugin that mounts children carrying their own inject, e.g. `dsh-agent-core` and the app packages) does NOT crash on a stray `export default` — `unwrapExports` silently drops `Config`/`name` and the plugin boots anyway — so a Loader smoke stays green while the export shape is broken. For such plugins add an EXPLICIT assertion that the regression fails: `expect('default' in mod).toBe(false)` plus running the module through the real `Loader.prototype.unwrapExports` and asserting `name`/`Config`/`apply` survive. Prove it: add `export default apply`, watch the test go red, revert.
   - **"Real entry path" means the PUBLISHED ARTIFACT, not the dev runtime.** A test (or a `demo:*` smoke) that boots `src/bin.ts` under `tsx` is NOT the same code a consumer runs — the package `bin` field points at the built `lib/bin.js` under plain `node`. tsx masks failure modes the published artifact has: a boot settle-race that exits 0 before the app's handles attach, module-resolution differences (the unbuilt `paths` map vs node_modules), and a load failure that `loader.await()`'s `Promise.allSettled` SWALLOWS so a typo'd config silently exits 0. The guard is a smoke that runs the built `lib/bin.js` under plain `node` in a node_modules-shaped temp dir (symlinked workspace + vendor packages), asserts the real output, AND asserts a genuinely-missing config exits NON-ZERO. The tsx demo is necessary but not sufficient; the published-bin smoke is what catches "green under tsx, broken on install".
 - **Tag spelling and EOF hygiene.** cordis.yml interpolates env via the `!!js` tag (js-yaml resolves custom tags under `tag:yaml.org,2002:js`), not `!js` — keep code, comments, and docs consistent. Files end with exactly one trailing newline; `git diff --check` (a pre-push gate) rejects new blank lines at EOF.
+- **`child_process.spawn` narrows non-null `stdout`/`stderr` only from a LITERAL `stdio` tuple.** A ternary or variable in a `stdio` slot (e.g. `stdio: [wantStdin ? 'pipe' : 'ignore', 'pipe', 'pipe']`) selects the generic `spawn` overload, widening the child's streams to nullable — which then trips `no-non-null-assertion` (forbidden in `src`). Write two full `spawn(...)` calls with literal tuples in an `if`/`else` (or a ternary between two complete calls), as [`dsh-bash-local`'s `run.ts`](packages/bash/bash-local/src/run.ts) does, so each branch's literal tuple keeps the typed overload. This trap bit twice — recognize it the moment a conditional `stdio` slot appears.
+- **`AgentLoop.create(id, options)` DROPS `options.meta` — only the async factory path threads it.** The synchronous `create()` prepares its session with a hardcoded `{ meta: {} }`; a test (or caller) that needs `session.header.cwd` or other header metadata to take effect must use the factory `ctx.agents.create({ agentId, sessionId, meta, agentOptions })` (or `resume`), which passes `meta: options.meta ?? {}`. A cwd-dependent test that silently sees an empty cwd is almost always this. See `packages/core/agent-loop/src/index.ts`.
 
 ## Type Safety and Documentation
 

From 907c8d387100514c3c1671d4d029cf3cf53b0959 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Fri, 3 Jul 2026 01:25:34 +0800
Subject: [PATCH 2/4] docs(AGENTS): fix two factual imprecisions caught by
 Codex
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Codex convergence verified the retrospective's claims against the code and
caught two inaccuracies (everything else — the spawn-overload claim, create()
dropping meta, ask-codex invocation, test:coverage guidance, waitFor, markdown
— checked out):

- The meta-threading factory `ctx.agents.create()` (createAgent) is
  SYNCHRONOUS, not "the async factory path"; the async one is `resume` (which
  reloads the persisted header). Reworded.
- The generated-artifact bullet conflated triggers: gen-cordis-catalog reads
  `interface Events`/`Context` member JSDoc (not top module docs);
  gen-module-graph is driven by package peerDependencies (not event/doc edits);
  and module-graph freshness is `verify-module-graph`, a SEPARATE gate from
  `doc-sync`. Split the guidance per artifact.
---
 AGENTS.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 47109ebfbe..2985859fe5 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -43,7 +43,7 @@ Hard-won from the hooks stack (#118–#129). The recurring theme: a mechanical g
 - **Codex convergence is for the class of defect gates STRUCTURALLY cannot catch — spend it there.** `xhigh` Codex reliably finds what `typecheck`/`lint`/`coverage`/`doc-sync` are blind to: (a) **prose/RFC/comment drift** the doc-sync scope doesn't scan — e.g. two package READMEs still advertising a removed event, or an RFC claiming a `block` decision "carries context too" when that union has no such field; (b) **a bug you INTRODUCED while fixing** — the fix's own new branch, un-covered by the test you wrote for the original bug; (c) **dishonest test comments** blessing a wrong assertion. Treat a Codex finding as a claim to verify against the code, then re-bucket it yourself (its own (A)/(B)/(C) label is an input, not a verdict) — but know that "clean gates" is exactly when Codex earns its keep.
 - **Scope a Codex review to ONE fix or concern.** A convergence prompt bundling two independent fixes plus verification context timed out at the 850s cap with no verdict — a wasted ~14-minute run — then completed fine once split into two smaller serial reviews. One concern per review is faster AND yields a sharper verdict. (For the invocation: the prompt is a POSITIONAL arg to `ask-codex.sh`, not `--file`; the only flags are `--codex-model`, `--codex-timeout`. Multi-paragraph prompts go via `"$(cat file)"`.)
 - **A cleanup or removal discovered mid-review that exceeds the reviewed RFC's scope goes in a NEW stacked PR, even though pre-release churn is cheap.** Do not retroactively widen a diff a reviewer already signed off on, and do not fold a fresh decision into a converged PR. Before deleting an event/seam, first enumerate every consumer and prove redundancy (here: `agent/stream-chunk` was proven a pure mirror of the durable `assistant/chunk` — ACP already read the durable one, the stdio UI ignored the live-only args), then grill the removal ("am I deleting a seam someone will re-add?"). The removal became its own PR-G with its own RFC, not an amendment to the reviewed #118.
-- **Regenerate a generated artifact as PART of the edit that invalidates it, not as a gate to fail.** Any edit to a `types.ts` `interface Events`/`Context` block or a module doc the generator reads makes `docs/cordis-catalog/events-and-services.md` stale; run `pnpm run gen-cordis-catalog` (and `gen-module-graph`) in the same step rather than letting `doc-sync` discover it. Likewise run `pnpm run lint:fix` before hand-fixing a new test file — the auto-fixable churn (quotes, `max-len`) should never consume review attention meant for the real errors.
+- **Regenerate a generated artifact as PART of the edit that invalidates it, not as a gate to fail.** Know what triggers each: `docs/cordis-catalog/events-and-services.md` is generated from the `interface Events` / `interface Context` member JSDoc (not top module docs), so run `pnpm run gen-cordis-catalog` in the same step you touch an event/service declaration or its JSDoc — rather than letting `verify-cordis-catalog` (part of `doc-sync`) discover it stale. `docs/module-graph.md` is generated from package `peerDependencies`, so regenerate it (`pnpm run gen-module-graph`; checked by the separate `verify-module-graph`, NOT `doc-sync`) only when you change a package's `@deepseek-ai/dsh-*` peer edges. Likewise run `pnpm run lint:fix` before hand-fixing a new test file — the auto-fixable churn (quotes, `max-len`) should never consume review attention meant for the real errors.
 - **Read a failure before reacting: environmental ≠ code.** `ENOSPC: file watchers` from many concurrent worktrees fails the `tsx`-based `demo:echo` smoke, but the label/output already rendered correctly before the watcher died and the published-artifact built-bin smoke (plain `node`, no watcher) is unaffected. Recognize the class on the FIRST occurrence — fall back to the watcher-free check or prune stale worktrees — rather than burning retry cycles on a transient the code never caused.
 
 
@@ -289,7 +289,7 @@ Each bullet is a bug class that bit us; the rule prevents the reoccurrence.
   - **"Real entry path" means the PUBLISHED ARTIFACT, not the dev runtime.** A test (or a `demo:*` smoke) that boots `src/bin.ts` under `tsx` is NOT the same code a consumer runs — the package `bin` field points at the built `lib/bin.js` under plain `node`. tsx masks failure modes the published artifact has: a boot settle-race that exits 0 before the app's handles attach, module-resolution differences (the unbuilt `paths` map vs node_modules), and a load failure that `loader.await()`'s `Promise.allSettled` SWALLOWS so a typo'd config silently exits 0. The guard is a smoke that runs the built `lib/bin.js` under plain `node` in a node_modules-shaped temp dir (symlinked workspace + vendor packages), asserts the real output, AND asserts a genuinely-missing config exits NON-ZERO. The tsx demo is necessary but not sufficient; the published-bin smoke is what catches "green under tsx, broken on install".
 - **Tag spelling and EOF hygiene.** cordis.yml interpolates env via the `!!js` tag (js-yaml resolves custom tags under `tag:yaml.org,2002:js`), not `!js` — keep code, comments, and docs consistent. Files end with exactly one trailing newline; `git diff --check` (a pre-push gate) rejects new blank lines at EOF.
 - **`child_process.spawn` narrows non-null `stdout`/`stderr` only from a LITERAL `stdio` tuple.** A ternary or variable in a `stdio` slot (e.g. `stdio: [wantStdin ? 'pipe' : 'ignore', 'pipe', 'pipe']`) selects the generic `spawn` overload, widening the child's streams to nullable — which then trips `no-non-null-assertion` (forbidden in `src`). Write two full `spawn(...)` calls with literal tuples in an `if`/`else` (or a ternary between two complete calls), as [`dsh-bash-local`'s `run.ts`](packages/bash/bash-local/src/run.ts) does, so each branch's literal tuple keeps the typed overload. This trap bit twice — recognize it the moment a conditional `stdio` slot appears.
-- **`AgentLoop.create(id, options)` DROPS `options.meta` — only the async factory path threads it.** The synchronous `create()` prepares its session with a hardcoded `{ meta: {} }`; a test (or caller) that needs `session.header.cwd` or other header metadata to take effect must use the factory `ctx.agents.create({ agentId, sessionId, meta, agentOptions })` (or `resume`), which passes `meta: options.meta ?? {}`. A cwd-dependent test that silently sees an empty cwd is almost always this. See `packages/core/agent-loop/src/index.ts`.
+- **`AgentLoop.create(id, options)` DROPS `options.meta` — only the programmatic factory `create` threads it.** The convenience `create()` prepares its session with a hardcoded `{ meta: {} }`; a test (or caller) that needs `session.header.cwd` or other header metadata to take effect must use the factory `ctx.agents.create({ agentId, sessionId, meta, agentOptions })` (which passes `meta: options.meta ?? {}`), or `resume` (which reloads the persisted header). Both are synchronous. A cwd-dependent test that silently sees an empty cwd is almost always the wrong creation path. See `packages/core/agent-loop/src/index.ts`.
 
 ## Type Safety and Documentation
 

From c699e948e6fd999db39bcd6f278eff2c64bcdce7 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Fri, 3 Jul 2026 23:48:32 +0800
Subject: [PATCH 3/4] =?UTF-8?q?docs(AGENTS):=20fix=20review=20findings=20?=
 =?UTF-8?q?=E2=80=94=20correct=20resume=20async,=20cut=20stack-locked=20le?=
 =?UTF-8?q?ssons?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Address ds-review-bot review on #131:

- Blocking: "Both are synchronous" was wrong for `resume`
  (`ctx.agents.resume`/`AgentLoop.resume` return `Promise<AgentHandle>` and
  await the persisted load). State the true split: `create` is synchronous,
  `resume` is async.
- De-anchor the section: drop the "#118–#129" opening and every hooks-stack
  reference (agent/stream-chunk, PR-G, #118, ENOSPC-from-worktrees) so the
  lessons stand alone for a reader who wasn't there, matching the generalized
  style of the "Orchestrating review feedback" section above it.
- Cut bullets that duplicated existing AGENTS.md content: the standalone
  "prove regression tests RED" bullet (already stated in the Orchestrating
  section and Defensive patterns) and the ENOSPC "environmental ≠ code" bullet
  (no repo-specific rule survives generalizing).
- Generalize the survivors to their transferable rule; trim the
  regenerate-artifact bullet to the one gate-backed example.
- Collapse the double blank line before ## Architecture.
---
 AGENTS.md | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 2985859fe5..4b408fafb5 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -36,16 +36,13 @@ A wave of review comments lands across several PRs in a dependent stack (`A ←
 
 ## Landing changes cleanly: gates, Codex, and scope
 
-Hard-won from the hooks stack (#118–#129). The recurring theme: a mechanical gate proves lines ran and types check; it does NOT prove semantics, doc accuracy, or that a test guards anything. Layer the cheap human/AI judgment on top, in the right order, and keep each unit of work honestly scoped.
-
-- **Every regression test must be proven RED on the unfixed code, and this is the top-billed discipline, not a footnote.** Neuter the fix (comment out the one line, or revert the source), run the new test, watch it fail, then restore. A guard that passes both ways guards nothing — and a green 100%-coverage suite actively hides this (the line ran; it just asserted nothing load-bearing). This caught real bugs repeatedly here: a `structuredClone` aliasing fix, a bridge `expectedEventName` discriminator guard, a blocking-Stop-hook reason fallback. Do it for EVERY guard, every time; the proof takes thirty seconds and is the only thing that certifies the test.
-- **Run `pnpm run test:coverage` (the FULL suite), not an isolated `-t` filter, before trusting green.** Test-isolation bugs surface only in the full run: here twelve fixed-`setTimeout` waits raced under full-suite load and passed in isolation but flaked together — fixed by replacing every fixed sleep with a `waitFor(predicate)` poll (ties to [§ Defensive patterns](#defensive-patterns-hard-won) "Async state is not synchronous state"). A suite that is green under `-t <name>` but red under `test:coverage` is telling you about shared state, not a flake to rerun.
-- **Codex convergence is for the class of defect gates STRUCTURALLY cannot catch — spend it there.** `xhigh` Codex reliably finds what `typecheck`/`lint`/`coverage`/`doc-sync` are blind to: (a) **prose/RFC/comment drift** the doc-sync scope doesn't scan — e.g. two package READMEs still advertising a removed event, or an RFC claiming a `block` decision "carries context too" when that union has no such field; (b) **a bug you INTRODUCED while fixing** — the fix's own new branch, un-covered by the test you wrote for the original bug; (c) **dishonest test comments** blessing a wrong assertion. Treat a Codex finding as a claim to verify against the code, then re-bucket it yourself (its own (A)/(B)/(C) label is an input, not a verdict) — but know that "clean gates" is exactly when Codex earns its keep.
-- **Scope a Codex review to ONE fix or concern.** A convergence prompt bundling two independent fixes plus verification context timed out at the 850s cap with no verdict — a wasted ~14-minute run — then completed fine once split into two smaller serial reviews. One concern per review is faster AND yields a sharper verdict. (For the invocation: the prompt is a POSITIONAL arg to `ask-codex.sh`, not `--file`; the only flags are `--codex-model`, `--codex-timeout`. Multi-paragraph prompts go via `"$(cat file)"`.)
-- **A cleanup or removal discovered mid-review that exceeds the reviewed RFC's scope goes in a NEW stacked PR, even though pre-release churn is cheap.** Do not retroactively widen a diff a reviewer already signed off on, and do not fold a fresh decision into a converged PR. Before deleting an event/seam, first enumerate every consumer and prove redundancy (here: `agent/stream-chunk` was proven a pure mirror of the durable `assistant/chunk` — ACP already read the durable one, the stdio UI ignored the live-only args), then grill the removal ("am I deleting a seam someone will re-add?"). The removal became its own PR-G with its own RFC, not an amendment to the reviewed #118.
-- **Regenerate a generated artifact as PART of the edit that invalidates it, not as a gate to fail.** Know what triggers each: `docs/cordis-catalog/events-and-services.md` is generated from the `interface Events` / `interface Context` member JSDoc (not top module docs), so run `pnpm run gen-cordis-catalog` in the same step you touch an event/service declaration or its JSDoc — rather than letting `verify-cordis-catalog` (part of `doc-sync`) discover it stale. `docs/module-graph.md` is generated from package `peerDependencies`, so regenerate it (`pnpm run gen-module-graph`; checked by the separate `verify-module-graph`, NOT `doc-sync`) only when you change a package's `@deepseek-ai/dsh-*` peer edges. Likewise run `pnpm run lint:fix` before hand-fixing a new test file — the auto-fixable churn (quotes, `max-len`) should never consume review attention meant for the real errors.
-- **Read a failure before reacting: environmental ≠ code.** `ENOSPC: file watchers` from many concurrent worktrees fails the `tsx`-based `demo:echo` smoke, but the label/output already rendered correctly before the watcher died and the published-artifact built-bin smoke (plain `node`, no watcher) is unaffected. Recognize the class on the FIRST occurrence — fall back to the watcher-free check or prune stale worktrees — rather than burning retry cycles on a transient the code never caused.
+The recurring failure mode: a mechanical gate proves lines ran and types check; it never proves semantics, doc accuracy, or that a test guards anything. Layer the cheap human/AI judgment on top, in the right order, and keep each unit of work honestly scoped.
 
+- **Run `pnpm run test:coverage` (the FULL suite), not an isolated `-t` filter, before trusting green.** Test-isolation bugs surface only in the full run: fixed-`setTimeout` waits that pass in isolation race under full-suite load and flake together — replace every fixed sleep with a `waitFor(predicate)` poll (ties to [§ Defensive patterns](#defensive-patterns-hard-won) "Async state is not synchronous state"). A suite green under `-t <name>` but red under `test:coverage` is telling you about shared state, not a flake to rerun.
+- **Codex convergence is for the class of defect gates STRUCTURALLY cannot catch — spend it there.** `xhigh` Codex reliably finds what `typecheck`/`lint`/`coverage`/`doc-sync` are blind to: (a) **prose/RFC/comment drift** the doc-sync scope doesn't scan — a package README still advertising a removed event, or an RFC claiming a decision "carries context too" when that union has no such field; (b) **a bug you INTRODUCED while fixing** — the fix's own new branch, un-covered by the test you wrote for the original bug; (c) **dishonest test comments** blessing a wrong assertion. Treat a Codex finding as a claim to verify against the code, then re-bucket it yourself (its own (a)/(b)/(c) label is an input, not a verdict) — but know that "clean gates" is exactly when Codex earns its keep.
+- **Scope a Codex review to ONE fix or concern.** A convergence prompt bundling two independent fixes plus verification context timed out at the 850s cap with no verdict, then completed fine once split into two smaller serial reviews. One concern per review is faster AND yields a sharper verdict. (For the invocation: the prompt is a POSITIONAL arg to `ask-codex.sh`, not `--file`; the only flags are `--codex-model`, `--codex-timeout`. Multi-paragraph prompts go via `"$(cat file)"`.)
+- **Before deleting an event or seam, enumerate every consumer and prove redundancy, then grill the removal ("am I deleting a seam someone will re-add?").** A seam that is a pure mirror of something a consumer already reads is safe to cut; a live-only field one consumer still uses is not — prove which before removing, not after.
+- **Regenerate a generated artifact as PART of the edit that invalidates it, not as a gate to fail.** Know what triggers each: `docs/cordis-catalog/events-and-services.md` is generated from the `interface Events` / `interface Context` member JSDoc (not top module docs), so run `pnpm run gen-cordis-catalog` in the same step you touch an event/service declaration or its JSDoc — rather than letting `verify-cordis-catalog` (part of `doc-sync`) discover it stale.
 
 ## Architecture
 
@@ -289,7 +286,7 @@ Each bullet is a bug class that bit us; the rule prevents the reoccurrence.
   - **"Real entry path" means the PUBLISHED ARTIFACT, not the dev runtime.** A test (or a `demo:*` smoke) that boots `src/bin.ts` under `tsx` is NOT the same code a consumer runs — the package `bin` field points at the built `lib/bin.js` under plain `node`. tsx masks failure modes the published artifact has: a boot settle-race that exits 0 before the app's handles attach, module-resolution differences (the unbuilt `paths` map vs node_modules), and a load failure that `loader.await()`'s `Promise.allSettled` SWALLOWS so a typo'd config silently exits 0. The guard is a smoke that runs the built `lib/bin.js` under plain `node` in a node_modules-shaped temp dir (symlinked workspace + vendor packages), asserts the real output, AND asserts a genuinely-missing config exits NON-ZERO. The tsx demo is necessary but not sufficient; the published-bin smoke is what catches "green under tsx, broken on install".
 - **Tag spelling and EOF hygiene.** cordis.yml interpolates env via the `!!js` tag (js-yaml resolves custom tags under `tag:yaml.org,2002:js`), not `!js` — keep code, comments, and docs consistent. Files end with exactly one trailing newline; `git diff --check` (a pre-push gate) rejects new blank lines at EOF.
 - **`child_process.spawn` narrows non-null `stdout`/`stderr` only from a LITERAL `stdio` tuple.** A ternary or variable in a `stdio` slot (e.g. `stdio: [wantStdin ? 'pipe' : 'ignore', 'pipe', 'pipe']`) selects the generic `spawn` overload, widening the child's streams to nullable — which then trips `no-non-null-assertion` (forbidden in `src`). Write two full `spawn(...)` calls with literal tuples in an `if`/`else` (or a ternary between two complete calls), as [`dsh-bash-local`'s `run.ts`](packages/bash/bash-local/src/run.ts) does, so each branch's literal tuple keeps the typed overload. This trap bit twice — recognize it the moment a conditional `stdio` slot appears.
-- **`AgentLoop.create(id, options)` DROPS `options.meta` — only the programmatic factory `create` threads it.** The convenience `create()` prepares its session with a hardcoded `{ meta: {} }`; a test (or caller) that needs `session.header.cwd` or other header metadata to take effect must use the factory `ctx.agents.create({ agentId, sessionId, meta, agentOptions })` (which passes `meta: options.meta ?? {}`), or `resume` (which reloads the persisted header). Both are synchronous. A cwd-dependent test that silently sees an empty cwd is almost always the wrong creation path. See `packages/core/agent-loop/src/index.ts`.
+- **`AgentLoop.create(id, options)` DROPS `options.meta` — only the programmatic factory `create` threads it.** The convenience `create()` prepares its session with a hardcoded `{ meta: {} }`; a test (or caller) that needs `session.header.cwd` or other header metadata to take effect must use the factory `ctx.agents.create({ agentId, sessionId, meta, agentOptions })` (which passes `meta: options.meta ?? {}`), or `resume` (which reloads the persisted header). `create` is synchronous; `resume` is async — it awaits the persisted load. A cwd-dependent test that silently sees an empty cwd is almost always the wrong creation path. See `packages/core/agent-loop/src/index.ts`.
 
 ## Type Safety and Documentation
 

From 4e70489c7335080ad7c6b13d8ad2360180f65133 Mon Sep 17 00:00:00 2001
From: Tianyi Cui <53024+tianyicui@users.noreply.github.com>
Date: Sat, 4 Jul 2026 00:04:48 +0800
Subject: [PATCH 4/4] docs(AGENTS): collapse the section to its theme; drop the
 two Defensive-patterns additions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The bulleted lessons and the two Defensive-patterns traps did not earn their
place. Reduce the new section to its one load-bearing sentence — gates prove
lines ran, not semantics/doc-accuracy/that a test guards anything; layer
judgment on top and lean on an independent reviewer for what gates can't see —
and drop the spawn-narrowing and AgentLoop.create-meta bullets entirely.
---
 AGENTS.md | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 4b408fafb5..5f06b26765 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -34,15 +34,9 @@ A wave of review comments lands across several PRs in a dependent stack (`A ←
 - **Delegated work is trust-but-verify.** When sub-agents implement fixes in parallel, their report describes what they INTENDED, not necessarily what landed. Re-run the gates yourself on the actual tree, and for a regression guard, **prove it FAILS on the unfixed code** (introduce the regression, watch the test go red, revert) — a guard that passes both ways guards nothing. A sub-agent that "reframes the problem as already-handled" instead of fixing it is a signal to dig in personally, not to accept the reframing.
 - **Triage on the merits, then reply in-thread.** Verify each comment against the code before acting (a reviewer flagging the right symptom can still mis-diagnose the cause — confirm both). Reply in the GitHub review thread (`gh api …/pulls/{pr}/comments/{id}/replies`), not as a top-level comment, stating the fix and the commit that carries it.
 
-## Landing changes cleanly: gates, Codex, and scope
+## Landing changes cleanly: gates and judgment
 
-The recurring failure mode: a mechanical gate proves lines ran and types check; it never proves semantics, doc accuracy, or that a test guards anything. Layer the cheap human/AI judgment on top, in the right order, and keep each unit of work honestly scoped.
-
-- **Run `pnpm run test:coverage` (the FULL suite), not an isolated `-t` filter, before trusting green.** Test-isolation bugs surface only in the full run: fixed-`setTimeout` waits that pass in isolation race under full-suite load and flake together — replace every fixed sleep with a `waitFor(predicate)` poll (ties to [§ Defensive patterns](#defensive-patterns-hard-won) "Async state is not synchronous state"). A suite green under `-t <name>` but red under `test:coverage` is telling you about shared state, not a flake to rerun.
-- **Codex convergence is for the class of defect gates STRUCTURALLY cannot catch — spend it there.** `xhigh` Codex reliably finds what `typecheck`/`lint`/`coverage`/`doc-sync` are blind to: (a) **prose/RFC/comment drift** the doc-sync scope doesn't scan — a package README still advertising a removed event, or an RFC claiming a decision "carries context too" when that union has no such field; (b) **a bug you INTRODUCED while fixing** — the fix's own new branch, un-covered by the test you wrote for the original bug; (c) **dishonest test comments** blessing a wrong assertion. Treat a Codex finding as a claim to verify against the code, then re-bucket it yourself (its own (a)/(b)/(c) label is an input, not a verdict) — but know that "clean gates" is exactly when Codex earns its keep.
-- **Scope a Codex review to ONE fix or concern.** A convergence prompt bundling two independent fixes plus verification context timed out at the 850s cap with no verdict, then completed fine once split into two smaller serial reviews. One concern per review is faster AND yields a sharper verdict. (For the invocation: the prompt is a POSITIONAL arg to `ask-codex.sh`, not `--file`; the only flags are `--codex-model`, `--codex-timeout`. Multi-paragraph prompts go via `"$(cat file)"`.)
-- **Before deleting an event or seam, enumerate every consumer and prove redundancy, then grill the removal ("am I deleting a seam someone will re-add?").** A seam that is a pure mirror of something a consumer already reads is safe to cut; a live-only field one consumer still uses is not — prove which before removing, not after.
-- **Regenerate a generated artifact as PART of the edit that invalidates it, not as a gate to fail.** Know what triggers each: `docs/cordis-catalog/events-and-services.md` is generated from the `interface Events` / `interface Context` member JSDoc (not top module docs), so run `pnpm run gen-cordis-catalog` in the same step you touch an event/service declaration or its JSDoc — rather than letting `verify-cordis-catalog` (part of `doc-sync`) discover it stale.
+The recurring failure mode: a mechanical gate proves lines ran and types check; it never proves semantics, doc accuracy, or that a test guards anything. Layer the cheap human/AI judgment on top, in the right order, and keep each unit of work honestly scoped — and lean on an independent agent to review for the class of defect gates structurally cannot catch (prose/RFC/comment drift, a bug introduced while fixing, a test that asserts nothing load-bearing).
 
 ## Architecture
 
@@ -285,8 +279,6 @@ Each bullet is a bug class that bit us; the rule prevents the reoccurrence.
   - **A real-load-path test only GUARDS the export shape if a broken shape actually FAILS it.** The original crash (`cannot get property … without inject`) fired because that plugin HAS `inject`. A plugin with NO `inject` (a composition/bundle plugin that mounts children carrying their own inject, e.g. `dsh-agent-core` and the app packages) does NOT crash on a stray `export default` — `unwrapExports` silently drops `Config`/`name` and the plugin boots anyway — so a Loader smoke stays green while the export shape is broken. For such plugins add an EXPLICIT assertion that the regression fails: `expect('default' in mod).toBe(false)` plus running the module through the real `Loader.prototype.unwrapExports` and asserting `name`/`Config`/`apply` survive. Prove it: add `export default apply`, watch the test go red, revert.
   - **"Real entry path" means the PUBLISHED ARTIFACT, not the dev runtime.** A test (or a `demo:*` smoke) that boots `src/bin.ts` under `tsx` is NOT the same code a consumer runs — the package `bin` field points at the built `lib/bin.js` under plain `node`. tsx masks failure modes the published artifact has: a boot settle-race that exits 0 before the app's handles attach, module-resolution differences (the unbuilt `paths` map vs node_modules), and a load failure that `loader.await()`'s `Promise.allSettled` SWALLOWS so a typo'd config silently exits 0. The guard is a smoke that runs the built `lib/bin.js` under plain `node` in a node_modules-shaped temp dir (symlinked workspace + vendor packages), asserts the real output, AND asserts a genuinely-missing config exits NON-ZERO. The tsx demo is necessary but not sufficient; the published-bin smoke is what catches "green under tsx, broken on install".
 - **Tag spelling and EOF hygiene.** cordis.yml interpolates env via the `!!js` tag (js-yaml resolves custom tags under `tag:yaml.org,2002:js`), not `!js` — keep code, comments, and docs consistent. Files end with exactly one trailing newline; `git diff --check` (a pre-push gate) rejects new blank lines at EOF.
-- **`child_process.spawn` narrows non-null `stdout`/`stderr` only from a LITERAL `stdio` tuple.** A ternary or variable in a `stdio` slot (e.g. `stdio: [wantStdin ? 'pipe' : 'ignore', 'pipe', 'pipe']`) selects the generic `spawn` overload, widening the child's streams to nullable — which then trips `no-non-null-assertion` (forbidden in `src`). Write two full `spawn(...)` calls with literal tuples in an `if`/`else` (or a ternary between two complete calls), as [`dsh-bash-local`'s `run.ts`](packages/bash/bash-local/src/run.ts) does, so each branch's literal tuple keeps the typed overload. This trap bit twice — recognize it the moment a conditional `stdio` slot appears.
-- **`AgentLoop.create(id, options)` DROPS `options.meta` — only the programmatic factory `create` threads it.** The convenience `create()` prepares its session with a hardcoded `{ meta: {} }`; a test (or caller) that needs `session.header.cwd` or other header metadata to take effect must use the factory `ctx.agents.create({ agentId, sessionId, meta, agentOptions })` (which passes `meta: options.meta ?? {}`), or `resume` (which reloads the persisted header). `create` is synchronous; `resume` is async — it awaits the persisted load. A cwd-dependent test that silently sees an empty cwd is almost always the wrong creation path. See `packages/core/agent-loop/src/index.ts`.
 
 ## Type Safety and Documentation