From 1fdf6dd29246fda3096fa2c39f84eccc13d38758 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 4 Aug 2026 11:34:48 -0500 Subject: [PATCH 1/8] fix(ide): the ai:assist gate could never fire, and a degraded policy read could switch it back on (BACKLOG #330) Two defects in the VS Code extension's AI-assist policy gate, fixed in the load-bearing order. Neither is a live exposure: MessageFoundry is a not-deployed beta, so these describe what a deploying site would hit. 1. THE GUARD, FIRST. resolveAiPolicy wrote the freshly-read policy to LAST_POLICY_KEY unconditionally. `assist_permitted` is identity-dependent, so any read the engine cannot attribute answers `null` -- and writing that raw would overwrite a cached, authoritatively-observed deny. A degraded read would UPGRADE assistance that a central policy had switched off. The write now goes through mergeAuthoritativePolicy, a pure rule in the new zero-import ide/src/aiPolicyModel.ts so it is asserted node-side on every CI leg rather than only in the Windows-only Extension Host leg. The rule is asymmetric on purpose: a cached deny is sticky over a non-answer, a cached PERMIT is not (fabricating a permit from stale state is the fail-open direction), any evaluable true/false wins outright, and `mode` always comes fresh so a central off->byo re-enable still propagates. "Not evaluable" is deliberately wider than the literal `null`. AiPolicyWire is a compile-time claim JSON.parse does not enforce, so a 200 that OMITS assist_permitted arrives as `undefined` -- which a `=== null` guard lets straight through, and which is not `false` either, so the cache would be poisoned past recovery and every later answer would find nothing to retain. The bit is narrowed at the boundary (evaluatedPermission) on both authoritative paths: the engine read, and the CLI fallback, which never reaches the merge at all. 2. THEN THE BEARER. The read was unauthenticated, so the engine could only ever answer `null` and ADR 0035's ai:assist deny branch could not fire. resolveAiPolicy now attaches the cached token behind the existing SEC-005 assertTargetAllowed check, via peekToken -- NEVER ensureToken, which would pop an interactive sign-in modal out of a chat turn. The two functions are structurally identical, so tsc cannot tell them apart; the control is a test asserting the field IS peekToken by identity. Order matters and is satisfied a fortiori here: both land in ONE commit, so no tree ever exists in which the bearer is attached while the cache write is unguarded. A two-commit split would be the weaker guarantee. 3. THE TWO /ai/policy READERS. statusBar.ts's periodic read stays TOKENLESS and must: it runs off the 15s timer, where a bearer would keep refreshing the session's idle clock and make the engine's 30-minute idle timeout unreachable (CWE-613). The distinction is now data, not a comment -- ENVIRONMENT_PLAN (authenticated: false) and ASSIST_GATE_PLAN (authenticated: true) sit beside POLL_PLAN / VERIFY_PLAN and are both asserted in CI, on the same route with opposite answers, so a later reader cannot "unify" them back into the bug. 20 new tests, each falsified against a planted defect -- including one plant (readToken := ensureToken) that reproduced the forbidden harm directly: T11 red AND the end-to-end command test hanging 60s on a sign-in modal, while tsc stayed green. Docs of record updated in the same commit rather than left to drift: - docs/AI.md: the gating table said a byo read answering `null` is Enabled, which the sticky-deny rule contradicts; the IDE-read-is-tokenless premise is retired (the ENGINE endpoint stays tokenless-readable, and so does the status bar's separate read); and the offline-fallback paragraph still described a `byo` default that had already become fail-closed `unverified`. - ADR 0035: AC-7 and AC-8 added. AC-8's status-bar clause is scoped to what the tests actually pin (the plan constant), with the evidence gap recorded rather than over-claimed -- no suite constructs EngineStatusBar. - ADR 0110: amended; it owns the probe-plan vocabulary, now shared. - master test plan ch.12: three citations this change moved (aiPolicy.ts:60-68 -> :78-86, engineStatusModel.ts:124 -> :131) and the test-count row (474/560, 86 excluded -> 487/580, 93 excluded), plus a row for the new node-side suite. --- docs/AI.md | 37 +++- ...ide-extension-workspace-trust-and-scope.md | 44 +++- ...ells-the-truth-about-the-promote-target.md | 35 ++++ .../12-vs-code-ide-extension.md | 9 +- ide/src/aiPolicy.ts | 145 ++++++++------ ide/src/aiPolicyModel.ts | 125 ++++++++++++ ide/src/engineStatusModel.ts | 41 +++- ide/src/statusBar.ts | 14 +- ide/src/test/suite/ai-policy-model.test.ts | 126 ++++++++++++ ide/src/test/suite/ai-policy.test.ts | 188 +++++++++++++++++- ide/src/test/suite/engine-client.test.ts | 43 ++++ ide/src/test/suite/engine-doctor.test.ts | 51 +++++ 12 files changed, 780 insertions(+), 78 deletions(-) create mode 100644 ide/src/aiPolicyModel.ts create mode 100644 ide/src/test/suite/ai-policy-model.test.ts diff --git a/docs/AI.md b/docs/AI.md index b71f47d0..5251853e 100644 --- a/docs/AI.md +++ b/docs/AI.md @@ -175,9 +175,11 @@ stdout); on error it prints `{"error": "..."}`. It prints **config only, never m ## IDE gating behavior The IDE assistant ([ide/src/chat.ts](../ide/src/chat.ts)) resolves the policy **before** every -request: it first calls `GET /ai/policy` (authoritative); on any error it falls back to the local -`messagefoundry ai-policy` CLI; if that also fails it uses a conservative built-in default -(`byo` / `code_only` / `prod`, `assist_permitted: null`) so the safe assistant still works offline. +request: it first calls `GET /ai/policy` (authoritative, and cached on success); on any error it falls +back to that cached authoritative policy, then to the local `messagefoundry ai-policy` CLI; if none of +those can positively confirm a policy it uses a fail-closed built-in default (`mode: unverified`), +which **disables** assistance rather than re-enabling BYO — a central *off* must not be bypassable by +taking the engine offline (SEC-022). Then it applies the effective policy: @@ -186,14 +188,33 @@ Then it applies the effective policy: | `mode == off` | **Disabled.** "AI assistance is turned off by your MessageFoundry policy." | | `mode == managed_claude` / `managed_claude_baa` | **Disabled.** This IDE version can't service a managed provider; it does **not** silently fall back to BYO (that would violate operator intent). | | `mode == byo` and `assist_permitted == false` | **Disabled.** "Your role does not include the `ai:assist` permission." | -| `mode == byo` and `assist_permitted` is `true` **or** `null` | **Enabled** (proceeds as today). | +| `mode == byo` and `assist_permitted` is `true` **or** `null` | **Enabled** — *unless* an authoritative `false` was previously observed; see the sticky-deny rule below. | +| `mode == unverified` (nothing could confirm a policy) | **Disabled.** Fail-closed; see above. | -**The tokenless-IDE / `assist_permitted == null` trust note.** Under BYO, `null` (RBAC not evaluable -offline) is **allowed**. This is safe by construction: BYO sends only **code-only** context to the -developer's own provider — it never sees the engine or any message data, so there is no PHI to -protect with RBAC at this stage. The central *off* switch is still honored because `mode` is read +**The `assist_permitted == null` trust note.** Under BYO, `null` (RBAC not evaluable) is **allowed**. +This is safe by construction: BYO sends only **code-only** context to the developer's own provider — +it never sees the engine or any message data, so there is no PHI to protect with RBAC at this stage. +The central *off* switch is honored regardless, because `mode` is identity-independent and is read straight from the policy, token or not. +**The IDE's gate read is authenticated (BACKLOG #330).** `assist_permitted` is computed from the +acting identity, so a tokenless caller can only ever be told `null` and the deny row above could never +fire. `resolveAiPolicy` therefore attaches the cached bearer — never prompting for one, and never over +plain `http://` to a non-loopback host. Two things this does **not** change: the engine endpoint stays +tokenless-*readable* (the `GET /ai/policy` section above is unchanged and still true), and the status +bar's **separate**, timer-driven read of the same route stays **tokenless** — it wants only the +identity-independent `environment`, and a bearer on that timer would keep refreshing the session's +idle clock and make the engine's 30-minute idle timeout unreachable (CWE-613). + +**The sticky-deny rule (ADR 0035 AC-7).** Because `null` means "could not be evaluated" rather than +"permitted", a fresh `null` must not *upgrade* assistance a central policy switched off: an +authoritative `assist_permitted: false` the IDE has already observed is **retained** over a later +`null`, so under BYO that combination resolves to **Disabled**. The rule is deliberately one-way — a +cached `true` is *not* sticky, since fabricating a permit from stale state is the fail-open direction +— and any evaluable `true`/`false` replaces the cached value outright, so signing in is the escape +hatch. Anything that is not the literal `true`/`false`, **including a response that omits the field**, +counts as "not evaluated" and never as a permit. + `messagefoundry.showAiPolicy` (command **"MessageFoundry: Show AI Policy"**) displays the current resolved policy in the IDE. diff --git a/docs/adr/0035-ide-extension-workspace-trust-and-scope.md b/docs/adr/0035-ide-extension-workspace-trust-and-scope.md index 65b2fbca..5240bae1 100644 --- a/docs/adr/0035-ide-extension-workspace-trust-and-scope.md +++ b/docs/adr/0035-ide-extension-workspace-trust-and-scope.md @@ -2,7 +2,7 @@ - **Status:** Accepted - **Date:** 2026-06-26 -- **Related:** ADR 0007 (GUI-manageable connections.toml) · ADR 0024 (AI policy) · CLAUDE.md §9 (PHI/HIPAA), §10 (Console) · SEC-004, SEC-005, SEC-022 +- **Related:** ADR 0007 (GUI-manageable connections.toml) · docs/AI.md (the AI policy model) · ADR 0135 (the engine-brokered path) · CLAUDE.md §9 (PHI/HIPAA), §10 (Console) · SEC-004, SEC-005, SEC-022 --- @@ -74,6 +74,48 @@ online-permitted BYO assistant all keep working. - **AC-6** — WHEN the engine policy is read successfully, THE EXTENSION SHALL cache it so a previously-seen central "off" is not overridable by going offline. → `ide/src/test/suite/ai-policy.test.ts` (`pickOfflinePolicy` cached-wins case) +- **AC-7** — WHEN the engine's answer to `GET /ai/policy` does NOT carry an evaluable + `assist_permitted` (the literal `true` or `false`) AND a cached authoritative policy holds + `assist_permitted: false`, THE EXTENSION SHALL retain the `false` in both the returned and the cached + policy, and SHALL NOT retain a cached `true` the same way. + → `ide/src/test/suite/ai-policy-model.test.ts` (the pure rule, node-side on every leg) + + `ide/src/test/suite/ai-policy.test.ts` (the cache write) +
The asymmetry is the decision: `assist_permitted` is identity-dependent, so a non-answer means + "could not be evaluated", and a degraded read must not *upgrade* assistance a central policy switched + off. A cached `true` is deliberately **not** sticky — a non-answer under BYO is allowed by design + (docs/AI.md, *"the `assist_permitted == null` trust note"*), so carrying a permit forward would + fabricate one, which is the fail-open direction. +
**"Not evaluable" is wider than `null` on purpose.** `AiPolicyWire` is a compile-time claim about + a response and `JSON.parse` does not enforce it, so a 200 that merely OMITS the field arrives as + `undefined`. A guard keyed on the literal `null` would let that through — and because `undefined` is + not `false` either, the cache would be poisoned so no later answer could restore the deny. The bit is + therefore narrowed at the boundary (`evaluatedPermission`), and only `true`/`false` count as answers. +
**Accepted consequences**, stated as decisions rather than surprises: (a) the deny is one-way + until an evaluable answer replaces it, so a user granted `ai:assist` later, who holds no valid + session, sees assistance disabled until the engine answers `true` for them — the escape hatch is + signing in, as there is no "clear cached policy" command; and (b) the cache is a **single global + key**, not keyed per engine URL as the bearer is, so a deny observed against one engine also + suppresses assistance against another until an evaluable answer arrives from it. Both are the + fail-CLOSED direction, which is why they are accepted here rather than treated as defects. +- **AC-8** — WHEN `resolveAiPolicy` reads the authoritative policy, THE EXTENSION SHALL attach the + cached bearer (never prompting for one) so `assist_permitted` is resolvable for the acting user, and + SHALL NOT attach it to a non-loopback plain-`http://` target. The status bar's periodic `/ai/policy` + environment read SHALL be expressed as an `authenticated: false` plan constant. + → `ide/src/test/suite/ai-policy.test.ts` (the bearer reaches the request; `peekToken` by identity) + + `ide/src/test/suite/engine-client.test.ts` (the `Authorization` header, both polarities) + + `ide/src/test/suite/engine-doctor.test.ts` (`ASSIST_GATE_PLAN` / `ENVIRONMENT_PLAN`) +
The two `/ai/policy` readers give opposite answers about the bearer *on purpose*: the status + bar's read is timer-driven and wants the identity-independent `environment`, where a bearer would + defeat the engine's idle timeout (CWE-613, ADR 0110 §2); this one is user-initiated and wants the + identity-dependent `assist_permitted`, which no tokenless caller can ever receive. Both are expressed + as CI-asserted plan constants rather than call-site arguments. +
**Known gap in the evidence, recorded rather than papered over.** The third clause is deliberately + written about the CONSTANT, because that is all the tests pin. No suite constructs `EngineStatusBar`, + so nothing asserts that `readEnvironment` actually *uses* `ENVIRONMENT_PLAN` — rewiring that call site + to send a bearer would type-check and leave every test green. The constant is load-bearing only given + the call site reads it (`runProbe` attaches a bearer iff `entry.authenticated`), which is verified by + reading. Closing it needs an injectable-fetch seam on `EngineStatusBar` asserting `token === undefined`; + that is a test-infrastructure change beyond BACKLOG #330 and wants its own item. ## Options considered diff --git a/docs/adr/0110-ide-engine-link-doctor-the-status-bar-tells-the-truth-about-the-promote-target.md b/docs/adr/0110-ide-engine-link-doctor-the-status-bar-tells-the-truth-about-the-promote-target.md index e766f1ad..f3f2618d 100644 --- a/docs/adr/0110-ide-engine-link-doctor-the-status-bar-tells-the-truth-about-the-promote-target.md +++ b/docs/adr/0110-ide-engine-link-doctor-the-status-bar-tells-the-truth-about-the-promote-target.md @@ -221,6 +221,41 @@ then it cannot warn about the states that do block an action: `unreachable`/`for - **#26 (no visual/declarative authoring) is untouched — #26-clean:** this surface authors nothing, projects to no `.py`, and executes no logic. No PySide6. No "channel"/"route" element. +## Amendment — 2026-08-04 (BACKLOG #330) + +`/ai/policy` now has **two readers, with deliberately opposite probe plans**. This ADR is the record of +authority for the probe-plan vocabulary (`ProbePlanEntry`, `PROBE_ENDPOINTS`, `POLICY_ROUTE`, +`POLL_PLAN`, `VERIFY_PLAN`), so the second reader is recorded here rather than only in ADR 0035. + +- **`ENVIRONMENT_PLAN` (`authenticated: false`)** — the status bar's environment read, unchanged in + behaviour. It was previously a literal `undefined` token argument at the `readEnvironment` call site; + it is now driven by a named constant, so its tokenlessness is data CI asserts rather than an argument + a later tidy-up could "fix". +- **`ASSIST_GATE_PLAN` (`authenticated: true`)** — `aiPolicy.ts`'s gate read of the same route. It wants + `assist_permitted`, which the engine computes from the acting identity and answers as `null` to any + caller it cannot attribute, so a tokenless read left ADR 0035's `ai:assist` half unable to fire at all. + +**Nothing in §2 or §5 is relaxed.** §2's prohibition on a bearer from the timer stands verbatim and +still governs the status bar; §5's *"the environment is **read** (tokenless `/ai/policy`) and displayed, +never set"* bullet is unchanged and describes `ENVIRONMENT_PLAN` exactly. The new reader is **not** on a +timer: its only callers are user-initiated (a chat turn, and the *Show AI Policy* command), so it sits +under **`VERIFY_PLAN`'s** rationale — a click or an activation is real user activity, so refreshing the +idle clock there is honest rather than a forgery — and not under `POLL_PLAN`'s. + +Consequences for the acceptance criteria above, both verified rather than assumed: + +- **AC-3 still holds verbatim.** It quantifies over `POLL_PLAN`, which is untouched; and + `ENVIRONMENT_PLAN`, the other constant on the timer path, is `authenticated: false` and is now + asserted to be, which it was not before. +- **AC-6 still holds as written.** `PROBE_ENDPOINTS` is unchanged — `/ai/policy` was already on the + allowlist — so no new route is probed by anything. + +The vocabulary itself is now **shared** rather than private to the status bar: `engineStatusModel.ts` +is the IDE's probe-plan module, and `aiPolicy.ts` imports from it. That is the point of recording this +here — without it, the next reader finds an `authenticated: true` plan for `/ai/policy` inside a module +whose header forbids a bearer on the poll, and reasonably reads it as the exact bug this ADR exists to +prevent. + ## Alternatives considered - **A webview "Engine Doctor" panel** — rejected: opens from the same command dispatch (fixes a missed click no better); diff --git a/docs/testing/master-test-plan/12-vs-code-ide-extension.md b/docs/testing/master-test-plan/12-vs-code-ide-extension.md index 4785507b..daa0734d 100644 --- a/docs/testing/master-test-plan/12-vs-code-ide-extension.md +++ b/docs/testing/master-test-plan/12-vs-code-ide-extension.md @@ -62,15 +62,16 @@ actually fail a merge. | Evidence | What it proves | |---|---| | `.github/workflows/ci.yml:263-317` — `ide build (ubuntu-latest \| windows-latest)` | `npm ci` from `ide/package-lock.json`, `tsc --noEmit` strict type-check, esbuild bundle on both OSes; `npm run test:unit` on every leg; `npm test` (headless VS Code) on the Windows leg only | -| `ide/package.json:822` `test:unit` | 474 of 560 tests run with no Extension Host; a hand-maintained `--ignore` list excludes 8 files (86 tests) whose module-under-test transitively imports `vscode` | +| `ide/package.json:822` `test:unit` | 487 of 580 tests run with no Extension Host; a hand-maintained `--ignore` list excludes 8 files (93 tests) whose module-under-test transitively imports `vscode` | | `src/test/suite/extension.test.ts` | The extension activates with no workspace; every contributed command is registered; one non-interactive command (`showAiPolicy`) executes end to end | | `src/test/suite/settings-scope.test.ts:23-90` | ADR 0035 AC-2/AC-3 as a **family invariant**: every declared setting is classified, anything whose name matches `/url\|endpoint\|host\|python\|exec\|command\|token\|credential/i` must be `scope: "machine"`, and `capabilities.untrustedWorkspaces.supported === "limited"` | | `src/test/suite/pythonpath.test.ts` | ADR 0035 AC-1 — `resolvePythonPath` (`cli.ts:25-46`) never prefers a workspace `.venv` when untrusted; an explicit `pythonPath` is honoured verbatim; win32 + posix layouts | | `src/test/suite/engine-target.test.ts` | ADR 0035 AC-4 — `assertTargetAllowed` (`engineTarget.ts:29-44`) refuses plain `http://` to a non-loopback host; loopback http allowed; unparseable URL fails safe | -| `src/test/suite/ai-policy.test.ts` | ADR 0035 AC-5/AC-6 — no cache + no CLI policy ⇒ disabled `"unverified"`; a cached authoritative `off` survives going offline (`aiPolicy.ts:60-68`) | -| `src/test/suite/engine-doctor.test.ts` | ADR 0110 AC-3/AC-6/AC-8 — the `EngineLink` field allowlist and `PROBE_ENDPOINTS = ["/ai/policy","/config/provenance","/health"]` (`engineStatusModel.ts:124`) are frozen; every `POLL_PLAN` entry is `authenticated:false`; a command link planted in a 403 detail cannot reach the trusted hover | +| `src/test/suite/ai-policy.test.ts` | ADR 0035 AC-5/AC-6 — no cache + no CLI policy ⇒ disabled `"unverified"`; a cached authoritative `off` survives going offline (`aiPolicy.ts:78-86`). ADR 0035 AC-7/AC-8 — `resolveAiPolicy`'s fetch-and-cache path via an injected `AiPolicyIo`: the bearer reaches the request, `DEFAULT_AI_POLICY_IO.readToken` **is** `peekToken` by identity (never `ensureToken`), SEC-005 withholds it from a non-loopback `http://` target, and a degraded answer does not overwrite a cached deny in the STORE | +| `src/test/suite/ai-policy-model.test.ts` | ADR 0035 AC-7 — `mergeAuthoritativePolicy`, the only assertion of the merge rule that runs **node-side on every leg** (zero-import module, so not on the `--ignore` list). The deny is sticky over a non-evaluable answer and reaches `assistantState`; a cached permit is **not** sticky; `mode` always comes fresh; "non-evaluable" covers an omitted/non-boolean field, not just `null` | +| `src/test/suite/engine-doctor.test.ts` | ADR 0110 AC-3/AC-6/AC-8 — the `EngineLink` field allowlist and `PROBE_ENDPOINTS = ["/ai/policy","/config/provenance","/health"]` (`engineStatusModel.ts:131`) are frozen; every `POLL_PLAN` entry is `authenticated:false`; `ENVIRONMENT_PLAN` is `authenticated:false` and `ASSIST_GATE_PLAN` is `authenticated:true` on the **same** route (BACKLOG #330 — asserted so the two cannot be "unified"); a command link planted in a 403 detail cannot reach the trusted hover | | `src/test/suite/engine-status.test.ts` | ADR 0110 AC-1/AC-2/AC-5 — `classifyHealth` can never return `ok`; `version:null` vs version-present verdicts; an earned verdict decays; glyph/hover rendering | -| `src/test/suite/engine-client.test.ts` | ADR 0110 AC-4 — an unanswered request rejects tagged `MF_TIMEOUT`; `ECONNREFUSED` is distinct; hung vs dead render differently | +| `src/test/suite/engine-client.test.ts` | ADR 0110 AC-4 — an unanswered request rejects tagged `MF_TIMEOUT`; `ECONNREFUSED` is distinct; hung vs dead render differently. ADR 0035 AC-8 — against a real loopback server, a token becomes `Authorization: Bearer ` and a tokenless call sends **no** `Authorization` header at all (the status bar's read depends on the latter) | | `src/test/suite/engine-control.test.ts` | ADR 0112 AC-3/AC-6 — the serve argv is `serve --config ` with no `--db`/`--env`/`--port`; the preflight classifier picks the right remedy; `runDirHasEngine` fork guard; `planActions` gating | | `src/test/suite/engine-setup.test.ts` | Every button on the guided setup page resolves to a known `CMD` id (the webview cannot smuggle an arbitrary command id), ids are unique, `CMD.startEngine` only in the dev-tone section | | `src/test/suite/connection-merge.test.ts` | Non-rendered `connections.toml` keys survive an edit; a cleared rendered field deletes the key; retry merges field-wise; clone direction-flip drops inapplicable keys; name-collision refusal; `planSave` is the single merge policy for both writers | diff --git a/ide/src/aiPolicy.ts b/ide/src/aiPolicy.ts index becf7fba..d42254dd 100644 --- a/ide/src/aiPolicy.ts +++ b/ide/src/aiPolicy.ts @@ -1,22 +1,34 @@ // AI-assistance policy resolution for the IDE. The policy is centrally governed by the engine; the // IDE reads it (never sets it) and gates the @messagefoundry chat assistant accordingly. Resolution -// is authoritative-engine-first so a central "off" is honored even by a tokenless client, falling -// back to the local CLI (which reads messagefoundry.toml) when the engine is unreachable. +// is authoritative-engine-first, falling back to the local CLI (which reads messagefoundry.toml) when +// the engine is unreachable. +// +// The policy has TWO halves and they are answered differently: +// - `mode` is identity-INDEPENDENT, so a central "off" is honored even by a tokenless client; +// - `assist_permitted` is identity-DEPENDENT — the engine answers `null` to anyone it cannot +// attribute — so this read attaches the cached bearer (BACKLOG #330). A degraded/unattributed +// answer must not overwrite a cached deny; that is mergeAuthoritativePolicy's job. import * as vscode from "vscode"; +import { peekToken } from "./auth"; +import { type AiPolicy, evaluatedPermission, mergeAuthoritativePolicy } from "./aiPolicyModel"; import { engineUrl, runJson, workspaceDir } from "./cli"; import { getJson } from "./engineClient"; +import { ASSIST_GATE_PLAN } from "./engineStatusModel"; +import { assertTargetAllowed } from "./engineTarget"; -export interface AiPolicy { - mode: string; - dataScope: string; - environment: string | null; - // null = RBAC could not be evaluated (no/invalid token under enabled auth, or resolved offline). - assistPermitted: boolean | null; - reason: string | null; -} +// The policy shape lives in the zero-import aiPolicyModel so the merge rule below it can be asserted +// by the node-side runner that executes on every CI leg (this module imports vscode, so its own test +// file is `--ignore`d by `test:unit`). Re-exported in TYPE form: esbuild bundles with isolatedModules +// semantics, where a value-form re-export of a type-only symbol fails at BUNDLE time while +// `tsc --noEmit` stays green. Existing importers (chat.ts, extension.ts, the tests) are unchanged. +export type { AiPolicy } from "./aiPolicyModel"; +// `assistantState` moved there too: it is the predicate the merge rule feeds, and the two are only +// meaningful asserted together. Value-form re-export (it IS a value), so chat.ts and the existing +// ai-policy.test.ts import sites are untouched — ADR 0035 AC-5's test pointer stays valid. +export { assistantState } from "./aiPolicyModel"; // Snake_case wire shape shared by GET /ai/policy and `messagefoundry ai-policy --json`. -interface AiPolicyWire { +export interface AiPolicyWire { mode: string; data_scope: string; environment: string | null; @@ -40,12 +52,18 @@ const UNVERIFIED_POLICY: AiPolicy = { reason: null, }; +/** + * Wire → resolved policy. `AiPolicyWire` is a claim about a response, not a guarantee: `JSON.parse` + * validates nothing, so `assist_permitted` is NARROWED here rather than copied. This is the boundary + * for BOTH authoritative sources — the engine read and the CLI fallback — and only the former also + * passes through {@link mergeAuthoritativePolicy}, so the narrowing has to happen here to cover both. + */ function fromWire(w: AiPolicyWire): AiPolicy { return { mode: w.mode, dataScope: w.data_scope, environment: w.environment, - assistPermitted: w.assist_permitted, + assistPermitted: evaluatedPermission(w.assist_permitted), reason: w.reason, }; } @@ -67,71 +85,82 @@ export function pickOfflinePolicy(cached: AiPolicy | null, cli: AiPolicy | null) return UNVERIFIED_POLICY; } +/** + * The injectable IO seam for {@link resolveAiPolicy}. It exists so the fetch-and-cache path — where + * both of BACKLOG #330's defects lived — is testable at all: it was previously unreachable from a test + * because every dependency was a direct module-level call. + */ +export interface AiPolicyIo { + url: () => string; + readToken: (ctx: vscode.ExtensionContext, url: string) => Promise; + getPolicy: (url: string, route: string, token: string | undefined) => Promise; + getCliPolicy: () => Promise; +} + +/** + * Production wiring. + * + * `readToken` is {@link peekToken} and must NEVER become `ensureToken`: a chat turn resolves the policy + * before every request, and `ensureToken` would pop an interactive sign-in modal out of it. The two + * functions are structurally identical, so the type system cannot tell them apart — the control is the + * test that asserts this field IS `peekToken` by identity, not this comment. + */ +export const DEFAULT_AI_POLICY_IO: AiPolicyIo = { + url: engineUrl, + readToken: peekToken, + getPolicy: (url, route, token) => getJson(url, route, token), + getCliPolicy: () => runJson(["ai-policy"], workspaceDir()), +}; + /** * Resolve the effective AI policy. Order: (a) the running engine [authoritative — includes the * identity-dependent `assist_permitted`; cached on success]; (b) when the engine is unreachable, the * last cached authoritative policy; (c) the local CLI [reads messagefoundry.toml]; (d) a fail-closed * "unverified" policy (assistance disabled) so a central "off" can't be bypassed by going offline. + * + * The read is AUTHENTICATED (BACKLOG #330). `assist_permitted` is computed from the acting identity, so + * a tokenless read can only ever answer `null` and the `ai:assist` deny branch could never fire — half + * of ADR 0035's SEC-022 control was inert in the shipped code. The bearer is driven by + * {@link ASSIST_GATE_PLAN}, the same mechanism that keeps the status bar's read of this very route + * tokenless, so "who authenticates" is CI-asserted data rather than an argument at a call site. */ -export async function resolveAiPolicy(ctx: vscode.ExtensionContext): Promise { +export async function resolveAiPolicy( + ctx: vscode.ExtensionContext, + io: AiPolicyIo = DEFAULT_AI_POLICY_IO, +): Promise { try { - const policy = fromWire(await getJson(engineUrl(), "/ai/policy")); - await ctx.globalState.update(LAST_POLICY_KEY, policy); // remember the authoritative answer - return policy; + const url = io.url(); + const plan = ASSIST_GATE_PLAN[0]; + // SEC-005 (ADR 0035 / engineTarget.ts): never send a bearer in clear to a non-loopback http:// + // target. Same shape as the liveStatus poll's guard. peekToken, never ensureToken — see + // DEFAULT_AI_POLICY_IO. Driving the send off `plan.authenticated` is what makes the constant a + // control on this side too: flipping it to false actually turns the bearer off. + const token = + plan.authenticated && assertTargetAllowed(url).ok ? await io.readToken(ctx, url) : undefined; + const fresh = fromWire(await io.getPolicy(url, plan.route, token)); + // GUARDED write. An answer the engine could not attribute to an identity carries + // `assist_permitted: null`, and writing that raw would overwrite a cached, authoritatively-observed + // deny — a degraded read UPGRADING assistance a central policy had switched off. The merge keeps + // the deny and nothing else; see mergeAuthoritativePolicy for why it is one-way. + const merged = mergeAuthoritativePolicy( + ctx.globalState.get(LAST_POLICY_KEY) ?? null, + fresh, + ); + await ctx.globalState.update(LAST_POLICY_KEY, merged); // remember the authoritative answer + return merged; } catch { // Engine unreachable or errored — fall back to the cached authoritative / CLI / fail-closed view. } const cached = ctx.globalState.get(LAST_POLICY_KEY) ?? null; let cli: AiPolicy | null = null; try { - cli = fromWire(await runJson(["ai-policy"], workspaceDir())); + cli = fromWire(await io.getCliPolicy()); } catch { // CLI unavailable too (no Python / no workspace / untrusted) — leave cli null. } return pickOfflinePolicy(cached, cli); } -/** - * Apply the gating rules to a resolved policy. `enabled` false means the chat handler must not call - * the model and should stream `message` instead. The only ENABLED case is BYO with the permission - * granted or unknown (null) — BYO is PHI-safe by construction (code-only context). - */ -export function assistantState(p: AiPolicy): { enabled: boolean; message?: string } { - if (p.mode === "off") { - return { enabled: false, message: "AI assistance is turned off by your MessageFoundry policy." }; - } - if (p.mode === "unverified") { - // Engine unreachable + no cached policy + no positive local CLI policy: fail closed so a central - // "off" / ai:assist deny can't be bypassed by going offline (SEC-022). - return { - enabled: false, - message: - "MessageFoundry AI policy could not be verified (engine unreachable) — assistance is disabled until it can be confirmed.", - }; - } - if (p.mode === "managed_endpoint") { - // Engine-brokered path (ADR 0135): the ENGINE brokers the call to the customer-managed / self-hosted - // LLM under central per-use audit. Enabled when the caller holds ai:assist (or it is unknown offline); - // chat.ts routes this mode to the engine broker instead of the local vscode.lm model. - if (p.assistPermitted === false) { - return { enabled: false, message: "Your role does not include the ai:assist permission." }; - } - return { enabled: true }; - } - if (p.mode === "managed_claude" || p.mode === "managed_claude_baa") { - return { - enabled: false, - message: - "Your MessageFoundry policy uses a managed AI provider, which this extension version does not yet support. Assistance is unavailable.", - }; - } - if (p.mode === "byo" && p.assistPermitted === false) { - return { enabled: false, message: "Your role does not include the ai:assist permission." }; - } - // BYO with assistPermitted true OR null (RBAC not evaluable offline) — allowed. - return { enabled: true }; -} - /** Fetch the current policy and surface it to the user (command: messagefoundry.showAiPolicy). */ export async function showAiPolicy(ctx: vscode.ExtensionContext): Promise { const p = await resolveAiPolicy(ctx); diff --git a/ide/src/aiPolicyModel.ts b/ide/src/aiPolicyModel.ts new file mode 100644 index 00000000..25967b9e --- /dev/null +++ b/ide/src/aiPolicyModel.ts @@ -0,0 +1,125 @@ +// The pure, vscode-free half of the IDE's AI-policy handling: the resolved policy shape, and the +// authoritative-merge rule that decides what a FRESH engine answer may and may not overwrite. +// +// THIS MODULE MUST KEEP ZERO IMPORTS. It is separated from aiPolicy.ts (which imports `vscode`) +// for one measured reason: `ide/package.json`'s `test:unit` script explicitly `--ignore`s +// `out/test/suite/ai-policy.test.js`, so anything asserted only there runs solely in the +// Windows-only `npm test` leg. A zero-import module is assertable in the node-side runner that +// executes on EVERY ide CI leg — which is where a security invariant belongs. Adding a value +// import of any vscode-touching module kills `ai-policy-model.test.ts` in that runner with +// "Cannot find module 'vscode'". + +/** The resolved AI policy, as the IDE uses it (camelCase; see aiPolicy.ts's `fromWire`). */ +export interface AiPolicy { + mode: string; + dataScope: string; + environment: string | null; + // null = RBAC could not be evaluated (no/invalid token under enabled auth, or resolved offline). + assistPermitted: boolean | null; + reason: string | null; +} + +/** + * Narrow a permission bit to the EVALUABLE domain. Only the literals `true` and `false` are answers; + * everything else means "not evaluated", NEVER "permitted". + * + * This exists because {@link AiPolicy} is a COMPILE-TIME claim about a network response and + * `JSON.parse` does not honour it. The engine ships `assist_permitted: bool | None`, but a 200 from + * anything else on that URL — a proxy, a mistyped target, an engine build predating the field — can + * omit it, and an absent property reads as `undefined`. Without this narrowing `undefined` slips past + * the `=== null` guard below (`undefined !== null`), overwrites a cached deny, and — not being `false` + * either — POISONS the cache so no later answer can restore the deny. That is the fail-OPEN direction, + * reached by exactly the degraded answer this control exists to survive, so the narrowing is part of + * the control rather than defensive tidying. Takes `unknown` because at runtime it genuinely is. + */ +export function evaluatedPermission(v: unknown): boolean | null { + return v === true || v === false ? v : null; +} + +/** + * Merge a FRESH authoritative (engine) policy over the last cached one, retaining an + * `assistPermitted: false` that the fresh answer could not re-evaluate. + * + * The engine computes `assist_permitted` from the ACTING IDENTITY, so it is `null` for any read the + * engine could not attribute to a user (no bearer, an expired session, auth disabled mid-flight). + * Without this rule such a read silently overwrites a cached, authoritatively-observed deny — i.e. a + * degraded answer would UPGRADE assistance that a central policy had switched off (SEC-022 is the + * control this completes; ADR 0035). + * + * Four properties, each of which is a test — change one and a named test goes red: + * + * 1. **Asymmetric on purpose — only a DENY is sticky.** A cached `true` is NOT carried over a fresh + * `null`, because `null` under BYO is *allowed by design* (docs/AI.md's tokenless-IDE trust note: + * BYO sends code-only context to the developer's own provider, so there is no PHI for RBAC to + * protect at that stage). Carrying a `true` forward would fabricate a permit from stale state, + * which is the fail-OPEN direction. + * 2. **Only `assistPermitted` is retained.** `mode` / `dataScope` / `environment` / `reason` always + * come fresh, because `mode` is identity-INDEPENDENT: a central `off` → `byo` re-enable must + * propagate on the very next read, and a frozen policy would defeat that. + * 3. **Any EVALUABLE fresh answer wins outright.** `true` and `false` both replace the cache, so an + * administrator granting `ai:assist` un-sticks the deny the moment that user's next authenticated + * read returns `true`. The escape hatch is signing in, not clearing extension state. + * 4. **`cached === null` (the first ever read) is a plain pass-through.** + * 5. **The fresh bit is NARROWED before anything is decided** ({@link evaluatedPermission}). Any + * non-boolean — `null`, or the `undefined` a 200 that OMITS the field produces — is treated as + * "not evaluated" and normalized to `null` on the way out, so a degraded answer can neither slip + * past the guard nor be written to the cache in a shape no later comparison can match. + * + * Consequence, stated so it is a decision and not a surprise: once a `false` is cached, every later + * `null` preserves it. A user whose permission is granted later but who holds no valid session sees + * assistance disabled until the engine answers `true` for them. That is the fail-CLOSED direction and + * matches SEC-022's intent, but there is no "clear cached policy" affordance in the extension. + */ +export function mergeAuthoritativePolicy(cached: AiPolicy | null, fresh: AiPolicy): AiPolicy { + const permitted = evaluatedPermission(fresh.assistPermitted); + if (permitted === null && cached?.assistPermitted === false) { + return { ...fresh, assistPermitted: false }; + } + return { ...fresh, assistPermitted: permitted }; +} + +/** + * Apply the gating rules to a resolved policy. `enabled` false means the chat handler must not call + * the model and should stream `message` instead. The only ENABLED case is BYO with the permission + * granted or unknown (null) — BYO is PHI-safe by construction (code-only context). + * + * It lives here rather than in aiPolicy.ts (it is re-exported from there, so every caller is unchanged) + * because it is the PREDICATE {@link mergeAuthoritativePolicy} feeds. Asserting the merge rule without + * asserting that the retained bit actually reaches this gate would prove only that a field survived in + * a struct — and that coupling has to be provable in the node-side runner that runs on every CI leg. + */ +export function assistantState(p: AiPolicy): { enabled: boolean; message?: string } { + if (p.mode === "off") { + return { enabled: false, message: "AI assistance is turned off by your MessageFoundry policy." }; + } + if (p.mode === "unverified") { + // Engine unreachable + no cached policy + no positive local CLI policy: fail closed so a central + // "off" / ai:assist deny can't be bypassed by going offline (SEC-022). + return { + enabled: false, + message: + "MessageFoundry AI policy could not be verified (engine unreachable) — assistance is disabled until it can be confirmed.", + }; + } + if (p.mode === "managed_endpoint") { + // Engine-brokered path (ADR 0135): the ENGINE brokers the call to the customer-managed / self-hosted + // LLM under central per-use audit. Enabled when the caller holds ai:assist (or it is unknown offline); + // chat.ts routes this mode to the engine broker instead of the local vscode.lm model. + if (p.assistPermitted === false) { + return { enabled: false, message: "Your role does not include the ai:assist permission." }; + } + return { enabled: true }; + } + if (p.mode === "managed_claude" || p.mode === "managed_claude_baa") { + return { + enabled: false, + message: + "Your MessageFoundry policy uses a managed AI provider, which this extension version does not yet support. Assistance is unavailable.", + }; + } + if (p.mode === "byo" && p.assistPermitted === false) { + return { enabled: false, message: "Your role does not include the ai:assist permission." }; + } + // BYO with assistPermitted true OR null (RBAC not evaluable offline) — allowed. + return { enabled: true }; +} diff --git a/ide/src/engineStatusModel.ts b/ide/src/engineStatusModel.ts index 0717c42b..ddefcf35 100644 --- a/ide/src/engineStatusModel.ts +++ b/ide/src/engineStatusModel.ts @@ -3,6 +3,13 @@ // MEAN?", "what may the user do about it?", "what does the item say?") is unit-testable node-side // without launching an Extension Host (mirrors promoteTarget.ts / engineTarget.ts). No vscode, no I/O. // +// It also holds the IDE's SHARED PROBE-PLAN VOCABULARY — `ProbePlanEntry`, `PROBE_ENDPOINTS`, and the +// named plans below (ADR 0110, amended by BACKLOG #330). Those are no longer private to the status +// bar: `aiPolicy.ts` drives its own read of `/ai/policy` off `ASSIST_GATE_PLAN`. The plans are the +// place where "does this caller attach a bearer?" is DATA that CI asserts, rather than a literal +// argument at a call site that a later tidy-up could silently flip. Read the two /ai/policy plans +// together — they name the same route and give opposite answers, deliberately. +// // THE RULE THIS MODULE EXISTS TO ENFORCE (ADR — engine-link doctor): // Green means "the IDE can USE this engine", not "a socket answered". // The previous model folded EVERY http answer — including a 401 — into "reachable" and painted it @@ -126,6 +133,11 @@ export const PROBE_ENDPOINTS = ["/ai/policy", "/config/provenance", "/health"] a /** The tokenless liveness probe. Cheap, and safe on a timer precisely BECAUSE it is tokenless. */ export const HEALTH_ROUTE = "/health"; +/** The policy read that names the engine's active environment (ADR 0017 owns `[ai].environment`). + * Declared here, beside HEALTH_ROUTE, because the probe plans below reference it — a `const` read + * before its declaration is a TDZ ReferenceError at module load, not a compile error. */ +export const POLICY_ROUTE = "/ai/policy"; + /** One planned HTTP probe. `authenticated` decides whether a bearer is attached — and it is the whole * ballgame, so it is DATA (asserted in CI) rather than a decision buried in the shell. */ export interface ProbePlanEntry { @@ -154,9 +166,32 @@ export const VERIFY_PLAN: readonly ProbePlanEntry[] = [ { route: HEALTH_ROUTE, authenticated: true }, // learns the version, which a tokenless probe never sees ]; -/** The tokenless policy read that names the engine's active environment (ADR 0017). Best-effort: any - * failure just means the hover omits the environment line. */ -export const POLICY_ROUTE = "/ai/policy"; +/** + * The status bar's environment read. **TOKENLESS** — it wants `environment`, which is + * identity-INDEPENDENT, and it runs off the 15s timer, where a bearer would refresh the session's idle + * clock and make the engine's 30-minute idle timeout unreachable (CWE-613; see the file header and + * ADR 0110 §2). Best-effort: any failure just means the hover omits the environment line. + * + * Same route as {@link ASSIST_GATE_PLAN}, opposite answer. That is not an oversight — it IS the + * distinction, and both halves are asserted in CI so a later reader cannot "unify" them. + */ +export const ENVIRONMENT_PLAN: readonly ProbePlanEntry[] = [ + { route: POLICY_ROUTE, authenticated: false }, +]; + +/** + * `aiPolicy.ts`'s gate read of the same route. **AUTHENTICATED** — it wants `assist_permitted`, which + * is identity-DEPENDENT and is `null` for every tokenless caller (`api/app.py`: `permitted = None if + * identity is None else identity.has(Permission.AI_ASSIST)`), so without a bearer the deny branch can + * never fire at all. + * + * Safe here for exactly the reason {@link VERIFY_PLAN} is: its only callers are user-initiated — a chat + * turn (`chat.ts`) and the "Show AI Policy" command (`aiPolicy.ts`) — never a timer, so refreshing the + * idle clock is honest activity rather than a forgery. + */ +export const ASSIST_GATE_PLAN: readonly ProbePlanEntry[] = [ + { route: POLICY_ROUTE, authenticated: true }, +]; /** * The route that EARNS a green check. It must be (a) authenticated, (b) permission-gated, and (c) NOT diff --git a/ide/src/statusBar.ts b/ide/src/statusBar.ts index 65b8682e..f1cd4024 100644 --- a/ide/src/statusBar.ts +++ b/ide/src/statusBar.ts @@ -36,7 +36,7 @@ import { logAction, logProbe, logState, showEngineLog } from "./engineLog"; import { assertTargetAllowed, isLocalEngine } from "./engineTarget"; import { CMD, - POLICY_ROUTE, + ENVIRONMENT_PLAN, POLL_PLAN, VERIFY_PLAN, classifyDeep, @@ -297,7 +297,15 @@ export class EngineStatusBar implements vscode.Disposable { } } - /** Best-effort, tokenless: name the engine's active environment in the hover. Never fails the probe. */ + /** + * Best-effort, TOKENLESS: name the engine's active environment in the hover. Never fails the probe. + * + * It runs off the 15s poll, so it must never carry a bearer (CWE-613 — see the file header). That is + * driven by {@link ENVIRONMENT_PLAN}'s `authenticated: false` rather than a literal `undefined` token + * argument, so the tokenlessness is DATA that CI asserts. `aiPolicy.ts` reads the SAME route WITH a + * bearer under `ASSIST_GATE_PLAN`, because it wants the identity-dependent `assist_permitted` and is + * user-initiated; the difference between the two is deliberate (ADR 0110 amendment, BACKLOG #330). + */ private async readEnvironment(): Promise { if ( this.link.environment || @@ -309,7 +317,7 @@ export class EngineStatusBar implements vscode.Disposable { } const gen = this.targetGen; const target = this.link.target; - const outcome = await this.fetch(target.url, POLICY_ROUTE, undefined, PROBE_TIMEOUT_MS); + const outcome = (await this.runProbe(ENVIRONMENT_PLAN[0], target.url, PROBE_TIMEOUT_MS)).outcome; if (outcome.kind !== "ok" || gen !== this.targetGen) { return; } diff --git a/ide/src/test/suite/ai-policy-model.test.ts b/ide/src/test/suite/ai-policy-model.test.ts new file mode 100644 index 00000000..8c8fb2e7 --- /dev/null +++ b/ide/src/test/suite/ai-policy-model.test.ts @@ -0,0 +1,126 @@ +import * as assert from "assert"; + +import { assistantState, mergeAuthoritativePolicy, type AiPolicy } from "../../aiPolicyModel"; + +// BACKLOG #330, defect 1 — the authoritative-merge guard, and its coupling to the gate it feeds. +// +// WHY THIS FILE EXISTS SEPARATELY FROM ai-policy.test.ts: `ide/package.json`'s `test:unit` script +// `--ignore`s `out/test/suite/ai-policy.test.js` (aiPolicy.ts imports vscode), so anything asserted +// only there runs solely in the Windows-only `npm test` leg. These import the zero-import +// aiPolicyModel, so they execute in the node-side runner on EVERY ide CI leg — which is where the +// security invariant belongs. +// +// The rule under test is ASYMMETRIC, and the asymmetry is the whole design: a cached DENY survives a +// fresh `null`; a cached PERMIT does not. Tests T1 and T3 are the two poles, and neither alone can +// tell a correct guard from a broken one. +function policy(p: Partial): AiPolicy { + return { + mode: "byo", + dataScope: "code_only", + environment: null, + assistPermitted: null, + reason: null, + ...p, + }; +} + +suite("mergeAuthoritativePolicy (BACKLOG #330 — a degraded read must not upgrade assistance)", () => { + test("T1: a fresh null does NOT overwrite a cached ai:assist deny", () => { + // The exact defect-1 regression. `assist_permitted` is identity-dependent, so an unattributed read + // answers null; writing that raw would re-enable assistance a central policy had switched off. + const merged = mergeAuthoritativePolicy( + policy({ assistPermitted: false }), + policy({ assistPermitted: null }), + ); + assert.strictEqual(merged.assistPermitted, false); + }); + + test("T2: an evaluable GRANT un-sticks the deny (the escape hatch is signing in)", () => { + // Without this, the guard would be a permanent lockout: once false was cached, nothing could clear + // it. T1 alone passes for a guard that never lets go — this is what distinguishes the two. + const merged = mergeAuthoritativePolicy( + policy({ assistPermitted: false }), + policy({ assistPermitted: true }), + ); + assert.strictEqual(merged.assistPermitted, true); + }); + + test("T3: a cached PERMIT is NOT sticky — null wins, and stays enabled", () => { + // The fail-OPEN direction, pinned. Carrying a cached `true` over a fresh `null` would fabricate a + // permit from stale state; `null` under BYO is allowed by design (docs/AI.md's trust note), so the + // correct behaviour is to take the null and remain enabled — not to invent a grant. + const merged = mergeAuthoritativePolicy( + policy({ assistPermitted: true }), + policy({ assistPermitted: null }), + ); + assert.strictEqual(merged.assistPermitted, null); + assert.strictEqual(assistantState(merged).enabled, true); + }); + + test("T4: `mode` is never resurrected from the cache — a central re-enable propagates", () => { + // Only assistPermitted is retained. mode is identity-INDEPENDENT, so an admin flipping off → byo + // must take effect on the very next read; a guard that froze the whole policy would pass T1. + const merged = mergeAuthoritativePolicy( + policy({ mode: "off", assistPermitted: false }), + policy({ mode: "byo", assistPermitted: null }), + ); + assert.strictEqual(merged.mode, "byo"); + assert.strictEqual(merged.assistPermitted, false, "the deny is still retained"); + }); + + test("T5: the first ever read (no cache) is a plain pass-through and does not throw", () => { + const fresh = policy({ mode: "byo", assistPermitted: null }); + assert.deepStrictEqual(mergeAuthoritativePolicy(null, fresh), fresh); + }); + + test("T6: the retained deny actually reaches the GATE, not just the struct", () => { + // The item's real claim. A field that survives a merge but never changes assistantState's verdict + // would be a fix in name only. + const merged = mergeAuthoritativePolicy( + policy({ mode: "byo", assistPermitted: false }), + policy({ mode: "byo", assistPermitted: null }), + ); + const state = assistantState(merged); + assert.strictEqual(state.enabled, false); + assert.ok(/ai:assist/i.test(state.message ?? ""), "the message names the missing permission"); + }); + + // T7-T9: the guard must key on "is this an EVALUABLE answer", not on the single literal `null`. + // T1-T6 all construct `assistPermitted: null`, so none of them can see a value that is merely + // not-null — and `undefined !== null`, so a `=== null` guard lets one straight through. + test("T7: a fresh answer that OMITS the permission does not overwrite a cached deny", () => { + // The reachable degraded case: a 200 whose body has no `assist_permitted` at all parses to + // `undefined`. `AiPolicyWire` is a compile-time claim and JSON.parse does not honour it, so this + // reaches the merge as a real value. Unguarded it is the fail-OPEN direction — and worse than a + // one-off, because `undefined` is not `false` either, so the cache is poisoned permanently. + const merged = mergeAuthoritativePolicy(policy({ assistPermitted: false }), { + ...policy({}), + assistPermitted: undefined as unknown as boolean | null, + }); + assert.strictEqual(merged.assistPermitted, false, "the deny survives an absent field"); + assert.strictEqual(assistantState(merged).enabled, false, "and it still reaches the gate"); + }); + + test("T8: a non-boolean answer is not a permit either", () => { + // Same rule, other shape: a proxy or a mistyped target answering a string must not read as + // "permitted". Anything that is not the literal true/false means "not evaluated". + const merged = mergeAuthoritativePolicy(policy({ assistPermitted: false }), { + ...policy({}), + assistPermitted: "yes" as unknown as boolean | null, + }); + assert.strictEqual(merged.assistPermitted, false); + assert.strictEqual(assistantState(merged).enabled, false); + }); + + test("T9: a non-evaluable answer is NORMALIZED to null, so the cache stays comparable", () => { + // With no cache there is no deny to retain — but the value written must still be `null`, not the + // `undefined` that came in. Storing `undefined` would make every later `cached?.assistPermitted + // === false` test false, i.e. a single degraded read would disarm the guard for good. + const merged = mergeAuthoritativePolicy(null, { + ...policy({}), + assistPermitted: undefined as unknown as boolean | null, + }); + assert.strictEqual(merged.assistPermitted, null); + assert.ok("assistPermitted" in merged, "the key is present, not dropped"); + }); +}); diff --git a/ide/src/test/suite/ai-policy.test.ts b/ide/src/test/suite/ai-policy.test.ts index 2bf30db3..76d0b4ac 100644 --- a/ide/src/test/suite/ai-policy.test.ts +++ b/ide/src/test/suite/ai-policy.test.ts @@ -1,6 +1,17 @@ import * as assert from "assert"; -import { assistantState, pickOfflinePolicy, type AiPolicy } from "../../aiPolicy"; +import type * as vscode from "vscode"; + +import { + DEFAULT_AI_POLICY_IO, + assistantState, + pickOfflinePolicy, + resolveAiPolicy, + type AiPolicy, + type AiPolicyIo, + type AiPolicyWire, +} from "../../aiPolicy"; +import { peekToken } from "../../auth"; // SEC-022 regression. The offline AI-policy resolution must FAIL CLOSED: when the engine is // unreachable and nothing can positively confirm a policy, assistance is disabled (an org-set central @@ -63,3 +74,178 @@ suite("pickOfflinePolicy (SEC-022)", () => { assert.strictEqual(assistantState(fallback).enabled, false); }); }); + +// BACKLOG #330 — resolveAiPolicy's fetch-and-cache path, where BOTH defects lived and which had no +// test at all. It needs the vscode-importing module, so these run only under `npm test` (the +// Extension-Host leg); the pure merge rule they depend on is asserted node-side on every leg in +// ai-policy-model.test.ts. That split is deliberate — see R4 in the plan. + +/** A fake globalState that records what was written, so a guard cannot pass by not writing. */ +function fakeCtx(seed?: AiPolicy): { + ctx: vscode.ExtensionContext; + stored: () => AiPolicy | undefined; + updates: () => number; +} { + let value = seed; + let updates = 0; + const ctx = { + globalState: { + get: (_key: string): AiPolicy | undefined => value, + update: async (_key: string, v: AiPolicy): Promise => { + value = v; + updates++; + }, + }, + } as unknown as vscode.ExtensionContext; + return { ctx, stored: () => value, updates: () => updates }; +} + +function wire(w: Partial): AiPolicyWire { + return { + mode: "byo", + data_scope: "code_only", + environment: null, + assist_permitted: null, + reason: null, + ...w, + }; +} + +/** An io whose calls are recorded. `url` defaults to loopback, which SEC-005 permits over http. */ +function fakeIo(opts: { + url?: string; + token?: string; + answer: AiPolicyWire; +}): AiPolicyIo & { calls: { url: string; route: string; token: string | undefined }[] } { + const calls: { url: string; route: string; token: string | undefined }[] = []; + return { + calls, + url: () => opts.url ?? "http://127.0.0.1:8765", + readToken: async () => opts.token, + getPolicy: async (url, route, token) => { + calls.push({ url, route, token }); + return opts.answer; + }, + getCliPolicy: async () => { + throw new Error("the CLI must not be consulted when the engine answered"); + }, + }; +} + +suite("resolveAiPolicy — the bearer (BACKLOG #330, defect 2)", () => { + test("T10: the cached bearer is attached to the /ai/policy read", async () => { + const { ctx } = fakeCtx(); + const io = fakeIo({ token: "tok-abc", answer: wire({ assist_permitted: false }) }); + const p = await resolveAiPolicy(ctx, io); + assert.strictEqual(io.calls.length, 1); + assert.strictEqual(io.calls[0].token, "tok-abc", "the bearer must reach the request"); + assert.strictEqual(io.calls[0].route, "/ai/policy"); + // And the whole point: with an identity attached, the engine's `false` now arrives and gates. + assert.strictEqual(p.assistPermitted, false); + assert.strictEqual(assistantState(p).enabled, false); + }); + + test("T11: the production io reads the token PASSIVELY — peekToken, never ensureToken", async () => { + // `ensureToken` has an identical signature, so the type system cannot rule it out; this identity + // check is the actual control. A chat turn resolves the policy before every request — an + // ensureToken here would pop a sign-in modal out of typing a question. + assert.strictEqual(DEFAULT_AI_POLICY_IO.readToken, peekToken); + }); + + test("T12: SEC-005 — no bearer to a non-loopback plain-http target, but yes to loopback", async () => { + const offBox = fakeIo({ + url: "http://engine.example.test:8765", + token: "tok-abc", + answer: wire({}), + }); + await resolveAiPolicy(fakeCtx().ctx, offBox); + assert.strictEqual( + offBox.calls[0].token, + undefined, + "a bearer must never go in clear to a non-loopback http:// host", + ); + + // The other polarity, so the guard cannot pass by refusing everything. + const loopback = fakeIo({ url: "http://127.0.0.1:8765", token: "tok-abc", answer: wire({}) }); + await resolveAiPolicy(fakeCtx().ctx, loopback); + assert.strictEqual(loopback.calls[0].token, "tok-abc", "loopback over http is the dev default"); + }); +}); + +suite("resolveAiPolicy — the guarded cache write (BACKLOG #330, defect 1)", () => { + test("T13: a null answer does not overwrite a cached deny, in the RETURN or the STORE", async () => { + const seeded = { + mode: "byo", + dataScope: "code_only", + environment: null, + assistPermitted: false, + reason: null, + } satisfies AiPolicy; + const f = fakeCtx(seeded); + const io = fakeIo({ token: "tok-abc", answer: wire({ assist_permitted: null }) }); + const p = await resolveAiPolicy(f.ctx, io); + // Assert BOTH: a half-fix that returns the merged policy while storing the raw one would leave the + // deny to be lost on the next read. + assert.strictEqual(p.assistPermitted, false, "the returned policy keeps the deny"); + assert.strictEqual(f.stored()?.assistPermitted, false, "the STORED policy keeps the deny"); + assert.strictEqual(assistantState(p).enabled, false); + }); + + test("T13b: a 200 that OMITS assist_permitted cannot launder the deny either", async () => { + // The COMPOSED behaviour, end to end: `AiPolicyWire` is a compile-time claim that JSON.parse does + // not enforce, so a body with no `assist_permitted` key arrives as `undefined` — which is NOT + // `null`. Two independent lines stop it (the `fromWire` narrowing and the merge's), so this test + // stays green if either survives; T7 in ai-policy-model.test.ts is what pins the merge's line + // specifically, and T13c below pins `fromWire`'s. Kept because it is the only assertion that the + // whole path — wire → narrow → merge → store → gate — holds together. + const seeded = { + mode: "byo", + dataScope: "code_only", + environment: null, + assistPermitted: false, + reason: null, + } satisfies AiPolicy; + const f = fakeCtx(seeded); + const degraded = { + mode: "byo", + data_scope: "code_only", + environment: null, + reason: null, + } as unknown as AiPolicyWire; // a proxy, or an engine build predating the field + const p = await resolveAiPolicy(f.ctx, fakeIo({ token: "tok-abc", answer: degraded })); + assert.strictEqual(p.assistPermitted, false, "the returned policy keeps the deny"); + assert.strictEqual(f.stored()?.assistPermitted, false, "the STORED policy keeps the deny"); + assert.strictEqual(assistantState(p).enabled, false); + }); + + test("T13c: the CLI fallback narrows too — it never reaches the merge, so fromWire must", async () => { + // The path that proves `fromWire`'s narrowing is not redundant with the merge's: when the engine + // is unreachable, `resolveAiPolicy` runs `fromWire(await io.getCliPolicy())` and hands the result + // to `pickOfflinePolicy` WITHOUT ever calling mergeAuthoritativePolicy (aiPolicy.ts). So an + // unnarrowed copy survives to the caller as `undefined`, and `showAiPolicy` renders that with + // `assistPermitted === null ? "unknown" : p.assistPermitted ? "yes" : "no"` — i.e. it would tell a + // user "assist_permitted=no", asserting a deny the engine never issued. + const io: AiPolicyIo = { + url: () => "http://127.0.0.1:8765", + readToken: async () => "tok-abc", + getPolicy: async () => { + throw new Error("engine unreachable"); + }, + getCliPolicy: async () => + ({ mode: "byo", data_scope: "code_only", environment: "dev", reason: null }) as unknown as AiPolicyWire, + }; + const p = await resolveAiPolicy(fakeCtx().ctx, io); + assert.strictEqual(p.assistPermitted, null, "an absent CLI field is 'unknown', never a deny"); + assert.notStrictEqual(p.assistPermitted, undefined, "and never the raw undefined"); + }); + + test("T14: AC-6 still holds — a successful read is still CACHED (the guard is not a skip)", async () => { + // The obvious wrong turn is to "fix" an unguarded write by removing it. That would break ADR 0035 + // AC-6: the cache is what makes a central "off" survive the engine going offline. + const f = fakeCtx(); + const io = fakeIo({ token: "tok-abc", answer: wire({ mode: "off" }) }); + await resolveAiPolicy(f.ctx, io); + assert.strictEqual(f.updates(), 1, "the authoritative answer must be written exactly once"); + assert.strictEqual(f.stored()?.mode, "off"); + }); +}); diff --git a/ide/src/test/suite/engine-client.test.ts b/ide/src/test/suite/engine-client.test.ts index 2349ddfe..6af52369 100644 --- a/ide/src/test/suite/engine-client.test.ts +++ b/ide/src/test/suite/engine-client.test.ts @@ -83,3 +83,46 @@ suite("engineClient — getJson timeout (F2)", () => { assert.ok(/nothing is listening/i.test(dead.reason ?? "")); }); }); + +// BACKLOG #330 — the LAST HOP of the chain. Every other test in this change stops at a call boundary +// (a recorded argument, a plan constant); this one puts a real request on a real socket and reads the +// header the engine would see. Without it the item's headline claim — "the read is authenticated" — +// rests on a header nobody has observed. Both polarities are asserted: that a token becomes a Bearer +// header, AND that a tokenless call really sends nothing (the status bar's read depends on the latter). +suite("engineClient — getJson sends the bearer only when given one (BACKLOG #330)", () => { + let server: http.Server; + let url: string; + let seen: string | undefined; + let sawHeader = false; + + setup(async () => { + seen = undefined; + sawHeader = false; + server = http.createServer((req, res) => { + seen = req.headers.authorization; + sawHeader = "authorization" in req.headers; + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ ok: true })); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const addr = server.address() as AddressInfo; + url = `http://127.0.0.1:${addr.port}`; + }); + + teardown(async () => { + await new Promise((resolve) => server.close(() => resolve())); + }); + + test("T15a: a token is sent as `Authorization: Bearer `", async () => { + await getJson(url, "/ai/policy", "tok-abc"); + assert.strictEqual(seen, "Bearer tok-abc"); + }); + + test("T15b: a tokenless call sends NO Authorization header at all", async () => { + // Not merely "not a valid token" — the header must be absent. `Bearer undefined` would be a string + // the engine tries to resolve, and on the status bar's 15s path any bearer at all is the CWE-613 bug. + await getJson(url, "/ai/policy"); + assert.strictEqual(seen, undefined); + assert.strictEqual(sawHeader, false, "the header key must not be present"); + }); +}); diff --git a/ide/src/test/suite/engine-doctor.test.ts b/ide/src/test/suite/engine-doctor.test.ts index 5c86e647..0f2ffdc6 100644 --- a/ide/src/test/suite/engine-doctor.test.ts +++ b/ide/src/test/suite/engine-doctor.test.ts @@ -1,9 +1,12 @@ import * as assert from "assert"; import { + ASSIST_GATE_PLAN, CMD, DEEP_PROBE_ROUTE, ENGINE_LINK_FIELDS, + ENVIRONMENT_PLAN, + POLICY_ROUTE, POLL_PLAN, PROBE_ENDPOINTS, VERIFY_PLAN, @@ -107,6 +110,54 @@ suite("engine doctor — the boundary, as CI", () => { } }); + test("T7: the status bar's /ai/policy environment read stays TOKENLESS (CWE-613)", () => { + // statusBar.readEnvironment runs off the same 15s timer as the poll, via runProbe — which attaches a + // bearer IFF the plan entry says `authenticated`. Flipping this to true would put a bearer on the + // timer and make the engine's 30-minute idle timeout unreachable, on the exact client the + // automatic-logoff control exists for. It wants `environment`, which is identity-INDEPENDENT, so it + // has nothing to gain from a token in the first place. + assert.ok(ENVIRONMENT_PLAN.length > 0); + for (const entry of ENVIRONMENT_PLAN) { + assert.strictEqual( + entry.authenticated, + false, + `the status bar's environment read must not authenticate (${entry.route}) — it is on a timer`, + ); + assert.ok( + (PROBE_ENDPOINTS as readonly string[]).includes(entry.route), + `${entry.route} is not an allowed probe endpoint`, + ); + } + assert.strictEqual(ENVIRONMENT_PLAN[0].route, POLICY_ROUTE); + }); + + test("T8: aiPolicy's gate read DOES authenticate — same route, opposite answer, on purpose", () => { + // BACKLOG #330. `assist_permitted` is computed from the acting identity (api/app.py), so a tokenless + // read can only ever answer null and the ai:assist deny branch can never fire. Unlike the status + // bar's read, this one is user-initiated only (a chat turn / the Show AI Policy command), so it sits + // under VERIFY_PLAN's rationale rather than POLL_PLAN's: refreshing the idle clock there is honest + // activity, not a forgery. + assert.strictEqual(ASSIST_GATE_PLAN.length, 1); + assert.strictEqual(ASSIST_GATE_PLAN[0].authenticated, true); + assert.ok( + (PROBE_ENDPOINTS as readonly string[]).includes(ASSIST_GATE_PLAN[0].route), + `${ASSIST_GATE_PLAN[0].route} is not an allowed probe endpoint`, + ); + // The distinction itself, asserted — so a later reader who finds two plans for one route cannot + // "unify" them without turning this red. Same route; the ANSWER is what differs, and it differs + // because the two callers want different FIELDS of the same document. + assert.strictEqual( + ENVIRONMENT_PLAN[0].route, + ASSIST_GATE_PLAN[0].route, + "both plans read the same route", + ); + assert.notStrictEqual( + ENVIRONMENT_PLAN[0].authenticated, + ASSIST_GATE_PLAN[0].authenticated, + "the timer-driven read and the user-initiated read must NOT agree about the bearer", + ); + }); + test("the user-initiated deep check MAY authenticate — and only it may", () => { // A click / activation / promote IS real user activity, so refreshing the idle clock there is honest. assert.ok(VERIFY_PLAN.some((e) => e.authenticated)); From 2240c9030473ffd0f79711ffc8805bf431ce1598 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 4 Aug 2026 11:35:04 -0500 Subject: [PATCH 2/8] docs(backlog): flip #330's banner to FIXED (BACKLOG #330) Isolated from the code+tests commit, per the ledger convention. Only the single banner line under "## 330." changed -- verified BY NUMBER, not by banner text: the one changed line is 3038, and the nearest "## " heading above it is line 3036, "## 330. The IDE's `ai:assist` gate can never fire". Exactly one status blockquote exists under that heading, it is the CLOSED glyph, and no OPEN glyph coexists with it. That check is the evidence, not the status gate exiting 0 -- the gate validates that a banner is present and self-consistent, never that it belongs to the item it sits under, so it would pass just as happily on a banner pasted from a neighbouring item. The CENSUS WAS NOT RECOMPUTED. The four distribution lines and the ranked table are untouched; this commit changes one item's banner only. The banner records three residuals rather than claiming a clean close: (a) the status bar's tokenlessness is asserted on the plan constant, not on readEnvironment's use of it (nothing constructs EngineStatusBar); (b) the policy cache is one global key while the bearer is keyed per engine URL, so a deny seen against one engine also suppresses another -- fail-closed, and recorded in ADR 0035 AC-7; and (c) the pre-existing engineUrl() vs environments()[0].url targeting gap, which would leave the gate unable to fire for a user whose only session is against a named environment URL. That one needs its own number. --- docs/BACKLOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index da09dba2..57f843d9 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -3093,7 +3093,7 @@ That distinction matters concretely for the ASVS record. The scorecard's absence ## 330. The IDE's `ai:assist` gate can never fire -> 🔢 **Filed 2026-08-01 — not started.** Value **5/10** · Difficulty **3/10** · _fill-in_. ADR 0035's SEC-022 `ai:assist` half was never wired — `resolveAiPolicy` omits `getJson`'s token argument (`ide/src/aiPolicy.ts:78`, against the header-when-present at `ide/src/engineClient.ts:141`) so the engine can only ever answer `null` and `docs/AI.md:188` publishes a deny row no code path produces — but no PHI is at risk, the brokered path is server-gated, and the `mode` half still covers the central-off case; TypeScript in one module, ordered so the unconditional cache write at `aiPolicy.ts:79` is guarded before the bearer lands, with the status-bar reader left tokenless or the CWE-613 idle clock becomes unreachable. +> ✅ **FIXED 2026-08-04 — ADR 0035 AC-7/AC-8 added, ADR 0110 amended.** Both defects are closed, in the load-bearing order. **(1) The guard landed first:** the write to `LAST_POLICY_KEY` now goes through a pure `mergeAuthoritativePolicy` (`ide/src/aiPolicyModel.ts`, zero imports so it is asserted node-side on every CI leg), so an answer that does not carry an evaluable `assist_permitted` can no longer overwrite a cached `false`. "Not evaluable" is deliberately wider than the literal `null`: `AiPolicyWire` is a compile-time claim `JSON.parse` does not enforce, so a 200 that OMITS the field arrives as `undefined` — which a `=== null` guard would let through, and which is not `false` either, so the cache would be poisoned past recovery. The bit is narrowed at the boundary (`evaluatedPermission`) on both authoritative paths, the engine read and the CLI fallback. The retention is one-way by design — a cached `true` is **not** sticky (fabricating a permit is the fail-open direction), an evaluable `true`/`false` always wins outright, and `mode` always comes fresh so a central `off`→`byo` re-enable still propagates. **(2) Then the bearer:** `resolveAiPolicy` attaches the cached token via `peekToken` (never `ensureToken` — a chat turn must not pop a sign-in modal) behind the SEC-005 `assertTargetAllowed` gate, so the engine can resolve the identity-dependent `assist_permitted` and the `ai:assist` deny branch can fire at all. The two orderings are not equivalent: attaching the bearer first would open a window in which an authenticated-but-degrading read poisons the cache. **`statusBar.ts`'s `/ai/policy` read stays TOKENLESS** — the two readers are now the named constants `ENVIRONMENT_PLAN` (`authenticated: false`, timer-driven, wants the identity-independent `environment`) and `ASSIST_GATE_PLAN` (`authenticated: true`, user-initiated only), same route and opposite answer, both asserted in CI so a later reader cannot "unify" them into the CWE-613 bug. 20 new tests (`ai-policy-model.test.ts`, `ai-policy.test.ts`, `engine-doctor.test.ts`, `engine-client.test.ts`), each falsified against a planted defect. **Residuals, deliberately not closed here:** (a) nothing constructs `EngineStatusBar`, so the status bar's tokenlessness is asserted on the plan CONSTANT, not on `readEnvironment`'s use of it — rewiring that call site would type-check and stay green (recorded in ADR 0035 AC-8); (b) the cache is one global key while the bearer is keyed per engine URL, so a deny observed against one engine also suppresses assistance against another (fail-closed, recorded in AC-7); and (c) the bearer is looked up under `engineUrl()`, but sign-in happens against the status-bar/promote target, which is `environments()[0].url` whenever `messagefoundry.environments` is configured — so for a user whose only session is against a named environment URL the read is still unattributed and the gate still cannot fire for them. Retargeting `resolveAiPolicy` changes WHICH engine the policy is read from, a behaviour change this item does not ask for; it needs its own number. **Cluster:** Security & Compliance / IDE & Authoring. **Priority:** P2. **Verdict:** build. **Severity:** medium. From 6d71938323717d4ee49b062d4938a3efaf140c81 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 4 Aug 2026 13:51:52 -0500 Subject: [PATCH 3/8] test(steps): the webview drop mirrors had no gate, and one had already diverged (BACKLOG #233) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ide/media/stepsWebview.js` is loaded as a classic script into a `default-src 'none'` webview, so it cannot import `ide/src/stepsModel.ts` and re-implements ten pure model functions by hand. The drag/drop PREVIEW comes from the webview copy; the COMMITTED splice coordinates come from the model copy. A divergence lands a statement somewhere other than where the indicator said, byte-stably, re-parsing clean, with every existing test green — and nothing compared the two. Owner ruling: option (c) of the item — a differential test, NOT de-duplication. The duplication stands; it is now gated instead of eliminated. BACKLOG #233 — the parity gate * New `ide/src/test/suite/steps-mirror.test.ts` (1,492 lines, 50 cases). It loads the REAL webview script under jsdom with a recording `acquireVsCodeApi` double and reaches the mirrors through an opt-in `window.__mfStepsTestExports` hook — a hook handing out the SAME function objects the page uses, never a second implementation. * Two populations, because the mirrors split in two: - the five row-array mirrors (`blockExtent`, `captureBlock`+`clipLabel`, `buildDropSlots`, `walkMove`) are swept over 2,000 seeded generated row sets (`mulberry32`; the seed is printed with any divergence so it reproduces exactly); - the four DOM-bound ones (`canDrop`, `resolveDrop`, `barAnchor`, `scopeLabel`) take `
  • ` elements and a `getBoundingClientRect`, so they run over four hand-authored adversarial cases x all ordered (drag, target) pairs x pointer fractions 0.1/0.4/0.5/0.6/0.9. 0.4 and 0.6 straddle the 1/3 and 2/3 tri-zone thresholds; a threshold drift to 1/2 is invisible to 0.1/0.5/0.9 alone (falsified: every failure landed at 0.4). * Adapter discipline: the webview side always comes from the RENDERED DOM (`renderRowHtml` -> dataset -> `stepsCtxRows`), the model side always from the view models, and every adapter is a one-line field read. The comparison therefore spans the real serialization boundary, where `suite`/`isControlHeader`/`draggable`/`data-is-return` are actually decided. * The one live divergence it found is fixed: `canDropRow` accepted a read-only `code` row as a drop target while the webview refused it. `target.draggable` does not exclude a code row — `renderRowHtml` marks one draggable ON PURPOSE so the gesture can be intercepted — so the model contradicted its own stated contract ("never treats a code row as a drop target"). On the shipped code a deploying site would have seen the insertion indicator refuse a code row while the model-side resolution accepted it. * `buildDropSlots` is exported so it can be compared; it has no production caller outside `walkMove`. An inventory guard fails on an 11th top-level webview function that has neither a parity assertion nor a "not a mirror" allowlist entry with a reason. * `jsdom@^29.1.1` (MIT) added as an `ide/` devDependency with `package-lock.json` re-locked in the same commit (DEP-1). The suite imports no `vscode`, and the file is outside `test:unit`'s `--ignore` list, so it runs on EVERY `ide` leg, not only the Windows Extension Host one. BACKLOG #234 — a save suppressed by the edit guard is DEFERRED, not dropped * `EditLoopGuard.shouldReactToDocumentChange()` returns false while an edit is in flight — right for our own `WorkspaceEdit`, but the provider consumed it as an unconditional return, so a USER save that merely landed inside an in-flight `lens rewrite` was discarded. On first deployment that would surface as "I saved and the Steps view did not update", with no signal, until the next save. * The guard records a clear-on-read debt (`noteSuppressedChange`/`takeSuppressedChange`); a new `releaseEdit(guard, onRefreshOwed?)` is the only sanctioned release and pays it. `drainEdits` releases through it, including on the unexpected-rejection path. * A re-projection DISCHARGES an owed refresh — it reads the whole current buffer — so `render()` opens by discharging both routes: cancelling the armed `RerenderDebouncer` (three release sites force a full re-projection right after releasing, which would otherwise replace the whole webview HTML a second time ~250 ms later) and taking the guard's debt (`drainEdits`' rejection handler renders BEFORE the release, so nothing is armed yet to cancel). Source-scan tests pin both, plus the absence of any bare `guard.endEdit()` in the provider. * ADR 0076 gains a dated Amendment C, marked PROPOSED — not ratified, following Amendment B's convention — with an index row in docs/adr/README.md. It argues the change STRENGTHENS the §5 "sync on save only" guardrail rather than relaxing it. Whether to relax the gate itself is #234's other half and is explicitly not decided here. * It also corrects a false premise the gate's own comment rested on: `render()` pipes `document.getText()` to `lens parse -` over stdin, so the rows are projected from the LIVE buffer, not from disk. The disk read belongs to the live-value trace, which #225 save-gates separately. A compensating control must not rest on a false premise (CLAUDE.md §11), and #234's remaining half was about to be argued against this one. Docs * `docs/testing/master-test-plan/13-steps-editor.md`: the §12.3 mirror-divergence risk row flips to detected; §12.2 gains the new suite; §S2 records that there is no enclosing IIFE to host the hook (the file is a classic script) and that the gate keys on `window.__mfStepsTestExportsEnabled`, not on `acquireVsCodeApi`; STEPS-06's un-automatable second clause moves onto STEPS-76's manual checklist; exit criterion 4 is amended to state honestly where the >=2,000-row-set volume applies and where it cannot. * This change moved `stepsView.ts` and `stepsModel.ts` by 2-40 lines, invalidating ~20 line anchors in the same documents. Every one is re-derived, or replaced by a symbol name where the cited file is edited by this same commit. * `19-execution-phasing-and-sign-off.md`'s "verified manually" claim about the mirrors is now past tense. --- ...yped-action-vocabulary-action-list-lens.md | 119 +- docs/adr/README.md | 2 +- .../master-test-plan/13-steps-editor.md | 63 +- .../19-execution-phasing-and-sign-off.md | 5 +- ide/media/stepsWebview.js | 32 + ide/package-lock.json | 523 ++++++ ide/package.json | 1 + ide/src/stepsModel.ts | 132 +- ide/src/stepsView.ts | 100 +- ide/src/test/suite/steps-edit.test.ts | 350 +++- ide/src/test/suite/steps-mirror.test.ts | 1494 +++++++++++++++++ 11 files changed, 2751 insertions(+), 70 deletions(-) create mode 100644 ide/src/test/suite/steps-mirror.test.ts diff --git a/docs/adr/0076-typed-action-vocabulary-action-list-lens.md b/docs/adr/0076-typed-action-vocabulary-action-list-lens.md index fbfdd3d4..2ac018dc 100644 --- a/docs/adr/0076-typed-action-vocabulary-action-list-lens.md +++ b/docs/adr/0076-typed-action-vocabulary-action-list-lens.md @@ -290,8 +290,10 @@ the comment merged into an unrelated row. **The build lands failing tests for (1 `delete_row` would begin failing with "internal: could not locate the statement" on **every** action that has a leading comment. Attachment ("a statement travels with its leading comment block") is a separate, larger item and is gated on BACKLOG #233 — `blockExtent` / `walkMove` / `resolveDrop` are - implemented twice (`ide/src/stepsModel.ts:1767` vs `ide/media/stepsWebview.js:68`) with no - differential test. + implemented twice (`blockExtent` in `ide/src/stepsModel.ts` vs `ide/media/stepsWebview.js:68`) with no + differential test. *(2026-08-04: the "no differential test" half is closed — + `ide/src/test/suite/steps-mirror.test.ts` is that test. The duplication itself stands; the owner chose + the differential gate over de-duplication.)* - **No inline/trailing-comment extraction.** Verified working today: `set_params` on `msg.set("PID-3.1", "X") # noqa: E501` preserves the pragma exactly, and interior comments in a multi-line call are absorbed into the action row's span. A note kind must not touch either. @@ -539,3 +541,116 @@ Acceptance Criteria bucket. Promote to the block above if and when the owner acc - **AC-D5** — Descended rows SHALL either carry live values (requiring an accepted ADR 0072 amendment widening the tracer's frame scope) or SHALL render an explicit "not traced" state distinguishable from PHI redaction — never a redacted placeholder that can never resolve. + +## Amendment C (2026-08-04) — the update-loop guard DEFERS a save-triggered re-projection instead of dropping it (BACKLOG #234) + +> **Status of this amendment: PROPOSED — not ratified.** It is written and built because BACKLOG #234 +> requires the guardrail it touches to be re-argued in a dated amendment in the same change, not because +> the owner has ruled on it; ratification is an owner decision, and until it lands this section follows +> Amendment B's convention (its acceptance criteria sit under a distinct heading, outside this ADR's +> counted Acceptance Criteria bucket). +> +> **What it claims:** it **strengthens** the §5 "sync on save only" guardrail and does **not** relax it. +> The projection still syncs on save and on save only. What changes is what happens to a save that +> arrives while a `lens rewrite` holds the single edit slot: it is now **deferred to slot release** +> instead of being **silently discarded**. Nothing here widens the sync trigger, adds a keystroke path, +> or touches the #225 live-value save gate. + +### C.1 The defect + +`EditLoopGuard.shouldReactToDocumentChange()` returns `false` while an edit is in flight — the correct +answer for the `WorkspaceEdit` the provider itself is applying, which must not feed back into a re-render +that fights the webview (the update loop §5's guardrail set exists to break). + +The provider's save subscription consumed that answer as an unconditional **return**. But the guard +cannot distinguish *our own* `WorkspaceEdit` from a *user* save that merely happened to land inside an +in-flight rewrite — and on that second case the early return **dropped the save**. The view would then +keep rendering a projection of the pre-save buffer with no signal, until the user saved again. On the +shipped code this would surface on first deployment as "I saved and the Steps view did not update"; +there are no deployments today, which is why there is still time to fix it properly rather than +document it. + +### C.2 The change + +- `EditLoopGuard` gains `noteSuppressedChange()` / `takeSuppressedChange()` — a single clear-on-read + boolean, mirroring the existing `queue()` / `takePending()` shape. One boolean, not a queue: a + re-projection reads the whole buffer, so any number of suppressed saves owe exactly **one** refresh. +- A new pure `releaseEdit(guard, onRefreshOwed?)` is **the only sanctioned way to release the slot**: + it calls `endEdit()` and then, only if a change was suppressed, invokes the callback. `drainEdits` + releases through it in its `finally`, including on the unexpected-rejection path. +- The provider records the debt in the save subscription's guard-rejected branch and pays it through the + **same 250 ms debounced `render()`** a real save uses — so a deferred refresh coalesces with a + subsequent save exactly like two rapid saves do, rather than adding a second, differently-timed render. +- That channel is a pure `RerenderDebouncer` (in `stepsModel.ts`, timer functions injected) with two + operations: `schedule()` and **`cancel()`**. **A re-projection DISCHARGES an owed refresh** — it reads + the whole current buffer, so the run starting now already satisfies whatever was owed — and `render()` + therefore begins by discharging both routes, synchronously, before its `lens parse` await (so a save + landing mid-render is recorded afresh and still honoured): + - `rerender.cancel()`, for the release-then-force order. Three of the four release sites + (`applyStructural`, `applyPickedEdit`, `applyUndoRedo`) release the slot and then FORCE a full + re-projection a few lines later; without the cancel a suppressed save produces **two** — the forced + one, then the armed one ~250 ms afterwards, replacing the whole webview HTML again. + - `guard.takeSuppressedChange()`, for the force-then-release order. `drainEdits`' unexpected-rejection + handler renders to revert the optimistic webview change *inside* the drain, and only then does the + `finally` release; nothing is armed yet, so a cancel cannot reach that path. + The paths that return BEFORE any render (a `lens rewrite` refusal, a disposed panel) reach neither + discharge, which is exactly where the deferral is the only route. +- A source-scan test asserts `ide/src/stepsView.ts` contains no bare `guard.endEdit()`, so a future + fourth release site cannot silently reintroduce the drop, and that `render()` opens with both + discharges. + +### C.3 Guardrail accounting (what is NOT changed) + +- **"Sync on save only" stands.** No keystroke, `onDidChangeTextDocument`, or timer path is added. The + deferred refresh is a *save* that already happened; it is being honoured late, not invented. +- **"One editor at a time" stands.** The slot semantics are untouched; `releaseEdit` releases exactly + when `endEdit()` did. +- **The update-loop guard stands.** `shouldReactToDocumentChange()` still returns `false` in flight, so + our own `WorkspaceEdit` still cannot trigger a re-render. The deferral fires *after* the slot frees, + which is precisely when a re-render is safe. +- **`clearPending()` is unchanged and NOT folded in.** Dropping a queued *param edit* on a structural op + (the orphaned-queue rule, §5 v2) and deferring a *document refresh* are different rules with different + reasons; both release sites keep their existing `clearPending()` / `takePending()` behaviour. +- **The #225 live-value save gate is untouched** — an explicit non-goal of BACKLOG #234. + +### C.4 A correction this amendment depends on + +The comment in `ide/src/stepsView.ts` that justified the save gate claimed "`lens parse` reads the file +from disk, so re-projecting on every keystroke would slice the current (dirty) buffer against line ranges +computed from stale disk content". **That premise is false**, and is corrected in the same change: +`render()` pipes `document.getText()` to `lens parse -` over stdin and slices that same snapshot for the +view models, exactly as this ADR's 2026-07-10 addendum states ("the rows are projected from the **live +buffer**"). The disk read belongs to the live-value **trace**, which is separately save-gated by #225. + +This matters beyond tidiness: BACKLOG #234's *other* half asks whether a bounded relaxation of the save +gate is safe, and that question was about to be argued against a premise that does not hold. A +compensating control must not rest on a false premise (CLAUDE.md §11). The real, surviving reasons for +the gate are re-shelling Python per keystroke and the fact that each re-projection replaces the entire +webview HTML — which would destroy focus, selection and any half-typed input mid-word. + +**Deliberately not decided here.** Whether to relax the gate to a debounced re-projection on *change* is +BACKLOG #234's remaining half. It stays open, and it should be decided against the corrected premise +above rather than the false one. This amendment lands the race fix **first**, on purpose: the dropped +refresh is a defect under the current gate and would widen materially under any relaxation. + +## Acceptance Criteria (Amendment C — proposed, not ratified; deliberately outside the counted block) + +*(Same convention as Amendment B: kept under a distinct heading so unratified criteria do not merge into +this ADR's accepted Acceptance Criteria bucket. Promote to the block above if and when the owner +accepts. The criteria are nonetheless **built and tested** — the amendment lands with its gate.)* + +- **AC-C1** — WHILE an edit holds the single edit slot, WHEN a document save for the projected document + is observed, THE SYSTEM SHALL record it and SHALL NOT re-project immediately → guard unit test refs. +- **AC-C2** — WHEN the edit slot is released after one or more suppressed saves, THE SYSTEM SHALL run + exactly ONE re-projection, in EITHER order relative to a forced one: the deferred run goes through the + same debounced channel a direct save uses, and any `render()` discharges the owed refresh on entry — + cancelling the armed channel (release-then-force) and taking the guard's debt (force-then-release) + → `releaseEdit` / `drainEdits` / `RerenderDebouncer` test refs, including a kept-in-tree falsification + showing a render that does not discharge yields two. +- **AC-C3** — WHERE no save was suppressed, releasing the slot SHALL NOT trigger any additional + re-projection → negative test ref. +- **AC-C4** — WHEN `apply` rejects unexpectedly during a drain, THE SYSTEM SHALL still release the slot + AND still run an owed re-projection → rejected-apply test ref. +- **AC-C5** — THE SYSTEM SHALL release the edit slot only through `releaseEdit`; a bare `endEdit()` call + in the provider SHALL fail a source-scan test, as SHALL a `render()` whose first statement is not the + debounce cancel → inventory test refs. diff --git a/docs/adr/README.md b/docs/adr/README.md index 4b8f91f2..83fce587 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -108,7 +108,7 @@ what is withheld and what you can request. | [0073](0073-ownership-scoped-recovery-single-consumer-lanes.md) | Ownership-scoped recovery + single delivery consumer per outbound lane — the N-active-shards-on-one-unified-store reliability runtime (builds ADR 0063's deferred primitive): `reset_stale_inflight(owned=OwnedLanes)` scopes startup/DR crash recovery to a shard's config-graph lanes (channel_id for ingress/routed/response, destination_name for outbound; empty set matches nothing; residual-`IN`/`ANY` predicate keeps the WS-B ready-index seek); deterministic rendezvous (sha256 HRW) outbound-lane ownership over the pinned shard universe, gated at the wake boundary + pooled lane provider + per-lane spawn (predicate, not set — a reload-dropped lane keeps exactly its owner); `--shard`+`[cluster]` refused fail-closed; shard-set-changing reloads refused (fleet restart required); owner-only outbound controls/purge (409 names the owner; `/connections` rows carry `owner_shard`); sharded-only non-owned-lane buildup/stall watchdog (hung-owner paging). N-active stays gated on the clean 4-engine no-loss bench before SYSTEM-REQUIREMENTS calls it supported | Accepted (2026-07-06) — built | | [0074](0074-adopter-capacity-estimator.md) | Adopter-run capacity estimator (BACKLOG #96) — productize the **built** `harness/load/` rate-walk + zero-loss reconcile as a supported `messagefoundry capacity` command an adopter points at *their* box/store/config to answer "does this carry my ~36 msg/s hospital with headroom?"; reports the **per-interface** no-loss ceiling + engine-wide aggregate + a **backend-aware limiting-factor** label + provision-at-≤50%-of-ceiling guidance. Hard requirements: **isolated throwaway store** (refuse to run against a non-isolated/production store — count-and-log intact), **synthetic PHI-free** payloads only (ADR 0030), backend-aware labels (SQLite knob rankings do **not** transfer to server backends — the B12 lesson), explicit harness-ceiling caveats (~450/s ACK per driver, ~135–144/s delivered per sink, poller-zero ⇒ sub-ceiling knee). v1 = rate-walk + limiting-factor labels; deeper per-stage diagnostics deferred. A productization of throughput-campaign evidence (PR #768), not a new measurement effort | Accepted (2026-07-07) — ratified. ⛔ **BUILD GATED (2026-07-14)**: a validity re-check vs STEP-4 Arm 0 found **14 confirmed blockers** in the *measurement* method (the named "only success gate" over-reports by **3–5.5×**; the poller-zero failure mode *satisfies* it; the estimand is intake not delivery; the aggregate-is-the-sum rule is measured-false; the ceiling is instant-partner). **Premise + hard requirements + the fail-closed guard layer still hold and remain buildable.** See the ADR's 2026-07-14 Amendment | | [0075](0075-per-hop-sql-statement-batching.md) | Per-hop SQL statement batching (`[pipeline].batch_handoff_statements`, default-ON (emergency off-switch), fail-closed, SQL-Server-only) — the last [ADR 0069](0069-durable-write-throughput-lever.md)-named feed lever ("batching SQL statements per executor hop"): fold a multi-statement handoff **body** (guard-DELETE + inserts + finalize applock + `messages.status` UPDATE + event, from the SAME shared `(sql,params)` builders that keep the async/sync twins in lockstep) into 1–2 `pyodbc.execute()` batches, cutting **network round-trips + aioodbc executor crossings — NOT transactions** (`commits/msg` stays 2.000; the ADR 0069 cross-lane/commit fence is not hit). Attacks the serial-RT co-bottleneck the [ADR 0071](0071-cut-executor-round-trips-b5.md) B5 fusion NO-GO could not (the ~11 ms inter-box store RTT × ~4–5 RT/msg) and works on the **default async path**. Microbench (adversarially re-reviewed, counts VERIFIED honest): per-hop drop 27–50%, but the **≥40% figure is CONDITIONAL on the applock-rc-fold** — under the strict interpretation 27–33% clears nothing, so the microbench JUSTIFIES a live-rig e2e A/B, it does not substitute for it. Content-vs-infra error attribution + a golden-SQL/living RT-count CI gate are load-bearing. **Promoted default-ON 2026-07-08** (Bench B distance-insurance A/B: harmless-near + helps-far, green SS correctness precondition) — flag retained only as an emergency off-switch | Accepted (2026-07-07) — promoted default-ON 2026-07-08 | -| [0076](0076-typed-action-vocabulary-action-list-lens.md) | Typed action vocabulary + structured action-list lens over Python Handlers (BACKLOG #222, under the #26 amendment) — phase 1: `messagefoundry/actions.py`, pure typed helpers mirroring Corepoint's action classes over the existing `Message` API (control flow stays native Python; no flow wrappers); phase 2: static-only `lens parse --json` (stdlib `ast`, never imports/executes config) + a VS Code `CustomTextEditorProvider` rendering any *parseable* Handler as a Corepoint-style action-list view — typed rows for the bounded structural grammar, in-place read-only `code` rows for anything else (coverage invariant: rows exactly partition the def body — never drop/reorder/synthesize) and whole-file refusal only on parse failure; phase 3 (bake + owner-go gated): row-scoped line-splice rewrites (byte-stable outside the edited row). The `.py` stays the **only artifact and execution path** — no interpreter, no declarative artifact, no canvas; InterSystems guardrails adopted (sync-on-save, one-editor-at-a-time, degrade-to-text-editor); live values reuse the ADR 0072 stream + `--show-phi` gate unchanged; stdlib-only, no new runtime dep (libcst deferred) | Accepted (2026-07-10) — **Amendment A ACCEPTED 2026-07-30 (owner-ratified, in force):** a `note` row kind so comment-only rows stop projecting as opaque `code`, **superseding ADR 0106 §5 (L)**, and reconciling the §3 enum to the kinds the parser already emits; build = BACKLOG #248. **Amendment B ⛔ DECLINED 2026-07-30 (owner ruling — too risky):** ADR 0089 Phase D "helper descent" is **not adopted and not to be built** — duplicate-call-site aliasing is unsolved in any ADR, and the yield is unmeasured and possibly negative; spec retained for auditability, reopening needs a new amendment. Better lever, not declined: recognize `msg["X"] = v` | +| [0076](0076-typed-action-vocabulary-action-list-lens.md) | Typed action vocabulary + structured action-list lens over Python Handlers (BACKLOG #222, under the #26 amendment) — phase 1: `messagefoundry/actions.py`, pure typed helpers mirroring Corepoint's action classes over the existing `Message` API (control flow stays native Python; no flow wrappers); phase 2: static-only `lens parse --json` (stdlib `ast`, never imports/executes config) + a VS Code `CustomTextEditorProvider` rendering any *parseable* Handler as a Corepoint-style action-list view — typed rows for the bounded structural grammar, in-place read-only `code` rows for anything else (coverage invariant: rows exactly partition the def body — never drop/reorder/synthesize) and whole-file refusal only on parse failure; phase 3 (bake + owner-go gated): row-scoped line-splice rewrites (byte-stable outside the edited row). The `.py` stays the **only artifact and execution path** — no interpreter, no declarative artifact, no canvas; InterSystems guardrails adopted (sync-on-save, one-editor-at-a-time, degrade-to-text-editor); live values reuse the ADR 0072 stream + `--show-phi` gate unchanged; stdlib-only, no new runtime dep (libcst deferred) | Accepted (2026-07-10) — **Amendment A ACCEPTED 2026-07-30 (owner-ratified, in force):** a `note` row kind so comment-only rows stop projecting as opaque `code`, **superseding ADR 0106 §5 (L)**, and reconciling the §3 enum to the kinds the parser already emits; build = BACKLOG #248. **Amendment B ⛔ DECLINED 2026-07-30 (owner ruling — too risky):** ADR 0089 Phase D "helper descent" is **not adopted and not to be built** — duplicate-call-site aliasing is unsolved in any ADR, and the yield is unmeasured and possibly negative; spec retained for auditability, reopening needs a new amendment. Better lever, not declined: recognize `msg["X"] = v`. **Amendment C PROPOSED 2026-08-04 (built, awaiting owner ratification):** the update-loop guard DEFERS a save-triggered re-projection to slot release instead of dropping it (BACKLOG #234) — claims to STRENGTHEN the §5 "sync on save only" guardrail, not relax it; also corrects the false premise the save gate's own comment rested on (`lens parse` reads the LIVE buffer over stdin, not disk). Whether to relax the gate itself is #234's other half and is explicitly not decided | | [0077](0077-action-bound-step-up.md) | Action-bound step-up re-verification for durable-takeover operations (ASVS 2.2.4 / BACKLOG #187) — a fresh re-authentication bound to the *specific* privileged action, not merely to a recent login, so a hijacked live session cannot silently perform a durable takeover | Accepted | | [0078](0078-certificate-revocation-posture.md) | Certificate revocation posture (OCSP/CRL, ASVS 12.1.4, BACKLOG #201) — **enforced start-time refusal + delegated proxy**, NOT in-engine OCSP (stdlib `ssl` has no OCSP/CRL fetch; a hand-rolled responder fetch fights on-prem offline-by-default). Refines [ADR 0002](0002-phase2-transport-security-and-strong-auth.md)'s *documented* revocation residual into an **enforced** control: `serve` REFUSES to start an in-process, off-loopback `[api]` TLS bind (`tls_cert_file` set + non-loopback `host`) UNLESS revocation is *proven in front* — a declared TLS-terminating proxy (`tls_terminated_upstream` + `trusted_proxies`, which does its own OCSP-must-staple/CRL) OR the operator opt-out `MEFOR_TLS_REVOCATION_ATTESTED=1`. Secure default = refuse; the loopback default + proxy-terminated paths start **byte-identically** (the pure `config/tls_policy.py:in_process_tls_revocation_refused` predicate short-circuits). Compensating controls: the SQL-Server SChannel path already does OS-managed revocation, and `pipeline/cert_expiry.py` alerts on expiring certs (steering short-lived certs). **Amendment 2026-07-12 (BACKLOG #201 residual):** extends the SAME posture-keyed refusal to the OUTBOUND verifying-TLS connectors — pure `revocation_hop_disposition(*, is_phi, production, is_loopback_hop, proxy_proven, attested)` + `RevocationHopGuard` in `config/tls_policy.py`, wired into MLLP-over-TLS egress, the REST/SOAP/FHIR https paths (`refuse_unrevoked_verified_hop`), and the Postgres asyncpg store hop (`_refuse_store_revocation`); per-connection `tls_revocation_attested` + the blanket `MEFOR_TLS_REVOCATION_ATTESTED` env are the opt-outs. Composes with #200 (fires only on a VERIFYING hop — no double-refusal). Still out of scope: SQL-Server/SChannel (already OS-managed), DICOM-SCU/FTPS, the FhirLookup read path. Flips the ASVS 12.1.4 row from documented-residual to enforced-delegation | Accepted (2026-07-10; amended 2026-07-12) — built | | [0079](0079-kerberos-idp-session-coordination.md) | Kerberos/AD engine-session lifetime coordinated with the directory (IdP) — terminate engine sessions when the directory revokes or disables the account, rather than letting a local session outlive its AD principal. **Amendment 2026-07-21:** mechanism 1's preferred input — the Kerberos ticket `endtime` — is **unobtainable** via pyspnego 0.12.1 (no expiry on the public `ContextProxy`; `SSPIProxy.step()` discards sspilib's `AcceptContextResult.expiry`), so on the Kerberos/LDAPS path it would degrade to a second local constant dressed as directory data. ASVS 7.1.3 therefore **closed by ACCEPTANCE** (signed register row, theme 3) and this ADR's Proposed→Accepted trigger is **NOT fired**; mechanism 1 ships only where the datum genuinely exists — the federated `id_token.exp` session cap. Mechanism 2 (background re-validation loop) stays deferred. **Amendment 2026-07-22: mechanism 2 is BUILT.** The deferral's stated cost was void — the candidate set derives from the existing `list_users()` + `list_sessions()`, so **no `sessions` schema change on any backend** (provenance columns were only ever mechanism 1's need). Recorded narrowing: `require_step_up` performs **no** directory bind (it compares the stored `reauth_at`), so the step-up surface is protected by inability to REFRESH — leaving a ≤`step_up_max_age_seconds` residual — while **bulk/raw PHI reads** (`require_phi_read`, 120/min) and **connection start/stop** (`require_paced`) survived to the 12 h cap. Adds three properties the design did not name: **two-strike** before revoking (the lookup returns one `None` for disabled ∪ deleted ∪ wrong-search-base), **all-or-nothing passes** (planned in `auth/reconcile.py` before any write), and a **mass-revoke circuit breaker** — a bad search base answers "not found" for everyone, so a pass exceeding **both** `ad_session_revoke_max` (5) **and** `ad_session_revoke_max_fraction` (0.34) aborts + alerts. AND, not OR: the floor alone signs out a 5-person site, the proportion alone fires on a 3-of-3 offboarding. Group re-diff rides the pass free (demotions no longer wait for a login); channel scope deliberately excluded. Default OFF (`ad_session_recheck_seconds = 0`) | Accepted (mechanism 2 built 2026-07-22; mech 1 Kerberos path closed by acceptance, federated path shipped in ADR 0142) | diff --git a/docs/testing/master-test-plan/13-steps-editor.md b/docs/testing/master-test-plan/13-steps-editor.md index 1a20ec10..ad4025d1 100644 --- a/docs/testing/master-test-plan/13-steps-editor.md +++ b/docs/testing/master-test-plan/13-steps-editor.md @@ -49,8 +49,9 @@ | `tests/test_actions.py` (44 tests) | All 15 vocabulary verbs incl. every `ValueError` guard; ADR 0076 gate 5 purity — `actions.py` top-level imports do no I/O (`:369`), `actions.py` + `lens.py` add no runtime dependency (`:375`) | | `tests/test_diagnostics.py` (3 tests) | `log_note` redacts operands unless the dev `_reveal` flag is set; a bad template never raises; `checkpoint` logs segment ids only | | `ide/src/test/suite/steps.test.ts` (~20 cases) | Row→view-model kind/title/params mapping; `code`-row verbatim passthrough; parse-error/no-handler/null-parse → text fallback; HTML escaping of HL7-derived params; `buildLensTraceArgs` never emits `--show-phi`; `traceRowValues` redacted by default; live-value line-containment mapping; the BACKLOG #225 dirty-buffer skip incl. a pre-fix regression demo | -| `ide/src/test/suite/steps-edit.test.ts` (1,842 lines, ~95 cases) | Edit→`lens rewrite` spec mapping; editable vs read-only rows; literal-only param editing; field-picker button placement; rewrite-result parsing; one-edit-at-a-time + queue/coalesce + orphaned-queue discard; F7 `expect_src` wiring from the *projected* source; every structural op spec; toolbar Add defaults; ADR 0103 before/after + `contextMenuEnablement` matrix + the server-rendered menu template; `canDropRow`/`resolveDrop`/`insertionBarAnchor`/`walkMove`/`blockExtent`/`captureBlock`/`blockLabel` | +| `ide/src/test/suite/steps-edit.test.ts` (2,186 lines, 121 cases) | Edit→`lens rewrite` spec mapping; editable vs read-only rows; literal-only param editing; field-picker button placement; rewrite-result parsing; one-edit-at-a-time + queue/coalesce + orphaned-queue discard; F7 `expect_src` wiring from the *projected* source; every structural op spec; toolbar Add defaults; ADR 0103 before/after + `contextMenuEnablement` matrix + the server-rendered menu template; `canDropRow`/`resolveDrop`/`insertionBarAnchor`/`walkMove`/`blockExtent`/`captureBlock`/`blockLabel`; the BACKLOG #234 suppressed-save deferral (guard debt, `releaseEdit`, `drainEdits`) and the `RerenderDebouncer` one-re-projection-per-release property with a fake clock | | `ide/src/test/suite/steps-addmenu.test.ts` (295 lines, ~20 cases) | `ADD_MENU_CATALOG` spans the four ADR 0106 groups; every item op is a supported lens op; Add→Send emits `insert_send`; `ADD_MENU_BY_ID` allowlist; `STRUCTURAL_OPS` forces re-projection; `buildAddMenuRequest` mappings; ADR 0108 `isReturnRow` + `add_destination` | +| `ide/src/test/suite/steps-mirror.test.ts` (1,492 lines, 50 cases; BACKLOG #233) | The webview↔model MIRROR parity gate. Loads the real `ide/media/stepsWebview.js` under **jsdom** with a recording `acquireVsCodeApi` double and reaches its mirrors through the opt-in `window.__mfStepsTestExports` hook, then asserts each against its `stepsModel` counterpart. **Two populations, deliberately, because the nine computation mirrors split in two** (the tenth, `stepsCtxRows`, IS the serialization boundary and is pinned directly). (i) The five that take a plain ROW ARRAY (`blockExtent`, `captureBlock`+`clipLabel`, `buildDropSlots`, `walkMove`) are swept over **2,000 seeded generated row sets** (`mulberry32`, seed printed on any failure so it reproduces) against ONE loaded page — they never touch the DOM, so a document per set would buy nothing. (ii) The four that are DOM-bound (`canDrop`, `resolveDrop`, `barAnchor`, and `scopeLabel`, which reads `.textContent` off the rendered header) take `
  • ` elements and a `getBoundingClientRect`, so they run over **four hand-authored adversarial cases** (nesting 0–4, if/elif/else, nested `for`, an empty-bodied header, `raise`, code rows, the ADR 0108 scaffold pair, an appended send, a `return []` filter, a multi-line row, titles carrying `& < > " '`) × ALL ordered (drag, target) pairs × pointer fractions **0.1/0.4/0.5/0.6/0.9**. 0.4 and 0.6 are the load-bearing ones: they straddle the 1/3 and 2/3 tri-zone thresholds, and 0.1/0.5/0.9 alone cannot see a threshold moving to 1/2 (verified by falsification — every failure landed at 0.4). Covers **at least** STEPS-06 (script loads under jsdom, one `alive` ping, no error diagnostic; its second clause stays manual), STEPS-07/08 (population i), STEPS-09 (population ii), STEPS-10 (context-menu enablement read back off the real server-rendered template) and STEPS-12 (the top-level-function inventory guard). It ALSO pins the row serialization boundary itself (`renderRowHtml` → dataset → `stepsCtxRows`), the code-row drag interception, and the hook's inertness when the opt-in flag is unset. Node-side, imports no `vscode`, so it runs on **every** `ide` leg — not only the Windows Extension Host one. STEPS-11 is satisfied by the seeded-divergence falsifications recorded in the PR, not by a permanently mutated copy in the tree | | `ide/src/test/suite/hl7scope.test.ts` + `completion-scope.test.ts` | ADR 0104 AC-9 — trigger→structure resolution, Z-segment + sample union, rank-never-remove, visibly distinct scope miss; decorator/type extraction; `occurrence=`/`repetition=` kwarg context | | `.github/workflows/ci.yml` `test` job (`:41`, matrix at `:374-376`) | The full **413-case** `lens`/`actions`/`diagnostics` pytest suite (verified by collection on this checkout) runs on `ubuntu-latest`, `windows-2022`, `windows-2025` (py3.14) as a **required** check — so CRLF, BOM and Windows-path behaviour of the engine half are genuinely exercised on a required leg | | `.github/workflows/ci.yml` `ide` job (`:263-320`) | `tsc --noEmit`, esbuild bundle, `npm run test:unit` (the Steps model suites) on both legs, and `npm test` (`@vscode/test-electron`, headless VS Code) on the Windows leg | @@ -62,15 +63,15 @@ | Risk | Failure mode | Blast radius | Detected today? | Priority | |---|---|---|---|---| | Row-contract drift Python→TypeScript | `lens.py` renames a field or changes the partition; the IDE keeps parsing the *frozen* fixture snapshot, so every `ide` test stays green while the live view mis-projects rows. A mis-projected line range means a byte-stable edit splices into the **wrong statement** | Silent wrong transform → wrong clinical data on every message through that Handler | **No.** Verified on this checkout: all 7 committed fixtures are stale — `suite` is missing from all of them, `label`/`operand` from `adt.json` and `IB_RADIOLOGY_SR.json`. `suite` is load-bearing for drag/drop scoping (`stepsModel.ts:29-32`) | P0 | -| Webview mirror divergence | `ide/media/stepsWebview.js` re-implements 10 pure model functions (`blockExtent:68`, `captureBlock:83`, `buildDropSlots:100`, `walkMove:126`, `clipLabel:309` — the `blockLabel` mirror, `canDrop:384`, `scopeLabel:397`, `resolveDrop:404`, `barAnchor:433`, menu enablement ~`:590`) and is explicitly **not** unit-tested (`steps-edit.test.ts:954-956`: "verified manually"). A diverged mirror computes a wrong move/drop target; the engine then applies it byte-stably and it re-parses clean | Moving a `msg.set` out of an `if` guard, or into the wrong branch, is a semantic change the byte-stability gates structurally cannot see | **No.** ADR 0108's acceptance requires "the model and the CSP-isolated mirror in agreement" with nothing enforcing it. `buildDropSlots` is not even exported from `stepsModel.ts` (`:1672`), so no test *could* compare it today | P0 | +| Webview mirror divergence | `ide/media/stepsWebview.js` re-implements 10 pure model functions (`blockExtent:68`, `captureBlock:83`, `buildDropSlots:100`, `walkMove:126`, `clipLabel:309` — the `blockLabel` mirror, `canDrop:384`, `scopeLabel:397`, `resolveDrop:404`, `barAnchor:433`, menu enablement ~`:590`) and, until BACKLOG #233, **was** explicitly not unit-tested (`steps-edit.test.ts` said so in as many words: "verified manually"). A diverged mirror computes a wrong move/drop target; the engine then applies it byte-stably and it re-parses clean | Moving a `msg.set` out of an `if` guard, or into the wrong branch, is a semantic change the byte-stability gates structurally cannot see | **Yes, as of BACKLOG #233** — `ide/src/test/suite/steps-mirror.test.ts` loads the webview script under jsdom and asserts every mirror against its model counterpart on every `ide` leg. Both of this row's original grounds are now spent: `buildDropSlots` **is** exported from `stepsModel.ts`, and the drop/clipboard mirrors are **no longer** "verified manually" (`steps-edit.test.ts` now points at the parity suite; what genuinely stays manual is the menu's positioning/dismissal/keyboard wiring, STEPS-76). The suite found exactly **one** live divergence — the model's `canDropRow` accepted a read-only `code` row as a drop target while the webview refused it, contradicting the model's own stated contract — and it is now closed. ADR 0108's "model and mirror in agreement" acceptance line is a gate rather than a claim | P0 | | Engine change never triggers the IDE tests | The `ide` job's PR path filter is `^(ide/\|\.github/workflows/ci\.yml)` (`ci.yml:448`). A PR touching `messagefoundry/lens.py` — the exact contract the Steps view consumes — does not run it at all. And `ci-gate` deliberately does **not** `needs: ide` (`ci.yml:265`), so even a red `ide` leg cannot block a merge | The whole analyst-facing surface can regress green. (It does re-run on push-to-main, `ci.yml:410` — after the merge, when it can no longer block anything) | **No** | P0 | | Zero action rows in the tested corpus | Census on this checkout: `lens parse` over all 12 `samples/config` handlers yields **12 code rows, 12 send rows, 4 control rows and 0 action/lookup/diagnostic rows**. ADR 0076 gate 1's named corpus therefore proves nothing about the action-row, param-edit or Add-palette surface — the part an analyst actually uses. The IDE fixtures inherit the same hole | Every projection/edit path for the editable surface is only ever tested against ad-hoc inline strings written by whoever wrote the test — no shared, reviewed adversarial corpus | Partially (inline strings in `test_lens_native/palette/fanout`) — but no corpus-level partition/byte-stability/ruff/`check` sweep over action rows | P1 | | `{"expr": …}` splice writes arbitrary, unnormalized Python | `_validated_expr` (`lens.py:1869`) checks only "parses as one expression" and "is exactly one call argument". **Verified on this checkout:** `set_field(msg, "PID-3", __import__("os").popen("whoami").read())` is accepted and written into the Handler body. **Also newly verified:** an expr is spliced **verbatim**, so `foo( 1,2 )` produces output that **fails `ruff format --check`** and **fails `ruff check --select F` (F821 undefined name)** — a direct breach of ADR 0076 gate 3 ("emitted code is first-class") that no existing test covers | Handlers execute in the engine process. ADR 0144's lint runs only inside `messagefoundry check`, never on the rewrite path, and the Steps view gives no in-editor signal — while pitching a form field at an analyst who does not know Python. It also silently breaks the purity invariant the at-least-once contract depends on | **No** on all three counts | P1 | | False completeness: helper-body writes invisible and unmarked | ADR 0089 Phase D (helper descent) is unbuilt — `_msh(msg)` renders as an opaque `code` row. ADR 0104 AC-10's **"unmodeled code present"** marker does not exist: grep for `unmodeled` across `ide/`, `messagefoundry/`, `tests/` returns only the ADR and `docs/research/message-model-eval.md`; the named test is absent | An analyst edits a PID mapping in the Steps view, sees no other write to that field, saves — and a helper's later write silently overrides it. Wrong clinical data, no failing test, no operator signal | **No** | P1 | -| The provider is entirely untested | No test file references `stepsView.ts`, `StepsEditorProvider` or `registerSteps`. Untested: save-only re-projection + 250 ms debounce (`stepsView.ts:839-856`, `:89`), fallback-to-text on refusal (`:281`), the 3-second script handshake toast (`:340-349`), exec-gate degradation (`:216`), `applyUndoRedo` (`:499`), `applyPickedEdit`'s drain-not-clear rule (`:472-483`), `applyStructural`'s `clearPending`-before-`endEdit` (`:452`), `retainContextWhenHidden` + `supportsMultipleEditorsPerDocument:false` (`:1127`) | Every ADR 0076 §6 IDE guardrail lives in this one file. A regression means the view writes on a keystroke, races itself, or shows a stale projection | **No** | P1 | -| Undo/redo not asserted end to end | Each op is one `WorkspaceEdit` (`stepsView.ts:380-386`, `:433-439`), so N Steps ops should be exactly N undo steps returning the file byte-for-byte | A coalesced or partial undo leaves a half-edited Handler that still parses and still passes every byte-stability gate. The operator believes they reverted; they shipped a partial transform | **No** | P1 | -| Steps view ↔ split text editor race | The engine F7 guard (`lens.py:1533`, tested at `test_lens_rewrite_v2.py:822`) and the IDE `expect_src` wiring (`steps-edit.test.ts:515-595`) are each tested **in isolation**, never interleaved. The code comment at `stepsView.ts:355-358` records the exact defect class: if `expect_src` is ever recomputed from the same buffer sent as stdin, the guard becomes a tautology | The named ADR 0076 §6 failure mode: a stale-coordinate edit splices into the wrong statement | **No** | P1 | -| Steps-authored destination never validated against the graph | `insert_send`/`add_destination` reject only an **empty** string (`lens.py:2294` — verified: `Send("OB_TYPO_DOES_NOT_EXIST", msg)` is accepted). The IDE destination picker degrades to free text when the graph can't be read (`stepsView.ts:98-106`) | An analyst ships a fan-out leg that silently never delivers. `checks.py:_check_send_target` (`:235-262`) would flag it as *advisory, non-blocking* — and no test proves the Steps path surfaces a Problems entry | **No** | P1 | +| The provider is entirely untested | No test file references `stepsView.ts`, `StepsEditorProvider` or `registerSteps`. Untested: save-only re-projection + 250 ms debounce (`stepsView.ts:884-898`, `:91`), fallback-to-text on refusal (`:300`), the 3-second script handshake toast (`:364-373`), exec-gate degradation (`:218`), `applyUndoRedo` (`:533`), `applyPickedEdit`'s drain-not-clear rule (`:505-516`), `applyStructural`'s `clearPending`-before-`releaseEdit` (`:482`), `retainContextWhenHidden` + `supportsMultipleEditorsPerDocument:false` (`:1167`) | Every ADR 0076 §6 IDE guardrail lives in this one file. A regression means the view writes on a keystroke, races itself, or shows a stale projection | **No** | P1 | +| Undo/redo not asserted end to end | Each op is one `WorkspaceEdit` (`stepsView.ts:404-410`, `:463-469`), so N Steps ops should be exactly N undo steps returning the file byte-for-byte | A coalesced or partial undo leaves a half-edited Handler that still parses and still passes every byte-stability gate. The operator believes they reverted; they shipped a partial transform | **No** | P1 | +| Steps view ↔ split text editor race | The engine F7 guard (`lens.py:1533`, tested at `test_lens_rewrite_v2.py:822`) and the IDE `expect_src` wiring (`steps-edit.test.ts:515-595`) are each tested **in isolation**, never interleaved. The code comment at `stepsView.ts:379-382` records the exact defect class: if `expect_src` is ever recomputed from the same buffer sent as stdin, the guard becomes a tautology | The named ADR 0076 §6 failure mode: a stale-coordinate edit splices into the wrong statement | **No** | P1 | +| Steps-authored destination never validated against the graph | `insert_send`/`add_destination` reject only an **empty** string (`lens.py:2294` — verified: `Send("OB_TYPO_DOES_NOT_EXIST", msg)` is accepted). The IDE destination picker degrades to free text when the graph can't be read (`stepsView.ts:100-108`) | An analyst ships a fan-out leg that silently never delivers. `checks.py:_check_send_target` (`:235-262`) would flag it as *advisory, non-blocking* — and no test proves the Steps path surfaces a Problems entry | **No** | P1 | | Doc↔code drift on the palette | Nothing in code or CI references `docs/STEPS-PALETTE.md`. It is already wrong at line 3 ("The **Steps view** (`/ui`, …)") — grep over `messagefoundry_webconsole/` for `steps`/`lens` returns nothing, and FEATURE-COVERAGE-PLAN.md `:1517` states outright "the web console has no Steps view" | The only user-facing description of the 27-item vocabulary misdirects analysts and reviewers about what code a step actually writes | **No** | P1 | | `docs/FEATURE-MAP.md` omits the whole subsystem | §11 "Surfaces — VS Code IDE" (`:175-186`) lists no Steps view, no action vocabulary, no `lens` CLI, no #222/ADR 0076 — while BACKLOG #222 (`:6689`) marks all three phases SHIPPED | A shipped analyst-facing subsystem is invisible in the project's status source of truth, so it is never scoped into release gates, coverage audits or support policy | **No** | P1 | | Architectural guardrail is a convention, not a gate | Nothing asserts that no engine package imports `messagefoundry/lens.py`. Today it holds — grep confirms only `__main__.py:2777` and `:2801` (both lazy) plus `tests/` | A declarative logic **execution** path would begin exactly by importing the row contract into `pipeline/`. If it is ever crossed, no test fires | **No** | P1 | @@ -78,8 +79,8 @@ | ADR 0104 AC-8 field-picker round-trip unproven | `ide/src/hl7Picker.ts` (171 lines, `pickHl7Path` at `:163`) has **no test file** — grep for `hl7Picker`/`pickHl7Path` across `ide/src/test/` returns nothing. AC-8's named test does not exist | An offered path that does not round-trip byte-identically silently corrupts a field write the moment the analyst clicks it | **No** | P1 | | Gate-3 claim overstated | ADR 0076 gate 3 claims rewritten files pass `mypy --strict` and `messagefoundry check` on the samples corpus. No test runs mypy on rewritten output; the sole `check` spot-check is `test_lens_rewrite_v2.py:852` on `adt.py` only, and it relies on that file's `# type: ignore`. The ruff gates `pytest.skip` when ruff is absent (`test_lens_rewrite_v2.py:85-91`) | An emitted form that re-parses but fails strict typing (the class the bare-tuple refusal exists to catch) ships as "first-class output" with the gate asserting nothing — and the gate can silently vanish | **No** | P2 | | PHI into the general log via a form field | `log_note` redacts operands but emits the **template verbatim** (`diagnostics.py:43`); the palette inserts an empty template the analyst fills in place (`stepsModel.ts:1001`). `diagnostics._reveal` (`:33`) has no environment clamp — the module docstring calls clamping "a wiring concern for the caller" and no caller does it | PHI at DEBUG in the general application log, authored through a form field by a non-programmer, with CLAUDE.md §9 unenforced on this path | **No** | P2 | -| Live-value sample picker offers "All files" | `stepsView.ts:251` filters `{ "HL7 messages": ["hl7"], "All files": ["*"] }`; `scopeFor` then reads the pick with `fs.readFileSync` (`:193`). Redaction bounds exposure to segment ids and `buildLensTraceArgs` structurally cannot emit `--show-phi`, but no test exercises the "operator picked a non-synthetic file" path | The PHI posture rests on "the picker defaults to `messageSetsDir`". Nothing enforces or warns | **No** | P2 | -| No projection budget for large Handlers | Measured on this checkout: a 5,000-statement Handler parses in **0.081 s** and rewrites in **0.155 s** (fine) but yields **5,001 rows / 965,409 bytes** of row JSON, and `buildHandlerViewModels`/`renderHandlersHtml` render every row of every handler with no virtualization (`stepsView.ts:311`, `:339`). `ide/src/cli.ts:162`/`:194` cap child stdout at 64 MB | A large ported Handler makes the view unusable or silently truncated with no notice — on exactly the migrated estates ADR 0089 targets | **No** | P2 | +| Live-value sample picker offers "All files" | `stepsView.ts:253` filters `{ "HL7 messages": ["hl7"], "All files": ["*"] }`; `scopeFor` then reads the pick with `fs.readFileSync` (`:195`). Redaction bounds exposure to segment ids and `buildLensTraceArgs` structurally cannot emit `--show-phi`, but no test exercises the "operator picked a non-synthetic file" path | The PHI posture rests on "the picker defaults to `messageSetsDir`". Nothing enforces or warns | **No** | P2 | +| No projection budget for large Handlers | Measured on this checkout: a 5,000-statement Handler parses in **0.081 s** and rewrites in **0.155 s** (fine) but yields **5,001 rows / 965,409 bytes** of row JSON, and `buildHandlerViewModels`/`renderHandlersHtml` render every row of every handler with no virtualization (`stepsView.ts:335`, `:363`). `ide/src/cli.ts:162`/`:194` cap child stdout at 64 MB | A large ported Handler makes the view unusable or silently truncated with no notice — on exactly the migrated estates ADR 0089 targets | **No** | P2 | | No git-diff-cleanliness assertion on a working-tree file | Byte-stability is proven against in-memory oracles only; nothing performs a Steps edit on a checked-in file and asserts `git diff` is exactly the intended hunk | Diffable, reviewable config is the stated rationale for the whole #26 decline. One stray whitespace/EOL flip per edit destroys review value on every Steps-authored PR | **No** | P2 | | No VSIX build/signature/attestation/publish | grep for `vsce`/`vsix`/`marketplace` over `.github/workflows/` returns nothing; the only path is manual `npm run package` (`ide/README.md:123`, which still names `messagefoundry-0.0.1.vsix` while `ide/package.json:5` says `0.0.34`) | The Steps editor reaches users only through a hand-built, unsigned, unattested VSIX. A missing `media/` asset ships with no gate — and no provenance for a surface that writes executable Handler code | **No** | P1 | | Dead coordinate-critical near-duplicate | `stepsModel.ts:261 splitLines` (splits on `\r?\n` only) sits beside `:272 physicalLines` (the correct `\r\n\|\r\|\n` mirror of `lens._physical_lines`, `lens.py:3490`). Only `physicalLines` is used in the build path (`:482`) | A future caller reaching for the wrong one desyncs IDE line slicing from AST coordinates on a CR-only file; F7 then compares mismatched text and either refuses everything or mis-splices | **No** | P2 | @@ -99,10 +100,10 @@ | STEPS-03 | `suite` id semantics hold for every row | Functional | pytest | container-CI | n/a | T | P0 | For every row of every corpus handler: `suite` equals the enclosing block header's line number as a string (the `def` line at nesting 0); two rows share a `suite` iff they are AST siblings in the same statement list; no row omits `suite` | | STEPS-04 | A single CI job carries both Python 3.14 and Node 24 | Functional | CI-leg | container-CI | n/a | T | P0 | A named job (`steps-contract`) installs both toolchains and runs STEPS-01/02/03/05..12; it appears in `ci-gate`'s `needs:` list (`ci.yml:1386`) and is configured as a required context in branch protection | | STEPS-05 | Older/partial contract degrades safely in the view model | Negative/Security | ide-mocha | container-CI | n/a | T | P1 | With `suite`, `label`, `operand`, `literal_params`, `appended` and `scaffold` each individually absent, `buildHandlerViewModels` returns the same row count and order, throws nothing, and (for absent `suite`) `canDropRow` returns `false` for every pair — never an unscoped drop | -| STEPS-06 | `stepsWebview.js` loads under jsdom and completes its handshake | Functional | ide-mocha | container-CI | n/a | T | P0 | Loaded under jsdom with a stub `acquireVsCodeApi`, the script runs to completion with no thrown error and posts exactly one `{command:"stepsDiag", level:"ping", text:"alive"}`; a second load against the retained `window.__mfStepsVscode` does not re-acquire | -| STEPS-07 | Mirror parity — `blockExtent` / `captureBlock` / `blockLabel` | Functional | ide-mocha | container-CI | n/a | T | P0 | Over ≥2,000 generated row sets (nesting 0–4, mixed kinds, control headers, returns, scaffold rows), the jsdom-loaded webview functions return values deep-equal to `stepsModel.blockExtent` (`:1767`), `captureBlock` (`:1803`), `blockLabel` (`:1839`) for identical inputs — noting the webview's `blockLabel` mirror is named `clipLabel` (`stepsWebview.js:309`). Any divergence fails | -| STEPS-08 | Mirror parity — `buildDropSlots` / `walkMove` | Functional | ide-mocha | container-CI | n/a | T | P0 | Same corpus; webview `buildDropSlots` (`:100`) and `walkMove` (`:126`) deep-equal `stepsModel` `buildDropSlots` (`:1672`) and `walkMove` (`:1861`). Requires `buildDropSlots` to be exported from `stepsModel.ts` — the export is part of the deliverable | -| STEPS-09 | Mirror parity — `canDrop` / `scopeLabel` / `resolveDrop` / `barAnchor` | Functional | ide-mocha | container-CI | n/a | T | P0 | Same corpus, all ordered (drag, target) pairs; webview `canDrop` (`:384`), `scopeLabel` (`:397`), `resolveDrop` (`:404`), `barAnchor` (`:433`) deep-equal `canDropRow` (`:1509`), `scopeLabel` (`:1594`), `resolveDrop` (`:1531`), `insertionBarAnchor` (`:1629`) | +| STEPS-06 | `stepsWebview.js` loads under jsdom and completes its handshake | Functional | ide-mocha | container-CI | n/a | T | P0 | Loaded under jsdom with a stub `acquireVsCodeApi`, the script runs to completion with no thrown error and posts exactly one `{command:"stepsDiag", level:"ping", text:"alive"}`. **The second clause — a second load against the retained `window.__mfStepsVscode` does not re-acquire — is NOT automated and is covered by STEPS-76's checklist instead** (recorded 2026-08-04): VS Code's retain-context reload gives the script a fresh realm holding a retained `window`, which one jsdom document cannot reproduce — re-running a classic script in the SAME global throws on its own top-level `const`, testing the harness rather than the product | +| STEPS-07 | Mirror parity — `blockExtent` / `captureBlock` / `blockLabel` | Functional | ide-mocha | container-CI | n/a | T | P0 | Over ≥2,000 **seeded generated** row sets (nesting 0–4, mixed kinds, control headers with and without bodies, elif/else continuations, returns, appended sends, scaffold rows, multi-line rows), the jsdom-loaded webview functions return values deep-equal to `stepsModel.blockExtent`, `captureBlock`, `blockLabel` for identical inputs — noting the webview's `blockLabel` mirror is named `clipLabel` (`stepsWebview.js:309`). The generator is deterministic and the seed is printed with any divergence, so a failure reproduces exactly. Any divergence fails | +| STEPS-08 | Mirror parity — `buildDropSlots` / `walkMove` | Functional | ide-mocha | container-CI | n/a | T | P0 | Same ≥2,000 generated row sets; webview `buildDropSlots` (`:100`) and `walkMove` (`:126`) deep-equal `stepsModel` `buildDropSlots` and `walkMove`. Requires `buildDropSlots` to be exported from `stepsModel.ts` — the export is part of the deliverable | +| STEPS-09 | Mirror parity — `canDrop` / `scopeLabel` / `resolveDrop` / `barAnchor` | Functional | ide-mocha | container-CI | n/a | T | P0 | These four are **DOM-bound** — they take `
  • ` elements and a `getBoundingClientRect`, so they cannot be driven from a generated row array and are swept over the four hand-authored adversarial cases instead, at **all ordered (drag, target) pairs × pointer fractions 0.1/0.4/0.5/0.6/0.9** (0.4 and 0.6 straddle the 1/3 and 2/3 tri-zone thresholds; without them a threshold drift to 1/2 is invisible). Webview `canDrop` (`:384`), `scopeLabel` (`:397`), `resolveDrop` (`:404`), `barAnchor` (`:433`) deep-equal `canDropRow`, `scopeLabel`, `resolveDrop`, `insertionBarAnchor`. A per-row `getBoundingClientRect` stub is mandatory (jsdom has no layout, and the webview falls back to a constant 0.5 fraction on a zero-height box) and the suite asserts the stub is what makes the fractions discriminate | | STEPS-10 | Mirror parity — context-menu enablement | Functional | ide-mocha | container-CI | n/a | T | P0 | For every row context in the corpus, the webview's enablement computation (~`:590`) yields the same enabled/disabled set as `contextMenuEnablement` (`stepsModel.ts:1178`) | | STEPS-11 | Seeded divergence is caught | Negative/Security | ide-mocha | container-CI | n/a | T | P0 | A deliberately mutated copy of one mirrored function (e.g. `blockExtent` returning `mj-1`) makes STEPS-07..10 fail with a diff naming the function and the failing input. Demonstrated once in the test suite as a self-check, then reverted | | STEPS-12 | Mirror inventory guard | Negative/Security | ide-mocha | container-CI | n/a | T | P1 | A source scan of `ide/media/stepsWebview.js` enumerates every top-level `function (` and asserts each name is either in an explicit "not-a-mirror" allowlist (DOM/wiring helpers) or has a parity assertion in STEPS-07..10. Adding an 11th mirror without a parity test fails | @@ -129,15 +130,15 @@ | STEPS-33 | Unparseable module → whole-file refusal → text fallback | Functional | ide-mocha + ide-electron | dev-PC | n/a | T | P1 | `lens parse` on a `SyntaxError` module exits non-zero with a `{"error": …}` body; `shouldFallBackToText` (`stepsModel.ts:500`) returns `fallback:true`; opening it as Steps in a real Extension Host reopens it as the default text editor and shows the notice, leaving the file unmodified | | STEPS-34 | Read-only rows refuse every op, including the newer ones | Negative/Security | pytest | container-CI | n/a | T | P1 | For a `code` row, a `control` row, a `collector_init` scaffold row and a `return_collector` scaffold row: each of the 11 `_SUPPORTED_OPS` targeting it is refused with `LensRewriteError` and zero source change | | STEPS-35 | Re-projection happens on save only, debounced | Functional | ide-electron | dev-PC | n/a | T | P1 | Typing 20 characters into the underlying document triggers **zero** `lens parse` invocations (counted through a stubbed CLI boundary); one save triggers exactly one after ~250 ms; three saves within 250 ms coalesce to one | -| STEPS-36 | One Steps editor per document | Functional | ide-electron | dev-PC | n/a | T | P1 | `vscode.openWith` twice on the same URI yields one Steps webview (`supportsMultipleEditorsPerDocument:false`, `stepsView.ts:1127`); the second call focuses the existing one | +| STEPS-36 | One Steps editor per document | Functional | ide-electron | dev-PC | n/a | T | P1 | `vscode.openWith` twice on the same URI yields one Steps webview (`supportsMultipleEditorsPerDocument:false`, `stepsView.ts:1167`); the second call focuses the existing one | | STEPS-37 | Every write is a `WorkspaceEdit`; no out-of-band file write | Negative/Security | ide-mocha + ide-electron | container-CI | n/a | T | P1 | A source scan finds no `fs.write*`/`writeFile`/`appendFile` in `ide/src/stepsView.ts` or `ide/media/stepsWebview.js`; in the Extension Host, an unsaved Steps edit leaves the on-disk mtime and bytes unchanged while `document.isDirty` is true | | STEPS-38 | Undo/redo fidelity across a mixed op sequence | Functional | ide-electron | dev-PC | n/a | T | P1 | Apply insert → move → delete → param-edit (4 ops). Four `undo` commands return `document.getText()` **byte-identical** to the original; four `redo` return it byte-identical to the post-edit form. Not 3, not 5 | | STEPS-39 | Concurrent Steps + split text editor is refused, not mis-spliced | Functional | ide-electron | dev-PC | n/a | T | P1 | Open Steps + a split text editor on the same file; insert a line above the target row in the text view; then trigger a Steps row edit. `lens rewrite` refuses on `expect_src` mismatch, an error toast appears, the view re-projects, and the file's target statement is unchanged. Repeat with the edit *below* the target row (must still succeed) | | STEPS-40 | Live values are skipped while the buffer is dirty | PHI | ide-electron | dev-PC | n/a | T | P1 | With a dirty buffer, no `dryrun --trace` child is spawned and no row shows a live-value marker; after save, markers reattach to the correct rows (line-containment verified against the saved text) | | STEPS-41 | Untrusted workspace degrades legibly | Negative/Security | ide-electron | dev-PC | n/a | T | P1 | With workspace trust off, `isExecGated()` is true, no child process is spawned for `lens parse`/`rewrite`/`graph`/`dryrun`, and the Steps view shows an explicit "workspace not trusted" state — not a blank page, not a silent empty row list | -| STEPS-42 | Script-handshake failure surfaces | Functional | ide-electron | dev-PC | n/a | T | P2 | With the webview script deliberately blocked, the 3-second timer (`stepsView.ts:340-349`) fires exactly one error message naming "View as Code"; with the script loading normally, it never fires | +| STEPS-42 | Script-handshake failure surfaces | Functional | ide-electron | dev-PC | n/a | T | P2 | With the webview script deliberately blocked, the 3-second timer (`stepsView.ts:364-373`) fires exactly one error message naming "View as Code"; with the script loading normally, it never fires | | STEPS-43 | Closing and reopening re-derives rows solely from the `.py` | Negative/Security | ide-electron | dev-PC | n/a | T | P1 | Close the Steps editor, mutate the `.py` on disk out of band, reopen as Steps: the projection reflects the on-disk file exactly. No `globalState`/`workspaceState` key holds rows (asserted by enumerating both stores) | -| STEPS-44 | A raced typed edit is drained, never dropped | Functional | ide-mocha | container-CI | n/a | T | P1 | With a field-picker edit in flight, a queued typed edit is applied after it settles (`applyPickedEdit` calls `takePending`, not `clearPending` — `stepsView.ts:472-483`); with a structural op in flight, the pending queue is cleared before `endEdit` (`:452`) and nothing drains in between | +| STEPS-44 | A raced typed edit is drained, never dropped | Functional | ide-mocha | container-CI | n/a | T | P1 | With a field-picker edit in flight, a queued typed edit is applied after it settles (`applyPickedEdit` calls `takePending`, not `clearPending` — `stepsView.ts:505-516`); with a structural op in flight, the pending queue is cleared before the slot is released (`guard.clearPending()` at `:482`, then `releaseEdit`) and nothing drains in between | | STEPS-45 | A Steps-authored bad destination reaches the Problems panel | Functional | pytest + ide-electron | dev-PC | n/a | T | P1 | pytest: `messagefoundry validate --json` over a config dir whose Handler sends to `OB_TYPO_DOES_NOT_EXIST` reports a dangling literal target (`checks.py:226 _check_send_target`). ide-electron: after Add→Send with a typed free-text destination and a save, a Problems diagnostic naming the unknown outbound appears within one validate cycle | | STEPS-46 | Steps is opt-in, never the default `.py` editor | Negative/Security | ide-mocha + ide-electron | dev-PC | n/a | T | P1 | `ide/package.json:549-556` declares `messagefoundry.stepsEditor` with `"priority": "option"` (asserted as a manifest test); opening a Handler `.py` normally yields the text editor; the "View as Steps" CodeLens appears only on a `@handler` in a config file; "Reopen With → Python" is always reachable from a Steps editor | | STEPS-47 | Import-graph guardrail: `lens.py` is CLI-only | Negative/Security | pytest | container-CI | n/a | T | P1 | An AST scan of `messagefoundry/**/*.py` finds `messagefoundry.lens` imported **only** from `messagefoundry/__main__.py` (lazily, inside `_lens_parse`/`_lens_rewrite`). Any import from `pipeline/`, `store/`, `transports/`, `config/`, `api/` or `messagefoundry/__init__.py` fails the test with an explicit "#26 carve-out breach" message | @@ -146,7 +147,7 @@ | STEPS-50 | The Steps view is never the artifact of record | Negative/Security | ide-mocha | container-CI | n/a | T | P1 | A source assertion over `ide/src/stepsView.ts` + `ide/media/stepsWebview.js`: no `fs` write API, and every `vscode.setState`/`getState` payload is confined to a declared shape of `{clipboard, selection}` (no `rows`, no `handlers`, no projection). A new persisted field fails the test | | STEPS-51 | The palette contains nothing non-executable (BACKLOG #231 line) | Negative/Security | ide-mocha + pytest | container-CI | n/a | T | P1 | Every `ADD_MENU_CATALOG` item's `op` ∈ `lens._SUPPORTED_OPS` (asserted against the engine's real set, exported to a manifest — not the hardcoded copy in `steps-addmenu.test.ts`), **and** every item's generated form re-parses to a non-`code`, non-decorative row. A decorative/grouping item with no executable projection fails | | STEPS-52 | Routers have no Steps view, at the provider level too | Negative/Security | pytest + ide-electron | dev-PC | n/a | T | P1 | pytest (extends `test_lens_parse.py:435`): a module containing only `@router` defs parses to `[]` handlers. ide-electron: opening that module as Steps falls back to text with the "no handler" notice; no router row, no router palette | -| STEPS-53 | No declarative field-mapping surface | Negative/Security | ide-mocha | container-CI | n/a | T | P1 | No catalog item, message command or persisted structure represents a source→destination mapping table; the HL7 field picker's only effect is to splice a **path string argument** into an existing call (`stepsView.ts:619-630` → `applyPickedEdit` → `set_params`). A commit adding a mapping-table message type or artifact fails the message-command allowlist assertion | +| STEPS-53 | No declarative field-mapping surface | Negative/Security | ide-mocha | container-CI | n/a | T | P1 | No catalog item, message command or persisted structure represents a source→destination mapping table; the HL7 field picker's only effect is to splice a **path string argument** into an existing call (`stepsView.ts:654-665` → `applyPickedEdit` → `set_params`). A commit adding a mapping-table message type or artifact fails the message-command allowlist assertion | | STEPS-54 | `docs/STEPS-PALETTE.md` ↔ `ADD_MENU_CATALOG` are 1:1 | Functional | ide-mocha | container-CI | n/a | T | P1 | A node test parses the four markdown tables and asserts: 27 rows total, group counts 14/3/8/2 matching the headings, and every row's **Item** label maps to exactly one catalog id with the same `group`. An added/renamed/removed catalog item fails | | STEPS-55 | Every documented "Generates" form round-trips | Functional | pytest | container-CI | n/a | T | P1 | For each of the 27 documented generated forms, inserting that literal source into a handler and re-parsing yields the row kind/action the doc implies (e.g. `msg.add_repetition("", "")` → `action`/`add_repetition`). A doc form the lens does not recognise fails | | STEPS-56 | `STEPS-PALETTE.md` names the correct surface | Functional | ide-mocha | container-CI | n/a | T | P1 | Line 3 no longer claims the Steps view is at `/ui`; a doc assertion requires the phrase identifying it as the VS Code custom editor `messagefoundry.stepsEditor`. Corroborated by: grep for `steps`/`lens` over `messagefoundry_webconsole/` returns nothing, and FEATURE-COVERAGE-PLAN.md `:1517` states "the web console has no Steps view" | @@ -155,7 +156,7 @@ | STEPS-59 | `diagnostics._reveal` is environment-clamped | PHI | pytest | container-CI | n/a | T | P2 | With `active_environment` resolving to `staging` or `prod`, enabling `diagnostics._reveal` either raises or is a no-op (operands stay `TRACE_REDACTED`). Today `diagnostics.py:33` documents clamping as "a wiring concern for the caller" and no caller does it | | STEPS-60 | Live-value trace never carries `--show-phi`, in the real argv | PHI | ide-electron | dev-PC | n/a | T | P1 | The argv of the spawned child in a real Extension Host run contains `dryrun`, `--trace`, `json` and never `--show-phi` (complements the pure assertion in `steps.test.ts` on `buildLensTraceArgs`, `stepsModel.ts:674`); every rendered live value is the redacted placeholder | | STEPS-61 | Live-value output is never persisted | PHI | ide-electron | dev-PC | n/a | T | P1 | After a live-value run, no new file appears under the workspace, the extension storage path, or the global storage path; `globalState`/`workspaceState` gain no key containing trace data. Verified by directory + state snapshot diff | -| STEPS-62 | A sample picked outside `messageSetsDir` still leaks nothing | PHI | ide-electron | dev-PC | n/a | T | P2 | With the "All files" filter used to pick a **synthetic** `.hl7` outside `messageSetsDir`, `scopeFor` (`stepsView.ts:185-204`) contributes only segment ids to the picker scope — no field value reaches the scope, the rows, the HTML, or any persisted state (asserted by scanning the rendered HTML for every field value in the file) | +| STEPS-62 | A sample picked outside `messageSetsDir` still leaks nothing | PHI | ide-electron | dev-PC | n/a | T | P2 | With the "All files" filter used to pick a **synthetic** `.hl7` outside `messageSetsDir`, `scopeFor` (`stepsView.ts:187-206`) contributes only segment ids to the picker scope — no field value reaches the scope, the rows, the HTML, or any persisted state (asserted by scanning the rendered HTML for every field value in the file) | | STEPS-63 | Steps test evidence carries no message bodies | PHI | CI-leg | container-CI | n/a | T | P1 | The Steps legs never redirect `lens parse`/`lens rewrite`/`dryrun`/`generate` stdout into a CI log line, a job summary, or an uploaded artifact; all corpus data is synthetic and committed as such. A grep of the job log for `PID\|` / `MSH\|` finds no message body | | STEPS-64 | Projection performance budget on a large Handler | Performance | pytest | container-CI | n/a | T | P2 | A generated 5,000-statement Handler: `lens parse` ≤ 0.5 s and `lens rewrite` ≤ 1.0 s wall clock on the CI runner (measured baseline on this checkout: 0.081 s / 0.155 s), and the emitted JSON is ≤ 2 MB (measured baseline: 965,409 bytes for 5,001 rows) | | STEPS-65 | Large projections are capped or virtualized with an explicit notice | Performance | ide-mocha | container-CI | n/a | T | P2 | Above a declared row threshold, `renderHandlersHtml` either virtualizes or emits an explicit "too large to project — open as code" notice; the rendered HTML for a 5,001-row contract is bounded and the notice text is present. Silent truncation fails | @@ -169,11 +170,11 @@ | STEPS-73 | Field-picker splice is byte-stable and scope-correct | Functional | ide-electron | dev-PC | n/a | T | P2 | Picking a path on an `action` row produces exactly one `set_params` edit changing only that argument; a scoped pick on a handler with `accepts_types` ranks the declared type's segments first and never removes the All-segments escape | | STEPS-74 | Deep nesting and wide fan-out survive every op | Functional | pytest | container-CI | n/a | T | P2 | A corpus handler nested 6 levels deep with a 5-destination accumulator fan-out: every op at every nesting level is either applied byte-stably (verified against the independent oracle) or refused with zero change; `suite` ids remain unique and correct after each | | STEPS-75 | Analyst comprehension trial on a representative Handler | Usability | manual | dev-PC | n/a | C | P1 | A non-Python HL7 interface analyst, given an unfamiliar anonymized ported Handler, correctly states what it does and makes one correct field-mapping edit that passes `messagefoundry check` — without opening the text editor. Recorded as pass/fail with the analyst's stated confusions. **C — the outcome is recorded, not gated:** exit criterion 14 blocks on the trial being *run*, not on its result, so it cannot fail a release. It becomes a **T** row the day the owner records a pass threshold. The core product claim; no automated proxy exists | -| STEPS-76 | Drag-and-drop and context-menu behaviour in a live webview | Usability | manual | dev-PC | n/a | T | P1 | A human confirms: the insertion bar lands where the statement lands; the tri-zone control-header drop (before / into body / after block) matches the resulting code; the cross-suite scope label is correct; a drop on a read-only `code` row is refused; the context menu clamps to the viewport, flips submenus at the right edge, reveals submenus mutually exclusively, and dismisses on Escape/outside-click/scroll/resize/blur | +| STEPS-76 | Drag-and-drop and context-menu behaviour in a live webview | Usability | manual | dev-PC | n/a | T | P1 | A human confirms: the insertion bar lands where the statement lands; the tri-zone control-header drop (before / into body / after block) matches the resulting code; the cross-suite scope label is correct; a drop on a read-only `code` row is refused; the context menu clamps to the viewport, flips submenus at the right edge, reveals submenus mutually exclusively, and dismisses on Escape/outside-click/scroll/resize/blur; **and (added 2026-08-04, from STEPS-06's un-automatable second clause) that a retain-context reload of the Steps panel does not re-acquire the VS Code API — one `alive` ping, no double-acquire error, the toolbar still live** | | STEPS-77 | Visual states an eyeball must confirm | Usability | manual | dev-PC | n/a | T | P2 | Muted read-only scaffold rows (`sends = []` / `return sends`), the `[blank]` placeholder on an empty editable input, greyed ↑/↓ at suite edges, the row selection focus ring, and the redacted `▸ ⋯` live-value placeholder all render as specified | | STEPS-78 | Clean-machine VSIX install and first Steps open | Compat | manual | dev-PC | n/a | T | P2 | On a machine with no prior extension state: `code --install-extension `, open a Handler, "View as Steps" — rows render, the toolbar enables, and one edit applies. No missing-asset error in the webview console | | STEPS-79 | Coverage-lift scan on the external estate | Usability | external | dev-PC | n/a | C | P2 | Re-running ADR 0089 §5's repeatable AST scan over the external 87-file / 486-function config repository reports the recognized-statement percentage and the residual `code`-row percentage per handler. **C — it publishes a number, and no threshold exists to fail against**; it becomes a **T** row when the owner records a minimum recognized-statement percentage. Needs a corpus that is not in this repo | -| STEPS-80 | Multi-author: a `git pull` rewrites the `.py` under an open Steps editor | Functional | ide-electron | dev-PC | n/a | T | P1 | With Steps open on a corpus Handler and the buffer **clean**, a second author's commit is applied to the working tree out of band (`git pull` / `git checkout `) so statements shift and one target statement changes text. Then: (a) the view re-projects from the new on-disk text within one 250 ms debounce (`stepsView.ts:89`, `:839-856`) and no row keeps a pre-pull coordinate; (b) a row edit posted from the **pre-pull** projection is refused on `expect_src` mismatch (`lens.py:1533`) with the file byte-unchanged — never spliced into the pulled text; (c) with the buffer **dirty** at pull time, the on-disk change does not silently overwrite the projection: the view either re-projects from the buffer or shows the stale-projection notice, and no `WorkspaceEdit` is applied against stale coordinates. Complements STEPS-39 (same-machine split editor) and STEPS-43 (out-of-band change while the editor is *closed*) — neither covers a live editor under a concurrent author | +| STEPS-80 | Multi-author: a `git pull` rewrites the `.py` under an open Steps editor | Functional | ide-electron | dev-PC | n/a | T | P1 | With Steps open on a corpus Handler and the buffer **clean**, a second author's commit is applied to the working tree out of band (`git pull` / `git checkout `) so statements shift and one target statement changes text. Then: (a) the view re-projects from the new on-disk text within one 250 ms debounce (`stepsView.ts:91`, `:884-898`) and no row keeps a pre-pull coordinate; (b) a row edit posted from the **pre-pull** projection is refused on `expect_src` mismatch (`lens.py:1533`) with the file byte-unchanged — never spliced into the pulled text; (c) with the buffer **dirty** at pull time, the on-disk change does not silently overwrite the projection: the view either re-projects from the buffer or shows the stale-projection notice, and no `WorkspaceEdit` is applied against stale coordinates. Complements STEPS-39 (same-machine split editor) and STEPS-43 (out-of-band change while the editor is *closed*) — neither covers a live editor under a concurrent author | **Row count: 80 (STEPS-01 … STEPS-80). Class: T 78, C 2 (STEPS-75, STEPS-79), A 0. P0: 12 (all T). P1: 51. P2: 17.** @@ -197,20 +198,20 @@ #### S2 — STEPS-06..12: the jsdom mirror-parity suite -**Preconditions.** `jsdom` added to `ide/package.json` devDependencies and `ide/package-lock.json` re-locked (DEP-1 applies to the lockfile). `buildDropSlots` exported from `ide/src/stepsModel.ts:1672`. +**Preconditions.** `jsdom` added to `ide/package.json` devDependencies and `ide/package-lock.json` re-locked (DEP-1 applies to the lockfile). `buildDropSlots` exported from `ide/src/stepsModel.ts`. **Steps.** -1. Build a row-set generator producing `RowDropContext[]` with: nesting 0–4, kinds `action`/`lookup`/`control`/`send`/`code`/`diagnostic`, control headers with and without bodies, `appended` sends, `collector_init`/`return_collector` scaffold rows, and `suite` ids consistent with the nesting. -2. Load `ide/media/stepsWebview.js` under jsdom with `window.acquireVsCodeApi` stubbed to a recording double and `document.querySelectorAll('li.row')` backed by a synthetic DOM built from the generated rows. -3. Extract the webview's mirrored functions from the loaded script's scope (expose them behind a test-only `window.__mfStepsTestExports` hook set inside the existing IIFE — a hook, not a second implementation). -4. For each generated row set, call each mirrored function and its `stepsModel` counterpart with identical inputs and `assert.deepStrictEqual`. -5. Repeat over all ordered (drag, target) pairs for `canDrop`/`resolveDrop`/`barAnchor`. +1. Build a **seeded** row-set generator producing `RowDropContext[]` with: nesting 0–4, kinds `action`/`lookup`/`control`/`send`/`code`/`diagnostic`, control headers with and without bodies, elif/else continuations, `appended` sends, `collector_init`/`return_collector` scaffold rows, and `suite` ids consistent with the nesting. Deterministic (`mulberry32`) so the seed alone reproduces a failing set. +2. Load `ide/media/stepsWebview.js` under jsdom with `window.acquireVsCodeApi` stubbed to a recording double and `document.querySelectorAll('li.row')` backed by a synthetic DOM built from the rendered rows. **Note (recorded 2026-08-04):** only the DOM-bound mirrors need this per case. The five row-array mirrors are pure, so the ≥2,000-set sweep runs against ONE loaded page; the render → DOM → read-back boundary they skip is pinned separately by the `stepsCtxRows`-vs-view-models comparison over the hand-authored cases. +3. Extract the webview's mirrored functions from the loaded script's scope behind a test-only `window.__mfStepsTestExports` hook — a hook handing out the SAME function objects the page uses, never a second implementation. **Placement (corrected 2026-08-04):** there is no enclosing IIFE to put it in. `ide/media/stepsWebview.js` is a standalone CLASSIC script loaded via the `