From 85026f6a059cb88df32631838be47a67e8a29d73 Mon Sep 17 00:00:00 2001 From: Rezolv Date: Mon, 14 Sep 2026 21:43:49 -0400 Subject: [PATCH] feat(#4668): add StateWriteIntent type surface and opaque-transform guard recognition (ADR-4629 C1) (#4676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#4668): add StateWriteIntent type surface and opaque-transform guard recognition (ADR-4629 C1) Child C1 of epic #4629 — migration-order step (1) of ADR-4629: the guard/type scaffolding, with NO behavior change and no caller migrated. 1. StateWriteIntent (src/state-transition.cts) extends StateTransaction with the ADR-4629 section 8.1 concepts: field/section assertions marked required vs best-effort, plus a declared mutation scope (narrow | broad). Frozen like its base. createStateWriteIntent builds one from an existing transaction. Nothing in production constructs it yet — section 8.1's caller-side rule is Required in Phase 2; C2 (the verifying executor) and C3+ (caller migration) consume it. 2. findOpaqueStateTransforms (scripts/lint-state-write-path-drift.cjs) recognizes a residual readModifyWriteStateMd(path, (content) => ...) write whose transform is an inline anonymous arrow/function — the opaque shape section 8.1 replaces with a declared StateWriteIntent. readModifyWriteStateMd goes THROUGH the seam (it is not a raw-write bypass, Axis 2's concern), but its opaque body transform is neither verified (section 8.2) nor bounded (section 8.3). This ships recognition as a CAPABILITY: exported and unit-tested (positive control on a seeded fixture) but DELIBERATELY NOT wired into collect()'s failing scan. Wiring it now would turn the ~16 residual callers red at once, and ADR-3473 section 8.6 retired the ratchet that would otherwise absorb them. C2 wires it terminal as the verifying executor lands and callers migrate under ADR-3408 section 6 phasing. No behavior change: the guard is green on the tree (detection not wired), every state verb's output is unchanged, and the relevant suites (1763 tests) plus lint:ci pass. Regression tests are failing-first: positive/negative controls for the guard capability and a shape test for the type, plus a pin that collect() has no opaque-transform findings (C1 must not enforce; that is C2). Closes #4668 * chore(#4668): backfill changeset pr field to the real PR number (#4676) pr: 0 is rejected by parseFragment as invalid_pr (it is not a valid placeholder); the fragment must carry the real PR number, which fixes both changeset-lint and docs-lint (fail_invalid_fragment / fail_malformed_fragment). --------- Co-authored-by: Tom Boucher --- .changeset/4668-state-write-intent-surface.md | 6 ++ scripts/lint-state-write-path-drift.cjs | 93 +++++++++++++++++++ src/state-transition.cts | 76 +++++++++++++++ tests/state-transition.test.cjs | 43 +++++++++ tests/state-write-path-drift-guard.test.cjs | 52 +++++++++++ 5 files changed, 270 insertions(+) create mode 100644 .changeset/4668-state-write-intent-surface.md diff --git a/.changeset/4668-state-write-intent-surface.md b/.changeset/4668-state-write-intent-surface.md new file mode 100644 index 000000000..62ff14df8 --- /dev/null +++ b/.changeset/4668-state-write-intent-surface.md @@ -0,0 +1,6 @@ +--- +type: Changed +pr: 4676 +--- + +Internal (ADR-4629 section 8.1, epic #4629 child C1): introduce the `StateWriteIntent` type, extending `StateTransaction`, and a `readModifyWriteStateMd` opaque-transform recognition capability in the STATE.md write-path drift guard. This is the foundation for verified, bounded STATE.md writes. No user-facing behavior changes and no caller is migrated (that is Phase 2 and later). diff --git a/scripts/lint-state-write-path-drift.cjs b/scripts/lint-state-write-path-drift.cjs index 868909d1a..287cf3812 100644 --- a/scripts/lint-state-write-path-drift.cjs +++ b/scripts/lint-state-write-path-drift.cjs @@ -162,6 +162,12 @@ const REASON = Object.freeze({ // Axis 2 (§8.6, retained): a raw `fs.writeFileSync(` call targeting the // state path — see `findRawStateWrites`. RAW_STATE_WRITE: 'raw_state_write', + // ADR-4629 §8.1 (epic #4629, C1): a residual `readModifyWriteStateMd(path, + // (content) => …)` call whose transform is an inline anonymous arrow/function + // — the opaque shape §8.1 replaces with a declared StateWriteIntent. Recognized + // by `findOpaqueStateTransforms`; a CAPABILITY in C1 (not wired into collect()), + // wired terminal in C2 as the residual callers migrate (ADR-3408 §6 phasing). + OPAQUE_STATE_TRANSFORM: 'opaque_state_transform', // Axis 4 (§8.3, Decision 4(d)): prompt-layer prose shelling out to a // write-side `gsd-tools` subcommand — see `findPromptSeamUses`. PROMPT_LAYER_STATE_WRITE: 'prompt_layer_state_write', @@ -645,6 +651,91 @@ function findRawStateWrites(rel, text) { return out; } +// ADR-4629 §8.1 (epic #4629, C1) — OPAQUE-TRANSFORM RECOGNITION. +// +// A residual `readModifyWriteStateMd(path, (content) => …)` call whose transform +// (2nd) argument is an inline anonymous arrow / function expression is the opaque +// shape §8.1 replaces with a declared StateWriteIntent: `readModifyWriteStateMd` +// goes THROUGH the seam (it is NOT a raw-write bypass — Axis 2's concern), but its +// opaque body transform is neither verified (§8.2) nor bounded (§8.3). This +// function RECOGNIZES that shape. +// +// C1 ships recognition as a CAPABILITY: it is exported and unit-tested (positive +// control on a seeded fixture) but is DELIBERATELY NOT wired into `collect()`'s +// failing scan. §8.1's caller-side rule is *Required — Phase 2*; wiring it now +// would turn the ~16 residual callers red at once, and ADR-3473 §8.6 retired the +// ratchet that would otherwise be needed to absorb them. C2 wires this terminal as +// the verifying executor lands and the callers migrate under ADR-3408 §6 phasing. +// +// String match, never an AST — same over-report-safe posture as every axis here. +const OPAQUE_RMW_CALL_RE = /\breadModifyWriteStateMd\s*\(/g; +// The transform arg is OPAQUE when it BEGINS an inline anonymous function: an +// arrow (`(…) =>`, `ident =>`, optionally `async`) or a `function` expression. A +// bare identifier / object (a declared StateWriteIntent, Phase 2+) is NOT opaque. +const ANON_TRANSFORM_RE = + /^\s*(?:async\s+)?(?:\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>|^\s*(?:async\s+)?function\b/; + +/** + * Capture the top-level, comma-separated argument list of a call, starting at + * `startIdx` (the index just AFTER the opening `(`) — across newlines and + * string-aware, tracking `()[]{}` depth. Returns the raw arg strings, or null if + * the parens never balance (a truncated arg list is never flagged). Generalizes + * `captureFirstArg`'s depth/inStr bookkeeping to every argument. + */ +function captureCallArgList(text, startIdx) { + let depth = 0; + let inStr = null; + const args = []; + let cur = ''; + for (let i = startIdx; i < text.length; i++) { + const ch = text[i]; + if (inStr) { + cur += ch; + if (ch === inStr && text[i - 1] !== '\\') inStr = null; + continue; + } + if (ch === '"' || ch === "'" || ch === '`') { inStr = ch; cur += ch; continue; } + if (ch === '(' || ch === '[' || ch === '{') { depth++; cur += ch; continue; } + if (ch === ')' || ch === ']' || ch === '}') { + if (ch === ')' && depth === 0) { + if (cur.trim() !== '' || args.length) args.push(cur); + return args; + } + depth--; cur += ch; continue; + } + if (ch === ',' && depth === 0) { args.push(cur); cur = ''; continue; } + cur += ch; + } + return null; // parens never balanced +} + +/** + * Recognize residual opaque-transform STATE.md writes (ADR-4629 §8.1). Returns a + * finding per `readModifyWriteStateMd(` call whose 2nd argument is an inline + * anonymous transform. CAPABILITY only in C1 — not called by `collect()`. + */ +function findOpaqueStateTransforms(rel, text) { + const rawLines = text.split('\n'); + const stripped = stripComments(text).join('\n'); + const out = []; + let m; + OPAQUE_RMW_CALL_RE.lastIndex = 0; + while ((m = OPAQUE_RMW_CALL_RE.exec(stripped)) !== null) { + const args = captureCallArgList(stripped, m.index + m[0].length); + if (!args || args.length < 2) continue; + if (!ANON_TRANSFORM_RE.test(args[1])) continue; + const lineNo = stripped.slice(0, m.index).split('\n').length; + out.push({ + reason: REASON.OPAQUE_STATE_TRANSFORM, + axis: 'opaque-transform', + file: sanitizeForReport(rel), + line: lineNo, + source: sanitizeForReport((rawLines[lineNo - 1] || '').trim()), + }); + } + return out; +} + // The two write-seam STAGE functions, matched only as CALLS (`\(` // immediately after, modulo whitespace) — never as bare mentions of the // name. `writeStateMd(` is deliberately NOT included here (that arm is @@ -941,6 +1032,8 @@ module.exports = { nearestPrecedingAssignment, findRawStateWrites, targetsStatePath, + findOpaqueStateTransforms, + captureCallArgList, findCompositionBypasses, findPromptSeamUses, isInsideCodeSpan, diff --git a/src/state-transition.cts b/src/state-transition.cts index c08f3dbc6..54da5f01a 100644 --- a/src/state-transition.cts +++ b/src/state-transition.cts @@ -519,6 +519,82 @@ export function rebuildStateTransaction(init: StateTransactionInit): StateTransa return createStateTransaction('rebuild', init, 'rebuildStateTransaction'); } +// ---------------------------------------------------------------------------- +// StateWriteIntent — ADR-4629 §8.1 (epic #4629, child C1, migration step 1) +// ---------------------------------------------------------------------------- +// +// ADR-1769 Decision 2 scoped the state-transition model to 10 transitions and +// REJECTED covering all 16 writers; the residual writers still ride an opaque +// `transformFn: (content: string) => string` (`readModifyWriteStateMd`, +// src/state.cts). The write seam preserves FRONTMATTER, but the opaque body +// transform is neither verified (did every intended assertion land? §8.2) nor +// bounded (did anything OUTSIDE the declared scope change? §8.3) — the residue +// behind the write-path bugs epic #4629 absorbs. +// +// StateWriteIntent is the declared replacement: which field/section assertions +// the write must land (required vs best-effort) and the mutation scope it may +// touch (narrow | broad). It EXTENDS StateTransaction so an intent IS-A +// transaction everywhere the write seam already expects one. C1 ships the TYPE + +// constructor only — §8.1's caller-side rule ("no residual caller supplies an +// anonymous transform") is statused *Required — Phase 2*, so nothing constructs +// this in production yet; C2 (the verifying executor) and C3+ (caller migration) +// consume it. + +export type StateAssertionRequirement = 'required' | 'best-effort'; +export type StateMutationScope = 'narrow' | 'broad'; + +/** One declared post-state assertion: a frontmatter field or a body section. */ +export type StateFieldAssertion = { + readonly field: string; + readonly requirement: StateAssertionRequirement; +}; + +export type StateWriteIntentInit = { + readonly assertions?: ReadonlyArray; + readonly scope?: StateMutationScope; +}; + +/** + * ADR-4629 §8.1: a StateTransaction PLUS the declared write intent — the + * assertions verified against the re-read file (§8.2) and the mutation scope the + * write may not exceed (§8.3). Both are Phase-2 consumers; the type exists now so + * Phase 2 has a surface to build on. + */ +export type StateWriteIntent = StateTransaction & { + readonly assertions: ReadonlyArray; + readonly scope: StateMutationScope; +}; + +/** + * Extend an existing StateTransaction into a StateWriteIntent. The base + * transaction is REQUIRED — an absent base is a construction failure, mirroring + * `createStateTransaction`'s ADR-3473 §8.6 posture (do not tolerate null). `scope` + * defaults to the conservative `'narrow'`; `assertions` defaults to none. Frozen + * so an intent, like a transaction, cannot be mutated after construction. + */ +export function createStateWriteIntent( + transaction: StateTransaction, + init: StateWriteIntentInit = {}, +): StateWriteIntent { + if (transaction === null || typeof transaction !== 'object' || Array.isArray(transaction)) { + const err = new Error( + 'createStateWriteIntent: a base StateTransaction is required (build it with ' + + 'openStateTransaction / rebuildStateTransaction first). Per ADR-4629 §8.1, an absent ' + + 'transaction is a construction failure — do not tolerate null.', + ) as Error & { code: string }; + err.code = 'STATE_WRITE_INTENT_TRANSACTION_REQUIRED'; + throw err; + } + const assertions: ReadonlyArray = Object.freeze( + (init.assertions ?? []).map((a) => Object.freeze({ field: a.field, requirement: a.requirement })), + ); + return Object.freeze({ + ...transaction, + assertions, + scope: init.scope ?? 'narrow', + }); +} + // ---------------------------------------------------------------------------- // applyStatePreservation — table-driven post-sync preservation (ADR-1769 #1796) // ---------------------------------------------------------------------------- diff --git a/tests/state-transition.test.cjs b/tests/state-transition.test.cjs index 79873b9e2..d7e8a40dc 100644 --- a/tests/state-transition.test.cjs +++ b/tests/state-transition.test.cjs @@ -4826,3 +4826,46 @@ describe('stateReplaceProgressPercent — anchored bold form leaves prose lookal ); }); }); + +// --------------------------------------------------------------------------- +// C1 of epic #4629 (ADR-4629 §8.1) — the StateWriteIntent type surface. +// StateWriteIntent EXTENDS StateTransaction, adding the §8.1 concepts: field/ +// section assertions marked required-vs-best-effort, plus a declared mutation +// scope (narrow | broad). C1 ships the type + constructor (no caller migrated — +// §8.1's caller-side rule is Required — Phase 2); C2+ consume it. Frozen like its +// base so a transaction cannot be mutated after construction. +describe('C1 (ADR-4629 §8.1): StateWriteIntent type surface', () => { + const { createStateWriteIntent } = require('../gsd-core/bin/lib/state-transition.cjs'); + + test('createStateWriteIntent extends a StateTransaction with assertions + declared scope', () => { + const base = openStateTransaction({ snapshot: {} }); + const intent = createStateWriteIntent(base, { + assertions: [ + { field: 'Progress', requirement: 'required' }, + { field: 'Notes', requirement: 'best-effort' }, + ], + scope: 'narrow', + }); + // still a StateTransaction (IS-A: every base field carried through) + assert.strictEqual(intent.kind, 'open'); + assert.strictEqual(typeof intent.snapshot, 'object'); + assert.strictEqual(intent.resync, false); + // plus the §8.1 additions + assert.deepStrictEqual( + intent.assertions.map((a) => [a.field, a.requirement]), + [['Progress', 'required'], ['Notes', 'best-effort']], + ); + assert.strictEqual(intent.scope, 'narrow'); + assert.ok(Object.isFrozen(intent), 'StateWriteIntent must be frozen like StateTransaction'); + }); + + test('createStateWriteIntent defaults scope to narrow and assertions to [] when omitted', () => { + const intent = createStateWriteIntent(openStateTransaction({ snapshot: {} }), {}); + assert.deepStrictEqual(intent.assertions, []); + assert.strictEqual(intent.scope, 'narrow'); + }); + + test('createStateWriteIntent rejects an absent base transaction (construction failure, per §8.6 posture)', () => { + assert.throws(() => createStateWriteIntent(null, { scope: 'narrow' }), /transaction/i); + }); +}); diff --git a/tests/state-write-path-drift-guard.test.cjs b/tests/state-write-path-drift-guard.test.cjs index 6b8db01b4..c2f8b41ae 100644 --- a/tests/state-write-path-drift-guard.test.cjs +++ b/tests/state-write-path-drift-guard.test.cjs @@ -634,3 +634,55 @@ describe('F3 — a guard that cannot fail is not a guard: the real CLI entry poi assert.ok(findings.every((f) => f.reason === REASON.COMPOSITION_BYPASS)); }); }); + +// --------------------------------------------------------------------------- +// C1 of epic #4629 (ADR-4629 §8.1, migration step 1) — the OPAQUE-TRANSFORM +// recognition CAPABILITY. `findOpaqueStateTransforms` recognizes a residual +// `readModifyWriteStateMd(path, (content) => …)` write whose transform is an +// inline anonymous arrow / function expression (the shape §8.1 replaces with a +// declared StateWriteIntent). C1 ships the capability proven on a seeded fixture; +// it is deliberately NOT wired into collect()'s failing scan (that is C2 — +// §8.2/§8.3 Required — Phase 2), so the ~16 existing residual callers do not turn +// lint:ci red and no ratchet is introduced (ADR-3408 §6 phased migration). +describe('C1 (ADR-4629 §8.1): findOpaqueStateTransforms — opaque readModifyWriteStateMd recognition', () => { + test('positive control: an inline arrow transform (the #4551 residue shape) is flagged', () => { + const text = [ + 'stateUpdated = readModifyWriteStateMd(', + ' statePath,', + ' (stateContent) => {', + " return stateContent.replace(/None/g, '');", + ' },', + ' cwd,', + ');', + ].join('\n'); + const out = guard.findOpaqueStateTransforms('src/some-residual-writer.cts', text); + assert.strictEqual(out.length, 1); + assert.strictEqual(out[0].reason, REASON.OPAQUE_STATE_TRANSFORM); + assert.strictEqual(out[0].axis, 'opaque-transform'); + }); + + test('positive control: a function-expression transform is also flagged (Postel: over-report-safe)', () => { + const text = 'readModifyWriteStateMd(p, function (c) { return c; }, cwd);'; + assert.strictEqual(guard.findOpaqueStateTransforms('src/x.cts', text).length, 1); + }); + + test('negative control: a declared-intent (identifier) 2nd arg is NOT flagged', () => { + const text = 'readModifyWriteStateMd(p, writeIntent, cwd);'; + assert.strictEqual(guard.findOpaqueStateTransforms('src/x.cts', text).length, 0); + }); + + test('negative control: a non-readModifyWriteStateMd call with an arrow is NOT flagged', () => { + const text = 'someOther(p, (c) => c, cwd);'; + assert.strictEqual(guard.findOpaqueStateTransforms('src/x.cts', text).length, 0); + }); + + test('no behavior change: the capability is NOT wired into the failing scan (collect has no opaque-transform findings)', () => { + // C1 ships recognition; C2 wires enforcement. Wiring it now would red the ~16 residual callers. + const { findings } = guard.collect(); + assert.strictEqual( + findings.some((f) => f.reason === REASON.OPAQUE_STATE_TRANSFORM), + false, + 'C1 must not wire opaque-transform into collect() (that is C2, §8.2/§8.3 Required — Phase 2)', + ); + }); +});