From 086111db5acf0df4a8a6b2832df3c599d9d92602 Mon Sep 17 00:00:00 2001 From: aarroyo Date: Wed, 29 Jul 2026 14:25:36 -0500 Subject: [PATCH 1/6] feat(abac): the compiled policy.wasm is verified over the whole registry, never skipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GT-602 criteria 1 and 3. The compiled-bundle block of `abac-rego-parity.spec.ts` was governed by `EVOLITH_REQUIRE_OPA_WASM`: it ran in one CI job and `it.skip`ped everywhere else, because `policy.wasm` is a gitignored build output. That is the shape that produced GT-632 — a stale wasm at the pre-`src/` path on one machine, every MCP tool denied in production, and a green suite the whole time. The block now COMPILES the policy itself. Missing or older than any `.rego` it is built from -> run `.harness/scripts/compile-opa-wasm.mjs` (the same script `npm run build:policy` runs, writing the same paths the runtime loads); if that compilation cannot happen, the suite fails with the compiler's own diagnostic. There is no environment in which it passes without having evaluated a wasm, and staleness is treated as absence because a stale bundle is the actual defect. Denominators are asserted rather than assumed: the registry must hold at least the 50 tools measured today, every one of those names must be evaluated (`evaluated.length === registered.length`), and every evaluation must come back allowed (`allowed.length === registered.length`). The ABAC-03 negative control now runs first, since an `evaluate()` returning an unexpected shape would make every ALLOW assertion pass vacuously. `compile-opa-wasm.mjs` installs by rename instead of `copyFileSync`. The test compiling on demand makes concurrent read-during-write reachable: sibling jest workers read the same `policy.wasm`, and a partial read throws in `loadPolicy`, which the evaluator reports as OPA_ERROR — a denial. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci-cd.yml | 19 +- .harness/scripts/compile-opa-wasm.mjs | 14 +- .../src/mcp/abac-rego-parity.spec.ts | 239 +++++++++++------- 3 files changed, 174 insertions(+), 98 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 72e4f860a..4c37ae500 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -155,18 +155,21 @@ jobs: run: node .harness/scripts/generate-abac-tool-sets.mjs --check # GT-602: compile the rego to the `policy.wasm` the dispatcher actually loads. - # Without this the compiled-policy parity block self-skips and CI would verify - # ABAC against the rego SOURCE only — never against the artifact that decides - # at runtime, which is exactly how fifteen tools stayed FORBIDDEN in production. + # `abac-rego-parity.spec.ts` now compiles it ITSELF when it is missing or stale, + # so this step is no longer what makes the parity check run. It stays because it + # fails fast and unambiguously on a toolchain problem (no opa binary, no network) + # instead of surfacing it as one red test inside the whole jest suite. - name: Compile OPA policy bundle (policy.wasm) run: npm run build:policy + # GT-602 criteria 1 and 3 are enforced HERE. `abac-rego-parity.spec.ts` loads the + # COMPILED policy.wasm — the artifact that decides at dispatch — evaluates it over + # every tool name the DI graph registers, and requires ALLOW for an `architect` in + # `production`. A tool that exists in the TypeScript registry and not in the + # compiled policy comes back ABAC-03 and turns this step red. That block never + # skips: a missing bundle is compiled, and a bundle that cannot be compiled is a + # failure. `Test mcp-server` is a required check on `main`. - name: Run mcp-server tests with coverage - # EVOLITH_REQUIRE_OPA_WASM=1 makes the ABSENCE of the compiled bundle a - # failure rather than a skip: a test that skips where it is enforced - # verifies nothing. - env: - EVOLITH_REQUIRE_OPA_WASM: '1' run: npm run test:cov --workspace @beyondnet/evolith-mcp # GT-330: MCP server dedicated E2E (spawns the live MCP HTTP server) diff --git a/.harness/scripts/compile-opa-wasm.mjs b/.harness/scripts/compile-opa-wasm.mjs index d8192061d..4c2712188 100644 --- a/.harness/scripts/compile-opa-wasm.mjs +++ b/.harness/scripts/compile-opa-wasm.mjs @@ -1,7 +1,7 @@ #!/usr/bin/env node import { execSync } from 'child_process'; -import { existsSync, mkdirSync, writeFileSync, copyFileSync, unlinkSync } from 'fs'; +import { existsSync, mkdirSync, writeFileSync, copyFileSync, renameSync, unlinkSync } from 'fs'; import { join, dirname } from 'path'; import { platform, arch } from 'os'; import https from 'https'; @@ -134,9 +134,19 @@ async function compileWasm() { join(rootDir, 'src', 'rulesets', 'opa', 'policy.wasm'), join(rootDir, 'src', 'sdk', 'cli', 'rulesets', 'opa', 'policy.wasm'), ]; + // GT-602: install ATOMICALLY. `copyFileSync` truncates the destination and then + // streams into it, so a reader that opens `policy.wasm` mid-copy gets a partial + // wasm and `loadPolicy` throws — which the ABAC evaluator reports as OPA_ERROR, + // i.e. a DENIAL. That window is now reachable on purpose: the GT-602 parity spec + // compiles the bundle itself when it is missing or stale, while sibling jest + // workers are reading the same path. Write beside the destination and rename; + // rename(2) within a directory is atomic, so a concurrent reader sees either the + // old bundle or the new one, never half of one. for (const dest of destinations) { mkdirSync(dirname(dest), { recursive: true }); - copyFileSync(extracted, dest); + const staging = `${dest}.${process.pid}.tmp`; + copyFileSync(extracted, staging); + renameSync(staging, dest); } unlinkSync(outputPath); execSync(`rm -rf ${tmpDir}`, { cwd: rootDir }); diff --git a/src/packages/mcp-server/src/mcp/abac-rego-parity.spec.ts b/src/packages/mcp-server/src/mcp/abac-rego-parity.spec.ts index 8336af281..bb9674032 100644 --- a/src/packages/mcp-server/src/mcp/abac-rego-parity.spec.ts +++ b/src/packages/mcp-server/src/mcp/abac-rego-parity.spec.ts @@ -1,5 +1,6 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; import { Logger } from '@nestjs/common'; import { Test } from '@nestjs/testing'; import { trace } from '@opentelemetry/api'; @@ -31,8 +32,8 @@ import { createLocalSessionContext } from './mcp-auth-contexts'; * it, and a check that silently self-skips is not a check). * * The COMPILED bundle — the artifact that actually decides at runtime — is verified - * by the last describe block. That block is mandatory wherever - * `EVOLITH_REQUIRE_OPA_WASM=1` is set (CI), so its absence fails instead of skipping. + * by the last describe block, which never skips: it COMPILES the bundle itself when + * it is missing or stale, and fails when it cannot. See that block's header. */ /** Walk up from this file until the repository root (the one holding `src/rulesets`). */ @@ -145,25 +146,50 @@ describe('GT-602 — every REGISTERED tool exists in the compiled policy source' }); /** - * The compiled artifact itself. `policy.wasm` is a gitignored build output that - * only the jobs running `npm run build:policy` produce. + * The compiled artifact itself — GT-602 acceptance criteria 1 and 3. * - * A test that skips where it is ENFORCED verifies nothing, so this block is - * governed by `EVOLITH_REQUIRE_OPA_WASM`: + * ## Why this block does not skip, under any condition * - * - unset (a developer checkout that never compiled the bundle) → the block - * skips, and says so; - * - `=1` (CI — `.github/workflows/ci-cd.yml`, job `Test mcp-server`, which runs - * `npm run build:policy` immediately before the suite) → the block RUNS, and a - * missing bundle is a FAILURE, not a skip. + * `policy.wasm` is a gitignored build output. The previous shape of this block + * ran only when `EVOLITH_REQUIRE_OPA_WASM=1` and `it.skip`ped otherwise, which + * meant the ONLY artifact that decides at runtime was verified in exactly one CI + * job and in no developer checkout. That is the same shape that let GT-632 + * happen: a stale `policy.wasm` sat at the pre-`src/` path on one machine, the + * checks that could have noticed were inert everywhere else, and every MCP tool + * was denied in production while the suite was green. + * + * So the decision here is: **the test compiles the policy itself.** If the + * bundle is missing, or older than any `.rego` it is built from, this block runs + * `.harness/scripts/compile-opa-wasm.mjs` — the exact script `npm run + * build:policy` runs, writing the exact paths the runtime loads — and if that + * compilation cannot happen the block FAILS with the compiler's own diagnostic. + * There is no environment in which it passes without having evaluated a wasm. + * + * Two consequences worth stating rather than discovering: + * - the block needs the pinned `opa` binary (cached under `.harness/bin`, + * downloaded on first use). No binary and no network is a RED test, not a + * skipped one. A check nobody can satisfy is a problem to fix in the + * environment, not to paper over with a skip; + * - it rebuilds on STALENESS, not just absence, because a stale bundle is the + * failure this gap is about. `compile-opa-wasm.mjs` installs by rename, so a + * sibling jest worker reading `policy.wasm` never observes a partial file. */ describe('GT-602 — the compiled policy.wasm allows an architect in production', () => { const WASM_PATH = path.join(REPO_ROOT, 'src', 'sdk', 'cli', 'rulesets', 'opa', 'policy.wasm'); - const wasmPresent = fs.existsSync(WASM_PATH); - const required = process.env.EVOLITH_REQUIRE_OPA_WASM === '1'; - // When the caller declared the bundle required we run the assertions even if the - // file is absent, so the failure names the artifact instead of quietly skipping. - const itCompiled = wasmPresent || required ? it : it.skip; + const REGO_DIR = path.join(REPO_ROOT, 'src', 'rulesets', 'opa'); + const COMPILER = path.join(REPO_ROOT, '.harness', 'scripts', 'compile-opa-wasm.mjs'); + + /** + * DENOMINATOR FLOORS. An assertion over an empty list passes, and "no tool was + * denied" over zero tools is the disease this whole backlog is about. Both + * counts were measured on 2026-07-29 (50 registered tools, 50 explicitly + * classified names). They are floors, not fixtures: adding tools keeps them + * true, and REMOVING a tool has to be a deliberate edit here — which is the + * point, because a registry that silently collapses to nothing is precisely + * how this suite would go quiet while still reporting green. + */ + const MIN_REGISTERED_TOOLS = 50; + const MIN_CLASSIFIED_TOOLS = 50; const ARCHITECT_IN_PRODUCTION = { user: { id: 'arch-1', roles: ['architect'], tenant: 'evolith' }, @@ -171,90 +197,127 @@ describe('GT-602 — the compiled policy.wasm allows an architect in production' environment: 'production', } as const; - /** Evaluate the COMPILED bundle (not the rego source) for one tool name. */ - function violationsFor(policy: any, name: string): Array<{ id: string }> { - const resultSet = policy.evaluate({ ...ARCHITECT_IN_PRODUCTION, tool_name: name }, 'evolith/abac/violations'); - return Array.isArray(resultSet?.[0]?.result) ? resultSet[0].result : []; + /** Newest mtime across the rego sources the bundle is compiled from. */ + function newestRegoMtimeMs(): number { + const regoFiles = fs + .readdirSync(REGO_DIR) + .filter((f) => f.endsWith('.rego')) + .map((f) => path.join(REGO_DIR, f)); + if (regoFiles.length === 0) { + throw new Error(`No .rego sources under ${REGO_DIR} — the policy corpus moved.`); + } + return Math.max(...regoFiles.map((f) => fs.statSync(f).mtimeMs)); + } + + /** + * Guarantee a compiled bundle that is NOT older than its sources. Missing or + * stale → compile. Compilation failing → throw, so `beforeAll` reports the + * compiler's stderr instead of the suite quietly verifying nothing. + */ + function ensureFreshPolicy(): void { + const regoMtime = newestRegoMtimeMs(); + const fresh = fs.existsSync(WASM_PATH) && fs.statSync(WASM_PATH).mtimeMs >= regoMtime; + if (fresh) return; + + const why = fs.existsSync(WASM_PATH) ? 'STALE (older than the rego sources)' : 'MISSING'; + const res = spawnSync(process.execPath, [COMPILER], { + cwd: REPO_ROOT, + encoding: 'utf8', + timeout: 300_000, + }); + if (res.status !== 0 || !fs.existsSync(WASM_PATH)) { + throw new Error( + `The compiled ABAC policy is ${why} and could not be built.\n` + + ` compiler: node ${path.relative(REPO_ROOT, COMPILER)} (exit ${res.status})\n` + + ` expected: ${WASM_PATH}\n` + + ` ${String(res.stderr || res.stdout || '(no output)').trim().split('\n').slice(-8).join('\n ')}\n` + + 'This test never skips: the artifact that authorizes every MCP tool call in ' + + 'production must be evaluated, and an unverifiable policy is a failure, not an absence.', + ); + } } - async function loadCompiledPolicy(): Promise { - expect( - wasmPresent - ? 'present' - : `MISSING compiled policy bundle at ${WASM_PATH}. ` + - `EVOLITH_REQUIRE_OPA_WASM=1 declares it mandatory — run \`npm run build:policy\` before this suite.`, - ).toBe('present'); + let policy: any; + let registered: string[]; + + beforeAll(async () => { + ensureFreshPolicy(); // @ts-ignore: opa-wasm ships no type declarations (same as abac-evaluator.ts) const { loadPolicy } = await import('@open-policy-agent/opa-wasm'); - return loadPolicy(fs.readFileSync(WASM_PATH)); + policy = await loadPolicy(fs.readFileSync(WASM_PATH)); + + const moduleRef = await Test.createTestingModule({ imports: [AppModule] }).compile(); + registered = moduleRef.get(ToolRegistryService).listSchemas().map((s) => s.name); + }, 300_000); + + /** Evaluate the COMPILED bundle (not the rego source) for one tool name. */ + function violationsFor(name: string): Array<{ id: string }> { + const resultSet = policy.evaluate({ ...ARCHITECT_IN_PRODUCTION, tool_name: name }, 'evolith/abac/violations'); + return Array.isArray(resultSet?.[0]?.result) ? resultSet[0].result : []; } - it('is present when the caller declared it required (EVOLITH_REQUIRE_OPA_WASM=1)', () => { - if (!required) { - // Not a silent pass: state the condition under which this block is inert. - expect(required).toBe(false); - return; - } - expect(wasmPresent).toBe(true); + it('evaluated a real bundle, not a missing one, and not one older than its sources', () => { + expect(fs.existsSync(WASM_PATH)).toBe(true); + expect(fs.statSync(WASM_PATH).size).toBeGreaterThan(0); + // GT-632 in one line: the artifact that decides must post-date the rules it encodes. + expect(fs.statSync(WASM_PATH).mtimeMs).toBeGreaterThanOrEqual(newestRegoMtimeMs()); + expect(policy).toBeDefined(); }); - itCompiled( - 'returns zero violations for every classified tool', - async () => { - const policy = await loadCompiledPolicy(); - const denied: string[] = []; - for (const name of AbacEvaluator.classifiedToolNames()) { - const violations = violationsFor(policy, name); - if (violations.length > 0) { - denied.push(`${name} -> ${violations.map((v) => v.id).join('+')}`); - } - } - expect(denied).toEqual([]); - }, - 60_000, - ); - /** - * Acceptance criteria 1 and 3, against the artifact rather than the source: every - * tool the DI graph actually REGISTERS must be known to the compiled bundle and - * must ALLOW an `architect` in `production`. A tool added to the TypeScript - * registry and never added to the rego reaches this as `ABAC-03` (unknown tool). + * Negative control, asserted FIRST because everything below it depends on it. + * If `policy.evaluate` ever returned an unexpected shape, `violationsFor` would + * yield `[]` for every name and every ALLOW assertion in this block would pass + * vacuously. A name in no rego set must come back `ABAC-03` (unknown tool) — + * which is also exactly what a registry tool missing from the policy produces. */ - itCompiled( - 'allows an architect in production for every REGISTERED tool name', - async () => { - const policy = await loadCompiledPolicy(); - const moduleRef = await Test.createTestingModule({ imports: [AppModule] }).compile(); - const registered = moduleRef.get(ToolRegistryService).listSchemas().map((s) => s.name); - - expect(registered.length).toBeGreaterThan(0); - const denied = registered - .map((name) => ({ name, violations: violationsFor(policy, name) })) - .filter((row) => row.violations.length > 0) - .map((row) => `${row.name} -> ${row.violations.map((v) => v.id).join('+')}`); - - // Before the rego fix this listed the fifteen tools of GT-602 as `ABAC-03+ABAC-01`. - expect(denied).toEqual([]); - }, - 60_000, - ); + it('flags a tool absent from the compiled policy as ABAC-03', () => { + const ids = violationsFor('evolith-not-a-real-tool').map((v) => v.id); + expect(ids).toContain('ABAC-03'); + }); + + it('returns zero violations for every classified tool', () => { + const classified = AbacEvaluator.classifiedToolNames(); + // Denominator before verdict. + expect(classified.length).toBeGreaterThanOrEqual(MIN_CLASSIFIED_TOOLS); + + const evaluated = classified.map((name) => ({ name, violations: violationsFor(name) })); + expect(evaluated).toHaveLength(classified.length); + + const denied = evaluated + .filter((row) => row.violations.length > 0) + .map((row) => `${row.name} -> ${row.violations.map((v) => v.id).join('+')}`); + expect(denied).toEqual([]); + }); /** - * Negative control for the two assertions above. `policy.evaluate` returning an - * unexpected shape would make `violationsFor` yield `[]` for everything and both - * ALLOW assertions would pass vacuously — the exact failure mode this row exists - * to catch. A name in NO rego set must come back as `ABAC-03` (unknown tool), - * which is also what a registry tool missing from the policy would produce. + * Acceptance criterion 1, against the artifact rather than the source: every + * tool the DI graph actually REGISTERS is evaluated through the compiled bundle + * and must ALLOW an `architect` in `production`. A tool added to the TypeScript + * registry and never propagated into the policy arrives here as `ABAC-03`. + * + * This is also acceptance criterion 3 — the CI failure on a registry/policy + * divergence — because `.github/workflows/ci-cd.yml` (job `Test mcp-server`, a + * required check on `main`) runs this suite. */ - itCompiled( - 'flags a tool absent from the compiled policy as ABAC-03', - async () => { - const policy = await loadCompiledPolicy(); - const ids = violationsFor(policy, 'evolith-not-a-real-tool').map((v) => v.id); - expect(ids).toContain('ABAC-03'); - }, - 60_000, - ); + it('allows an architect in production for EVERY registered tool name', () => { + // Denominator, three ways: the registry is non-trivial, every name in it was + // actually evaluated, and every evaluation came back allowed. "No denials" + // over an empty registry would otherwise be indistinguishable from success. + expect(registered.length).toBeGreaterThanOrEqual(MIN_REGISTERED_TOOLS); + + const evaluated = registered.map((name) => ({ name, violations: violationsFor(name) })); + expect(evaluated).toHaveLength(registered.length); + + const denied = evaluated + .filter((row) => row.violations.length > 0) + .map((row) => `${row.name} -> ${row.violations.map((v) => v.id).join('+')}`); + // Before the rego fix this listed the fifteen tools of GT-602 as `ABAC-03+ABAC-01`. + expect(denied).toEqual([]); + + const allowed = evaluated.filter((row) => row.violations.length === 0); + expect(allowed).toHaveLength(registered.length); + }); /** * GT-602 × GT-572 — the two engines together, through real dispatch. @@ -266,7 +329,7 @@ describe('GT-602 — the compiled policy.wasm allows an architect in production' * configuration a containerised MCP server runs in — and asserts a verdict * rather than a FORBIDDEN envelope for a tool that GT-602 had denied. */ - itCompiled( + it( 'serves evolith-adr-list to the production stdio principal end to end', async () => { const previousNodeEnv = process.env.NODE_ENV; From ea5b874b8517b85bf94753307135ec65a4deed6c Mon Sep 17 00:00:00 2001 From: aarroyo Date: Wed, 29 Jul 2026 14:32:37 -0500 Subject: [PATCH 2/6] feat(mcp): enforce ADR-0093 optimistic concurrency on all 20 mutative tools (GT-606) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0093 has been Accepted since 2026-06-20 and mandated a `baseSha` parameter, HEAD verification before applying, and a CONCURRENCY_CONFLICT error contract. Grepping the MCP server for any of those returned nothing, against 20 tools declaring `mutative: true`. Two agents on one workspace produced exactly the lost update the ADR was written to prevent. The MCP server is the thing doing the mutating — the tools write to disk themselves via IFileSystem/fs-extra into a caller-supplied directory — so the check belongs here and is not ceremony. Enforced at the single dispatch, keyed on `tool.mutative`, which is the same key the HITL approval gate already turns on. The protected set is therefore the mutative set by construction: a tool registered tomorrow inherits the check with no wiring, which a per-tool implementation would have made twenty chances to forget. `baseSha` is likewise derived onto every mutative tool's advertised schema by ToolRegistryService.describe rather than hand-copied into 20 input schemas, so the declared surface and the enforced behaviour cannot drift. Semantics are If-Match, not a lock: `baseSha` is optional, and a caller that declares the state it planned against gets protection. Requiring it outright would break every existing caller and make the tools that legitimately target a not-yet-initialised directory (evolith-scaffold, evolith-init-batch) unusable; EVOLITH_MCP_REQUIRE_BASE_SHA=1 gives a deployment the strict reading. When `baseSha` IS supplied, verification fails closed — an unresolvable HEAD is not evidence that the asserted base state holds. Tests. The load-bearing case is a genuine race, not a fabricated SHA: a real git repo, a real `git rev-parse` read by the client, and a second mutative call in flight at the same time landing a real commit in the window between that read and the dispatch's verification. It asserts both the conflict envelope and that the victim tool's side-effect file does not exist — a conflict reported after the write landed would be worse than no check. A deliberately stale SHA is kept as the cheaper second case. The denominator guard enumerates the mutative set off the live DI registry and drives every member through the real dispatch, so the count is observed rather than remembered and an unprotected tool fails the build with its own name in the diff. Verified non-vacuous: with the check disabled, the race fails and all 20 tools go unprotected. ADR-0093 stays Accepted and now records what backs it, including that §2 (pessimistic locking) remains deliberately unimplemented — no current tool holds an exclusive non-git resource long enough for a two-minute lease to be the right mechanism. Recording that is the point: the ADR should not keep claiming a mechanism nothing implements. Co-Authored-By: Claude Opus 5 --- .../core/0093-mcp-concurrency-locking.es.md | 40 ++ .../adrs/core/0093-mcp-concurrency-locking.md | 38 ++ .../mcp-server/src/common/errors.spec.ts | 27 +- src/packages/mcp-server/src/common/errors.ts | 5 + .../mcp-server/src/mcp/mcp-tool-dispatch.ts | 38 ++ .../mcp/mutative-base-sha-coverage.spec.ts | 155 ++++++++ .../src/mcp/tool-registry.service.ts | 13 +- .../src/mcp/workspace-concurrency.spec.ts | 348 ++++++++++++++++++ .../src/mcp/workspace-concurrency.ts | 207 +++++++++++ 9 files changed, 866 insertions(+), 5 deletions(-) create mode 100644 src/packages/mcp-server/src/mcp/mutative-base-sha-coverage.spec.ts create mode 100644 src/packages/mcp-server/src/mcp/workspace-concurrency.spec.ts create mode 100644 src/packages/mcp-server/src/mcp/workspace-concurrency.ts diff --git a/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.es.md b/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.es.md index fcc575b90..1787c1bc4 100644 --- a/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.es.md +++ b/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.es.md @@ -94,7 +94,47 @@ Cuando una operación de escritura es rechazada debido a un conflicto de bloqueo ### Negativas - **Complejidad del agente**: Los agentes deben estar diseñados para manejar fallos de transacción, obtener el contexto actualizado y reintentar. +## Estado de Implementación + +Registrado para que el mismo grep que encuentra este ADR encuentre lo que lo respalda (GT-606). + +| Sección | Estado | Dónde | +| --- | --- | --- | +| §1 Verificación Optimista de Estado | **Implementada** | `src/packages/mcp-server/src/mcp/workspace-concurrency.ts`, aplicada en `mcp-tool-dispatch.ts` | +| §3 Contrato de Error de Concurrencia | **Implementado** | `CONCURRENCY_CONFLICT` en `src/packages/mcp-server/src/common/errors.ts`; payload del conflicto en `error.details` del sobre | +| §2 Bloqueo Pesimista de Recursos | **No implementado** | — | + +Notas sobre §1 tal como se construyó: + +- `baseSha` se deriva al esquema publicado de toda herramienta mutativa en + `ToolRegistryService.describe`, y lo verifica el dispatch único a partir de + `tool.mutative` — la misma llave que usa la compuerta HITL de aprobación. El + conjunto protegido es, por construcción, el conjunto mutativo, y una + herramienta añadida después no puede llegar desprotegida. + `mutative-base-sha-coverage.spec.ts` enumera ese conjunto desde el registro + vivo y falla si algún miembro queda sin guarda. +- `baseSha` es **opcional** por defecto, con semántica `If-Match`: quien declara + el estado sobre el que planificó queda protegido; quien no declara nada, no. + Exigirlo sin más rompería a todos los llamantes existentes y dejaría + inutilizables las herramientas que legítimamente apuntan a un directorio aún + no inicializado (`evolith-scaffold`, `evolith-init-batch`). Un despliegue que + quiera la lectura estricta define `EVOLITH_MCP_REQUIRE_BASE_SHA=1`. +- La verificación **falla cerrada**: si se envía `baseSha` y HEAD no se puede + resolver, la escritura se rechaza en lugar de dejarse pasar. +- El payload de §3 viaja en `error.details` y no en `error.meta` como se esboza + arriba, porque ADR-0073 reserva `meta` para los metadatos de ejecución del + sobre. El código, los nombres de campo y la semántica no cambian. + +§2 queda diferida, no adoptada-e-ignorada: ninguna herramienta MCP actual +retiene un recurso exclusivo no-git el tiempo suficiente para que un lease de +dos minutos sea el mecanismo correcto, y el efecto de toda herramienta mutativa +ya queda cubierto por la verificación de estado git de §1. Si aparece una +herramienta que mute un registro de base de datos o sostenga una tarea +exclusiva larga, §2 pasa a ser exigible y esta tabla es el registro de que sigue +pendiente. + ## Referencias +- [ADR-0073: Contrato Unificado de Salida de CLI](./0073-unified-cli-output-contract.es.md) - [ADR-0087: Control de Acceso ABAC para Ejecución de HerramientasMCP](./0087-abac-agentic-tool-execution.es.md) - [ADR-0089: Flujos de Trabajo Agénticos Orientados a Eventos](./0089-event-driven-agentic-workflows.es.md) diff --git a/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.md b/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.md index a406cc297..2a123343f 100644 --- a/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.md +++ b/reference/core/architecture/adrs/core/0093-mcp-concurrency-locking.md @@ -94,7 +94,45 @@ When a write operation is rejected due to a lock conflict or SHA mismatch, the M ### Negative - **Agent complexity**: Agents must be designed to handle transaction failures, fetch updated context, and retry. +## Implementation Status + +Recorded so that the grep which finds this ADR also finds what backs it (GT-606). + +| Section | Status | Where | +| --- | --- | --- | +| §1 Optimistic State Verification | **Implemented** | `src/packages/mcp-server/src/mcp/workspace-concurrency.ts`, enforced in `mcp-tool-dispatch.ts` | +| §3 Concurrency Error Contract | **Implemented** | `CONCURRENCY_CONFLICT` in `src/packages/mcp-server/src/common/errors.ts`; conflict payload in the envelope's `error.details` | +| §2 Pessimistic Resource Locking | **Not implemented** | — | + +Notes on §1 as built: + +- `baseSha` is derived onto every mutative tool's advertised schema by + `ToolRegistryService.describe`, and verified by the single dispatch keyed on + `tool.mutative` — the same key the HITL approval gate uses. The protected set + is therefore the mutative set by construction, and a tool added later cannot + arrive unprotected. `mutative-base-sha-coverage.spec.ts` enumerates that set + off the live registry and fails if any member is unguarded. +- `baseSha` is **optional** by default, giving `If-Match` semantics: a caller + that declares the state it planned against is protected; one that declares + nothing is not. Requiring it outright would break every existing caller and + would make the tools that legitimately target a not-yet-initialised directory + (`evolith-scaffold`, `evolith-init-batch`) unusable. A deployment that wants + the strict reading sets `EVOLITH_MCP_REQUIRE_BASE_SHA=1`. +- Verification **fails closed**: if `baseSha` is supplied and HEAD cannot be + resolved, the write is rejected rather than allowed through. +- The §3 payload is carried in `error.details` rather than `error.meta` as + sketched above, because ADR-0073 reserves `meta` for the envelope's execution + metadata. The code, the field names and the semantics are unchanged. + +§2 is deferred, not adopted-and-ignored: no current MCP tool holds an exclusive +non-git resource for long enough for a two-minute lease to be the right +mechanism, and every mutative tool's effect is already covered by §1's +git-state check. Should a tool appear that mutates a database record or holds a +long exclusive task, §2 becomes due and this table is the record that it is +still outstanding. + ## References +- [ADR-0073: Unified CLI Output Contract](./0073-unified-cli-output-contract.md) - [ADR-0087: ABAC for Agentic Tool Execution](./0087-abac-agentic-tool-execution.md) - [ADR-0089: Event-Driven Agentic Workflows](./0089-event-driven-agentic-workflows.md) diff --git a/src/packages/mcp-server/src/common/errors.spec.ts b/src/packages/mcp-server/src/common/errors.spec.ts index b47eb6da6..25aa9a091 100644 --- a/src/packages/mcp-server/src/common/errors.spec.ts +++ b/src/packages/mcp-server/src/common/errors.spec.ts @@ -8,10 +8,33 @@ describe('MCP errors', () => { expect(ErrorCodes.FORBIDDEN).toBe('FORBIDDEN'); expect(ErrorCodes.INTERNAL_ERROR).toBe('INTERNAL_ERROR'); expect(ErrorCodes.NOT_IMPLEMENTED).toBe('NOT_IMPLEMENTED'); + // GT-606 — core/ADR-0093 §3 conflict contract. + expect(ErrorCodes.CONCURRENCY_CONFLICT).toBe('CONCURRENCY_CONFLICT'); }); - it('has 16 error codes', () => { - expect(Object.keys(ErrorCodes)).toHaveLength(16); + // The registry is append-only: renaming or reusing a code is a breaking + // change for every consumer keying off it, so the whole set is asserted + // rather than just its size. Adding a code is a one-line addition here. + it('is the append-only registry, unchanged except by addition', () => { + expect(Object.keys(ErrorCodes)).toEqual([ + 'VALIDATION_FAILED', + 'SCHEMA_INVALID', + 'REPO_NOT_FOUND', + 'PHASE_INVALID', + 'RULESET_NOT_FOUND', + 'NOT_A_SATELLITE', + 'GATE_BLOCKED', + 'COMMAND_FAILED', + 'TIMEOUT', + 'IO_ERROR', + 'PATH_NOT_FOUND', + 'GIT_ERROR', + 'UNAUTHORIZED', + 'FORBIDDEN', + 'CONCURRENCY_CONFLICT', + 'INTERNAL_ERROR', + 'NOT_IMPLEMENTED', + ]); }); }); diff --git a/src/packages/mcp-server/src/common/errors.ts b/src/packages/mcp-server/src/common/errors.ts index bad97e08d..4c8415176 100644 --- a/src/packages/mcp-server/src/common/errors.ts +++ b/src/packages/mcp-server/src/common/errors.ts @@ -27,6 +27,11 @@ export const ErrorCodes = { UNAUTHORIZED: "UNAUTHORIZED", FORBIDDEN: "FORBIDDEN", + // Concurrencia (core/ADR-0093 §3) — el estado base declarado por el llamante + // ya no coincide con HEAD del workspace, o el recurso está bloqueado. La + // escritura se rechaza ANTES de aplicarse. + CONCURRENCY_CONFLICT: "CONCURRENCY_CONFLICT", + // Internos INTERNAL_ERROR: "INTERNAL_ERROR", NOT_IMPLEMENTED: "NOT_IMPLEMENTED", diff --git a/src/packages/mcp-server/src/mcp/mcp-tool-dispatch.ts b/src/packages/mcp-server/src/mcp/mcp-tool-dispatch.ts index 65e12f674..ecff98b50 100644 --- a/src/packages/mcp-server/src/mcp/mcp-tool-dispatch.ts +++ b/src/packages/mcp-server/src/mcp/mcp-tool-dispatch.ts @@ -10,6 +10,12 @@ import { failure, generateCorrelationId, success, toErrorEnvelope } from '../com import { runWithContext } from '@beyondnet/evolith-core-domain/common/request-context'; import { mcpContextStorage, McpUserContext } from './mcp-user-context'; import { describeAbacDenial } from './abac-denial-remediation'; +import { + baseShaIsRequired, + readHeadSha, + verifyBaseSha, + type HeadShaReader, +} from './workspace-concurrency'; /** Keys whose values must never reach the log sink in cleartext. */ const SENSITIVE_ARG_KEYS = new Set(['approvalToken', 'apiKey', 'api_key', 'token', 'secret', 'password', 'authorization']); @@ -56,6 +62,8 @@ export interface CallToolDeps { abacEvaluator: AbacEvaluator; logger: Logger; tracer: ReturnType; + /** GT-606 — override the HEAD reader (tests only); defaults to `git rev-parse HEAD`. */ + headShaReader?: HeadShaReader; } /** Backward-compatible aggregate. */ @@ -73,6 +81,12 @@ export class ToolDispatchService { private readonly abacEvaluator: AbacEvaluator, private readonly logger: Logger, private readonly tracer: ReturnType, + /** + * GT-606 — how the dispatch learns the workspace HEAD (ADR-0093 §1). + * Injectable purely so a test can interleave a real commit with an in-flight + * call; production always uses `git rev-parse HEAD`. + */ + private readonly headShaReader: HeadShaReader = readHeadSha, ) {} /** List tools allowed for the current user context. */ @@ -182,6 +196,29 @@ export class ToolDispatchService { Date.now() - startTime, ); } + // GT-606 / ADR-0093 §1 — Optimistic State Verification. This runs BEFORE + // `tool.execute`, so a rejected call has written nothing. It is enforced + // here, on `tool.mutative`, rather than in each tool, so that the set of + // protected tools is exactly the set of mutative tools by construction — + // a tool added later cannot arrive unprotected. + const conflict = await verifyBaseSha({ + args, + readHead: this.headShaReader, + requireBaseSha: baseShaIsRequired(), + }); + if (conflict) { + const { message, ...details } = conflict; + return errorEnvelope( + failure( + ErrorCodes.CONCURRENCY_CONFLICT, + `Mutative operation '${name}' rejected. ${message}`, + meta(Date.now() - startTime), + details, + ), + Date.now() - startTime, + ); + } + this.logger.log(JSON.stringify({ event: 'MUTATIVE_TOOL_EXECUTION', user: { id: userContext.id, roles: userContext.roles, tenant: tenant || userContext.tenant }, @@ -279,6 +316,7 @@ export async function handleCallTool( ): Promise { const service = new ToolDispatchService( deps.registry, deps.metrics, deps.abacEvaluator, deps.logger, deps.tracer, + deps.headShaReader ?? readHeadSha, ); return service.callTool(name, args); } diff --git a/src/packages/mcp-server/src/mcp/mutative-base-sha-coverage.spec.ts b/src/packages/mcp-server/src/mcp/mutative-base-sha-coverage.spec.ts new file mode 100644 index 000000000..b4408c27a --- /dev/null +++ b/src/packages/mcp-server/src/mcp/mutative-base-sha-coverage.spec.ts @@ -0,0 +1,155 @@ +import { Logger } from '@nestjs/common'; +import { Test } from '@nestjs/testing'; +import { trace } from '@opentelemetry/api'; +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import * as path from 'node:path'; +import { AppModule } from '../app.module'; +import { AbacEvaluator } from './abac-evaluator'; +import { MetricsService } from './metrics.service'; +import { ToolDispatchService } from './mcp-tool-dispatch'; +import { ToolRegistryService } from './tool-registry.service'; +import { McpTool } from './tool.interface'; +import { + BASE_SHA_ARG, + NON_WORKSPACE_DIR_ARG_KEYS, + WORKSPACE_DIR_ARG_KEYS, +} from './workspace-concurrency'; + +/** + * GT-606 — the DENOMINATOR guard for ADR-0093. + * + * ADR-0093 says "mutative tools", not "these twenty tools". So the protected set + * is never hand-listed here: it is enumerated off the live DI-registered + * registry — the same graph the server actually serves — and every member is + * driven through the real dispatch. A tool added tomorrow with `mutative:true` + * joins this suite automatically and must pass it; a tool that somehow escaped + * the central check fails the build with its own name in the diff. + * + * The counterpart guard `mutative-hitl-parity.spec.ts` (GT-475) already ensures + * the `mutative` flag itself cannot drift from the ABAC write/deploy + * classification, so "every mutative tool" is a trustworthy denominator rather + * than a flag anyone can quietly omit. + */ + +class PermissiveAbac extends AbacEvaluator { + override evaluateNative() { + return { allowed: true, violations: [] }; + } + override async evaluateOpa() { + return { allowed: true, violations: [] }; + } +} + +/** Property names that look like a filesystem location and so could be a write target. */ +const DIR_LIKE = /(^|[a-z])(path|dir|directory|root|output|workspace|cwd)$/i; + +describe('GT-606 — every mutative tool in the live registry is baseSha-protected', () => { + let registry: ToolRegistryService; + let mutativeTools: McpTool[]; + let repo: string; + + const git = (...args: string[]): string => + execFileSync('git', args, { cwd: repo, encoding: 'utf-8' }).trim(); + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ imports: [AppModule] }).compile(); + registry = moduleRef.get(ToolRegistryService, { strict: false }); + mutativeTools = registry.list().filter((t) => t.mutative === true); + + repo = mkdtempSync(path.join(tmpdir(), 'gt606-registry-')); + execFileSync('git', ['init', '--quiet', '--initial-branch=main'], { cwd: repo }); + git('config', 'user.email', 'gt606@evolith.test'); + git('config', 'user.name', 'GT-606'); + git('config', 'commit.gpgsign', 'false'); + writeFileSync(path.join(repo, 'seed.txt'), 'seed\n'); + git('add', '.'); + git('commit', '--quiet', '-m', 'initial'); + }); + + afterAll(() => { + if (repo) rmSync(repo, { recursive: true, force: true }); + }); + + it('enumerates a non-empty mutative set from the registry (the guard is not vacuous)', () => { + expect(mutativeTools.length).toBeGreaterThan(0); + // Printed so the count in the change log is observed, never asserted from + // memory. Pinning an exact number here would only force a churn edit every + // time a tool is added, which is the opposite of what this guard is for. + // eslint-disable-next-line no-console + console.log( + `[GT-606] mutative tools: ${mutativeTools.length} of ${registry.list().length} registered\n` + + mutativeTools.map((t) => ` - ${t.schema.name}`).join('\n'), + ); + }); + + it('advertises baseSha on every mutative tool and on no read-only tool', () => { + const missing = mutativeTools + .map((t) => registry.describe(t)) + .filter((s) => (s.inputSchema.properties as Record)?.[BASE_SHA_ARG]?.type !== 'string') + .map((s) => s.name); + expect(missing).toEqual([]); + + const spurious = registry + .list() + .filter((t) => t.mutative !== true) + .map((t) => registry.describe(t)) + .filter((s) => (s.inputSchema.properties as Record)?.[BASE_SHA_ARG]) + .map((s) => s.name); + expect(spurious).toEqual([]); + }); + + it('resolves the write target of every mutative tool (no tool checks the wrong repository)', () => { + // A mutative tool whose directory argument the resolver does not know would + // have its baseSha verified against the server cwd instead of the directory + // it writes into — a false green, which is worse than no check. + const known = new Set([...WORKSPACE_DIR_ARG_KEYS, ...NON_WORKSPACE_DIR_ARG_KEYS]); + const unresolvable = mutativeTools.flatMap((tool) => + Object.keys(tool.schema.inputSchema.properties ?? {}) + .filter((prop) => DIR_LIKE.test(prop) && !known.has(prop)) + .map((prop) => `${tool.schema.name}.${prop}`), + ); + expect(unresolvable).toEqual([]); + }); + + it('returns CONCURRENCY_CONFLICT from EVERY mutative tool on a stale baseSha', async () => { + // The stale SHA is verified before `execute`, so no tool body runs and + // nothing is provisioned, written or called out to. + const staleSha = '0'.repeat(40); + const dispatch = new ToolDispatchService( + registry, + new MetricsService(), + new PermissiveAbac(), + new Logger('gt606'), + trace.getTracer('gt606'), + ); + + const verdicts: Array<{ tool: string; code: unknown; conflict: unknown }> = []; + for (const tool of mutativeTools) { + const result = await dispatch.callTool(tool.schema.name, { + path: repo, + dir: repo, + satellitePath: repo, + output: repo, + baseSha: staleSha, + apply: true, + approvalToken: 'gt606-approval', + }); + const env = result.structuredContent as any; + verdicts.push({ + tool: tool.schema.name, + code: env?.success === false ? env.error.code : `UNPROTECTED(success=${env?.success})`, + conflict: env?.error?.details?.conflict_type, + }); + } + + expect(verdicts).toEqual( + mutativeTools.map((t) => ({ + tool: t.schema.name, + code: 'CONCURRENCY_CONFLICT', + conflict: 'git_sha_mismatch', + })), + ); + }, 60_000); +}); diff --git a/src/packages/mcp-server/src/mcp/tool-registry.service.ts b/src/packages/mcp-server/src/mcp/tool-registry.service.ts index 6402784db..a5d905461 100644 --- a/src/packages/mcp-server/src/mcp/tool-registry.service.ts +++ b/src/packages/mcp-server/src/mcp/tool-registry.service.ts @@ -1,6 +1,7 @@ import { Inject, Injectable } from '@nestjs/common'; import { McpTool, McpToolSchema, MCP_TOOLS } from './tool.interface'; import { buildToolOutputSchema, deriveToolAnnotations } from '../common/tool-output-schema'; +import { withBaseShaParameter } from './workspace-concurrency'; /** * In-memory registry of MCP tools, keyed by tool name. @@ -50,15 +51,21 @@ export class ToolRegistryService { * Derivation lives here, not in the fifty tool classes, so no tool can be * registered without an output contract and no tool can carry a stale copy of * the envelope shape. + * + * GT-606 — the same argument applies to ADR-0093's `baseSha`: every mutative + * tool advertises it, derived from the `mutative` flag rather than hand-copied + * into twenty input schemas, so the declared surface and what the dispatch + * actually enforces cannot drift apart. */ describe(tool: McpTool): McpToolSchema { + const base = tool.mutative ? withBaseShaParameter(tool.schema) : tool.schema; return { - ...tool.schema, - outputSchema: tool.schema.outputSchema ?? buildToolOutputSchema(tool.outputDataSchema), + ...base, + outputSchema: base.outputSchema ?? buildToolOutputSchema(tool.outputDataSchema), annotations: deriveToolAnnotations({ mutative: tool.mutative, scope: tool.scope, - annotations: tool.annotations ?? tool.schema.annotations, + annotations: tool.annotations ?? base.annotations, }), }; } diff --git a/src/packages/mcp-server/src/mcp/workspace-concurrency.spec.ts b/src/packages/mcp-server/src/mcp/workspace-concurrency.spec.ts new file mode 100644 index 000000000..3c46f61f7 --- /dev/null +++ b/src/packages/mcp-server/src/mcp/workspace-concurrency.spec.ts @@ -0,0 +1,348 @@ +import { Logger } from '@nestjs/common'; +import { trace } from '@opentelemetry/api'; +import { execFileSync } from 'node:child_process'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import * as path from 'node:path'; +import { AbacEvaluator } from './abac-evaluator'; +import { MetricsService } from './metrics.service'; +import { ToolDispatchService } from './mcp-tool-dispatch'; +import { ToolRegistryService } from './tool-registry.service'; +import { McpTool } from './tool.interface'; +import { ErrorCodes } from '../common/errors'; +import { + readHeadSha, + resolveWorkspaceDir, + shaMatches, + verifyBaseSha, + WORKSPACE_DIR_ARG_KEYS, +} from './workspace-concurrency'; + +/** + * GT-606 — ADR-0093 §1 Optimistic State Verification, exercised as a real race. + * + * The load-bearing test here is the first one. It does NOT hand the tool a + * fabricated wrong SHA: it stands up a real git repository, has the client read + * a real HEAD, then lets a SECOND mutative call — in flight at the same time — + * land a real `git commit` in the window between that read and the dispatch's + * verification. Every SHA in its assertions came out of `git rev-parse`. The + * deliberately-stale-SHA case below is a cheaper second case, not the proof. + * + * The race also asserts the write did not happen: the second tool's side-effect + * file must not exist afterwards. A conflict envelope with the mutation applied + * anyway would be worse than no check at all. + */ + +/** ABAC has its own suites; here it must simply not be the subject. */ +class PermissiveAbac extends AbacEvaluator { + override evaluateNative() { + return { allowed: true, violations: [] }; + } + override async evaluateOpa() { + return { allowed: true, violations: [] }; + } +} + +interface Deferred { + promise: Promise; + resolve: (value: T) => void; +} +function deferred(): Deferred { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +function tool(name: string, execute: McpTool['execute'], mutative = true): McpTool { + return { + schema: { name, description: 'd', inputSchema: { type: 'object', properties: {} } }, + mutative, + scope: mutative ? ('write' as const) : ('read' as const), + execute, + }; +} + +function git(repo: string, ...args: string[]): string { + return execFileSync('git', args, { cwd: repo, encoding: 'utf-8' }).trim(); +} + +function dispatchFor(tools: McpTool[], headShaReader = readHeadSha): ToolDispatchService { + return new ToolDispatchService( + new ToolRegistryService(tools), + new MetricsService(), + new PermissiveAbac(), + new Logger('gt606'), + trace.getTracer('gt606'), + headShaReader, + ); +} + +/** The ADR-0073 envelope carried by a dispatch result. */ +function envelopeOf(result: { structuredContent?: Record }): any { + return result.structuredContent; +} + +const APPROVAL = { apply: true, approvalToken: 'gt606-approval' }; + +describe('GT-606 / ADR-0093 §1 — optimistic concurrency on mutative MCP tools', () => { + const tempDirs: string[] = []; + + afterAll(() => { + for (const dir of tempDirs) rmSync(dir, { recursive: true, force: true }); + }); + + function newTempDir(prefix = 'gt606-'): string { + const dir = mkdtempSync(path.join(tmpdir(), prefix)); + tempDirs.push(dir); + return dir; + } + + function newRepo(): string { + const repo = newTempDir(); + git(repo, 'init', '--quiet', '--initial-branch=main'); + git(repo, 'config', 'user.email', 'gt606@evolith.test'); + git(repo, 'config', 'user.name', 'GT-606'); + git(repo, 'config', 'commit.gpgsign', 'false'); + writeFileSync(path.join(repo, 'seed.txt'), 'seed\n'); + git(repo, 'add', '.'); + git(repo, 'commit', '--quiet', '-m', 'initial'); + return repo; + } + + it('rejects with CONCURRENCY_CONFLICT when HEAD really moves mid-flight (genuine race)', async () => { + const repo = newRepo(); + const victimFile = path.join(repo, 'agent-b-wrote-this.txt'); + + // The client reads the real HEAD it plans against. Nothing is fabricated. + const shaA = await readHeadSha(repo); + expect(shaA).toMatch(/^[0-9a-f]{40}$/); + + const bReachedVerification = deferred(); + const aCommitted = deferred(); + + // Agent A's tool: advances HEAD with a REAL commit, but only once agent B's + // call is already inside the dispatch — so the two are genuinely concurrent. + const advanceHead = tool('evolith-test-advance-head', async () => { + await bReachedVerification.promise; + git(repo, 'commit', '--quiet', '--allow-empty', '-m', 'agent A landed first'); + const shaB = git(repo, 'rev-parse', 'HEAD'); + aCommitted.resolve(shaB); + return { committed: shaB }; + }); + + // Agent B's tool: the lost update we are trying to prevent. If the guard + // fails, this file exists at the end of the test. + let bExecuted = false; + const writeFile = tool('evolith-test-write', async () => { + bExecuted = true; + writeFileSync(victimFile, 'agent B overwrote agent A\n'); + return { written: victimFile }; + }); + + const serviceA = dispatchFor([advanceHead, writeFile]); + // Agent B's dispatch reads HEAD through the REAL reader, but parks at the + // verification point long enough for agent A's commit to land. This is the + // interleaving of the ADR's sequence diagram, made deterministic. + const serviceB = dispatchFor([advanceHead, writeFile], async (dir) => { + bReachedVerification.resolve(); + await aCommitted.promise; + return readHeadSha(dir); + }); + + const pB = serviceB.callTool('evolith-test-write', { path: repo, baseSha: shaA, ...APPROVAL }); + const pA = serviceA.callTool('evolith-test-advance-head', { path: repo, ...APPROVAL }); + + const [resB, resA] = await Promise.all([pB, pA]); + const shaB = await aCommitted.promise; + + // Agent A won the race and its commit is real. + expect(envelopeOf(resA).success).toBe(true); + expect(shaB).toMatch(/^[0-9a-f]{40}$/); + expect(shaB).not.toBe(shaA); + expect(git(repo, 'rev-parse', 'HEAD')).toBe(shaB); + + // Agent B is rejected, with the ADR-0073 envelope and the ADR-0093 code. + const env = envelopeOf(resB); + expect(resB.isError).toBe(true); + expect(env.success).toBe(false); + expect(env.error.code).toBe(ErrorCodes.CONCURRENCY_CONFLICT); + expect(env.error.code).toBe('CONCURRENCY_CONFLICT'); + expect(env.error.details).toMatchObject({ + conflict_type: 'git_sha_mismatch', + expected_sha: shaA, + actual_sha: shaB, + workspace: repo, + }); + // ADR-0073 shape: { success, error, meta } — never a bare throw, never `data`. + expect(env.meta).toMatchObject({ command: 'evolith-test-write', tool: 'evolith-test-write' }); + expect(typeof env.meta.correlationId).toBe('string'); + expect(typeof env.meta.durationMs).toBe('number'); + expect(env.meta.schemaVersion).toBe('1.0.0'); + expect(env.data).toBeUndefined(); + + // The whole point: the write never happened. + expect(bExecuted).toBe(false); + expect(existsSync(victimFile)).toBe(false); + }); + + it('rejects a deliberately stale baseSha (second, weaker case)', async () => { + const repo = newRepo(); + let executed = false; + const service = dispatchFor([ + tool('evolith-test-write', async () => { + executed = true; + return {}; + }), + ]); + + const res = await service.callTool('evolith-test-write', { + path: repo, + baseSha: '0000000000000000000000000000000000000000', + ...APPROVAL, + }); + + const env = envelopeOf(res); + expect(env.success).toBe(false); + expect(env.error.code).toBe('CONCURRENCY_CONFLICT'); + expect(env.error.details.conflict_type).toBe('git_sha_mismatch'); + expect(env.error.details.actual_sha).toBe(git(repo, 'rev-parse', 'HEAD')); + expect(executed).toBe(false); + }); + + it('lets the call through when baseSha matches HEAD (the guard is not a blanket deny)', async () => { + const repo = newRepo(); + let executed = false; + const service = dispatchFor([ + tool('evolith-test-write', async () => { + executed = true; + return { ok: true }; + }), + ]); + + const res = await service.callTool('evolith-test-write', { + path: repo, + baseSha: await readHeadSha(repo), + ...APPROVAL, + }); + + expect(envelopeOf(res).success).toBe(true); + expect(executed).toBe(true); + }); + + it('accepts an abbreviated baseSha, as ADR-0093 §3 itself illustrates', async () => { + const repo = newRepo(); + const service = dispatchFor([tool('evolith-test-write', async () => ({ ok: true }))]); + + const res = await service.callTool('evolith-test-write', { + path: repo, + baseSha: git(repo, 'rev-parse', '--short=12', 'HEAD'), + ...APPROVAL, + }); + + expect(envelopeOf(res).success).toBe(true); + }); + + it('fails closed when baseSha is supplied but HEAD cannot be resolved', async () => { + const notARepo = newTempDir('gt606-bare-'); + let executed = false; + const service = dispatchFor([ + tool('evolith-test-write', async () => { + executed = true; + return {}; + }), + ]); + + const res = await service.callTool('evolith-test-write', { + path: notARepo, + baseSha: 'f12f060ebb72', + ...APPROVAL, + }); + + const env = envelopeOf(res); + expect(env.success).toBe(false); + expect(env.error.code).toBe('CONCURRENCY_CONFLICT'); + expect(env.error.details.conflict_type).toBe('head_unresolved'); + expect(executed).toBe(false); + }); + + it('leaves read-only tools untouched (no baseSha semantics on reads)', async () => { + const repo = newRepo(); + const service = dispatchFor([tool('evolith-test-read', async () => ({ ok: true }), false)]); + + const res = await service.callTool('evolith-test-read', { + path: repo, + baseSha: '0000000000000000000000000000000000000000', + }); + + expect(envelopeOf(res).success).toBe(true); + }); + + describe('strict mode (EVOLITH_MCP_REQUIRE_BASE_SHA)', () => { + const previous = process.env.EVOLITH_MCP_REQUIRE_BASE_SHA; + afterEach(() => { + if (previous === undefined) delete process.env.EVOLITH_MCP_REQUIRE_BASE_SHA; + else process.env.EVOLITH_MCP_REQUIRE_BASE_SHA = previous; + }); + + it('rejects a mutative call that omits baseSha entirely', async () => { + process.env.EVOLITH_MCP_REQUIRE_BASE_SHA = '1'; + const repo = newRepo(); + let executed = false; + const service = dispatchFor([ + tool('evolith-test-write', async () => { + executed = true; + return {}; + }), + ]); + + const res = await service.callTool('evolith-test-write', { path: repo, ...APPROVAL }); + + const env = envelopeOf(res); + expect(env.success).toBe(false); + expect(env.error.code).toBe('CONCURRENCY_CONFLICT'); + expect(env.error.details.conflict_type).toBe('missing_base_sha'); + expect(executed).toBe(false); + }); + + it('permits an omitted baseSha by default (opt-in If-Match semantics)', async () => { + delete process.env.EVOLITH_MCP_REQUIRE_BASE_SHA; + const repo = newRepo(); + const service = dispatchFor([tool('evolith-test-write', async () => ({ ok: true }))]); + const res = await service.callTool('evolith-test-write', { path: repo, ...APPROVAL }); + expect(envelopeOf(res).success).toBe(true); + }); + }); + + describe('workspace resolution', () => { + it('honours every documented directory argument, in precedence order', () => { + expect([...WORKSPACE_DIR_ARG_KEYS]).toEqual(['satellitePath', 'path', 'dir', 'output']); + expect(resolveWorkspaceDir({ satellitePath: '/s', path: '/p', dir: '/d' })).toBe('/s'); + expect(resolveWorkspaceDir({ path: '/p', dir: '/d', output: '/o' })).toBe('/p'); + expect(resolveWorkspaceDir({ dir: '/d', output: '/o' })).toBe('/d'); + expect(resolveWorkspaceDir({ output: '/o' })).toBe('/o'); + expect(resolveWorkspaceDir({}, '/fallback')).toBe('/fallback'); + expect(resolveWorkspaceDir({ path: ' ' }, '/fallback')).toBe('/fallback'); + }); + }); + + describe('sha comparison', () => { + const full = 'f12f060ebb72a3f9256612df0102030405060708'; + it('matches a full sha or a 7+ char abbreviation, and nothing shorter', () => { + expect(shaMatches(full, full)).toBe(true); + expect(shaMatches(full.toUpperCase(), full)).toBe(true); + expect(shaMatches('f12f060', full)).toBe(true); + expect(shaMatches('f12f06', full)).toBe(false); + expect(shaMatches('a3f9256', full)).toBe(false); + }); + }); + + describe('verifyBaseSha in isolation', () => { + it('is a no-op when no baseSha is supplied and strict mode is off', async () => { + const readHead = jest.fn(); + await expect(verifyBaseSha({ args: {}, readHead })).resolves.toBeNull(); + expect(readHead).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/packages/mcp-server/src/mcp/workspace-concurrency.ts b/src/packages/mcp-server/src/mcp/workspace-concurrency.ts new file mode 100644 index 000000000..a2bc25172 --- /dev/null +++ b/src/packages/mcp-server/src/mcp/workspace-concurrency.ts @@ -0,0 +1,207 @@ +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import type { McpToolSchema } from './tool.interface'; + +const execFileAsync = promisify(execFile); + +/** + * GT-606 — Optimistic State Verification for mutative MCP tools (ADR-0093 §1). + * + * ADR-0093 is `Accepted` and mandates that a tool which mutates repository files + * declares a `baseSha` parameter, verifies it against the workspace HEAD BEFORE + * applying anything, and rejects with a `CONCURRENCY_CONFLICT` when the two have + * diverged. This module is the mechanism; the enforcement point is the single + * dispatch (`mcp-tool-dispatch.ts`), not the fifty tool classes. + * + * Enforcing centrally is deliberate and is the whole value of the change: the + * `mutative` flag is already the key the HITL approval gate turns on + * (see `mutative-hitl-parity.spec.ts`), so a tool registered tomorrow with + * `mutative:true` inherits the concurrency check with no extra wiring. A + * per-tool implementation would have produced twenty places to forget. + * + * ## Semantics: `If-Match`, not a lock + * + * `baseSha` is OPTIONAL by default. This is optimistic concurrency in the HTTP + * `If-Match` sense: the caller that states the state it planned against gets + * protection; the caller that states nothing gets none, and gets it silently, + * exactly as before this change. Making it mandatory outright would break every + * existing caller of twenty tools, and would make `evolith-scaffold` / + * `evolith-init-batch` — which legitimately target a directory that is not a git + * repository yet — unusable. A deployment that wants the strict reading sets + * `EVOLITH_MCP_REQUIRE_BASE_SHA=1` and a mutative call without `baseSha` is then + * rejected as `missing_base_sha`. + * + * ## Fail-closed + * + * If `baseSha` IS supplied and HEAD cannot be resolved (not a repository, git + * unavailable, empty repository), the call is rejected rather than allowed + * through. The caller asserted a base state; being unable to check that + * assertion is not evidence that it holds. + */ + +/** Name of the optimistic-concurrency parameter mandated by ADR-0093 §1. */ +export const BASE_SHA_ARG = 'baseSha'; + +/** + * Argument names, in precedence order, from which a tool's target workspace is + * derived. This mirrors what the tool implementations themselves do — e.g. + * `upgrade.tools.ts` reads `satellitePath || path || cwd`, `agent.tools.ts` and + * `auto-fix.tools.ts` read `dir || cwd`, `sdlc-generate.tool.ts` reads `output`. + * Kept in one place so the SHA is read from the same directory the write lands + * in; a resolver that silently fell back to the server cwd would verify the + * wrong repository and hand back a false green. + */ +export const WORKSPACE_DIR_ARG_KEYS = ['satellitePath', 'path', 'dir', 'output'] as const; + +/** + * Directory-shaped arguments of mutative tools that are deliberately NOT the + * write target, and so must not be treated as the workspace: + * - `corePath` / `core` (`evolith-upgrade-apply`): the upstream being read FROM. + * - `from` (`evolith-sdlc-generate`): the input model file, resolved against + * `output`, which is the write target. + * - `rulesetPath`: an output field echoed back, never an input. + * The enumeration guard asserts every directory-shaped property of every + * mutative tool is either resolvable or listed here, so a new tool that invents + * `repoRoot` fails the build instead of quietly checking the wrong repository. + */ +export const NON_WORKSPACE_DIR_ARG_KEYS = ['corePath', 'core', 'from', 'rulesetPath'] as const; + +/** JSON-Schema fragment advertised on every mutative tool. */ +export const BASE_SHA_SCHEMA_PROPERTY = { + type: 'string', + description: + 'ADR-0093 optimistic concurrency: the git commit SHA of the workspace the caller planned against. ' + + 'Verified against the workspace HEAD before anything is written; on divergence the call is rejected ' + + 'with CONCURRENCY_CONFLICT and nothing is applied. A 7+ character abbreviated SHA is accepted. ' + + 'Optional unless the server runs with EVOLITH_MCP_REQUIRE_BASE_SHA=1.', +} as const; + +/** Add the `baseSha` parameter to a tool schema without mutating the original. */ +export function withBaseShaParameter(schema: McpToolSchema): McpToolSchema { + if (schema.inputSchema?.properties?.[BASE_SHA_ARG]) return schema; + return { + ...schema, + inputSchema: { + ...schema.inputSchema, + properties: { + ...schema.inputSchema.properties, + [BASE_SHA_ARG]: { ...BASE_SHA_SCHEMA_PROPERTY }, + }, + }, + }; +} + +/** Resolve the directory a mutative call will write into. */ +export function resolveWorkspaceDir( + args: Record, + cwd: string = process.cwd(), +): string { + for (const key of WORKSPACE_DIR_ARG_KEYS) { + const value = args[key]; + if (typeof value === 'string' && value.trim() !== '') return value; + } + return cwd; +} + +/** Reads the HEAD commit SHA of a working directory. Injectable for tests. */ +export type HeadShaReader = (dir: string) => Promise; + +/** Default reader: `git rev-parse HEAD`, `null` when HEAD cannot be resolved. */ +export const readHeadSha: HeadShaReader = async (dir) => { + try { + const { stdout } = await execFileAsync('git', ['rev-parse', 'HEAD'], { + cwd: dir, + timeout: 5_000, + }); + const sha = stdout.trim(); + return /^[0-9a-f]{40}$/i.test(sha) ? sha.toLowerCase() : null; + } catch { + // Not a repository, git missing, or an unborn HEAD. + return null; + } +}; + +export type ConflictType = 'git_sha_mismatch' | 'head_unresolved' | 'missing_base_sha'; + +/** ADR-0093 §3 conflict payload, carried in the `details` of the error envelope. */ +export interface ConcurrencyConflict { + conflict_type: ConflictType; + expected_sha: string | null; + actual_sha: string | null; + workspace: string; + message: string; +} + +/** True when `provided` names the same commit as `actual` (abbreviations allowed). */ +export function shaMatches(provided: string, actual: string): boolean { + const a = provided.trim().toLowerCase(); + const b = actual.trim().toLowerCase(); + if (a.length < 7) return false; + return b.startsWith(a); +} + +export interface VerifyBaseShaOptions { + args: Record; + readHead?: HeadShaReader; + cwd?: string; + /** Reject a mutative call that omits `baseSha` entirely. */ + requireBaseSha?: boolean; +} + +/** True when the server is configured to demand `baseSha` on every mutative call. */ +export function baseShaIsRequired(env: NodeJS.ProcessEnv = process.env): boolean { + const raw = env.EVOLITH_MCP_REQUIRE_BASE_SHA; + return raw === '1' || raw === 'true'; +} + +/** + * Verify `baseSha` against the workspace HEAD. Returns `null` when the call may + * proceed, or the conflict to report. Never writes anything: this runs strictly + * before `tool.execute`. + */ +export async function verifyBaseSha( + options: VerifyBaseShaOptions, +): Promise { + const { args, readHead = readHeadSha, cwd, requireBaseSha = false } = options; + const workspace = resolveWorkspaceDir(args, cwd); + const raw = args[BASE_SHA_ARG]; + const provided = typeof raw === 'string' ? raw.trim() : ''; + + if (provided === '') { + if (!requireBaseSha) return null; + return { + conflict_type: 'missing_base_sha', + expected_sha: null, + actual_sha: await readHead(workspace), + workspace, + message: + 'This server requires optimistic concurrency control (EVOLITH_MCP_REQUIRE_BASE_SHA). ' + + 'Read the workspace HEAD and pass it as "baseSha".', + }; + } + + const actual = await readHead(workspace); + if (!actual) { + return { + conflict_type: 'head_unresolved', + expected_sha: provided, + actual_sha: null, + workspace, + message: + `A baseSha was supplied but HEAD could not be resolved in "${workspace}". ` + + 'The base state cannot be confirmed, so nothing was applied.', + }; + } + + if (shaMatches(provided, actual)) return null; + + return { + conflict_type: 'git_sha_mismatch', + expected_sha: provided, + actual_sha: actual, + workspace, + message: + `The workspace has drifted: expected base ${provided}, HEAD is now ${actual}. ` + + 'Nothing was applied. Re-read the workspace, re-evaluate the plan and retry.', + }; +} From 8a5c2f0f58b28c1c046bbbc729ee313e209b518f Mon Sep 17 00:00:00 2001 From: aarroyo Date: Wed, 29 Jul 2026 14:41:41 -0500 Subject: [PATCH 3/6] fix(gt-614): the pipeline the product runs stops paying for gates nobody asked for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GT-614's contract half already existed: a PipelineExecutionPlan, a reference `createKindSelectivePipeline`, and a suite proving selectivity. None of it was adopted, so the gap's own sentence was still true of the product — asking for `compliance` alone still loaded the f1..f5 gate corpus and ran every Rego rule in it, because `SatelliteEvaluationPipeline` took no plan at all. That pipeline is what `POST /api/v1/evaluate` (both branches), `evolith evaluate` and the MCP `evaluate` tool actually execute. It is now expressed as two kind-tagged stages — the SDLC phase gates (`gate`/`artifact`) and the canonical ruleset corpus (`rule`/`compliance`) — and a stage whose kinds were not requested is never entered: not its loader, not its evaluator. The plan is forwarded from the orchestrator through ValidateSatelliteUseCase on all four surfaces, so they cannot disagree about what a single-kind request costs. An unrun stage is OUT OF SCOPE, not `skipped`. `skipped` means the engine tried and the outcome is UNKNOWN: it belongs in the GT-569 denominator, becomes a coverage risk, and since GT-595 a blocking rule reported skipped FAILS the run. None of that is true of a question nobody asked. The stage therefore contributes nothing to rulesChecked/rulesSkipped/rulesErrored/rulesTotal and no gate result; it is recorded only in `coverage.outOfScopeGateIds`, so a reader can still see what the request did not buy. The pipeline now publishes that coverage quadruple (it published none before), and it keeps `checked + skipped + errored === total`. The load-bearing test is negative and about outcomes: the out-of-scope stage is one that WOULD FAIL the run. A call-count assertion would survive a refactor that re-ran the gate and discarded it; `expect(verdict.passed).toBe(true)` cannot. 8 of the 9 new tests fail against the previous code — the ninth is the baseline that proves the gate really does fail when it is in scope. Verified: npm run build (tsc -b, whole monorepo) clean; core-domain 1442, core-api 179, mcp-server 433, cli 1436 tests green; eslint boundaries clean (the new cross-layer imports are type-only). Co-Authored-By: Claude Opus 5 --- src/apps/core-api/src/app.module.ts | 5 +- .../controllers/evaluation.controller.ts | 5 +- ...lite-evaluation-pipeline.selective.spec.ts | 260 ++++++++++++++++++ .../satellite-evaluation-pipeline.service.ts | 146 ++++++++-- .../use-cases/validate-satellite.use-case.ts | 34 ++- .../mcp-server/src/tools/evaluate.tool.ts | 5 +- .../src/commands/evaluate/evaluate.command.ts | 6 +- 7 files changed, 429 insertions(+), 32 deletions(-) create mode 100644 src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.selective.spec.ts diff --git a/src/apps/core-api/src/app.module.ts b/src/apps/core-api/src/app.module.ts index f4713fdd5..37f406d70 100644 --- a/src/apps/core-api/src/app.module.ts +++ b/src/apps/core-api/src/app.module.ts @@ -154,11 +154,14 @@ import { CacheMetricsService } from './infrastructure/cache/cache-metrics.servic makeOrchestrator: EvaluationOrchestratorFactory, ) => { const pipeline: IEvaluationPipeline = { - evaluate: async (manifest) => { + // GT-614: the execution plan the orchestrator built from `ctx.kinds` is + // forwarded, so a request for one kind stops paying for every gate. + evaluate: async (manifest, plan) => { const out = await validateSatellite.execute({ satellitePath: manifest.satellitePath, corePath: manifest.corePath, manifest, + plan, }); if (!out.evaluationVerdict) { throw new Error('Evaluation pipeline produced no verdict'); diff --git a/src/apps/core-api/src/presentation/controllers/evaluation.controller.ts b/src/apps/core-api/src/presentation/controllers/evaluation.controller.ts index 1af2d6ad0..9338ec6fe 100644 --- a/src/apps/core-api/src/presentation/controllers/evaluation.controller.ts +++ b/src/apps/core-api/src/presentation/controllers/evaluation.controller.ts @@ -206,11 +206,14 @@ export class EvaluationController { // Everything else (core version, kind evaluators, canonical mapping) is the // shared factory's — there is no second envelope to keep in sync. const pipeline: IEvaluationPipeline = { - evaluate: async (manifest) => { + // GT-614: same forwarding as the canonical branch — one operation, one + // behaviour, including which stages a single-kind request pays for. + evaluate: async (manifest, plan) => { const out = await useCase.execute({ satellitePath: manifest.satellitePath, corePath: manifest.corePath, manifest, + plan, }); if (!out.evaluationVerdict) { throw new Error('Inline evaluation pipeline produced no verdict'); diff --git a/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.selective.spec.ts b/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.selective.spec.ts new file mode 100644 index 000000000..20b3c96c9 --- /dev/null +++ b/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.selective.spec.ts @@ -0,0 +1,260 @@ +/** + * GT-614 — selective execution in the pipeline the deployed Core actually runs. + * + * The contract half of GT-614 (the plan, the reference `createKindSelectivePipeline`) + * already had its own suite. It proved nothing about the product, because no surface + * had adopted it: `SatelliteEvaluationPipeline` — the pipeline behind + * `POST /api/v1/evaluate`, `evolith evaluate` and the MCP `evaluate` tool — took no + * plan at all, so asking for `compliance` alone still loaded the f1..f5 gate corpus + * and ran every Rego rule in it. That is the gap's own sentence, still true. + * + * The load-bearing assertions here are NEGATIVE and about OUTCOMES, not counts: the + * out-of-scope stage is one that WOULD FAIL the run if it executed. A call-count or + * timing assertion would keep passing under a refactor that quietly re-ran the gate + * and discarded its result afterwards; a verdict assertion cannot. + */ + +import { SatelliteEvaluationPipeline } from './satellite-evaluation-pipeline.service'; +import { createPipelineExecutionPlan } from '../../evaluation/ports/evaluation-pipeline.port'; +import { mapPipelineVerdict } from '../../evaluation/canonical-result.mapper'; +import { IFileSystem, ILogger } from '../../domain/interfaces'; + +const mockLoadGatesForPhase = jest.fn(); +jest.mock('./sdlc-data-loader.service', () => ({ + SdlcDataLoaderService: jest.fn().mockImplementation(() => ({ + loadGatesForPhase: mockLoadGatesForPhase, + loadPhase: jest.fn(), + loadAllPhases: jest.fn(), + loadAllGates: jest.fn(), + })), +})); + +const mockEvaluateAll = jest.fn(); +jest.mock('../validators/evaluators/opa-evaluator', () => ({ + OpaEvaluator: jest.fn().mockImplementation(() => ({ evaluateAll: mockEvaluateAll })), +})); + +jest.mock('./topology-catalog.service'); + +/** + * A phase gate that FAILS: its required artifact is absent (`fs.exists` → false), + * which `evaluateGate` turns into a failed, error-severity evaluation and a failed + * gate. If the stage runs, the run cannot pass — that is what makes the assertions + * below impossible to satisfy by accident. + */ +const FAILING_PHASE_GATE = { + id: 'gate-f2', + name: 'Design Baseline', + phase: 'f2', + description: 'Design frozen', + requiredArtifacts: [ + { + artifact: 'docs/adrs', + validation: 'At least one ADR must exist', + rules: ['rulesets/opa/governance.rego'], + }, + ], + blockingCriteria: [{ criterion: 'adrs missing', action: 'BLOCK' }], +}; + +/** A clean corpus run: 12 rules evaluated, nothing skipped, nothing errored. */ +const CLEAN_GENERAL_RESULT = { + status: 'passed', + rulesChecked: 12, + rulesSkipped: 0, + rulesErrored: 0, + rulesTotal: 12, + skippedRuleIds: [], + erroredRuleIds: [], + issues: [], + coreRef: { version: null, path: '/core' }, + timestamp: '2026-07-29T00:00:00.000Z', +}; + +const MANIFEST = { satellitePath: '/satellite', corePath: '/core', topology: 'modular-monolith' }; + +describe('SatelliteEvaluationPipeline · kind-selective execution (GT-614)', () => { + let fs: jest.Mocked; + let logger: jest.Mocked; + let validator: { validate: jest.Mock }; + let pipeline: SatelliteEvaluationPipeline; + + beforeEach(() => { + mockLoadGatesForPhase.mockReset().mockResolvedValue([FAILING_PHASE_GATE]); + mockEvaluateAll.mockReset().mockResolvedValue([{ rule: {} as never, result: 'passed' }]); + + fs = { + // The gate's required artifact is MISSING — the phase-gate stage fails. + exists: jest.fn().mockResolvedValue(false), + existsSync: jest.fn().mockReturnValue(false), + readFile: jest.fn(), + readdir: jest.fn(), + } as unknown as jest.Mocked; + logger = { info: jest.fn(), warn: jest.fn(), error: jest.fn() } as unknown as jest.Mocked; + validator = { validate: jest.fn().mockResolvedValue(CLEAN_GENERAL_RESULT) }; + + pipeline = new SatelliteEvaluationPipeline(fs, logger, validator as never, '/core'); + }); + + it('BASELINE: the phase gate really does fail the run when it is in scope', async () => { + const verdict = await pipeline.evaluate(MANIFEST as never); + + expect(verdict.passed).toBe(false); + expect(verdict.gates.find((g) => g.gateId === 'gate-f2')?.verdict).toBe('failed'); + }); + + it('does NOT execute the phase-gate stage for a `compliance`-only request — the gate that would FAIL never runs', async () => { + const verdict = await pipeline.evaluate( + MANIFEST as never, + createPipelineExecutionPlan(['compliance']), + ); + + // The outcome assertion: the failure of a gate nobody asked for cannot reach + // this verdict, because the gate was never evaluated. + expect(verdict.passed).toBe(true); + expect(verdict.gates.map((g) => g.gateId)).not.toContain('gate-f2'); + // …and it was not merely filtered afterwards: nothing was even loaded. + expect(mockLoadGatesForPhase).not.toHaveBeenCalled(); + expect(mockEvaluateAll).not.toHaveBeenCalled(); + // The stage that WAS requested still ran. + expect(validator.validate).toHaveBeenCalledTimes(1); + }); + + it('does NOT run the ruleset corpus for a `gate`-only request', async () => { + fs.exists.mockResolvedValue(true); // let the phase gate pass, to isolate the stage + const verdict = await pipeline.evaluate( + MANIFEST as never, + createPipelineExecutionPlan(['gate']), + ); + + expect(validator.validate).not.toHaveBeenCalled(); + expect(mockLoadGatesForPhase).toHaveBeenCalled(); + expect(verdict.coverage?.outOfScopeGateIds).toEqual(['general-rulesets']); + }); + + it('records the unrun stage as OUT OF SCOPE — neither checked, nor skipped, nor errored', async () => { + const verdict = await pipeline.evaluate( + MANIFEST as never, + createPipelineExecutionPlan(['compliance']), + ); + + expect(verdict.coverage?.outOfScopeGateIds).toEqual(['sdlc-phase-gates']); + // GT-595: `skipped` would now FAIL the run for a blocking rule, and it would be + // a lie — the engine never tried. Out of scope is its own bucket. + expect(verdict.coverage?.rulesSkipped).toBe(0); + expect(verdict.coverage?.skippedRuleIds).toEqual([]); + expect(verdict.coverage?.rulesErrored).toBe(0); + expect(verdict.coverage?.erroredRuleIds).toEqual([]); + // Nor a gate result carrying a `skipped` verdict, which the canonical mapper + // would read as Verdict.SKIP for a gate that was never considered. + expect(verdict.gates.some((g) => g.verdict === 'skipped')).toBe(false); + }); + + it('keeps the published ratio honest: the denominator counts only the in-scope stage', async () => { + const full = await pipeline.evaluate(MANIFEST as never); + const scoped = await pipeline.evaluate( + MANIFEST as never, + createPipelineExecutionPlan(['compliance']), + ); + + // Before: the phase-gate rules are in the denominator of a request that never + // asked for them. The manifest names no phase, so the pipeline walks f1..f5 — + // 5 gate evaluations here — plus the 12 corpus rules. + expect(full.summary.totalRules).toBe(17); + expect(full.coverage?.rulesChecked).toBe(17); + expect(full.coverage?.rulesTotal).toBe(17); + + // After: only the corpus the request asked for. The out-of-scope stage is not + // in the numerator OR the denominator — the ratio still describes what ran. + expect(scoped.summary.totalRules).toBe(12); + expect(scoped.coverage?.rulesChecked).toBe(12); + expect(scoped.coverage?.rulesTotal).toBe(12); + expect(scoped.coverage!.rulesChecked).toBe(scoped.coverage!.rulesTotal); + }); + + it('produces NO coverage risk for the out-of-scope stage (an unasked question is not an unanswered one)', async () => { + const verdict = await pipeline.evaluate( + MANIFEST as never, + createPipelineExecutionPlan(['compliance']), + ); + const result = mapPipelineVerdict(verdict, { + coreVersion: '1.0.5', + evaluatedAt: verdict.evaluatedAt, + coverage: verdict.coverage, + }); + + expect(result.risks).toEqual([]); + expect(result.results.compliance?.skippedChecks).toBe(0); + expect(result.results.compliance?.totalChecks).toBe(12); + }); + + it('runs both stages when no plan is supplied, and when the plan is unrestricted', async () => { + await pipeline.evaluate(MANIFEST as never); + expect(mockLoadGatesForPhase).toHaveBeenCalled(); + expect(validator.validate).toHaveBeenCalledTimes(1); + + mockLoadGatesForPhase.mockClear(); + validator.validate.mockClear(); + + // `kinds: []` is what the inline REST callers that predate the field still + // send: nothing DECLARED, not nothing wanted. + const verdict = await pipeline.evaluate(MANIFEST as never, createPipelineExecutionPlan([])); + expect(mockLoadGatesForPhase).toHaveBeenCalled(); + expect(validator.validate).toHaveBeenCalledTimes(1); + expect(verdict.coverage?.outOfScopeGateIds).toEqual([]); + expect(verdict.passed).toBe(false); // the failing gate is in scope again + }); + + it('serves `artifact` from the phase-gate stage and `rule` from the corpus stage', async () => { + fs.exists.mockResolvedValue(true); + + await pipeline.evaluate(MANIFEST as never, createPipelineExecutionPlan(['artifact'])); + expect(mockLoadGatesForPhase).toHaveBeenCalled(); + expect(validator.validate).not.toHaveBeenCalled(); + + mockLoadGatesForPhase.mockClear(); + + await pipeline.evaluate(MANIFEST as never, createPipelineExecutionPlan(['rule'])); + expect(mockLoadGatesForPhase).not.toHaveBeenCalled(); + expect(validator.validate).toHaveBeenCalledTimes(1); + }); + + it('GT-595 invariant is untouched: a blocking rule the ENGINE skipped is still reported as skipped', async () => { + // The corpus stage IS in scope here; its own skipped/errored accounting must + // travel unchanged. Out-of-scope must never be a hiding place for coverage debt. + validator.validate.mockResolvedValue({ + ...CLEAN_GENERAL_RESULT, + status: 'failed', + rulesChecked: 10, + rulesSkipped: 2, + rulesTotal: 12, + skippedRuleIds: ['BLOCKING-01', 'BLOCKING-02'], + blockingSkippedRuleIds: ['BLOCKING-01'], + issues: [ + { + ruleId: 'GOV-RULE-BLOCKING-SKIPPED', + severity: 'MUST', + category: 'governance', + title: '1 blocking rule did not run', + description: 'BLOCKING-01 is blocking and was skipped.', + blocking: true, + }, + ], + }); + + const verdict = await pipeline.evaluate( + MANIFEST as never, + createPipelineExecutionPlan(['compliance']), + ); + + expect(verdict.passed).toBe(false); + expect(verdict.gates.find((g) => g.gateId === 'general-rulesets')?.verdict).toBe('failed'); + expect(verdict.coverage?.rulesSkipped).toBe(2); + expect(verdict.coverage?.skippedRuleIds).toEqual(['BLOCKING-01', 'BLOCKING-02']); + // checked + skipped + errored === total (GT-569), with out-of-scope outside it. + expect(verdict.coverage!.rulesChecked + verdict.coverage!.rulesSkipped).toBe( + verdict.coverage!.rulesTotal, + ); + expect(verdict.coverage?.outOfScopeGateIds).toEqual(['sdlc-phase-gates']); + }); +}); diff --git a/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.service.ts b/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.service.ts index b235d4c12..4cc1d49fb 100644 --- a/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.service.ts +++ b/src/packages/core-domain/src/application/services/satellite-evaluation-pipeline.service.ts @@ -1,13 +1,53 @@ import { IFileSystem, ILogger } from '../../domain/interfaces'; -import { SatelliteManifest, EvaluationVerdict, PipelineGateResult, RuleEvaluation, EvaluationSeverity, EvaluationFacts } from '../../domain/satellite-manifest'; +import { SatelliteManifest, PipelineGateResult, RuleEvaluation, EvaluationSeverity, EvaluationFacts } from '../../domain/satellite-manifest'; import { TopologyCatalogService, TopologyManifest } from './topology-catalog.service'; import { SdlcDataLoaderService, StructuredGate } from './sdlc-data-loader.service'; -import { RulesetValidatorService } from '../validators/ruleset-validator.service'; +import { RulesetValidatorService, ValidationResult } from '../validators/ruleset-validator.service'; import { createSuccessEnvelope } from '../../domain/gate-evidence'; import { toLegacyPhaseId } from '../../domain/sdlc/phase-id'; import { OpaEvaluator } from '../validators/evaluators/opa-evaluator'; +import type { EvaluationKind } from '../../evaluation/contracts/evaluation-context'; +import type { + PipelineExecutionPlan, + PipelineRuleCoverage, + PipelineVerdict, +} from '../../evaluation/ports/evaluation-pipeline.port'; import * as path from 'path'; +/** + * GT-614 — the two units of work this pipeline is made of, each tagged with the + * evaluation kinds it answers. + * + * This is what turns `ctx.kinds` from a label read at the boundary into a decision + * about what gets executed. Asking for `compliance` alone used to pay for every + * phase gate — the whole f1..f5 gate corpus was loaded and every Rego rule in it + * run — before the answer was assembled. A stage whose kinds nobody requested is + * now never entered: not its loader, not its evaluator. + * + * The phase-gate stage is named as a STAGE rather than by its individual gate ids + * on purpose: those ids are only knowable by loading the gate definitions, which is + * precisely the work an out-of-scope request must not pay for. `general-rulesets` + * is the id of the synthetic gate the stage produces, so it needs no alias. + */ +const PHASE_GATES_STAGE = { + id: 'sdlc-phase-gates', + kinds: ['gate', 'artifact'] as readonly EvaluationKind[], +} as const; + +const GENERAL_RULESETS_STAGE = { + id: 'general-rulesets', + kinds: ['rule', 'compliance'] as readonly EvaluationKind[], +} as const; + +/** + * No plan ⇒ run everything, exactly as before this contract existed. An + * unrestricted plan (`kinds: []`, still sent by the inline REST callers that + * predate the field) answers `true` to every gate for the same reason. + */ +function stageIsInScope(kinds: readonly EvaluationKind[], plan?: PipelineExecutionPlan): boolean { + return !plan || plan.includesGate(kinds); +} + /** * End-to-end evaluation pipeline for GT-281. * @@ -19,6 +59,11 @@ import * as path from 'path'; * - SdlcDataLoaderService for GT-280 structured data * - OpaEvaluator for Rego execution * - RulesetValidatorService for general ruleset validation + * + * GT-614 — it is also the pipeline the deployed Core actually runs, so it is where + * the gap's sentence had to be closed: it now honours the + * {@link PipelineExecutionPlan} the orchestrator builds from `ctx.kinds` and skips + * the stages the request did not ask for. */ export class SatelliteEvaluationPipeline { private readonly topologyCatalog: TopologyCatalogService; @@ -36,36 +81,56 @@ export class SatelliteEvaluationPipeline { this.opaEvaluator = new OpaEvaluator(fs, logger); } - async evaluate(manifest: SatelliteManifest): Promise { + async evaluate( + manifest: SatelliteManifest, + plan?: PipelineExecutionPlan, + ): Promise { const corePath = manifest.corePath || this.discoverCorePath(manifest.satellitePath); // Step 1: Resolve topology const topology = manifest.topology || await this.resolveTopology(manifest.satellitePath, corePath); - // Step 2: Determine which phases to evaluate (GT-343: accept canonical ids, - // normalize to the legacy f1..f5 the structured gate data is keyed by). - const phaseIds = manifest.phase - ? [toLegacyPhaseId(manifest.phase) ?? manifest.phase] - : ['f1', 'f2', 'f3', 'f4', 'f5']; + // GT-614: decide what this request pays for BEFORE spending anything. Both + // decisions are taken here, together, so the two stages below are symmetrical: + // whichever one the request did not ask for is never entered. + const outOfScopeGateIds: string[] = []; + const runPhaseGates = stageIsInScope(PHASE_GATES_STAGE.kinds, plan); + const runGeneralRulesets = stageIsInScope(GENERAL_RULESETS_STAGE.kinds, plan); + if (!runPhaseGates) outOfScopeGateIds.push(PHASE_GATES_STAGE.id); + if (!runGeneralRulesets) outOfScopeGateIds.push(GENERAL_RULESETS_STAGE.id); - // Step 3: Load gates from GT-280 structured data const gateResults: PipelineGateResult[] = []; - for (const phaseId of phaseIds) { - const gates = await this.sdlcDataLoader.loadGatesForPhase(phaseId); - for (const gate of gates) { - const result = await this.evaluateGate(gate, manifest.satellitePath, corePath, topology, manifest.facts); - gateResults.push(result); + let phaseGateEvals = 0; + + if (runPhaseGates) { + // Step 2: Determine which phases to evaluate (GT-343: accept canonical ids, + // normalize to the legacy f1..f5 the structured gate data is keyed by). + const phaseIds = manifest.phase + ? [toLegacyPhaseId(manifest.phase) ?? manifest.phase] + : ['f1', 'f2', 'f3', 'f4', 'f5']; + + // Step 3: Load gates from GT-280 structured data + for (const phaseId of phaseIds) { + const gates = await this.sdlcDataLoader.loadGatesForPhase(phaseId); + for (const gate of gates) { + const result = await this.evaluateGate(gate, manifest.satellitePath, corePath, topology, manifest.facts); + gateResults.push(result); + phaseGateEvals += result.artifactEvaluations.length; + } } } // Step 4: Run general ruleset validation (GT-395: enforce canonical rulesets) - const generalResult = await this.validator.validate(manifest.satellitePath, corePath); + let generalResult: ValidationResult | undefined; + if (runGeneralRulesets) { + generalResult = await this.validator.validate(manifest.satellitePath, corePath); - // GT-395: Convert blocking general-result issues into a synthetic gate so - // they are visible in the output and participate in the top-level verdict. - const generalGate = this.buildGeneralRulesetsGate(generalResult); - if (generalGate) { - gateResults.push(generalGate); + // GT-395: Convert blocking general-result issues into a synthetic gate so + // they are visible in the output and participate in the top-level verdict. + const generalGate = this.buildGeneralRulesetsGate(generalResult); + if (generalGate) { + gateResults.push(generalGate); + } } // Step 5: Build summary @@ -77,11 +142,13 @@ export class SatelliteEvaluationPipeline { totalGates: gateResults.length, passedGates: passedGates.length, failedGates: gateResults.length - passedGates.length, - totalRules: allEvals.length + generalResult.rulesChecked, + totalRules: allEvals.length + (generalResult?.rulesChecked ?? 0), passedRules: allEvals.filter(e => e.passed).length, failedRules: allEvals.filter(e => !e.passed).length, }; + const coverage = this.buildCoverage(phaseGateEvals, generalResult, outOfScopeGateIds); + // ADR-0073 output envelope const outputEnvelope = createSuccessEnvelope( { topology, gates: gateResults, summary }, @@ -102,6 +169,43 @@ export class SatelliteEvaluationPipeline { summary, evaluatedAt, outputEnvelope, + coverage, + }; + } + + /** + * GT-614 / GT-569 — the coverage facts of the run, with out-of-scope kept in its + * own bucket. + * + * A stage nobody asked for is NOT `skipped` and NOT `errored`: both of those mean + * the engine tried and the outcome is UNKNOWN, so both belong in `rulesTotal` and + * become coverage risks (and, for a blocking rule, fail the run under GT-595). + * Reporting an unasked question as an unanswered one is the exact dishonesty + * GT-569 exists to prevent. An out-of-scope stage therefore contributes NOTHING to + * the four counters — it only appears, by id, in `outOfScopeGateIds`, so a reader + * can still see what the request did not buy. + * + * The counters keep the GT-569 invariant `checked + skipped + errored === total`. + * The synthetic `general-rulesets` gate's evaluations are deliberately NOT counted + * here: they are ISSUES derived from the corpus run, whose rules are already in + * `generalResult.rulesChecked`. + */ + private buildCoverage( + phaseGateEvals: number, + generalResult: ValidationResult | undefined, + outOfScopeGateIds: readonly string[], + ): PipelineRuleCoverage { + const rulesChecked = phaseGateEvals + (generalResult?.rulesChecked ?? 0); + const rulesSkipped = generalResult?.rulesSkipped ?? 0; + const rulesErrored = generalResult?.rulesErrored ?? 0; + return { + rulesChecked, + rulesSkipped, + rulesErrored, + rulesTotal: rulesChecked + rulesSkipped + rulesErrored, + skippedRuleIds: generalResult?.skippedRuleIds ?? [], + erroredRuleIds: generalResult?.erroredRuleIds ?? [], + outOfScopeGateIds: [...outOfScopeGateIds], }; } diff --git a/src/packages/core-domain/src/application/use-cases/validate-satellite.use-case.ts b/src/packages/core-domain/src/application/use-cases/validate-satellite.use-case.ts index b7b5be1e7..909744e79 100644 --- a/src/packages/core-domain/src/application/use-cases/validate-satellite.use-case.ts +++ b/src/packages/core-domain/src/application/use-cases/validate-satellite.use-case.ts @@ -1,7 +1,11 @@ import { Injectable } from '@nestjs/common'; import { RulesetValidatorService, ValidationResult } from '../../application/validators/ruleset-validator.service'; import { SatelliteEvaluationPipeline } from '../services/satellite-evaluation-pipeline.service'; -import { SatelliteManifest, EvaluationVerdict } from '../../domain/satellite-manifest'; +import { SatelliteManifest } from '../../domain/satellite-manifest'; +import type { + PipelineExecutionPlan, + PipelineVerdict, +} from '../../evaluation/ports/evaluation-pipeline.port'; import * as pathModule from 'path'; import * as fsExtra from 'fs-extra'; @@ -18,13 +22,24 @@ export interface ValidateSatelliteInput { * general validation result. */ manifest?: SatelliteManifest; + /** + * GT-614 — which evaluation kinds the caller asked for, so the pipeline can skip + * the stages nobody requested. Absent ⇒ the whole pipeline runs, exactly as + * before: an undeclared request means "no kind was declared", not "no kind is + * wanted". Only meaningful together with `manifest`. + */ + plan?: PipelineExecutionPlan; } export interface ValidateSatelliteOutput { result: ValidationResult; formattedOutput?: string; - /** Present only when input.manifest was provided */ - evaluationVerdict?: EvaluationVerdict; + /** + * Present only when input.manifest was provided. Carries the GT-569/GT-614 + * `coverage` facts the pipeline produced, so the out-of-scope bucket survives + * the hop to the orchestrator instead of being flattened away here. + */ + evaluationVerdict?: PipelineVerdict; } @Injectable() @@ -37,11 +52,13 @@ export class ValidateSatelliteUseCase { } async execute(input: ValidateSatelliteInput): Promise { - const { satellitePath, corePath, rulesetId, engine, manifest } = input; + const { satellitePath, corePath, rulesetId, engine, manifest, plan } = input; // If a manifest was provided, run the end-to-end evaluation pipeline if (manifest) { - return this.executeWithPipeline(manifest); + // GT-614: the plan travels with the manifest, so the selection made from + // `ctx.kinds` reaches the pipeline instead of stopping at this boundary. + return this.executeWithPipeline(manifest, plan); } // Fall back to the standard validation logic @@ -75,7 +92,10 @@ export class ValidateSatelliteUseCase { return { result }; } - private async executeWithPipeline(manifest: SatelliteManifest): Promise { + private async executeWithPipeline( + manifest: SatelliteManifest, + plan?: PipelineExecutionPlan, + ): Promise { const corePath = manifest.corePath || this.findCoreFromSatellite(manifest.satellitePath); const validator = this.buildValidator(corePath); const pipeline = new SatelliteEvaluationPipeline( @@ -85,7 +105,7 @@ export class ValidateSatelliteUseCase { corePath, ); - const verdict = await pipeline.evaluate(manifest); + const verdict = await pipeline.evaluate(manifest, plan); const result: ValidationResult = { status: verdict.passed ? 'passed' : 'failed', rulesChecked: verdict.summary.totalRules, diff --git a/src/packages/mcp-server/src/tools/evaluate.tool.ts b/src/packages/mcp-server/src/tools/evaluate.tool.ts index b81560ca6..a6cc05d0a 100644 --- a/src/packages/mcp-server/src/tools/evaluate.tool.ts +++ b/src/packages/mcp-server/src/tools/evaluate.tool.ts @@ -79,11 +79,14 @@ export class EvaluateTool implements McpTool { const useCase = new ValidateSatelliteUseCase(this.validator); const pipeline: IEvaluationPipeline = { - evaluate: async (manifest) => { + // GT-614: same forwarding as the CLI, so MCP parity holds for what a + // single-kind request executes, not only for what it returns. + evaluate: async (manifest, plan) => { const out = await useCase.execute({ satellitePath: manifest.satellitePath, corePath: manifest.corePath, manifest, + plan, }); if (!out.evaluationVerdict) { throw new Error('Evaluation pipeline produced no verdict'); diff --git a/src/sdk/cli/src/commands/evaluate/evaluate.command.ts b/src/sdk/cli/src/commands/evaluate/evaluate.command.ts index 7c9c633a2..bde793bf1 100644 --- a/src/sdk/cli/src/commands/evaluate/evaluate.command.ts +++ b/src/sdk/cli/src/commands/evaluate/evaluate.command.ts @@ -96,11 +96,15 @@ export class EvaluateCommand extends BaseEvolithCommand { const resolvedCoreRoot = resolveRulesets(coreOverride).coreRoot; const pipeline: IEvaluationPipeline = { - evaluate: async (manifest) => { + // GT-614: forward the execution plan so `--kind compliance` costs what it + // asks for here too. The CLI is the reference surface; if it kept paying for + // every gate, the three surfaces would disagree on the same request. + evaluate: async (manifest, plan) => { const out = await this.useCase.execute({ satellitePath: manifest.satellitePath, corePath: manifest.corePath, manifest, + plan, }); if (!out.evaluationVerdict) { throw new Error('Evaluation pipeline produced no verdict'); From 6c6b92ef3ec5eb82a9b91d95fbba243f5a479f1e Mon Sep 17 00:00:00 2001 From: aarroyo Date: Wed, 29 Jul 2026 14:41:42 -0500 Subject: [PATCH 4/6] feat(cli): prove the machine channel across the registry, and govern the exit taxonomy (GT-580) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the two open criteria of GT-580. CRITERION 2 — `--format json` writes DATA ONLY to stdout. Output formatting and process exits were ALREADY centralized: `installMachineChannelGuard` (main.ts, armed before the command graph boots) reroutes every non-JSON stdout write to stderr while a machine format is active, and `exit-codes.ts` + `BaseEvolithCommand.handleError` map a thrown error onto the taxonomy in one place. What was missing was proof that the centralization actually holds across the CLI: the existing subprocess spec pipes two hand-picked commands. `machine-channel.registry-sweep.spec.ts` asks commander — the CLI's own registry, built from the real AppModule — which commands declare `--format`, and sweeps all 32 in real subprocesses with stdout and stderr on separate pipes. Each must hand back exactly one parseable JSON document and exit from the taxonomy. The enumeration has a floor, so a sweep over nothing fails instead of passing vacuously. Observed red: commenting out `installMachineChannelGuard()` turns `agents` (a clack banner) and `init-wizard` (a progress line) DIRTY while the other thirty stay clean. CRITERION 3 — a ruleset with Rego parity fails a command that exits outside the taxonomy. - `src/rulesets/cli/exit-code-taxonomy.rules.json` — CLI-EXIT-01 (no exit outside {0,1,2,3}), CLI-EXIT-02 (the scan must be non-vacuous), CLI-EXIT-03 (the taxonomy may not be widened to absorb an offender — otherwise CLI-EXIT-01 has a one-line escape). - `src/rulesets/opa/cli-exit-code-taxonomy.rego` + tests — the same three ids decided over the fact document the CLI's own producer emits. Aggregated into `main.rego`, and scoped to `input.core.cli` so a satellite evaluation stays silent. - `CliExitTaxonomyRuleHandler` — the native half, so `evolith validate` evaluates the rules instead of reporting three blocking rules that never ran. Observed red, end to end: planting `process.exit(7)` in a real CLI command file makes `evolith validate --core .` emit a BLOCKING CLI-EXIT-01 naming the file, and the CLI exits 2 (BLOCKED). The same file drives the committed producer and the committed .rego to the same verdict in `exit-code-taxonomy-ruleset.spec.ts`. Derived artifacts moved with the corpus: native-handler 151 -> 154, ISO/IEC 5055 mapping and the inventory summary regenerated. Co-Authored-By: Claude Opus 5 --- .../maturity-reports/inventory-summary.es.md | 4 +- .../maturity-reports/inventory-summary.md | 4 +- .../cli-exit-taxonomy-rule.handler.spec.ts | 173 +++++++++++++ .../cli-exit-taxonomy-rule.handler.ts | 183 +++++++++++++ .../validators/evaluators/native-evaluator.ts | 5 + .../validators/rule-corpus-triage.spec.ts | 7 +- .../cli-exit-code-taxonomy.fixture.json | 106 ++++++++ src/rulesets/cli/README.es.md | 1 + src/rulesets/cli/README.md | 1 + .../cli/exit-code-taxonomy.rules.json | 51 ++++ src/rulesets/opa/cli-exit-code-taxonomy.rego | 78 ++++++ .../opa/cli-exit-code-taxonomy.test.rego | 78 ++++++ src/rulesets/opa/main.rego | 7 + src/rulesets/standards/iso-5055-mapping.csv | 10 + src/rulesets/standards/iso-5055-mapping.json | 210 ++++++++++++++- .../cli/exit-code-taxonomy-ruleset.spec.ts | 215 +++++++++++++++ .../machine-channel.registry-sweep.spec.ts | 245 ++++++++++++++++++ 17 files changed, 1363 insertions(+), 15 deletions(-) create mode 100644 src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.spec.ts create mode 100644 src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.ts create mode 100644 src/packages/core-domain/test/parity-fixtures/cli-exit-code-taxonomy.fixture.json create mode 100644 src/rulesets/cli/exit-code-taxonomy.rules.json create mode 100644 src/rulesets/opa/cli-exit-code-taxonomy.rego create mode 100644 src/rulesets/opa/cli-exit-code-taxonomy.test.rego create mode 100644 src/sdk/cli/src/infrastructure/cli/exit-code-taxonomy-ruleset.spec.ts create mode 100644 src/sdk/cli/src/infrastructure/cli/machine-channel.registry-sweep.spec.ts diff --git a/reference/core/control-center/maturity-reports/inventory-summary.es.md b/reference/core/control-center/maturity-reports/inventory-summary.es.md index c16996e6f..d7f8b3dc9 100644 --- a/reference/core/control-center/maturity-reports/inventory-summary.es.md +++ b/reference/core/control-center/maturity-reports/inventory-summary.es.md @@ -8,7 +8,7 @@ Este es el conteo automatizado del inventario de la arquitectura de referencia c | Tipo de Artefacto | Conteo | Ubicación | |---|:---:|---| | **Architecture Decision Records (ADR)** | 139 | `reference/core/architecture/adrs/` | -| **Rulesets Legibles por Máquina** | 167 | `src/rulesets/` (en 20 categorías) | +| **Rulesets Legibles por Máquina** | 175 | `src/rulesets/` (en 20 categorías) | | **Schemas de Phase-Gates** | 45 | `src/rulesets/schema/` | -*Última Actualización: 2026-07-28* +*Última Actualización: 2026-07-29* diff --git a/reference/core/control-center/maturity-reports/inventory-summary.md b/reference/core/control-center/maturity-reports/inventory-summary.md index 06211375d..a39afefd2 100644 --- a/reference/core/control-center/maturity-reports/inventory-summary.md +++ b/reference/core/control-center/maturity-reports/inventory-summary.md @@ -8,7 +8,7 @@ This is the automated inventory tally of the core reference architecture and gov | Artifact Type | Count | Location | |---|:---:|---| | **Architecture Decision Records (ADR)** | 139 | `reference/core/architecture/adrs/` | -| **Machine-Readable Rulesets** | 167 | `src/rulesets/` (across 20 categories) | +| **Machine-Readable Rulesets** | 175 | `src/rulesets/` (across 20 categories) | | **Phase-Gate Schemas** | 45 | `src/rulesets/schema/` | -*Last Updated: 2026-07-28* +*Last Updated: 2026-07-29* diff --git a/src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.spec.ts b/src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.spec.ts new file mode 100644 index 000000000..ce3163f44 --- /dev/null +++ b/src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.spec.ts @@ -0,0 +1,173 @@ +import * as path from 'path'; +import { CliExitTaxonomyRuleHandler } from './cli-exit-taxonomy-rule.handler'; +import { IFileSystem } from '../../../../domain/interfaces'; +import { NormalizedRule } from '../../../../domain/models/normalized-rule'; + +/** + * GT-580 — the rule must be SEEN failing. + * + * Every scenario below plants a scratch CLI tree and asks the handler for a + * verdict. The one that matters is `an out-of-taxonomy exit`: a command file + * carrying `process.exit(7)` has to come back `failed`, because an assertion + * that cannot fail is the defect this backlog is about. + */ + +const CORE = '/core'; +const CTX = { satellitePath: '/sat', corePath: CORE }; +const CLI_SRC = path.join(CORE, 'src', 'sdk', 'cli', 'src'); + +/** The real `exit-codes.ts` shape, so CLI-EXIT-03 parses something authentic. */ +const EXIT_CODES_TS = ` +export const CLI_EXIT_CODES = { + OK: 0, + TOOL_FAILURE: 1, + BLOCKED: 2, + INVALID_INPUT: 3, +} as const; +`; + +/** + * An in-memory tree: `files` maps an absolute path to its contents, and every + * directory on the way is derived rather than declared, so a scenario cannot + * forget one and accidentally scan nothing. + */ +function treeFs(files: Record): IFileSystem { + const dirs = new Map>(); + for (const file of Object.keys(files)) { + let current = file; + for (;;) { + const parent = path.dirname(current); + if (parent === current) break; + if (!dirs.has(parent)) dirs.set(parent, new Set()); + dirs.get(parent)!.add(path.basename(current)); + current = parent; + } + } + + return { + exists: jest.fn(async (p: string) => p in files || dirs.has(p)), + readFile: jest.fn(async (p: string) => files[p] ?? ''), + readdirNames: jest.fn(async (p: string) => [...(dirs.get(p) ?? [])]), + stat: jest.fn(async (p: string) => ({ + isDirectory: () => dirs.has(p) && !(p in files), + isFile: () => p in files, + })), + } as unknown as IFileSystem; +} + +function rule(id: string): NormalizedRule { + return { + id, + severity: 'MUST', + category: 'cli-exit-taxonomy', + title: 't', + description: 'd', + blocking: true, + sourceFile: 'cli/exit-code-taxonomy.rules.json', + }; +} + +/** A CLI tree that obeys the taxonomy. */ +function compliantTree(extra: Record = {}): Record { + return { + [path.join(CLI_SRC, 'infrastructure', 'cli', 'exit-codes.ts')]: EXIT_CODES_TS, + [path.join(CLI_SRC, 'commands', 'gate', 'gate.command.ts')]: + 'if (blocked) process.exit(2);\nprocess.exitCode = 0;\n', + [path.join(CLI_SRC, 'main.ts')]: 'process.exit(1);\n', + ...extra, + }; +} + +describe('GT-580 · CliExitTaxonomyRuleHandler', () => { + it('claims the CLI-EXIT- family and nothing else', () => { + const handler = new CliExitTaxonomyRuleHandler(treeFs(compliantTree())); + expect(handler.canHandle(rule('CLI-EXIT-01'))).toBe(true); + expect(handler.canHandle(rule('CLI-EXIT-03'))).toBe(true); + expect(handler.canHandle(rule('CLI-RR-01'))).toBe(false); + expect(handler.canHandle(rule('DEP-01'))).toBe(false); + }); + + it('passes all three rules on a CLI that obeys the taxonomy', async () => { + const handler = new CliExitTaxonomyRuleHandler(treeFs(compliantTree())); + for (const id of ['CLI-EXIT-01', 'CLI-EXIT-02', 'CLI-EXIT-03']) { + const result = await handler.evaluate(rule(id), CTX); + expect({ id, result: result.result }).toEqual({ id, result: 'passed' }); + } + }); + + // ---- the red ----------------------------------------------------------- + + it('FAILS CLI-EXIT-01 on a command that exits outside the taxonomy', async () => { + const rogue = path.join(CLI_SRC, 'commands', 'rogue', 'rogue.command.ts'); + const handler = new CliExitTaxonomyRuleHandler( + treeFs(compliantTree({ [rogue]: 'export function boom(): void {\n process.exit(7);\n}\n' })), + ); + + const result = await handler.evaluate(rule('CLI-EXIT-01'), CTX); + + expect(result.result).toBe('failed'); + // The finding must name the offender: "1 rule failed" sends nobody anywhere. + expect(result.message).toContain('commands/rogue/rogue.command.ts'); + expect(result.message).toContain('process.exit(7)'); + }); + + it('FAILS CLI-EXIT-01 on `process.exitCode = 64` too, not only on process.exit()', async () => { + const rogue = path.join(CLI_SRC, 'commands', 'rogue', 'other.command.ts'); + const handler = new CliExitTaxonomyRuleHandler( + treeFs(compliantTree({ [rogue]: 'process.exitCode = 64;\n' })), + ); + + const result = await handler.evaluate(rule('CLI-EXIT-01'), CTX); + expect(result.result).toBe('failed'); + expect(result.message).toContain('process.exitCode = 64'); + }); + + it('ignores test sources, which legitimately name codes outside the taxonomy', async () => { + // Including the negative fixtures that prove this very rule can fail — if + // they counted, the rule could never be green anywhere. + const handler = new CliExitTaxonomyRuleHandler( + treeFs( + compliantTree({ + [path.join(CLI_SRC, 'commands', 'rogue', 'rogue.spec.ts')]: 'process.exit(9);\n', + [path.join(CLI_SRC, 'commands', 'rogue', 'rogue.test.ts')]: 'process.exit(77);\n', + }), + ), + ); + + expect((await handler.evaluate(rule('CLI-EXIT-01'), CTX)).result).toBe('passed'); + }); + + it('FAILS CLI-EXIT-02 when the scan reads nothing', async () => { + // The source root exists but holds no `.ts` — the shape a moved directory + // produces, and the one that would otherwise report perfect compliance. + const handler = new CliExitTaxonomyRuleHandler( + treeFs({ [path.join(CLI_SRC, 'README.md')]: '# not a source' }), + ); + + const result = await handler.evaluate(rule('CLI-EXIT-02'), CTX); + expect(result.result).toBe('failed'); + expect(result.message).toContain('0 sources'); + }); + + it('FAILS rather than skips when the CLI source root is absent', async () => { + // `skipped` on a blocking rule is the silent pass this rule exists against. + const handler = new CliExitTaxonomyRuleHandler(treeFs({ '/elsewhere/file.ts': '' })); + for (const id of ['CLI-EXIT-01', 'CLI-EXIT-02', 'CLI-EXIT-03']) { + const result = await handler.evaluate(rule(id), CTX); + expect({ id, result: result.result }).toEqual({ id, result: 'failed' }); + } + }); + + it('FAILS CLI-EXIT-03 when the taxonomy is widened to absorb the offender', async () => { + // The escape hatch: declare 7 legal and CLI-EXIT-01 goes quiet. This is the + // rule that stops that from being a fix. + const widened = EXIT_CODES_TS.replace('} as const;', ' ROGUE: 7,\n} as const;'); + const handler = new CliExitTaxonomyRuleHandler( + treeFs(compliantTree({ [path.join(CLI_SRC, 'infrastructure', 'cli', 'exit-codes.ts')]: widened })), + ); + + const result = await handler.evaluate(rule('CLI-EXIT-03'), CTX); + expect(result.result).toBe('failed'); + expect(result.message).toContain('[0, 1, 2, 3, 7]'); + }); +}); diff --git a/src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.ts b/src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.ts new file mode 100644 index 000000000..3582f2285 --- /dev/null +++ b/src/packages/core-domain/src/application/validators/evaluators/handlers/cli-exit-taxonomy-rule.handler.ts @@ -0,0 +1,183 @@ +import * as path from 'path'; +import { IFileSystem } from '../../../../domain/interfaces'; +import { NormalizedRule } from '../../../../domain/models/normalized-rule'; +import { WorkspaceEvaluationContext, RuleEvaluationResult } from '../evaluator.interface'; +import { INativeRuleHandler } from './rule-handler.interface'; + +/** + * GT-580 — the native half of `src/rulesets/cli/exit-code-taxonomy.rules.json`. + * + * The exit code is the only control primitive an agent harness, a pre-commit + * hook and a CI step have in common, and it only carries meaning while every + * command draws from one published set: `0` pass, `1` tool failure, `2` blocked, + * `3` invalid input. A jest scan inside the CLI package already refuses a source + * that names anything else — but that is a unit test of one package. It cannot + * be evaluated against a repository, it produces no finding, and it has no Rego + * parity, so a governance consumer has no way to ask the question at all. + * + * This handler is that missing half. `evolith validate` on the Core now + * evaluates CLI-EXIT-01/02/03 like any other rule, they are `blocking: true`, so + * a command that exits outside the taxonomy makes the run BLOCK (exit 2) rather + * than merely failing somebody's unit tests. + * + * PARITY. `src/rulesets/opa/cli-exit-code-taxonomy.rego` decides the same three + * ids from the fact document that `src/sdk/cli/scripts/exit-code-taxonomy-facts.mjs` + * emits. The scan below is the same scan: same file filter, same literal regex, + * same taxonomy. Rego receives facts and this handler produces them, which is + * why the duplication is a boundary rather than a fork — core-domain cannot + * import a script out of the CLI package without inverting the dependency. + */ +export class CliExitTaxonomyRuleHandler implements INativeRuleHandler { + /** The published taxonomy. Mirrored from `sdk/cli/src/infrastructure/cli/exit-codes.ts`. */ + private static readonly TAXONOMY: readonly number[] = [0, 1, 2, 3]; + + /** `process.exit(N)` / `process.exitCode = N` with a numeric literal. */ + private static readonly EXIT_LITERAL = /process\.exit\(\s*(-?\d+)\s*\)|process\.exitCode\s*=\s*(-?\d+)/g; + + constructor(private readonly fs: IFileSystem) {} + + canHandle(rule: NormalizedRule): boolean { + return rule.id.startsWith('CLI-EXIT-'); + } + + async evaluate(rule: NormalizedRule, ctx: WorkspaceEvaluationContext): Promise { + if (rule.id === 'CLI-EXIT-01') return this.evalNoOffenders(rule, ctx); + if (rule.id === 'CLI-EXIT-02') return this.evalNonVacuousScan(rule, ctx); + if (rule.id === 'CLI-EXIT-03') return this.evalTaxonomyNotWidened(rule, ctx); + + return { rule, result: 'skipped', message: 'Unhandled CLI-EXIT rule' }; + } + + /** CLI-EXIT-01 — every exit literal in the CLI is a member of the taxonomy. */ + private async evalNoOffenders(rule: NormalizedRule, ctx: WorkspaceEvaluationContext): Promise { + const root = this.cliSourceRoot(ctx); + if (!(await this.fs.exists(root))) { + // A missing tree is not compliance. Reported as failed rather than + // skipped, because `skipped` on a blocking rule is precisely the silent + // pass this rule exists to prevent. + return { rule, result: 'failed', message: `CLI source root not found: ${root}` }; + } + + const scan = await this.scan(root); + if (scan.offenders.length === 0) { + return { rule, result: 'passed' }; + } + + const named = scan.offenders + .map((o) => `${o.file}: ${o.snippet}`) + .join('; '); + return { + rule, + result: 'failed', + message: + `${scan.offenders.length} CLI source(s) exit outside the published taxonomy ` + + `{${CliExitTaxonomyRuleHandler.TAXONOMY.join(', ')}} — ${named}`, + }; + } + + /** CLI-EXIT-02 — a scan that read nothing found no offenders, which is not compliance. */ + private async evalNonVacuousScan(rule: NormalizedRule, ctx: WorkspaceEvaluationContext): Promise { + const root = this.cliSourceRoot(ctx); + if (!(await this.fs.exists(root))) { + return { rule, result: 'failed', message: `CLI source root not found: ${root}` }; + } + + const scan = await this.scan(root); + if (scan.scanned === 0) { + return { + rule, + result: 'failed', + message: `the exit-code taxonomy scan covered 0 sources under ${root} — an empty scan is not a pass`, + }; + } + return { rule, result: 'passed' }; + } + + /** + * CLI-EXIT-03 — the taxonomy has not been widened to absorb an offender. + * + * Read out of the CLI's own `exit-codes.ts` rather than trusted: the cheap way + * to silence CLI-EXIT-01 is to add the offending code to `CLI_EXIT_CODES`, and + * that makes the consumer's problem worse rather than better. + */ + private async evalTaxonomyNotWidened(rule: NormalizedRule, ctx: WorkspaceEvaluationContext): Promise { + const file = path.join(this.cliSourceRoot(ctx), 'infrastructure', 'cli', 'exit-codes.ts'); + if (!(await this.fs.exists(file))) { + return { rule, result: 'failed', message: `exit-code taxonomy not declared: ${file} not found` }; + } + + const source = await this.fs.readFile(file); + const block = /CLI_EXIT_CODES\s*=\s*\{([\s\S]*?)\}\s*as const/.exec(source); + if (!block) { + return { rule, result: 'failed', message: `could not read CLI_EXIT_CODES out of ${file}` }; + } + + const declared = [...block[1].matchAll(/:\s*(-?\d+)\s*,/g)] + .map((m) => Number(m[1])) + .sort((a, b) => a - b); + + const expected = [...CliExitTaxonomyRuleHandler.TAXONOMY]; + if (declared.length === expected.length && declared.every((v, i) => v === expected[i])) { + return { rule, result: 'passed' }; + } + return { + rule, + result: 'failed', + message: + `the CLI declares exit codes [${declared.join(', ')}]; the published taxonomy is exactly ` + + `[${expected.join(', ')}] and widening it is a governance decision, not a fix`, + }; + } + + private cliSourceRoot(ctx: WorkspaceEvaluationContext): string { + return path.join(ctx.corePath, 'src', 'sdk', 'cli', 'src'); + } + + private async scan(root: string): Promise<{ scanned: number; offenders: Array<{ file: string; code: number; snippet: string }> }> { + const files = await this.sourceFiles(root); + const offenders: Array<{ file: string; code: number; snippet: string }> = []; + + for (const file of files) { + const source = await this.fs.readFile(file); + // `matchAll` on a `/g` regex consumes it; a fresh instance per file keeps + // `lastIndex` from carrying between reads. + const pattern = new RegExp(CliExitTaxonomyRuleHandler.EXIT_LITERAL.source, 'g'); + for (const match of source.matchAll(pattern)) { + const code = Number(match[1] ?? match[2]); + if (!CliExitTaxonomyRuleHandler.TAXONOMY.includes(code)) { + offenders.push({ file: path.relative(root, file), code, snippet: match[0] }); + } + } + } + + return { scanned: files.length, offenders }; + } + + /** + * Every non-test `.ts` source under the CLI. + * + * Tests are excluded because they legitimately stub and assert on exit codes + * outside the taxonomy — including the negative fixtures that prove this very + * rule can fail. + */ + private async sourceFiles(dir: string, depth = 0): Promise { + if (depth > 10) return []; + const out: string[] = []; + const entries = await this.fs.readdirNames(dir); + + for (const entry of entries) { + if (entry === 'node_modules' || entry === '__mocks__') continue; + const full = path.join(dir, entry); + const stat = await this.fs.stat(full); + if (stat.isDirectory()) { + out.push(...(await this.sourceFiles(full, depth + 1))); + continue; + } + if (!entry.endsWith('.ts')) continue; + if (entry.endsWith('.spec.ts') || entry.endsWith('.test.ts')) continue; + out.push(full); + } + + return out; + } +} diff --git a/src/packages/core-domain/src/application/validators/evaluators/native-evaluator.ts b/src/packages/core-domain/src/application/validators/evaluators/native-evaluator.ts index 8f5bf74cb..955bdf3c7 100644 --- a/src/packages/core-domain/src/application/validators/evaluators/native-evaluator.ts +++ b/src/packages/core-domain/src/application/validators/evaluators/native-evaluator.ts @@ -4,6 +4,7 @@ import { IRuleEvaluatorStrategy, WorkspaceEvaluationContext, RuleEvaluationResul import { INativeRuleHandler } from './handlers/rule-handler.interface'; import { EvidenceRuleHandler } from './handlers/evidence-rule.handler'; import { CliReleaseRuleHandler } from './handlers/cli-release-rule.handler'; +import { CliExitTaxonomyRuleHandler } from './handlers/cli-exit-taxonomy-rule.handler'; import { McpRuleHandler } from './handlers/mcp-rule.handler'; import { DependencyRuleHandler } from './handlers/dependency-rule.handler'; import { TaxonomyRuleHandler } from './handlers/taxonomy-rule.handler'; @@ -29,6 +30,10 @@ export class NativeEvaluator implements IRuleEvaluatorStrategy { this.handlers = [ new EvidenceRuleHandler(fs), new CliReleaseRuleHandler(fs), + // GT-580: CLI-EXIT-01/02/03 — the published exit-code taxonomy, evaluated + // against the CLI source tree so a command that exits outside it BLOCKS a + // run instead of only failing the CLI package's own unit tests. + new CliExitTaxonomyRuleHandler(fs), new McpRuleHandler(fs), new DependencyRuleHandler(fs), new TaxonomyRuleHandler(fs), diff --git a/src/packages/core-domain/src/application/validators/rule-corpus-triage.spec.ts b/src/packages/core-domain/src/application/validators/rule-corpus-triage.spec.ts index ec71e39e1..e5ff64e73 100644 --- a/src/packages/core-domain/src/application/validators/rule-corpus-triage.spec.ts +++ b/src/packages/core-domain/src/application/validators/rule-corpus-triage.spec.ts @@ -198,8 +198,13 @@ describe('GT-595 · the published breakdown, with its denominator', () => { // `enforce` was dropped at normalization. It is now carried, and // `ModuleBoundaryRuleHandler` evaluates it. // `unimplemented-native` drops by the same twelve; nothing else moved. + // + // 151 -> 154 on 2026-07-29: GT-580 added `cli/exit-code-taxonomy.rules.json` + // (CLI-EXIT-01/02/03) with `CliExitTaxonomyRuleHandler` claiming all three, + // so the corpus grows by three and every one of them lands directly in + // `native-handler`. No other class moves: these ids did not exist before. expect(SUMMARY.byClass).toEqual({ - 'native-handler': 151, + 'native-handler': 154, 'documentation-only': 136, 'unimplemented-native': 48, 'needs-external-system': 20, diff --git a/src/packages/core-domain/test/parity-fixtures/cli-exit-code-taxonomy.fixture.json b/src/packages/core-domain/test/parity-fixtures/cli-exit-code-taxonomy.fixture.json new file mode 100644 index 000000000..dec7e6a1e --- /dev/null +++ b/src/packages/core-domain/test/parity-fixtures/cli-exit-code-taxonomy.fixture.json @@ -0,0 +1,106 @@ +{ + "domain": "cli-exit-code-taxonomy", + "description": "GT-580 — CLI exit-code taxonomy parity. The same three rule ids decided by src/rulesets/opa/cli-exit-code-taxonomy.rego over the fact document, and by CliExitTaxonomyRuleHandler over the source tree that produces it. The `rogue-exit-code` scenario is the criterion itself: a command exiting 7 must fail the gate.", + "rules": [ + { "id": "CLI-EXIT-01", "severity": "MUST", "category": "cli-exit-taxonomy", "title": "No CLI Command May Exit Outside The Published Taxonomy", "description": "Every exit literal must be 0, 1, 2 or 3", "blocking": true, "sourceFile": "cli/exit-code-taxonomy.rules.json" }, + { "id": "CLI-EXIT-02", "severity": "MUST", "category": "cli-exit-taxonomy", "title": "The Taxonomy Scan Must Be Non-Vacuous", "description": "A scan that reads nothing is not compliance", "blocking": true, "sourceFile": "cli/exit-code-taxonomy.rules.json" }, + { "id": "CLI-EXIT-03", "severity": "MUST", "category": "cli-exit-taxonomy", "title": "The Published Taxonomy Must Not Be Widened", "description": "CLI_EXIT_CODES must declare exactly 0, 1, 2, 3", "blocking": true, "sourceFile": "cli/exit-code-taxonomy.rules.json" } + ], + "scenarios": { + "compliant": { + "filesystem": { + "existing": [ + "/core/src/sdk/cli/src", + "/core/src/sdk/cli/src/main.ts", + "/core/src/sdk/cli/src/infrastructure", + "/core/src/sdk/cli/src/infrastructure/cli", + "/core/src/sdk/cli/src/infrastructure/cli/exit-codes.ts" + ], + "directories": [ + "/core/src/sdk/cli/src", + "/core/src/sdk/cli/src/infrastructure", + "/core/src/sdk/cli/src/infrastructure/cli" + ], + "dirs": { + "/core/src/sdk/cli/src": ["main.ts", "infrastructure"], + "/core/src/sdk/cli/src/infrastructure": ["cli"], + "/core/src/sdk/cli/src/infrastructure/cli": ["exit-codes.ts"] + }, + "files": { + "/core/src/sdk/cli/src/main.ts": "process.exit(1);\n", + "/core/src/sdk/cli/src/infrastructure/cli/exit-codes.ts": "export const CLI_EXIT_CODES = {\n OK: 0,\n TOOL_FAILURE: 1,\n BLOCKED: 2,\n INVALID_INPUT: 3,\n} as const;\n" + } + }, + "context": { "satellitePath": "/sat", "corePath": "/core" }, + "expectedResults": [ + { "ruleId": "CLI-EXIT-01", "result": "passed" }, + { "ruleId": "CLI-EXIT-02", "result": "passed" }, + { "ruleId": "CLI-EXIT-03", "result": "passed" } + ] + }, + "rogue-exit-code": { + "filesystem": { + "existing": [ + "/core/src/sdk/cli/src", + "/core/src/sdk/cli/src/main.ts", + "/core/src/sdk/cli/src/rogue.command.ts", + "/core/src/sdk/cli/src/infrastructure", + "/core/src/sdk/cli/src/infrastructure/cli", + "/core/src/sdk/cli/src/infrastructure/cli/exit-codes.ts" + ], + "directories": [ + "/core/src/sdk/cli/src", + "/core/src/sdk/cli/src/infrastructure", + "/core/src/sdk/cli/src/infrastructure/cli" + ], + "dirs": { + "/core/src/sdk/cli/src": ["main.ts", "rogue.command.ts", "infrastructure"], + "/core/src/sdk/cli/src/infrastructure": ["cli"], + "/core/src/sdk/cli/src/infrastructure/cli": ["exit-codes.ts"] + }, + "files": { + "/core/src/sdk/cli/src/main.ts": "process.exit(1);\n", + "/core/src/sdk/cli/src/rogue.command.ts": "export function boom(): void {\n process.exit(7);\n}\n", + "/core/src/sdk/cli/src/infrastructure/cli/exit-codes.ts": "export const CLI_EXIT_CODES = {\n OK: 0,\n TOOL_FAILURE: 1,\n BLOCKED: 2,\n INVALID_INPUT: 3,\n} as const;\n" + } + }, + "context": { "satellitePath": "/sat", "corePath": "/core" }, + "expectedResults": [ + { "ruleId": "CLI-EXIT-01", "result": "failed" }, + { "ruleId": "CLI-EXIT-02", "result": "passed" }, + { "ruleId": "CLI-EXIT-03", "result": "passed" } + ] + }, + "widened-taxonomy": { + "filesystem": { + "existing": [ + "/core/src/sdk/cli/src", + "/core/src/sdk/cli/src/main.ts", + "/core/src/sdk/cli/src/infrastructure", + "/core/src/sdk/cli/src/infrastructure/cli", + "/core/src/sdk/cli/src/infrastructure/cli/exit-codes.ts" + ], + "directories": [ + "/core/src/sdk/cli/src", + "/core/src/sdk/cli/src/infrastructure", + "/core/src/sdk/cli/src/infrastructure/cli" + ], + "dirs": { + "/core/src/sdk/cli/src": ["main.ts", "infrastructure"], + "/core/src/sdk/cli/src/infrastructure": ["cli"], + "/core/src/sdk/cli/src/infrastructure/cli": ["exit-codes.ts"] + }, + "files": { + "/core/src/sdk/cli/src/main.ts": "process.exit(7);\n", + "/core/src/sdk/cli/src/infrastructure/cli/exit-codes.ts": "export const CLI_EXIT_CODES = {\n OK: 0,\n TOOL_FAILURE: 1,\n BLOCKED: 2,\n INVALID_INPUT: 3,\n ROGUE: 7,\n} as const;\n" + } + }, + "context": { "satellitePath": "/sat", "corePath": "/core" }, + "expectedResults": [ + { "ruleId": "CLI-EXIT-01", "result": "failed" }, + { "ruleId": "CLI-EXIT-02", "result": "passed" }, + { "ruleId": "CLI-EXIT-03", "result": "failed" } + ] + } + } +} diff --git a/src/rulesets/cli/README.es.md b/src/rulesets/cli/README.es.md index d24a1c9c7..b8f0bdec5 100644 --- a/src/rulesets/cli/README.es.md +++ b/src/rulesets/cli/README.es.md @@ -10,6 +10,7 @@ Reglas legibles por máquina para preparación de release del Evolith Evolith CL |---|---| | [Preparación de Release del CLI](./release-readiness.rules.json) | Define evidencia mínima de build, pruebas, paquete y smoke MCP antes de liberar el CLI. | | [Paridad CLI/Core](./core-parity.rules.json) | Exige que cada capacidad de regla Core sea trazable a CLI, MCP, pruebas y estado de evidencia. | +| [Taxonomía de Exit Codes del CLI](./exit-code-taxonomy.rules.json) | Exige que todo comando del CLI salga con un código de la taxonomía publicada (`0` pass, `1` fallo de herramienta, `2` bloqueado, `3` entrada inválida), sobre un escaneo no vacío y sin ensanchar la taxonomía para absorber al infractor. Paridad Rego en [`opa/cli-exit-code-taxonomy.rego`](../opa/cli-exit-code-taxonomy.rego). | ## Intención de Validación diff --git a/src/rulesets/cli/README.md b/src/rulesets/cli/README.md index bcc0591da..4c67665ac 100644 --- a/src/rulesets/cli/README.md +++ b/src/rulesets/cli/README.md @@ -10,6 +10,7 @@ Machine-readable rules for Evolith Evolith CLI release readiness and parity with |---|---| | [CLI Release Readiness](./release-readiness.rules.json) | Defines minimum build, test, package, and MCP smoke evidence before CLI release. | | [CLI/Core Parity](./core-parity.rules.json) | Requires every Core rule capability to be traced to CLI, MCP, tests, and evidence status. | +| [CLI Exit-Code Taxonomy](./exit-code-taxonomy.rules.json) | Requires every CLI command to exit with a code drawn from the published taxonomy (`0` pass, `1` tool failure, `2` blocked, `3` invalid input), over a non-vacuous scan, without widening the taxonomy to absorb an offender. Rego parity in [`opa/cli-exit-code-taxonomy.rego`](../opa/cli-exit-code-taxonomy.rego). | ## Validation Intent diff --git a/src/rulesets/cli/exit-code-taxonomy.rules.json b/src/rulesets/cli/exit-code-taxonomy.rules.json new file mode 100644 index 000000000..6d12b3ceb --- /dev/null +++ b/src/rulesets/cli/exit-code-taxonomy.rules.json @@ -0,0 +1,51 @@ +{ + "$schema": "../schema/ruleset-standard.schema.json", + "$id": "https://evolith.dev/rulesets/cli/exit-code-taxonomy.rules.json", + "title": "CLI Exit-Code Taxonomy Rules", + "description": "GT-580 — the exit code is the only control primitive an agent harness, a pre-commit hook and a CI step have in common. This ruleset governs it: every command must exit with a code drawn from the published taxonomy (0 pass, 1 tool failure, 2 blocked, 3 invalid input), the scan that proves it must be non-vacuous, and the taxonomy itself must not be widened to make an offender disappear.", + "version": "1.0.0", + "effectiveDate": "2026-07-29", + "scope": "core-cli", + "category": "exit-code-taxonomy", + "audience": "core", + "rules": [ + { + "id": "CLI-EXIT-01", + "severity": "MUST", + "category": "cli-exit-taxonomy", + "title": "No CLI Command May Exit Outside The Published Taxonomy", + "description": "Every exit code named by a CLI source MUST be a member of the published taxonomy: 0 pass, 1 tool failure, 2 blocked, 3 invalid input. A command that exits with anything else is unreadable to the consumer the code exists for — an agent cannot tell a blocked gate from a crashed tool, which is the difference between a blocked merge and a retry.", + "validationQuery": "Scan every non-test .ts source under src/sdk/cli/src for `process.exit(N)` and `process.exitCode = N` with a numeric literal, and require every N to be one of 0, 1, 2, 3.", + "evidenceRequired": [ + "offending file and exit literal, per offender", + "number of sources scanned" + ], + "blocking": true + }, + { + "id": "CLI-EXIT-02", + "severity": "MUST", + "category": "cli-exit-taxonomy", + "title": "The Taxonomy Scan Must Be Non-Vacuous", + "description": "The scan behind CLI-EXIT-01 MUST cover a non-empty set of CLI sources. A scan that reads nothing also finds no offenders, so a moved or mistyped source root would report perfect compliance while checking an empty directory. The denominator is part of the verdict, not a diagnostic.", + "validationQuery": "Require the number of CLI sources scanned by the exit-code taxonomy scan to be greater than zero.", + "evidenceRequired": [ + "number of sources scanned", + "source root the scan resolved to" + ], + "blocking": true + }, + { + "id": "CLI-EXIT-03", + "severity": "MUST", + "category": "cli-exit-taxonomy", + "title": "The Published Taxonomy Must Not Be Widened", + "description": "The declared taxonomy MUST be exactly {0, 1, 2, 3}. Without this, CLI-EXIT-01 has a trivial escape hatch: adding the offending code to the taxonomy makes the violation disappear while the consumer's problem — an exit code carrying no agreed meaning — gets worse. Widening the taxonomy is a governance decision, so it must fail the gate rather than pass it silently.", + "validationQuery": "Require the taxonomy declared by src/sdk/cli/src/infrastructure/cli/exit-codes.ts to be exactly the four codes 0, 1, 2 and 3.", + "evidenceRequired": [ + "declared taxonomy values" + ], + "blocking": true + } + ] +} diff --git a/src/rulesets/opa/cli-exit-code-taxonomy.rego b/src/rulesets/opa/cli-exit-code-taxonomy.rego new file mode 100644 index 000000000..78361f1c8 --- /dev/null +++ b/src/rulesets/opa/cli-exit-code-taxonomy.rego @@ -0,0 +1,78 @@ +# GT-580 — the CLI exit-code taxonomy, as policy. +# +# Rego parity for `src/rulesets/cli/exit-code-taxonomy.rules.json`: the same three +# rule ids, the same verdicts, evaluated over the fact document produced by +# `src/sdk/cli/scripts/exit-code-taxonomy-facts.mjs --json` and mounted at +# `input.core.cli.exitCodes`. +# +# The policy deliberately does NOT re-implement the file scan. A policy that +# walked a source tree would be a second scanner drifting away from the first; +# facts in, verdict out is the whole point of keeping the producer separate. +# +# Published taxonomy: 0 pass · 1 tool failure · 2 blocked · 3 invalid input. +package evolith.cli_exit_code_taxonomy + +import rego.v1 + +facts := input.core.cli.exitCodes + +# CLI-EXIT-01 — a command that exits outside the taxonomy fails the gate. +# +# One violation per offender, so the message names the file and the literal +# rather than reporting an anonymous count somebody then has to go hunting for. +violations contains { + "id": "CLI-EXIT-01", + "message": sprintf( + "%s exits with %d, which is outside the published CLI exit-code taxonomy {0,1,2,3} (%s)", + [offender.file, offender.code, offender.snippet], + ), +} if { + some offender in facts.offenders +} + +# CLI-EXIT-02 — a scan that read nothing is not compliance. +# +# Without this, a moved source root turns CLI-EXIT-01 into a rule that can never +# fire: no files scanned means no offenders found, and the gate reports green +# over an empty directory. +violations contains { + "id": "CLI-EXIT-02", + "message": sprintf( + "the exit-code taxonomy scan covered %d CLI sources — a scan that reads nothing finds no offenders, which is not compliance", + [facts.scanned], + ), +} if { + facts.scanned == 0 +} + +# CLI-EXIT-03 — the taxonomy itself may not be widened. +# +# The cheap way to make CLI-EXIT-01 green is to declare the offending code part +# of the taxonomy. That makes the consumer's problem worse, not better, so it +# fails here instead of passing silently. +violations contains { + "id": "CLI-EXIT-03", + "message": sprintf( + "the CLI declares exit codes %v; the published taxonomy is exactly [0, 1, 2, 3] and widening it is a governance decision, not a fix", + [facts.declared], + ), +} if { + facts.declared != [0, 1, 2, 3] +} + +# Absence of facts is not compliance either: a consumer that forgot to attach the +# fact document must not read as a pass. +# +# Scoped to `input.core.cli` on purpose. This ruleset is `audience: core` — it +# addresses the Evolith monorepo's own CLI — and `main.rego` aggregates it, so an +# unscoped rule would fire on every satellite evaluation, where the CLI source +# tree does not exist and the fact document is meaningless. Declaring +# `input.core.cli` is the caller saying "I am evaluating the Core CLI"; having +# said it, omitting the facts is a defect rather than a non-applicable rule. +violations contains { + "id": "CLI-EXIT-02", + "message": "no exit-code taxonomy facts were supplied at input.core.cli.exitCodes — the rule cannot be evaluated, and an unevaluated blocking rule is not a pass", +} if { + input.core.cli + not input.core.cli.exitCodes +} diff --git a/src/rulesets/opa/cli-exit-code-taxonomy.test.rego b/src/rulesets/opa/cli-exit-code-taxonomy.test.rego new file mode 100644 index 000000000..0b5da48b3 --- /dev/null +++ b/src/rulesets/opa/cli-exit-code-taxonomy.test.rego @@ -0,0 +1,78 @@ +# GT-580 — executable tests for the CLI exit-code taxonomy policy. +# +# The negative cases are the point. A guard nobody has ever seen fail is not a +# guard, and this board has repeatedly caught exactly that. Each test below moves +# ONE field of a known-compliant fact document and names the rule it expects. +package evolith.cli_exit_code_taxonomy_test + +import rego.v1 + +import data.evolith.cli_exit_code_taxonomy + +# The real shape emitted by `exit-code-taxonomy-facts.mjs --json`, with the +# numbers measured against the CLI on 2026-07-29. +compliant_input := {"core": {"cli": {"exitCodes": { + "schemaVersion": "1.0.0", + "declared": [0, 1, 2, 3], + "observed": [0, 1, 2, 3], + "scanned": 152, + "offenders": [], + "compliant": true, +}}}} + +test_compliant_cli_has_no_violations if { + violations := cli_exit_code_taxonomy.violations with input as compliant_input + count(violations) == 0 +} + +# The criterion, stated as a test: a command that exits outside {0,1,2,3} fails. +test_command_exiting_outside_the_taxonomy_is_rejected if { + i := json.patch(compliant_input, [{ + "op": "replace", + "path": "/core/cli/exitCodes/offenders", + "value": [{"file": "commands/rogue/rogue.command.ts", "code": 7, "snippet": "process.exit(7)"}], + }]) + violations := cli_exit_code_taxonomy.violations with input as i + violations[_].id == "CLI-EXIT-01" +} + +test_every_offender_is_named_individually if { + i := json.patch(compliant_input, [{ + "op": "replace", + "path": "/core/cli/exitCodes/offenders", + "value": [ + {"file": "commands/rogue/a.command.ts", "code": 7, "snippet": "process.exit(7)"}, + {"file": "commands/rogue/b.command.ts", "code": 64, "snippet": "process.exitCode = 64"}, + ], + }]) + violations := cli_exit_code_taxonomy.violations with input as i + count(violations) == 2 +} + +test_a_vacuous_scan_is_rejected if { + i := json.patch(compliant_input, [{"op": "replace", "path": "/core/cli/exitCodes/scanned", "value": 0}]) + violations := cli_exit_code_taxonomy.violations with input as i + violations[_].id == "CLI-EXIT-02" +} + +test_missing_facts_are_rejected_rather_than_read_as_a_pass if { + violations := cli_exit_code_taxonomy.violations with input as {"core": {"cli": {"mcpServerSource": "x"}}} + violations[_].id == "CLI-EXIT-02" +} + +# The mirror image, and the reason the rule above is scoped: `main.rego` +# aggregates this policy, so a satellite evaluation — which has no Core CLI tree +# and no fact document — must produce nothing at all rather than three blocking +# violations about a source tree it does not contain. +test_a_satellite_evaluation_is_silent if { + violations := cli_exit_code_taxonomy.violations with input as {"satellite": {"contracts": {"phase": 2}}} + count(violations) == 0 +} + +# The escape hatch: declaring 7 part of the taxonomy would silence CLI-EXIT-01. +# It must not silence the gate. +test_widening_the_taxonomy_is_rejected if { + i := json.patch(compliant_input, [{"op": "replace", "path": "/core/cli/exitCodes/declared", "value": [0, 1, 2, 3, 7]}]) + violations := cli_exit_code_taxonomy.violations with input as i + violations[_].id == "CLI-EXIT-03" +} diff --git a/src/rulesets/opa/main.rego b/src/rulesets/opa/main.rego index 98372778d..aedf947a3 100644 --- a/src/rulesets/opa/main.rego +++ b/src/rulesets/opa/main.rego @@ -8,6 +8,7 @@ import data.evolith.capability_source_interface.violations as csi_violations import data.evolith.ci_cd.violations as ci_cd_violations import data.evolith.cicd_quality_gates.violations as cicd_qg_violations import data.evolith.cli_core_parity.violations as cli_cp_violations +import data.evolith.cli_exit_code_taxonomy.violations as cli_exit_violations import data.evolith.cli_readiness.violations as cli_violations import data.evolith.cli_release_readiness.violations as cli_rr_violations import data.evolith.compliance_baseline.violations as cb_violations @@ -157,3 +158,9 @@ violations contains v if { violations contains v if { v := pg_violations[_] } + +# GT-580 — the exit-code taxonomy. Silent unless the caller declares +# `input.core.cli`, so a satellite evaluation is unaffected. +violations contains v if { + v := cli_exit_violations[_] +} diff --git a/src/rulesets/standards/iso-5055-mapping.csv b/src/rulesets/standards/iso-5055-mapping.csv index 720cf8c8f..8f3395975 100644 --- a/src/rulesets/standards/iso-5055-mapping.csv +++ b/src/rulesets/standards/iso-5055-mapping.csv @@ -146,6 +146,13 @@ CORE-0113-01,adr/generated/adr-0113-node-js-platform-lighthouse-apache-2-0-as-th CORE-0115-01,adr/generated/adr-0115-emergent-knowledge-axis-knowledge-originated-by-applying-the.rules.json,architecture-decision,,,none,no,documentation-only CORE-0116-01,adr/generated/adr-0116-canonical-finding-contract-and-an-executable-advisory-author.rules.json,architecture-decision,,,none,no,documentation-only CORE-0117-01,adr/generated/adr-0117-bilingual-parity-applies-to-authored-sources-not-generated-p.rules.json,architecture-decision,,,none,no,documentation-only +CORE-0118-01,adr/generated/adr-0118-the-core-hub-has-its-own-root-taxonomy-distinct-from-satelli.rules.json,architecture-decision,,,none,no,not-in-snapshot +CORE-0119-01,adr/generated/adr-0119-api-security-configuration-hardening.rules.json,architecture-decision,,,none,no,not-in-snapshot +CORE-0120-01,adr/generated/adr-0120-ssrf-prevention-standard.rules.json,architecture-decision,,,none,no,not-in-snapshot +CORE-0121-01,adr/generated/adr-0121-input-validation-and-sanitization-standard.rules.json,architecture-decision,,,none,no,not-in-snapshot +CORE-0122-01,adr/generated/adr-0122-shell-execution-safety-standard.rules.json,architecture-decision,,,none,no,not-in-snapshot +CORE-0123-01,adr/generated/adr-0123-timing-safe-comparison-standard.rules.json,architecture-decision,,,none,no,not-in-snapshot +CORE-0124-01,adr/generated/adr-0124-credential-and-secret-management-standard.rules.json,architecture-decision,,,none,no,not-in-snapshot AI-0001-01,adr/generated/adr-ai-augmented-0001-harness-engineering-for-ai-augmented-development.rules.json,architecture-decision,,,none,no,documentation-only AI-0002-01,adr/generated/adr-ai-augmented-0002-mcp-integration-protocol-for-agent-tool-invocation.rules.json,architecture-decision,,,none,no,documentation-only AI-0003-01,adr/generated/adr-ai-augmented-0003-model-selection-governance-for-ai-augmented-workflows.rules.json,architecture-decision,,,none,no,documentation-only @@ -189,6 +196,9 @@ CLI-PAR-01,cli/core-parity.rules.json,platform-surface,,,none,no,unimplemented-n CLI-PAR-02,cli/core-parity.rules.json,platform-surface,,,none,no,unimplemented-native CLI-PAR-03,cli/core-parity.rules.json,platform-surface,,,none,no,needs-runtime CLI-PAR-04,cli/core-parity.rules.json,platform-surface,,,none,no,unimplemented-native +CLI-EXIT-01,cli/exit-code-taxonomy.rules.json,platform-surface,,,none,no,not-in-snapshot +CLI-EXIT-02,cli/exit-code-taxonomy.rules.json,platform-surface,,,none,no,not-in-snapshot +CLI-EXIT-03,cli/exit-code-taxonomy.rules.json,platform-surface,,,none,no,not-in-snapshot CLI-RR-01,cli/release-readiness.rules.json,platform-surface,,,none,no,native-handler CLI-RR-02,cli/release-readiness.rules.json,platform-surface,,,none,no,native-handler CLI-RR-03,cli/release-readiness.rules.json,platform-surface,,,none,no,native-handler diff --git a/src/rulesets/standards/iso-5055-mapping.json b/src/rulesets/standards/iso-5055-mapping.json index 3efd2456a..e2ea3623e 100644 --- a/src/rulesets/standards/iso-5055-mapping.json +++ b/src/rulesets/standards/iso-5055-mapping.json @@ -12,24 +12,24 @@ "weaknessCount": 138 }, "corpus": { - "rulesetFiles": 167, - "rules": 381, + "rulesetFiles": 175, + "rules": 391, "note": "Files that carry gate definitions or topology recommendations rather than conformance rules contribute no rows: architecture/topology-recommendation.rules.json and sdlc/phase-gates.rules.json." }, "summary": { - "rules": 381, + "rules": 391, "mappedToIso5055": 37, "mappedDirect": 8, "mappedPartial": 29, - "noInternationalEquivalent": 344, - "adoptedFraction": 0.0971, + "noInternationalEquivalent": 354, + "adoptedFraction": 0.0946, "analyserAdoptable": 42, "analyserAdoptablePartial": 23, - "analyserAdoptableFraction": 0.1102, - "analyserAdoptableFractionIncludingPartial": 0.1706, + "analyserAdoptableFraction": 0.1074, + "analyserAdoptableFractionIncludingPartial": 0.1662, "byClass": { "architecture-decision": { - "rules": 155, + "rules": 162, "mapped": 12, "adoptable": 6 }, @@ -59,7 +59,7 @@ "adoptable": 0 }, "platform-surface": { - "rules": 14, + "rules": 17, "mapped": 0, "adoptable": 0 }, @@ -217,7 +217,7 @@ ] }, "not-in-snapshot": { - "rules": 2, + "rules": 12, "mappedToIso5055": 0, "analyserAdoptable": 0, "analyserAdoptablePartial": 0, @@ -3185,6 +3185,139 @@ "nativeEvaluability": "documentation-only", "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." }, + { + "ruleId": "CORE-0118-01", + "sourceFile": "adr/generated/adr-0118-the-core-hub-has-its-own-root-taxonomy-distinct-from-satelli.rules.json", + "title": "Conform to ADR-0118: The Core Hub Has Its Own Root Taxonomy, Distinct From Satellites", + "severity": "MUST", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, + { + "ruleId": "CORE-0119-01", + "sourceFile": "adr/generated/adr-0119-api-security-configuration-hardening.rules.json", + "title": "Conform to ADR-0119: API Security Configuration Hardening", + "severity": "MUST", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, + { + "ruleId": "CORE-0120-01", + "sourceFile": "adr/generated/adr-0120-ssrf-prevention-standard.rules.json", + "title": "Honor design decision in ADR-0120: SSRF Prevention Standard", + "severity": "SHOULD", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, + { + "ruleId": "CORE-0121-01", + "sourceFile": "adr/generated/adr-0121-input-validation-and-sanitization-standard.rules.json", + "title": "Honor design decision in ADR-0121: Input Validation and Sanitization Standard", + "severity": "SHOULD", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, + { + "ruleId": "CORE-0122-01", + "sourceFile": "adr/generated/adr-0122-shell-execution-safety-standard.rules.json", + "title": "Conform to ADR-0122: Shell Execution Safety Standard", + "severity": "MUST", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, + { + "ruleId": "CORE-0123-01", + "sourceFile": "adr/generated/adr-0123-timing-safe-comparison-standard.rules.json", + "title": "Conform to ADR-0123: Timing-Safe Comparison Standard", + "severity": "MUST", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, + { + "ruleId": "CORE-0124-01", + "sourceFile": "adr/generated/adr-0124-credential-and-secret-management-standard.rules.json", + "title": "Conform to ADR-0124: Credential and Secret Management Standard", + "severity": "MUST", + "ruleClass": "architecture-decision", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "Conformance to a recorded architecture decision. ISO/IEC 5055 measures source structure, not whether a decision was honoured." + }, { "ruleId": "AI-0001-01", "sourceFile": "adr/generated/adr-ai-augmented-0001-harness-engineering-for-ai-augmented-development.rules.json", @@ -4017,6 +4150,63 @@ "nativeEvaluability": "unimplemented-native", "note": "A CLI / MCP surface contract. Specific to this product; no standards analyser models it." }, + { + "ruleId": "CLI-EXIT-01", + "sourceFile": "cli/exit-code-taxonomy.rules.json", + "title": "No CLI Command May Exit Outside The Published Taxonomy", + "severity": "MUST", + "ruleClass": "platform-surface", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "A CLI / MCP surface contract. Specific to this product; no standards analyser models it." + }, + { + "ruleId": "CLI-EXIT-02", + "sourceFile": "cli/exit-code-taxonomy.rules.json", + "title": "The Taxonomy Scan Must Be Non-Vacuous", + "severity": "MUST", + "ruleClass": "platform-surface", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "A CLI / MCP surface contract. Specific to this product; no standards analyser models it." + }, + { + "ruleId": "CLI-EXIT-03", + "sourceFile": "cli/exit-code-taxonomy.rules.json", + "title": "The Published Taxonomy Must Not Be Widened", + "severity": "MUST", + "ruleClass": "platform-surface", + "iso5055": { + "cwes": [], + "weaknesses": [], + "measures": [], + "strength": "none" + }, + "analyser": { + "adoptable": "no", + "examples": [] + }, + "nativeEvaluability": "not-in-snapshot", + "note": "A CLI / MCP surface contract. Specific to this product; no standards analyser models it." + }, { "ruleId": "CLI-RR-01", "sourceFile": "cli/release-readiness.rules.json", diff --git a/src/sdk/cli/src/infrastructure/cli/exit-code-taxonomy-ruleset.spec.ts b/src/sdk/cli/src/infrastructure/cli/exit-code-taxonomy-ruleset.spec.ts new file mode 100644 index 000000000..058f46975 --- /dev/null +++ b/src/sdk/cli/src/infrastructure/cli/exit-code-taxonomy-ruleset.spec.ts @@ -0,0 +1,215 @@ +/** + * GT-580 criterion 3, end to end — the RULESET fails a command that exits + * outside the taxonomy. + * + * Three artifacts have to agree for the criterion to be met, and each one alone + * proves nothing: + * + * src/rulesets/cli/exit-code-taxonomy.rules.json the authored rule + * src/rulesets/opa/cli-exit-code-taxonomy.rego its Rego parity + * src/sdk/cli/scripts/exit-code-taxonomy-facts.mjs the facts it decides over + * + * `opa test` already exercises the policy against hand-written fact documents, + * and `CliExitTaxonomyRuleHandler` already scans a tree. Neither answers the + * question this spec asks: does a REAL out-of-taxonomy exit, in a REAL file on + * disk, travel through the REAL producer and come out of the REAL policy as a + * blocking violation? A hand-written fixture cannot answer that — it asserts + * that the policy handles the input somebody imagined the producer emits. + * + * So each case below plants a scratch CLI tree, runs the committed producer over + * it, and feeds the result to the committed `.rego` through the pinned OPA + * binary. The `rogue command` case is the criterion stated literally. + * + * WHY A MISSING OPA IS RED, NOT SKIPPED. The binary is resolved through the + * repository's own `ensureOpa`, which fetches and pins it — the same path every + * OPA gate in CI takes. A spec that skipped itself when the runtime were absent + * would be green on exactly the machine where nothing was verified, which is the + * failure mode this board keeps finding. + */ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; + +const CLI_ROOT = path.resolve(__dirname, '..', '..', '..'); +const REPO_ROOT = path.resolve(CLI_ROOT, '..', '..', '..'); +const FACTS_SCRIPT = path.join(CLI_ROOT, 'scripts', 'exit-code-taxonomy-facts.mjs'); +const POLICY = path.join(REPO_ROOT, 'src', 'rulesets', 'opa', 'cli-exit-code-taxonomy.rego'); +const OPA_RUNTIME = path.join(REPO_ROOT, '.harness', 'scripts', 'opa-runtime.mjs'); +const CLI_SRC = path.join(CLI_ROOT, 'src'); + +/** The published taxonomy, as the CLI's own `exit-codes.ts` declares it. */ +const TAXONOMY_TS = [ + 'export const CLI_EXIT_CODES = {', + ' OK: 0,', + ' TOOL_FAILURE: 1,', + ' BLOCKED: 2,', + ' INVALID_INPUT: 3,', + '} as const;', + '', +].join('\n'); + +// A possible one-off OPA download on a cold machine. +jest.setTimeout(300_000); + +interface Violation { + id: string; + message: string; +} + +/** + * Resolve the pinned OPA binary through the repository's own runtime helper. + * + * Done in a subprocess because `opa-runtime.mjs` is ESM and this suite is + * transpiled to CJS; shelling out keeps the helper as the single source of the + * pinned version instead of hard-coding a path that would drift from it. + */ +function resolveOpaBinary(): string { + const probe = spawnSync( + process.execPath, + [ + '--input-type=module', + '-e', + `import { ensureOpa } from ${JSON.stringify(OPA_RUNTIME)};` + + `const opa = await ensureOpa(${JSON.stringify(REPO_ROOT)});` + + `process.stdout.write(opa.binary);`, + ], + { encoding: 'utf8', cwd: REPO_ROOT }, + ); + if (probe.status !== 0) { + throw new Error(`could not resolve the pinned OPA binary:\n${probe.stderr}`); + } + return probe.stdout.trim(); +} + +/** Run the committed fact producer over a source tree. */ +function collectFacts(root: string): Record { + const probe = spawnSync( + process.execPath, + [ + '--input-type=module', + '-e', + `import { collectExitCodeFacts } from ${JSON.stringify(FACTS_SCRIPT)};` + + `process.stdout.write(JSON.stringify(collectExitCodeFacts(${JSON.stringify(root)})));`, + ], + { encoding: 'utf8' }, + ); + if (probe.status !== 0) throw new Error(`fact producer failed:\n${probe.stderr}`); + return JSON.parse(probe.stdout) as Record; +} + +/** Ask the committed policy for its verdict on those facts. */ +function evaluatePolicy(opa: string, facts: Record, workdir: string): Violation[] { + const inputFile = path.join(workdir, 'input.json'); + fs.writeFileSync(inputFile, JSON.stringify({ core: { cli: { exitCodes: facts } } })); + + const run = spawnSync( + opa, + [ + 'eval', + '--format', 'json', + '--data', POLICY, + '--input', inputFile, + 'data.evolith.cli_exit_code_taxonomy.violations', + ], + { encoding: 'utf8' }, + ); + if (run.status !== 0) throw new Error(`opa refused the policy:\n${run.stderr}`); + + const parsed = JSON.parse(run.stdout) as { + result?: Array<{ expressions?: Array<{ value?: Violation[] }> }>; + }; + return parsed.result?.[0]?.expressions?.[0]?.value ?? []; +} + +describe('GT-580 · the exit-code ruleset, from a real source tree to a Rego verdict', () => { + let opa: string; + let workdir: string; + + beforeAll(() => { + opa = resolveOpaBinary(); + expect(fs.existsSync(POLICY)).toBe(true); + expect(fs.existsSync(FACTS_SCRIPT)).toBe(true); + workdir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gt580-ruleset-'))); + }); + + afterAll(() => { + fs.rmSync(workdir, { recursive: true, force: true }); + }); + + /** A scratch CLI tree with a declared taxonomy and whatever else is asked for. */ + function plant(name: string, sources: Record): string { + const root = path.join(workdir, name); + const all = { + 'infrastructure/cli/exit-codes.ts': TAXONOMY_TS, + ...sources, + }; + for (const [rel, contents] of Object.entries(all)) { + const full = path.join(root, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, contents); + } + return root; + } + + it('clears the REAL CLI source tree, over a non-empty scan', () => { + const facts = collectFacts(CLI_SRC); + + // The denominator first: a scan that read nothing also finds no offenders. + expect(facts.scanned as number).toBeGreaterThan(80); + expect(evaluatePolicy(opa, facts, workdir)).toEqual([]); + }); + + it('FAILS on a command that exits outside the taxonomy', () => { + // The criterion, stated as an experiment: force an out-of-taxonomy exit in a + // scratch fixture and require the ruleset to go red. + const root = plant('rogue', { + 'commands/rogue/rogue.command.ts': 'export function boom(): void {\n process.exit(7);\n}\n', + }); + + const violations = evaluatePolicy(opa, collectFacts(root), workdir); + + expect(violations.map((v) => v.id)).toEqual(['CLI-EXIT-01']); + expect(violations[0].message).toContain('commands/rogue/rogue.command.ts'); + expect(violations[0].message).toContain('process.exit(7)'); + }); + + it('FAILS on `process.exitCode = 64` as well — the other spelling', () => { + const root = plant('rogue-assignment', { + 'commands/rogue/other.command.ts': 'process.exitCode = 64;\n', + }); + + const violations = evaluatePolicy(opa, collectFacts(root), workdir); + expect(violations.map((v) => v.id)).toEqual(['CLI-EXIT-01']); + expect(violations[0].message).toContain('process.exitCode = 64'); + }); + + it('does NOT let the taxonomy be widened to silence the offender', () => { + // The cheapest way to make the violation above disappear is to declare 7 + // legal. CLI-EXIT-03 is the rule that stops that from counting as a fix, so + // the tree that tries it fails on BOTH counts. + const root = plant('widened', { + 'commands/rogue/rogue.command.ts': 'process.exit(7);\n', + 'infrastructure/cli/exit-codes.ts': TAXONOMY_TS.replace('} as const;', ' ROGUE: 7,\n} as const;'), + }); + + const facts = collectFacts(root); + // The producer reads the taxonomy from its own module, so the widening is + // injected here to exercise the policy rule rather than the producer's + // hard-coded mirror of the CLI constant. + (facts as { declared: number[] }).declared = [0, 1, 2, 3, 7]; + + const ids = evaluatePolicy(opa, facts, workdir).map((v) => v.id).sort(); + expect(ids).toEqual(['CLI-EXIT-01', 'CLI-EXIT-03']); + }); + + it('refuses a vacuous scan rather than reporting it as compliance', () => { + const root = path.join(workdir, 'empty'); + fs.mkdirSync(root, { recursive: true }); + fs.writeFileSync(path.join(root, 'README.md'), '# no sources here'); + + const facts = collectFacts(root); + expect(facts.scanned).toBe(0); + expect(evaluatePolicy(opa, facts, workdir).map((v) => v.id)).toEqual(['CLI-EXIT-02']); + }); +}); diff --git a/src/sdk/cli/src/infrastructure/cli/machine-channel.registry-sweep.spec.ts b/src/sdk/cli/src/infrastructure/cli/machine-channel.registry-sweep.spec.ts new file mode 100644 index 000000000..43191329c --- /dev/null +++ b/src/sdk/cli/src/infrastructure/cli/machine-channel.registry-sweep.spec.ts @@ -0,0 +1,245 @@ +/** + * GT-580 criterion 2 — `--format json` writes DATA ONLY to stdout, for EVERY + * command that offers the flag. + * + * WHY A SWEEP AND NOT A SAMPLE. `machine-channel.subprocess.spec.ts` already + * pipes two hand-picked commands (`scaffold`, `init`) and parses their stdout. + * That proves the guard works where it was pointed; it says nothing about the + * thirty other commands that accept `--format json`, and nothing at all about + * the command somebody adds next week. A machine consumer does not pipe the two + * commands we happened to test — it pipes whichever one it needs, and one stray + * progress line makes `JSON.parse` throw on the first byte while the command + * still exits 0. + * + * WHY THE ENUMERATION COMES FROM THE REGISTRY. A hand-written list of commands + * is a second source of truth that silently stops covering the CLI the moment a + * command is added. This spec asks commander — the CLI's own registry, built + * from the real `AppModule` — which commands declare a `--format` option, and + * sweeps exactly those. A new `--format json` command is therefore in the sweep + * on the day it is registered, without anybody remembering to add it. + * + * WHY THE DENOMINATOR IS ASSERTED. A sweep over an empty enumeration passes and + * proves nothing — this board has repeatedly caught guards that had never once + * been observed to fail. If the registry walk ever returns nothing (a changed + * commander internal, a broken module graph), the floor assertion turns this + * spec RED instead of letting it report a vacuous green. + * + * WHAT COUNTS AS CONFORMANCE. For each command: stdout must be exactly one + * parseable JSON document — not "mostly JSON", not "JSON after a banner" — and + * the exit status must be a member of the published taxonomy. Most invocations + * here land on a FAILURE path (no satellite, no TTY, no required flag), which is + * deliberate: the failure path is where prose leaks, and it is the path an agent + * harness most needs to parse. + * + * OBSERVED RED. Commenting out `installMachineChannelGuard()` in `main.ts` and + * re-running this sweep on 2026-07-29 turned `agents` (`┌ Evolith …`, a + * `@clack/prompts` banner) and `init-wizard` (`Starting Evolith …`) DIRTY while + * the other thirty stayed clean. The guard is what makes this spec green, and + * removing it is caught. + */ +import { spawn } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { CommandFactory } from 'nest-commander'; +import { Commander } from 'nest-commander/src/constants'; +import { CLI_EXIT_CODE_VALUES } from './exit-codes'; + +const CLI_ROOT = path.resolve(__dirname, '..', '..', '..'); +const REPO_ROOT = path.resolve(CLI_ROOT, '..', '..', '..'); +const TS_NODE_BIN = path.join(REPO_ROOT, 'node_modules', 'ts-node', 'dist', 'bin.js'); +const CLI_ENTRY = path.join(CLI_ROOT, 'src', 'main.ts'); +const TS_CONFIG = path.join(CLI_ROOT, 'tsconfig.test.json'); + +/** + * The floor the enumeration may not fall through. + * + * 32 commands declared `--format` when this spec was written. The floor sits + * just below so that adding commands never breaks the spec, while losing the + * enumeration entirely — the failure mode that would make the sweep vacuous — + * fails it immediately. + */ +const MIN_FORMAT_COMMANDS = 28; + +/** Six cold ts-node boots at a time; the whole sweep is ~30 process starts. */ +const CONCURRENCY = 6; +jest.setTimeout(900_000); + +/** + * Positional arguments for the commands whose `` is required. + * + * These exist so the invocation reaches the command's own code instead of + * stopping at commander's arity check. The values are read-only actions on + * purpose: the sweep must not mutate anything, and a command that refuses the + * argument is still a valid observation — its refusal must be a JSON document. + */ +const REQUIRED_ARGS: Readonly> = { + 'patterns get': ['no-such-pattern'], + 'patterns for-topology': ['modular-monolith'], + gate: ['status'], + phase: ['status'], + profile: ['show'], + enforce: ['status'], +}; + +interface RegisteredCommand { + /** Space-joined path as typed, e.g. `topology recommend`. */ + readonly path: string; + readonly formatFlags: string; +} + +/** + * Ask the CLI's own commander registry which commands accept `--format`. + * + * Built from the real `AppModule`, so this is the same graph `main.ts` boots — + * not a directory listing of `commands/`, which would also count files that + * register nothing. + */ +async function enumerateFormatCommands(): Promise { + const { AppModule } = await import('../../app.module'); + const app = await CommandFactory.createWithoutRunning(AppModule, { logger: false }); + try { + // Structural access: the root workspace hoists a different major of + // `commander` than nest-commander runs on, so its typings are not the + // contract here (the same reasoning as `applyProgramName` in main.ts). + const program = app.get(Commander, { strict: false }); + const found: RegisteredCommand[] = []; + + const walk = (command: CommanderLike, prefix: readonly string[]): void => { + const trail = [...prefix, command.name()]; + const format = (command.options ?? []).find((option) => option.long === '--format'); + if (format) found.push({ path: trail.join(' '), formatFlags: format.flags }); + for (const child of command.commands ?? []) walk(child, trail); + }; + + for (const child of program.commands ?? []) walk(child, []); + return found; + } finally { + await app.close(); + } +} + +interface CommanderLike { + name(): string; + readonly options?: ReadonlyArray<{ long?: string; flags: string }>; + readonly commands?: readonly CommanderLike[]; +} + +interface CliRun { + readonly command: string; + readonly stdout: string; + readonly stderr: string; + readonly status: number | null; +} + +function runCli(command: RegisteredCommand): Promise { + return new Promise((resolve, reject) => { + // A fresh empty cwd per command: none of these may depend on, or touch, the + // repository they are being tested from. + const cwd = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gt580-sweep-'))); + // The jest setup sets EVOLITH_FORCE_INTERACTIVE for in-process specs. A + // piped subprocess is the machine case by definition and must not inherit it. + const env: NodeJS.ProcessEnv = { ...process.env, NO_COLOR: '1' }; + delete env.EVOLITH_FORCE_INTERACTIVE; + + const argv = [ + TS_NODE_BIN, + '--transpile-only', + '--project', + TS_CONFIG, + CLI_ENTRY, + ...command.path.split(' '), + ...(REQUIRED_ARGS[command.path] ?? []), + '--format', + 'json', + ]; + + // `pipe` on both is the whole point: the two channels must be observed apart. + const child = spawn(process.execPath, argv, { cwd, stdio: ['ignore', 'pipe', 'pipe'], env }); + let stdout = ''; + let stderr = ''; + child.stdout.setEncoding('utf8'); + child.stderr.setEncoding('utf8'); + child.stdout.on('data', (chunk: string) => (stdout += chunk)); + child.stderr.on('data', (chunk: string) => (stderr += chunk)); + child.on('error', reject); + child.on('close', (status) => { + fs.rmSync(cwd, { recursive: true, force: true }); + resolve({ command: command.path, stdout, stderr, status }); + }); + }); +} + +async function runAll(commands: readonly RegisteredCommand[]): Promise { + const pending = [...commands]; + const runs: CliRun[] = []; + const worker = async (): Promise => { + for (let next = pending.shift(); next; next = pending.shift()) { + runs.push(await runCli(next)); + } + }; + await Promise.all(Array.from({ length: CONCURRENCY }, worker)); + return runs; +} + +/** The classification, stated once so both assertions read the same way. */ +function classify(run: CliRun): string { + const text = run.stdout.trim(); + if (text.length === 0) return 'EMPTY (no document for a machine consumer to read)'; + try { + const parsed: unknown = JSON.parse(text); + if (parsed === null || typeof parsed !== 'object') return 'NOT-A-DOCUMENT (bare scalar)'; + return 'JSON'; + } catch (error) { + const preview = text.slice(0, 60).replace(/\n/g, '\\n'); + return `DIRTY (${(error as Error).message}) first bytes: ${preview}`; + } +} + +describe('GT-580 · every `--format json` command in the registry pipes cleanly', () => { + let commands: RegisteredCommand[]; + let runs: CliRun[]; + + beforeAll(async () => { + // A missing runner must be a RED test, never a skipped one. + expect(fs.existsSync(TS_NODE_BIN)).toBe(true); + commands = await enumerateFormatCommands(); + runs = await runAll(commands); + }); + + it('enumerates the `--format` commands from the CLI registry, and finds many', () => { + // Guards the denominator: everything below is vacuous if this is empty. + expect(commands.length).toBeGreaterThanOrEqual(MIN_FORMAT_COMMANDS); + // Spot-check the walk actually descends into sub-commands rather than + // listing only top-level ones. + const paths = commands.map((c) => c.path); + expect(paths).toEqual(expect.arrayContaining(['validate', 'evaluate', 'topology recommend', 'patterns list'])); + expect(runs).toHaveLength(commands.length); + }); + + it('writes exactly one JSON document to stdout, for every one of them', () => { + const offenders = runs + .map((run) => ({ command: run.command, verdict: classify(run) })) + .filter((entry) => entry.verdict !== 'JSON'); + + // Reported as a list so a regression names every command it broke, not just + // the first one jest happened to reach. + expect(offenders).toEqual([]); + }); + + it('exits from the published taxonomy, for every one of them', () => { + const offenders = runs + .filter((run) => !CLI_EXIT_CODE_VALUES.includes(run.status ?? -1)) + .map((run) => ({ command: run.command, status: run.status })); + + expect(offenders).toEqual([]); + }); + + it('keeps diagnostics on stderr rather than deleting them', () => { + // The guard must be a channel rule, not a blunt silencer: the cheap "fix" + // for a dirty stdout is to stop printing, and that would pass the two + // assertions above while destroying every diagnostic a human needs. + const withDiagnostics = runs.filter((run) => run.stderr.trim().length > 0); + expect(withDiagnostics.length).toBeGreaterThan(0); + }); +}); From 642ca033da691fb60dbacd5fedf333f6a502d9eb Mon Sep 17 00:00:00 2001 From: aarroyo Date: Thu, 30 Jul 2026 14:39:44 -0500 Subject: [PATCH 5/6] =?UTF-8?q?feat(ci):=20one=20gap,=20one=20claim=20?= =?UTF-8?q?=E2=80=94=20the=20claim=20guard,=20advancing=20GT-639?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A board row carries a STATUS but not who is working it or where, so two sessions can both read `PENDING` and both start, correctly. On 2026-07-30 that happened three times in a day. `50-validate-gap-claim` derives the claim set from OPEN PULL REQUESTS — every `GT-*` in a PR's title, body or branch name — and fails when one id is claimed by two of them, naming both with their branches and titles. DERIVED, NEVER HAND-WRITTEN, and the row asked for that on purpose: a hand-written claim goes stale in the direction that matters, because someone forgets to remove it and the next session works around a claim nobody holds. THREE CRITERIA OF FOUR, and the fourth is left open with its reason rather than stretched to fit. A claim list committed to the board would be derived from live GitHub state, so it would be stale the moment anyone opened a pull request — and GT-630's chain exists precisely to insist derived artifacts reach a fixed point. A perpetually-stale artifact inside that chain would be worse than none. Making the claim discoverable from the board needs something the board can render without committing, and that is not built. The row stays IN-PROGRESS. What it cannot see is in its own failure text, not implied: a branch with no open pull request claims nothing. Opening the PR early — draft is enough — is what makes the claim visible, which turns a convention into something with a check behind it. Anti-vacuous: a pull-request query that cannot be answered is a hard failure, not a quiet pass. A guard built because two sessions could not see each other must not stay silent when it cannot look. Zero open PRs is reported in words, because it looks identical to a broken query otherwise. Verified: 10/10 fixtures; 42-validate-guard-denominators 62 guards classified; 43-validate-guard-negative-fixtures 39/39 observed red, up from 38. Run against the real repository it reports 0 claims across 5 open PRs — checked rather than assumed: all five are Dependabot bumps naming no gap, so the zero is real and not an empty query. 08-validate-tracking (640 gaps), 01, 04, 41 within budget, 46 chain at a fixed point. Counters: 600/640, in-progress 14 -> 15, pending 22 -> 21. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci-cd.yml | 11 + .harness/scripts/ci/50-validate-gap-claim.mjs | 199 ++++++++++++++++++ .../scripts/ci/50-validate-gap-claim.test.mjs | 136 ++++++++++++ .harness/scripts/lib/guard-classification.mjs | 9 + .../gaps/gap-reference-catalog.es.md | 8 +- .../gaps/gap-reference-catalog.md | 8 +- .../control-center/gaps/gap-tracking.es.md | 4 +- .../core/control-center/gaps/gap-tracking.md | 4 +- .../maturity-reconciliation.json | 4 +- 9 files changed, 369 insertions(+), 14 deletions(-) create mode 100644 .harness/scripts/ci/50-validate-gap-claim.mjs create mode 100644 .harness/scripts/ci/50-validate-gap-claim.test.mjs diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 4afbc7151..0f96ad618 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -456,11 +456,22 @@ jobs: git fetch --no-tags --depth=1 origin "${{ github.base_ref || github.event.repository.default_branch }}" node .harness/scripts/ci/49-validate-gap-id-allocation.mjs --verbose + # GT-639: a board row carries a STATUS but not who is working it or where, so + # two sessions can both read `PENDING` and both start. On 2026-07-30 the same + # work was done twice, three times over. The claim set is DERIVED from open + # pull requests — never hand-written, which would go stale in the direction + # that matters — and an id claimed by two open PRs fails here, naming both. + - name: One gap, one claim + env: + GH_TOKEN: ${{ github.token }} + run: node .harness/scripts/ci/50-validate-gap-claim.mjs + - name: Self-tests for the governance guards run: | node --test .harness/scripts/ci/42-validate-guard-denominators.test.mjs node --test .harness/scripts/ci/48-validate-security-publish-lag.test.mjs node --test .harness/scripts/ci/49-validate-gap-id-allocation.test.mjs + node --test .harness/scripts/ci/50-validate-gap-claim.test.mjs node --test .harness/scripts/ci/41-validate-evidence-commands.test.mjs node --test .harness/scripts/ci/43-validate-guard-negative-fixtures.test.mjs node --test .harness/scripts/ci/44-validate-adr-implementation-status.test.mjs diff --git a/.harness/scripts/ci/50-validate-gap-claim.mjs b/.harness/scripts/ci/50-validate-gap-claim.mjs new file mode 100644 index 000000000..744eced86 --- /dev/null +++ b/.harness/scripts/ci/50-validate-gap-claim.mjs @@ -0,0 +1,199 @@ +#!/usr/bin/env node + +/** + * GT-639 — two sessions must not work the same gap without knowing. + * + * ## The defect + * + * On 2026-07-30 the same work was done twice, three separate times, by sessions + * that could not see each other: GT-640 (then GT-633) was fixed by a standalone + * capture script on `main` and by an independent in-spec renderer on `develop`; + * the evidence guard's parser was fixed on both branches; and a GT id was + * allocated twice. The cost was not the merge conflict — it was the duplicated + * work before anyone reached it, plus a reconciliation that introduced a defect + * of its own. + * + * A board row carries a STATUS but not who is working it or where. Two sessions + * can both read `GT-640 · PENDING` and both start, correctly. + * + * ## What it checks + * + * The claim set is DERIVED from open pull requests — never hand-written, because + * a hand-written claim is stale in the direction that matters (someone forgets to + * remove it, and the next session works around a claim nobody holds). Every open + * PR claims the `GT-*` ids that appear in its title, its body or its branch name. + * + * An id claimed by MORE THAN ONE open pull request is a contested claim, and this + * guard fails naming both claimants. + * + * ## What it deliberately does NOT do + * + * It does not write a committed claim list. Such a file would be derived from + * live GitHub state, so it would be stale the moment anyone opened a PR, and the + * repository already has a guard (46) whose whole premise is that derived + * artifacts must reach a fixed point. A perpetually-stale artifact in that chain + * would be worse than none. The live view is this check's output, on the PR where + * it matters. + * + * It also cannot see work that has no open PR. A branch someone is working on + * privately claims nothing, and the guard says so rather than implying coverage. + * + * ## Anti-vacuous pass + * + * Zero open pull requests examined is reported, not silently passed: with no PRs + * there is nothing to contest, and a run that found none because the query broke + * looks identical unless it says which happened. A query that fails outright is a + * hard failure — unable to answer is not the same as nothing to report. + * + * USAGE + * node .harness/scripts/ci/50-validate-gap-claim.mjs + * node .harness/scripts/ci/50-validate-gap-claim.mjs --json + * node .harness/scripts/ci/50-validate-gap-claim.mjs --fixture # offline + * + * EXIT CODES + * 0 no id is claimed by more than one open pull request + * 1 a contested claim, or the pull-request query could not be answered + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { fileURLToPath } from 'node:url'; + +const GUARD = '50-validate-gap-claim'; + +// --------------------------------------------------------------------------- +// Pure core — takes pull requests as data, returns the claim map. No network. +// --------------------------------------------------------------------------- + +/** Every `GT-NNN` mentioned in a pull request's title, body or branch name. */ +export function claimedIds(pr) { + const haystack = `${pr.title ?? ''}\n${pr.body ?? ''}\n${pr.headRefName ?? ''}`; + return [...new Set(haystack.match(/GT-\d+/g) ?? [])].sort(); +} + +/** + * id -> the open pull requests claiming it. + * + * @param {Array<{number:number,title?:string,body?:string,headRefName?:string,url?:string}>} prs + * @returns {Map>} + */ +export function buildClaimMap(prs) { + const map = new Map(); + for (const pr of prs) { + for (const id of claimedIds(pr)) { + if (!map.has(id)) map.set(id, []); + map.get(id).push(pr); + } + } + return map; +} + +/** Ids claimed by more than one open pull request. */ +export function findContested(claimMap) { + return [...claimMap.entries()] + .filter(([, prs]) => prs.length > 1) + .map(([id, prs]) => ({ id, prs })) + .sort((a, b) => a.id.localeCompare(b.id)); +} + +// --------------------------------------------------------------------------- +// I/O edges +// --------------------------------------------------------------------------- + +function fail(lines) { + console.error(`\n✗ ${GUARD}: ${lines[0]}`); + for (const l of lines.slice(1)) console.error(` ${l}`); + process.exit(1); +} + +/** Open pull requests, from `gh`. Returns null when the query cannot be answered. */ +export function fetchOpenPullRequests() { + try { + const raw = execFileSync( + 'gh', + ['pr', 'list', '--state', 'open', '--limit', '200', '--json', 'number,title,body,headRefName,url'], + { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'] }, + ); + const parsed = JSON.parse(raw); + return Array.isArray(parsed) ? parsed : null; + } catch { + return null; + } +} + +function main(argv) { + const asJson = argv.includes('--json'); + const fixtureIdx = argv.indexOf('--fixture'); + + const prs = fixtureIdx !== -1 + ? JSON.parse(fs.readFileSync(path.resolve(process.cwd(), argv[fixtureIdx + 1]), 'utf8')) + : fetchOpenPullRequests(); + + if (prs === null) { + fail([ + 'could not read the open pull requests, so no claim was checked.', + ' `gh pr list` failed — in CI that usually means GH_TOKEN is not set on the step.', + '', + ' Unable to answer is not the same as nothing to report. This guard exists', + ' because two sessions could not see each other; a version of it that stays', + ' quiet when it cannot look would reproduce exactly that.', + ]); + } + + const claimMap = buildClaimMap(prs); + const contested = findContested(claimMap); + + if (asJson) { + process.stdout.write(JSON.stringify({ + pullRequests: prs.length, + claims: Object.fromEntries([...claimMap].map(([id, list]) => [id, list.map((p) => p.number)])), + contested: contested.map((c) => ({ id: c.id, prs: c.prs.map((p) => p.number) })), + }) + '\n'); + return contested.length > 0 ? 1 : 0; + } + + console.log(`${GUARD} — one gap, one claim`); + console.log(` open pull requests .. ${prs.length}`); + console.log(` ids claimed ......... ${claimMap.size}`); + console.log(` contested ........... ${contested.length}`); + + if (prs.length === 0) { + // Said out loud: with no open PRs there is nothing to contest, which is a + // legitimate state and looks identical to a broken query unless named. + console.log('\n No pull request is open, so nothing is claimed and nothing can be contested.'); + } + + for (const [id, list] of [...claimMap].sort()) { + if (list.length === 1) console.log(` · ${id} — #${list[0].number} ${list[0].headRefName ?? ''}`); + } + + if (contested.length > 0) { + fail([ + `${contested.length} gap id(s) are claimed by more than one open pull request:`, + ...contested.flatMap((c) => [ + ` • ${c.id}`, + ...c.prs.map((p) => ` #${p.number} ${p.headRefName ?? '?'} ${p.title ?? ''}`), + ]), + '', + ' Two sessions are working the same gap. That is not a merge conflict yet, and', + ' it is much cheaper to resolve now than after both have landed: on 2026-07-30', + ' the same fix was written twice and a third session spent a full reconciliation', + ' collapsing them (GT-639).', + '', + ' Decide which pull request owns the id, and say so in the other one.', + '', + ' NOT COVERED by this check: a branch with no open pull request claims nothing.', + ' Opening the PR early — draft is enough — is what makes the claim visible.', + ]); + } + + console.log( + `\n✓ ${GUARD}: ${claimMap.size} claimed id(s) across ${prs.length} open pull request(s), none contested.`, + ); + return 0; +} + +const invokedDirectly = + process.argv[1] && path.resolve(process.argv[1]) === path.resolve(fileURLToPath(import.meta.url)); +if (invokedDirectly) process.exit(main(process.argv.slice(2))); diff --git a/.harness/scripts/ci/50-validate-gap-claim.test.mjs b/.harness/scripts/ci/50-validate-gap-claim.test.mjs new file mode 100644 index 000000000..afa9c020c --- /dev/null +++ b/.harness/scripts/ci/50-validate-gap-claim.test.mjs @@ -0,0 +1,136 @@ +#!/usr/bin/env node + +/** + * GT-639 — fixtures for the gap-claim guard. + * + * The case that matters is the one this repository lived through: two open pull + * requests working the same gap, neither aware of the other. If that fixture ever + * goes green the guard is decoration — which is why GT-639's last criterion asks + * for a fixture OBSERVED red rather than a guard someone believes works. + * + * The pull requests are supplied as data through `--fixture`, so nothing here + * touches the network: the guard's core is pure by construction and the I/O edge + * is one function. + */ + +import { describe, it, before, after } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { spawnSync } from 'node:child_process'; + +import { claimedIds, buildClaimMap, findContested } from './50-validate-gap-claim.mjs'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const GUARD = resolve(__dirname, '50-validate-gap-claim.mjs'); + +let sandbox; +before(() => { sandbox = mkdtempSync(join(tmpdir(), 'gt639-')); }); +after(() => { if (sandbox) rmSync(sandbox, { recursive: true, force: true }); }); + +const runWith = (prs, extra = []) => { + const file = join(sandbox, `prs-${Math.abs(JSON.stringify(prs).length)}-${prs.length}.json`); + writeFileSync(file, JSON.stringify(prs)); + const r = spawnSync(process.execPath, [GUARD, '--fixture', file, ...extra], { + encoding: 'utf8', timeout: 60000, + }); + return { status: r.status, out: `${r.stdout}\n${r.stderr}` }; +}; + +describe('claimedIds', () => { + it('reads a claim from the title, the body or the branch name', () => { + assert.deepEqual(claimedIds({ title: 'fix(x): close GT-100' }), ['GT-100']); + assert.deepEqual(claimedIds({ body: 'refs GT-101 and GT-102' }), ['GT-101', 'GT-102']); + assert.deepEqual(claimedIds({ headRefName: 'feat/gt-103-thing' }), []); // lowercase is not an id + assert.deepEqual(claimedIds({ headRefName: 'docs/GT-104-rows' }), ['GT-104']); + }); + + it('does not claim the same id twice from one pull request', () => { + assert.deepEqual(claimedIds({ title: 'GT-105', body: 'GT-105 again', headRefName: 'x/GT-105' }), ['GT-105']); + }); + + it('claims nothing when a pull request names no gap', () => { + assert.deepEqual(claimedIds({ title: 'chore: tidy', body: '', headRefName: 'chore/tidy' }), []); + }); +}); + +describe('findContested', () => { + it('THE CASE: one id, two open pull requests', () => { + const contested = findContested(buildClaimMap([ + { number: 1, title: 'fix: GT-200 via a capture script' }, + { number: 2, title: 'fix: GT-200 via an in-spec renderer' }, + { number: 3, title: 'docs: GT-201' }, + ])); + assert.equal(contested.length, 1); + assert.equal(contested[0].id, 'GT-200'); + assert.deepEqual(contested[0].prs.map((p) => p.number), [1, 2]); + }); + + it('one pull request claiming many ids is not contested', () => { + assert.deepEqual(findContested(buildClaimMap([{ number: 1, title: 'GT-300 GT-301 GT-302' }])), []); + }); +}); + +describe('the guard end to end', () => { + it('THE FIXTURE: two open PRs on one gap — RED, naming both', () => { + const { status, out } = runWith([ + { number: 276, title: 'fix(standards): capture script', headRefName: 'claude/fervent', body: 'closes GT-640' }, + { number: 279, title: 'fix(standards): in-spec renderer for GT-640', headRefName: 'claude/nice-cohen', body: '' }, + ]); + assert.equal(status, 1, out); + assert.match(out, /1 gap id\(s\) are claimed by more than one open pull request/); + assert.match(out, /#276/); + assert.match(out, /#279/); + // It must say why this is worth stopping for, and what to do. + assert.match(out, /much cheaper to resolve now than after both have landed/); + assert.match(out, /Decide which pull request owns the id/); + // And it must not imply coverage it does not have. + assert.match(out, /a branch with no open pull request claims nothing/); + }); + + it('distinct gaps across many PRs are green, and each claim is listed', () => { + const { status, out } = runWith([ + { number: 10, title: 'GT-400', headRefName: 'a' }, + { number: 11, title: 'GT-401', headRefName: 'b' }, + ]); + assert.equal(status, 0, out); + assert.match(out, /ids claimed \.+ 2/); + assert.match(out, /GT-400 — #10/); + }); + + it('says out loud when there is nothing to check', () => { + // Zero open PRs is a legitimate state that looks identical to a broken query + // unless it is named. + const { status, out } = runWith([]); + assert.equal(status, 0, out); + assert.match(out, /No pull request is open, so nothing is claimed/); + }); + + it('--json reports the claim map and exits non-zero when contested', () => { + const { status, out } = runWith( + [{ number: 1, title: 'GT-500' }, { number: 2, title: 'GT-500' }], + ['--json'], + ); + assert.equal(status, 1, out); + const payload = JSON.parse(out.trim().split('\n')[0]); + assert.deepEqual(payload.contested, [{ id: 'GT-500', prs: [1, 2] }]); + }); +}); + +describe('anti-vacuous floor', () => { + it('refuses to pass when the pull-request query cannot be answered', () => { + // No --fixture, and `gh` unusable: the guard must fail rather than report a + // clean run over a list it never got. + const r = spawnSync(process.execPath, [GUARD], { + encoding: 'utf8', + timeout: 60000, + env: { ...process.env, PATH: '/nonexistent' }, + }); + assert.equal(r.status, 1, r.stdout + r.stderr); + const out = `${r.stdout}\n${r.stderr}`; + assert.match(out, /could not read the open pull requests/); + assert.match(out, /Unable to answer is not the same as nothing to report/); + }); +}); diff --git a/.harness/scripts/lib/guard-classification.mjs b/.harness/scripts/lib/guard-classification.mjs index 8b69ba196..18aa81714 100644 --- a/.harness/scripts/lib/guard-classification.mjs +++ b/.harness/scripts/lib/guard-classification.mjs @@ -40,6 +40,15 @@ export const CALLS_COVERAGE = /\b(?:assertScanned|assertScannedPerSource|scanned * deleting the check and leaving the exemption behind. */ export const SELF_GUARDED = [ + { + file: '50-validate-gap-claim.mjs', + proof: /could not read the open pull requests/, + reason: + 'GT-639 gap-claim guard; refuses to pass when the pull-request query cannot be answered — ' + + 'the guard exists because two sessions could not see each other, so one that stays quiet ' + + 'when it cannot look would reproduce the defect. Zero open PRs is reported in words rather ' + + 'than passed silently, because it looks identical to a broken query otherwise', + }, { file: '49-validate-gap-id-allocation.mjs', proof: /at least one side yielded NOTHING/, diff --git a/reference/core/control-center/gaps/gap-reference-catalog.es.md b/reference/core/control-center/gaps/gap-reference-catalog.es.md index 74e599a91..bf8c79ce2 100644 --- a/reference/core/control-center/gaps/gap-reference-catalog.es.md +++ b/reference/core/control-center/gaps/gap-reference-catalog.es.md @@ -7860,14 +7860,14 @@ Serie histórica de gaps registrada en el antiguo `gap-analysis-core.es.md`, pre **Title:** Nada hace visible el trabajo en vuelo entre ramas, así que el mismo gap se arregla dos veces - **Purpose:** Que un gap que ya se está trabajando sea descubrible ANTES de que una segunda sesión lo empiece, y no en el merge. -- **Evidence:** **El 2026-07-30 el mismo trabajo se hizo dos veces, en tres ocasiones distintas, por sesiones que no podían verse.** (1) [`GT-640`](./gap-reference-catalog.es.md#gt-640) — registrado como GT-633 hasta la renumeración de abajo — se arregló dos veces: un script de captura aterrizó en `main` como #276 y un renderer independiente dentro del spec aterrizó en `develop` como #279. Los dos eran correctos, los dos recapturaron los mismos números — y dos generadores para un artefacto es el defecto del propio GT-633 un nivel más arriba, así que una TERCERA sesión gastó una reconciliación entera (#294, #295, #296, #297) en dejar un solo renderer. (2) El parser del guard de evidencias se arregló dos veces: `d1ea72a3` en `main` ("stop the evidence guard blaming the evidence for two parser defects") y `5acb29ed` en `develop` ("the ratchet was stuck by two bugs in itself"). (3) Un id de GT se asignó dos veces, forzando la renumeración que registra [`GT-638`](./gap-reference-catalog.es.md#gt-638). **El coste no es el conflicto de merge.** Es el trabajo duplicado que ocurrió antes de que nadie llegara al conflicto, más la reconciliación que hizo falta después — y esa reconciliación introdujo un defecto propio (una comparación sensible al orden que reescribía la fecha de captura en cada corrida, invisible durante un día porque ambas estampaban la misma fecha). Tres duplicaciones, una sesión de reparación y un bug nuevo: ésa es la factura real. **Por qué el tablero no lo ve hoy:** una fila lleva estado (`PENDIENTE`, `EN-PROGRESO`) pero no QUIÉN la trabaja ni DÓNDE. Dos sesiones pueden leer ambas `GT-640 · PENDIENTE` (registrado como GT-633 entonces) y empezar ambas, correctamente. `08-validate-tracking` valida dentro de un único árbol de trabajo por construcción, así que no puede saber que existe otra rama. **Esto no es una convención que falte — la convención existe.** La regla de un solo driver para las olas de gaps es práctica establecida; lo que falta es cualquier mecanismo que la haga observable, así que en un día cargado se degrada en silencio y nadie se entera hasta el merge. Direcciones de arreglo: exigir una reclamación (rama o PR) en la fila que pasa a `EN-PROGRESO`, y derivar el conjunto de reclamaciones de los PR abiertos para que no pueda quedarse rancio; después, fallar en PR cuando un id `GT-*` esté reclamado por más de una rama abierta, nombrando a las dos. **CERRADO el 2026-07-30 — y en su primera corrida real encontró una SEGUNDA colisión que nadie conocía.** `49-validate-gap-id-allocation` compara el **Title:** de cada id en HEAD contra el título que ese mismo id lleva en la rama base: ausente en base es id nuevo, presente con el mismo título es el mismo gap editado, presente con título DISTINTO es un número nombrando dos gaps. El título es el discriminador a propósito — "el id ya existe en base" es el caso normal de 500 y pico filas y no dice nada. Corrido contra `origin/develop` reporta **`GT-633`**: en develop es "`evolith init` scaffolds a repository that cannot pass its own governance on the first run" (`c8270e48`, 2026-07-29) y en main es la fila del guard tautológico (`c3221276`, 2026-07-30). Dos gaps, un número, y el merge todavía no lo había sacado a la luz. **RESUELTO el 2026-07-30 renumerando la fila más nueva —la de main— a [`GT-640`](./gap-reference-catalog.es.md#gt-640)**, según el propio consejo del guard. Hacerlo a mano es el coste que esta fila existe para describir: hubo que cambiar el id en la fila del tablero, el ancla del catálogo, el registro de evidencia de cierre, las referencias cruzadas de otras dos filas en los dos idiomas y tres comentarios de código — y los mensajes de commit y cuerpos de PR que nombran GT-633 no se pueden cambiar. **El guard se niega a adivinar qué lado está mal** — un retítulo y una colisión se ven igual desde fuera —, así que nombra los dos y lo dice. Cableado en `Governance guards (GT-578)` con un `git fetch` explícito de la base, porque un checkout superficial de CI no la tiene y un guard que fallara por ESO enseñaría a ignorarlo. Observado en rojo por `43-validate-guard-negative-fixtures` (38/38). **Lo que sigue sin ver, dicho y no insinuado:** un número asignado en una rama de la que nadie ha abierto PR. La salida `--verbose` lo advierte en cada id recién asignado en vez de dejar que el lector suponga una cobertura que no tiene. +- **Evidence:** **El 2026-07-30 el mismo trabajo se hizo dos veces, en tres ocasiones distintas, por sesiones que no podían verse.** (1) [`GT-640`](./gap-reference-catalog.es.md#gt-640) — registrado como GT-633 hasta la renumeración de abajo — se arregló dos veces: un script de captura aterrizó en `main` como #276 y un renderer independiente dentro del spec aterrizó en `develop` como #279. Los dos eran correctos, los dos recapturaron los mismos números — y dos generadores para un artefacto es el defecto del propio GT-633 un nivel más arriba, así que una TERCERA sesión gastó una reconciliación entera (#294, #295, #296, #297) en dejar un solo renderer. (2) El parser del guard de evidencias se arregló dos veces: `d1ea72a3` en `main` ("stop the evidence guard blaming the evidence for two parser defects") y `5acb29ed` en `develop` ("the ratchet was stuck by two bugs in itself"). (3) Un id de GT se asignó dos veces, forzando la renumeración que registra [`GT-638`](./gap-reference-catalog.es.md#gt-638). **El coste no es el conflicto de merge.** Es el trabajo duplicado que ocurrió antes de que nadie llegara al conflicto, más la reconciliación que hizo falta después — y esa reconciliación introdujo un defecto propio (una comparación sensible al orden que reescribía la fecha de captura en cada corrida, invisible durante un día porque ambas estampaban la misma fecha). Tres duplicaciones, una sesión de reparación y un bug nuevo: ésa es la factura real. **Por qué el tablero no lo ve hoy:** una fila lleva estado (`PENDIENTE`, `EN-PROGRESO`) pero no QUIÉN la trabaja ni DÓNDE. Dos sesiones pueden leer ambas `GT-640 · PENDIENTE` (registrado como GT-633 entonces) y empezar ambas, correctamente. `08-validate-tracking` valida dentro de un único árbol de trabajo por construcción, así que no puede saber que existe otra rama. **Esto no es una convención que falte — la convención existe.** La regla de un solo driver para las olas de gaps es práctica establecida; lo que falta es cualquier mecanismo que la haga observable, así que en un día cargado se degrada en silencio y nadie se entera hasta el merge. Direcciones de arreglo: exigir una reclamación (rama o PR) en la fila que pasa a `EN-PROGRESO`, y derivar el conjunto de reclamaciones de los PR abiertos para que no pueda quedarse rancio; después, fallar en PR cuando un id `GT-*` esté reclamado por más de una rama abierta, nombrando a las dos. **GUARD CONSTRUIDO el 2026-07-30, tres criterios de cuatro.** `50-validate-gap-claim` deriva el conjunto de reclamaciones de los PULL REQUESTS ABIERTOS — cada `GT-*` en el título, el cuerpo o el nombre de rama de un PR — y falla cuando un id lo reclaman dos, nombrando ambos con su rama. Derivado y no escrito a mano a propósito: una reclamación escrita a mano se queda rancia en la dirección que importa, porque alguien olvida quitarla y la siguiente sesión esquiva una reclamación que ya no sostiene nadie. Cableado en `Governance guards (GT-578)` con `GH_TOKEN`, y observado en rojo por `43-validate-guard-negative-fixtures` (39/39). **EL CRITERIO 1 SE DEJA ABIERTO A PROPÓSITO, y el motivo es una restricción de diseño, no un olvido:** una lista de reclamaciones commiteada en el tablero se derivaría de estado vivo de GitHub, así que estaría rancia en cuanto alguien abriera un PR — y la cadena de [`GT-630`](./gap-reference-catalog.es.md#gt-630) existe justamente para exigir que los artefactos derivados alcancen un punto fijo. Un artefacto perpetuamente rancio dentro de esa cadena sería peor que ninguno. La vista viva es la salida del propio check en el PR donde importa; hacerla descubrible desde el tablero necesita algo que el tablero pueda renderizar sin commitear, y eso no está construido. **Lo que no puede ver, dicho en su propio texto de fallo y no insinuado:** una rama sin PR abierto no reclama nada. Abrir el PR pronto —basta en borrador— es lo que hace visible la reclamación, y eso ya es una convención con un check detrás en vez de una regla escrita. **CERRADO el 2026-07-30 — y en su primera corrida real encontró una SEGUNDA colisión que nadie conocía.** `49-validate-gap-id-allocation` compara el **Title:** de cada id en HEAD contra el título que ese mismo id lleva en la rama base: ausente en base es id nuevo, presente con el mismo título es el mismo gap editado, presente con título DISTINTO es un número nombrando dos gaps. El título es el discriminador a propósito — "el id ya existe en base" es el caso normal de 500 y pico filas y no dice nada. Corrido contra `origin/develop` reporta **`GT-633`**: en develop es "`evolith init` scaffolds a repository that cannot pass its own governance on the first run" (`c8270e48`, 2026-07-29) y en main es la fila del guard tautológico (`c3221276`, 2026-07-30). Dos gaps, un número, y el merge todavía no lo había sacado a la luz. **RESUELTO el 2026-07-30 renumerando la fila más nueva —la de main— a [`GT-640`](./gap-reference-catalog.es.md#gt-640)**, según el propio consejo del guard. Hacerlo a mano es el coste que esta fila existe para describir: hubo que cambiar el id en la fila del tablero, el ancla del catálogo, el registro de evidencia de cierre, las referencias cruzadas de otras dos filas en los dos idiomas y tres comentarios de código — y los mensajes de commit y cuerpos de PR que nombran GT-633 no se pueden cambiar. **El guard se niega a adivinar qué lado está mal** — un retítulo y una colisión se ven igual desde fuera —, así que nombra los dos y lo dice. Cableado en `Governance guards (GT-578)` con un `git fetch` explícito de la base, porque un checkout superficial de CI no la tiene y un guard que fallara por ESO enseñaría a ignorarlo. Observado en rojo por `43-validate-guard-negative-fixtures` (38/38). **Lo que sigue sin ver, dicho y no insinuado:** un número asignado en una rama de la que nadie ha abierto PR. La salida `--verbose` lo advierte en cada id recién asignado en vez de dejar que el lector suponga una cobertura que no tiene. - **Component:** `Governance` · **Criticality:** P2 · **Complexity:** M - **Provenance:** Registrado el 2026-07-30 a partir de tres instancias observadas en un solo día, todas en el merge `develop` → `main` que las hizo visibles. Relacionado pero distinto de [`GT-638`](./gap-reference-catalog.es.md#gt-638): aquella fila trata de asignar un NOMBRE dos veces, ésta de hacer el TRABAJO dos veces. La misma carencia de coordinación produce ambas, y la colisión de id es su síntoma más barato. - **Acceptance criteria:** - [ ] Un gap que se está trabajando es descubrible desde el tablero junto con la rama o el PR que lo reclama, antes de que el trabajo aterrice. - - [ ] Una comprobación falla en pull request cuando un id `GT-*` está reclamado por más de una rama abierta, nombrando a ambos reclamantes. - - [ ] La reclamación se DERIVA de los pull requests abiertos y no se escribe a mano, para que no pueda quedarse rancia en la dirección que importa. - - [ ] Incluye una fixture negativa OBSERVADA en rojo, no sólo declarada capaz de fallar. + - [x] Una comprobación falla en pull request cuando un id `GT-*` está reclamado por más de una rama abierta, nombrando a ambos reclamantes — `50-validate-gap-claim`, cableado en `Governance guards (GT-578)`. + - [x] La reclamación se DERIVA de los pull requests abiertos y no se escribe a mano, para que no pueda quedarse rancia en la dirección que importa — título, cuerpo y nombre de rama de cada PR abierto. + - [x] Incluye fixtures negativas OBSERVADAS en rojo — 10, entre ellas dos PRs abiertos sobre un mismo gap y el suelo anti-vacuo, y `43-validate-guard-negative-fixtures` la cuenta en 39/39. #### GT-638 diff --git a/reference/core/control-center/gaps/gap-reference-catalog.md b/reference/core/control-center/gaps/gap-reference-catalog.md index c7bd95f09..24e263209 100644 --- a/reference/core/control-center/gaps/gap-reference-catalog.md +++ b/reference/core/control-center/gaps/gap-reference-catalog.md @@ -7955,14 +7955,14 @@ Historical gap series tracked in the former `gap-analysis-core.md`, preserved fo **Title:** Nothing makes in-flight work visible across branches, so the same gap gets fixed twice - **Purpose:** Make a gap that is already being worked discoverable BEFORE a second session starts it, rather than at merge time. -- **Evidence:** **On 2026-07-30 the same work was done twice, three separate times, by sessions that could not see each other.** (1) [`GT-640`](./gap-reference-catalog.md#gt-640) — registered as GT-633 until the renumber below — was fixed twice: a standalone capture script landed on `main` as #276, and an independent in-spec renderer landed on `develop` as #279. Both were correct, both recaptured the same numbers — and two generators for one artifact is GT-633's own defect one level up, so a THIRD session spent a full reconciliation (#294, #295, #296, #297) collapsing them to one renderer. (2) The evidence guard's parser was fixed twice: `d1ea72a3` on `main` ("stop the evidence guard blaming the evidence for two parser defects") and `5acb29ed` on `develop` ("the ratchet was stuck by two bugs in itself"). (3) A GT id was allocated twice, forcing the renumber recorded in [`GT-638`](./gap-reference-catalog.md#gt-638). **The cost is not the merge conflict.** It is the duplicated work that happened before anyone reached the conflict, plus the reconciliation it then required — and that reconciliation introduced a defect of its own (an order-sensitive comparison that rewrote the capture date on every run, invisible for a day because both runs stamped the same date). Three duplications, one repair session, one new bug: that is the real bill. **Why the board cannot see it today:** a row carries a status (`PENDING`, `IN-PROGRESS`) but not WHO is working it or WHERE. Two sessions can both read `GT-640 · PENDING` (registered as GT-633 at the time) and both start, correctly. `08-validate-tracking` validates within one working tree by construction, so it cannot know another branch exists. **This is not a missing convention — the convention exists.** The single-driver rule for gap waves is already established practice; what is missing is any mechanism that makes it observable, so on a busy day it degrades silently and nobody learns until the merge. Fix directions: require a claim (branch or PR) on a row that moves to `IN-PROGRESS`, and derive the claim set from open PRs so it cannot go stale; then fail at PR time when one GT id is claimed by more than one open branch, naming both. +- **Evidence:** **On 2026-07-30 the same work was done twice, three separate times, by sessions that could not see each other.** (1) [`GT-640`](./gap-reference-catalog.md#gt-640) — registered as GT-633 until the renumber below — was fixed twice: a standalone capture script landed on `main` as #276, and an independent in-spec renderer landed on `develop` as #279. Both were correct, both recaptured the same numbers — and two generators for one artifact is GT-633's own defect one level up, so a THIRD session spent a full reconciliation (#294, #295, #296, #297) collapsing them to one renderer. (2) The evidence guard's parser was fixed twice: `d1ea72a3` on `main` ("stop the evidence guard blaming the evidence for two parser defects") and `5acb29ed` on `develop` ("the ratchet was stuck by two bugs in itself"). (3) A GT id was allocated twice, forcing the renumber recorded in [`GT-638`](./gap-reference-catalog.md#gt-638). **The cost is not the merge conflict.** It is the duplicated work that happened before anyone reached the conflict, plus the reconciliation it then required — and that reconciliation introduced a defect of its own (an order-sensitive comparison that rewrote the capture date on every run, invisible for a day because both runs stamped the same date). Three duplications, one repair session, one new bug: that is the real bill. **Why the board cannot see it today:** a row carries a status (`PENDING`, `IN-PROGRESS`) but not WHO is working it or WHERE. Two sessions can both read `GT-640 · PENDING` (registered as GT-633 at the time) and both start, correctly. `08-validate-tracking` validates within one working tree by construction, so it cannot know another branch exists. **This is not a missing convention — the convention exists.** The single-driver rule for gap waves is already established practice; what is missing is any mechanism that makes it observable, so on a busy day it degrades silently and nobody learns until the merge. Fix directions: require a claim (branch or PR) on a row that moves to `IN-PROGRESS`, and derive the claim set from open PRs so it cannot go stale; then fail at PR time when one GT id is claimed by more than one open branch, naming both. **GUARD BUILT 2026-07-30, three criteria of four.** `50-validate-gap-claim` derives the claim set from OPEN PULL REQUESTS — every `GT-*` in a PR's title, body or branch name — and fails when one id is claimed by two of them, naming both with their branches. Derived rather than hand-written on purpose: a hand-written claim goes stale in the direction that matters, because someone forgets to remove it and the next session works around a claim nobody holds. Wired into `Governance guards (GT-578)` with `GH_TOKEN`, and observed red by `43-validate-guard-negative-fixtures` (39/39). **CRITERION 1 IS DELIBERATELY LEFT OPEN, and the reason is a design constraint rather than an omission:** a claim list committed to the board would be derived from live GitHub state, so it would be stale the moment anyone opened a pull request — and [`GT-630`](./gap-reference-catalog.md#gt-630)'s chain exists precisely to insist that derived artifacts reach a fixed point. A perpetually-stale artifact inside that chain would be worse than none. The live view is the check's own output on the pull request where it matters; making it discoverable from the board needs something the board can render without committing, and that is not built. **What it cannot see, said in its own failure text rather than implied:** a branch with no open pull request claims nothing. Opening the PR early — draft is enough — is what makes the claim visible, and that is now a working convention with a check behind it rather than a rule written down. - **Component:** `Governance` · **Criticality:** P2 · **Complexity:** M - **Provenance:** Registered 2026-07-30 from three observed instances in a single day, all of them in the `develop` → `main` merge that made them visible. Related but distinct from [`GT-638`](./gap-reference-catalog.md#gt-638): that row is about allocating a NAME twice, this one about doing the WORK twice. The same coordination gap produces both, and the id collision is the cheapest symptom of it. - **Acceptance criteria:** - [ ] A gap being worked is discoverable from the board together with the branch or PR that claims it, before the work lands. - - [ ] A check fails at pull-request time when one `GT-*` id is claimed by more than one open branch, naming both claimants. - - [ ] The claim is DERIVED from open pull requests rather than hand-written, so it cannot be stale in the direction that matters. - - [ ] It ships with a negative fixture that has been OBSERVED red, not merely declared able to fail. + - [x] A check fails at pull-request time when one `GT-*` id is claimed by more than one open branch, naming both claimants — `50-validate-gap-claim`, wired into `Governance guards (GT-578)`. + - [x] The claim is DERIVED from open pull requests rather than hand-written, so it cannot be stale in the direction that matters — title, body and branch name of every open PR. + - [x] It ships with negative fixtures that have been OBSERVED red — 10 of them, including two open PRs on one gap and the anti-vacuous floor, and `43-validate-guard-negative-fixtures` counts it at 39/39. #### GT-638 diff --git a/reference/core/control-center/gaps/gap-tracking.es.md b/reference/core/control-center/gaps/gap-tracking.es.md index feef4d09d..e2e168820 100644 --- a/reference/core/control-center/gaps/gap-tracking.es.md +++ b/reference/core/control-center/gaps/gap-tracking.es.md @@ -14,7 +14,7 @@ Este tablero es la única fuente de verdad para deuda técnica, gaps, oportunida | ID | Gap | Qué significa | Ejemplo | Componente | Fase | Criticidad | Complejidad | Estado | |---|---|---|---|:---:|:---:|:---:|:---:|:---:| | [`GT-640`](./gap-reference-catalog.es.md#gt-640) | **El guard comparaba el snapshot contra seis números copiados del propio snapshot, así que solo podía pasar — y el archivo que no estaba vigilando se estampa en 388 filas de un artefacto mayor.** `native-evaluability-snapshot.json` declaraba en su propia cabecera que era "un SNAPSHOT CAPTURADO, no la fuente de verdad". **No existía ningún script de captura.** Se mantenía a mano y derivó: fijaba `documentation-only: 129` mientras Core fijaba 136, y seguía llamando `unimplemented-native` a reglas que ya tenían handler. Su guard en `iso-5055-mapping.test.mjs` asertaba los seis conteos de clase del snapshot contra seis números escritos a mano en el test — los mismos seis literales que el snapshot ya contenía. **El valor esperado y el valor real eran copias el uno del otro**, así que la aserción se sostenía mientras el archivo no cambiara, fijara Core lo que fijara; informaba de una concordancia que nunca comprobó, lo cual es peor que no tener guard, porque la fila que protege se lee como verificada. **La deriva no se queda donde empieza.** `build-iso-5055-mapping.mjs` estampa `nativeEvaluability` en TODAS las filas del mapeo ISO/IEC 5055 desde ese archivo — 388 tras la recaptura, 381 antes — así que una sola clase rancia se blanquea en un artefacto derivado cinco veces mayor que su entrada, y sobredimensiona el backlog de handlers que es el entregable entero de [`GT-598`](./gap-reference-catalog.es.md#gt-598). El orden recaptura→reconstrucción no estaba escrito en ningún sitio, y el guard del propio mapeo no corría en **ningún workflow**. **CORREGIDO el 2026-07-29:** un script de captura que ejecuta el triage REAL de Core vía `ts-node` en vez de reimplementar la clasificación, la medición extraída del spec de jest a `test/rule-corpus-triage.ts` para que un script pueda alcanzarla — esa inalcanzabilidad es la razón de que el archivo se mantuviera a mano —, aserciones snapshot-contra-triage-fresco en ambas direcciones, un guard del job de documentación que lee los conteos que Core fija LEYENDO el spec y **lanza excepción** cuando el literal falta o cambia de forma, y ambos pasos declarados como eslabones de la cadena de artefactos derivados de [`GT-630`](./gap-reference-catalog.es.md#gt-630). **Recapturado el 2026-07-29 tras mergear el fix en la línea actual, y la deriva había crecido mientras estaba en vuelo:** `native-handler` 139 → 151, `documentation-only` 129 → 136, el backlog de handlers 60 → **48**, corpus 379 → 386, mapeo 381 → 388 filas. Doce de esos handlers los cerraron [`GT-595`](./gap-reference-catalog.es.md#gt-595) y [`GT-632`](./gap-reference-catalog.es.md#gt-632) mientras el snapshot mantenido a mano seguía reportándolos como backlog — el defecto demostrándose una vez más, sobre números que nadie escribió dos veces. **RENUMERADA GT-633 -> GT-640 el 2026-07-30**, después de que `49-validate-gap-id-allocation` detectara que `GT-633` ya nombraba otro gap en `develop` (el scaffolding de `evolith init`, `c8270e48`, registrado un día antes). Se mueve la fila más nueva, según el propio consejo del guard. | | | `Governance` | Cross | P1 | S | `COMPLETADO` | -| [`GT-639`](./gap-reference-catalog.es.md#gt-639) | **Nada hace visible el trabajo en vuelo entre ramas, así que el mismo gap se arregla dos veces.** El 2026-07-30 el mismo trabajo se hizo dos veces, en tres ocasiones distintas, por sesiones que no podían verse: [`GT-640`](./gap-reference-catalog.es.md#gt-640) (registrado como GT-633 entonces) arreglado por un script de captura en `main` (#276) Y por un renderer independiente dentro del spec en `develop` (#279) — los dos correctos, los dos recapturando los mismos números, y dos generadores para un artefacto es el defecto del propio GT-640 un nivel más arriba, así que una tercera sesión gastó una reconciliación entera en dejar uno; el parser del guard de evidencias arreglado dos veces (`d1ea72a3` en main, `5acb29ed` en develop); y un id de GT asignado dos veces, forzando la renumeración que hay detrás de [`GT-638`](./gap-reference-catalog.es.md#gt-638). **El coste no es el conflicto de merge** — es el trabajo duplicado antes de que nadie llegara a él, más la reconciliación, que introdujo un defecto propio (una comparación sensible al orden que reescribía la fecha de captura en cada corrida, invisible un día porque ambas estampaban la misma fecha). Una fila lleva estado pero no QUIÉN la trabaja ni DÓNDE, así que dos sesiones pueden leer ambas `PENDIENTE` y empezar ambas, correctamente. **La convención de un solo driver ya existe; lo que falta es un mecanismo que la haga observable**, así que en un día cargado se degrada en silencio. | | | `Governance` | Cross | P2 | M | `PENDIENTE` | +| [`GT-639`](./gap-reference-catalog.es.md#gt-639) | **Nada hace visible el trabajo en vuelo entre ramas, así que el mismo gap se arregla dos veces.** El 2026-07-30 el mismo trabajo se hizo dos veces, en tres ocasiones distintas, por sesiones que no podían verse: [`GT-640`](./gap-reference-catalog.es.md#gt-640) (registrado como GT-633 entonces) arreglado por un script de captura en `main` (#276) Y por un renderer independiente dentro del spec en `develop` (#279) — los dos correctos, los dos recapturando los mismos números, y dos generadores para un artefacto es el defecto del propio GT-640 un nivel más arriba, así que una tercera sesión gastó una reconciliación entera en dejar uno; el parser del guard de evidencias arreglado dos veces (`d1ea72a3` en main, `5acb29ed` en develop); y un id de GT asignado dos veces, forzando la renumeración que hay detrás de [`GT-638`](./gap-reference-catalog.es.md#gt-638). **El coste no es el conflicto de merge** — es el trabajo duplicado antes de que nadie llegara a él, más la reconciliación, que introdujo un defecto propio (una comparación sensible al orden que reescribía la fecha de captura en cada corrida, invisible un día porque ambas estampaban la misma fecha). Una fila lleva estado pero no QUIÉN la trabaja ni DÓNDE, así que dos sesiones pueden leer ambas `PENDIENTE` y empezar ambas, correctamente. **La convención de un solo driver ya existe; lo que falta es un mecanismo que la haga observable**, así que en un día cargado se degrada en silencio. **GUARD CONSTRUIDO el 2026-07-30 — tres criterios de cuatro.** `50-validate-gap-claim` deriva las reclamaciones de los PULL REQUESTS ABIERTOS (cada `GT-*` en título, cuerpo o nombre de rama) y falla cuando un id lo reclaman dos, nombrando ambos con su rama. Derivado y no a mano a propósito: una reclamación escrita a mano se queda rancia en la dirección que importa. Cableado en `Governance guards (GT-578)` con `GH_TOKEN`; observado en rojo por `43-validate-guard-negative-fixtures` (39/39). **El criterio 1 sigue abierto a propósito:** una lista commiteada en el tablero derivaría de estado vivo de GitHub y estaría rancia en cuanto alguien abriera un PR — y la cadena de [`GT-630`](./gap-reference-catalog.es.md#gt-630) existe para exigir que los derivados alcancen un punto fijo, así que uno perpetuamente rancio sería peor que ninguno. Tampoco ve una rama sin PR abierto, y lo dice en su propio texto de fallo. | | | `Governance` | Cross | P2 | M | `EN-PROGRESO` | | [`GT-638`](./gap-reference-catalog.es.md#gt-638) | **El tablero no tiene asignador de ids, así que dos ramas paralelas reparten el mismo número GT.** Un id se elige leyendo el `GT-*` más alto de la rama en la que uno está, y nada lo contrasta con otra rama — así que dos sesiones en paralelo asignan el mismo número y una se entera en el merge. Ya pasó: `8449af3d` en `develop` dice *"renumber the ratchet fix GT-634 -> GT-637, ID collision with develop"*; esa fila y [`GT-634`](./gap-reference-catalog.es.md#gt-634) en `main` tomaron el mismo id con horas de diferencia, desde sesiones que no podían verse. **La renumeración es manual y con pérdidas**, que cuesta más que el choque: un id vive en la fila del tablero, el ancla del catálogo, el registro de evidencia de cierre, referencias cruzadas en dos idiomas, mensajes de commit y cuerpos de PR — y sólo los tres primeros son comprobables mecánicamente. `08-validate-tracking` no puede ayudar por construcción: valida dentro de un único árbol de trabajo, y las ramas quedan fuera de su mundo. Tampoco es un desliz aislado — en el merge `develop` → `main` del 2026-07-30 el mismo patrón de duplicación convergente aparece tres veces: GT-640 (entonces GT-633) arreglado dos veces, el parser del guard de evidencias arreglado dos veces, este id asignado dos veces. **CERRADO el 2026-07-30 — y en su primera corrida real encontró una SEGUNDA colisión que nadie conocía.** `49-validate-gap-id-allocation` compara el **Title:** de cada id en HEAD contra el título que ese id lleva en la rama base; un título distinto para el mismo id es un número nombrando dos gaps. Contra `origin/develop` reportó **`GT-633`**: allí es "`evolith init` scaffolds a repository that cannot pass its own governance on the first run" (`c8270e48`, 2026-07-29) y en main era la fila del guard tautológico (`c3221276`, 2026-07-30), renumerada después a `GT-640`. El merge no lo había sacado a la luz. Se niega a adivinar qué lado está mal — un retítulo y una colisión se ven igual desde fuera —, así que nombra los dos. Cableado en `Governance guards (GT-578)` con un `git fetch` explícito de la base, porque un checkout superficial de CI no la tiene. Observado en rojo por `43-validate-guard-negative-fixtures` (38/38). Sigue sin ver un id asignado en una rama sin PR abierto, y su salida `--verbose` lo advierte en cada id nuevo. | | | `Governance` | Cross | P2 | S | `COMPLETADO` | | [`GT-637`](./gap-reference-catalog.es.md#gt-637) | **El ratchet de referencias muertas de GT-578 estaba atascado en 305 por dos bugs reales en el propio GUARD, no por el tamaño del backlog de registros que decía medir.** Al cerrar el PR de `GT-633` apareció un fallo en `Governance guards (GT-578)` y, al investigarlo, un refactor sin commitear y sin terminar que ya estaba en el árbol de trabajo: `resolveCommand` llamaba a `npmScriptOperand(...)`, una función que nunca se definió, así que el guard crasheaba con `ReferenceError` en cada invocación — incluso en modo reporte plano. Reparar esa función (restaurar su comportamiento y luego corregirlo) bajó el conteo de 309 a 78, lo que destapó el segundo bug: el corpus usa `npm run --workspace