fix(docs): address Codex review round 2 — repoint every remaining moved-policy citation

The definitive sweep (audit every AGENTS.md mention in packages/, docs/, examples/, scripts/) found thirteen more citations of relocated policy and two citations of rules that never existed as quoted: - with-key policy comments (web deepseek/perplexity e2e headers) -> docs/testing.md; real-impl-over-mock comments (acp harness, load, stream-update specs) -> docs/testing.md; defensive-pattern quotes (acp index.ts x3, stream-update) -> docs/defensive-patterns.md. - md-tier repoints: real-api-e2e RFC, tool-schema-catalog RFC, postmortem 0001 guardrail row, adding-a-package cookbook, drop-bash-output-spill-files RFC, acp-subagent-backend RFC phrasing. - Two false attributions dropped in favor of self-contained reasoning: tool-todo's 'don't validate scenarios that can't happen' and the bash-stdin-env RFC's 'Don't add features beyond what the task requires' (neither rule ever existed under those names). - Citations of the two 'not golden truth' doctrines stay: those bullets survive verbatim in the root conventions. Note: packages/support/ui-stdio readline TTY spec flakes under full coverage on a heavily loaded box (passes standalone and passed the same tree's coverage run minutes earlier); untouched by this stack.
2026-07-04 15:43:51 +08:00
parent c3424e5487
commit 26339144e3
14 changed files with 18 additions and 18 deletions
@@ -46,4 +46,4 @@ pnpm run test:coverage  # 100% per-file over src (types.ts exempt)
 pnpm run build && pnpm run hygiene
 ```

-Test expectations: every registry/registration needs an HMR-safety test (register from a child fiber, dispose it, assert cleanup). Excessive tests are welcome — see AGENTS.md.
+Test expectations: every registry/registration needs an HMR-safety test (register from a child fiber, dispose it, assert cleanup). Excessive tests are welcome — see [docs/testing.md](../testing.md).
@@ -101,7 +101,7 @@ Both bugs share one root process gap: **no test exercised the plugin through its
 - **`AgentLoop.resume` reads `this.ctx.get('sessionPersistence')`** (`packages/core/agent-loop/src/index.ts`) — the Bug #2 fix, with a comment explaining the shadow-walk trap.
 - **No-key `session/new` e2e over real stdio** (`examples/acp-agent/tests/acp.e2e.ts`): boots the example as a subprocess through the real Loader and asserts `session/new` resolves. This fails loudly on Bug #1 with no API key. Verified it fails when `export default apply` is restored.
 - **`TSX_TSCONFIG_PATH` in the e2e spawn**: the subprocess runs from a temp cwd, where tsx cannot find the repo-root tsconfig `paths` map by searching upward — so dsh-* imports silently fell back to built `lib/`. Pointing tsx at the repo tsconfig makes resolution cwd-independent and ensures the test runs *source*, not a possibly-stale build.
- **AGENTS.md defensive pattern**: "Line coverage is not behavior coverage; test the REAL entry path, not a synthetic stand-in" — codifies the lesson for every future plugin.
+- **[docs/testing.md](../testing.md) rule**: "test the real entry path", line coverage is not behavior coverage — codifies the lesson for every future plugin.

 ## Lessons

@@ -26,7 +26,7 @@ Three deliberate choices:

 ## Scope: configurable scrub pattern is NOT included

-An earlier sketch of this work also proposed making `SENSITIVE_ENV_PATTERN` configurable. Validating against the code, that is **speculative and already subsumed**: `run.ts` documents a configurable whitelist as future work, and the new explicit `env` field — merged after the scrub — already gives a caller full control, including over credential-shaped vars. There is no current caller that needs to *broaden* the ambient scrub (the hazard runs the other way). Adding a config knob now would be a feature with no consumer, against [AGENTS.md](../../../../AGENTS.md) § "Don't add features beyond what the task requires". If a real workflow ever needs to forward a specific ambient credential, the explicit `env` field is the supported path; a configurable scrub can be reconsidered then.
+An earlier sketch of this work also proposed making `SENSITIVE_ENV_PATTERN` configurable. Validating against the code, that is **speculative and already subsumed**: `run.ts` documents a configurable whitelist as future work, and the new explicit `env` field — merged after the scrub — already gives a caller full control, including over credential-shaped vars. There is no current caller that needs to *broaden* the ambient scrub (the hazard runs the other way). Adding a config knob now would be a speculative surface with no consumer. If a real workflow ever needs to forward a specific ambient credential, the explicit `env` field is the supported path; a configurable scrub can be reconsidered then.

 ## Consequences

@@ -36,7 +36,7 @@ The child is a separate process, so it inherits an environment. Credential-shape

 ## Testing

-Designed at every tier the backend touches, per the AGENTS.md "design test infrastructure up front" rule:
+Designed at every tier the backend touches, per the root AGENTS.md rule that a new capability shape names its coverage at every tier at plan time:

 - **Keyless unit/integration** (`subagent-acp.spec.ts`): spawns a scripted mock ACP server subprocess (`tests/mock-acp-server.ts`) and drives it through the real backend over real ACP stdio. Covers: the prompt round-trip + output accumulation; every StopReason mapping; cancellation via `run.cancel()` and via the request signal; the already-aborted-before-start case; the cancel-races-ahead-of-newSession case; a torn-pipe-after-cancel (child crashes on cancel) settling `aborted`; permission auto-answer under both policies (including the allow-policy-no-allow-option fallback); a non-message update consumed but not accumulated; a nonexistent-command spawn failure settling `error`; HMR provider cleanup; and the namespace export shape. 100% per-file coverage.
 - **With-key e2e** (`subagent-acp.e2e.ts`): the harness drives ITSELF — the backend spawns the real `acp-agent` example process and a real model in that child answers a prompt (PONG) and does real file work (writes `proof.txt`, verified on disk). Self-skips without `DEEPSEEK_API_KEY`. This is the "talk to our own process" smoke and the out-of-process analogue of the in-process spawn e2e.
@@ -19,7 +19,7 @@ The cordis catalog is a pure TypeScript-AST pass because every event/service nam
 - `tool-subagent`'s tool name is `config.toolName ?? 'subagent'` — chosen at load, not a literal.
 - An MCP plugin can register **raw JSON Schema** directly via `ctx.tools.register()` without `defineTool` at all, so enumerating `defineTool(` call sites structurally under-counts.

-The only faithful source of truth is the schema the registry actually holds after the plugin loads. Booting is the [unit-test discipline](../../../../AGENTS.md) "verify the world, not a synthetic stand-in" applied to a doc generator: read the shipped artifact, not a re-derivation of it.
+The only faithful source of truth is the schema the registry actually holds after the plugin loads. Booting is the [testing-policy discipline](../../../testing.md) "verify the world, not the self-report" applied to a doc generator: read the shipped artifact, not a re-derivation of it.

 ### Restoring "nothing silently omitted"

@@ -22,7 +22,7 @@ ci.yml's value is that it is keyless, forkable, and always-green: any contributo

 ### Cost is not the constraint; reliability is

-The usual reason to ration real-API CI — token cost — does not apply here: we are DeepSeek and internal inference is effectively free. So the design optimizes for *coverage and signal*, not for minimizing calls. The suite runs in full (all six `*.e2e.ts` files), on multiple triggers, on every trusted PR. This is the CI embodiment of the AGENTS.md "lean on with-key e2e tests" policy.
+The usual reason to ration real-API CI — token cost — does not apply here: we are DeepSeek and internal inference is effectively free. So the design optimizes for *coverage and signal*, not for minimizing calls. The suite runs in full (all six `*.e2e.ts` files), on multiple triggers, on every trusted PR. This is the CI embodiment of the [docs/testing.md](../../../testing.md) with-key policy.

 ### Triggers: trusted events only

@@ -20,7 +20,7 @@ This proposal can land independently of [a generic long-running tool runtime](..
 - `OutputCollector` keeps bounded buffers only and deletes the temp-file machinery.
 - `renderResult()` reports truncation without a filesystem path.
 - Tests cover tail truncation and no longer assert full-output file contents.
- Security guidance in [root AGENTS.md](../../../../AGENTS.md) stops treating private spill files as a model-visible interface.
+- Security guidance in [docs/defensive-patterns.md](../../../defensive-patterns.md) stops treating private spill files as a model-visible interface.

 ## What we give up

@@ -51,8 +51,8 @@ const DESCRIPTION =
 * `InferArgs` maps an `enum` string prop to plain `string`, so the compiler sees
 * `args.todos` as `{ content: string; status: string }[]`; the
 * `status as TodoItem['status']` narrowing records that registry guarantee
- * rather than re-checking it (an unreachable re-check would be dead code — see
- * AGENTS.md "don't validate scenarios that can't happen"). What remains is the
+ * rather than re-checking it (an unreachable re-check would be dead code the
+ * coverage gate would flag). What remains is the
 * value rules the DSL has no vocabulary for: non-empty unique content (stored
 * trimmed, so the persisted value matches the dedupe/length key), and at most
 * one `in_progress` task.
@@ -284,7 +284,7 @@ export function apply(ctx: Context, config: AcpConfig): void {
    // sessionUpdate returns a promise; a closed connection rejects it. The
    // update is best-effort UI feed, never load-bearing for correctness, so a
    // throwing/rejecting send must not break the turn (the chunk is emitted
-    // inside the model step — see AGENTS.md "contain callback exceptions").
+    // inside the model step — see docs/defensive-patterns.md "contain callback exceptions").
    /* v8 ignore next 3 -- the rejection only fires on a stdout/connection write
       failure (closed pipe), which the in-memory test transport never induces;
       the swallow is a defensive best-effort guard like the loop's emit traps */
@@ -639,7 +639,7 @@ export function apply(ctx: Context, config: AcpConfig): void {
  conn = new AgentSideConnection(makeAgent, stream)

  /**
-   * Tear ALL live sessions down to quiescence (AGENTS.md "dispose must reach
+   * Tear ALL live sessions down to quiescence (docs/defensive-patterns.md "dispose must reach
   * quiescence"): for each session settle any pending prompt `cancelled`, then
   * run that session's {@link AgentHandle} `dispose()` — which stops the loop
   * (sets `disposed`, aborts the in-flight step), AWAITS the loop's exit (the
@@ -892,7 +892,7 @@ export class ToolPresenter {
   * @param onError invoked when a tool's `presentCall`/`presentResult` THROWS;
   *   the presenter swallows the error and falls back to the generic
   *   presentation so a buggy display callback can never fail a live turn or a
-   *   `session/load` replay (AGENTS.md "contain callback exceptions at the
+   *   `session/load` replay (docs/defensive-patterns.md "contain callback exceptions at the
   *   boundary"). Defaults to a no-op for callers that don't supply a logger.
   */
  constructor(
@@ -158,7 +158,7 @@ export async function makeBridgeHarness(options: {
   * Plug the REAL `dsh-bash-local` executor + `dsh-tool-bash` tools (instead of
   * a test's own inline tool). Lets a test drive the actual `bash` tool — its
   * real `presentCall`/`presentResult` — through the bridge, so tool-call UI
-   * tests verify the SHIPPING tool, not a stand-in (AGENTS.md "prefer the real
+   * tests verify the SHIPPING tool, not a stand-in (docs/testing.md "prefer the real
   * implementation over a mock in tests").
   */
  withBash?: boolean
@@ -62,7 +62,7 @@ describe('acp bridge — session/load replay', () => {
    // bridge. The replayed tool_call/tool_call_update must carry the tool's OWN
    // presentation — identical to how it streamed live — via a throwaway
    // presenter that pairs call→result as the log replays in order. Uses the
-    // shipping tool (withBash), not a stand-in (AGENTS.md "prefer the real
+    // shipping tool (withBash), not a stand-in (docs/testing.md "prefer the real
    // implementation over a mock in tests").
    live = await makeBridgeHarness({
      storageDir,
@@ -287,7 +287,7 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () =>

  it('a THROWING presentCall/presentResult is contained: generic fallback + onError, never propagates', () => {
    // A buggy tool whose display callbacks throw must NOT fail a live turn or a
-    // session/load replay (AGENTS.md "contain callback exceptions at the
+    // session/load replay (docs/defensive-patterns.md "contain callback exceptions at the
    // boundary"). The presenter swallows the throw, reports via onError, and
    // falls back to the generic presentation.
    const boom: ToolDefinition = {
@@ -379,7 +379,7 @@ describe('ToolPresenter (tool-owned presentation via the tool registry)', () =>
  it('forwards fs-tool render intents onto the wire (REAL read → generic locations, edit → diff content)', async () => {
    // Use the SHIPPING fs tools (not a stand-in), booted through their real
    // plugins, so the wire tool_call carries the actual presentCall output —
-    // read's follow-along `locations` and edit's `diff` content block. (AGENTS.md
+    // read's follow-along `locations` and edit's `diff` content block. (docs/testing.md
    // "prefer the real implementation over a mock".)
    const ctx = new Context()
    await ctx.plugin(SystemPrompt)
@@ -10,7 +10,7 @@ import {

 /**
 * Real-API smoke for the DeepSeek search provider. Self-skips without
- * `$DEEPSEEK_API_KEY`, per the with-key e2e policy in AGENTS.md § Secrets. This
+ * `$DEEPSEEK_API_KEY`, per the with-key e2e policy in docs/testing.md. This
 * is the only test that proves DeepSeek's Anthropic-compatible endpoint actually
 * triggers native `web_search` and returns the structured result blocks the
 * provider parses — a mock cannot confirm the wire shape is real.
@@ -3,7 +3,7 @@ import { PerplexitySearchProvider, PERPLEXITY_DEFAULT_BASE_URL, PERPLEXITY_DEFAU

 /**
 * Real-API smoke for the Perplexity search provider. Self-skips without
- * `$PERPLEXITY_API_KEY`, per the with-key e2e policy in AGENTS.md § Secrets.
+ * `$PERPLEXITY_API_KEY`, per the with-key e2e policy in docs/testing.md.
 */
 const apiKey = process.env.PERPLEXITY_API_KEY
 const maybe = apiKey !== undefined && apiKey.length > 0 ? describe : describe.skip