docs(rfc): define and enforce a uniform RFC format; adopt it across the corpus

Define the in-file RFC contract in docs/rfc/README.md § The file format: the header block (`# RFC: <title>` plus a dateless Status enum cross-checked against the lifecycle folder), the per-lifecycle body skeleton (a Problem opener everywhere; Proposal/Alternatives considered/ Acceptance criteria/Risks in proposed/; present-tense Decision/ Consequences with proposal-era headings banned in implemented/; the frozen proposal shape in rejected/), and a mandatory Alternatives considered section with a date-fenced grandfather comment for pre-format RFCs whose alternatives are not reconstructible from the record. Enforce it with a new doc-sync gate, scripts/verify-rfc-format.ts, and normalize all 112 RFCs to it: ~15 Status-line spellings collapse to the enum, 29 Context openers become Problem, the 39 legacy-format XXX debt markers are resolved and banned from reappearing, proposal-era sections in implemented RFCs are rewritten to shipped reality (including the web/fs/subagent seam RFCs' migration plans and test checklists, closing the doc-tiers deferred-work item on the web seam), every RFC gains an Alternatives considered section or the grandfather comment, and the bilingual pair is re-mirrored and re-recorded. Move the generated index tables out of README.md into a fully generated docs/rfc/INDEX.md — gen-rfc-index now writes the whole file, and verify-rfc-classification checks its freshness and rejects index-shaped rows in the curated README — which makes room for the format contract to live in the README front door instead of a separate FORMAT.md. The decision record, and the first RFC written in the new format, is docs/rfc/implemented/process/2026-07-05-uniform-rfc-format.md.
2026-07-05 22:58:25 +08:00
parent d5cb3297ad
commit e6fad266a6
123 changed files with 1221 additions and 847 deletions
@@ -1,12 +1,10 @@
 # RFC: Property-based testing for protocol-shaped code

-Status: implemented (proposed 2026-06-11, accepted 2026-06-14)
-
-<!-- XXX: legacy ADR/RFC body format, not yet normalized to a unified RFC template. -->
+Status: implemented

 > Merges the original proposal and the decision record for one topic. It found a real BlockAssembler duplicate-`block-end` bug on first run.

-## Context
+## Problem

 Example-based tests pin the cases we thought of. The harness's core is protocol-shaped — chunk streams, event logs, schema conversion, inbox scheduling — where the input space is combinatorial and the interesting bugs live in interleavings nobody wrote an example for. The motivating evidence: a block-assembly ordering bug once survived 100% line coverage of the happy paths. Per-file 100% coverage proves every line ran, not that every interleaving is correct.

@@ -25,3 +23,5 @@ Adopt `fast-check` (a root devDependency) with one `tests/properties.spec.ts` pe
 - **It already paid off:** the BlockAssembler stream found a real bug — a duplicate `block-end` at the same index overwrote an already-flushed block, so the streamed prefix disagreed with final `blocks()`. Fixed (first close wins, matching the existing straggler rule) with a dedicated regression test.
 - A property flake from a timeout is a finding, not something to retry away. The loop properties are deterministic by construction (settle on `agent/status`), so a hang is a real defect.
 - Property tests supplement, not replace, the example tests that pin specific branches for the 100%-coverage gate.
+
+<!-- rfc-format: alternatives-not-recorded (pre-format RFC) -->
@@ -1,10 +1,8 @@
 # RFC: ACP snapshot tests — record-once / replay-deterministic

-Status: implemented (accepted 2026-06-19)
+Status: implemented

-<!-- XXX: legacy ADR/RFC body format, not yet normalized to a unified RFC template. -->
-
-## Context
+## Problem

 The harness has two test tiers: keyless unit `.spec.ts` (the 100%-per-file coverage gate) and real-API `.e2e.ts` (key-gated, self-skipping in CI). Neither continuously verifies the **complete output transcript** an ACP editor (Zed) sees on its stdin/stdout. The existing ACP e2e ([examples/acp-agent/tests/acp.e2e.ts](../../../../examples/acp-agent/tests/acp.e2e.ts)) is the closest end-to-end check, but it is key-gated and asserts on a handful of *structured fields* (`stopReason`, a `tool_call` title), not the byte-for-byte stream of `session/update` frames. That leaves the "green units, broken product" gap: every unit test can pass while the actual editor-facing protocol output regresses — the same class of failure that shipped the inject bug ([docs/postmortem/0001](../../../postmortem/0001-acp-default-export-drops-inject.md)), where 178 hand-mounted tests stayed green while a real Zed session crashed instantly.

@@ -72,6 +70,12 @@ The replay plugin lives in its own package, `@deepseek-ai/dsh-llm-replay` (`pack

 `pnpm run test:snapshot` runs replay (keyless) and is composed into the default `pnpm run test` gate so every PR gets the regression check (the main `vitest.config.ts` include stays narrow; the gate is `test && test:snapshot`). `pnpm run test:snapshot:record` requires `DEEPSEEK_API_KEY` (loaded from repo `.env` first), hits the real API, harvests the produced `session.jsonl` (the replay source AND the expected-log artifact), and `--update`s the stdout golden in one pass. Both forward a scenario filter. A missing fixture in replay **fails loud** with a "record first" message rather than self-skipping (the e2e self-skip rule is a CI-secret accommodation, not appropriate here — a committed-fixture test that silently vanishes is a coverage hole). A no-model scenario's `session.jsonl` simply has no `assistant/chunk` events (empty derived script); fail-loud still applies if a model call happens with no entry. An orphan-fixture guard test fails on a golden/fixture not referenced by any scenario (Vitest does not prune orphaned raw goldens), and a per-kind required-fixture guard asserts each scenario ships exactly the files its kind needs (`input.json` + `stdout.golden.jsonl` + `session.jsonl` for ALL scenarios — the harness passes `<dir>/session.jsonl` to `llm-replay` unconditionally, so even a no-model scenario needs its header-only fixture or `loadReplayScript()` fails; `replay.override.json` additionally for authored model scenarios).

+## Alternatives considered
+
+- **A hand-authored `llm.json` of model chunks** — the earlier draft; reusing the real session log makes the fixture a genuine product of the system rather than a hand-built mock, and doubles it as a behavioral golden.
+- **A byte-level HTTP-record library (Polly/nock/MSW)** — rejected: adapter-specific, awkward with streaming SSE, and lower-level than the thing under test.
+- **Synthesizing throw/cancel entries from `turn/end {kind:'error'|'aborted'}`** — rejected: it couples `llm-replay` to loop-internal turn-closing semantics, and the `turn/end` reason is lossy (it cannot distinguish a thrown 401 from a finish-error); the explicit `replay.override.json` sidecar is the cleaner seam.
+
 ## Consequences

 A new test tier and its fixtures to maintain: each scenario is a directory of `input.json` (the client stdin script) + `session.jsonl` (the recorded log, which doubles as the expected re-persisted log) + an optional `replay.override.json` + an optional `workspace/` seed dir + the `stdout.golden.jsonl`, committed and reviewed. A scenario that needs the agent to operate on existing files (read, edit, grep) ships a `<scenario>/workspace/` directory; the harness copies its contents into the temp cwd before the run, so the seeded files are present for both record and replay (the cwd is normalized in the goldens, so the seeded paths stay stable). Re-recording when the model's phrasing changes churns the fixture and the stdout golden — visible in review, which is the point of committing them. Bought: deterministic, keyless, full-transcript regression coverage that boots the real Loader (so it still guards the export-shape bug class), exercises the real bash executor, and gives a one-command accept-the-diff loop. The tier is ACP-first but the harness (subprocess + tee + input-DSL + workspace seeding + normalization + JSONL-derived replay) is example-agnostic and extends to other examples.
@@ -1,10 +1,8 @@
 # RFC: Real-API e2e in CI against the external DeepSeek API

-Status: implemented (accepted 2026-06-19)
+Status: implemented

-<!-- XXX: legacy ADR/RFC body format, not yet normalized to a unified RFC template. -->
-
-## Context
+## Problem

 The harness leans hard on real-API tests by policy: [docs/testing.md](../../../testing.md) argues that a no-key suite proves the plumbing but not the product, and the [ACP inject postmortem](../../../postmortem/0001-acp-default-export-drops-inject.md) is the standing proof — 178 keyless tests stayed green while a real editor session crashed instantly. The real-API e2e suite (`pnpm run test:e2e`, the `*.e2e.ts` files) exists precisely to close that gap: it drives the agent against the live DeepSeek API — real model calls, real bash tools, multi-turn, resume, ACP-over-stdio.

@@ -86,6 +84,11 @@ What gets worse is the *surrounding* model, and these are the things to address

 None of these require changing the workflow to go public; they are operational steps plus the already-added `pull_request_target` guard comment.

+## Alternatives considered
+
+- **A secret-consuming job inside ci.yml** — rejected: it would couple the keyless, forkable, always-green gate to credential availability and a different trigger/concurrency policy; different lifecycles, different files.
+- **Omitting the `pull_request` trigger** (the smaller key-exposure surface) — rejected for the pre-merge signal; the Security section carries the accepted exposure analysis.
+
 ## Consequences

 A second CI workflow and the first repo secret to maintain. The real-API suite now gates merges (pre-merge on trusted PRs, post-merge on the main branch) and runs nightly, so a real break in the agent's interaction with the external API surfaces in CI rather than only in a developer's local run — at the cost of real (but internally free) API calls on every trusted PR and merge. The preflight makes secret misconfiguration self-announcing instead of silently disabling the net.
@@ -1,6 +1,6 @@
 # RFC: Use `session.jsonl` as the only snapshot session-log artifact

-Status: implemented (proposed and accepted 2026-06-20)
+Status: implemented

 ## Problem

@@ -8,9 +8,9 @@ Model-driving ACP snapshot scenarios ship both `session.jsonl` and `session.gold

 Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. The override file is a JSON array of `ReplayEntry` objects: `{ "kind": "chunks", "chunks": StreamChunk[] }`, `{ "kind": "throw", "chunks": StreamChunk[], "message": string, "code": string, "status"?: number }`, or `{ "kind": "hang" }`. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario.

-## Proposal
+## Decision

-Remove the `session.golden.jsonl` concept entirely. Every scenario has at most one committed session-log artifact, `session.jsonl`:
+The `session.golden.jsonl` concept is removed entirely. Every scenario has at most one committed session-log artifact, `session.jsonl`:

 - For recorded scenarios, `session.jsonl` remains the raw harvested log. Replay still derives model chunks from it, and the snapshot test compares the replay run's normalized persisted log against normalized `session.jsonl`.
 - For authored override scenarios, `replay.override.json` drives model behavior and `session.jsonl` holds the expected produced session log. The replay adapter ignores the fixture for model chunks when the override exists, so the same file can be the expected log without affecting replay behavior.
@@ -18,15 +18,15 @@ Remove the `session.golden.jsonl` concept entirely. Every scenario has at most o

 Stdout goldens remain unchanged; they are the editor-facing projection and are not redundant with the session fixture.

-## Acceptance criteria
+## Alternatives considered

- `session.golden.jsonl` disappears from the snapshot harness, fixtures, orphan guards, and docs.
- The snapshot test derives the expected session log from `session.jsonl` for every model scenario.
- Authored sidecar scenarios commit their expected produced log in `session.jsonl`; `replay.override.json` remains the model-behavior override.
- Orphan-fixture guards understand which files are required by scenario kind.
- The [ACP snapshot tests RFC](../../implemented/testing/2026-06-19-acp-snapshot-tests.md) is updated to describe the reduced fixture set.
+**Normalizing both sides against a shared (replay-run) context** — rejected: `normalizeSessionLog` scrubs cwd by exact string match, so the fixture's recorded cwd would survive unscrubbed and every compare would fail. Each side normalizes against its own header-derived context — the implementation note below carries the mechanics.

-## What we give up
+## Verification
+
+`session.golden.jsonl` appears nowhere in the snapshot harness, fixtures, orphan guards, or docs; the snapshot test derives the expected session log from `session.jsonl` for every model scenario; authored sidecar scenarios commit their expected produced log as `session.jsonl` with `replay.override.json` as the model-behavior override; and the orphan-fixture guards know which files each scenario kind requires. The [ACP snapshot tests RFC](../../implemented/testing/2026-06-19-acp-snapshot-tests.md) describes the reduced fixture set.
+
+## Consequences

 Reviewers lose one artifact name that made the expected persisted log visually separate from the replay fixture. The stdout golden still protects the editor transcript, and comparing replay output to `session.jsonl` preserves the loop/persistence regression check without duplicating files.

@@ -25,3 +25,5 @@ The fork backend seeds the child with the parent's **balanced completed-turn pre
 - `subagent-mixed` is the first snapshot scenario to drive two *different* subagent backends in one transcript, exercising the per-session replay keying across a spawn and a fork child simultaneously.
 - Out-of-process (ACP) subagent replay remains a different shape (each child is its own process with its own replay) and is still tracked as `TODO(acp-subagent-replay)` — these scenarios are in-process only.
 - Re-recording (`pnpm run test:snapshot:record`) regenerates all four fork/spawn fixtures from the live API; the two new scenarios self-skip without a key like every recorded scenario.
+
+<!-- rfc-format: alternatives-not-recorded (pre-format RFC) -->
@@ -31,6 +31,8 @@ This keys by WHO calls, not by global call order — so it stays correct even if

 The ordering key is the session header `createdAt`. In the current synchronous cut this is sound because sibling children are created **strictly sequentially** — the subagent tool awaits one child's result and disposes it before the parent's next tool call starts the next child — so their `createdAt` values are strictly ordered and match first-call order exactly. A same-millisecond sibling tie is therefore unreachable; the `recordedId` tiebreak only keeps such a degenerate collision deterministic, it does not recover first-call order. A future cut that runs siblings concurrently/backgrounded WOULD be able to create two children in the same millisecond, and must then thread a real first-call ordinal (the order live sessions first stream) rather than leaning on `createdAt` — flagged with `XXX(concurrent-subagents)` at the sort site.

+## Alternatives considered
+
 The alternative considered and rejected was a **call-ordered merge of the parent and child logs** into one global script (sound only because in-process subagent execution is strictly nested — the parent blocks on the child). It is simpler for today's synchronous cut but bakes in the parent-blocks-on-child invariant that a future backgrounded/concurrent subagent would break; per-session keying does not.

 ### 3. The harness harvests every log, primary-first
@@ -44,3 +44,5 @@ The matrix therefore covers every hook point that has a DETERMINISTIC, OBSERVABL
 - The block scenarios are keyless (no model turn); the rest replay keyless from recorded fixtures. `pnpm run test:snapshot:record` regenerates the recorded fixtures from the live API and self-skips without a key like every recorded scenario.
 - The prove-red discipline holds: tampering a hook config's output (e.g. changing a deny reason) turns its scenario red on replay — the hook process runs FOR REAL during replay (only the model is replayed), so the golden guards the actual hook→seam→loop path, not a mock of it.
 - The `acp-agent` demo now loads a Codex bridge it will usually no-op (no `codex-hooks.json` in a typical project), which is the intended fail-soft behavior, not a cost.
+
+<!-- rfc-format: alternatives-not-recorded (pre-format RFC) -->
@@ -12,7 +12,9 @@ Status: implemented

 One vendored-plugin fact the overlay depends on, deliberately: the include applies `patches` when it loads the file — its `refresh()`/`internal/update` paths re-read without re-patching — which is exactly enough for a one-shot replay boot (the replay app loads no `hmr` and nothing rewrites the config mid-run). The snapshot suite is the proof: all scenarios pass unchanged on the overlay, byte-identical goldens included.

-## Why not the alternatives?
+## Alternatives considered
+
+### Why not the alternatives?

 Keeping the full twin with a symmetry verify-gate was the recorded fallback — it would have removed the silent-drift class but kept a 125-line near-copy whose only content was one entry's difference, growing with every plugin the app gains. A bin-side swap (parse the config, replace the entry, delete the file) would have put YAML surgery inside a published artifact and moved the replay delta out of sight; the overlay keeps the delta declarative, readable, and next to the base config — the teaching value the twin's defenders actually wanted.