diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f370faa4f..f716b8ebb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -572,6 +572,65 @@ jobs: if-no-files-found: ignore retention-days: 3 + # #2966: turns a loop QA-walk "smell" into a decision — see + # scripts/qa-smell-ratchet.cjs's own header for the full design invariant. + # Gated the same way as `test`/`coverage-gate` (product_changed only): the + # scenarios drive the real gsd-tools binary end-to-end, so there is nothing + # to walk on a docs-only change. + qa-loop-walk: + name: QA loop walk (smell ratchet) + needs: changes + if: needs.changes.outputs.product_changed == 'true' + runs-on: ubuntu-latest + timeout-minutes: 15 + env: + GSD_PLUGIN_ROOT: .ci-gsd-plugin-root-disabled + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + persist-credentials: true + token: ${{ github.token }} + + - name: Guard — require GitHub-hosted runner + run: node scripts/ci-guard-runner.cjs + + - name: Set up Node.js 24 + uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f # v6.3.0 + with: + node-version: 24 + cache: 'npm' + + - name: Environment check + run: npm run check:env + + - name: Install dependencies + run: npm ci + + - name: Dependency integrity gate + run: node scripts/check-npm-integrity.cjs + + - name: Build runtime lib (required by the QA walk's gsd-tools invocations) + run: npm run build:lib + + - name: Run QA loop-walk scenarios + run: npm run test:qa + + # `-- --json qa-report.json` asks the ratchet to also write the full report + # (its default `npm run lint:qa-smells` invocation does not) — the report is + # gitignored and exists for the artifact upload below, not for the gate itself. + - name: QA smell ratchet + run: npm run lint:qa-smells -- --json qa-report.json + + - name: Upload QA report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: qa-report + path: qa-report.json + if-no-files-found: ignore + retention-days: 3 + required-tests: name: Required tests needs: @@ -581,6 +640,7 @@ jobs: - test-inert - test-full - coverage-gate + - qa-loop-walk if: always() runs-on: ubuntu-latest timeout-minutes: 1 @@ -595,6 +655,7 @@ jobs: INERT_RESULT: ${{ needs.test-inert.result }} FULL_TEST_RESULT: ${{ needs.test-full.result }} COVERAGE_GATE_RESULT: ${{ needs.coverage-gate.result }} + QA_LOOP_WALK_RESULT: ${{ needs.qa-loop-walk.result }} run: | set -euo pipefail echo "code_changed=$CODE_CHANGED" @@ -605,6 +666,7 @@ jobs: echo "test-inert=$INERT_RESULT" echo "test-full=$FULL_TEST_RESULT" echo "coverage-gate=$COVERAGE_GATE_RESULT" + echo "qa-loop-walk=$QA_LOOP_WALK_RESULT" if [ "$CHANGES_RESULT" != "success" ]; then echo "::error::test scope detection did not pass" @@ -643,6 +705,12 @@ jobs: echo "::error::coverage-gate did not pass" exit 1 fi + + # #2966: same product_changed-only gating as coverage-gate above. + if [ "$QA_LOOP_WALK_RESULT" != "success" ] && [ "$QA_LOOP_WALK_RESULT" != "skipped" ]; then + echo "::error::qa-loop-walk did not pass" + exit 1 + fi else if [ "$INERT_RESULT" != "success" ]; then echo "::error::inert CI lane did not pass" diff --git a/.gitignore b/.gitignore index 851484eb1..aa5437924 100644 --- a/.gitignore +++ b/.gitignore @@ -272,3 +272,6 @@ reports/mutation/ # Memtrace local daemon/runtime state (per-machine; never committed) .memdb/ .memtrace/ + +# QA-walk report artifact (generated by tests/qa/run-report.cjs) +qa-report.json diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index 1eb6babf6..aa126fdf2 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -13,6 +13,7 @@ This project's `tests/` directory uses **filename suffix markers** to group test | `install` | `*.install.test.cjs` | Tests that perform a real install/uninstall against a sandbox project. Slower; PR CI skips these on PRs and runs them on `main` push only. | | `security` | `*.security.test.cjs` | Adversarial input, prompt-injection guards, fixture-driven hostile-payload sweeps. | | `slow` | `*.slow.test.cjs` | Anything that routinely takes >5s wall-clock or holds significant memory. | +| `qa` | `*.qa.test.cjs` | End-to-end walks that drive the real `gsd-tools` binary across multiple loop steps against one accumulating temp project, with invariant oracles after every step. Slower than `unit`; excluded from the fast lane. | | `all` | (any) | Explicit alias for "no filter". Equivalent to running with no `--suite` flag. | ## How to place a new test @@ -26,6 +27,7 @@ Examples: - `tests/prompt-injection-guards.security.test.cjs` — `security` - `tests/installer-end-to-end.install.test.cjs` — `install` - `tests/sdk-mutation-stress.slow.test.cjs` — `slow` +- `tests/loop-walk.qa.test.cjs` — `qa` The suite-suffix convention was chosen over a directory layout (`tests/security/`) so the 545+ existing test files don't need to move. Existing files all classify as `unit` until someone explicitly retags them. @@ -134,6 +136,57 @@ are readable, and it preserves "the installer stopped shipping X" as a hard absolute failure with no attribution reasoning involved. Regenerate it with `npm run gen:install-tree` (folded into `npm run regen:derived`). +## The QA smell ratchet + +`tests/loop-walk.qa.test.cjs` (the `qa` suite) is the QA-walk harness's own +self-test. Separately, `scripts/qa-smell-ratchet.cjs` drives that same harness +end to end against the real `gsd-tools` binary and turns its findings into a +CI gate — run it with `npm run lint:qa-smells`. + +The harness's oracles (`tests/qa/oracles.cjs`) distinguish two severities: + +- A **violation** is the engine breaking a documented contract. It always + fails the build — baseline or no baseline, acknowledged or not. +- A **smell** is legal-but-questionable behavior. A smell **never fails a + build on its own merits**. What fails is the absence of a decision about + it: an **unacknowledged NEW smell**, or a **STALE** entry in + `tests/qa/smell-baseline.json` (one that stopped firing — the baseline is + shrink-only, so a fixed or changed scenario must be pruned, not left + behind). + +Every smell must terminate in exactly one of TWO states — there is no third +"accepted with a good explanation" state: + +1. **REAL** — an assigned defect. File it, then acknowledge the smell with an + entry (baseline entry or `tests/qa/smell-acks/` fragment) carrying that + `issue` number. +2. **FALSE POSITIVE** — the oracle itself is wrong. Fix the oracle + (`tests/qa/oracles.cjs`) so it stops firing. It is NEVER baselined. + +When the ratchet reports a NEW smell, there are exactly two legitimate +responses — fix the detector, or file a defect and cite its issue number: + +1. **Fix the underlying behavior (or the oracle, if it's a false positive)** + so the smell stops firing. +2. **File a defect and acknowledge it** by adding a fragment under + `tests/qa/smell-acks/` — the ratchet's failure output prints a paste-ready + skeleton naming the required `key`, `id`, `scenario`, and `issue` fields. + `issue` MUST be a positive integer naming the tracking issue; a free-text + `reason` may accompany it as an optional human note but can NEVER + substitute for `issue` — "write an explanation" is not a way to acknowledge + a smell. See `tests/qa/smell-acks/README.md` for the full shape and + lifecycle. + +Run `node scripts/qa-smell-ratchet.cjs --update` to regenerate +`tests/qa/smell-baseline.json` from the current run, folding in any acked +fragments and pruning stale entries. `--update` never invents an issue +number: a genuinely new smell is written with `issue: null` and a TODO +`reason`, and the very next plain (non-`--update`) run REJECTS that entry — +forcing a human to triage it before it can ship. The baseline only ever +shrinks: growth happens by adding an acknowledgment carrying a real issue +number (a reviewable diff), never by widening the generator's tolerance and +never by prose alone. + ## Running suites locally ```bash diff --git a/docs/adr/2966-loop-qa-walk.md b/docs/adr/2966-loop-qa-walk.md new file mode 100644 index 000000000..57c430884 --- /dev/null +++ b/docs/adr/2966-loop-qa-walk.md @@ -0,0 +1,99 @@ +# ADR-2966: Test the five-step loop as a continuous walk, not isolated points + +- **Status:** Accepted +- **Date:** 2026-08-01 +- **Issue:** [#2966](https://github.com/open-gsd/gsd-core/issues/2966) +- **Supersedes:** — +- **Relationship to prior work:** Extends `RULESET.TESTS.feedback-loop-convergence` (`CONTEXT.md`) from its existing single instance, `tests/estimate-loop-convergence.test.cjs`, to the full five-step loop. + +## Context + +131 of roughly 705 test files already drive the real `gsd-tools` binary through `runGsdTools` in `tests/helpers.cjs` — CLI-altitude testing is solved and well trodden. What no existing test does is carry one project's accumulating state across all five loop steps in a single run. Every loop test today asserts a step in isolation, against state fabricated for that step alone. `CONTEXT.md`'s `RULESET.TESTS.feedback-loop-convergence` already names the alternative — assert convergence across an accumulating sequence — and one instance of it exists for estimation (`tests/estimate-loop-convergence.test.cjs`). This ADR extends that pattern to the loop itself. + +## Decision + +### 1. Walk the loop as a continuous trajectory + +Build a walk driver, `tests/qa/loop-walk.cjs`, that runs the five loop steps in order against one project's accumulating on-disk state, rather than five independent point tests each seeded from scratch. This is `RULESET.TESTS.feedback-loop-convergence` generalized from estimation to the loop, anchored to the existing convergence instance rather than invented fresh. + +### 2. Layer over `tests/helpers.cjs`; do not build a second substrate + +`runGsdTools` already invokes the real subprocess with a 60s timeout and env injection, and 131 files depend on it. The walk driver is built strictly on top of it. The rejected alternative — a standalone harness re-implementing subprocess invocation — was considered and rejected as duplication (Gall's Law: a working complex system evolves from a working simple one). + +### 3. The stub agent seam, and its limit stated plainly + +`gsd-tools init new-project` (and the other loop init commands) write nothing — they are read-only context projections; the agent is the one that authors artifacts (STATE.md, phase files, etc.). A scripted stub writer standing in for the agent is therefore faithful to the real division of labor, and the entire engine — routing, state transitions, gates — runs for real behind it. + +The limit is stated plainly and not glossed over: this walk proves the engine's state machine is coherent across all five steps. It proves nothing about whether a real LLM agent emits artifacts of the shape the walk's fixtures assume. + +### 4. Oracles read a typed IR only, never rendered text + +`tests/qa/oracles.cjs`'s ten invariant oracles (seven `SEVERITY.VIOLATION` oracles plus three `SEVERITY.SMELL` oracles — see §5) assert exclusively against `tests/qa/result.cjs`'s typed `RunResult` IR, never against raw stdout/stderr text. This is forced, not stylistic: `CONTRIBUTING.md` → "Prohibited: Raw Text Matching on Test Outputs" and `RULESET.TESTS.no-source-grep.tmp-file-traps` both ban it. The same constraint governs the tree-idempotence oracle: it compares `fs.statSync` facts (`size`, `mtimeMs`, `isFile()`) across runs, never content hashing or reading the SUT's own tmp-file output, which would trip the identical lint. + +### 5. Findings carry severity; the harness reports evidence, it does not adjudicate + +`tests/qa/oracles.cjs`'s `check(ctx)` outcomes carry a `severity`: + +- `SEVERITY.VIOLATION` — the documented contract is broken. Fails `runOracles(ctx).failed` and therefore fails the build. +- `SEVERITY.SMELL` — legal under today's implementation but structurally questionable. Recorded in `.smells`, never folded into `.failed`, and never fails a build. + +Rationale: an oracle set derived from current behavior can only ever confirm current behavior — that framing turns the harness into a conformance test for the status quo. Severity is what lets the harness say "this works and is still wrong," which is the sentence a quality tool must be able to form. The maintainer decides which smells become fixes; the tool's job is to make the trade visible instead of invisible. + +Consequence, stated as an explicit safety property: `runOracles(ctx).failed` is a getter that deliberately aliases `violations` only — it never includes `smells`. Adding a smell oracle, or a new smell case to an existing oracle, can therefore never redden CI. The severity split is what makes it safe to keep adding observational oracles without turning every new observation into a build break. + +### 6. `output({error: …})` is not changed here — its cost is now visible on every occurrence + +42 call sites use `output({error: …})`: a JSON payload carrying an `error` key on stdout, exit 0. This is deliberately distinct from `error(msg, reason)`, which writes `{ok:false, reason, message}` to stderr and exits 1. `get_impact` rates `cmdStateSnapshot` — the function this idiom threads through — **CRITICAL** (55 affected symbols, 23 processes). Normalizing all 42 sites to the hard-failure path was considered and rejected: it would flip exit codes 0 → 1 across a seam that workflows and agents currently treat as a soft signal — Hyrum's Law applies directly, since the exit-0 behavior is observable and already depended on. + +This is not left alone because it is judged fine. It is not changed here because a blast radius of that size warrants a separate, deliberate decision, not one folded into a QA-harness ADR — and the `soft-error-exit-zero` smell (§5) now fires on every occurrence so the cost stays visible instead of quietly disappearing back into the status quo. The concrete cost for a caller: a shell caller's `if ! cmd; then` is blind to a failure reported through a payload key with exit 0 — the process exits 0, so the conditional never trips. The walk's `RunResult` IR types this shape explicitly (`kind: 'soft-error'`) so it can be reported as a smell rather than silently swallowed. + +### 7. The `docs/json-errors.md` vs. `src/cli-exit.cts` contract conflict is surfaced, not resolved + +`docs/json-errors.md` states that every error emits exactly one JSON line to stderr and exits 1. `src/cli-exit.cts`'s `runMain` returns before reaching the json-error branch for `ExitError`, so CLI usage errors deliberately emit plain text instead. Both behaviors are current and they contradict each other. + +This ADR does not pick a side — resolving it means changing every `ExitError` throw site, which is a separate decision with its own blast radius. What changes is how the conflict is surfaced: the `contract-conflict` smell (§5) fires on every occurrence in json-error mode and quotes both sides in its detail — the `docs/json-errors.md` sentence and the `src/cli-exit.cts` `runMain` early-return that produces the breach — so a reader gets the evidence needed to decide, not a bare note that a conflict exists. This is layered on top of `json-contract`, which already reports the same observation as a VIOLATION of the documented contract (`RunResult.kind: 'unstructured-error'` on rows the doc says should be structured); `contract-conflict` adds the reason it is ambiguous rather than a clear-cut bug. + +### 8. Fixture provenance is honest, not borrowed + +Scenario artifacts live at `tests/qa/fixtures/`, template-derived, carrying an explicit provenance comment. They deliberately do **not** live at `tests/fixtures/representative/`, which asserts real-user provenance this repo cannot substantiate — no `.planning/` directory exists in this repo and no real loop-artifact fixtures exist to source from. Per #2371, template-derived provenance is adequate for happy-path and sequence scenarios, because templates are exactly what agents are instructed to emit. It is **not** adequate for a negative fixture asserting that the engine correctly rejects malformed input — a rejection test needs to be grounded in what the engine's grammar actually forbids, not a plausible guess at malformed shape. Perturbations (`tests/qa/mutations.cjs`) are exempt from this constraint: a CRLF/BOM/truncate transform is drawn from a generic corruption catalog, not from the engine's grammar, so it carries no provenance claim to substantiate. + +### 9. New `qa` test-suite marker, excluded from the default `unit` lane + +`scripts/run-tests.cjs` gains a `qa` suite marker for `tests/loop-walk.qa.test.cjs` and its dependents, excluded from the default `unit` lane. The walk's subprocess fan-out is deliberately bounded and run sequentially within a file: `RULESET.HARNESS.test-memory-guard` denies node spawns above 4 GiB aggregate RSS, and parallelizing scenario runs would risk crossing it. + +### What the first run found + +Running the walk driver against the real engine — not a mock — produced 0 violations and three smell classes on this first pass: + +- `value-hygiene` — `init` returns `agents_dir` pointing outside the project tree. +- `untyped-success` — `smart-entry` emits prose unconditionally (`gsd-core/bin/lib/smart-entry.cjs:577-587`), so the routing oracle cannot assert on it until a `--json` mode exists. +- `soft-error-exit-zero` — `state-snapshot` reports a missing STATE.md through a payload key with exit 0. + +These are observations for a maintainer to weigh, not defects this PR fixes. + +## Consequences + +- **Positive — makes easy.** A defect that only manifests as accumulated state drifting across loop steps (a stale pointer, a field one step wrote that a later step misreads) is now catchable by a single walk instead of requiring a human to hand-construct multi-step state. The `RunResult` IR gives every future loop-facing test a typed, lint-compliant surface to assert against instead of re-deriving raw-text parsing per test. The convergence pattern (`RULESET.TESTS.feedback-loop-convergence`) now has a second, load-bearing instance beyond estimation, making it a demonstrated pattern rather than a one-off. +- **Positive — anti-vacuity guard.** The end-to-end test (`tests/loop-walk.qa.test.cjs`) asserts the walk produces at least one smell, because a QA harness reporting nothing on a first run against a real engine is more likely mis-specified than the engine is perfect. It deliberately does **not** assert exact smell ids or counts — pinning those would re-freeze current behavior, the same failure mode the severity split (§5) exists to avoid. +- **Negative — makes harder.** The walk is one more thing to keep in sync with the five-step loop's actual shape; `LOOP_HOST_CONTRACT` is consumed from the generated module rather than re-listed, specifically to prevent this from becoming a second copy that drifts (per the design's known-defect gauntlet). The `qa` suite's sequential-only execution bounds wall-clock cost as scenario count grows. +- **Known limits, stated rather than hidden.** + - The stub agent is not an LLM. The walk validates the engine's state machine; it says nothing about whether a real agent produces artifacts of the assumed shape. + - `ship` has no runnable predicate evaluator (`tests/loop-hooks-ship-pre-e2e.test.cjs`: "enforcement is ship.md prose only"), so the `ship` step is asserted at contract-shape level only, not behaviorally. + - `smart-entry` emits prose unconditionally on success (`gsd-core/bin/lib/smart-entry.cjs:577-587`), which is one of several untyped-success commands (`config-path`, `audit-open`, `skill-manifest`, and others). The routing oracle cannot assert on `smart-entry`'s routing decision until it gains a `--json` mode; that is future work, not covered here. + - The open `docs/json-errors.md` vs. `src/cli-exit.cts` contract conflict (§6) remains unresolved by this ADR. + +## References + +- `CONTEXT.md` → `RULESET.TESTS.feedback-loop-convergence` +- `tests/estimate-loop-convergence.test.cjs` — the existing convergence instance this ADR generalizes +- `tests/helpers.cjs` — `runGsdTools`, the substrate the walk is layered over +- `tests/qa/loop-walk.cjs`, `tests/qa/result.cjs`, `tests/qa/oracles.cjs`, `tests/qa/mutations.cjs`, `tests/qa/fixtures/**`, `tests/qa/scenarios/*.json` +- `tests/loop-walk.qa.test.cjs` +- `src/io.cts` — `output` (`output({error})` soft-failure idiom, 42 sites, `CRITICAL` per `get_impact`), `error` +- `src/state.cts:1388` — `cmdStateSnapshot` +- `src/cli-exit.cts` — `runMain`, the `ExitError` early-return that produces the §6 conflict +- `docs/json-errors.md` +- `CONTRIBUTING.md` → "Prohibited: Raw Text Matching on Test Outputs" +- `gsd-core/bin/lib/smart-entry.cjs:577-587` +- Issue #2371 (fixture provenance standard) +- `.gsd/phase/test-2966-loop-qa-walk/40-design.md` — the design this ADR records diff --git a/docs/adr/README.md b/docs/adr/README.md index a1da4c2fa..299ab6747 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -120,7 +120,7 @@ This replaces a hand-maintained table that had drifted to **40 of 65 ADRs** — -### Active decisions (54) +### Active decisions (55) These govern the system as it stands. Cite these. @@ -179,6 +179,7 @@ These govern the system as it stands. Cite these. | [ADR-2629](2629-phase-effort-estimation-calibration.md) | Phase effort is estimated against a calibrated smart-zone budget, not a static heuristic | Accepted | — | | [ADR-2719](2719-emitted-artifact-attribution.md) | Emitted-artifact attribution — replace the committed parity fixtures with a computed conservation law | Accepted | — | | [ADR-2782](2782-reviewer-lane-capability-surface.md) | Reviewer Lane — the cross-AI reviewer handoff becomes a declared capability surface | Accepted | — | +| [ADR-2966](2966-loop-qa-walk.md) | Test the five-step loop as a continuous walk, not isolated points | Accepted | — | | [ADR-3660](3660-runtime-artifact-layout-module.md) | Runtime Artifact Layout Module owns per-runtime artifact placement | Accepted | [ADR-1239](1239-gsd-embeddable-orchestration-engine.md) | ### Proposed (8) @@ -211,7 +212,7 @@ Historical record. **Do not follow these** — each names what replaced it, or w | [ADR-2264](2264-golden-parity-redesign.md) | Redesign golden-install-parity — single-source manifest builder + split invariant | Superseded | [ADR-2719](2719-emitted-artifact-attribution.md) | | [ADR-3524](3524-cjs-sdk-hard-seam.md) | CJS↔SDK hard seam — one source of truth per Shared Module | Superseded | [ADR-0174](0174-retire-gsd-sdk-package-boundary.md) | -_70 ADRs. Generated by `scripts/gen-adr-index.cjs` — run `--write` after adding or restatusing an ADR._ +_71 ADRs. Generated by `scripts/gen-adr-index.cjs` — run `--write` after adding or restatusing an ADR._ diff --git a/package.json b/package.json index 63509d162..2e755eb0f 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "hooks", "scripts", "!scripts/gen-emitted-baseline.cjs", + "!scripts/qa-smell-ratchet.cjs", "pi", "vscode" ], @@ -116,6 +117,7 @@ "lint:changeset": "node scripts/changeset/lint.cjs", "lint:generated-sync": "node scripts/gen-capability-registry.cjs --check && node scripts/gen-loop-host-contract.cjs --check && node scripts/gen-capability-matrix.cjs --check && node scripts/sync-manifest-versions.cjs --check && node scripts/gen-inventory-manifest.cjs --check && node scripts/generate-package-identity.cjs --check && node scripts/gen-plugin-skills.cjs --check && node scripts/gen-registry.cjs --check && node scripts/gen-adr-index.cjs --check && node scripts/check-glossary-refs.cjs --check && node scripts/lint-compiled-artifact-sync.cjs --check && node scripts/gen-context-index.cjs --check", "lint:docs": "node scripts/lint-docs-required.cjs", + "lint:qa-smells": "node scripts/qa-smell-ratchet.cjs", "lint:legacy-name": "node scripts/lint-legacy-dir-name.cjs", "ci:test-scope": "node scripts/ci-test-scope.cjs", "changeset": "node scripts/changeset/new.cjs", @@ -126,6 +128,7 @@ "test:install": "node scripts/run-tests.cjs --suite install", "test:security": "node scripts/run-tests.cjs --suite security", "test:slow": "node scripts/run-tests.cjs --suite slow", + "test:qa": "node scripts/run-tests.cjs --suite qa", "test:affected": "node scripts/run-affected-tests.cjs", "test:coverage": "c8 --check-coverage --lines 70 --branches 60 --reporter text --include 'gsd-core/bin/lib/*.cjs' --exclude 'tests/**' --all node scripts/run-tests.cjs", "test:coverage:scripts-floor": "c8 check-coverage --lines 55 --include 'scripts/**/*.cjs' --exclude 'tests/**' --all", diff --git a/scripts/qa-smell-ratchet.cjs b/scripts/qa-smell-ratchet.cjs new file mode 100644 index 000000000..20a0db622 --- /dev/null +++ b/scripts/qa-smell-ratchet.cjs @@ -0,0 +1,677 @@ +#!/usr/bin/env node +'use strict'; + +/** + * qa-smell-ratchet.cjs — turn a QA-walk "smell" into a decision (#2966). + * + * WHY THIS FILE EXISTS + * ──────────────────── + * `tests/qa/run-report.cjs` computes "smells" — legal-but-questionable engine + * behavior (see `oracles.cjs`'s `SEVERITY.SMELL`) — and writes them into a + * gitignored `qa-report.json` that nothing reads. In CI, that means every + * smell is invisible: a NEW one can appear silently and nobody notices. This + * script is the pipeline that turns a smell into a decision. + * + * ══════════════════════════════════════════════════════════════════════════ + * THE DESIGN INVARIANT (read this before touching anything below) + * ══════════════════════════════════════════════════════════════════════════ + * A smell must NEVER fail a build on its own merits. What fails is an + * UNACKNOWLEDGED NEW smell — i.e. the absence of a human decision. + * Existing/known smells stay green forever. + * + * Concretely, that means: + * - A smell whose fingerprint (`tests/qa/smell-fingerprint.cjs`) is already + * recorded in `tests/qa/smell-baseline.json` OR in any fragment under + * `tests/qa/smell-acks/` is KNOWN and never fails the build, no matter + * how many times it fires or how bad it sounds. + * - A smell whose fingerprint has never been seen before is NEW, and fails + * the build — not because the behavior is wrong (it may be perfectly + * fine), but because nobody has looked at it and said so in writing. + * - The baseline is SHRINK-ONLY: an entry that stops firing (the engine + * was fixed, or the scenario changed) becomes STALE and ALSO fails the + * build, forcing `--update` to prune it. A baseline that only ever grows + * would let acknowledgments outlive the behavior they describe. + * - A VIOLATION (`SEVERITY.VIOLATION` — the engine broke a documented + * contract) is a completely different thing and is NEVER acknowledgeable + * through this mechanism: it always fails, baseline or no baseline. This + * script's whole ratchet apparatus applies to smells alone. + * + * WHY A BASELINE FILE *AND* A FRAGMENTS DIRECTORY (not just one) + * ────────────────────────────────────────────────────────────── + * This follows the exact idiom `tests/emitted-drift-acks/` and `.changeset/` + * already use in this repo, for the exact same reason: `smell-baseline.json` + * is a single shared file every PR that acknowledges a smell would otherwise + * have to rewrite, guaranteeing merge conflicts between any two such PRs in + * flight at once. A fragment per PR under `tests/qa/smell-acks/` — uniquely + * named (its own issue/PR number) — means two PRs can never conflict on this + * seam. A maintainer periodically folds spent fragments into the committed + * baseline via `--update` and deletes them (see that directory's README). + * + * USAGE + * ───── + * node scripts/qa-smell-ratchet.cjs # check (CI entry point) + * node scripts/qa-smell-ratchet.cjs --update # regenerate the baseline + * node scripts/qa-smell-ratchet.cjs --json # also write the full qa-report + * node scripts/qa-smell-ratchet.cjs --keep # preserve scenario temp dirs + * # (real repro commands; see + * # `report.cjs`'s buildRepro) + * + * Exit code 0 only when: zero violations, zero NEW smells, zero STALE + * baseline/fragment entries. Exit code 1 otherwise. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const { runAllScenarios } = require('../tests/qa/run-report.cjs'); +const { buildReport } = require('../tests/qa/report.cjs'); +const { fingerprint } = require('../tests/qa/smell-fingerprint.cjs'); +const { ExitError, runMain } = require('./lib/cli-exit.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const BASELINE_REL_PATH = 'tests/qa/smell-baseline.json'; +const ACKS_DIR_REL_PATH = 'tests/qa/smell-acks'; +const BASELINE_PATH = path.join(REPO_ROOT, ...BASELINE_REL_PATH.split('/')); +const ACKS_DIR = path.join(REPO_ROOT, ...ACKS_DIR_REL_PATH.split('/')); + +/** Bump when `smell-baseline.json` / fragment shape changes incompatibly. */ +const BASELINE_VERSION = 1; + +/** + * Upper bound on fragment files read in one pass — mirrors the identical cap + * in `scripts/lint-emitted-drift-ack.cjs` (`MAX_ACK_FRAGMENTS`). Exceeding it + * throws rather than silently truncating the listing, which would silently + * drop acknowledgments from consideration — exactly the class of silent + * failure this whole seam exists to prevent. + */ +const MAX_ACK_FRAGMENTS = 500; + +/** + * `--update` writes this into a newly-discovered entry's OPTIONAL `reason` + * field (alongside `issue: null`) as a note-to-self, never as a substitute for + * `issue` — see the header's "THE DESIGN INVARIANT" and #2966 FIX 3. A plain + * (non-`--update`) run rejects any entry whose `issue` is not a positive + * integer regardless of what `reason` says, and additionally rejects a + * `reason` still carrying this placeholder prefix (see `isPlaceholderReason`), + * so the baseline can never silently ship with a smell nobody has triaged. + */ +const PLACEHOLDER_REASON_PREFIX = 'TODO(qa-smell-ratchet):'; +const PLACEHOLDER_REASON = + `${PLACEHOLDER_REASON_PREFIX} triage this smell — either file a defect and set "issue" to its number (REAL), ` + + 'or fix the oracle so it stops firing (FALSE POSITIVE). A "reason" alone, with no "issue", is never accepted.'; + +const isPlainObject = (v) => v !== null && typeof v === 'object' && !Array.isArray(v); +const isPlaceholderReason = (reason) => typeof reason === 'string' && reason.startsWith(PLACEHOLDER_REASON_PREFIX); + +/** + * Parse CLI argv into `{ update, jsonOut, keep }`. + * + * @param {string[]} argv + * @returns {{ update: boolean, jsonOut: string|null, keep: boolean }} + */ +function parseArgs(argv) { + let update = false; + let jsonOut = null; + let keep = false; + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]; + if (arg === '--update') { + update = true; + } else if (arg === '--json') { + const value = argv[i + 1]; + if (typeof value !== 'string' || value === '') { + throw new ExitError(2, 'qa-smell-ratchet: --json requires a path argument'); + } + jsonOut = path.resolve(value); + i += 1; + } else if (arg === '--keep') { + keep = true; + } else { + throw new ExitError( + 2, + `qa-smell-ratchet: unrecognized argument "${arg}" (expected --update, --json , and/or --keep)`, + ); + } + } + return { update, jsonOut, keep }; +} + +/** + * Validate one baseline/fragment entry, pushing a message per problem onto + * `errors`. Does not mutate `entry`. + * + * Every entry MUST carry the three string fields (`key`, `id`, `scenario`) + * AND a positive-integer `issue` — the ONLY two terminal states for a smell + * are REAL (an assigned defect, cited by its issue number) or FALSE POSITIVE + * (the oracle gets fixed and the entry is never baselined at all); there is + * no third "accepted with a good explanation" state, so a free-text `reason` + * can NEVER substitute for `issue` (#2966 FIX 3). `reason` remains an OPTIONAL + * human note: when present it must be a non-empty, non-placeholder string, + * but its absence is never itself an error. + * + * @param {unknown} entry + * @param {string} where human-readable location for error messages + * (e.g. `"tests/qa/smell-baseline.json.smells[3]"` or a fragment's own + * relative path). + * @param {string[]} errors + * @returns {boolean} true when `entry` has all required fields, a valid + * `issue`, and (if present) a real (non-placeholder) `reason`. + */ +function validateEntryFields(entry, where, errors) { + if (!isPlainObject(entry)) { + errors.push(`${where} must be an object, got ${JSON.stringify(entry)}`); + return false; + } + let ok = true; + for (const field of ['key', 'id', 'scenario']) { + if (typeof entry[field] !== 'string' || entry[field] === '') { + errors.push(`${where}.${field} must be a non-empty string, got ${JSON.stringify(entry[field])}`); + ok = false; + } + } + if (!Number.isInteger(entry.issue) || entry.issue <= 0) { + errors.push( + `${where}.issue must be a positive integer, got ${JSON.stringify(entry.issue)} — every acknowledged smell ` + + 'must be REAL (an assigned defect, cited by issue number) or a FALSE POSITIVE (the oracle is fixed, never ' + + 'baselined); a free-text "reason" can never substitute for a tracked issue number', + ); + ok = false; + } + if (entry.reason !== undefined) { + if (typeof entry.reason !== 'string' || entry.reason === '') { + errors.push(`${where}.reason, when present, must be a non-empty string, got ${JSON.stringify(entry.reason)}`); + ok = false; + } else if (isPlaceholderReason(entry.reason)) { + errors.push( + `${where}.reason is still the placeholder ("${entry.reason}") — either remove it or replace it with a ` + + 'real human note; either way, "issue" (not "reason") is what makes this entry valid', + ); + ok = false; + } + } + return ok; +} + +/** + * Read and validate `tests/qa/smell-baseline.json`. + * + * @param {{ allowMissing: boolean }} opts `allowMissing: true` is used only + * by `--update`'s bootstrap path — a not-yet-existing baseline is the + * expected first-run state there, never an error. In check mode a missing + * baseline is always an error (there is nothing to ratchet against). + * @returns {{ entries: Array<{key:string,id:string,scenario:string,issue:number,reason?:string}>, errors: string[], existed: boolean }} + */ +function readBaseline({ allowMissing }) { + const existed = fs.existsSync(BASELINE_PATH); + if (!existed) { + if (allowMissing) return { entries: [], errors: [], existed }; + return { + entries: [], + errors: [`${BASELINE_REL_PATH} is missing — run \`node scripts/qa-smell-ratchet.cjs --update\` to generate it`], + existed, + }; + } + + const raw = fs.readFileSync(BASELINE_PATH, 'utf8'); + if (raw.trim() === '') { + return { entries: [], errors: [`${BASELINE_REL_PATH} is present but empty`], existed }; + } + let doc; + try { + doc = JSON.parse(raw); + } catch (err) { + return { entries: [], errors: [`${BASELINE_REL_PATH} is not valid JSON: ${err.message}`], existed }; + } + const errors = []; + if (!isPlainObject(doc)) { + errors.push(`${BASELINE_REL_PATH} must be a JSON object, got ${Array.isArray(doc) ? 'array' : typeof doc}`); + return { entries: [], errors, existed }; + } + if (doc.version !== BASELINE_VERSION) { + errors.push(`${BASELINE_REL_PATH}: unsupported version ${JSON.stringify(doc.version)} (expected ${BASELINE_VERSION})`); + } + if (!Array.isArray(doc.smells)) { + errors.push(`${BASELINE_REL_PATH}: "smells" must be an array, got ${JSON.stringify(doc.smells)}`); + return { entries: [], errors, existed }; + } + + const entries = []; + doc.smells.forEach((entry, i) => { + const where = `${BASELINE_REL_PATH}.smells[${i}]`; + if (validateEntryFields(entry, where, errors)) entries.push(entry); + }); + return { entries, errors, existed }; +} + +/** + * Fragment filenames under `tests/qa/smell-acks/`, sorted. Absent directory + * == zero fragments. Throws (naming the dir, cap, and actual count) rather + * than silently truncating when the cap is exceeded. + * + * @returns {string[]} + */ +function listFragmentFiles() { + if (!fs.existsSync(ACKS_DIR)) return []; + const names = fs.readdirSync(ACKS_DIR).filter((n) => n.endsWith('.json')).sort(); + if (names.length > MAX_ACK_FRAGMENTS) { + throw new ExitError( + 1, + `qa-smell-ratchet: ${ACKS_DIR_REL_PATH} contains ${names.length} ack fragments, exceeding the cap of ` + + `${MAX_ACK_FRAGMENTS}. Refusing to read only some of them — a truncated read would silently drop ` + + 'acknowledgments. Prune spent fragments from this directory.', + ); + } + return names; +} + +/** + * Read and validate every fragment under `tests/qa/smell-acks/`. Each + * fragment is ONE acknowledgment: the same shape as a baseline entry — `key`, + * `id`, `scenario`, a positive-integer `issue`, and an OPTIONAL `reason` — + * validated identically via `validateEntryFields` (#2966 FIX 3: there is no + * separate "acknowledge via PR number" path; every acknowledgment cites the + * issue tracking the underlying defect). + * + * @returns {{ entries: Array<{key:string,id:string,scenario:string,issue:number,reason?:string,_source:string}>, errors: string[] }} + */ +function readAckFragments() { + const errors = []; + const entries = []; + for (const name of listFragmentFiles()) { + const label = `${ACKS_DIR_REL_PATH}/${name}`; + const raw = fs.readFileSync(path.join(ACKS_DIR, name), 'utf8'); + if (raw.trim() === '') { + errors.push(`${label} is present but empty`); + continue; + } + let doc; + try { + doc = JSON.parse(raw); + } catch (err) { + errors.push(`${label} is not valid JSON: ${err.message}`); + continue; + } + if (!validateEntryFields(doc, label, errors)) continue; + entries.push({ ...doc, _source: label }); + } + return { entries, errors }; +} + +/** + * Merge baseline entries and ack fragments into one `key -> entry` map (the + * full set of KNOWN smells this run is ratcheted against), plus the list of + * fragments that are now redundant because the baseline already carries + * their key (an advisory, not a failure — see this file's header on why + * cross-source duplication is not hard-blocked here). + * + * @param {Array<{key:string}>} baselineEntries + * @param {Array<{key:string,_source:string}>} fragmentEntries + * @returns {{ byKey: Map, redundantFragments: Array<{key:string, source:string}> }} + */ +function mergeKnown(baselineEntries, fragmentEntries) { + const byKey = new Map(); + for (const e of baselineEntries) byKey.set(e.key, { ...e, source: BASELINE_REL_PATH }); + + const redundantFragments = []; + for (const e of fragmentEntries) { + if (byKey.has(e.key)) { + redundantFragments.push({ key: e.key, source: e._source }); + continue; + } + byKey.set(e.key, { ...e, source: e._source }); + } + return { byKey, redundantFragments }; +} + +/** + * Walk `reportObject.scenarios[].steps[]` and split every finding into + * `smells` (fingerprinted) and `violations` (never acknowledgeable — see + * this file's header). Both carry the step's `repro` command for later use + * in failure messages / the GitHub step summary. + * + * @param {ReturnType} reportObject + * @returns {{ + * smells: Array<{key:string,id:string,scenario:string,argv:string[],detail:string,at:string,repro:string}>, + * violations: Array<{id:string,scenario:string,argv:string[],detail:string,at:string,repro:string}>, + * }} + */ +function collectFindings(reportObject) { + const smells = []; + const violations = []; + for (const scenario of reportObject.scenarios) { + for (const step of scenario.steps) { + for (const v of step.violations || []) { + violations.push({ + id: v.id, scenario: scenario.name, argv: step.argv, detail: v.detail, at: step.at, repro: step.repro, + }); + } + for (const smell of step.smells || []) { + const key = fingerprint(scenario.name, { id: smell.id, subject: smell.subject, argv: step.argv }); + smells.push({ + key, + id: smell.id, + scenario: scenario.name, + argv: step.argv, + detail: smell.detail, + at: step.at, + repro: step.repro, + }); + } + } + } + return { smells, violations }; +} + +/** Lowercase, hyphenate, and strip anything that isn't `[a-z0-9-]`, for a fragment-filename skeleton. */ +function slugify(value) { + return value + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') + .slice(0, 60); +} + +/** + * Render the paste-ready fragment skeleton for one NEW smell finding. + * + * @param {{key:string,id:string,scenario:string}} finding + * @returns {string} + */ +function fragmentSkeleton(finding) { + const doc = { + version: 1, + key: finding.key, + id: finding.id, + scenario: finding.scenario, + issue: '', + }; + const suggestedName = `${ACKS_DIR_REL_PATH}/-${slugify(finding.id)}-${slugify(finding.scenario)}.json`; + return `${suggestedName}:\n${JSON.stringify(doc, null, 2)}`; +} + +/** + * Build the markdown block appended to `GITHUB_STEP_SUMMARY`, when set — + * kept intentionally compact (a PR reviewer's first read, not a log dump). + * + * @param {{ + * smells: ReturnType['smells'], + * violations: ReturnType['violations'], + * newKeys: string[], + * staleEntries: Array<{key:string,id:string,scenario:string,source:string}>, + * smellSummary: Array<{id:string,count:number,examples:string[]}>, + * }} data + * @returns {string} + */ +function buildStepSummaryMarkdown({ smells, violations, newKeys, staleEntries, smellSummary }) { + const lines = []; + lines.push('## QA smell ratchet'); + lines.push(''); + lines.push( + `**${smells.length} smells** (${newKeys.length} new, ${staleEntries.length} stale) · ` + + `**${violations.length} violations**`, + ); + lines.push(''); + + if (newKeys.length) { + lines.push('### 🚨 NEW (unacknowledged) smells'); + lines.push(''); + for (const key of newKeys) { + const f = smells.find((s) => s.key === key); + lines.push(`- \`${f.id}\` in **${f.scenario}** — ${f.detail}`); + } + lines.push(''); + } + + if (staleEntries.length) { + lines.push('### Stale baseline/fragment entries (no longer produced)'); + lines.push(''); + for (const e of staleEntries) { + lines.push(`- \`${e.id}\` in **${e.scenario}** (${e.source})`); + } + lines.push(''); + } + + if (smellSummary.length) { + lines.push('### Smells by oracle'); + lines.push(''); + lines.push('| oracle id | count |'); + lines.push('|---|---|'); + for (const entry of smellSummary) { + lines.push(`| \`${entry.id}\` | ${entry.count} |`); + } + lines.push(''); + } + + const firstFailingRepro = (violations[0] && violations[0].repro) + || (newKeys.length && smells.find((s) => s.key === newKeys[0]).repro); + if (firstFailingRepro) { + lines.push('### Repro (first failing step)'); + lines.push(''); + lines.push('```sh'); + lines.push(firstFailingRepro); + lines.push('```'); + lines.push(''); + } + + return lines.join('\n'); +} + +function main() { + const { update, jsonOut, keep } = parseArgs(process.argv.slice(2)); + + const scenarioReports = runAllScenarios({ keep }); + const meta = { + nodeVersion: process.version, + platform: process.platform, + // Only ever used for report METADATA (and, when --json is passed, the + // written artifact's meta.generatedAt) — never fed into a fingerprint or + // into smell-baseline.json, which is what keeps this script's fingerprint + // and baseline output deterministic despite this one real clock read. + generatedAt: new Date().toISOString(), + }; + const reportObject = buildReport(scenarioReports, meta); + + if (jsonOut) { + fs.mkdirSync(path.dirname(jsonOut), { recursive: true }); + fs.writeFileSync(jsonOut, `${JSON.stringify(reportObject, null, 2)}\n`, 'utf8'); + } + + const { smells, violations } = collectFindings(reportObject); + const runKeys = new Set(smells.map((s) => s.key)); + + const baseline = readBaseline({ allowMissing: update }); + const fragments = readAckFragments(); + const sourceErrors = [...baseline.errors, ...fragments.errors]; + + if (update) { + if (sourceErrors.length) { + for (const e of sourceErrors) console.error(` - ${e}`); + throw new ExitError( + 1, + `qa-smell-ratchet --update: ${sourceErrors.length} problem(s) in existing baseline/fragment source(s) ` + + '(printed above) — fix or delete the offending source(s) by hand before regenerating.', + ); + } + + const { byKey: knownBeforeUpdate } = mergeKnown(baseline.entries, fragments.entries); + const oldBaselineKeys = new Set(baseline.entries.map((e) => e.key)); + + // `--update` NEVER invents an issue number (#2966 FIX 3). A key already + // carrying a real `issue` (from the committed baseline or a fragment) keeps + // it, along with its `reason` if any. A genuinely NEW smell — no prior + // acknowledgment exists — gets `issue: null` and a TODO `reason`; the very + // next plain (non-`--update`) run REJECTS that entry, forcing a human to + // triage it as REAL (cite the issue) or FALSE POSITIVE (fix the oracle). + const newBaselineEntries = [...runKeys].sort().map((key) => { + const representative = smells.find((s) => s.key === key); + const carried = knownBeforeUpdate.get(key); + const hasKnownIssue = !!carried && Number.isInteger(carried.issue) && carried.issue > 0; + const entry = { + key, + id: representative.id, + scenario: representative.scenario, + issue: hasKnownIssue ? carried.issue : null, + }; + if (hasKnownIssue && typeof carried.reason === 'string' && !isPlaceholderReason(carried.reason)) { + entry.reason = carried.reason; + } else if (!hasKnownIssue) { + entry.reason = PLACEHOLDER_REASON; + } + return entry; + }); + const newBaselineKeys = new Set(newBaselineEntries.map((e) => e.key)); + + const added = [...newBaselineKeys].filter((k) => !oldBaselineKeys.has(k)).sort(); + const removed = [...oldBaselineKeys].filter((k) => !newBaselineKeys.has(k)).sort(); + + fs.mkdirSync(path.dirname(BASELINE_PATH), { recursive: true }); + fs.writeFileSync( + BASELINE_PATH, + `${JSON.stringify({ version: BASELINE_VERSION, smells: newBaselineEntries }, null, 2)}\n`, + 'utf8', + ); + + console.log( + `qa-smell-ratchet --update: ${oldBaselineKeys.size} -> ${newBaselineKeys.size} baseline entries` + + (added.length ? ` | added: ${added.length}` : '') + + (removed.length ? ` | removed: ${removed.length}` : ''), + ); + for (const key of added) { + const e = newBaselineEntries.find((x) => x.key === key); + const placeholderNote = e.issue === null ? ' [issue: null — TODO, needs triage before the next check run]' : ''; + console.log(` + ${key}${placeholderNote}`); + } + for (const key of removed) console.log(` - ${key}`); + + const redundant = fragments.entries.filter((e) => newBaselineKeys.has(e.key)); + if (redundant.length) { + console.log( + `\n${redundant.length} fragment(s) are now redundant — their key is already in the regenerated baseline. ` + + 'Delete them (CONTRIBUTING.md fragment idiom: fold, then delete):', + ); + for (const e of redundant) console.log(` - ${e._source}`); + } + + if (process.env.GITHUB_STEP_SUMMARY) { + const md = buildStepSummaryMarkdown({ + smells, + violations, + newKeys: added, + staleEntries: removed.map((key) => ({ key, id: '(pruned)', scenario: '(pruned)', source: BASELINE_REL_PATH })), + smellSummary: reportObject.smellSummary, + }); + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${md}\n`); + } + + console.log( + `\nqa-smell-ratchet: ${smells.length} smells (${added.length} new, ${removed.length} stale), ` + + `${violations.length} violations`, + ); + + if (violations.length) { + printViolations(violations); + throw new ExitError(1, 'qa-smell-ratchet --update: baseline regenerated, but VIOLATIONS remain (never acknowledgeable — see above)'); + } + return; + } + + // ── check mode ────────────────────────────────────────────────────────── + const { byKey: known, redundantFragments } = mergeKnown(baseline.entries, fragments.entries); + const newKeys = [...runKeys].filter((k) => !known.has(k)).sort(); + const staleKeys = [...known.keys()].filter((k) => !runKeys.has(k)).sort(); + const staleEntries = staleKeys.map((key) => known.get(key)); + + if (sourceErrors.length) { + console.error(`qa-smell-ratchet: ${sourceErrors.length} problem(s) in baseline/fragment source(s):\n`); + for (const e of sourceErrors) console.error(` - ${e}`); + } + + if (violations.length) { + printViolations(violations); + } + + if (newKeys.length) { + console.error(`\nqa-smell-ratchet: ${newKeys.length} NEW (unacknowledged) smell(s):\n`); + for (const key of newKeys) { + const f = smells.find((s) => s.key === key); + console.error(`NEW smell: ${f.key}`); + console.error(` oracle: ${f.id}`); + console.error(` scenario: ${f.scenario}`); + console.error(` detail: ${f.detail}`); + console.error(' remedy: exactly two options — no third "accepted with an explanation" state:'); + console.error(' 1. fix the detector if this is a FALSE POSITIVE (the oracle is wrong; make it stop firing);'); + console.error(' 2. file a defect and add an entry citing its issue number (REAL) — a fragment:\n'); + console.error(`${fragmentSkeleton(f).split('\n').map((l) => ` ${l}`).join('\n')}\n`); + } + } + + if (staleKeys.length) { + console.error(`\nqa-smell-ratchet: ${staleKeys.length} STALE baseline/fragment entr${staleKeys.length === 1 ? 'y' : 'ies'} (no longer produced by the run):\n`); + for (const e of staleEntries) { + console.error(`STALE entry: ${e.key}`); + console.error(` source: ${e.source}`); + console.error(` oracle: ${e.id}`); + console.error(` scenario: ${e.scenario}`); + console.error(` issue: ${e.issue}`); + if (e.reason !== undefined) console.error(` reason: ${e.reason}`); + } + console.error('\n remedy: node scripts/qa-smell-ratchet.cjs --update'); + } + + if (redundantFragments.length) { + console.log( + `\n${redundantFragments.length} fragment(s) are already covered by the baseline and can be deleted:`, + ); + for (const e of redundantFragments) console.log(` - ${e.source} (key ${e.key})`); + } + + if (process.env.GITHUB_STEP_SUMMARY) { + const md = buildStepSummaryMarkdown({ + smells, + violations, + newKeys, + staleEntries, + smellSummary: reportObject.smellSummary, + }); + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, `${md}\n`); + } + + console.log( + `\nqa-smell-ratchet: ${smells.length} smells (${newKeys.length} new, ${staleKeys.length} stale), ` + + `${violations.length} violations`, + ); + + if (sourceErrors.length || violations.length || newKeys.length || staleKeys.length) { + throw new ExitError(1); + } +} + +/** + * @param {ReturnType['violations']} violations + */ +function printViolations(violations) { + console.error(`qa-smell-ratchet: ${violations.length} VIOLATION(s) — never acknowledgeable, always fail:\n`); + for (const v of violations) { + console.error(`VIOLATION: ${v.id}`); + console.error(` scenario: ${v.scenario}`); + console.error(` argv: ${v.argv.join(' ')}`); + console.error(` detail: ${v.detail}`); + console.error(` repro: ${v.repro}`); + } +} + +runMain(main); + +module.exports = { + parseArgs, + readBaseline, + readAckFragments, + mergeKnown, + collectFindings, + fragmentSkeleton, + slugify, + isPlaceholderReason, + PLACEHOLDER_REASON_PREFIX, + BASELINE_REL_PATH, + ACKS_DIR_REL_PATH, + MAX_ACK_FRAGMENTS, +}; diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 36379cf15..4d9912127 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -11,6 +11,7 @@ // node scripts/run-tests.cjs --suite integration # *.integration.test.cjs // node scripts/run-tests.cjs --suite install # *.install.test.cjs // node scripts/run-tests.cjs --suite slow # *.slow.test.cjs +// node scripts/run-tests.cjs --suite qa # *.qa.test.cjs // node scripts/run-tests.cjs --files "a.test.cjs b.test.cjs" // node scripts/run-tests.cjs --files-from /tmp/selected-tests.txt // node scripts/run-tests.cjs --suite unit --shard 1/3 # shard 1 of 3 (#1212) @@ -39,7 +40,7 @@ const { join, basename } = require('path'); const { execFileSync } = require('child_process'); const { ExitError, runMain } = require('./lib/cli-exit.cjs'); -const SUITES = ['all', 'unit', 'integration', 'install', 'security', 'slow']; +const SUITES = ['all', 'unit', 'integration', 'install', 'security', 'slow', 'qa']; // ADR-457 build-at-publish: gsd-core/bin/lib/*.cjs is generated from // src/*.cts and gitignored, so on a clean checkout (fresh CI, before any build) @@ -169,7 +170,7 @@ function ensureBuiltHooks(overrides = {}) { runBuild(); } } -const MARKED_SUITES = ['integration', 'install', 'security', 'slow']; +const MARKED_SUITES = ['integration', 'install', 'security', 'slow', 'qa']; // Recursively collect *.test.cjs files under dir, returning paths relative to dir. // Skips node_modules to avoid accidentally picking up decoy files. diff --git a/tests/fixtures/index.cjs b/tests/fixtures/index.cjs index 25862a95b..2bea76185 100644 --- a/tests/fixtures/index.cjs +++ b/tests/fixtures/index.cjs @@ -41,7 +41,11 @@ function createFixture(options = {}) { execSync('git config user.name "Test"', { cwd: tmpDir, stdio: 'pipe' }); execSync('git config commit.gpgsign false', { cwd: tmpDir, stdio: 'pipe' }); execSync('git add -A', { cwd: tmpDir, stdio: 'pipe' }); - execSync('git commit -m "initial commit"', { cwd: tmpDir, stdio: 'pipe' }); + // `--allow-empty`: a fixture with `git: true, planning: false, + // projectDoc: false` (e.g. a "greenfield" starting world) stages nothing, + // so a plain `git commit` would fail with "nothing to commit" and the + // caller would never get a usable repo (there'd be no HEAD at all). + execSync('git commit --allow-empty -m "initial commit"', { cwd: tmpDir, stdio: 'pipe' }); } return tmpDir; diff --git a/tests/loop-walk.qa.test.cjs b/tests/loop-walk.qa.test.cjs new file mode 100644 index 000000000..448d40f86 --- /dev/null +++ b/tests/loop-walk.qa.test.cjs @@ -0,0 +1,1336 @@ +'use strict'; + +/** + * loop-walk.qa.test.cjs — self-tests for the loop QA walk harness itself + * (`tests/qa/{result,oracles,loop-walk,mutations,scenario,fixtures/index}.cjs`). + * + * This file proves the harness's own building blocks behave as documented: + * the `RunResult` classifier, every oracle (both its pass AND its fail path — + * an oracle that cannot fail is decoration), the scenario DSL's validation, + * fixture-ref resolution, the mutation catalog, and finally a real end-to-end + * walk of the greenfield-happy-path scenario against the actual CLI. + */ + +const { describe, test, before, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const { execFile } = require('node:child_process'); +const { promisify } = require('node:util'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); +const { getLiveCommandTokens } = require('./helpers/live-command-registry.cjs'); + +const { KIND, classify } = require('./qa/result.cjs'); +const { ORACLES, runOracles, SEVERITY } = require('./qa/oracles.cjs'); +const { LoopWalk } = require('./qa/loop-walk.cjs'); +const { MUTATIONS, apply, NOOP } = require('./qa/mutations.cjs'); +const { loadScenario, runScenario, assertWiringIsLive } = require('./qa/scenario.cjs'); +const { resolveRef } = require('./qa/fixtures/index.cjs'); +const { resolveWithin, resolveForCompare } = require('./qa/paths.cjs'); +const { LOOP_HOST_CONTRACT } = require('../gsd-core/bin/lib/loop-host-contract.cjs'); +const { extractFrontmatter } = require('../gsd-core/bin/lib/frontmatter.cjs'); +const { evaluateUatPassed } = require('../gsd-core/bin/lib/uat-predicate.cjs'); + +const execFileAsync = promisify(execFile); + +/** + * Recursively collect every `.md` file under `dir` (absolute paths). + * + * @param {string} dir + * @param {string[]} [out] + * @returns {string[]} + */ +function collectFixtureMarkdownFiles(dir, out = []) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const abs = path.join(dir, entry.name); + if (entry.isDirectory()) { + collectFixtureMarkdownFiles(abs, out); + } else if (entry.isFile() && abs.endsWith('.md')) { + out.push(abs); + } + } + return out; +} + +/** + * Is `content` conceptually "a document with a frontmatter block", ignoring any + * leading blank lines and/or leading HTML comment(s) (e.g. the #2371 provenance + * marker)? + * + * WHY skip leading comments/blanks rather than requiring byte-0 `---`: the whole + * point of this predicate is to catch DEFECT 1's shape — a provenance comment + * placed BEFORE the frontmatter fence, which makes `extractFrontmatter` (which + * only recognizes `---` at byte 0, `gsd-core/bin/lib/frontmatter.cjs`) silently + * return `{}`. A predicate that itself required byte-0 `---` would only ever + * fire on already-correct fixtures and could never catch this regression class — + * it would be exactly as vacuous as the bug it exists to guard against. + * + * @param {string} content + * @returns {boolean} + */ +function hasFrontmatterShape(content) { + const lines = content.split(/\r?\n/); + let i = 0; + let inComment = false; + while (i < lines.length) { + const line = lines[i]; + if (inComment) { + if (line.includes('-->')) inComment = false; + i += 1; + continue; + } + const trimmed = line.trim(); + if (trimmed === '') { + i += 1; + continue; + } + if (trimmed.startsWith('')) inComment = true; + i += 1; + continue; + } + break; + } + return lines[i] === '---'; +} + +/** Looks up an oracle by id, failing loudly if the catalog ever drops one. */ +function getOracle(id) { + const found = ORACLES.find((o) => o.id === id); + assert.ok(found, `test setup: oracle "${id}" not found in ORACLES`); + return found; +} + +describe('RunResult classification', () => { + test('classifies a JSON object at exit 0 as JSON', () => { + const raw = { exitCode: 0, stdout: JSON.stringify({ total_plans: 3 }), stderr: '', argv: ['progress'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.deepStrictEqual(result.json, { total_plans: 3 }); + }); + + test('classifies non-JSON stdout at exit 0 as PROSE', () => { + const raw = { exitCode: 0, stdout: 'Project initialized successfully.', stderr: '', argv: ['init'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.PROSE); + }); + + test('classifies empty stdout and stderr at exit 0 as EMPTY', () => { + const raw = { exitCode: 0, stdout: '', stderr: '', argv: ['noop'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.EMPTY); + }); + + test('classifies a JSON payload carrying an "error" key at exit 0 as SOFT_ERROR', () => { + const raw = { exitCode: 0, stdout: JSON.stringify({ error: 'no phases found' }), stderr: '', argv: ['progress'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.SOFT_ERROR); + }); + + test('classifies exit 1 with a warning line before the JSON envelope as STRUCTURED_ERROR', () => { + const stderr = [ + 'gsd-tools: warning: unknown config key(s) in .planning/config.json: foo', + JSON.stringify({ ok: false, reason: 'bad-config', message: 'config invalid' }), + ].join('\n'); + const raw = { exitCode: 1, stdout: '', stderr, argv: ['review-lane'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.STRUCTURED_ERROR); + assert.strictEqual(result.err.reason, 'bad-config'); + }); + + test('classifies non-JSON stderr at exit 1 as UNSTRUCTURED_ERROR', () => { + const raw = { exitCode: 1, stdout: '', stderr: 'Fatal: something went wrong', argv: ['bad'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.UNSTRUCTURED_ERROR); + }); + + test('classifies an exit code outside {0,1} as UNEXPECTED_EXIT', () => { + const raw = { exitCode: 2, stdout: '', stderr: '', argv: ['weird'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.UNEXPECTED_EXIT); + }); + + test('classifies a timed-out invocation as TIMEOUT regardless of exit code', () => { + const raw = { exitCode: null, stdout: '', stderr: '', timedOut: true, argv: ['slow'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.TIMEOUT); + }); + + test('classifies a bare JSON number 0 as JSON (non-object scalar)', () => { + const raw = { exitCode: 0, stdout: '0', stderr: '', argv: ['x'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.strictEqual(result.json, 0); + }); + + test('classifies a bare JSON string "s" as JSON (non-object scalar)', () => { + const raw = { exitCode: 0, stdout: '"s"', stderr: '', argv: ['x'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.strictEqual(result.json, 's'); + }); + + test('classifies a bare JSON array [] as JSON (not probed for an error key)', () => { + const raw = { exitCode: 0, stdout: '[]', stderr: '', argv: ['x'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.deepStrictEqual(result.json, []); + }); + + test('classifies a bare JSON null as JSON, not SOFT_ERROR', () => { + const raw = { exitCode: 0, stdout: 'null', stderr: '', argv: ['x'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.strictEqual(result.json, null); + }); + + test('classifies a bare JSON true as JSON', () => { + const raw = { exitCode: 0, stdout: 'true', stderr: '', argv: ['x'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.strictEqual(result.json, true); + }); + + test('exit 1 with a healthy JSON-looking stdout still classifies as an error (exit code outranks payload)', () => { + const raw = { exitCode: 1, stdout: JSON.stringify({ ok: true, total_plans: 5 }), stderr: '', argv: ['progress'] }; + const result = classify(raw); + assert.notStrictEqual(result.kind, KIND.JSON); + assert.strictEqual(result.kind, KIND.UNSTRUCTURED_ERROR); + }); + + test('warnings array captures all stderr lines except the last', () => { + const stderr = ['line1', 'line2', JSON.stringify({ ok: false, reason: 'r', message: 'm' })].join('\n'); + const raw = { exitCode: 1, stdout: '', stderr, argv: ['x'] }; + const result = classify(raw); + assert.deepStrictEqual(result.warnings, ['line1', 'line2']); + }); + + test('an @file: pointer is followed and its JSON payload parsed', (t) => { + const dir = createTempDir('gsd-runresult-pointer-'); + t.after(() => cleanup(dir)); + const payloadPath = path.join(dir, 'payload.json'); + fs.writeFileSync(payloadPath, JSON.stringify({ ok: true, phases: 4 }), 'utf-8'); + const raw = { exitCode: 0, stdout: `@file:${payloadPath}`, stderr: '', argv: ['big-output'] }; + const result = classify(raw); + assert.strictEqual(result.kind, KIND.JSON); + assert.strictEqual(result.pointer, payloadPath); + assert.deepStrictEqual(result.json, { ok: true, phases: 4 }); + }); + + test('an unreadable @file: pointer classifies as UNSTRUCTURED_ERROR rather than throwing', () => { + const pointerPath = '/definitely/not/a/real/path-xyz.json'; + const raw = { exitCode: 0, stdout: `@file:${pointerPath}`, stderr: '', argv: ['big-output'] }; + const io = { + readFileSync: () => { + throw new Error('injected: pointee unreadable'); + }, + }; + const result = classify(raw, io); + assert.strictEqual(result.kind, KIND.UNSTRUCTURED_ERROR); + assert.strictEqual(result.pointer, pointerPath); + }); +}); + +describe('oracle self-tests', () => { + test('ORACLES has exactly 10 entries (7 violation-severity + 3 smell-severity)', () => { + assert.strictEqual(ORACLES.length, 10); + }); + + test('exit-contract passes on a clean context', () => { + const outcome = getOracle('exit-contract').check({ result: { kind: KIND.JSON } }); + assert.strictEqual(outcome.ok, true); + }); + + test('exit-contract fails on a broken context (TIMEOUT)', () => { + const outcome = getOracle('exit-contract').check({ result: { kind: KIND.TIMEOUT } }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('json-contract passes on a clean context (well-formed STRUCTURED_ERROR)', () => { + const outcome = getOracle('json-contract').check({ + result: { kind: KIND.STRUCTURED_ERROR, err: { ok: false, reason: 'bad-thing' } }, + }); + assert.strictEqual(outcome.ok, true); + }); + + test('json-contract fails on a broken context (UNSTRUCTURED_ERROR)', () => { + const outcome = getOracle('json-contract').check({ result: { kind: KIND.UNSTRUCTURED_ERROR } }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('value-hygiene passes on a clean context', () => { + const outcome = getOracle('value-hygiene').check({ result: { json: { a: 1, note: 'fine' } } }); + assert.strictEqual(outcome.ok, true); + }); + + test('value-hygiene VIOLATION on a NaN leaf', () => { + const outcome = getOracle('value-hygiene').check({ result: { json: { a: Number.NaN } } }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('value-hygiene VIOLATION on each coercion-artifact sentinel string', () => { + for (const sentinel of ['undefined', 'null', 'NaN', '[object Object]']) { + const outcome = getOracle('value-hygiene').check({ result: { json: { a: sentinel } } }); + assert.strictEqual(outcome.ok, false, `sentinel ${JSON.stringify(sentinel)} should violate`); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + } + }); + + test('value-hygiene SMELL (not a violation) on an absolute path outside ctx.projectDir', (t) => { + const projectDir = createTempDir('gsd-hygiene-outside-'); + const outsideDir = createTempDir('gsd-hygiene-outside-sibling-'); + t.after(() => { + cleanup(projectDir); + cleanup(outsideDir); + }); + const outcome = getOracle('value-hygiene').check({ + result: { json: { p: path.join(outsideDir, 'leak.md') } }, + projectDir, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.SMELL); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + // Confirm the severity split is honored end-to-end via runOracles too. + const { violations, smells } = runOracles({ result: { json: { p: path.join(outsideDir, 'leak.md') } }, projectDir }); + assert.strictEqual(violations.some((v) => v.id === 'value-hygiene'), false); + assert.strictEqual(smells.some((s) => s.id === 'value-hygiene'), true); + }); + + test('value-hygiene has no finding for an in-project path, including one that does not exist yet', (t) => { + const projectDir = createTempDir('gsd-hygiene-inproject-'); + t.after(() => cleanup(projectDir)); + // `init` returns paths for files the agent has not written yet — realpath + // throws ENOENT on those, and the oracle must not crash or false-positive. + const notYetWritten = path.join(projectDir, '.planning', 'NOT-YET.md'); + let outcome; + assert.doesNotThrow(() => { + outcome = getOracle('value-hygiene').check({ result: { json: { p: notYetWritten } }, projectDir }); + }); + assert.strictEqual(outcome.ok, true); + }); + + test('value-hygiene has no finding when ctx.projectDir is absent (skips, does not guess)', () => { + const outcome = getOracle('value-hygiene').check({ result: { json: { p: '/some/unrelated/absolute/path' } } }); + assert.strictEqual(outcome.ok, true); + }); + + test('value-hygiene has no finding (not even a SMELL) for an allowlisted external-path key like agents_dir', (t) => { + const projectDir = createTempDir('gsd-hygiene-allowlist-'); + const outsideDir = createTempDir('gsd-hygiene-allowlist-outside-'); + t.after(() => { + cleanup(projectDir); + cleanup(outsideDir); + }); + const outcome = getOracle('value-hygiene').check({ + result: { json: { agents_dir: path.join(outsideDir, 'agents') } }, + projectDir, + }); + assert.strictEqual(outcome.ok, true); + const { violations, smells } = runOracles({ + result: { json: { agents_dir: path.join(outsideDir, 'agents') } }, + projectDir, + }); + assert.strictEqual(violations.some((v) => v.id === 'value-hygiene'), false); + assert.strictEqual(smells.some((s) => s.id === 'value-hygiene'), false); + }); + + test('value-hygiene still SMELLs on a non-allowlisted out-of-project key even when agents_dir is also present', (t) => { + const projectDir = createTempDir('gsd-hygiene-mixed-'); + const outsideDir = createTempDir('gsd-hygiene-mixed-outside-'); + t.after(() => { + cleanup(projectDir); + cleanup(outsideDir); + }); + const outcome = getOracle('value-hygiene').check({ + result: { + json: { + agents_dir: path.join(outsideDir, 'agents'), + leaked_path: path.join(outsideDir, 'leak.md'), + }, + }, + projectDir, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.SMELL); + assert.strictEqual(outcome.subject.key, '$.leaked_path'); + }); + + test('value-hygiene SMELLs on a sibling-prefix path (containment is path-segment, not string-prefix)', (t) => { + const projectDir = createTempDir('gsd-hygiene-sibling-'); + t.after(() => cleanup(projectDir)); + // `${projectDir}-evil` starts with the exact same characters as `projectDir`, + // so a naive string-prefix/`.startsWith()` containment check would (wrongly) + // treat it as inside. Path-segment containment must not. + const siblingPath = path.join(`${projectDir}-evil`, 'x'); + const outcome = getOracle('value-hygiene').check({ result: { json: { p: siblingPath } }, projectDir }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.SMELL); + }); + + test('value-hygiene does not crash on a cyclic json object', () => { + const cyclic = {}; + cyclic.self = cyclic; + let outcome; + assert.doesNotThrow(() => { + outcome = getOracle('value-hygiene').check({ result: { json: cyclic } }); + }); + assert.strictEqual(outcome.ok, true); + }); + + test('value-hygiene does not crash on non-object json', () => { + const outcome = getOracle('value-hygiene').check({ result: { json: 'just a plain string' } }); + assert.strictEqual(outcome.ok, true); + }); + + test('read-only-idempotence passes on a clean context', () => { + const outcome = getOracle('read-only-idempotence').check({ + readOnly: true, + result: { json: { a: 1 } }, + repeatResult: { json: { a: 1 } }, + statsBefore: new Map([['f.md', { size: 10, mtimeMs: 100 }]]), + statsAfter: new Map([['f.md', { size: 10, mtimeMs: 100 }]]), + }); + assert.strictEqual(outcome.ok, true); + }); + + test('read-only-idempotence fails on a broken context (repeatResult.json diverges)', () => { + const outcome = getOracle('read-only-idempotence').check({ + readOnly: true, + result: { json: { a: 1 } }, + repeatResult: { json: { a: 2 } }, + statsBefore: new Map([['f.md', { size: 10, mtimeMs: 100 }]]), + statsAfter: new Map([['f.md', { size: 10, mtimeMs: 100 }]]), + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('monotonic-progress passes on a clean context', () => { + const outcome = getOracle('monotonic-progress').check({ + history: [{ json: { total_plans: 1 } }], + result: { json: { total_plans: 3 } }, + }); + assert.strictEqual(outcome.ok, true); + }); + + test('monotonic-progress fails on a broken context (value decreased)', () => { + // Neither observation carries milestone_version/milestone_name at all (branch 2 of the + // three-way scope rule, #2966 FIX 2b) — they share the same (absent) scope by construction, + // so this MUST still compare normally and violate. A prior fix over-broadened the "skip when + // scope is unknowable" rule to skip ANY scope-less payload, which silently disabled this exact + // check for a minimal payload like this one; only the full remote suite caught it. + const outcome = getOracle('monotonic-progress').check({ + history: [{ json: { total_plans: 5 } }], + result: { json: { total_plans: 2 } }, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(outcome.subject.key, 'total_plans'); + assert.strictEqual(outcome.subject.from, 5); + assert.strictEqual(outcome.subject.to, 2); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('monotonic-progress: a decrease with total_summaries (the exact self-test shape) still violates when neither side has scope', () => { + // Regression coverage for the #2966 self-test payload shape reported by the full suite: + // `{ total_summaries: n }`, no milestone fields, no argv at all. + const outcome = getOracle('monotonic-progress').check({ + history: [{ json: { total_summaries: 3 } }], + result: { json: { total_summaries: 1 } }, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(outcome.subject.key, 'total_summaries'); + assert.strictEqual(outcome.subject.from, 3); + assert.strictEqual(outcome.subject.to, 1); + }); + + test('monotonic-progress: a decrease WITHIN the same milestone_version is a VIOLATION', () => { + const outcome = getOracle('monotonic-progress').check({ + history: [{ json: { total_plans: 5, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }], + result: { json: { total_plans: 2, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(outcome.subject.key, 'total_plans'); + assert.strictEqual(outcome.subject.from, 5); + assert.strictEqual(outcome.subject.to, 2); + }); + + test('monotonic-progress: the SAME decrease ACROSS a milestone_version change is NOT reported at all (silent reset, not even a SMELL)', () => { + const ctx = { + history: [{ json: { total_plans: 5, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }], + result: { json: { total_plans: 2, milestone_version: 'v2.0', milestone_name: 'Milestone Two' }, argv: ['progress'] }, + }; + const outcome = getOracle('monotonic-progress').check(ctx); + assert.strictEqual(outcome.ok, true, 'a milestone-version boundary crossing must reset silently, not fire at all'); + + // Confirm this never reaches `runOracles(ctx).failed` NOR `.smells` — a boundary + // crossing is expected behavior, not evidence worth a human look (#2966 FIX 2a). + const { failed, smells } = runOracles(ctx); + assert.strictEqual(failed.find((f) => f.id === 'monotonic-progress'), undefined); + assert.strictEqual(smells.find((s) => s.id === 'monotonic-progress'), undefined); + }); + + test('monotonic-progress: a decrease ACROSS a --ws workstream switch is NOT reported at all (silent reset, not even a SMELL)', () => { + const ctx = { + history: [{ json: { total_plans: 5 }, argv: ['--ws', 'alpha', 'progress'] }], + result: { json: { total_plans: 0 }, argv: ['--ws', 'beta', 'progress'] }, + }; + const outcome = getOracle('monotonic-progress').check(ctx); + assert.strictEqual(outcome.ok, true, 'a workstream boundary crossing must reset silently, not fire at all'); + const { failed, smells } = runOracles(ctx); + assert.strictEqual(failed.find((f) => f.id === 'monotonic-progress'), undefined); + assert.strictEqual(smells.find((s) => s.id === 'monotonic-progress'), undefined); + }); + + test('monotonic-progress: a subsequent same-scope decrease AFTER a boundary crossing is still a VIOLATION (the reset re-arms the check)', () => { + const outcome = getOracle('monotonic-progress').check({ + history: [ + { json: { total_plans: 5, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }, + { json: { total_plans: 0, milestone_version: 'v2.0', milestone_name: 'Milestone Two' }, argv: ['progress'] }, + { json: { total_plans: 3, milestone_version: 'v2.0', milestone_name: 'Milestone Two' }, argv: ['progress'] }, + ], + result: { json: { total_plans: 1, milestone_version: 'v2.0', milestone_name: 'Milestone Two' }, argv: ['progress'] }, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(outcome.subject.from, 3); + assert.strictEqual(outcome.subject.to, 1); + }); + + test('monotonic-progress: a MIXED pair (one scoped, one not) is skipped — genuinely indeterminate, not a boundary guess', () => { + // Mirrors `roadmap analyze`'s real payload shape sitting between two scoped `progress` + // observations: neither milestone_version nor milestone_name present on the middle entry. + // Branch 3 of the three-way rule (#2966 FIX 2c): that ONE pairing is skipped and does not + // become the new reference point, so the surrounding same-scope comparison still applies — + // see the next test. + const ctx = { + history: [ + { json: { total_plans: 5, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }, + { json: { total_plans: 1 }, argv: ['roadmap', 'analyze'] }, + ], + result: { json: { total_plans: 5, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }, + }; + const outcome = getOracle('monotonic-progress').check(ctx); + assert.strictEqual(outcome.ok, true, 'the scope-less entry must be invisible to the comparison, not a violation or a smell'); + const { failed, smells } = runOracles(ctx); + assert.strictEqual(failed.find((f) => f.id === 'monotonic-progress'), undefined); + assert.strictEqual(smells.find((s) => s.id === 'monotonic-progress'), undefined); + }); + + test('monotonic-progress: a scope-less observation does not mask a real same-scope decrease around it', () => { + const outcome = getOracle('monotonic-progress').check({ + history: [ + { json: { total_plans: 5, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }, + { json: { total_plans: 1 }, argv: ['roadmap', 'analyze'] }, + ], + result: { json: { total_plans: 2, milestone_version: 'v1.0', milestone_name: 'milestone' }, argv: ['progress'] }, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.VIOLATION); + assert.strictEqual(outcome.subject.from, 5); + assert.strictEqual(outcome.subject.to, 2); + }); + + test('routing-validity passes on a clean context', () => { + const outcome = getOracle('routing-validity').check({ + result: { json: { recommended: '/gsd-plan-phase' } }, + liveCommands: ['/gsd-plan-phase'], + }); + assert.strictEqual(outcome.ok, true); + }); + + test('routing-validity fails on a broken context (token not in liveCommands)', () => { + const outcome = getOracle('routing-validity').check({ + result: { json: { recommended: '/gsd-plan-phase' } }, + liveCommands: ['/gsd-something-else'], + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('determinism passes on a clean context', () => { + const outcome = getOracle('determinism').check({ + result: { kind: KIND.JSON }, + repeatResult: { kind: KIND.JSON }, + }); + assert.strictEqual(outcome.ok, true); + }); + + test('determinism fails on a broken context (repeat kind diverges)', () => { + const outcome = getOracle('determinism').check({ + result: { kind: KIND.JSON }, + repeatResult: { kind: KIND.PROSE }, + }); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + }); + + test('soft-error-exit-zero passes on a clean context', () => { + const outcome = getOracle('soft-error-exit-zero').check({ result: { kind: KIND.JSON, argv: ['progress'] } }); + assert.strictEqual(outcome.ok, true); + }); + + test('soft-error-exit-zero SMELLs (not a violation) on a SOFT_ERROR result', () => { + const ctx = { result: { kind: KIND.SOFT_ERROR, argv: ['progress'], json: { error: 'no phases found' } } }; + const outcome = getOracle('soft-error-exit-zero').check(ctx); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.SMELL); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + assert.deepEqual(outcome.subject.argv, ['progress'], 'subject.argv must name the offending command'); + const { violations, smells } = runOracles(ctx); + assert.strictEqual(violations.some((v) => v.id === 'soft-error-exit-zero'), false); + assert.strictEqual(smells.some((s) => s.id === 'soft-error-exit-zero'), true); + }); + + test('untyped-success passes on a clean context', () => { + const outcome = getOracle('untyped-success').check({ result: { kind: KIND.JSON, argv: ['progress'] } }); + assert.strictEqual(outcome.ok, true); + }); + + test('untyped-success SMELLs (not a violation) on a PROSE result', () => { + const ctx = { result: { kind: KIND.PROSE, argv: ['init'] } }; + const outcome = getOracle('untyped-success').check(ctx); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.SMELL); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + assert.deepEqual(outcome.subject.argv, ['init'], 'subject.argv must name the offending command'); + const { violations, smells } = runOracles(ctx); + assert.strictEqual(violations.some((v) => v.id === 'untyped-success'), false); + assert.strictEqual(smells.some((s) => s.id === 'untyped-success'), true); + }); + + test('contract-conflict passes on a clean context', () => { + const outcome = getOracle('contract-conflict').check({ + jsonErrorMode: true, + result: { kind: KIND.JSON, argv: ['progress'] }, + }); + assert.strictEqual(outcome.ok, true); + }); + + test('contract-conflict SMELLs (not a violation) when --json-errors still produced unstructured error text', () => { + const ctx = { jsonErrorMode: true, result: { kind: KIND.UNSTRUCTURED_ERROR, argv: ['bad-usage'] } }; + const outcome = getOracle('contract-conflict').check(ctx); + assert.strictEqual(outcome.ok, false); + assert.strictEqual(outcome.severity, SEVERITY.SMELL); + assert.strictEqual(typeof outcome.detail, 'string'); + assert.ok(outcome.detail.length > 0); + assert.deepEqual(outcome.subject.argv, ['bad-usage'], 'subject.argv must name the offending command'); + const { violations, smells } = runOracles(ctx); + assert.strictEqual(violations.some((v) => v.id === 'contract-conflict'), false); + assert.strictEqual(smells.some((s) => s.id === 'contract-conflict'), true); + }); +}); + +describe('severity model', () => { + test('a smell must never appear in failed (the contract that keeps smells from breaking builds)', () => { + const ctx = { result: { kind: KIND.PROSE, argv: ['init'] } }; + const { failed, smells } = runOracles(ctx); + assert.ok(smells.some((s) => s.id === 'untyped-success')); + assert.strictEqual(failed.some((f) => f.id === 'untyped-success'), false); + }); + + test('failed.length === violations.length for a context producing both a violation and a smell', () => { + // value-hygiene fires a VIOLATION on the NaN leaf; untyped-success fires a + // SMELL on the PROSE kind. Both fire from the same ctx. + const ctx = { result: { kind: KIND.PROSE, argv: ['init'], json: { a: Number.NaN } } }; + const { failed, violations, smells } = runOracles(ctx); + assert.ok(violations.length > 0); + assert.ok(smells.length > 0); + assert.strictEqual(failed.length, violations.length); + }); + + test('SEVERITY is frozen and has exactly the expected keys', () => { + assert.strictEqual(Object.isFrozen(SEVERITY), true); + assert.deepStrictEqual(Object.keys(SEVERITY).sort(), ['SMELL', 'VIOLATION'].sort()); + assert.strictEqual(SEVERITY.VIOLATION, 'violation'); + assert.strictEqual(SEVERITY.SMELL, 'smell'); + }); +}); + +describe('scenario DSL validation', () => { + let expectedPoints; + + before(() => { + expectedPoints = new Set(); + for (const entry of LOOP_HOST_CONTRACT) { + for (const point of entry.points) expectedPoints.add(point); + } + }); + + function writeScenarioFile(t, obj) { + const dir = createTempDir('gsd-scenario-dsl-'); + t.after(() => cleanup(dir)); + const file = path.join(dir, 'scenario.json'); + fs.writeFileSync(file, JSON.stringify(obj), 'utf-8'); + return file; + } + + test('loadScenario rejects an empty steps array', (t) => { + const file = writeScenarioFile(t, { name: 'empty-steps', fixture: 'greenfield', steps: [] }); + assert.throws(() => loadScenario(file), /"steps"/); + }); + + test('loadScenario rejects an unknown fixture', (t) => { + const file = writeScenarioFile(t, { + name: 'bad-fixture', + fixture: 'nonexistent-fixture', + steps: [{ at: 'discuss:pre' }], + }); + assert.throws(() => loadScenario(file), /nonexistent-fixture/); + }); + + test('loadScenario rejects an "at" point not present in the generated loop contract', (t) => { + const file = writeScenarioFile(t, { + name: 'bad-point', + fixture: 'greenfield', + steps: [{ at: 'totally-bogus-point' }], + }); + assert.throws(() => loadScenario(file), /totally-bogus-point/); + }); + + test('loadScenario rejects a non-boolean "jsonErrors" field, naming it', (t) => { + const file = writeScenarioFile(t, { + name: 'bad-json-errors', + fixture: 'greenfield', + steps: [{ at: 'discuss:pre', jsonErrors: 'yes' }], + }); + assert.throws(() => loadScenario(file), /jsonErrors/); + }); + + test('loadScenario rejects a malformed expect entry', (t) => { + const file = writeScenarioFile(t, { + name: 'bad-expect', + fixture: 'greenfield', + steps: [{ at: 'discuss:pre', expect: [{ foo: 'bar' }] }], + }); + assert.throws(() => loadScenario(file), /expect\[0\]/); + }); + + test('the legal point set derives from loop-host-contract.cjs: a known-good point loads', (t) => { + const knownGoodPoint = [...expectedPoints][0]; + assert.strictEqual(expectedPoints.has(knownGoodPoint), true); + const file = writeScenarioFile(t, { + name: 'known-good', + fixture: 'greenfield', + steps: [{ at: knownGoodPoint }], + }); + const scenario = loadScenario(file); + assert.strictEqual(scenario.steps[0].at, knownGoodPoint); + }); + + test('the legal point set derives from loop-host-contract.cjs: a fabricated point throws', (t) => { + const fabricatedPoint = 'zzz:not-a-real-point'; + assert.strictEqual(expectedPoints.has(fabricatedPoint), false); + const file = writeScenarioFile(t, { + name: 'fabricated', + fixture: 'greenfield', + steps: [{ at: fabricatedPoint }], + }); + assert.throws(() => loadScenario(file), /zzz:not-a-real-point/); + }); +}); + +describe('fixture refs', () => { + test("resolveRef('@project/minimal') returns non-empty content", () => { + const text = resolveRef('@project/minimal'); + assert.strictEqual(typeof text, 'string'); + assert.ok(text.length > 0); + }); + + test('resolveRef throws on an unknown ref, naming the ref', () => { + assert.throws(() => resolveRef('@nope/nope'), /@nope\/nope/); + }); +}); + +describe('mutations', () => { + const SAMPLE_ARTIFACT_TEXT = [ + '---', + 'title: sample', + 'phase: 1', + '---', + '', + '## Phase 1', + '', + '| 1 | Task | Status |', + '| --- | --- | --- |', + '| 1 | Do the thing | pending |', + '', + 'Some body text describing the phase.', + ].join('\n'); + + test('MUTATIONS has exactly 11 entries', () => { + assert.strictEqual(MUTATIONS.length, 11); + }); + + test('every mutation id is unique', () => { + const ids = MUTATIONS.map((m) => m.id); + assert.strictEqual(new Set(ids).size, ids.length); + }); + + test('apply("truncate-frontmatter", ...) shortens text and drops the closing delimiter', () => { + const result = apply('truncate-frontmatter', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + assert.ok(result.length < SAMPLE_ARTIFACT_TEXT.length); + }); + + test('apply("crlf", ...) converts line endings to CRLF', () => { + const result = apply('crlf', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + assert.ok(result.includes('\r\n')); + }); + + test('apply("bom", ...) prefixes text with a byte-order-mark', () => { + const result = apply('bom', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + assert.strictEqual(result.charCodeAt(0), 0xfeff); + }); + + test('apply("empty", ...) replaces text with an empty string', () => { + const result = apply('empty', SAMPLE_ARTIFACT_TEXT); + assert.strictEqual(result, ''); + }); + + test('apply("duplicate-phase-id", ...) duplicates the first phase-id line', () => { + const result = apply('duplicate-phase-id', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + }); + + test('apply("nonsequential-phases", ...) renumbers phase-id occurrences when present', () => { + const result = apply('nonsequential-phases', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, NOOP); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + }); + + test('apply("nonsequential-phases", ...) returns the NOOP sentinel when no phase-id occurs', () => { + const noPhaseText = ['# Just a title', '', 'No phase markers in this document.'].join('\n'); + const result = apply('nonsequential-phases', noPhaseText); + assert.strictEqual(result, NOOP); + }); + + test('apply("unicode-headings", ...) replaces every heading\'s text', () => { + const result = apply('unicode-headings', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + }); + + test('apply("oversized", ...) pads text past a target larger than the current size', () => { + const targetBytes = Buffer.byteLength(SAMPLE_ARTIFACT_TEXT, 'utf8') + 50; + const result = apply('oversized', SAMPLE_ARTIFACT_TEXT, { targetBytes }); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + assert.ok(Buffer.byteLength(result, 'utf8') > targetBytes); + }); + + test('apply("escaped-pipes", ...) injects an escaped-pipe cell into the first table row', () => { + const result = apply('escaped-pipes', SAMPLE_ARTIFACT_TEXT); + assert.notStrictEqual(result, SAMPLE_ARTIFACT_TEXT); + }); + + test('apply("crlf", ...) is idempotent and never produces \\r\\r\\n', () => { + const once = apply('crlf', SAMPLE_ARTIFACT_TEXT); + const twice = apply('crlf', once); + assert.strictEqual(twice, once); + assert.strictEqual(twice.includes('\r\r\n'), false); + }); + + test('apply("oversized", ...) at targetBytes - 1 (input already over target) leaves input unchanged', () => { + const input = 'A'.repeat(10); + const result = apply('oversized', input, { targetBytes: 9 }); + assert.strictEqual(result, input); + }); + + test('apply("oversized", ...) at targetBytes === input size pads to exactly one byte over', () => { + const input = 'A'.repeat(10); + const result = apply('oversized', input, { targetBytes: 10 }); + assert.strictEqual(Buffer.byteLength(result, 'utf8'), 11); + }); + + test('apply("oversized", ...) at targetBytes + 1 (input under target) pads to one byte over', () => { + const input = 'A'.repeat(10); + const result = apply('oversized', input, { targetBytes: 11 }); + assert.strictEqual(Buffer.byteLength(result, 'utf8'), 12); + }); + + test('apply(...) throws on an unknown mutation id', () => { + assert.throws(() => apply('not-a-real-mutation', 'x'), /not-a-real-mutation/); + }); + + describe('file mutations', () => { + let mutDir; + + beforeEach(() => { + mutDir = createTempDir('gsd-mutations-file-'); + }); + + afterEach(() => { + cleanup(mutDir); + }); + + test('apply("delete", ...) removes the file on disk', () => { + const relPath = 'artifact.md'; + fs.writeFileSync(path.join(mutDir, relPath), SAMPLE_ARTIFACT_TEXT, 'utf-8'); + apply('delete', { dir: mutDir, relPath }); + assert.strictEqual(fs.existsSync(path.join(mutDir, relPath)), false); + }); + + test('apply("symlink", ...) replaces the file with a symlink or hardlink of identical size', () => { + const relPath = 'artifact.md'; + const abs = path.join(mutDir, relPath); + fs.writeFileSync(abs, SAMPLE_ARTIFACT_TEXT, 'utf-8'); + const sizeBefore = fs.statSync(abs).size; + apply('symlink', { dir: mutDir, relPath }); + const lstat = fs.lstatSync(abs); + const sizeAfter = fs.statSync(abs).size; + assert.strictEqual(sizeAfter, sizeBefore); + assert.strictEqual(lstat.isSymbolicLink() || lstat.isFile(), true); + }); + }); +}); + +describe('path containment', () => { + test('resolveWithin rejects a traversing relPath, naming it and the base', (t) => { + const dir = createTempDir('gsd-pathguard-'); + t.after(() => cleanup(dir)); + assert.throws(() => resolveWithin(dir, '../../etc/hosts'), (err) => { + assert.ok(err instanceof Error); + assert.strictEqual(err.code, 'EPATHESCAPE'); + assert.strictEqual(err.attemptedPath, '../../etc/hosts'); + assert.strictEqual(err.base, dir); + return true; + }); + }); + + test('resolveWithin rejects an absolute relPath', (t) => { + const dir = createTempDir('gsd-pathguard-abs-'); + t.after(() => cleanup(dir)); + assert.throws(() => resolveWithin(dir, '/etc/hosts'), (err) => { + assert.strictEqual(err.code, 'EPATHESCAPE'); + assert.strictEqual(err.attemptedPath, '/etc/hosts'); + assert.strictEqual(err.base, dir); + return true; + }); + }); + + test('resolveWithin rejects a sibling-prefix escape (path-segment, not string-prefix, containment)', (t) => { + const dir = createTempDir('gsd-pathguard-sibling-'); + t.after(() => cleanup(dir)); + assert.throws(() => resolveWithin(dir, `../${path.basename(dir)}-evil/x`), /escapes/); + }); + + test('resolveWithin accepts an in-project path that does not exist yet', (t) => { + const dir = createTempDir('gsd-pathguard-notyet-'); + t.after(() => cleanup(dir)); + const resolved = resolveWithin(dir, 'deep/not/created/yet.md'); + assert.strictEqual(typeof resolved, 'string'); + assert.ok(resolved.length > 0); + }); + + test('LoopWalk#writeArtifact rejects a traversing relPath', (t) => { + const walk = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-pathguard-write-' }); + t.after(() => walk.cleanup()); + assert.throws(() => walk.writeArtifact('../../escaped.md', 'x'), /escapes|absolute/); + assert.strictEqual(fs.existsSync(path.join(path.dirname(walk.dir), 'escaped.md')), false); + }); + + test('LoopWalk#writeArtifact still writes a legitimate in-project path after the guard', (t) => { + const walk = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-pathguard-write-ok-' }); + t.after(() => walk.cleanup()); + walk.writeArtifact('.planning/PROJECT.md', '# ok\n'); + assert.strictEqual(fs.existsSync(path.join(walk.dir, '.planning', 'PROJECT.md')), true); + }); + + test('mutations apply("delete", ...) rejects a traversing relPath', (t) => { + const mutDir = createTempDir('gsd-pathguard-delete-'); + t.after(() => cleanup(mutDir)); + assert.throws(() => apply('delete', { dir: mutDir, relPath: '../../../../etc/hosts' }), /escapes|absolute/); + }); + + test('mutations apply("symlink", ...) rejects a traversing relPath', (t) => { + const mutDir = createTempDir('gsd-pathguard-symlink-'); + t.after(() => cleanup(mutDir)); + assert.throws(() => apply('symlink', { dir: mutDir, relPath: '../../../../etc/hosts' }), /escapes|absolute/); + }); + + test('loadScenario rejects a mutate.target that traverses out of the project, at load time', (t) => { + const dir = createTempDir('gsd-pathguard-scenario-mutate-'); + t.after(() => cleanup(dir)); + const file = path.join(dir, 's.json'); + fs.writeFileSync(file, JSON.stringify({ + name: 'traversal-mutate', + fixture: 'greenfield', + steps: [{ at: 'plan:pre', mutate: { id: 'crlf', target: '../../evil.md' }, run: [['progress']] }], + })); + assert.throws(() => loadScenario(file), /evil|\.\./); + }); + + test('loadScenario rejects an absolute mutate.target, at load time', (t) => { + const dir = createTempDir('gsd-pathguard-scenario-mutate-abs-'); + t.after(() => cleanup(dir)); + const file = path.join(dir, 's.json'); + fs.writeFileSync(file, JSON.stringify({ + name: 'absolute-mutate', + fixture: 'greenfield', + steps: [{ at: 'plan:pre', mutate: { id: 'crlf', target: '/etc/evil.md' }, run: [['progress']] }], + })); + assert.throws(() => loadScenario(file), /absolute/); + }); + + test('loadScenario rejects an agent.write key that traverses out of the project, at load time', (t) => { + const dir = createTempDir('gsd-pathguard-scenario-write-'); + t.after(() => cleanup(dir)); + const file = path.join(dir, 's.json'); + fs.writeFileSync(file, JSON.stringify({ + name: 'traversal-write', + fixture: 'greenfield', + steps: [{ at: 'plan:pre', agent: { write: { '../../escaped.md': '@project/minimal' } }, run: [['progress']] }], + })); + assert.throws(() => loadScenario(file), /escaped|\.\./); + }); + + test('loadScenario rejects an absolute agent.write key, at load time', (t) => { + const dir = createTempDir('gsd-pathguard-scenario-write-abs-'); + t.after(() => cleanup(dir)); + const file = path.join(dir, 's.json'); + fs.writeFileSync(file, JSON.stringify({ + name: 'absolute-write', + fixture: 'greenfield', + steps: [{ at: 'plan:pre', agent: { write: { '/etc/evil.md': '@project/minimal' } }, run: [['progress']] }], + })); + assert.throws(() => loadScenario(file), /etc|absolute/i); + }); +}); + +describe('greenfield walk (end-to-end)', () => { + let liveCommands; + + before(() => { + // tests/helpers/live-command-registry.cjs's real API: getLiveCommandTokens() + // returns a memoized Set of every live slash-command token derived + // from commands/gsd/*.md frontmatter (e.g. "/gsd-plan-phase"). The + // routing-validity oracle checks result.json.recommended against this set. + liveCommands = [...getLiveCommandTokens()]; + }); + + test('runs every step of the greenfield-happy-path scenario clean', () => { + const scenarioPath = path.join(__dirname, 'qa', 'scenarios', 'greenfield-happy-path.json'); + const scenario = loadScenario(scenarioPath); + const report = runScenario(scenario, { LoopWalk, runOracles, liveCommands }); + + assert.strictEqual(report.steps.length, scenario.steps.length); + for (const step of report.steps) { + assert.deepStrictEqual(step.expectFailures, []); + assert.deepStrictEqual(step.oracleFailures, []); + } + assert.strictEqual(report.ok, true); + + // Anti-vacuity: a QA harness that reports NOTHING on a first real walk + // against the actual CLI is far more likely to be mis-specified (oracles + // that never fire, a wiring bug that drops ctx fields, a scenario that + // never exercises the paths that produce smells) than the engine is + // genuinely flawless. "Found nothing" is itself a failure signal for a + // harness whose whole job is to keep known trade-offs visible, so the + // walk must be able to speak at least once. Deliberately NOT asserting an + // exact smell count or exact oracle ids here — that would pin today's + // engine behavior into the test and defeat the point of a smell channel + // that is allowed to evolve without becoming a build break. + const totalSmells = report.steps.reduce((sum, step) => sum + step.smells.length, 0); + assert.ok(totalSmells > 0, 'expected the greenfield walk to surface at least one smell'); + assert.ok(report.smellSummary.length > 0, 'expected a non-empty smellSummary'); + }); +}); + +describe('scenario discovery (mutations wired for real)', () => { + /** + * Every `.json` scenario file under `tests/qa/scenarios/`, EXCLUDING + * underscore-prefixed ones (`_selftest-must-fail.json`) — an + * underscore-prefixed scenario is deliberately broken (see + * `assertWiringIsLive`) and must never run as a normal walk. + * + * @returns {string[]} absolute file paths. + */ + function discoverScenarioFiles() { + const scenariosDir = path.join(__dirname, 'qa', 'scenarios'); + return fs + .readdirSync(scenariosDir) + .filter((name) => name.endsWith('.json') && !name.startsWith('_')) + .sort() + .map((name) => path.join(scenariosDir, name)); + } + + test('discovery excludes underscore-prefixed self-test scenarios', () => { + const names = discoverScenarioFiles().map((p) => path.basename(p)); + assert.ok(names.includes('greenfield-happy-path.json')); + assert.ok(names.includes('perturbation-crlf.json')); + assert.ok(names.includes('perturbation-truncated-frontmatter.json')); + assert.ok(names.includes('perturbation-delete-artifact.json')); + assert.strictEqual(names.includes('_selftest-must-fail.json'), false); + }); + + test('every discovered perturbation scenario applies its mutation and runs to completion without a harness crash', () => { + const liveCommands = [...getLiveCommandTokens()]; + const perturbationFiles = discoverScenarioFiles().filter((p) => path.basename(p).startsWith('perturbation-')); + assert.ok(perturbationFiles.length >= 3, 'expected at least the crlf, truncated-frontmatter, and delete-artifact scenarios'); + + // Anti-vacuity for perturbations specifically: a mutation that changes + // nothing observable in ANY scenario is indistinguishable from a + // mutation that was never applied (see `scenario.cjs`'s + // `mutationObserved` computation). At least one mutated step across the + // whole perturbation set must show a genuinely different result from its + // own clean baseline — if none ever does, that is a finding about which + // engine surfaces are sensitive to corruption, not something to paper + // over by weakening this assertion. + let anyMutationObserved = false; + + for (const file of perturbationFiles) { + const scenario = loadScenario(file); + const report = runScenario(scenario, { LoopWalk, runOracles, liveCommands }); + + assert.strictEqual(report.steps.length, scenario.steps.length, `${scenario.name}: a step went missing from the report`); + for (const step of report.steps) { + assert.strictEqual( + step.oracleFailures.some((f) => f.id === 'step-exception'), + false, + `${scenario.name}: step at "${step.at}" crashed the harness: ${JSON.stringify(step.oracleFailures)}`, + ); + } + + const mutatedStep = report.steps.find((s) => s.mutation); + assert.ok(mutatedStep, `${scenario.name}: no step recorded a mutation — mutations remain unwired`); + assert.strictEqual(mutatedStep.mutationNoop, false, `${scenario.name}: mutation no-oped on a real roadmap artifact`); + + if (mutatedStep.mutationObserved) anyMutationObserved = true; + } + + assert.strictEqual( + anyMutationObserved, + true, + 'no perturbation scenario produced an observable mutation against its probed surface — ' + + 'every corruption was silently absorbed', + ); + }); +}); + +describe('wiring self-test (anti-vacuity)', () => { + test('the self-test scenario proves the expect/oracle assertion machinery actually fires', () => { + const liveCommands = [...getLiveCommandTokens()]; + const report = assertWiringIsLive({ LoopWalk, runOracles, liveCommands }); + assert.strictEqual(report.ok, false); + assert.ok(report.steps.some((s) => s.expectFailures.length > 0)); + }); +}); + +describe('fixture integrity', () => { + /** + * Absolute path to `tests/qa/fixtures/`, the corpus every QA scenario/fixture-ref + * draws from (see `tests/qa/fixtures/index.cjs`'s `resolveRef`). + */ + const FIXTURES_ROOT = path.join(__dirname, 'qa', 'fixtures'); + + /** + * Every fixture `.md` file, read once, keyed by absolute path — every test below + * reuses this instead of re-walking the tree. + * + * WHY module-content, not module-frontmatter: this guards against defect classes + * where a provenance comment (or any other byte-0 preamble) silently defeats + * `extractFrontmatter`, which only recognizes a frontmatter block that starts at + * byte 0 of the file (`gsd-core/bin/lib/frontmatter.cjs` — `content.startsWith('---\n')`). + */ + const fixtureFiles = collectFixtureMarkdownFiles(FIXTURES_ROOT); + + test('discovered at least the known fixture directories (sanity, not vacuous)', () => { + assert.ok(fixtureFiles.length > 0, 'expected at least one fixture .md file under tests/qa/fixtures'); + }); + + test('every fixture that is frontmatter-shaped (optionally preceded by a leading comment) parses to a NON-EMPTY object', () => { + const offenders = []; + for (const file of fixtureFiles) { + const content = fs.readFileSync(file, 'utf-8'); + if (!hasFrontmatterShape(content)) continue; + const fm = extractFrontmatter(content, file); + if (Object.keys(fm).length === 0) { + offenders.push(path.relative(FIXTURES_ROOT, file)); + } + } + assert.deepStrictEqual( + offenders, + [], + `fixture(s) are frontmatter-shaped but extractFrontmatter returned {} (frontmatter not at byte 0 — ` + + `likely a leading comment ahead of the fence — or malformed): ${JSON.stringify(offenders)}`, + ); + }); + + test('every fixture carries the #2371 provenance marker somewhere in the file', () => { + const offenders = []; + for (const file of fixtureFiles) { + const content = fs.readFileSync(file, 'utf-8'); + if (!content.includes('provenance:')) { + offenders.push(path.relative(FIXTURES_ROOT, file)); + } + } + assert.deepStrictEqual( + offenders, + [], + `fixture(s) missing the #2371 provenance marker: ${JSON.stringify(offenders)}`, + ); + }); + + describe('UAT fixtures flip the REAL evaluateUatPassed verdict', () => { + let dir; + + beforeEach(() => { + dir = createTempDir('gsd-uat-fixture-integrity-'); + }); + + afterEach(() => { + cleanup(dir); + }); + + test('the passing UAT fixture yields passed:true, no_uat_artifacts:false', () => { + const content = fs.readFileSync(path.join(FIXTURES_ROOT, 'uat', 'current-test-passing.md'), 'utf-8'); + fs.writeFileSync(path.join(dir, 'feature-UAT.md'), content, 'utf-8'); + const report = evaluateUatPassed(dir); + assert.strictEqual(report.passed, true); + assert.strictEqual(report.no_uat_artifacts, false); + assert.ok(report.checks.length > 0, 'expected at least one real parsed check'); + }); + + test('the failing UAT fixture yields passed:false, no_uat_artifacts:false', () => { + const content = fs.readFileSync(path.join(FIXTURES_ROOT, 'uat', 'current-test-failing.md'), 'utf-8'); + fs.writeFileSync(path.join(dir, 'feature-UAT.md'), content, 'utf-8'); + const report = evaluateUatPassed(dir); + assert.strictEqual(report.passed, false); + assert.strictEqual(report.no_uat_artifacts, false); + assert.ok(report.checks.length > 0, 'expected at least one real parsed check'); + }); + }); +}); + +describe('walk isolation', () => { + test('two LoopWalk instances never share a project directory', (t) => { + const walkA = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-walk-isolation-a-' }); + const walkB = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-walk-isolation-b-' }); + t.after(() => { + walkA.cleanup(); + walkB.cleanup(); + }); + assert.notStrictEqual(walkA.dir, walkB.dir); + }); + + test('a read-only command does not change the stat snapshot', (t) => { + const walk = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-walk-isolation-readonly-' }); + t.after(() => walk.cleanup()); + const statsBefore = walk.statSnapshot(); + walk.run('progress'); + const statsAfter = walk.statSnapshot(); + assert.deepStrictEqual(statsBefore, statsAfter); + }); +}); + +describe('worktree-concurrency (dedicated — trajectory 9 is not expressible as a single scenario)', () => { + /** + * WHY THIS IS A DEDICATED TEST, NOT A `scenarios/*.json` FILE + * ──────────────────────────────────────────────────────────── + * `scenario.cjs#runScenario` drives exactly one `LoopWalk` through a + * strictly sequential `steps` array — `LoopWalk#run` itself is built on + * `execFileSync` (see `loop-walk.cjs`), so within the DSL there is no way to + * have two `gsd-tools` invocations genuinely in flight at the same wall-clock + * moment. "Two LoopWalk instances driving simultaneously" (issue #2966's + * ninth trajectory) is therefore implemented here directly against + * `child_process.execFile` (async) so both subprocesses are launched before + * either has resolved — real concurrency, not two sequential calls dressed + * up as one. + */ + + /** Absolute path to the gsd-tools entry point, mirrored from `helpers.cjs`'s `TOOLS_PATH`. */ + const TOOLS_PATH = path.join(__dirname, '..', 'gsd-core', 'bin', 'gsd-tools.cjs'); + + /** + * Runs a single `gsd-tools --json-errors ` invocation asynchronously + * against `dir`, sanitizing ambient `GSD_*` env vars exactly as + * `LoopWalk#run` does (see that method's header for why), and pinning the + * clock so both concurrent invocations are reproducible. + * + * @param {string} dir + * @param {string[]} argv + * @returns {Promise<{stdout: string, startedAtMs: number, finishedAtMs: number}>} + */ + async function runConcurrent(dir, argv) { + /** @type {Record} */ + const sanitize = {}; + for (const key of Object.keys(process.env)) { + if (key.startsWith('GSD_')) sanitize[key] = undefined; + } + const env = { ...sanitize, GSD_TEST_MODE: '1', GSD_NOW_MS: '1767225600000' }; + const startedAtMs = Date.now(); + const { stdout } = await execFileAsync( + process.execPath, + [TOOLS_PATH, '--json-errors', ...argv], + { cwd: dir, encoding: 'utf-8', env, timeout: 60000 }, + ); + return { stdout: stdout.trim(), startedAtMs, finishedAtMs: Date.now() }; + } + + test('two LoopWalk projects driven via genuinely concurrent async subprocesses stay isolated', async (t) => { + const walkA = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-concurrency-a-' }); + const walkB = LoopWalk.create({ fixture: 'greenfield', prefix: 'gsd-concurrency-b-' }); + t.after(() => { + walkA.cleanup(); + walkB.cleanup(); + }); + + walkA.writeArtifact('.planning/PROJECT.md', resolveRef('@project/minimal')); + walkA.writeArtifact('.planning/ROADMAP.md', resolveRef('@roadmap/three-phase')); + walkB.writeArtifact('.planning/PROJECT.md', resolveRef('@project/minimal')); + walkB.writeArtifact('.planning/ROADMAP.md', resolveRef('@roadmap/three-phase')); + + // Both promises are created (and their subprocesses spawned) in the same + // synchronous tick, BEFORE either `await`s — this is what makes the two + // invocations genuinely concurrent rather than sequential-looking-parallel. + const promiseA = runConcurrent(walkA.dir, ['init', 'new-project']); + const promiseB = runConcurrent(walkB.dir, ['init', 'new-project']); + const [resultA, resultB] = await Promise.all([promiseA, promiseB]); + + // Proof of genuine overlap: A's subprocess was still running when B's was + // launched (both started before either finished). If the harness had + // silently serialized these (e.g. a shared lock), one start time would be + // >= the other's finish time. + const overlapped = resultA.startedAtMs < resultB.finishedAtMs && resultB.startedAtMs < resultA.finishedAtMs; + assert.ok( + overlapped, + `expected the two subprocess invocations to overlap in wall-clock time (A: ${resultA.startedAtMs}-${resultA.finishedAtMs}, B: ${resultB.startedAtMs}-${resultB.finishedAtMs})`, + ); + + const jsonA = JSON.parse(resultA.stdout); + const jsonB = JSON.parse(resultB.stdout); + + // Isolation: each concurrent run must observe and report its OWN project + // root, never the other's — a shared-state bug (e.g. a global cwd) would + // show up here as both reporting the same root. + assert.strictEqual(resolveForCompare(jsonA.project_root), resolveForCompare(walkA.dir)); + assert.strictEqual(resolveForCompare(jsonB.project_root), resolveForCompare(walkB.dir)); + assert.notStrictEqual(resolveForCompare(jsonA.project_root), resolveForCompare(jsonB.project_root)); + + // Both saw their own freshly-written PROJECT.md/ROADMAP.md, independent of + // the other walk's concurrent writes to a different temp directory. + assert.strictEqual(jsonA.project_exists, true); + assert.strictEqual(jsonB.project_exists, true); + }); +}); diff --git a/tests/qa/fixtures/code/sample-source.md b/tests/qa/fixtures/code/sample-source.md new file mode 100644 index 000000000..b4c53ea45 --- /dev/null +++ b/tests/qa/fixtures/code/sample-source.md @@ -0,0 +1,2 @@ + +console.log('sample brownfield source file'); diff --git a/tests/qa/fixtures/codebase/ARCHITECTURE.md b/tests/qa/fixtures/codebase/ARCHITECTURE.md new file mode 100644 index 000000000..82a095a5e --- /dev/null +++ b/tests/qa/fixtures/codebase/ARCHITECTURE.md @@ -0,0 +1,4 @@ + +# Architecture + +Single-file CLI entry point, no layering. diff --git a/tests/qa/fixtures/codebase/CONCERNS.md b/tests/qa/fixtures/codebase/CONCERNS.md new file mode 100644 index 000000000..d55451c72 --- /dev/null +++ b/tests/qa/fixtures/codebase/CONCERNS.md @@ -0,0 +1,4 @@ + +# Concerns + +None recorded. diff --git a/tests/qa/fixtures/codebase/CONVENTIONS.md b/tests/qa/fixtures/codebase/CONVENTIONS.md new file mode 100644 index 000000000..524209439 --- /dev/null +++ b/tests/qa/fixtures/codebase/CONVENTIONS.md @@ -0,0 +1,4 @@ + +# Conventions + +No conventions established yet. diff --git a/tests/qa/fixtures/codebase/INTEGRATIONS.md b/tests/qa/fixtures/codebase/INTEGRATIONS.md new file mode 100644 index 000000000..4c1673e8c --- /dev/null +++ b/tests/qa/fixtures/codebase/INTEGRATIONS.md @@ -0,0 +1,4 @@ + +# Integrations + +None. diff --git a/tests/qa/fixtures/codebase/STACK.md b/tests/qa/fixtures/codebase/STACK.md new file mode 100644 index 000000000..80870cda5 --- /dev/null +++ b/tests/qa/fixtures/codebase/STACK.md @@ -0,0 +1,4 @@ + +# Stack + +Node.js, plain CommonJS, no framework. diff --git a/tests/qa/fixtures/codebase/STRUCTURE.md b/tests/qa/fixtures/codebase/STRUCTURE.md new file mode 100644 index 000000000..3bc4113c9 --- /dev/null +++ b/tests/qa/fixtures/codebase/STRUCTURE.md @@ -0,0 +1,4 @@ + +# Structure + +`src/` holds the single entry point. diff --git a/tests/qa/fixtures/codebase/TESTING.md b/tests/qa/fixtures/codebase/TESTING.md new file mode 100644 index 000000000..d06db7e4d --- /dev/null +++ b/tests/qa/fixtures/codebase/TESTING.md @@ -0,0 +1,4 @@ + +# Testing + +No test suite exists yet. diff --git a/tests/qa/fixtures/index.cjs b/tests/qa/fixtures/index.cjs new file mode 100644 index 000000000..1e798f601 --- /dev/null +++ b/tests/qa/fixtures/index.cjs @@ -0,0 +1,80 @@ +'use strict'; + +/** + * fixtures/index.cjs — resolves `@dir/name` scenario refs to fixture file + * contents. + * + * WHY THIS EXISTS + * ──────────────── + * Scenario JSON (`tests/qa/scenario.cjs`) writes agent-authored artifacts by + * reference (`"@project/minimal"`) rather than inlining markdown bodies, so + * scenario files stay short and fixtures stay reviewable in one place. A ref + * that fails to resolve MUST be loud: a scenario step that silently wrote an + * empty artifact would make the rest of the walk vacuously green (the engine + * would be reacting to an empty PROJECT.md and nobody would know). This + * module therefore throws — naming both the bad ref and the full list of + * refs that DO resolve — rather than returning `''` or `undefined` on a miss. + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +/** Directory this module lives in — every fixture path is resolved relative to it. */ +const FIXTURES_ROOT = __dirname; + +/** + * Map of ref (`"dir/name"`, no leading `@`) -> absolute file path. + * Built once at module load by scanning the fixture subdirectories. + * + * @returns {Map} + */ +function buildRefMap() { + /** @type {Map} */ + const map = new Map(); + const entries = fs.readdirSync(FIXTURES_ROOT, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const dirName = entry.name; + const dirPath = path.join(FIXTURES_ROOT, dirName); + const files = fs.readdirSync(dirPath, { withFileTypes: true }); + for (const file of files) { + if (!file.isFile() || !file.name.endsWith('.md')) continue; + const baseName = file.name.slice(0, -'.md'.length); + map.set(`${dirName}/${baseName}`, path.join(dirPath, file.name)); + } + } + return map; +} + +const REF_MAP = buildRefMap(); + +/** + * Resolve a `@dir/name` ref to its fixture file's UTF-8 contents. + * + * @param {string} ref e.g. `"@project/minimal"` (leading `@` optional). + * @returns {string} file contents. + * @throws {Error} when `ref` does not resolve — names the ref AND lists every + * available ref, so a missing/renamed fixture fails loudly instead of a + * scenario step silently writing an empty artifact. + */ +function resolveRef(ref) { + if (typeof ref !== 'string' || ref === '') { + throw new Error(`resolveRef: ref must be a non-empty string, got ${JSON.stringify(ref)}`); + } + const key = ref.startsWith('@') ? ref.slice(1) : ref; + const filePath = REF_MAP.get(key); + if (!filePath) { + const available = [...REF_MAP.keys()].sort().join(', '); + throw new Error(`resolveRef: unknown fixture ref "${ref}" (available refs: ${available})`); + } + return fs.readFileSync(filePath, 'utf-8'); +} + +/** + * @returns {string[]} every resolvable ref, in `@dir/name` form, sorted. + */ +function listRefs() { + return [...REF_MAP.keys()].map((key) => `@${key}`).sort(); +} + +module.exports = { resolveRef, listRefs }; diff --git a/tests/qa/fixtures/plan/post-rollover.md b/tests/qa/fixtures/plan/post-rollover.md new file mode 100644 index 000000000..b3e0159a8 --- /dev/null +++ b/tests/qa/fixtures/plan/post-rollover.md @@ -0,0 +1,13 @@ +--- +phase: "04" +plan: "04-01" +created: 2026-01-01 +--- + + +# Plan 04-01: Post-Rollover Feature + +## Tasks + +- [ ] Implement the post-rollover feature +- [ ] Cover it with a plan-scoped test diff --git a/tests/qa/fixtures/plan/renderer.md b/tests/qa/fixtures/plan/renderer.md new file mode 100644 index 000000000..ded04891f --- /dev/null +++ b/tests/qa/fixtures/plan/renderer.md @@ -0,0 +1,12 @@ +--- +phase: "02" +plan: "02-01" +created: 2026-01-01 +--- + + +# Plan 02-01: Printable Renderer + +## Tasks + +- [ ] Render parsed items to a printable page diff --git a/tests/qa/fixtures/plan/tokenizer-drifted.md b/tests/qa/fixtures/plan/tokenizer-drifted.md new file mode 100644 index 000000000..cbb966628 --- /dev/null +++ b/tests/qa/fixtures/plan/tokenizer-drifted.md @@ -0,0 +1,14 @@ +--- +phase: "01" +plan: "01-01" +created: 2026-01-01 +--- + + +# Plan 01-01: Tokenizer + +## Tasks + +- [ ] Write the markdown task-list tokenizer AND a new unplanned feature added after execution started +- [ ] Handle malformed list items without crashing +- [ ] Unplanned: add a caching layer nobody discussed diff --git a/tests/qa/fixtures/plan/tokenizer.md b/tests/qa/fixtures/plan/tokenizer.md new file mode 100644 index 000000000..58738396f --- /dev/null +++ b/tests/qa/fixtures/plan/tokenizer.md @@ -0,0 +1,13 @@ +--- +phase: "01" +plan: "01-01" +created: 2026-01-01 +--- + + +# Plan 01-01: Tokenizer + +## Tasks + +- [ ] Write the markdown task-list tokenizer +- [ ] Handle malformed list items without crashing diff --git a/tests/qa/fixtures/project/minimal.md b/tests/qa/fixtures/project/minimal.md new file mode 100644 index 000000000..4b2346285 --- /dev/null +++ b/tests/qa/fixtures/project/minimal.md @@ -0,0 +1,11 @@ + +# Loop QA Walk Sample + +## What This Is + +A tiny sample app that converts markdown task lists into a printable +checklist. Built for a single user who wants a quick offline tool. + +## Core Value + +Any markdown checklist can be converted to a printable page in one command. diff --git a/tests/qa/fixtures/roadmap/post-rollover-v2.md b/tests/qa/fixtures/roadmap/post-rollover-v2.md new file mode 100644 index 000000000..ef4f640b5 --- /dev/null +++ b/tests/qa/fixtures/roadmap/post-rollover-v2.md @@ -0,0 +1,40 @@ + + +# Roadmap: Loop QA Walk Sample + +## Overview + +🚧 **v2.0 Post-Rollover Milestone** — extend the shipped v1.0 parser with one +more phase now that Phase 1-3 have shipped. Phases for this milestone are not +yet drafted — the next `phase add` call is what introduces Phase 4, exactly +as a planner would after the roadmapper hands off a version-only roadmap. + +## Phases + +- [ ] **Phase 4: Post-Rollover Work** - Extend the shipped parser with a new feature + +## Phase Details + +### Phase 4: Post-Rollover Work +**Goal**: Extend the shipped parser with a new feature +**Depends on**: Nothing (new milestone) +**Requirements**: [REQ-06] +**Success Criteria** (what must be TRUE): + 1. The post-rollover feature is implemented and covered by a plan +**Plans**: 1 plan + +Plans: +- [ ] 04-01: Implement the post-rollover feature + +## Progress + +**Execution Order:** +Phases execute in numeric order: 4 + +| Phase | Plans Complete | Status | Completed | +|-------|----------------|--------|-----------| +| 4. Post-Rollover Work | 0/1 | Not started | - | diff --git a/tests/qa/fixtures/roadmap/three-phase.md b/tests/qa/fixtures/roadmap/three-phase.md new file mode 100644 index 000000000..772b83300 --- /dev/null +++ b/tests/qa/fixtures/roadmap/three-phase.md @@ -0,0 +1,62 @@ + +# Roadmap: Loop QA Walk Sample + +## Overview + +Ship a minimal checklist converter in three phases: get the parser working, +add printable output, and polish the CLI surface. + +## Phases + +- [ ] **Phase 1: Parser** - Parse a markdown task list into structured items +- [ ] **Phase 2: Printable Output** - Render parsed items to a printable page +- [ ] **Phase 3: CLI Polish** - Wire the parser and renderer into a usable CLI + +## Phase Details + +### Phase 1: Parser +**Goal**: Parse a markdown task list into structured items +**Depends on**: Nothing (first phase) +**Requirements**: [REQ-01, REQ-02] +**Success Criteria** (what must be TRUE): + 1. A markdown checklist file is parsed into an ordered item list + 2. Malformed list items are skipped without crashing the parser +**Plans**: 2 plans + +Plans: +- [ ] 01-01: Implement the markdown task-list tokenizer +- [ ] 01-02: Implement the item-list builder + +### Phase 2: Printable Output +**Goal**: Render parsed items to a printable page +**Depends on**: Phase 1 +**Requirements**: [REQ-03] +**Success Criteria** (what must be TRUE): + 1. Parsed items render as a printable checklist page +**Plans**: 1 plan + +Plans: +- [ ] 02-01: Implement the printable renderer + +### Phase 3: CLI Polish +**Goal**: Wire the parser and renderer into a usable CLI +**Depends on**: Phase 2 +**Requirements**: [REQ-04, REQ-05] +**Success Criteria** (what must be TRUE): + 1. Running the CLI on a markdown file produces a printable page + 2. Invalid input paths produce a clear error message +**Plans**: 1 plan + +Plans: +- [ ] 03-01: Wire parser and renderer into the CLI entry point + +## Progress + +**Execution Order:** +Phases execute in numeric order: 1 → 2 → 3 + +| Phase | Plans Complete | Status | Completed | +|-------|----------------|--------|-----------| +| 1. Parser | 0/2 | Not started | - | +| 2. Printable Output | 0/1 | Not started | - | +| 3. CLI Polish | 0/1 | Not started | - | diff --git a/tests/qa/fixtures/state/interrupted-agent-id.md b/tests/qa/fixtures/state/interrupted-agent-id.md new file mode 100644 index 000000000..237ccf9f0 --- /dev/null +++ b/tests/qa/fixtures/state/interrupted-agent-id.md @@ -0,0 +1,2 @@ + +executor-01-01-a1b2c3d4 diff --git a/tests/qa/fixtures/state/mid-execute.md b/tests/qa/fixtures/state/mid-execute.md new file mode 100644 index 000000000..0a9a0c618 --- /dev/null +++ b/tests/qa/fixtures/state/mid-execute.md @@ -0,0 +1,22 @@ +--- +current_phase: "01" +current_phase_name: "Parser" +current_plan: "01-01" +total_plans_in_phase: "1" +status: "In Progress" +--- + + +# STATE + +## Current Position + +Executing Phase 01 (Parser), Plan 01-01. + +## Decisions Made + +| Phase | Summary | Rationale | +|-------|---------|-----------| + +## Blockers + diff --git a/tests/qa/fixtures/summary/tokenizer-complete.md b/tests/qa/fixtures/summary/tokenizer-complete.md new file mode 100644 index 000000000..41b9c105b --- /dev/null +++ b/tests/qa/fixtures/summary/tokenizer-complete.md @@ -0,0 +1,16 @@ +--- +phase: "01" +plan: "01-01" +status: complete +--- + + +# Plan 01-01 Summary + +## Completed + +- Tokenizer implemented and passing its own tests. + +## Outcome + +Phase 1 tokenizer work is done. diff --git a/tests/qa/fixtures/summary/tokenizer-partial.md b/tests/qa/fixtures/summary/tokenizer-partial.md new file mode 100644 index 000000000..a4adf14eb --- /dev/null +++ b/tests/qa/fixtures/summary/tokenizer-partial.md @@ -0,0 +1,17 @@ +--- +phase: "01" +plan: "01-01" +status: in-progress +--- + + +# Plan 01-01 Summary (PARTIAL — agent abandoned mid-execute) + +## Completed + +- Tokenizer skeleton scaffolded. + +## Remaining + +- Item-list builder not started. +- No tests written yet. diff --git a/tests/qa/fixtures/uat/current-test-failing.md b/tests/qa/fixtures/uat/current-test-failing.md new file mode 100644 index 000000000..2b5a60095 --- /dev/null +++ b/tests/qa/fixtures/uat/current-test-failing.md @@ -0,0 +1,26 @@ +--- +phase: "01" +name: "Parser" +created: 2026-01-01 +status: failed +--- + + +# Phase 1: Parser — User Acceptance Testing + +## Current Test + +### 1. Parse checklist + +expected: Items parse without crashing +result: failed + +## Test Results + +| # | Test | Status | Notes | +|---|------|--------|-------| +| 1 | Parse checklist | FAIL | tokenizer crashes on empty file | + +## Summary + +UAT FAILED — tokenizer crashes on empty file. diff --git a/tests/qa/fixtures/uat/current-test-passing.md b/tests/qa/fixtures/uat/current-test-passing.md new file mode 100644 index 000000000..8c4170c6b --- /dev/null +++ b/tests/qa/fixtures/uat/current-test-passing.md @@ -0,0 +1,28 @@ +--- +phase: "01" +name: "Parser" +created: 2026-01-01 +status: passed +--- + + +# Phase 1: Parser — User Acceptance Testing + +## Current Test + +[TESTING COMPLETE] + +### 1. Parse checklist + +expected: Items parse without crashing +result: passed + +## Test Results + +| # | Test | Status | Notes | +|---|------|--------|-------| +| 1 | Parse checklist | PASS | fixed empty-file crash | + +## Summary + +UAT PASSED after remediation. diff --git a/tests/qa/loop-walk.cjs b/tests/qa/loop-walk.cjs new file mode 100644 index 000000000..ff5b700ec --- /dev/null +++ b/tests/qa/loop-walk.cjs @@ -0,0 +1,330 @@ +'use strict'; + +/** + * loop-walk.cjs — a small stateful harness around `tests/helpers.cjs` for the + * loop QA walk. This module owns NONE of the subprocess mechanics itself + * (spawn, retry-on-kill, quote-splitting) — those live in `runGsdTools` / + * `createTempGitProject` and are reused verbatim. What this module adds is + * the walk-specific concerns: turning a raw helper result into the typed + * `RunResult` from `./result.cjs`, guaranteeing the child never inherits an + * ambient `GSD_*` variable that would silently redirect the engine, pinning + * the clock so output is reproducible run-to-run, and giving callers a + * content-free way to observe what the SUT wrote to disk. + */ + +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, cleanup } = require('../helpers.cjs'); +const { classify } = require('./result.cjs'); +const { createFixture } = require('../fixtures/index.cjs'); +const { LOOP_HOST_CONTRACT } = require('../../gsd-core/bin/lib/loop-host-contract.cjs'); +const { resolveWithin } = require('./paths.cjs'); + +/** + * WHY re-derive rather than re-list: `loop-host-contract.cjs` is itself + * generated (see its own header) from workflow markers by + * `scripts/gen-loop-host-contract.cjs`. If this file hardcoded + * `['discuss', 'plan', 'execute', 'verify', 'ship']`, a future regeneration + * that renames, adds, or removes a step would leave the QA walk silently + * testing a stale step list — a second source of truth that can drift out + * from under the generated one with no signal anywhere. Deriving `LOOP_STEPS` + * from the same contract object the generator produced means drift is + * structurally impossible: this array always has exactly the steps the + * contract currently declares. + * + * @type {string[]} + */ +const LOOP_STEPS = LOOP_HOST_CONTRACT.map((entry) => entry.step); + +/** Default pinned clock value (2026-01-01T00:00:00.000Z) — see `create({nowMs})`. */ +const DEFAULT_NOW_MS = 1767225600000; + +/** + * Starting-world builders keyed by `fixture` name, each a thin wrapper over + * `createFixture` (`tests/fixtures/index.cjs`). + * + * WHY this exists at all: `createFixture`'s `projectDoc` option defaults to + * `projectDoc = git` (`tests/fixtures/index.cjs:21`) — so any caller that + * only passes `git: true` silently also gets `.planning/PROJECT.md` seeded. + * A "greenfield" walk built that way would report `project_exists: true` + * from its very first step, which makes the greenfield trajectory + * meaningless: it can never exercise the create path (`/gsd-new-project`) + * because a project already exists before the walk begins. Every entry + * below therefore states `projectDoc` explicitly rather than relying on + * that default, and `greenfield` is the harness default precisely because + * an empty git repo with no `.planning/` at all is what a real user's + * working tree looks like before running `/gsd-new-project` for the first + * time. + * + * @type {Record string>} + */ +const FIXTURE_BUILDERS = { + // A git repo with NO `.planning/` at all — what a user has before + // `/gsd-new-project`. + greenfield: (prefix) => createFixture({ prefix, git: true, planning: false, projectDoc: false }), + // `.planning/phases/` exists, but no PROJECT.md yet. + planning: (prefix) => createFixture({ prefix, git: true, planning: true, projectDoc: false }), + // The old `createTempGitProject` behavior: a fully seeded project. + seeded: (prefix) => createFixture({ prefix, git: true, planning: true, projectDoc: true }), +}; + +/** + * Recursively collect `{size, mtimeMs}` stat facts for every regular file + * under `root`, excluding `.git/`. + * + * WHY stat-only, never read: `RULESET.TESTS.no-source-grep.tmp-file-traps` + * (see `result.cjs` header) forbids reading the content of files the SUT + * (system under test) wrote and then string-matching against it — that + * pattern is exactly the raw-text-matching anti-pattern the project's test + * conventions ban, just relocated from stdout to disk. `fs.statSync` proves + * a file exists, changed size, or changed mtime without ever opening its + * content, so a walk can assert "did this step write/touch a file" without + * ever being tempted into `readFileSync(...).includes(...)`. + * + * @param {string} root - absolute directory to walk. + * @returns {Map} keyed by + * POSIX-normalized path relative to `root`. + */ +function collectStatSnapshot(root) { + /** @type {Map} */ + const out = new Map(); + + function walk(dir) { + const entries = fs.readdirSync(dir, { withFileTypes: true }); + for (const entry of entries) { + if (entry.name === '.git') continue; + const abs = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(abs); + continue; + } + if (!entry.isFile()) continue; + const stat = fs.statSync(abs); + // WHY unconditional replace, not platform-conditional: `path.sep` is + // '/' on POSIX so a conditional swap looks like a no-op there, but a + // path segment can still literally contain a backslash character + // (e.g. an artifact file the SUT names with one) on Linux — so the + // normalization must run every time, not only when path.sep === '\\'. + const rel = path.relative(root, abs).replace(/\\/g, '/'); + out.set(rel, { size: stat.size, mtimeMs: stat.mtimeMs }); + } + } + + walk(root); + return out; +} + +class LoopWalk { + /** + * @param {string} dir - absolute project root (an already-created temp git project). + * @param {number} nowMs - pinned epoch ms passed to every `run()` as `GSD_NOW_MS`. + */ + constructor(dir, nowMs) { + this.dir = dir; + this.nowMs = nowMs; + } + + /** + * Alias for `this.dir` under the name `oracles.cjs`'s `ctx.projectDir` + * expects (see `value-hygiene`'s absolute-path-leak smell check). Kept as + * a getter rather than a second stored field so the two can never drift. + * + * @returns {string} + */ + get projectDir() { + return this.dir; + } + + /** + * Create a fresh temp git project and a `LoopWalk` bound to it. + * + * WHY `fixture` defaults to `'greenfield'`, not the old seeded behavior: + * see `FIXTURE_BUILDERS` above — a walk that starts with + * `.planning/PROJECT.md` already present can never exercise the + * project-creation path, which is the whole point of a "greenfield" walk. + * An unrecognized `fixture` name throws rather than silently falling back + * to a default, because a typo'd fixture name silently testing the wrong + * starting world is exactly the failure this harness exists to catch. + * + * @param {{prefix?: string, nowMs?: number, fixture?: 'greenfield'|'planning'|'seeded'}} [opts] + * @returns {LoopWalk} + */ + static create(opts = {}) { + const { prefix = 'gsd-loop-walk-', nowMs = DEFAULT_NOW_MS, fixture = 'greenfield' } = opts; + const build = FIXTURE_BUILDERS[fixture]; + if (!build) { + throw new Error( + `LoopWalk.create: unknown fixture "${fixture}" (expected one of: ${Object.keys(FIXTURE_BUILDERS).join(', ')})` + ); + } + const dir = build(prefix); + return new LoopWalk(dir, nowMs); + } + + /** + * Run a `gsd-tools` invocation inside this walk's project and return the + * typed `RunResult` from `classify()`. + * + * SIGNATURE: `run(...argvTokens)` where the LAST argument, if it is a + * plain object (not a string), is stripped off and treated as an options + * bag rather than an argv token — so `walk.run('progress')` and + * `walk.run('progress', { jsonErrors: false })` both read naturally + * against every existing call site in this repo (`walk.run(...argv)` in + * `scenario.cjs`, `walk.run('progress')` in the self-tests) without + * requiring callers to restructure a spread argv array around a leading + * options object. + * + * WHY `jsonErrors` defaults to `true`: `--json-errors` (`docs/json-errors.md`) + * is a real CLI flag, but it is the TOOLING/TEST surface — a human or a real + * workflow invokes `gsd_run ` directly, WITHOUT it. Defaulting to + * `true` keeps every pre-existing call site's behavior byte-for-byte + * unchanged (they all exercised `--json-errors` before this option + * existed), while `{ jsonErrors: false }` lets a scenario step opt into + * driving the human path instead — otherwise the harness would only ever + * prove the tooling surface works and could never catch a regression a + * real user would actually hit. + * + * WHY the env is built the way it is (ambient `GSD_*` sanitization): + * `runGsdTools` composes the child env as + * `{ ...process.env, ...TEST_ENV_BASE, ...env }` — so whatever `GSD_*` + * variables happen to be set in the *parent* shell (e.g. a developer or CI + * runner with `GSD_WORKSTREAM` / `GSD_PROJECT` exported for an unrelated + * reason) flow straight through into the child and can silently redirect + * the engine at a different workstream or project root than the one this + * walk created — an invisible, non-deterministic test-pollution vector. + * `runGsdTools`'s own merge order means the last object spread wins, so + * this method builds an `env` override that sets EVERY ambient `GSD_*` key + * (scanned live from `process.env`, not a hardcoded list — a new leaking + * var needs no code change here to be caught) to `undefined`, then layers + * the two intentionally-pinned vars on top. + * + * The `undefined` trick is deliberate, not a placeholder: Node's child + * process env normalization (`lib/child_process.js` `normalizeSpawnArgs`, + * exercised here via `execFileSync`) iterates `Object.keys(env)` and + * OMITS any key whose value is `undefined` from the actual `KEY=VALUE` + * pairs handed to the OS — it does not stringify it to the literal text + * `"undefined"`. That means `{ GSD_WORKSTREAM: undefined }` in the `env` + * option makes the child process behave exactly as if `GSD_WORKSTREAM` + * were never exported at all, even though `process.env.GSD_WORKSTREAM` is + * still set and non-empty in the parent. This was verified empirically + * (not assumed) — see the module verification transcript — because + * `delete`-based approaches were not available here (the merge is inside + * `runGsdTools`, not under this method's control) and a stringified + * `"undefined"` would have been a silent correctness bug indistinguishable + * from a passing run until an actual leak test caught it. + * + * ⚠️ KNOWN LIMIT — SUCCESS-PATH STDERR IS NOT OBSERVABLE THROUGH THIS + * SUBSTRATE, SO `result.warnings` IS ERROR-PATH-ONLY TODAY: this method + * builds `raw.stderr` as `result.success ? '' : (result.error ?? '')` + * (below), and `runGsdTools` (`tests/helpers.cjs`) invokes the child via + * `execFileSync`, which discards the child's stderr stream entirely on a + * clean (non-throwing) exit — Node never captures it, so there is no text + * to forward even if this method wanted to. The practical effect: for any + * exit-0 invocation, `classify()` always receives `stderr: ''`, so + * `result.warnings` can never be non-empty on the success path, no matter + * what the real `gsd-tools` process actually wrote to stderr. `warnings` + * only ever populates on the exit-1 (error) path, where `result.error` + * (helpers.cjs's captured stderr-on-failure text) is threaded through. + * Capturing success-path stderr would require changing `runGsdTools` / + * `tests/helpers.cjs` (e.g. to `spawnSync`) — out of scope here because + * that helper is shared by ~131 test files. DO NOT build an oracle that + * assumes `.warnings` reflects success-path stderr; it structurally cannot + * today, and a check written against that assumption is silently vacuous. + * + * @param {...(string|{jsonErrors?: boolean})} args - argv tokens, optionally + * followed by a trailing `{jsonErrors?: boolean}` options object. + * @returns {ReturnType} + */ + run(...args) { + const trailing = args[args.length - 1]; + const hasOptions = trailing !== null && typeof trailing === 'object' && !Array.isArray(trailing); + const options = hasOptions ? trailing : {}; + const argvTokens = hasOptions ? args.slice(0, -1) : args; + const { jsonErrors = true } = options; + const argv = jsonErrors ? ['--json-errors', ...argvTokens] : argvTokens; + + /** @type {Record} */ + const sanitize = {}; + for (const key of Object.keys(process.env)) { + if (key.startsWith('GSD_')) sanitize[key] = undefined; + } + const env = { + ...sanitize, + GSD_TEST_MODE: '1', + GSD_NOW_MS: String(this.nowMs), + }; + + let raw; + try { + const result = runGsdTools(argv, this.dir, env); + raw = { + exitCode: result.exitCode, + stdout: result.output, + stderr: result.success ? '' : (result.error ?? ''), + timedOut: false, + argv, + }; + } catch { + // `runGsdTools` throws only after a retried, persistent subprocess + // kill (host OOM / scheduler contention — see helpers.cjs + // `throwResourceStarvation`). That is a statement about the HOST, not + // the engine under test, so a walk must degrade to a TIMEOUT result + // rather than propagate and abort the whole walk over a transient + // resource condition it cannot control. + raw = { exitCode: null, stdout: '', stderr: '', timedOut: true, argv }; + } + return classify(raw); + } + + /** + * Write a planning artifact into this walk's project, standing in for what + * a real agent (researcher/planner/executor/...) would produce mid-loop. + * Creates parent directories as needed. + * + * `relPath` is resolved via `resolveWithin` before any I/O — a scenario- or + * caller-supplied path that escapes `this.dir` (e.g. `"../../escaped.md"`) + * throws rather than reaching `fs.writeFileSync` outside the temp project. + * + * @param {string} relPath - path relative to `this.dir`. + * @param {string} content + */ + writeArtifact(relPath, content) { + const abs = resolveWithin(this.dir, relPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content, 'utf-8'); + } + + /** + * Content-free snapshot of every file under this walk's project (excluding + * `.git/`). See `collectStatSnapshot` for why this never reads file bytes. + * + * @returns {Map} + */ + statSnapshot() { + return collectStatSnapshot(this.dir); + } + + /** + * Remove this walk's temp project. Safe to call multiple times. + * + * `opts.keep` (default `false`) skips the removal entirely — the caller + * (a QA-report run, typically via `--keep` / `GSD_QA_KEEP=1`) wants the + * failing/inspected tree left on disk for a human to `cd` into. When kept, + * this returns `this.dir` so the caller can record it (e.g. as + * `preservedDir` on a scenario report); when actually cleaned up, it + * returns `undefined`. + * + * @param {{keep?: boolean}} [opts] + * @returns {string|undefined} + */ + cleanup(opts = {}) { + const { keep = false } = opts; + if (keep) return this.dir; + cleanup(this.dir); + return undefined; + } +} + +LoopWalk.LOOP_STEPS = LOOP_STEPS; + +module.exports = { LoopWalk, LOOP_STEPS }; diff --git a/tests/qa/mutations.cjs b/tests/qa/mutations.cjs new file mode 100644 index 000000000..cb8461727 --- /dev/null +++ b/tests/qa/mutations.cjs @@ -0,0 +1,463 @@ +'use strict'; + +/** + * mutations.cjs — a catalog of deterministic corruptions applied to a + * planning-artifact string (or to the file on disk, for the two structural + * ones). + * + * WHY THIS FILE EXISTS + * ──────────────────── + * A QA harness walks an otherwise-valid scenario and, at one chosen step, + * applies exactly one mutation from `MUTATIONS`. The harness then asserts + * the engine under test degrades to a structured error rather than + * crashing, hanging, or silently producing wrong state. Every mutation here + * is a PURE, DETERMINISTIC transform (content-kind) or a scripted disk + * operation (file-kind) — no `Math.random()`, no `Date.now()`, no hidden + * global state — so a failing run is always reproducible from the mutation + * id alone. + * + * CONTENT-KIND mutations take a string and return a string (except + * `nonsequential-phases`, documented below, which may return the exported + * `NOOP` sentinel). FILE-KIND mutations take `{dir, relPath}` and mutate the + * file at `path.join(dir, relPath)` on disk; they return nothing. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const { resolveWithin } = require('./paths.cjs'); + +/** Distinguishable placeholder heading/content used by unicode-headings. */ +const UNICODE_HEADING_TEXT = '🚀 مرحبا Ünïcøde'; + +/** Cell content injected by escaped-pipes: an escaped pipe next to a real one. */ +const ESCAPED_PIPE_CELL = 'a \\| b | c'; + +/** + * Sentinel returned by `apply('nonsequential-phases', text)` when `text` + * contains no recognizable phase-id occurrence to renumber. A mutation that + * silently no-ops on unrecognized input is a vacuous test: returning this + * frozen, identity-comparable sentinel — instead of quietly handing back the + * unchanged string — forces callers to explicitly decide to skip the step + * rather than accidentally asserting against an untouched artifact. + * + * @type {{ readonly noop: true, readonly reason: string }} + */ +const NOOP = Object.freeze({ noop: true, reason: 'no phase-id occurrence found to renumber' }); + +/** + * Frozen catalog of every mutation this module implements. + * @type {ReadonlyArray<{ id: string, kind: 'content'|'file', describe: string }>} + */ +const MUTATIONS = Object.freeze([ + { + id: 'truncate-frontmatter', + kind: 'content', + describe: 'Cuts the text mid-YAML-frontmatter so the closing "---" is missing.', + }, + { + id: 'crlf', + kind: 'content', + describe: 'Converts every line ending to CRLF.', + }, + { + id: 'bom', + kind: 'content', + describe: 'Prefixes the text with a UTF-8 byte-order-mark character.', + }, + { + id: 'empty', + kind: 'content', + describe: 'Replaces the text with an empty string.', + }, + { + id: 'duplicate-phase-id', + kind: 'content', + describe: 'Duplicates the first phase-id heading or table row line.', + }, + { + id: 'nonsequential-phases', + kind: 'content', + describe: 'Renumbers phase-id occurrences to a non-sequential sequence (1, 3, 7, ...).', + }, + { + id: 'unicode-headings', + kind: 'content', + describe: 'Replaces every markdown heading\'s text with mixed unicode/emoji/RTL text.', + }, + { + id: 'oversized', + kind: 'content', + describe: 'Pads the text to just over a caller-supplied byte target.', + }, + { + id: 'escaped-pipes', + kind: 'content', + describe: 'Injects a table cell containing an escaped pipe next to a real one.', + }, + { + id: 'delete', + kind: 'file', + describe: 'Removes the file on disk.', + }, + { + id: 'symlink', + kind: 'file', + describe: 'Replaces the file with a symlink (or hardlink fallback) to a sibling copy.', + }, +]); + +/** + * Returns '\r\n' when `text` already contains at least one CRLF pair, + * otherwise '\n'. Used so line-oriented mutations rejoin with the + * predominant line ending already present rather than forcing one style. + * + * @param {string} text + * @returns {'\r\n'|'\n'} + */ +function detectEol(text) { + return text.includes('\r\n') ? '\r\n' : '\n'; +} + +/** + * Mutation: truncate-frontmatter. + * + * If `text` opens with a YAML frontmatter block (`---\n ... \n---`), the + * output is cut to a point strictly inside the frontmatter body, before the + * closing delimiter — so the closing `---` (and everything after it, + * including the real document body) is missing. If `text` has no + * frontmatter, a valid-looking opening block is synthesized and then itself + * truncated, so the mutation is never a no-op. + * + * @param {string} text + * @returns {string} + */ +function truncateFrontmatter(text) { + const openMatch = /^---\r?\n/.exec(text); + if (!openMatch) { + const synthetic = '---\r\ntitle: mutated\r\ndescription: truncated-frontmatter\r\n'; + return synthetic.slice(0, Math.floor(synthetic.length / 2)); + } + const openEnd = openMatch[0].length; + const closeMatch = /\r?\n---\r?\n?/.exec(text.slice(openEnd)); + if (!closeMatch) { + // Already has no closer (or is degenerate) — truncate further into + // whatever body remains so the mutation still meaningfully shortens it. + return text.slice(0, Math.max(openEnd, Math.floor(text.length / 2))); + } + const closeStart = openEnd + closeMatch.index; + const cutPoint = Math.max(openEnd, Math.floor((openEnd + closeStart) / 2)); + return text.slice(0, cutPoint); +} + +/** + * Mutation: crlf. + * + * Normalizes any existing CRLF to LF first, then converts every LF to CRLF — + * so text that already contains CRLF pairs is never doubled into `\r\r\n`, + * and the mutation is idempotent: `crlf(crlf(text)) === crlf(text)`. + * + * @param {string} text + * @returns {string} + */ +function crlf(text) { + return text.replace(/\r\n/g, '\n').replace(/\n/g, '\r\n'); +} + +/** + * Mutation: bom. + * + * Prefixes `text` with U+FEFF (byte-order-mark). + * + * @param {string} text + * @returns {string} + */ +function bom(text) { + return `${text}`; +} + +/** + * Mutation: duplicate-phase-id. + * + * Finds the first line matching a phase-id heading (`## Phase N`) or a + * phase-id table row (`| N | ... |`) and duplicates that whole line + * immediately after itself. If no such line exists, duplicates the first + * non-blank line instead, so the mutation still produces a structural + * duplicate. Returns `text` unchanged only when every line is blank. + * + * @param {string} text + * @returns {string} + */ +function duplicatePhaseId(text) { + const eol = detectEol(text); + const lines = text.split(/\r?\n/); + const headingRe = /^##\s+Phase\s+\d+\b/i; + const rowRe = /^\s*\|\s*\d+\s*\|/; + + let idx = lines.findIndex((line) => headingRe.test(line) || rowRe.test(line)); + if (idx === -1) { + idx = lines.findIndex((line) => line.trim() !== ''); + } + if (idx === -1) return text; + + const out = [...lines.slice(0, idx + 1), lines[idx], ...lines.slice(idx + 1)]; + return out.join(eol); +} + +/** + * Generates the k-th term (0-indexed) of the deterministic non-sequential + * skip series used by `nonsequential-phases`: 1, 3, 7, 15, 31, ... (2^(k+1) - 1). + * The first three terms are exactly the "1, 3, 7" example from the mutation + * catalog; the closed form extends deterministically to any number of + * phase-id occurrences without repeating a value. + * + * @param {number} k zero-based occurrence index + * @returns {number} + */ +function skipSeriesTerm(k) { + return Math.pow(2, k + 1) - 1; +} + +/** + * Mutation: nonsequential-phases. + * + * Rewrites every phase-id occurrence — `## Phase N` headings and `| N | ... |` + * table-row ids — to the deterministic skip series 1, 3, 7, 15, ... in order + * of appearance. If `text` contains no phase-id occurrence at all, there is + * nothing to renumber: returns the exported `NOOP` sentinel instead of + * silently handing back the unchanged string, so a caller cannot mistake a + * no-op for a genuine mutation. + * + * @param {string} text + * @returns {string | typeof NOOP} + */ +function nonsequentialPhases(text) { + let occurrence = 0; + let matched = false; + + const withHeadings = text.replace(/(##\s+Phase\s+)(\d+)/gi, (_match, prefix) => { + matched = true; + return `${prefix}${skipSeriesTerm(occurrence++)}`; + }); + + const withRows = withHeadings.replace(/^(\s*\|\s*)(\d+)(\s*\|)/gm, (_match, prefix, _num, suffix) => { + matched = true; + return `${prefix}${skipSeriesTerm(occurrence++)}${suffix}`; + }); + + return matched ? withRows : NOOP; +} + +/** + * Mutation: unicode-headings. + * + * Replaces every markdown heading's text (the part after the `#` run and + * whitespace) with a fixed mixed unicode/emoji/RTL string, preserving the + * heading level and leading whitespace. + * + * @param {string} text + * @returns {string} + */ +function unicodeHeadings(text) { + return text.replace(/^(#{1,6})(\s+).*$/gm, (_match, hashes, ws) => `${hashes}${ws}${UNICODE_HEADING_TEXT}`); +} + +/** + * Mutation: oversized. + * + * Pads `text` with a deterministic ASCII filler so the UTF-8 byte length of + * the result is strictly greater than `opts.targetBytes` (exactly one byte + * over when `text` is already at or under the target, since the filler is + * single-byte-per-character). Callers drive their own boundary tests + * (limit-1 / limit / limit+1) by varying `targetBytes` across calls; if + * `text` already exceeds `targetBytes`, it is returned unchanged since it is + * already oversized relative to that target. + * + * @param {string} text + * @param {{ targetBytes: number }} opts + * @returns {string} + */ +function oversized(text, opts) { + if (!opts || !Number.isFinite(opts.targetBytes) || opts.targetBytes < 0) { + throw new Error('apply("oversized", text, opts): opts.targetBytes must be a non-negative finite number'); + } + const { targetBytes } = opts; + const currentBytes = Buffer.byteLength(text, 'utf8'); + if (currentBytes > targetBytes) return text; + const FILL_CHAR = 'X'; + const deficit = targetBytes - currentBytes + 1; + return text + FILL_CHAR.repeat(deficit); +} + +/** + * Mutation: escaped-pipes. + * + * Finds the first markdown table row (a line whose trimmed form starts and + * ends with `|`) and appends a cell containing an escaped pipe next to a + * real one (`a \| b | c`) before the row's closing pipe. If no table row + * exists, appends a new one-row table containing that cell. + * + * @param {string} text + * @returns {string} + */ +function escapedPipes(text) { + const eol = detectEol(text); + const lines = text.split(/\r?\n/); + const rowRe = /^\s*\|.*\|\s*$/; + const idx = lines.findIndex((line) => rowRe.test(line)); + + if (idx === -1) { + const separator = text === '' ? '' : eol; + return `${text}${separator}| ${ESCAPED_PIPE_CELL} |`; + } + + const trimmed = lines[idx].replace(/\s+$/, ''); + const out = [...lines]; + out[idx] = `${trimmed} ${ESCAPED_PIPE_CELL} |`; + return out.join(eol); +} + +/** + * Resolves `{dir, relPath}` to an absolute on-disk path via `resolveWithin` + * (`./paths.cjs`) — the single containment guard for this harness. `relPath` + * is scenario-supplied (`step.mutate.target`), so this is the seam both + * `deleteFile` and `symlinkFile` inherit: a traversing `relPath` (e.g. + * `"../../../../etc/hosts"`) throws here rather than reaching + * `fs.unlinkSync` / `fs.symlinkSync` outside `dir`. + * + * @param {string} dir + * @param {string} relPath + * @returns {string} + */ +function resolveTargetPath(dir, relPath) { + return resolveWithin(dir, relPath); +} + +/** + * Mutation: delete (file-kind). + * + * Removes the file at `path.join(dir, relPath)`. + * + * @param {{ dir: string, relPath: string }} target + * @returns {void} + */ +function deleteFile(target) { + const filePath = resolveTargetPath(target.dir, target.relPath); + fs.unlinkSync(filePath); +} + +/** + * Mutation: symlink (file-kind). + * + * Replaces the file at `path.join(dir, relPath)` with a symlink pointing at + * a sibling file (in the same directory) holding the same bytes. On + * platforms where `fs.symlinkSync` fails with a permission error (e.g. + * Windows without developer mode / elevated privileges), falls back to a + * hard link (`fs.linkSync`). + * + * Never leaves the tree half-mutated: link capability is first proven + * against a disposable probe path in the same directory (which never + * touches the real target). Only once that probe succeeds is the real + * target removed and immediately replaced using the now-proven method — so + * a platform that supports neither symlinks nor hard links throws before + * the target file is ever deleted, and a target that IS deleted is + * guaranteed a same-shaped replacement (barring an out-of-process deletion + * racing this function, which is out of scope for a single-writer QA tool). + * + * @param {{ dir: string, relPath: string }} target + * @returns {void} + */ +function symlinkFile(target) { + const filePath = resolveTargetPath(target.dir, target.relPath); + const bytes = fs.readFileSync(filePath); + const siblingPath = path.join(path.dirname(filePath), `.mutation-sibling-${path.basename(filePath)}`); + fs.writeFileSync(siblingPath, bytes); + + const probePath = `${filePath}.mutation-probe`; + try { + fs.unlinkSync(probePath); + } catch { + // No stale probe from a previous failed run — nothing to clean up. + } + + let useHardlink = false; + try { + fs.symlinkSync(siblingPath, probePath); + } catch (symlinkErr) { + if (process.platform !== 'win32') throw symlinkErr; + try { + fs.linkSync(siblingPath, probePath); + useHardlink = true; + } catch (linkErr) { + throw new Error( + 'symlink mutation unavailable on this platform: ' + + `symlink failed (${symlinkErr.message}) and hardlink fallback failed (${linkErr.message})`, + ); + } + } + fs.unlinkSync(probePath); + + // Link capability proven above — safe to swap the real target now. + fs.unlinkSync(filePath); + if (useHardlink) { + fs.linkSync(siblingPath, filePath); + } else { + fs.symlinkSync(siblingPath, filePath); + } +} + +/** + * Applies a single mutation by id. + * + * - content-kind mutations: `apply(id, input)` where `input` is a string, + * returning the mutated string (or, only for `nonsequential-phases`, the + * `NOOP` sentinel — see that mutation's doc comment). + * - `oversized` additionally takes a third argument: `apply('oversized', + * text, {targetBytes})`. + * - file-kind mutations: `apply(id, {dir, relPath})`, mutating the file on + * disk and returning nothing. + * + * @param {string} id one of MUTATIONS[].id + * @param {string | { dir: string, relPath: string }} input + * @param {{ targetBytes?: number }} [opts] + * @returns {string | void | typeof NOOP} + */ +function apply(id, input, opts) { + const entry = MUTATIONS.find((m) => m.id === id); + if (!entry) { + throw new Error(`apply: unknown mutation id "${String(id)}" (known ids: ${MUTATIONS.map((m) => m.id).join(', ')})`); + } + + switch (id) { + case 'truncate-frontmatter': + return truncateFrontmatter(input); + case 'crlf': + return crlf(input); + case 'bom': + return bom(input); + case 'empty': + return ''; + case 'duplicate-phase-id': + return duplicatePhaseId(input); + case 'nonsequential-phases': + return nonsequentialPhases(input); + case 'unicode-headings': + return unicodeHeadings(input); + case 'oversized': + return oversized(input, opts); + case 'escaped-pipes': + return escapedPipes(input); + case 'delete': + return deleteFile(input); + case 'symlink': + return symlinkFile(input); + /* istanbul ignore next -- unreachable: entry lookup above already validated id */ + default: + throw new Error(`apply: unhandled mutation id "${id}"`); + } +} + +module.exports = { + MUTATIONS, + apply, + NOOP, +}; diff --git a/tests/qa/oracles.cjs b/tests/qa/oracles.cjs new file mode 100644 index 000000000..0c1d6cec9 --- /dev/null +++ b/tests/qa/oracles.cjs @@ -0,0 +1,716 @@ +'use strict'; + +/** + * oracles.cjs — the QA-walk assertion set for `RunResult` values produced by + * `tests/qa/result.cjs`. + * + * WHY THIS FILE EXISTS + * ──────────────────── + * `CONTRIBUTING.md` → "Prohibited: Raw Text Matching on Test Outputs" and + * `RULESET.TESTS.no-source-grep.tmp-file-traps` forbid two things an oracle + * might otherwise reach for: (1) inspecting a child process's raw stdout / + * stderr text, and (2) reading the *content* of a file the system under test + * wrote. `result.cjs` is the single place raw bytes are turned into the typed + * `RunResult` (`{kind, exitCode, argv, json, err, pointer, warnings}`); every + * oracle in this file therefore asserts ONLY on: + * + * - `RunResult.kind` (one of the frozen `KIND` values), + * - parsed `json` / `err` fields (already-structured data, never prose), + * - `fs.statSync` FACTS handed in via `ctx.statsBefore` / `ctx.statsAfter` + * (`size`, `mtimeMs`, `isFile()`) — never file *contents*. + * + * No oracle here calls `fs.readFileSync` on an SUT-written artifact, and none + * does substring/regex matching against a raw output string. Where a check + * looks like it could be tempted into substring matching (value-hygiene, + * below) it deliberately uses strict equality against a closed sentinel set + * instead, which is what keeps it honest. + * + * ORACLE 3 AND ABSOLUTE PATHS + * ─────────────────────────── + * `value-hygiene` flags a string only when it is *exactly* one of + * `'undefined' | 'null' | 'NaN' | '[object Object]'` (the canonical + * stringification artifacts of a real bug — a coercion that dropped a value). + * It never does a substring/`.includes()` scan for a fragment like `null` or + * `undefined` inside a larger string. That distinction matters because + * `init` legitimately returns absolute filesystem paths, and an absolute path + * can coincidentally *contain* a directory or file segment that looks like + * one of those words (e.g. a user-named directory). A substring scan would + * flag those as defects; a strict-equality scan against the closed sentinel + * set structurally cannot, because a real path is never bit-for-bit equal to + * `'undefined'` etc. False positives are worse than a missed defect for a QA + * tool — they train operators to ignore the tool — so this oracle is + * deliberately conservative. + * + * SEVERITY MODEL: VIOLATION vs SMELL + * ─────────────────────────────────── + * The original nine oracles encoded today's engine behavior as the spec: + * anything the engine currently does was, by construction, "legal" — the + * harness could confirm the status quo but never say "this works but is + * questionable." `SEVERITY` splits `check(ctx)` outcomes into two kinds: + * + * - `SEVERITY.VIOLATION` — the documented contract is broken. This is what + * `failed` (and therefore `failed.length === 0` build gates) has always + * meant, and it keeps meaning exactly that after this change. + * - `SEVERITY.SMELL` — behavior that is legal today, does not fail any + * documented contract, but is evidence worth a human's attention (a + * design trade-off, a doc/code disagreement, a leaking value). A smell + * is reported in `runOracles(ctx).smells` and MUST NEVER appear in + * `failed` — it must never break a build. Its only job is to keep a + * known trade-off visible instead of silently invisible. + * + * A `check(ctx)` that fails now returns `{ ok: false, severity, detail }`; + * `severity` defaults to `SEVERITY.VIOLATION` for every original oracle. + */ + +const nodePath = require('node:path'); +const { KIND } = require('./result.cjs'); +const { resolveForCompare, isUnderProjectDir } = require('./paths.cjs'); + +/** Severity of a failed oracle outcome. A SMELL is evidence, not a verdict — it never fails a build. */ +const SEVERITY = Object.freeze({ VIOLATION: 'violation', SMELL: 'smell' }); + +/** Exact-match sentinel strings that indicate a value was coerced by mistake. */ +const SENTINEL_STRINGS = new Set(['undefined', 'null', 'NaN', '[object Object]']); + +/** + * Leaf JSON-key names whose CONTRACT is to point OUTSIDE the project directory — + * `value-hygiene`'s absolute-path-leak sub-check allowlists these by LEAF key + * name only (never by value, never by full path), so a legitimately-external + * field never manufactures a finding. This is an allowlist of contractually + * external fields, NOT a general suppression: adding a key here requires + * actually knowing that field's contract — that EVERY value it ever holds is + * expected to live outside `ctx.projectDir` — not just that it happened to fire + * once. The sub-check still fires (as a SMELL) for any absolute path outside + * the project on a key NOT in this set — see #2966 FIX 1. + * + * - `agents_dir` — `agent-install-check.cts`'s `getAgentsDir` / + * `checkAgentsInstalled`, surfaced on every `init` subcommand's response via + * `init.cts`'s `withProjectRoot`. Points at the INSTALL tree (the runtime's + * global config dir, or — for the `claude` runtime — the `agents/` directory + * bundled as a sibling of `gsd-core/`), never at the project: agents are + * installed once, not per-project. + */ +const EXTERNAL_PATH_ALLOWED_KEYS = Object.freeze(new Set(['agents_dir'])); + +/** + * Extract the leaf key name from a `walk()`-built path (e.g. `"$.agents_dir"` + * -> `"agents_dir"`, `"$.foo.bar[3]"` -> `"bar"` for the array element itself, + * `"$.arr[3]"` -> `"arr"` is NOT how this parses — array indices are not key + * names, so a leaf under an array index has no matching allowlist entry by + * design; only a genuine object key can match `EXTERNAL_PATH_ALLOWED_KEYS`. + * + * @param {string} walkPath + * @returns {string} + */ +function leafKeyOf(walkPath) { + const segments = walkPath.split('.'); + const last = segments[segments.length - 1]; + const bracketIdx = last.indexOf('['); + return bracketIdx === -1 ? last : last.slice(0, bracketIdx); +} + +/** `json` field names checked by `monotonic-progress`, in no particular order. */ +const PROGRESS_KEYS = ['total_plans', 'total_summaries', 'phases_completed']; + +/** + * Extract the active workstream id from an argv array, e.g. + * `['--json-errors', '--ws', 'alpha', 'progress']` -> `'alpha'`. Returns `null` + * when no `--ws` flag is present (the default/unnamed workstream) — never + * `undefined`, so two "no workstream" entries compare equal via `===`. + * + * @param {unknown} argv + * @returns {string | null} + */ +function workstreamFromArgv(argv) { + if (!Array.isArray(argv)) return null; + const idx = argv.indexOf('--ws'); + if (idx === -1 || idx + 1 >= argv.length) return null; + const value = argv[idx + 1]; + return typeof value === 'string' ? value : null; +} + +/** + * The comparability scope for a `monotonic-progress` entry: two entries are + * comparable only when their milestone (`milestone_version` + `milestone_name`, + * the fields a real `progress`/`stats` payload actually exposes — see + * `roadmap-parser.cts` `getMilestoneInfo`) AND active workstream (derived from + * the invocation's own `argv`, since no payload observed in this codebase + * carries a workstream field) all match. `milestone_version` alone is NOT + * sufficient: a fixture/project whose ROADMAP.md never carries an explicit + * `vX.Y` marker keeps reporting the same fallback `"v1.0"`/`"milestone"` pair + * across a real milestone-complete boundary until a roadmap for the *next* + * milestone is actually written (verified empirically against `milestone + * complete` — see scenarios/milestone-rollover.json) — `milestone_name` is + * threaded in alongside version for the same reason value-hygiene documents + * elsewhere in this file: a cheap, purely-structural signal is preferred over + * inferring intent from command names. + * + * @param {{json?: unknown, argv?: unknown}} entry + * @returns {{milestoneVersion: unknown, milestoneName: unknown, workstream: string | null}} + */ +function progressScopeOf(entry) { + const json = entry && entry.json; + const isPlainObject = json !== null && typeof json === 'object' && !Array.isArray(json); + return { + milestoneVersion: isPlainObject ? json.milestone_version : undefined, + milestoneName: isPlainObject ? json.milestone_name : undefined, + workstream: workstreamFromArgv(entry && entry.argv), + }; +} + +/** + * @param {ReturnType} a + * @param {ReturnType} b + * @returns {boolean} + */ +function scopeEqual(a, b) { + return a.milestoneVersion === b.milestoneVersion + && a.milestoneName === b.milestoneName + && a.workstream === b.workstream; +} + +/** + * Deep-walk an arbitrary JSON-ish value, invoking `visit(primitive, path)` for + * every non-object leaf. Cycle-safe via a `WeakSet` of visited objects/arrays, + * so a self-referential structure terminates instead of recursing forever. + * + * @param {unknown} value + * @param {(leaf: unknown, path: string) => void} visit + * @param {WeakSet} seen + * @param {string} path + */ +function walk(value, visit, seen, path) { + if (value === null || typeof value !== 'object') { + visit(value, path); + return; + } + if (seen.has(value)) return; + seen.add(value); + if (Array.isArray(value)) { + value.forEach((item, i) => walk(item, visit, seen, `${path}[${i}]`)); + return; + } + for (const key of Object.keys(value)) { + walk(value[key], visit, seen, `${path}.${key}`); + } +} + +/** + * Structural equality for JSON-ish values. Cycle-tolerant via a `WeakMap` that + * pairs "already compared" object references, so a self-referential structure + * terminates instead of recursing forever. + * + * @param {unknown} a + * @param {unknown} b + * @param {WeakMap} seen + * @returns {boolean} + */ +function deepEqual(a, b, seen) { + if (Object.is(a, b)) return true; + if (a === null || b === null) return false; + if (typeof a !== 'object' || typeof b !== 'object') return false; + if (Array.isArray(a) !== Array.isArray(b)) return false; + if (seen.get(a) === b) return true; + seen.set(a, b); + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (aKeys.length !== bKeys.length) return false; + for (const key of aKeys) { + if (!Object.prototype.hasOwnProperty.call(b, key)) return false; + if (!deepEqual(a[key], b[key], seen)) return false; + } + return true; +} + +/** + * @typedef {{ ok: true } | { ok: false, severity: string, detail: string, subject?: object }} OracleOutcome + * + * `subject` — OPTIONAL structured data backing `detail`, present where the oracle knows a + * machine-checkable shape (e.g. `{ argv: string[] }` for a command-scoped finding, + * `{ key: string, value: unknown }` for a value-hygiene leaf, `{ missing: string }` for + * read-only-idempotence's absent-input case). `detail` remains a human-readable string for + * console/report output; tests MUST assert on `subject`, never on substrings of `detail` + * (CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs"). + */ + +/** + * Frozen array of `{ id, describe, check(ctx) -> OracleOutcome }` oracles. + * + * `ctx` shape (all optional except `result`): + * { result, prevResult, repeatResult, statsBefore, statsAfter, history, + * liveCommands, readOnly } + * + * Every `check` is wrapped in its own try/catch so a bug in one oracle can + * never take the whole walk down — an oracle that throws is converted into a + * `{ok:false}` naming the exception rather than crashing `runOracles`. + */ +const ORACLES = Object.freeze([ + Object.freeze({ + id: 'exit-contract', + describe: 'result.kind must not be UNEXPECTED_EXIT or TIMEOUT.', + check(ctx) { + try { + const kind = ctx && ctx.result && ctx.result.kind; + if (kind === KIND.UNEXPECTED_EXIT || kind === KIND.TIMEOUT) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `result.kind is "${kind}"` }; + } + return { ok: true }; + } catch (err) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `exit-contract threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'json-contract', + describe: + 'result.kind must not be UNSTRUCTURED_ERROR; a STRUCTURED_ERROR must carry err.ok===false and a non-empty err.reason.', + check(ctx) { + try { + const result = ctx && ctx.result; + const kind = result && result.kind; + if (kind === KIND.UNSTRUCTURED_ERROR) { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: 'result.kind is "unstructured-error" (raw error text; --json-errors was ignored)', + }; + } + if (kind === KIND.STRUCTURED_ERROR) { + const err = result.err; + if (!err || typeof err !== 'object') { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: `result.err is ${JSON.stringify(err)}, expected an object`, + }; + } + if (err.ok !== false) { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: `result.err.ok is ${JSON.stringify(err.ok)}, expected false`, + }; + } + if (typeof err.reason !== 'string' || err.reason === '') { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: `result.err.reason is ${JSON.stringify(err.reason)}, expected a non-empty string`, + }; + } + } + return { ok: true }; + } catch (err) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `json-contract threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'value-hygiene', + describe: + 'result.json must not contain a NaN number or a string exactly equal to a coercion-artifact sentinel ' + + '(VIOLATION). When ctx.projectDir is supplied, an absolute-path string outside it is reported as a SMELL — ' + + 'never a violation — UNLESS its leaf key name is in EXTERNAL_PATH_ALLOWED_KEYS (a field whose contract is to ' + + 'point outside the project, e.g. agents_dir), which is skipped entirely: not a smell, not a violation. Without ' + + 'ctx.projectDir the path check is skipped rather than guessed.', + check(ctx) { + try { + /** @type {{message: string, key: string, value: unknown}[]} */ + const violations = []; + /** @type {{message: string, key: string, value: unknown}[]} */ + const smells = []; + const seen = new WeakSet(); + const json = ctx && ctx.result ? ctx.result.json : undefined; + const projectDir = ctx && typeof ctx.projectDir === 'string' ? ctx.projectDir : null; + // Resolved once per runOracles call (this check runs exactly once per ctx), not once + // per candidate string below — projectDir is the same value for every leaf in the walk. + const resolvedProjectDir = projectDir !== null ? resolveForCompare(projectDir) : null; + walk(json, (leaf, path) => { + if (typeof leaf === 'number' && Number.isNaN(leaf)) { + violations.push({ message: `NaN at ${path}`, key: path, value: leaf }); + } else if (typeof leaf === 'string' && SENTINEL_STRINGS.has(leaf)) { + violations.push({ message: `sentinel string ${JSON.stringify(leaf)} at ${path}`, key: path, value: leaf }); + } else if ( + resolvedProjectDir !== null && + typeof leaf === 'string' && + nodePath.isAbsolute(leaf) && + !isUnderProjectDir(resolveForCompare(leaf), resolvedProjectDir) && + !EXTERNAL_PATH_ALLOWED_KEYS.has(leafKeyOf(path)) + ) { + smells.push({ + message: `absolute path ${JSON.stringify(leaf)} at ${path} is outside ctx.projectDir ${JSON.stringify(projectDir)}`, + key: path, + value: leaf, + }); + } + }, seen, '$'); + if (violations.length) { + return { + ok: false, + severity: SEVERITY.VIOLATION, + subject: { key: violations[0].key, value: violations[0].value }, + detail: violations.map((v) => v.message).join('; '), + }; + } + if (smells.length) { + return { + ok: false, + severity: SEVERITY.SMELL, + subject: { key: smells[0].key, value: smells[0].value }, + detail: smells.map((s) => s.message).join('; '), + }; + } + return { ok: true }; + } catch (err) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `value-hygiene threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'read-only-idempotence', + describe: + 'For commands documented as read-only: repeatResult.json must deep-equal result.json, and statsBefore/statsAfter must be identical on every key. ' + + 'An oracle asked to check idempotence without the data to check it FAILS (VIOLATION) rather than passing vacuously — missing repeatResult, ' + + 'statsBefore, or statsAfter is itself a finding, named explicitly in the detail.', + check(ctx) { + try { + if (!ctx || !ctx.readOnly) return { ok: true }; + const findings = []; + const missing = []; + if (!ctx.repeatResult) { + missing.push('repeatResult'); + findings.push('ctx.readOnly is true but ctx.repeatResult is missing — idempotence cannot be checked'); + } else if (ctx.result) { + if (!deepEqual(ctx.result.json, ctx.repeatResult.json, new WeakMap())) { + findings.push('repeatResult.json is not deep-equal to result.json'); + } + } + if (!(ctx.statsBefore instanceof Map)) { + missing.push('statsBefore'); + findings.push('ctx.readOnly is true but ctx.statsBefore is missing — idempotence cannot be checked'); + } + if (!(ctx.statsAfter instanceof Map)) { + missing.push('statsAfter'); + findings.push('ctx.readOnly is true but ctx.statsAfter is missing — idempotence cannot be checked'); + } + const before = ctx.statsBefore instanceof Map ? ctx.statsBefore : new Map(); + const after = ctx.statsAfter instanceof Map ? ctx.statsAfter : new Map(); + const beforeKeys = new Set(before.keys()); + const afterKeys = new Set(after.keys()); + const sameKeys = + beforeKeys.size === afterKeys.size && [...beforeKeys].every((k) => afterKeys.has(k)); + if (!sameKeys) { + findings.push( + `statsBefore/statsAfter key sets differ: before=[${[...beforeKeys].join(',')}] after=[${[...afterKeys].join(',')}]`, + ); + } else { + for (const key of beforeKeys) { + const b = before.get(key); + const a = after.get(key); + if (b && a && b.size !== a.size) { + findings.push(`stat "${key}".size changed: ${b.size} -> ${a.size}`); + } + if (b && a && b.mtimeMs !== a.mtimeMs) { + findings.push(`stat "${key}".mtimeMs changed: ${b.mtimeMs} -> ${a.mtimeMs}`); + } + } + } + if (!findings.length) return { ok: true }; + const subject = missing.length ? { missing: missing.join(', ') } : { mismatches: findings }; + return { ok: false, severity: SEVERITY.VIOLATION, subject, detail: findings.join('; ') }; + } catch (err) { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: `read-only-idempotence threw: ${err && err.message}`, + }; + } + }, + }), + + Object.freeze({ + id: 'monotonic-progress', + describe: + 'Numeric total_plans/total_summaries/phases_completed fields must never decrease across history + result, ' + + 'in walk order, WITHIN a comparable scope (VIOLATION). A scope is the pair of {milestone_version + ' + + 'milestone_name} from the payload and the active workstream derived from the invocation\'s own argv ' + + '(`--ws `, see `progressScopeOf`). Two consecutive observations are compared using a THREE-WAY rule ' + + 'keyed on whether each one\'s payload carries a milestone scope at all (`hasScope`, true when the payload ' + + 'has `milestone_version` and/or `milestone_name`; see below):\n' + + ' 1. BOTH have scope -> compare via `scopeEqual` (milestone_version + milestone_name + workstream). Same ' + + ' scope: a decrease is a VIOLATION. Different scope: a milestone/workstream boundary crossing legally ' + + ' resets the counters, so this resets the comparison SILENTLY — expected behavior, never reported (not ' + + ' a violation, not a smell). See #2966 FIX 2(a). This observation becomes the new reference point.\n' + + ' 2. NEITHER has scope -> they share the same (absent) milestone scope by construction, so the SAME ' + + ' `scopeEqual` comparison applies (it degenerates to comparing `undefined === undefined` plus ' + + ' workstream, e.g. a `--ws` switch still resets silently even with no milestone fields at all): a ' + + ' same-scope decrease is a VIOLATION. This is the branch a prior fix got wrong (see WHY THE BLANKET ' + + ' SKIP WAS WRONG below) and is the one #2966 FIX 2(b) restores. This observation becomes the new ' + + ' reference point.\n' + + ' 3. MIXED (one has scope, the other does not) -> genuinely indeterminate; this pair is not compared, and ' + + ' — unlike branches 1/2 — the mixed observation does NOT replace the reference point, so the NEXT ' + + ' entry is compared against the last observation whose scope-category matched. This is the `roadmap ' + + ' analyze` (no milestone fields) sitting between two `progress` observations (milestone fields) case ' + + ' that motivated scope-awareness in the first place: it must not mask a real same-scope decrease on ' + + ' either side of it.\n\n' + + 'WHY THE BLANKET SKIP WAS WRONG: an earlier version of this oracle skipped ANY entry lacking both milestone ' + + 'fields entirely — it neither compared it nor let it become the reference point. That was meant to fix ' + + 'branch 3 above, but it silently also disabled branch 2: a minimal payload like `{ total_summaries: n }` ' + + '(no milestone fields at all, e.g. the oracle\'s own self-test fixtures) could never trigger a violation no ' + + 'matter how far it decreased, because it could never be compared against anything. That is a false ' + + 'NEGATIVE in the exact defect this oracle exists to catch, and it is strictly worse than the false ' + + 'POSITIVE the blanket skip was trying to fix — it was caught only by the full remote suite\'s self-tests, ' + + 'not by any local check. Do not re-broaden this to a blanket "no scope fields -> skip" rule; keep the ' + + 'three-way branch above, where "neither has scope" is fully comparable and only a genuine MIX is skipped.', + check(ctx) { + try { + const history = ctx && Array.isArray(ctx.history) ? ctx.history : []; + const entries = ctx && ctx.result ? [...history, ctx.result] : history; + /** @type {{key:string, from:number, to:number, fromIndex:number, toIndex:number}[]} */ + const violations = []; + for (const key of PROGRESS_KEYS) { + /** @type {{value:number, index:number, scope:ReturnType, hasScope:boolean} | undefined} */ + let prev; + entries.forEach((entry, index) => { + const json = entry && entry.json; + if (json === null || typeof json !== 'object' || Array.isArray(json)) return; + const value = json[key]; + if (typeof value !== 'number' || Number.isNaN(value)) return; + const scope = progressScopeOf(entry); + const hasScope = scope.milestoneVersion !== undefined || scope.milestoneName !== undefined; + if (prev === undefined) { + prev = { value, index, scope, hasScope }; + return; + } + if (prev.hasScope !== hasScope) { + // Branch 3: mixed — genuinely indeterminate. Skip this comparison AND leave + // `prev` untouched, so the next entry is still compared against the last + // scope-category-matching observation (see JSDoc above). + return; + } + // Branch 1 (both scoped) and Branch 2 (neither scoped) are the SAME check: + // `scopeEqual` naturally degenerates to comparing undefined===undefined plus + // workstream when neither side carries milestone fields. + if (scopeEqual(prev.scope, scope) && value < prev.value) { + violations.push({ + key, from: prev.value, to: value, fromIndex: prev.index, toIndex: index, + }); + } + prev = { value, index, scope, hasScope }; + }); + } + if (violations.length) { + const first = violations[0]; + return { + ok: false, + severity: SEVERITY.VIOLATION, + subject: { key: first.key, from: first.from, to: first.to, fromIndex: first.fromIndex, toIndex: first.toIndex }, + detail: violations + .map((v) => `${v.key} decreased from ${v.from} (entry ${v.fromIndex}) to ${v.to} (entry ${v.toIndex})`) + .join('; '), + }; + } + return { ok: true }; + } catch (err) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `monotonic-progress threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'routing-validity', + describe: + 'A result.json.recommended / recommended_command token must name a command present in ctx.liveCommands.', + check(ctx) { + try { + const json = ctx && ctx.result ? ctx.result.json : undefined; + if (json === null || typeof json !== 'object' || Array.isArray(json)) return { ok: true }; + const field = Object.prototype.hasOwnProperty.call(json, 'recommended') + ? 'recommended' + : Object.prototype.hasOwnProperty.call(json, 'recommended_command') + ? 'recommended_command' + : null; + if (field === null) return { ok: true }; + const value = json[field]; + if (typeof value !== 'string') return { ok: true }; + const liveCommands = ctx && Array.isArray(ctx.liveCommands) ? ctx.liveCommands : []; + if (!liveCommands.includes(value)) { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: `${field}=${JSON.stringify(value)} is not in ctx.liveCommands`, + }; + } + return { ok: true }; + } catch (err) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `routing-validity threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'determinism', + describe: 'When a repeatResult is present, its kind must match the original result.kind.', + check(ctx) { + try { + if (!ctx || !ctx.repeatResult || !ctx.result) return { ok: true }; + if (ctx.repeatResult.kind !== ctx.result.kind) { + return { + ok: false, + severity: SEVERITY.VIOLATION, + detail: `repeatResult.kind "${ctx.repeatResult.kind}" !== result.kind "${ctx.result.kind}"`, + }; + } + return { ok: true }; + } catch (err) { + return { ok: false, severity: SEVERITY.VIOLATION, detail: `determinism threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'soft-error-exit-zero', + describe: + 'SMELL, not a violation: result.kind === KIND.SOFT_ERROR means the operation reported failure through a ' + + 'payload key ({error:...}) while exiting 0. Every shell caller\'s `if ! cmd; then` is blind to that failure. ' + + 'This is legal today (42 call sites use output({error:...})) and is deliberately NOT changed here — the ' + + 'blast radius of changing it is CRITICAL — but it is reported so the trade stays visible instead of invisible.', + check(ctx) { + try { + const result = ctx && ctx.result; + if (!result || result.kind !== KIND.SOFT_ERROR) return { ok: true }; + const argv = Array.isArray(result.argv) ? result.argv : []; + const argvDisplay = argv.length ? argv.join(' ') : '(no argv)'; + const errorValue = result.json && typeof result.json === 'object' ? result.json.error : undefined; + return { + ok: false, + severity: SEVERITY.SMELL, + subject: { argv }, + detail: `command "${argvDisplay}" exited 0 but carries result.json.error = ${JSON.stringify(errorValue)}`, + }; + } catch (err) { + return { ok: false, severity: SEVERITY.SMELL, detail: `soft-error-exit-zero threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'untyped-success', + describe: + 'SMELL, not a violation: result.kind === KIND.PROSE means the command only emits rendered text with no ' + + 'typed surface to assert on. CONTRIBUTING.md forbids asserting on rendered text, so a prose-only command is ' + + 'permanently unassertable by this harness — the gap is in the product, not the test.', + check(ctx) { + try { + const result = ctx && ctx.result; + if (!result || result.kind !== KIND.PROSE) return { ok: true }; + const argv = Array.isArray(result.argv) ? result.argv : []; + const argvDisplay = argv.length ? argv.join(' ') : '(no argv)'; + return { + ok: false, + severity: SEVERITY.SMELL, + subject: { argv }, + detail: `command "${argvDisplay}" emits KIND.PROSE only; no typed surface exists to assert on`, + }; + } catch (err) { + return { ok: false, severity: SEVERITY.SMELL, detail: `untyped-success threw: ${err && err.message}` }; + } + }, + }), + + Object.freeze({ + id: 'contract-conflict', + describe: + 'SMELL, not a violation: fires only when ctx.jsonErrorMode === true and result.kind === KIND.UNSTRUCTURED_ERROR. ' + + 'Deliberately overlaps json-contract, which reports the same observation as a VIOLATION of the documented ' + + 'contract in docs/json-errors.md ("On any error, exactly one JSON line is written to stderr and the process ' + + 'exits with code 1"). This oracle instead reports that the two controlling documents disagree: src/cli-exit.cts ' + + 'runMain returns for an ExitError before reaching the json-error branch, so CLI usage errors emit plain text by ' + + 'design. A reader of both oracles sees the breach (json-contract) and the reason it is ambiguous (contract-conflict).', + check(ctx) { + try { + const result = ctx && ctx.result; + if (!ctx || ctx.jsonErrorMode !== true) return { ok: true }; + if (!result || result.kind !== KIND.UNSTRUCTURED_ERROR) return { ok: true }; + const argv = Array.isArray(result.argv) ? result.argv : []; + const argvDisplay = argv.length ? argv.join(' ') : '(no argv)'; + return { + ok: false, + severity: SEVERITY.SMELL, + subject: { argv }, + detail: + `command "${argvDisplay}" emitted unstructured-error text under --json-errors, conflicting docs: ` + + 'docs/json-errors.md says "On any error, exactly one JSON line is written to stderr and the process ' + + 'exits with code 1", but src/cli-exit.cts runMain returns for an ExitError before the json-error branch, ' + + 'so CLI usage errors emit plain text by design', + }; + } catch (err) { + return { ok: false, severity: SEVERITY.SMELL, detail: `contract-conflict threw: ${err && err.message}` }; + } + }, + }), +]); + +/** + * Run every oracle against `ctx` and bucket the outcomes by severity. + * + * Each `check` is additionally guarded here (belt-and-suspenders on top of + * each oracle's own try/catch) so a defect in one oracle can never abort the + * walk for the rest. An outcome with no recognized `severity` is treated as + * `SEVERITY.VIOLATION` — the conservative default, so a bug in an oracle can + * never silently downgrade a break into a smell. + * + * The returned object carries a `failed` GETTER that is an alias for + * `violations` ONLY — it deliberately excludes `smells`. This preserves the + * meaning every existing caller already relies on (`failed.length === 0` as + * a build gate): a SMELL must never fail a build. Smells are evidence for a + * human, surfaced separately in `.smells`, never folded into the pass/fail + * verdict. + * + * @param {object} ctx + * @returns {{ + * passed: string[], + * violations: { id: string, detail: string, subject?: object }[], + * smells: { id: string, detail: string, subject?: object }[], + * readonly failed: { id: string, detail: string, subject?: object }[], + * }} + */ +function runOracles(ctx) { + const passed = []; + const violations = []; + const smells = []; + for (const oracle of ORACLES) { + let outcome; + try { + outcome = oracle.check(ctx); + } catch (err) { + outcome = { ok: false, severity: SEVERITY.VIOLATION, detail: `${oracle.id} threw: ${err && err.message}` }; + } + if (outcome && outcome.ok) { + passed.push(oracle.id); + continue; + } + const finding = { id: oracle.id, detail: (outcome && outcome.detail) || 'oracle failed with no detail' }; + if (outcome && outcome.subject !== undefined) { + finding.subject = outcome.subject; + } + if (outcome && outcome.severity === SEVERITY.SMELL) { + smells.push(finding); + } else { + violations.push(finding); + } + } + return { + passed, + violations, + smells, + get failed() { + return violations; + }, + }; +} + +module.exports = { ORACLES, runOracles, SEVERITY }; diff --git a/tests/qa/paths.cjs b/tests/qa/paths.cjs new file mode 100644 index 000000000..be9b28853 --- /dev/null +++ b/tests/qa/paths.cjs @@ -0,0 +1,227 @@ +'use strict'; + +/** + * paths.cjs — the single containment-check source of truth for the QA-walk + * harness. + * + * WHY THIS FILE EXISTS + * ──────────────────── + * Several harness call sites take a scenario-supplied, project-relative path + * (`step.mutate.target`, `step.agent.write` keys, `LoopWalk#writeArtifact`'s + * `relPath`) and join it onto a real temp-project directory before touching + * the filesystem (`fs.writeFileSync` / `fs.unlinkSync` / `fs.symlinkSync`). + * Without a containment check, a scenario (or a helper copied from one) could + * supply `"../../../../etc/hosts"` and reach arbitrary paths outside the temp + * project — this module is the one place that risk is closed. + * + * `resolveForCompare` / `realpathNearestAncestor` / `isUnderProjectDir` were + * originally written in `oracles.cjs` to fix a macOS `/var` vs `/private/var` + * symlink false positive in the `value-hygiene` oracle's absolute-path-leak + * check. They are moved here, unchanged, as the single source of truth — + * `oracles.cjs` now requires them from this module rather than keeping a + * second copy, which would otherwise be exactly the duplicated-containment- + * check divergence class this codebase calls out. + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +/** + * Realpath-resolves `candidate` by walking up to its nearest EXISTING ancestor and rejoining + * the non-existent suffix, rather than requiring the whole path to exist. Plain + * `fs.realpathSync` throws ENOENT for the *entire* path when any segment is missing, which is + * exactly the case for QA payloads — `init` returns `project_path` / `roadmap_path` for files + * the agent has not written yet, nested under a project directory that DOES exist, and + * `resolveWithin` below is deliberately called BEFORE an artifact exists on disk. Falling back + * to the fully-unresolved raw string in that case would compare an unresolved `d` against a + * resolved `projectDir` and reintroduce the same false positive this function exists to fix + * (verified: `.planning/NOT-YET.md` under an mkdtemp'd macOS `/var/...` dir was flagged as + * outside a `/private/var/...`-resolved project root). Resolving the nearest existing ancestor + * and rejoining the missing suffix keeps the comparison correct without requiring the leaf to + * exist. + * + * @param {string} candidate absolute path (may or may not exist on disk) + * @returns {string} realpath-resolved path, with any non-existent suffix rejoined + */ +function realpathNearestAncestor(candidate) { + try { + return fs.realpathSync(candidate); + } catch { + const parent = path.dirname(candidate); + if (parent === candidate) return candidate; // reached a root that itself doesn't exist + return path.join(realpathNearestAncestor(parent), path.basename(candidate)); + } +} + +/** + * Best-effort realpath resolution for containment comparisons: resolves `candidate` via + * `realpathNearestAncestor` and backslash-normalizes the result. + * + * MACOS SYMLINK CASE (do not "simplify" this back to a lexical compare): on macOS, + * `os.tmpdir()` / `TMPDIR` resolves to `/var/folders/...`, which is itself a symlink to + * `/private/var/folders/...`. `fs.mkdtempSync` returns the unresolved `/var/...` form, but + * `gsd-tools` output paths are realpath-resolved by the OS/child process to the + * `/private/var/...` form. Comparing those two strings *lexically* makes the very same + * directory compare as "outside the project" and fires a spurious `value-hygiene` smell on + * every macOS run — and would make a legitimate `resolveWithin` call throw a false containment + * violation for the very same reason. Resolving both sides through `fs.realpathSync` before + * comparison is what fixes that — a lexical `path.relative`/prefix compare on the raw strings + * will regress it. + * + * @param {string} candidate absolute path (may or may not exist on disk) + * @returns {string} realpath-resolved (nearest-ancestor fallback) path, backslash-normalized + */ +function resolveForCompare(candidate) { + // Unconditional: backslash-separated path strings can arrive as *data* (e.g. a + // Windows-style path embedded in JSON) even when running on Linux/macOS, not only when + // `fs.realpathSync` itself returns a drive-letter path on native Windows. + return realpathNearestAncestor(candidate).replace(/\\/g, '/'); +} + +/** + * True when `resolvedCandidate` is `resolvedProjectDir` itself or a path-segment descendant + * of it. Both arguments MUST already be realpath-resolved and backslash-normalized via + * `resolveForCompare` — this function does no I/O itself, so the (syscall-bearing) realpath + * work happens once per candidate/project-dir string, not repeatedly inside this comparator. + * + * Uses `path.posix.relative` structurally (never substring/`.includes()` on the raw string, + * and never a string-prefix test) so a sibling directory like `/tmp/proj-evil` does NOT count + * as inside `/tmp/proj`: a relative path that escapes `resolvedProjectDir` either starts with + * a `..` path segment or is itself still absolute (e.g. a different drive on Windows). + * `path.posix` (not the platform-specific `path`) is used deliberately because both inputs + * were already forward-slash-normalized by `resolveForCompare`, and comparisons must stay + * consistent regardless of the host OS. + * + * @param {string} resolvedCandidate realpath-resolved, backslash-normalized absolute path + * @param {string} resolvedProjectDir realpath-resolved, backslash-normalized absolute project root + * @returns {boolean} + */ +function isUnderProjectDir(resolvedCandidate, resolvedProjectDir) { + const rel = path.posix.relative(resolvedProjectDir, resolvedCandidate); + return rel === '' || (rel.split('/')[0] !== '..' && !path.posix.isAbsolute(rel)); +} + +/** + * True when `relPath` is an absolute path — checked both platform-natively via + * `path.isAbsolute` and, since a path can arrive as *data* with foreign separators + * regardless of host OS (e.g. a Windows-style path embedded in scenario JSON), via + * `path.posix.isAbsolute` on the backslash-normalized form and a drive-letter pattern. + * + * This is the single source of truth for the "looks absolute" predicate — both + * `resolveWithin` (below) and `tests/qa/scenario.cjs`'s load-time validation call this + * rather than each keeping their own copy. + * + * @param {string} relPath + * @returns {boolean} + */ +function isAbsoluteLike(relPath) { + const normRel = relPath.replace(/\\/g, '/'); + return path.isAbsolute(relPath) || path.posix.isAbsolute(normRel) || /^[a-zA-Z]:\//.test(normRel); +} + +/** + * True when `relPath` contains a `..` path segment, checked on its backslash-normalized form + * so a Windows-style separator arriving as data is still caught on a POSIX host. + * + * @param {string} relPath + * @returns {boolean} + */ +function hasTraversalSegment(relPath) { + const normRel = relPath.replace(/\\/g, '/'); + return normRel.split('/').includes('..'); +} + +/** + * Builds a typed containment-violation Error for `resolveWithin` to throw: callers that need + * to distinguish "this relPath escaped its base" from any other Error MUST branch on + * `err.code === 'EPATHESCAPE'` (and may read `err.attemptedPath` / `err.base`) rather than + * matching on `err.message` text — see CONTRIBUTING.md "Prohibited: Raw Text Matching on Test + * Outputs". + * + * @param {string} message human-readable message (unchanged shape from before this typed error existed) + * @param {{attemptedPath: string, base: string}} fields + * @returns {Error & {code: 'EPATHESCAPE', attemptedPath: string, base: string}} + */ +function pathEscapeError(message, { attemptedPath, base }) { + const err = new Error(message); + err.code = 'EPATHESCAPE'; + err.attemptedPath = attemptedPath; + err.base = base; + return err; +} + +/** + * The single containment guard for every scenario-supplied, project-relative path this harness + * turns into a filesystem write/delete/symlink call. Resolves `relPath` against `baseDir` and + * throws unless the result is provably `baseDir` itself or a path-segment descendant of it. + * + * Rejects, before doing any I/O beyond the realpath resolution needed to prove containment: + * - a non-string or empty `relPath` + * - a `relPath` containing a NUL byte (`\0`) — never a valid path-segment character, and a + * known argument-injection primitive against some native path APIs + * - an absolute `relPath` (checked both platform-natively via `path.isAbsolute` and, since + * scenario JSON travels as data and can carry POSIX- or Windows-style separators on either + * host, via `path.posix.isAbsolute` on the backslash-normalized form and a drive-letter + * pattern) — an absolute path is never "project-relative" regardless of where it points + * - any `relPath` (however constructed, including via `..` segments) that resolves outside + * `baseDir` — proven via realpath-resolved, path-segment containment (`isUnderProjectDir`), + * never a lexical/string-prefix compare, so a sibling directory like `-evil` is + * correctly treated as OUTSIDE `baseDir` + * + * The candidate need not exist yet — QA artifacts are routinely written before they exist on + * disk (see `realpathNearestAncestor`) — so containment is proven against the nearest EXISTING + * ancestor with the missing suffix rejoined, never a raw lexical join. + * + * @param {string} baseDir absolute path to the containing directory (e.g. a temp project root). + * @param {string} relPath a project-relative path, as supplied by scenario JSON or harness code. + * @returns {string} the absolute, realpath-resolved path — guaranteed to be `baseDir` or a + * descendant of it. + * @throws {Error} naming both the offending `relPath` and `baseDir` on any violation above. + */ +function resolveWithin(baseDir, relPath) { + if (typeof relPath !== 'string' || relPath === '') { + throw new Error( + `resolveWithin: relPath must be a non-empty string, got ${JSON.stringify(relPath)} (base=${JSON.stringify(baseDir)})`, + ); + } + if (relPath.includes('\0')) { + throw new Error( + `resolveWithin: relPath must not contain a NUL byte, got ${JSON.stringify(relPath)} (base=${JSON.stringify(baseDir)})`, + ); + } + + // Unconditional: a backslash-separated path can arrive as *data* even on a POSIX host (e.g. a + // Windows-style path embedded in scenario JSON) — see `resolveForCompare`'s header comment for + // the same rule applied to comparison output. + const normRel = relPath.replace(/\\/g, '/'); + + if (isAbsoluteLike(relPath)) { + throw pathEscapeError( + `resolveWithin: relPath must be project-relative, got an absolute path ${JSON.stringify(relPath)} (base=${JSON.stringify(baseDir)})`, + { attemptedPath: relPath, base: baseDir }, + ); + } + + const resolvedBase = resolveForCompare(baseDir); + const rawCandidate = path.join(baseDir, normRel); + const resolvedCandidate = resolveForCompare(rawCandidate); + + if (!isUnderProjectDir(resolvedCandidate, resolvedBase)) { + throw pathEscapeError( + `resolveWithin: "${relPath}" escapes base "${baseDir}" ` + + `(resolved candidate "${resolvedCandidate}" is outside resolved base "${resolvedBase}")`, + { attemptedPath: relPath, base: baseDir }, + ); + } + + return resolvedCandidate; +} + +module.exports = { + resolveWithin, + resolveForCompare, + realpathNearestAncestor, + isUnderProjectDir, + isAbsoluteLike, + hasTraversalSegment, +}; diff --git a/tests/qa/report.cjs b/tests/qa/report.cjs new file mode 100644 index 000000000..37c56fdf3 --- /dev/null +++ b/tests/qa/report.cjs @@ -0,0 +1,203 @@ +'use strict'; + +/** + * report.cjs — turns an array of `runScenario` reports (see `scenario.cjs`) + * into a single, serializable `qa-report.json` document. + * + * WHY THIS FILE EXISTS + * ──────────────────── + * `runScenario` reports one walk at a time and is deliberately silent about + * anything cross-scenario (totals, a merged smell index, a human-runnable + * repro line). This module is the aggregation seam: `buildReport` takes the + * raw per-scenario reports plus run metadata and produces one plain object; + * `writeReport` serializes it to disk. Neither function performs a CLI + * invocation, discovers scenario files, or reads the clock — see + * `run-report.cjs` for the executable that wires this to the filesystem and + * to `LoopWalk`/`runOracles`. + * + * DETERMINISM: `buildReport` never calls `Date.now()` / `new Date()` — the + * caller supplies `meta.generatedAt`. A report builder that stamped its own + * wall-clock time would make two builds of the exact same walk compare as + * different documents, which defeats diffing/reviewing a report in CI. + * + * REPRO LINES ARE ALWAYS STRINGS: `step.repro` is either a real, + * copy-pasteable `cd && node gsd-core/bin/gsd-tools.cjs ...` command, + * or a string clearly prefixed `NOT RUNNABLE: ...` explaining why (the tree + * was not preserved, or the step declared no CLI invocation at all). A + * repro line that *looks* runnable but points at a directory that was + * already deleted is worse than no repro line, so the two cases are never + * conflated into one shape that "sometimes has a command". + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +/** Bump when the shape of the emitted report document changes incompatibly. */ +const REPORT_VERSION = 1; + +/** + * Build a single copy-pasteable repro command/explanation for one step. + * + * @param {{preservedDir?: string, argv: string[]}} params + * @returns {string} + */ +function buildRepro({ preservedDir, argv }) { + if (!Array.isArray(argv) || argv.length === 0) { + return 'NOT RUNNABLE: this step declared no CLI invocation (no "run" array) — there is nothing to reproduce.'; + } + if (!preservedDir) { + return 'NOT RUNNABLE: the scenario tree was not preserved for this run — re-run with `--keep` (or `GSD_QA_KEEP=1`) to get a reproducible command.'; + } + return `cd ${preservedDir} && node gsd-core/bin/gsd-tools.cjs --json-errors ${argv.join(' ')}`; +} + +/** + * Merge one scenario's already-computed `smellSummary` (see + * `scenario.cjs`'s `summarizeSmells`) into the running whole-report index. + * + * @param {Map} byId + * @param {Array<{id: string, count: number, examples: string[]}>} smellSummary + */ +function mergeSmellSummary(byId, smellSummary) { + for (const entry of smellSummary || []) { + const existing = byId.get(entry.id) || { count: 0, examples: [] }; + existing.count += entry.count; + if (existing.examples.length < 3) { + existing.examples = existing.examples.concat(entry.examples).slice(0, 3); + } + byId.set(entry.id, existing); + } +} + +/** + * Build the plain, serializable report document from an array of + * `runScenario(...)` reports and run metadata. Never throws away input it + * cannot classify — a scenario report with an unrecognized shape fails loud + * (naming the offending index) rather than silently producing a hollow + * document. + * + * @param {Array<{ + * name: string, + * ok: boolean, + * fixture?: string, + * steps: Array<{at: string, argv: string[], kind: string|null, + * expectFailures: string[], oracleFailures: {id:string,detail:string}[], + * smells: {id:string,detail:string}[], mutation: {id:string,target:string}|null, + * mutationNoop: boolean, mutationObserved: boolean}>, + * smellSummary?: Array<{id:string, count:number, examples:string[]}>, + * preservedDir?: string, + * }>} scenarioReports the array of objects returned by `runScenario` + * (optionally carrying a `fixture` field attached by the caller — see + * `run-report.cjs`, which knows the scenario's `fixture` even though + * `runScenario`'s own return value does not). + * @param {{nodeVersion: string, platform: string, generatedAt: string}} meta + * caller-supplied run metadata. `generatedAt` MUST be supplied by the + * caller (e.g. `new Date().toISOString()`) — this function never reads the + * clock itself, to keep its output deterministic and reviewable. + * @returns {object} the plain report document (see this file's header for + * its top-level shape). + */ +function buildReport(scenarioReports, meta) { + if (!Array.isArray(scenarioReports)) { + throw new Error(`buildReport: scenarioReports must be an array, got ${JSON.stringify(scenarioReports)}`); + } + if (!meta || typeof meta !== 'object') { + throw new Error(`buildReport: meta must be an object, got ${JSON.stringify(meta)}`); + } + for (const key of ['nodeVersion', 'platform', 'generatedAt']) { + if (typeof meta[key] !== 'string' || meta[key] === '') { + throw new Error(`buildReport: meta.${key} must be a non-empty string, got ${JSON.stringify(meta[key])}`); + } + } + + let totalSteps = 0; + let totalViolations = 0; + let mutationsApplied = 0; + let mutationsObserved = 0; + /** @type {Map} */ + const smellById = new Map(); + + const scenarios = scenarioReports.map((sr, index) => { + if (!sr || typeof sr !== 'object' || typeof sr.name !== 'string' || !Array.isArray(sr.steps)) { + throw new Error(`buildReport: scenarioReports[${index}] does not look like a runScenario report, got ${JSON.stringify(sr)}`); + } + + const preservedDir = typeof sr.preservedDir === 'string' ? sr.preservedDir : undefined; + + const steps = sr.steps.map((step) => { + totalSteps += 1; + const violations = step.oracleFailures || []; + const expectFailures = step.expectFailures || []; + totalViolations += violations.length + expectFailures.length; + + if (step.mutation && !step.mutationNoop) mutationsApplied += 1; + if (step.mutationObserved) mutationsObserved += 1; + + return { + at: step.at, + argv: Array.isArray(step.argv) ? step.argv : [], + kind: step.kind, + expectFailures, + violations, + smells: step.smells || [], + mutation: step.mutation || null, + mutationNoop: !!step.mutationNoop, + mutationObserved: !!step.mutationObserved, + repro: buildRepro({ preservedDir, argv: step.argv }), + }; + }); + + mergeSmellSummary(smellById, sr.smellSummary); + + return { + name: sr.name, + ok: !!sr.ok, + fixture: typeof sr.fixture === 'string' ? sr.fixture : null, + steps, + ...(preservedDir ? { preservedDir } : {}), + }; + }); + + const totalSmells = [...smellById.values()].reduce((sum, entry) => sum + entry.count, 0); + const smellSummary = [...smellById.entries()] + .map(([id, entry]) => ({ id, count: entry.count, examples: entry.examples })) + .sort((a, b) => a.id.localeCompare(b.id)); + + return { + reportVersion: REPORT_VERSION, + meta, + totals: { + scenarios: scenarios.length, + steps: totalSteps, + violations: totalViolations, + smells: totalSmells, + mutationsApplied, + mutationsObserved, + }, + scenarios, + smellSummary, + }; +} + +/** + * Serialize `reportObject` to `outPath` as pretty-printed JSON, creating any + * missing parent directories, and return the absolute path written. + * + * @param {object} reportObject + * @param {string} outPath + * @returns {string} the absolute path the report was written to. + */ +function writeReport(reportObject, outPath) { + if (!reportObject || typeof reportObject !== 'object') { + throw new Error(`writeReport: reportObject must be an object, got ${JSON.stringify(reportObject)}`); + } + if (typeof outPath !== 'string' || outPath === '') { + throw new Error(`writeReport: outPath must be a non-empty string, got ${JSON.stringify(outPath)}`); + } + const abs = path.resolve(outPath); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, `${JSON.stringify(reportObject, null, 2)}\n`, 'utf-8'); + return abs; +} + +module.exports = { buildReport, writeReport, REPORT_VERSION }; diff --git a/tests/qa/result.cjs b/tests/qa/result.cjs new file mode 100644 index 000000000..c5bf796ed --- /dev/null +++ b/tests/qa/result.cjs @@ -0,0 +1,210 @@ +'use strict'; + +/** + * result.cjs — the typed `RunResult` IR for the loop QA walk. + * + * WHY THIS FILE EXISTS + * ──────────────────── + * `CONTRIBUTING.md` → "Prohibited: Raw Text Matching on Test Outputs" forbids + * asserting on a child process's stdout/stderr text, and + * `RULESET.TESTS.no-source-grep.tmp-file-traps` forbids reading tmp files the + * SUT wrote. An oracle therefore may never look at raw output. This module is + * the single place where raw bytes are turned into a typed value; everything + * downstream asserts on `kind` and on parsed fields, never on prose. + * + * The classification is deliberately strict (Postel inverted): every observed + * shape gets an explicit `kind`, so oracles need no leniency. Leniency in an + * oracle is how a QA tool reports green on a broken engine. + * + * Every KIND below corresponds to a shape the real CLI actually produces; each + * was observed against `gsd-tools`, not imagined. See + * `.gsd/phase/test-2966-loop-qa-walk/40-design.md` for the behavior table. + */ + +const fs = require('node:fs'); + +/** + * Frozen classification of a single `gsd-tools` invocation. + * + * Tests assert on these constants, never on their string values, so renaming a + * value is a one-line change here rather than a sweep through the suite. + */ +const KIND = Object.freeze({ + /** exit 0, stdout parsed as JSON (object OR scalar OR array). */ + JSON: 'json', + /** exit 0, stdout non-empty but not JSON — an untyped success. NOT an error. Requires non-empty stdout. */ + PROSE: 'prose', + /** exit 0, no stdout payload. NOT a crash. stderr may still carry warnings. */ + EMPTY: 'empty', + /** exit 0, stdout JSON carrying an `error` key — the documented soft-failure idiom. */ + SOFT_ERROR: 'soft-error', + /** exit 1, last stderr line parsed as `{ok:false,reason,message}`. */ + STRUCTURED_ERROR: 'structured-error', + /** exit 1, stderr present but not parseable as the structured envelope. */ + UNSTRUCTURED_ERROR: 'unstructured-error', + /** exit code outside {0,1}. */ + UNEXPECTED_EXIT: 'unexpected-exit', + /** subprocess exceeded its timeout — degraded, never fatal to the walk. */ + TIMEOUT: 'timeout', +}); + +/** + * `io.cjs` `output()` diverts payloads above this many characters to a temp + * file and writes `@file:` to stdout instead. A classifier that does not + * know this reports NOT-JSON for perfectly healthy commands as soon as a + * scenario's payload grows past the threshold. + */ +const FILE_POINTER_PREFIX = '@file:'; + +/** + * Parse text as JSON, returning a sentinel rather than throwing. + * + * @param {string} text + * @returns {{ ok: true, value: unknown } | { ok: false }} + */ +function tryParseJson(text) { + if (typeof text !== 'string' || text.trim() === '') return { ok: false }; + try { + return { ok: true, value: JSON.parse(text) }; + } catch { + return { ok: false }; + } +} + +/** + * Return the last non-empty line of a stream. + * + * Load-bearing: `gsd-tools` writes operator warnings to stderr *before* the + * structured error envelope — e.g. + * `gsd-tools: warning: unknown config key(s) in .planning/config.json: …` + * followed by `{"ok":false,…}`. Parsing the whole stderr blob therefore fails + * on a command that is in fact conforming. During research this exact shape + * produced a false positive against `review-lane`, which is why this function + * exists instead of a bare `JSON.parse(stderr)`. + * + * @param {string} text + * @returns {string} last non-empty line, or '' when there is none + */ +function lastNonEmptyLine(text) { + if (typeof text !== 'string') return ''; + const lines = text.split('\n'); + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i].trim(); + if (line !== '') return line; + } + return ''; +} + +/** + * Resolve stdout that may be an `@file:` pointer into the payload text. + * + * IO failure here is reported, never thrown: a walk that dies because a + * pointee vanished tells you nothing about the engine under test. + * + * @param {string} stdout + * @param {{ readFileSync?: typeof fs.readFileSync }} [io] injection seam for + * fault tests — `chmod 0o000` is a no-op under root and is banned. + * @returns {{ text: string, pointer: string | null, unreadable: boolean }} + */ +function resolveFilePointer(stdout, io = {}) { + const readFileSync = io.readFileSync || fs.readFileSync; + const trimmed = typeof stdout === 'string' ? stdout.trim() : ''; + if (!trimmed.startsWith(FILE_POINTER_PREFIX)) { + return { text: stdout, pointer: null, unreadable: false }; + } + const pointer = trimmed.slice(FILE_POINTER_PREFIX.length); + try { + return { text: readFileSync(pointer, 'utf-8'), pointer, unreadable: false }; + } catch { + return { text: '', pointer, unreadable: true }; + } +} + +/** + * Classify a raw invocation into the typed IR. + * + * Precedence is deliberate and asserted by the test matrix: + * timeout > unexpected exit > exit 1 (error family) > exit 0 (success family) + * An exit-1 run with a healthy-looking stdout payload is still an error — the + * exit code outranks the payload. + * + * CLASSIFICATION KEYS OFF STDOUT, NOT STDERR (exit-0 family): whether a + * successful run is EMPTY, PROSE, JSON, or SOFT_ERROR depends solely on the + * (file-pointer-resolved) stdout payload. stderr on the exit-0 path is never + * used to pick a `kind` — including when stdout is empty and stderr is not — + * it is only ever surfaced through `warnings` (see below). + * + * @param {{ exitCode: number|null, stdout: string, stderr: string, timedOut?: boolean, argv?: string[] }} raw + * @param {{ readFileSync?: typeof fs.readFileSync }} [io] + * @returns {{ + * kind: string, exitCode: number|null, argv: string[], + * json: unknown, err: object|null, pointer: string|null, + * warnings: string[], + * }} + * `warnings` is populated only when the substrate actually supplied stderr + * text — today that means the exit-1 (error) family, since `loop-walk.cjs`'s + * `run()` discards stderr on a clean exit (`execFileSync` swallows it). See + * the JSDoc on `warnings` usage in `tests/qa/loop-walk.cjs` `run()` for the + * full explanation; do not build an oracle assuming success-path stderr is + * observable through that substrate. + */ +function classify(raw, io = {}) { + const argv = Array.isArray(raw.argv) ? raw.argv : []; + const stderr = typeof raw.stderr === 'string' ? raw.stderr : ''; + const base = { exitCode: raw.exitCode, argv, json: null, err: null, pointer: null, warnings: [] }; + + if (raw.timedOut) return { ...base, kind: KIND.TIMEOUT }; + if (raw.exitCode !== 0 && raw.exitCode !== 1) return { ...base, kind: KIND.UNEXPECTED_EXIT }; + + const stderrLines = stderr.split('\n').map((l) => l.trim()).filter((l) => l !== ''); + + if (raw.exitCode === 1) { + // Warnings are every stderr line except the last: the last line is + // consumed below as the candidate structured-error envelope, so it is + // not itself a warning. + const warnings = stderrLines.slice(0, Math.max(0, stderrLines.length - 1)); + const parsed = tryParseJson(lastNonEmptyLine(stderr)); + const envelope = parsed.ok && parsed.value !== null && typeof parsed.value === 'object' + && parsed.value.ok === false && typeof parsed.value.reason === 'string'; + return envelope + ? { ...base, kind: KIND.STRUCTURED_ERROR, err: parsed.value, warnings } + : { ...base, kind: KIND.UNSTRUCTURED_ERROR, warnings }; + } + + // exit 0: no envelope is ever parsed out of stderr, so every stderr line is + // a warning candidate — none of it is consumed the way exit-1's last line is. + const warnings = stderrLines; + + const resolved = resolveFilePointer(raw.stdout, io); + if (resolved.unreadable) { + return { ...base, kind: KIND.UNSTRUCTURED_ERROR, pointer: resolved.pointer, warnings }; + } + const text = typeof resolved.text === 'string' ? resolved.text : ''; + // Classification keys off STDOUT ONLY: a non-empty stderr with empty stdout + // is still EMPTY (see module header + KIND.EMPTY docstring above), and the + // stderr content is preserved in `warnings`, never dropped. + if (text.trim() === '') return { ...base, kind: KIND.EMPTY, warnings }; + + const parsed = tryParseJson(text); + if (!parsed.ok) return { ...base, kind: KIND.PROSE, pointer: resolved.pointer, warnings }; + + // `typeof null === 'object'`, and an array is an object too — both must fall + // through to JSON rather than be probed for an `error` key. + const isPlainObject = parsed.value !== null + && typeof parsed.value === 'object' + && !Array.isArray(parsed.value); + const kind = isPlainObject && Object.prototype.hasOwnProperty.call(parsed.value, 'error') + ? KIND.SOFT_ERROR + : KIND.JSON; + + return { ...base, kind, json: parsed.value, pointer: resolved.pointer, warnings }; +} + +module.exports = { + KIND, + FILE_POINTER_PREFIX, + classify, + lastNonEmptyLine, + resolveFilePointer, + tryParseJson, +}; diff --git a/tests/qa/run-report.cjs b/tests/qa/run-report.cjs new file mode 100644 index 000000000..bc062eb8e --- /dev/null +++ b/tests/qa/run-report.cjs @@ -0,0 +1,144 @@ +#!/usr/bin/env node +'use strict'; + +/** + * run-report.cjs — developer/CI entry point that discovers every QA-walk + * scenario, runs it for real against `gsd-tools`, and writes a single + * `qa-report.json` document (see `report.cjs`). + * + * This is a TOOL, not a test file — it is invoked directly with `node`, + * never through `gsd-test` / `node --test`, and is deliberately NOT named + * `*.test.cjs` so it is never picked up by the test runner's glob. + * + * Usage: + * node tests/qa/run-report.cjs [--out ] [--keep] + * + * `--out ` defaults to `qa-report.json` at the repo root. + * `--keep` (or `GSD_QA_KEEP=1`) preserves every scenario's temp project + * directory instead of deleting it, and threads real repro commands for it + * into the report — see `report.cjs`'s `buildRepro`. + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +const { loadScenario, runScenario } = require('./scenario.cjs'); +const { buildReport, writeReport } = require('./report.cjs'); +const { LoopWalk } = require('./loop-walk.cjs'); +const { runOracles } = require('./oracles.cjs'); +const { getLiveCommandTokens } = require('../helpers/live-command-registry.cjs'); + +/** Absolute path to the repo root (`tests/qa/` -> `tests/` -> repo root). */ +const REPO_ROOT = path.join(__dirname, '..', '..'); + +/** Absolute path to the scenarios directory. */ +const SCENARIOS_DIR = path.join(__dirname, 'scenarios'); + +/** + * Parse `argv` (excluding `node`/script name) into `{ out, keep }`. + * + * @param {string[]} argv + * @returns {{out: string, keep: boolean}} + */ +function parseArgs(argv) { + let out = path.join(REPO_ROOT, 'qa-report.json'); + let keep = false; + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]; + if (arg === '--out') { + const value = argv[i + 1]; + if (typeof value !== 'string' || value === '') { + throw new Error('run-report: --out requires a path argument'); + } + out = path.resolve(value); + i += 1; + } else if (arg === '--keep') { + keep = true; + } else { + throw new Error(`run-report: unrecognized argument "${arg}" (expected --out and/or --keep)`); + } + } + return { out, keep }; +} + +/** + * Every `.json` scenario file under `tests/qa/scenarios/`, EXCLUDING + * underscore-prefixed self-test scenarios (e.g. `_selftest-must-fail.json`), + * which are deliberately broken and must never run as a normal walk — see + * `scenario.cjs`'s `assertWiringIsLive`. + * + * @returns {string[]} absolute file paths, sorted for a deterministic run order. + */ +function discoverScenarioFiles() { + return fs + .readdirSync(SCENARIOS_DIR) + .filter((name) => name.endsWith('.json') && !name.startsWith('_')) + .sort() + .map((name) => path.join(SCENARIOS_DIR, name)); +} + +/** + * Run every discovered scenario for real and return the raw array of + * per-scenario reports (each carrying its own `fixture` field — see + * `runScenario`'s header). This is the shared "drive every scenario" seam: + * `main()` below feeds this straight into `buildReport`, and + * `scripts/qa-smell-ratchet.cjs` reuses this EXACT function rather than + * re-implementing scenario discovery + walking, so the two tools can never + * silently diverge on which scenarios ran or how. + * + * @param {{keep?: boolean, liveCommands?: string[]}} [opts] `keep` (default + * `false`) is forwarded to `runScenario` — see its own header for the + * `GSD_QA_KEEP=1` interaction. `liveCommands` defaults to a fresh call to + * `getLiveCommandTokens()` when omitted. + * @returns {Array & {fixture: string}>} + * @throws {Error} when no scenario files are discovered. + */ +function runAllScenarios(opts) { + const { keep = false, liveCommands = [...getLiveCommandTokens()] } = opts || {}; + + const scenarioFiles = discoverScenarioFiles(); + if (scenarioFiles.length === 0) { + throw new Error(`run-report: no scenario files discovered under "${SCENARIOS_DIR}"`); + } + + return scenarioFiles.map((file) => { + const scenario = loadScenario(file); + const report = runScenario(scenario, { LoopWalk, runOracles, liveCommands, keep }); + // `runScenario`'s own return value carries no `fixture` field — attach it + // here from the (already-validated) scenario so the report document can + // show which starting world each scenario walked. + return { ...report, fixture: scenario.fixture }; + }); +} + +function main() { + const { out, keep } = parseArgs(process.argv.slice(2)); + + const scenarioReports = runAllScenarios({ keep }); + + const meta = { + nodeVersion: process.version, + platform: process.platform, + generatedAt: new Date().toISOString(), + }; + + const reportObject = buildReport(scenarioReports, meta); + const absOut = writeReport(reportObject, out); + + console.log(`qa-report written to ${absOut}`); + console.log( + `scenarios=${reportObject.totals.scenarios} steps=${reportObject.totals.steps} ` + + `violations=${reportObject.totals.violations} smells=${reportObject.totals.smells} ` + + `mutationsApplied=${reportObject.totals.mutationsApplied} mutationsObserved=${reportObject.totals.mutationsObserved}`, + ); +} + +// Guarded so `scripts/qa-smell-ratchet.cjs` (and anything else) can +// `require('./run-report.cjs')` for its exports — `discoverScenarioFiles`, +// `runAllScenarios` — without triggering a second full scenario run and a +// stray `qa-report.json` write as a side effect of loading the module. +if (require.main === module) { + main(); +} + +module.exports = { discoverScenarioFiles, runAllScenarios, parseArgs, REPO_ROOT, SCENARIOS_DIR }; diff --git a/tests/qa/scenario.cjs b/tests/qa/scenario.cjs new file mode 100644 index 000000000..8241362e7 --- /dev/null +++ b/tests/qa/scenario.cjs @@ -0,0 +1,567 @@ +'use strict'; + +/** + * scenario.cjs — the QA-walk scenario DSL interpreter. + * + * WHY THIS FILE EXISTS + * ──────────────────── + * A scenario is a small JSON document describing a sequence of loop-host + * points, artifacts an "agent" would write at each point, and `gsd-tools` + * invocations to run there. `loadScenario` validates the JSON shape so a + * malformed scenario fails fast and loud (never a silent no-op walk); + * `runScenario` drives a `LoopWalk` through the steps, evaluating both + * declared `expect` assertions and the shared `oracles.cjs` checks at every + * step, and NEVER throws on a step failure — a single bad step is recorded + * and the walk continues, so one broken step can never hide the rest of the + * walk's results. + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const { resolveRef } = require('./fixtures/index.cjs'); +const { LOOP_HOST_CONTRACT } = require('../../gsd-core/bin/lib/loop-host-contract.cjs'); +const { MUTATIONS, apply, NOOP } = require('./mutations.cjs'); +const { resolveWithin, isAbsoluteLike, hasTraversalSegment } = require('./paths.cjs'); + +/** + * True when `relPath` is absolute or contains a `..` path segment. Delegates to + * `paths.cjs`'s `isAbsoluteLike` / `hasTraversalSegment` — the single source of truth for + * both predicates — rather than keeping a second copy here. Used at `validateScenario` LOAD + * time, ahead of `resolveWithin`'s own (equally strict) runtime check: failing a malformed + * scenario fast and loud at load time is better than failing at apply time — the scenario is + * malformed, not the run. + * + * @param {string} relPath + * @returns {boolean} + */ +function isTraversalOrAbsolute(relPath) { + if (typeof relPath !== 'string' || relPath === '') return true; + return isAbsoluteLike(relPath) || hasTraversalSegment(relPath); +} + +/** Scenario `fixture` must be one of these — see `loop-walk.cjs` `FIXTURE_BUILDERS`. */ +const VALID_FIXTURES = new Set(['greenfield', 'planning', 'seeded']); + +/** Every known mutation id, derived from `mutations.cjs` (never hardcoded — see that module's `MUTATIONS`). */ +const VALID_MUTATION_IDS = new Set(MUTATIONS.map((m) => m.id)); + +/** `id -> {id, kind, describe}` lookup, derived once from `MUTATIONS`. */ +const MUTATION_BY_ID = new Map(MUTATIONS.map((m) => [m.id, m])); + +/** + * The legal set of `step.at` values, derived from the generated loop-host + * contract rather than hardcoded — see `loop-walk.cjs`'s `LOOP_STEPS` header + * comment for why a hardcoded copy would silently drift from the generator. + * + * @returns {Set} + */ +function getLegalPoints() { + const points = new Set(); + for (const entry of LOOP_HOST_CONTRACT) { + for (const point of entry.points) points.add(point); + } + return points; +} + +/** + * Structural (deep) equality for JSON-ish values. Cycle-tolerant via a + * `WeakMap` pairing "already compared" object references. + * + * @param {unknown} a + * @param {unknown} b + * @param {WeakMap} [seen] + * @returns {boolean} + */ +function deepEqual(a, b, seen = new WeakMap()) { + if (Object.is(a, b)) return true; + if (a === null || b === null) return false; + if (typeof a !== 'object' || typeof b !== 'object') return false; + if (Array.isArray(a) !== Array.isArray(b)) return false; + if (seen.get(a) === b) return true; + seen.set(a, b); + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (aKeys.length !== bKeys.length) return false; + for (const key of aKeys) { + if (!Object.prototype.hasOwnProperty.call(b, key)) return false; + if (!deepEqual(a[key], b[key], seen)) return false; + } + return true; +} + +/** + * Look up a dot-path (e.g. `"a.b.c"` or `"a.b[0].c"`) inside a JSON-ish value. + * + * @param {unknown} value + * @param {string} dotPath + * @returns {{ found: boolean, value: unknown }} + */ +function dotGet(value, dotPath) { + const segments = dotPath + .split('.') + .flatMap((seg) => { + const parts = []; + const re = /^([^[\]]*)((?:\[\d+\])*)$/; + const m = re.exec(seg); + if (!m) return [seg]; + if (m[1] !== '') parts.push(m[1]); + const indices = m[2].match(/\[\d+\]/g) || []; + for (const idx of indices) parts.push(Number(idx.slice(1, -1))); + return parts; + }); + + let current = value; + for (const segment of segments) { + if (current === null || current === undefined) return { found: false, value: undefined }; + if (typeof current !== 'object') return { found: false, value: undefined }; + const key = segment; + if (Array.isArray(current)) { + if (typeof key !== 'number' || key < 0 || key >= current.length) { + return { found: false, value: undefined }; + } + current = current[key]; + continue; + } + if (!Object.prototype.hasOwnProperty.call(current, key)) return { found: false, value: undefined }; + current = current[key]; + } + return { found: true, value: current }; +} + +/** + * Validate a parsed scenario object, throwing on the first violation with a + * message naming the offending field/step. + * + * @param {unknown} scenario + * @param {string} [sourceLabel] e.g. an absolute file path, for error context. + * @returns {object} `scenario`, unmodified, once fully validated. + */ +function validateScenario(scenario, sourceLabel) { + const label = sourceLabel ? ` (from ${sourceLabel})` : ''; + + if (!scenario || typeof scenario !== 'object' || Array.isArray(scenario)) { + throw new Error(`loadScenario: scenario${label} must be a JSON object, got ${JSON.stringify(scenario)}`); + } + if (typeof scenario.name !== 'string' || scenario.name.trim() === '') { + throw new Error(`loadScenario: "name"${label} must be a non-empty string, got ${JSON.stringify(scenario.name)}`); + } + if (!VALID_FIXTURES.has(scenario.fixture)) { + throw new Error( + `loadScenario: "fixture"${label} must be one of ${[...VALID_FIXTURES].join(', ')}, got ${JSON.stringify(scenario.fixture)}`, + ); + } + if (!Array.isArray(scenario.steps) || scenario.steps.length === 0) { + throw new Error(`loadScenario: "steps"${label} must be a non-empty array — an empty scenario is an error, not a silent pass`); + } + if (scenario.selfTest !== undefined && typeof scenario.selfTest !== 'boolean') { + throw new Error(`loadScenario: "selfTest"${label} must be a boolean, got ${JSON.stringify(scenario.selfTest)}`); + } + + const legalPoints = getLegalPoints(); + scenario.steps.forEach((step, index) => { + const where = `steps[${index}]${label}`; + if (!step || typeof step !== 'object' || Array.isArray(step)) { + throw new Error(`loadScenario: ${where} must be an object, got ${JSON.stringify(step)}`); + } + if (typeof step.at !== 'string' || !legalPoints.has(step.at)) { + throw new Error( + `loadScenario: ${where}.at is ${JSON.stringify(step.at)}, which is not a legal loop-host-contract point ` + + `(legal points: ${[...legalPoints].sort().join(', ')})`, + ); + } + if (step.run !== undefined) { + if (!Array.isArray(step.run)) { + throw new Error(`loadScenario: ${where}.run must be an array of argv arrays, got ${JSON.stringify(step.run)}`); + } + step.run.forEach((argv, ri) => { + if (!Array.isArray(argv) || argv.length === 0 || !argv.every((t) => typeof t === 'string')) { + throw new Error(`loadScenario: ${where}.run[${ri}] must be a non-empty array of strings, got ${JSON.stringify(argv)}`); + } + }); + } + if (step.expect !== undefined) { + if (!Array.isArray(step.expect)) { + throw new Error(`loadScenario: ${where}.expect must be an array, got ${JSON.stringify(step.expect)}`); + } + step.expect.forEach((exp, ei) => { + const isValid = exp && typeof exp === 'object' && !Array.isArray(exp) + && typeof exp.path === 'string' && exp.path !== '' + && Object.prototype.hasOwnProperty.call(exp, 'is'); + if (!isValid) { + throw new Error( + `loadScenario: ${where}.expect[${ei}] must be {path: , is: }, got ${JSON.stringify(exp)}`, + ); + } + }); + } + if (step.jsonErrors !== undefined && typeof step.jsonErrors !== 'boolean') { + throw new Error(`loadScenario: ${where}.jsonErrors must be a boolean, got ${JSON.stringify(step.jsonErrors)}`); + } + if (step.mutate !== undefined) { + if (!step.mutate || typeof step.mutate !== 'object' || Array.isArray(step.mutate)) { + throw new Error(`loadScenario: ${where}.mutate must be an object, got ${JSON.stringify(step.mutate)}`); + } + if (typeof step.mutate.id !== 'string' || !VALID_MUTATION_IDS.has(step.mutate.id)) { + throw new Error( + `loadScenario: ${where}.mutate.id is ${JSON.stringify(step.mutate.id)}, which is not a known mutation id ` + + `(valid ids: ${[...VALID_MUTATION_IDS].sort().join(', ')})`, + ); + } + if (typeof step.mutate.target !== 'string' || step.mutate.target === '') { + throw new Error(`loadScenario: ${where}.mutate.target must be a non-empty project-relative path string, got ${JSON.stringify(step.mutate.target)}`); + } + if (isTraversalOrAbsolute(step.mutate.target)) { + throw new Error( + `loadScenario: ${where}.mutate.target ${JSON.stringify(step.mutate.target)} must be project-relative ` + + '— absolute paths and ".." segments are rejected at load time', + ); + } + if (step.mutate.targetBytes !== undefined) { + const { targetBytes } = step.mutate; + if (typeof targetBytes !== 'number' || !Number.isFinite(targetBytes) || targetBytes < 0) { + throw new Error(`loadScenario: ${where}.mutate.targetBytes must be a non-negative finite number, got ${JSON.stringify(targetBytes)}`); + } + } + } + if (step.agent !== undefined) { + if (!step.agent || typeof step.agent !== 'object' || Array.isArray(step.agent)) { + throw new Error(`loadScenario: ${where}.agent must be an object, got ${JSON.stringify(step.agent)}`); + } + if (step.agent.write !== undefined) { + if (!step.agent.write || typeof step.agent.write !== 'object' || Array.isArray(step.agent.write)) { + throw new Error(`loadScenario: ${where}.agent.write must be an object, got ${JSON.stringify(step.agent.write)}`); + } + for (const [relPath, ref] of Object.entries(step.agent.write)) { + if (typeof ref !== 'string' || ref === '') { + throw new Error(`loadScenario: ${where}.agent.write["${relPath}"] must be a non-empty ref string, got ${JSON.stringify(ref)}`); + } + if (isTraversalOrAbsolute(relPath)) { + throw new Error( + `loadScenario: ${where}.agent.write key ${JSON.stringify(relPath)} must be project-relative ` + + '— absolute paths and ".." segments are rejected at load time', + ); + } + } + } + } + }); + + return scenario; +} + +/** + * Load and validate a scenario JSON file. + * + * @param {string} absPathToJson + * @returns {object} the validated scenario object. + * @throws {Error} on missing/unparsable file or any validation violation + * (message names the offending field). + */ +function loadScenario(absPathToJson) { + let text; + try { + text = fs.readFileSync(absPathToJson, 'utf-8'); + } catch (err) { + throw new Error(`loadScenario: cannot read "${absPathToJson}": ${err && err.message}`); + } + let parsed; + try { + parsed = JSON.parse(text); + } catch (err) { + throw new Error(`loadScenario: "${absPathToJson}" is not valid JSON: ${err && err.message}`); + } + return validateScenario(parsed, path.resolve(absPathToJson)); +} + +/** + * Evaluate a step's `expect` array against a `RunResult`. + * + * @param {Array<{path: string, is: unknown}>} expectations + * @param {{ json: unknown }} result + * @returns {string[]} human-readable failure descriptions, empty when all pass. + */ +function evaluateExpectations(expectations, result) { + const failures = []; + for (const exp of expectations || []) { + const { found, value } = dotGet(result ? result.json : undefined, exp.path); + if (!found) { + failures.push(`path "${exp.path}": not found in result.json`); + continue; + } + if (!deepEqual(value, exp.is)) { + failures.push(`path "${exp.path}": expected ${JSON.stringify(exp.is)}, got ${JSON.stringify(value)}`); + } + } + return failures; +} + +/** + * Drive a `LoopWalk` through every step of `scenario`, evaluating `expect` + * assertions and the shared oracle set at each step. Never throws on a step + * failure — failures are recorded on that step's report entry and the walk + * continues, so one bad step never hides the rest. + * + * @param {object} scenario a scenario already validated by `loadScenario`. + * @param {{ + * LoopWalk: { create(opts: object): object }, + * runOracles: (ctx: object) => { passed: string[], violations: {id:string,detail:string}[], smells: {id:string,detail:string}[], failed: {id:string,detail:string}[] }, + * liveCommands?: string[], + * keep?: boolean, + * }} opts `keep` (default `false`) preserves the walk's temp project instead + * of removing it in `finally` — see `LoopWalk#cleanup`. Also honored via + * the `GSD_QA_KEEP=1` environment variable (an `||`, not an override: either + * one being truthy keeps the tree), since a CI operator invoking this + * through a shell cannot pass a JS option. + * @returns {{ + * name: string, + * steps: Array<{ at: string, argv: string[], kind: string|null, expectFailures: string[], oracleFailures: {id:string,detail:string}[], smells: {id:string,detail:string}[], mutation: {id:string,target:string}|null, mutationNoop: boolean, mutationObserved: boolean }>, + * ok: boolean, + * smellSummary: Array<{ id: string, count: number, examples: string[] }>, + * preservedDir?: string, + * }} + */ +function runScenario(scenario, opts) { + const { LoopWalk, runOracles, liveCommands = [], keep = false } = opts || {}; + if (!LoopWalk || typeof LoopWalk.create !== 'function') { + throw new Error('runScenario: opts.LoopWalk (with a create() factory) is required'); + } + if (typeof runOracles !== 'function') { + throw new Error('runScenario: opts.runOracles (function) is required'); + } + const shouldKeep = keep || process.env.GSD_QA_KEEP === '1'; + + const walk = LoopWalk.create({ fixture: scenario.fixture }); + /** @type {object[]} */ + const history = []; + /** @type {Array<{at:string, argv:string[], kind:string|null, expectFailures:string[], oracleFailures:{id:string,detail:string}[], smells:{id:string,detail:string}[]}>} */ + const steps = []; + let preservedDir; + + try { + for (const step of scenario.steps) { + try { + if (step.agent && step.agent.write) { + for (const [relPath, ref] of Object.entries(step.agent.write)) { + walk.writeArtifact(relPath, resolveRef(ref)); + } + } + + // Anti-vacuity for perturbations: BEFORE the mutation is applied, + // run this step's own `run` sequence once against the CLEAN (as-yet + // unmutated) world and keep only the last result's `kind`/`json` — + // never pushed to `history`, never fed to oracles, never counted in + // `statsBefore/After`. This baseline exists solely so that, once the + // mutated run happens below, the two can be compared: a mutation + // that changes nothing observable is indistinguishable from a + // mutation that was never applied, so `mutationObserved` gives that + // distinction a name instead of leaving it implicit in a diff nobody + // looks at. + let cleanBaseline = null; + if (step.mutate) { + const jsonErrorModeForBaseline = step.jsonErrors !== false; + const baselineOptions = { jsonErrors: jsonErrorModeForBaseline }; + const baselineRuns = Array.isArray(step.run) ? step.run : []; + let baselineResult = null; + for (const argv of baselineRuns) { + baselineResult = walk.run(...argv, baselineOptions); + } + if (baselineResult) { + cleanBaseline = { kind: baselineResult.kind, json: baselineResult.json }; + } + } + + // Mutation is applied AFTER `agent.write` and BEFORE `run` — a step + // can write a valid artifact and then corrupt it, so `run` observes + // the corrupted world exactly as a real engine invocation would. + let mutationRecord = null; + let mutationNoop = false; + if (step.mutate) { + const { id, target, targetBytes } = step.mutate; + const entry = MUTATION_BY_ID.get(id); + const absTarget = resolveWithin(walk.dir, target); + if (entry.kind === 'content') { + // This `readFileSync` is harness plumbing, not a test assertion — + // it reads a planning ARTIFACT the walk itself just wrote so the + // mutation catalog's pure string transforms have input to work + // on. It is never string-matched/asserted against; the mutated + // bytes are written straight back to disk for `run` to react to. + // Do NOT "fix" this into a stat-only check — see `mutations.cjs` + // and this file's header for why oracles must never read SUT + // output, which does not apply to this harness-owned write path. + const before = fs.readFileSync(absTarget, 'utf-8'); + const mutateOpts = targetBytes !== undefined ? { targetBytes } : undefined; + const after = apply(id, before, mutateOpts); + if (after === NOOP) { + mutationNoop = true; + } else { + fs.writeFileSync(absTarget, after, 'utf-8'); + } + } else { + apply(id, { dir: walk.dir, relPath: target }); + } + mutationRecord = { id, target }; + } + + const readOnly = !!step.readOnly; + const runs = Array.isArray(step.run) ? step.run : []; + // `step.jsonErrors === false` opts a step into the human-invocation + // path (`gsd_run ` without `--json-errors`); any other value + // (including undefined) keeps `LoopWalk#run`'s own default of true. + const jsonErrorMode = step.jsonErrors !== false; + const runOptions = { jsonErrors: jsonErrorMode }; + + const statsBefore = walk.statSnapshot(); + let result = null; + let lastArgv = []; + for (const argv of runs) { + result = walk.run(...argv, runOptions); + lastArgv = argv; + } + const statsAfter = walk.statSnapshot(); + + let repeatResult = null; + if (readOnly && lastArgv.length > 0) { + repeatResult = walk.run(...lastArgv, runOptions); + } + + const expectFailures = evaluateExpectations(step.expect, result); + const { violations: oracleFailures, smells } = runOracles({ + result, + repeatResult, + statsBefore, + statsAfter, + history, + liveCommands, + readOnly, + projectDir: walk.dir, + jsonErrorMode, + }); + + if (result) history.push(result); + + // `mutationObserved` is only meaningful for a mutated step: it is + // `true` when the mutated run's result differs from the clean + // baseline captured above, by `kind` OR by deep-inequality of + // `json` — a `kind`-only comparison would miss a mutation that + // keeps `kind: "json"` but silently changes the payload (e.g. + // `found: true` -> `found: false`), which is exactly the class of + // corruption a perturbation scenario exists to catch. + const mutationObserved = step.mutate && cleanBaseline + ? (cleanBaseline.kind !== (result ? result.kind : null) || !deepEqual(cleanBaseline.json, result ? result.json : null)) + : false; + + steps.push({ + at: step.at, + argv: lastArgv, + kind: result ? result.kind : null, + expectFailures, + oracleFailures, + smells, + mutation: mutationRecord, + mutationNoop, + mutationObserved, + }); + } catch (err) { + steps.push({ + at: step && step.at, + argv: [], + kind: null, + expectFailures: [], + oracleFailures: [{ id: 'step-exception', detail: `${err && err.message}` }], + smells: [], + mutation: null, + mutationNoop: false, + mutationObserved: false, + }); + } + } + } finally { + preservedDir = walk.cleanup({ keep: shouldKeep }); + } + + // A SMELL is evidence, never a build break: `ok` is derived from + // `expectFailures` + `oracleFailures` (violations) ONLY. A step whose sole + // findings are smells still counts as `ok`. + const ok = steps.every((s) => s.expectFailures.length === 0 && s.oracleFailures.length === 0); + const smellSummary = summarizeSmells(steps); + return { + name: scenario.name, + steps, + ok, + smellSummary, + ...(preservedDir ? { preservedDir } : {}), + }; +} + +/** + * Group every step's `smells` by oracle `id` across the whole walk, so a + * reader sees "this oracle fired N times, here are up to 3 examples" rather + * than a flat wall of per-step repeats. + * + * @param {Array<{smells: {id:string, detail:string}[]}>} steps + * @returns {Array<{id: string, count: number, examples: string[]}>} + */ +function summarizeSmells(steps) { + /** @type {Map} */ + const byId = new Map(); + for (const step of steps) { + for (const smell of step.smells || []) { + const list = byId.get(smell.id) || []; + list.push(smell.detail); + byId.set(smell.id, list); + } + } + return [...byId.entries()].map(([id, details]) => ({ + id, + count: details.length, + examples: details.slice(0, 3), + })); +} + +/** Absolute path to the wiring self-test scenario — see `assertWiringIsLive`. */ +const SELF_TEST_SCENARIO_PATH = path.join(__dirname, 'scenarios', '_selftest-must-fail.json'); + +/** + * The REAL wiring detector for this harness's assertion machinery. + * + * `totalSmells > 0` on a happy-path walk proves almost nothing: it leans on + * well-known engine behaviors that fire regardless of whether `expect` / + * oracle plumbing actually works. This function instead loads the + * deliberately-broken `scenarios/_selftest-must-fail.json` scenario — whose + * `expect` block asserts something KNOWN FALSE about a real command — and + * runs it for real. If the assertion machinery is wired correctly, the run + * MUST fail (`ok === false`, non-empty `expectFailures`); if it silently + * passes, `expect` is not actually being evaluated and this function throws + * naming that fact, rather than letting a broken harness report a clean bill + * of health. + * + * @param {{ LoopWalk: object, runOracles: Function, liveCommands?: string[] }} opts + * @returns {ReturnType} the self-test's own report, for + * callers that want to inspect or log it. + * @throws {Error} when the self-test scenario is missing `"selfTest": true`, + * or when it does NOT fail — either case means the expect/oracle assertion + * machinery is not provably wired. + */ +function assertWiringIsLive(opts) { + const scenario = loadScenario(SELF_TEST_SCENARIO_PATH); + if (scenario.selfTest !== true) { + throw new Error(`assertWiringIsLive: "${SELF_TEST_SCENARIO_PATH}" is missing "selfTest": true`); + } + const report = runScenario(scenario, opts); + if (report.ok !== false) { + throw new Error( + 'assertWiringIsLive: the self-test scenario (a KNOWN-FALSE expectation) did not fail — ' + + 'the expect/oracle assertion machinery is not wired', + ); + } + const hasExpectFailure = report.steps.some((s) => s.expectFailures.length > 0); + if (!hasExpectFailure) { + throw new Error( + 'assertWiringIsLive: the self-test scenario failed via oracleFailures but recorded no expectFailures — ' + + 'the "expect" assertion machinery specifically is not provably wired', + ); + } + return report; +} + +module.exports = { loadScenario, runScenario, deepEqual, dotGet, assertWiringIsLive }; diff --git a/tests/qa/scenarios/_selftest-must-fail.json b/tests/qa/scenarios/_selftest-must-fail.json new file mode 100644 index 000000000..fc6ae062d --- /dev/null +++ b/tests/qa/scenarios/_selftest-must-fail.json @@ -0,0 +1,15 @@ +{ + "name": "_selftest-must-fail", + "fixture": "greenfield", + "selfTest": true, + "steps": [ + { + "at": "discuss:pre", + "run": [["init", "new-project"]], + "readOnly": true, + "expect": [ + { "path": "project_exists", "is": true } + ] + } + ] +} diff --git a/tests/qa/scenarios/abandoned-mid-execute-resume.json b/tests/qa/scenarios/abandoned-mid-execute-resume.json new file mode 100644 index 000000000..84f2f2b47 --- /dev/null +++ b/tests/qa/scenarios/abandoned-mid-execute-resume.json @@ -0,0 +1,44 @@ +{ + "name": "abandoned-mid-execute-resume", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer" + } + }, + "run": [["progress"]] + }, + { + "at": "execute:wave:pre", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-SUMMARY.md": "@summary/tokenizer-partial", + ".planning/STATE.md": "@state/mid-execute", + ".planning/current-agent-id.txt": "@state/interrupted-agent-id" + } + }, + "run": [["progress"]] + }, + { + "at": "execute:post", + "run": [["init", "resume"]], + "expect": [ + { "path": "state_exists", "is": true }, + { "path": "has_interrupted_agent", "is": true } + ] + } + ] +} diff --git a/tests/qa/scenarios/brownfield-with-map.json b/tests/qa/scenarios/brownfield-with-map.json new file mode 100644 index 000000000..9966240d4 --- /dev/null +++ b/tests/qa/scenarios/brownfield-with-map.json @@ -0,0 +1,52 @@ +{ + "name": "brownfield-with-map", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:pre", + "run": [["init", "new-project"]], + "readOnly": true, + "expect": [ + { "path": "is_brownfield", "is": false }, + { "path": "has_codebase_map", "is": false } + ] + }, + { + "at": "discuss:pre", + "agent": { + "write": { + "src/index.js": "@code/sample-source" + } + }, + "run": [["init", "new-project"]], + "readOnly": true, + "expect": [ + { "path": "is_brownfield", "is": true }, + { "path": "has_codebase_map", "is": false }, + { "path": "needs_codebase_map", "is": true } + ] + }, + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/codebase/STACK.md": "@codebase/STACK", + ".planning/codebase/ARCHITECTURE.md": "@codebase/ARCHITECTURE", + ".planning/codebase/STRUCTURE.md": "@codebase/STRUCTURE", + ".planning/codebase/CONVENTIONS.md": "@codebase/CONVENTIONS", + ".planning/codebase/TESTING.md": "@codebase/TESTING", + ".planning/codebase/INTEGRATIONS.md": "@codebase/INTEGRATIONS", + ".planning/codebase/CONCERNS.md": "@codebase/CONCERNS", + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]], + "expect": [ + { "path": "is_brownfield", "is": true }, + { "path": "has_codebase_map", "is": true }, + { "path": "needs_codebase_map", "is": false } + ] + } + ] +} diff --git a/tests/qa/scenarios/greenfield-happy-path.json b/tests/qa/scenarios/greenfield-happy-path.json new file mode 100644 index 000000000..4e49c6a12 --- /dev/null +++ b/tests/qa/scenarios/greenfield-happy-path.json @@ -0,0 +1,47 @@ +{ + "name": "greenfield-happy-path", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:pre", + "run": [["init", "new-project"]], + "readOnly": true, + "expect": [ + { "path": "project_exists", "is": false }, + { "path": "planning_exists", "is": false } + ] + }, + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]], + "expect": [ + { "path": "project_exists", "is": true } + ] + }, + { + "at": "plan:pre", + "run": [["progress"]], + "readOnly": true + }, + { + "at": "plan:post", + "run": [["smart-entry"]] + }, + { + "at": "execute:pre", + "run": [["progress"]], + "readOnly": true + }, + { + "at": "verify:pre", + "run": [["state-snapshot"]], + "readOnly": true + } + ] +} diff --git a/tests/qa/scenarios/milestone-rollover.json b/tests/qa/scenarios/milestone-rollover.json new file mode 100644 index 000000000..7b9f2e561 --- /dev/null +++ b/tests/qa/scenarios/milestone-rollover.json @@ -0,0 +1,90 @@ +{ + "name": "milestone-rollover", + "fixture": "greenfield", + "notes": "The point of this scenario is the boundary crossing itself: complete milestone 1.0, confirm the archival side-effects (phases clear returns cleared:0, phase numbering carries forward across the boundary rather than resetting), THEN write the next milestone's ROADMAP.md (v2.0) and observe monotonic-progress's oracle correctly demote the resulting counter reset to a SMELL (scope changed) rather than a VIOLATION. Do not trim/reorder these steps to dodge the oracle — the boundary IS the scenario.", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer" + } + }, + "run": [["progress"]] + }, + { + "at": "execute:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-SUMMARY.md": "@summary/tokenizer-complete" + } + }, + "run": [["progress"]], + "expect": [ + { "path": "percent", "is": 100 } + ] + }, + { + "at": "execute:post", + "run": [["stats"]] + }, + { + "at": "ship:post", + "run": [ + ["milestone", "complete", "1.0", "--force"] + ] + }, + { + "at": "ship:post", + "run": [ + ["phases", "clear", "--confirm"] + ], + "expect": [ + { "path": "cleared", "is": 0 } + ] + }, + { + "at": "ship:post", + "run": [ + ["phase", "add", "Phase Four: Post-Rollover Work"] + ], + "expect": [ + { "path": "phase_number", "is": 4 } + ] + }, + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/ROADMAP.md": "@roadmap/post-rollover-v2" + } + }, + "run": [["progress"], ["stats"]], + "expect": [ + { "path": "milestone_version", "is": "v2.0" } + ] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/04-phase-four-post-rollover-work/04-01-PLAN.md": "@plan/post-rollover" + } + }, + "run": [["progress"]], + "expect": [ + { "path": "total_plans", "is": 1 } + ] + } + ] +} diff --git a/tests/qa/scenarios/multi-phase-dependencies.json b/tests/qa/scenarios/multi-phase-dependencies.json new file mode 100644 index 000000000..6f3541550 --- /dev/null +++ b/tests/qa/scenarios/multi-phase-dependencies.json @@ -0,0 +1,43 @@ +{ + "name": "multi-phase-dependencies", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer" + } + }, + "run": [ + ["roadmap", "annotate-dependencies", "1"], + ["roadmap", "get-phase", "1"] + ], + "expect": [ + { "path": "phase_number", "is": "1" } + ] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/02-printable-output/02-01-PLAN.md": "@plan/renderer" + } + }, + "run": [ + ["roadmap", "annotate-dependencies", "2"], + ["roadmap", "analyze"] + ] + } + ] +} diff --git a/tests/qa/scenarios/multi-workstream.json b/tests/qa/scenarios/multi-workstream.json new file mode 100644 index 000000000..205f95d7e --- /dev/null +++ b/tests/qa/scenarios/multi-workstream.json @@ -0,0 +1,72 @@ +{ + "name": "multi-workstream", + "fixture": "greenfield", + "notes": "The point of this scenario is workstream isolation: create two workstreams, populate only one, and prove progress stays isolated per-`--ws` in both directions (populated vs empty, and switching back and forth). No payload field names the active workstream, so monotonic-progress's oracle derives scope from the invocation's own `--ws ` argv token — a workstream switch is a scope change, so an apparent counter reset when switching to an untouched workstream is a SMELL, never a VIOLATION. Do not trim/reorder these steps to dodge the oracle — the isolation crossing IS the scenario.", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "plan:pre", + "run": [ + ["workstream", "create", "alpha"], + ["workstream", "create", "beta"] + ] + }, + { + "at": "plan:post", + "run": [["--ws", "beta", "progress"]], + "expect": [ + { "path": "total_plans", "is": 0 } + ] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/workstreams/alpha/ROADMAP.md": "@roadmap/three-phase", + ".planning/workstreams/alpha/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer" + } + }, + "run": [["--ws", "alpha", "progress"]], + "expect": [ + { "path": "total_plans", "is": 1 } + ] + }, + { + "at": "plan:post", + "run": [["--ws", "beta", "progress"]], + "expect": [ + { "path": "total_plans", "is": 0 } + ] + }, + { + "at": "execute:post", + "agent": { + "write": { + ".planning/workstreams/alpha/phases/01-parser/01-01-SUMMARY.md": "@summary/tokenizer-complete" + } + }, + "run": [["--ws", "alpha", "progress"]], + "expect": [ + { "path": "total_plans", "is": 1 }, + { "path": "total_summaries", "is": 1 }, + { "path": "percent", "is": 100 } + ] + }, + { + "at": "execute:post", + "run": [["progress"]], + "expect": [ + { "path": "total_plans", "is": 0 } + ] + } + ] +} diff --git a/tests/qa/scenarios/out-of-order.json b/tests/qa/scenarios/out-of-order.json new file mode 100644 index 000000000..07dcd9fd5 --- /dev/null +++ b/tests/qa/scenarios/out-of-order.json @@ -0,0 +1,36 @@ +{ + "name": "out-of-order", + "fixture": "greenfield", + "steps": [ + { + "at": "verify:pre", + "run": [["state-snapshot"]], + "readOnly": true + }, + { + "at": "ship:pre", + "run": [["phase", "uat-passed", "1"]], + "readOnly": true + }, + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "verify:pre", + "run": [["phase", "uat-passed", "1"]], + "readOnly": true + }, + { + "at": "ship:pre", + "run": [["roadmap", "get-phase", "1"]], + "readOnly": true + } + ] +} diff --git a/tests/qa/scenarios/perturbation-bom.json b/tests/qa/scenarios/perturbation-bom.json new file mode 100644 index 000000000..c3b0c584d --- /dev/null +++ b/tests/qa/scenarios/perturbation-bom.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-bom", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "bom", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-crlf.json b/tests/qa/scenarios/perturbation-crlf.json new file mode 100644 index 000000000..d9fbafa82 --- /dev/null +++ b/tests/qa/scenarios/perturbation-crlf.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-crlf", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "crlf", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-delete-artifact.json b/tests/qa/scenarios/perturbation-delete-artifact.json new file mode 100644 index 000000000..a6edea15b --- /dev/null +++ b/tests/qa/scenarios/perturbation-delete-artifact.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-delete-artifact", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "delete", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-duplicate-phase-id.json b/tests/qa/scenarios/perturbation-duplicate-phase-id.json new file mode 100644 index 000000000..05a7bc2c0 --- /dev/null +++ b/tests/qa/scenarios/perturbation-duplicate-phase-id.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-duplicate-phase-id", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "duplicate-phase-id", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-empty.json b/tests/qa/scenarios/perturbation-empty.json new file mode 100644 index 000000000..6fc442350 --- /dev/null +++ b/tests/qa/scenarios/perturbation-empty.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-empty", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "empty", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-escaped-pipes.json b/tests/qa/scenarios/perturbation-escaped-pipes.json new file mode 100644 index 000000000..291ed27ba --- /dev/null +++ b/tests/qa/scenarios/perturbation-escaped-pipes.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-escaped-pipes", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "escaped-pipes", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-nonsequential-phases.json b/tests/qa/scenarios/perturbation-nonsequential-phases.json new file mode 100644 index 000000000..0444912ce --- /dev/null +++ b/tests/qa/scenarios/perturbation-nonsequential-phases.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-nonsequential-phases", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "nonsequential-phases", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "analyze"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-oversized.json b/tests/qa/scenarios/perturbation-oversized.json new file mode 100644 index 000000000..9c75a2bb9 --- /dev/null +++ b/tests/qa/scenarios/perturbation-oversized.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-oversized", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "oversized", "target": ".planning/ROADMAP.md", "targetBytes": 2500 }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-symlink.json b/tests/qa/scenarios/perturbation-symlink.json new file mode 100644 index 000000000..ce3800556 --- /dev/null +++ b/tests/qa/scenarios/perturbation-symlink.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-symlink", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "symlink", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-truncated-frontmatter.json b/tests/qa/scenarios/perturbation-truncated-frontmatter.json new file mode 100644 index 000000000..ed379d7d3 --- /dev/null +++ b/tests/qa/scenarios/perturbation-truncated-frontmatter.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-truncated-frontmatter", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "truncate-frontmatter", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/perturbation-unicode-headings.json b/tests/qa/scenarios/perturbation-unicode-headings.json new file mode 100644 index 000000000..94557e912 --- /dev/null +++ b/tests/qa/scenarios/perturbation-unicode-headings.json @@ -0,0 +1,21 @@ +{ + "name": "perturbation-unicode-headings", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["progress"]] + }, + { + "at": "plan:pre", + "mutate": { "id": "unicode-headings", "target": ".planning/ROADMAP.md" }, + "run": [["roadmap", "get-phase", "1"]] + } + ] +} diff --git a/tests/qa/scenarios/plan-drift.json b/tests/qa/scenarios/plan-drift.json new file mode 100644 index 000000000..f3784caaa --- /dev/null +++ b/tests/qa/scenarios/plan-drift.json @@ -0,0 +1,56 @@ +{ + "name": "plan-drift", + "fixture": "greenfield", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer" + } + }, + "run": [["progress"]] + }, + { + "at": "execute:pre", + "run": [["progress"]], + "readOnly": true + }, + { + "at": "execute:wave:pre", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer-drifted" + } + }, + "run": [ + ["progress"], + ["roadmap", "get-phase", "1"] + ] + }, + { + "at": "execute:wave:post", + "run": [["drift-guard", "severity", "--status", "VERIFIED"]], + "readOnly": true, + "expect": [ + { "path": "severity", "is": "none" }, + { "path": "hardBlock", "is": false } + ] + }, + { + "at": "execute:post", + "run": [["drift-guard", "severity", "--status", "MISSING"]], + "readOnly": true + } + ] +} diff --git a/tests/qa/scenarios/uat-fail-then-remediate.json b/tests/qa/scenarios/uat-fail-then-remediate.json new file mode 100644 index 000000000..7c8bba90c --- /dev/null +++ b/tests/qa/scenarios/uat-fail-then-remediate.json @@ -0,0 +1,73 @@ +{ + "name": "uat-fail-then-remediate", + "fixture": "greenfield", + "notes": "FIXED: the prior FINDING here (dated before the fixture fix) was itself a fixture defect, not an engine defect — @uat/current-test-failing and @uat/current-test-passing carried NO `### N. Name` / `result:` block that `parseUatResultItems` (src/uat-predicate.cts) recognizes, so both fixtures always produced zero parsed `checks` and the same `no_uat_artifacts:true, passed:false` payload regardless of remediation. Both fixtures now carry a real `### 1. ...` / `result: passed|failed` block, and the `expect` blocks below assert the real before/after divergence: `passed:false, no_uat_artifacts:true` while failing, `passed:true, no_uat_artifacts:false` after remediation.", + "steps": [ + { + "at": "discuss:post", + "agent": { + "write": { + ".planning/PROJECT.md": "@project/minimal", + ".planning/ROADMAP.md": "@roadmap/three-phase" + } + }, + "run": [["init", "new-project"]] + }, + { + "at": "plan:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-PLAN.md": "@plan/tokenizer" + } + }, + "run": [["progress"]] + }, + { + "at": "execute:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-01-SUMMARY.md": "@summary/tokenizer-complete" + } + }, + "run": [["progress"]] + }, + { + "at": "verify:pre", + "agent": { + "write": { + ".planning/phases/01-parser/01-UAT.md": "@uat/current-test-failing" + } + }, + "run": [ + ["uat", "render-checkpoint", "--file", ".planning/phases/01-parser/01-UAT.md"], + ["phase", "uat-passed", "1"] + ], + "expect": [ + { "path": "passed", "is": false }, + { "path": "no_uat_artifacts", "is": false } + ] + }, + { + "at": "plan:post", + "run": [["progress"]] + }, + { + "at": "execute:post", + "run": [["progress"]] + }, + { + "at": "verify:post", + "agent": { + "write": { + ".planning/phases/01-parser/01-UAT.md": "@uat/current-test-passing" + } + }, + "run": [["phase", "uat-passed", "1"]], + "expect": [ + { "path": "blockers", "is": [] }, + { "path": "passed", "is": true }, + { "path": "no_uat_artifacts", "is": false } + ] + } + ] +} diff --git a/tests/qa/smell-acks/README.md b/tests/qa/smell-acks/README.md new file mode 100644 index 000000000..43abada0f --- /dev/null +++ b/tests/qa/smell-acks/README.md @@ -0,0 +1,90 @@ +# tests/qa/smell-acks/ + +Per-PR acknowledgment fragments for `scripts/qa-smell-ratchet.cjs` (#2966). + +## The two terminal states (read this before adding a fragment) + +Every smell the ratchet reports must end in exactly ONE of two states — there +is no third "accepted with a good explanation" state: + +1. **REAL** — an assigned defect. File it, then add a fragment here citing + that issue's number. +2. **FALSE POSITIVE** — the oracle (`tests/qa/oracles.cjs`) is wrong. Fix the + oracle so it stops firing. Do NOT add a fragment for it. + +A free-text `reason` can NEVER substitute for a real `issue` — it is at most +an optional human note alongside a real `issue`, never a replacement for one. + +## Why fragments, not one shared file + +Same reason `.changeset/` and `tests/emitted-drift-acks/` use fragments +instead of one shared mutable document: a single `tests/qa/smell-baseline.json` +that every acknowledging PR has to rewrite guarantees a merge conflict +between any two such PRs in flight at once. A fragment per PR — uniquely +named so concurrent PRs never touch the same file — means two PRs can never +conflict on this seam. + +## Shape + +One fragment = one acknowledged smell finding: + +```json +{ + "version": 1, + "key": "", + "id": "", + "scenario": "", + "issue": 2966, + "reason": "" +} +``` + +`issue` is REQUIRED and must be a positive integer naming the tracking issue +for the underlying defect (a REAL smell) — it is not a PR number and it is +not satisfied by prose. `reason` is OPTIONAL; when present it must be a +non-empty string that is not the literal `TODO(qa-smell-ratchet):`-prefixed +placeholder `--update` writes for a brand-new (untriaged) entry. + +A missing, zero, non-integer, or non-numeric `issue` — or a `reason` still +carrying that placeholder text — is rejected by a plain (non-`--update`) +ratchet run, with a message naming exactly which field is wrong. + +## Naming + +Name the file so nobody else can collide with it: include the tracking issue +number and something identifying the smell, e.g.: + +``` +tests/qa/smell-acks/2979-untyped-success-smart-entry.json +``` + +If one PR needs to acknowledge more than one NEW smell, add one fragment +file per smell — do not bundle several findings into one fragment (that +would defeat the "uniquely named, never conflicting" property for a PR that +adds a second smell to an existing fragment someone else is also touching). + +## Lifecycle + +- The ratchet's failure output for a NEW smell prints a paste-ready skeleton + for exactly this shape — copy it, fill in the real `issue` number, done. + There is no "write a reason instead" option: the two legitimate responses to + a NEW smell are fixing the detector (false positive) or filing a defect and + citing its issue number (real) here. +- A fragment is honored by `scripts/qa-smell-ratchet.cjs` for as long as it + exists here, in addition to whatever is already in the committed + `tests/qa/smell-baseline.json`. +- When a maintainer runs `node scripts/qa-smell-ratchet.cjs --update`, every + currently-firing smell (including ones only acknowledged via a fragment + here) is folded into the regenerated `tests/qa/smell-baseline.json`, + carrying over each fragment's own `issue` (and `reason`, if any). `--update` + never invents an issue number: a genuinely new, never-acknowledged smell is + written with `issue: null` and a TODO `reason`, and the very next plain + (non-`--update`) run REJECTS that entry — forcing a human to triage it + before it can ship. **Delete the fragment once its entry is folded into the + baseline** — a fragment left behind after that point is redundant (the + ratchet will say so, non-fatally, pointing at the exact file) and should be + removed in the same PR that runs `--update`. +- If the underlying behavior is fixed instead of accepted, delete the + fragment (or, if it was already folded, let `--update` prune it from the + baseline as a STALE entry) rather than leaving a dead acknowledgment + behind. diff --git a/tests/qa/smell-baseline.json b/tests/qa/smell-baseline.json new file mode 100644 index 000000000..e8cb8693b --- /dev/null +++ b/tests/qa/smell-baseline.json @@ -0,0 +1,40 @@ +{ + "version": 1, + "smells": [ + { + "key": "0cad2954807d::soft-error-exit-zero|greenfield-happy-path|state-snapshot|(subject-with-no-stable-discriminator)", + "id": "soft-error-exit-zero", + "scenario": "greenfield-happy-path", + "issue": 2980, + "reason": "state-snapshot uses the documented soft-failure idiom (output({error:...}) while exiting 0) when STATE.md does not exist yet on a greenfield project. 42 call sites across the engine use this idiom; oracles.cjs rates changing it CRITICAL blast radius — tracked as a known trade-off in #2980." + }, + { + "key": "373270350622::soft-error-exit-zero|perturbation-delete-artifact|roadmap get-phase 1|(subject-with-no-stable-discriminator)", + "id": "soft-error-exit-zero", + "scenario": "perturbation-delete-artifact", + "issue": 2980, + "reason": "`roadmap get-phase 1` reports ROADMAP.md not found via the same output({error:...})-exit-0 soft-failure idiom after the perturbation deletes the artifact — same known trade-off as the state-snapshot occurrences above, tracked in #2980." + }, + { + "key": "383aca0b3b13::soft-error-exit-zero|perturbation-unicode-headings|roadmap get-phase 1|(subject-with-no-stable-discriminator)", + "id": "soft-error-exit-zero", + "scenario": "perturbation-unicode-headings", + "issue": 2980, + "reason": "Same roadmap get-phase soft-failure idiom as perturbation-delete-artifact, fired here because the unicode-heading perturbation leaves the phase unresolvable — tracked in #2980." + }, + { + "key": "8ff1a5363a3f::untyped-success|greenfield-happy-path|smart-entry|(subject-with-no-stable-discriminator)", + "id": "untyped-success", + "scenario": "greenfield-happy-path", + "issue": 2979, + "reason": "smart-entry emits KIND.PROSE unconditionally, with no typed JSON surface for the harness to assert on — a real product gap tracked in #2979, not a QA-harness defect." + }, + { + "key": "92b6b4718571::soft-error-exit-zero|out-of-order|state-snapshot|(subject-with-no-stable-discriminator)", + "id": "soft-error-exit-zero", + "scenario": "out-of-order", + "issue": 2980, + "reason": "Same state-snapshot soft-failure idiom as greenfield-happy-path, fired here because the out-of-order walk reaches state-snapshot before STATE.md exists — tracked in #2980." + } + ] +} diff --git a/tests/qa/smell-fingerprint.cjs b/tests/qa/smell-fingerprint.cjs new file mode 100644 index 000000000..60ce0ce82 --- /dev/null +++ b/tests/qa/smell-fingerprint.cjs @@ -0,0 +1,196 @@ +'use strict'; + +/** + * smell-fingerprint.cjs — a STABLE identity for one QA-walk "smell" finding + * (see `oracles.cjs`'s `SEVERITY.SMELL`), across runs, temp dirs, and hosts. + * + * WHY THIS FILE EXISTS + * ──────────────────── + * `scripts/qa-smell-ratchet.cjs` needs to answer "is this the SAME smell we + * already decided about, or a NEW one?" across two runs that never share a + * filesystem state — every run gets its own `mkdtemp`'d project directory + * (see `loop-walk.cjs`), so anything that leaks a temp path, a byte count, a + * timestamp, or free-form prose into the identity would make every run look + * "new" even when nothing about the underlying behavior changed. That would + * make the ratchet permanently red (or, worse, silently trained to be + * ignored) — exactly the failure mode `assertWithinAllowlist` + * (`scripts/lib/allowlist-ratchet.cjs`) exists to prevent for other guards. + * + * STABLE INPUTS ONLY + * ─────────────────── + * `fingerprint(scenarioName, smell)` composes from exactly four stable + * fields: + * 1. `smell.id` — the oracle id (e.g. `"value-hygiene"`), a closed, + * versioned enum (`ORACLES` in `oracles.cjs`). + * 2. `scenarioName` — the scenario's own `name` field, author-chosen and + * fixed at scenario-authoring time. + * 3. `smell.argv` — the exact CLI invocation (`step.argv`, an array of + * literal argument strings) joined with a single + * space. This is deterministic: two runs of the same + * scenario always issue the same argv for the same + * step. + * 4. `smell.subject` — a STRUCTURAL discriminator extracted from the + * oracle's own `subject` payload (see `oracles.cjs`'s + * `OracleOutcome` typedef), via `subjectDiscriminator` + * below. Only whitelisted, provably-stable fields are + * read from `subject` — see that function's header. + * + * `smell.detail` (free-form prose, may embed a temp path or a byte count) is + * NEVER read here. Neither is anything derived from `Date.now()` / + * `Math.random()` / the process's own PID or hostname — this module performs + * NO I/O and reads NO ambient state, so `fingerprint()` is a pure function of + * its two arguments. + * + * WHY A HASH *AND* A READABLE COMPOSITE (not one or the other) + * ────────────────────────────────────────────────────────────── + * A pure hash (e.g. sha256 of the four fields) is technically sufficient as + * an identity, but it makes `tests/qa/smell-baseline.json` an opaque blob: a + * reviewer diffing a PR that touches the baseline sees `"a1b2c3d4e5f6"` roll + * to `"9f8e7d6c5b4a"` and cannot tell what changed without re-running the + * ratchet themselves. A pure readable composite (no hash) is diffable but + * fragile to injection: nothing stops a scenario named `"a|b"` combined with + * an argv token containing `|` from colliding with an unrelated + * oracle/scenario/argv/subject tuple that happens to serialize to the same + * joined string, and JSON.stringify-based hashing of an array does not have + * that ambiguity (array elements are individually length-prefixed by the + * serializer's own quoting/escaping rules). + * + * The chosen key is therefore BOTH, concatenated as `"::"`: + * - `` (first 12 hex chars of sha256 over an unambiguous JSON-array + * encoding of the four fields) is the collision-safe, canonical identity + * — this is the part any two runs of the same finding are GUARANTEED to + * agree on bit-for-bit, regardless of what punctuation an author put in + * a scenario name or an argv token. + * - `` (the four fields pipe-joined, human-legible) is what makes + * `smell-baseline.json` diffable in a PR review — a reviewer can read + * `value-hygiene|greenfield-happy-path|--json-errors init new-project|key=$.agents_dir` + * and immediately know what fired, without decoding a hash. + * Per the brief's "prefer the readable composite as the key if it is + * deterministic" guidance: the readable half is deterministic here (all four + * inputs are literal, author-controlled strings/arrays with no free + * variation run-to-run), so it is safe to keep in the identity rather than + * discarding it in favor of the hash alone. + * + * @module tests/qa/smell-fingerprint + */ + +const crypto = require('node:crypto'); + +/** + * Deterministically stringify a plain JSON-ish value with object keys sorted + * recursively, so two structurally-equal objects with keys inserted in a + * different order always serialize identically. Arrays keep their order + * (order is semantically meaningful for an array; it is not for object keys). + * + * Used only on the small, already-whitelisted `subject.fromScope` / + * `subject.toScope` scope objects (see `progressScopeOf` in `oracles.cjs`) — + * never on an arbitrary/attacker-shaped value — so cycle-safety is + * deliberately NOT handled here (those objects are always flat + * `{milestoneVersion, milestoneName, workstream}` records). + * + * @param {unknown} value + * @returns {string} + */ +function stableStringify(value) { + if (value === null || typeof value !== 'object') return JSON.stringify(value); + if (Array.isArray(value)) return `[${value.map(stableStringify).join(',')}]`; + const keys = Object.keys(value).sort(); + return `{${keys.map((k) => `${JSON.stringify(k)}:${stableStringify(value[k])}`).join(',')}}`; +} + +/** + * Extract a stable, human-readable discriminator from an oracle's `subject` + * payload (see `oracles.cjs`'s `OracleOutcome` typedef for the per-oracle + * shapes). Reads ONLY fields that are, by construction, free of run-to-run + * variation: + * + * - `subject.key` (`value-hygiene`'s JSON-path leaf, e.g. + * `"$.agents_dir"` — a structural path, never + * the leaked VALUE at that path, which may be + * a temp-dir-dependent absolute path) + * - `subject.missing` (`read-only-idempotence`'s absent-input + * case — a fixed, closed set of field names) + * - `subject.fromScope`/`toScope` (`monotonic-progress`'s scope-boundary + * case — `{milestoneVersion, milestoneName, + * workstream}`, all literal/author-controlled + * strings; the sibling `from`/`to`/ + * `fromIndex`/`toIndex` counters on that same + * subject are deliberately NEVER read here, + * since counters are exactly the kind of + * per-run-variable field the brief bans) + * + * `subject.argv` (present on `soft-error-exit-zero` / `untyped-success` / + * `contract-conflict`) is deliberately NOT read here — it duplicates + * `smell.argv`, which `fingerprint()` already folds in separately, so reading + * it again here would add nothing but risk drifting out of sync with that + * field. A `subject.mismatches` array (`read-only-idempotence`'s + * data-present-but-differs case) is also deliberately NOT read here — its + * entries embed live stat facts (`size`, `mtimeMs`) that vary by definition + * between runs, so no discriminator can be derived from it without violating + * the "no counts" rule; that oracle is a VIOLATION-only check today (never a + * SMELL — see `oracles.cjs`), so this gap is currently unreachable in + * practice, but is documented here rather than silently mishandled if that + * ever changes. + * + * @param {unknown} subject + * @returns {string} + */ +function subjectDiscriminator(subject) { + if (!subject || typeof subject !== 'object' || Array.isArray(subject)) return '(no-subject)'; + const parts = []; + if (typeof subject.key === 'string') parts.push(`key=${subject.key}`); + if (typeof subject.missing === 'string') parts.push(`missing=${subject.missing}`); + if (subject.fromScope !== undefined) parts.push(`fromScope=${stableStringify(subject.fromScope)}`); + if (subject.toScope !== undefined) parts.push(`toScope=${stableStringify(subject.toScope)}`); + return parts.length ? parts.join(';') : '(subject-with-no-stable-discriminator)'; +} + +/** + * Compute the stable fingerprint for one smell finding within one scenario. + * + * PURITY GUARANTEE: this function performs no I/O, reads no ambient state + * (`Date.now()`, `Math.random()`, env vars, the filesystem), and its return + * value is a pure function of `scenarioName` and the four stable fields read + * off `smell` (see this file's header). Two calls with structurally-equal + * arguments — even across two separate process invocations, two different + * temp directories, two different hosts — MUST return byte-identical + * strings. This is proven empirically in `scripts/qa-smell-ratchet.cjs`'s + * `--update` flow (fingerprinting the same scenario corpus twice, in two + * separate `mkdtemp` runs, and diffing the resulting key sets) and in the + * VERIFY section of the PR that introduced this module. + * + * @param {string} scenarioName the owning scenario's `name` field. + * @param {{id: string, subject?: object, argv?: string[]}} smell one finding + * from a step's `smells` array, AUGMENTED with that step's own `argv` (the + * raw `runOracles` finding shape from `oracles.cjs` carries `id`/`detail`/ + * `subject` only — `detail` is intentionally never read by this function; + * the caller is responsible for attaching the owning step's `argv`). + * @returns {string} `"<12-hex-char sha256>::|||"`. + * @throws {TypeError} when `scenarioName` is not a non-empty string or + * `smell.id` is not a non-empty string — a fingerprint computed from a + * malformed input would silently corrupt the whole ratchet's identity + * space, so this fails loudly instead. + */ +function fingerprint(scenarioName, smell) { + if (typeof scenarioName !== 'string' || scenarioName === '') { + throw new TypeError(`fingerprint: scenarioName must be a non-empty string, got ${JSON.stringify(scenarioName)}`); + } + if (!smell || typeof smell !== 'object' || typeof smell.id !== 'string' || smell.id === '') { + throw new TypeError(`fingerprint: smell.id must be a non-empty string, got ${JSON.stringify(smell && smell.id)}`); + } + + const argvJoined = Array.isArray(smell.argv) ? smell.argv.join(' ') : ''; + const subjectPart = subjectDiscriminator(smell.subject); + + const readable = `${smell.id}|${scenarioName}|${argvJoined}|${subjectPart}`; + const hash = crypto + .createHash('sha256') + .update(JSON.stringify([smell.id, scenarioName, argvJoined, subjectPart])) + .digest('hex') + .slice(0, 12); + + return `${hash}::${readable}`; +} + +module.exports = { fingerprint, subjectDiscriminator, stableStringify };