From a45bc8da67cb5d49d10de606e1fe1614174b2cc3 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 14 Jun 2026 10:36:16 +0800 Subject: [PATCH 1/2] fix(invariants): deepFreeze walks already-frozen objects' descendants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session.append accepts event data from arbitrary plugins/tools, so a caller can pass a SHALLOW-frozen object with mutable descendants. The old Object.isFrozen early-return skipped such an object entirely, leaving its descendants mutable in the log — exactly the history mutation ADR 0012 means to catch. Now always descend, tracking visited objects in a WeakSet for cycle-termination and idempotence. Addresses PR review finding. --- packages/invariants/src/index.ts | 22 +++++++++------ packages/invariants/tests/invariants.spec.ts | 29 ++++++++++++++------ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/packages/invariants/src/index.ts b/packages/invariants/src/index.ts index 6824ea76af..c5d9b30d00 100644 --- a/packages/invariants/src/index.ts +++ b/packages/invariants/src/index.ts @@ -66,19 +66,23 @@ interface SessionTrace { /** * Deep-freeze a value and everything reachable from it. * - * Sound because this is only ever called top-down on event objects we just - * appended: by the time a node is frozen, this same walk has already frozen - * its descendants, so a frozen node implies frozen descendants — skipping it - * is correct and avoids re-walking on HMR replay. (We never pass an - * externally shallow-frozen object, which is the only input that would make - * the early-return unsound.) + * Walks every object's own properties even when the object itself is already + * frozen: `Session.append()` accepts event data from arbitrary plugins/tools, + * so a caller can hand us a SHALLOW-frozen object whose descendants are still + * mutable. Skipping an already-frozen node (the obvious idempotence shortcut) + * would leave exactly the kind of mutable history ADR 0012 means to catch. A + * `WeakSet` of visited objects keeps it terminating on cycles and avoids + * re-walking shared subtrees / already-processed seed events. */ -function deepFreeze(value: unknown): void { +function deepFreeze(value: unknown, seen: WeakSet = new WeakSet()): void { if (value === null || typeof value !== 'object') return - if (Object.isFrozen(value)) return + if (seen.has(value)) return + seen.add(value) + // Freeze the node (no-op if a caller pre-froze it), then ALWAYS descend — + // a frozen container can still hold mutable children. Object.freeze(value) for (const key of Object.keys(value)) { - deepFreeze((value as Record)[key]) + deepFreeze((value as Record)[key], seen) } } diff --git a/packages/invariants/tests/invariants.spec.ts b/packages/invariants/tests/invariants.spec.ts index 7c8e383141..0a621434ab 100644 --- a/packages/invariants/tests/invariants.spec.ts +++ b/packages/invariants/tests/invariants.spec.ts @@ -222,16 +222,29 @@ describe('dev-freeze', () => { expect(Object.isFrozen(session.events[0])).toBe(true) }) - it('is idempotent over already-frozen sub-structures', async () => { + it('freezes mutable descendants of a shallow-frozen event datum', async () => { const { ctx } = await setup() const session = ctx.sessions.create() - // Pre-freeze a block before appending; deepFreeze must short-circuit on it - // (the already-frozen guard) while still freezing the enclosing event. - const block = Object.freeze({ type: 'text' as const, text: 'pre-frozen' }) - const event = session.append('user/message', { content: [block], source: { kind: 'user' } }) - expect(Object.isFrozen(event)).toBe(true) - expect(Object.isFrozen(event.data.content)).toBe(true) - expect(Object.isFrozen(event.data.content[0])).toBe(true) + // A caller hands in a SHALLOW-frozen block whose nested array is still + // mutable. deepFreeze must descend into the already-frozen object and + // freeze the descendant, not short-circuit on the frozen container — + // otherwise dev-mode misses exactly the history mutation ADR 0012 catches. + const innerContent: { type: 'text'; text: string }[] = [{ type: 'text', text: 'inner' }] + const block = Object.freeze({ type: 'tool-result' as const, toolCallId: CallId('c1'), content: innerContent, isError: false }) + session.append('user/message', { content: [block], source: { kind: 'user' } }) + expect(Object.isFrozen(block.content)).toBe(true) + expect(Object.isFrozen(block.content[0])).toBe(true) + expect(() => { block.content.push({ type: 'text', text: 'mutation' }) }).toThrow() + }) + + it('terminates on a cyclic event datum (WeakSet guard)', async () => { + const { ctx } = await setup() + const session = ctx.sessions.create() + // A self-referential structure must not loop forever. + const cyclic: Record = { type: 'text', text: 'x' } + cyclic['self'] = cyclic + expect(() => session.append('user/message', { content: [cyclic as never], source: { kind: 'user' } })).not.toThrow() + expect(Object.isFrozen(cyclic)).toBe(true) }) }) From fa7d1df6f2c6dbbae32ecbaf0fb8275c407b8828 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sun, 14 Jun 2026 10:40:20 +0800 Subject: [PATCH 2/2] build: run doc-sync gates in the local pre-push hook too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The doc-sync gates were CI-only, so the AGENTS.md doc-sync promise could be missed locally until after push. Add a shared `doc-sync` package.json script (doc-typecheck + verify-event-taxonomy) wired into the lefthook pre-push job, and point the CI step at the same script — one source of truth per ADR 0007. Addresses PR review finding. --- .github/workflows/ci.yml | 5 +++-- docs/adr/0014-doc-sync-enforcement.md | 6 +++--- lefthook.yml | 3 +++ package.json | 1 + 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d23efa17eb..21965beb21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,9 +46,10 @@ jobs: # Doc-sync gates (RFC 006). doc-typecheck compiles the fenced ts blocks in # the docs and resolves vendor packages via their built declarations, which # the typecheck step above emits — so it runs after typecheck. The event - # taxonomy check only reads source. + # taxonomy check only reads source. Same `doc-sync` script the pre-push + # hook runs (ADR 0007: one source of truth). - name: Doc-sync gates (doc code blocks + event taxonomy) - run: yarn doc-typecheck && yarn verify-event-taxonomy + run: yarn doc-sync - name: Tests with coverage gate (per-file 100%) run: yarn test:coverage diff --git a/docs/adr/0014-doc-sync-enforcement.md b/docs/adr/0014-doc-sync-enforcement.md index d6868f0e28..9a6596c46b 100644 --- a/docs/adr/0014-doc-sync-enforcement.md +++ b/docs/adr/0014-doc-sync-enforcement.md @@ -8,16 +8,16 @@ AGENTS.md promises that docs and code stay strictly in sync, but the promise was ## Decision -Two CI gates, mirroring the existing `scripts/` style (tsx ESM, one job each): +Two gates, mirroring the existing `scripts/` style (tsx ESM, one job each): 1. **`doc-typecheck`** extracts every fenced ` ```ts ` block from `README.md`, `docs/**`, and `packages/*/README.md`, writes them to a temp project, and compiles with `tsc --noEmit`. The temp tsconfig copies only resolution-relevant options and the workspace `paths` map from `tsconfig.typecheck.json` (vendor → built `lib`, harness → `src`) — resolving vendor to `lib` is essential, or tsc type-checks raw vendor source and floods the run. A block that is a deliberate sketch opts out with an explicit ` ```ts ignore-check ` info string; the script reports the opt-out ratio and fails if it exceeds half, so the escape hatch can't quietly become the norm. 2. **`verify-event-taxonomy`** extracts the event names from the `interface Events` blocks across `packages/*/src` and from the taxonomy table in `docs/architecture.md`, and asserts the two sets match exactly. Verify, don't generate: the table keeps its hand-written Mode/Purpose columns; only the set of names is checked. (Landing this surfaced three events the table had been missing — `tools/change`, `llm/adapter-change`, `system-prompt/change`.) -Both run in CI after `yarn typecheck` (which emits the vendor `lib/` that doc-typecheck resolves against). API-extractor golden reports (RFC 006 part 3) were deliberately **deferred** — low value for an internal monorepo where reviewers already see the source diff, and a heavy, finicky dependency. +Both run via a shared `doc-sync` package.json script that the lefthook pre-push hook and CI both invoke (ADR 0007: hooks and CI call the same scripts, so the gate fires locally before a push — not only after it). They run after `yarn typecheck` (which emits the vendor `lib/` that doc-typecheck resolves against). API-extractor golden reports (RFC 006 part 3) were deliberately **deferred** — low value for an internal monorepo where reviewers already see the source diff, and a heavy, finicky dependency. ## Consequences -- Doc drift in the two checkable classes now fails CI instead of waiting for a reviewer to notice. This is an instance of ADR 0007's "mechanical gates over prose." +- Doc drift in the two checkable classes now fails the pre-push hook and CI instead of waiting for a reviewer to notice. This is an instance of ADR 0007's "mechanical gates over prose." - Making doc snippets compile costs a few stub imports/`declare`s; the `ignore-check` ratio must stay low or the gate is theater (the ratio guard enforces this). - The taxonomy check is name-only — a wrong Mode or Purpose column still needs human review. Generating the table from source was considered and rejected as more machinery than the problem warrants. - API reports remain available to revisit if the packages are ever published externally. diff --git a/lefthook.yml b/lefthook.yml index 7d2d9985e8..6f29bb0990 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -27,3 +27,6 @@ pre-push: - name: hygiene run: yarn hygiene + + - name: doc-sync + run: yarn doc-sync diff --git a/package.json b/package.json index 72469c4ca7..c56ba924e5 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "publint": "tsx scripts/publint-all.ts", "doc-typecheck": "tsx scripts/doc-typecheck.ts", "verify-event-taxonomy": "tsx scripts/verify-event-taxonomy.ts", + "doc-sync": "yarn doc-typecheck && yarn verify-event-taxonomy", "hygiene": "yarn knip && yarn publint && yarn constraints", "demo:echo": "node --expose-internals --import tsx examples/echo-agent/start.ts", "demo:coding": "node --expose-internals --import tsx examples/coding-agent/start.ts",