From a613caaeef951d0bb89b6ebb4e6034cb9936b429 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 27 Jul 2026 19:55:37 -0400 Subject: [PATCH] enhance(#2721): regenerating merge driver, regen:derived, and a name for the emitted-artifact family (#2730) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#2721): failing-first suite for the gsd-regen driver and CONTEXT.md parity Tests precede the implementation per the TDD gate. The driver module does not exist yet, so tests/git-merge-regen-driver.test.cjs fails at require time; the contributor-standards parity assertions fail against next as it stands today, where the standards doc names two CONTEXT.md headings that have never existed. Refs #2721 * feat(#2721): add the gsd-regen merge driver and regen:derived The golden parity manifests and the two size baselines are pure functions of the source tree, so their only correct merge is "recompute" -- something git's ours/theirs interface cannot express. 140 of 143 conflicted-file instances across the open PR queue are these files. The driver deliberately does NOT regenerate. Four probes established that at merge-driver time neither the working tree nor the index reflects the merge: both hold the ours side, a file added by theirs does not exist yet, and MERGE_HEAD is unwritten. Git also invokes the driver once per conflicted path (20 here). A regenerating driver would therefore read the ours-side tree and emit a plausible-but-wrong hash manifest -- worse than a conflict, because a conflict is visible. So it accepts %A, runs zero subprocesses, records the resolved paths, and prints one notice pointing at npm run regen:derived. Staleness stays caught where it already was, by golden-install-parity in CI. Every failure path degrades toward today's behaviour (a normal conflict). install-tree is deliberately excluded per ADR-2719 section 7. Also folded in, per the no-defer rule: workflow-size.cjs claimed .md files have no eol=lf in .gitattributes; git check-attr shows eol: lf, set by .gitattributes line 2 since #1088. Refs #2721 * docs(#2721): document regen:derived and the gsd-regen merge driver Adds the how-to a contributor actually reaches for when the generated parity manifests or size baselines conflict, in both places they would look: the merge-conflict path in CONTRIBUTING.md and the full guide in TESTING-SUITES.md, including what the driver deliberately does not do (it does not clear GitHub's CONFLICTING label, and it does not regenerate mid-merge). Also scopes the new contributor-standards parity assertion to the doc's own CONTEXT.md section. Its first run flagged `## Decision`, `## Consequences` and `## Standards followed`, which the doc attributes to an ADR body and a PR body rather than to CONTEXT.md -- a doc-wide extractor would have demanded CONTEXT.md grow headings that do not belong to it. Refs #2721 * fix(#2721): stop passing %P to the merge driver — shell injection The isolated adversarial review found, and I independently reproduced, local arbitrary command execution. Git does not invoke a merge driver with an argv array. It substitutes %O %A %B %L %P textually into the configured string and runs the whole thing through a shell, and $(...) executes inside POSIX double quotes -- so quoting the placeholder does not neutralise it. %O/%A/%B are git-generated temp names and %L is an integer, but %P is the file's own path, chosen freely by any contributor. A branch renaming a covered fixture to evil$(touch PWNED_SENTINEL).json executed that command on the machine of every maintainer who merged it, and the merge still reported success. Fix removes the input rather than filtering it: %P is no longer registered, so the driver receives no attacker-controlled argument at all. The marker records a count instead of path names. A metacharacter filter would have been a guess about shell grammar; passing nothing is a property. Re-ran the identical exploit against the fixed command: nothing executed, conflict still resolved. Two regressions guard it -- a platform-independent assertion that the registered command carries no %P, and a real merge driven by the actual planInstall output with a $(...) filename. Also from review: CLI dispatch had no coverage at all (CONTRIBUTING's "CLI and command routing" matrix), which is why runInstall/runStatus now take {repoRoot} -- hardcoding REPO_ROOT was what made them untestable. Renamed planResolution to resolveAndRecord since the plan* prefix promised purity it did not have. Reconciled the eleven-vs-twelve generator count across CONTEXT.md, CONTRIBUTING.md and the changeset. Refs #2721 * test(#2721): scope safe.directory for the check-attr helper The 66f4d85a run failed 11 assertions, all in the .gitattributes scoping block, with "fatal: detected dubious ownership in repository at '/work'". The test container checks the repo out at a path its user does not own, so git refuses check-attr outright. Everything else passed (27,185). `check-attr` is a pure read of .gitattributes -- no hooks, no filters -- so the exemption is scoped to that one invocation. It is deliberately NOT applied to the driver's own production `git config` calls, which run in the user's own clone and should keep the protection. Refs #2721 * test(#2721): delete the stale assertion that the driver command carries %P The plex2 run on bdfd0856 left exactly two failures, both this test: it still asserted the pre-fix command string, i.e. the vulnerable behaviour. Deleted rather than relaxed, per RULESET.TESTS.delete-bad-tests -- its useful half is already covered, in both directions, by registeredDriverCommandNeverPassesThePlaceholderForTheFilePath. Refs #2721 * test(#2721): drive the end-to-end merges from the real planInstall output The e2e helper hand-rolled its own driver registration, and still carried %P. That meant the five real-git tests were not exercising the production command string at all -- planInstall could drift and they would keep passing. They now register exactly what a contributor gets from npm run setup:merge-driver. Refs #2721 * chore(#2721): backfill changeset pr number to 2730 --- .changeset/curious-deer-cheer.md | 5 + .gitattributes | 23 + CONTEXT.md | 4 + CONTRIBUTING.md | 21 + docs/TESTING-SUITES.md | 44 ++ docs/contributor-standards.md | 8 +- package.json | 2 + scripts/git-merge-regen-driver.cjs | 367 +++++++++++ scripts/workflow-size.cjs | 13 +- tests/contributor-standards.test.cjs | 133 ++++ tests/git-merge-regen-driver.test.cjs | 879 ++++++++++++++++++++++++++ 11 files changed, 1491 insertions(+), 8 deletions(-) create mode 100644 .changeset/curious-deer-cheer.md create mode 100644 scripts/git-merge-regen-driver.cjs create mode 100644 tests/git-merge-regen-driver.test.cjs diff --git a/.changeset/curious-deer-cheer.md b/.changeset/curious-deer-cheer.md new file mode 100644 index 000000000..bc5c20d82 --- /dev/null +++ b/.changeset/curious-deer-cheer.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 2730 +--- +**`npm run regen:derived` regenerates every derived artifact in one command, and a `gsd-regen` merge driver ends hand-resolving the generated parity manifests** — the golden install-parity fixtures and the workflow/agent size baselines are pure functions of the source tree, so neither side of a merge conflict on them is ever correct. Run `npm run setup:merge-driver` once per clone and conflicts on those files resolve to your branch's copy with a one-line notice; `npm run regen:derived` then recomputes them, replacing seven separate invocations with one dependency-ordered command over twelve generators. (#2721) diff --git a/.gitattributes b/.gitattributes index 56654af1f..55e515071 100644 --- a/.gitattributes +++ b/.gitattributes @@ -10,3 +10,26 @@ *.woff2 binary *.ttf binary *.pdf binary + +# --- Emitted Artifact Provenance (#2721, ADR-2719 Phase 1) ------------------- +# +# These artifacts are pure functions of the source tree. Git can offer ours or +# theirs; both are wrong, because the only correct value is recomputed from the +# merged tree — so `merge=gsd-regen` keeps your branch's copy and tells you to +# run `npm run regen:derived`, instead of asking you to hand-merge 7,500 lines +# of hex. `linguist-generated` stops GitHub rendering them expanded in diffs. +# +# BRIDGE — DELETE THIS BLOCK IN #2724, which removes these files entirely. +# Register the driver in your clone with: npm run setup:merge-driver +# +# Declared by exact path, never by a `tests/*-size-baseline.json` glob: a glob +# would silently capture any future baseline that had not been reasoned about. +tests/fixtures/golden-install-parity/*.json merge=gsd-regen linguist-generated=true +tests/workflow-size-baseline.json merge=gsd-regen linguist-generated=true +tests/agent-size-baseline.json merge=gsd-regen linguist-generated=true + +# tests/fixtures/install-tree/*.json is deliberately ABSENT from the block above. +# ADR-2719 §7 keeps it committed and normally-merged: it conflicts on 0 of 7, its +# diffs are readable, and it preserves "the installer stopped shipping X" as a +# hard absolute failure with no attribution reasoning involved. Capturing it here +# would silently convert that absolute into an auto-resolve. diff --git a/CONTEXT.md b/CONTEXT.md index 8512928a4..e4417f647 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -409,6 +409,9 @@ The producer half of the async external-job contract (#1164, part of #1105). Def ### Broken Windows Ledger The enforced cross-phase defect register operationalizing GSD's no-defer discipline as a tracked artifact (#1950). Markdown file at `.planning/WINDOWS.md` (project-level, cross-phase) with YAML frontmatter carrying scalar counts (`schema_version`, `open_count`, `waived_count`, `fixed_count`, `total_count`, `last_updated`) for the FAST path the gate reads via jq without parsing JSON, plus a JSON code block as the AUTHORITATIVE entries source; the two cross-check and fail closed on drift. Each entry: `{ id, kind, phase, file, line, description, status, reason, recorded_at, resolved_at }`; kinds are closed (`stub | todo | fixme | skipped-test | lint-warning | unmet-truth | unrun-verify | deviation`); statuses are closed (`open | waived | fixed`). The `broken-windows` Capability (`capabilities/broken-windows/capability.json`) registers one `ship:pre` gate with predicate `artifact-frontmatter-equals WINDOWS.md open_count == 0`; federated config key `workflow.windows_enforce` (default `false` — opt-in enforcement, tracking-only by default so a project can adopt the ledger before turning the gate on). Population is best-effort and never blocks execution: `agents/gsd-executor.md` appends stubs/skipped-tests/unrun-verifies via `gsd_run windows append` after writing SUMMARY.md. Source of truth: `src/broken-windows.cts` → `gsd-core/bin/lib/broken-windows.cjs` (pure `parseLedger`/`renderLedger`/`appendWindow`/`markWaived`/`markFixed` + I/O `cmdWindowsStatus`/`Append`/`Waive`/`MarkFixed`); CLI surface `gsd-tools windows status|append|waive|fixed`. Ship gate enforcement is a `capId == "broken-windows"` branch in `gsd-core/workflows/ship.md` preflight (sibling to the `security` branch); it reads `gsd_run windows status --raw` and fails closed on a non-zero/non-numeric `open_count` (an unparseable ledger is itself a broken window). `/gsd:progress` surfaces the open+waived count. The ledger is optional and backward-compatible: a project with no `.planning/WINDOWS.md` reports `open_count: 0` and ships cleanly, and with `workflow.windows_enforce=false` (the default) ship never blocks on it. Frozen `REASON` enum: `WINDOWS_LEDGER_MISSING | WINDOWS_LEDGER_MALFORMED | WINDOWS_ID_NOT_FOUND | WINDOWS_ALREADY_RESOLVED | WINDOWS_WAIVE_REASON_EMPTY | WINDOWS_INVALID_KIND | WINDOWS_INVALID_FILE | WINDOWS_INVALID_ID | WINDOWS_APPEND_MISSING_FIELD | WINDOWS_USAGE | WINDOWS_OK` — surfaced through `--json-errors` for typed test assertions. Test seam: `tests/broken-windows.test.cjs`. Origin: *The Pragmatic Programmer* Topic 3 (Hunt & Thomas — software transplant of Wilson & Kelling's broken-windows metaphor) plus Cunningham's debt metaphor (decay accrues interest ⇒ accounting, not just habit). +### Emitted Artifact Provenance +Cross-seam principle (ADR-2719, epic #2719): a committed artifact that is a pure function of the source tree is not reviewable state — it is derived state wearing a review costume, and it must be *attributable* rather than *pinned*. Concept, not a Module: it ships nothing, so it takes no `Module` suffix (follows the `### Resolution Provenance` precedent). Scope is the emitted-artifact family named by `RULESET.EMITTED_ATTRIBUTION`. The principle: every emitted path whose hash moved between `next` HEAD and PR HEAD must be attributable — through a declarative provenance table — to a path the pull request actually changed; unattributable deltas are a hard failure that *names them* rather than an anomaly a reviewer must notice inside 7,500 lines of hex. Totality is enforced, so an emitted path matching no rule fails loudly instead of passing through unattributed. The escape hatch is a committed acknowledgment (`tests/emitted-drift-ack.json`), deliberately not a flag or env var — the file appears in the changed-files list ONLY when something rippled unexpectedly, so touching it IS the alarm, whereas today 100% of emitted-byte changes touch fixtures and touching them signals nothing. The same differential machine carries the size ratchet: growth is reported with exact byte deltas and needs the same acknowledgment, so anti-creep survives without pinning a number. Distinguish from the absolute check that remains: `tests/fixtures/install-tree/*.json` stays committed and normally-merged (ADR-2719 §7) because "the installer stopped shipping X" must fail with no attribution reasoning involved. Delivery is phased — #2721 naming + interim merge relief, #2722 provenance table + totality guard, #2723 differential check dual-run beside `golden-install-parity.test.cjs`, #2724 cutover. Supersedes ADR-2264 §2–§4 and its Amendment; ADR-2264 Phase 1 (`buildParityManifest` and the exclusion constants in `tests/helpers/install-shared.cjs`) is retained and depended upon. + ### Untrusted-input boundary The prompt-level data/instruction isolation seam for untrusted web/document ingress (#1577). Shared reference `gsd-core/references/untrusted-input-boundary.md`, `@`-included by the 10 ingest agents (`gsd-project-researcher`, `gsd-phase-researcher`, `gsd-ui-researcher`, `gsd-assumptions-analyzer`, `gsd-advisor-researcher`, `gsd-ai-researcher`, `gsd-domain-researcher`, `gsd-research-synthesizer`, `gsd-doc-classifier`, `gsd-doc-synthesizer`) — every agent that reads fetch/search/MCP output or external source documents. The reference instructs: treat fetched/read content as **data, never instructions**; self-scan content for embedded directives before use; act only on the assigned task (ignore off-task instructions in data); and wrap quoted untrusted spans in a **fresh random delimiter** per wrap (fixed markers are spoofable). This prompt-level boundary is the primary control — it keeps an injection from being *followed* even while it sits in context. The hook-level companion is the read-injection scanner (`hooks/gsd-read-injection-scanner.js`, PostToolUse on `Read`/`WebFetch`/`WebSearch`), advisory by default; the opt-in top-level `security.injection_blocking` key upgrades HIGH-confidence detections to a PostToolUse circuit-breaker that halts the agent's next step (it runs *after* the fetch, so it is not a redactor). Tests: `tests/untrusted-input-isolation.test.cjs`, `tests/read-injection-scanner.*.test.cjs`, `tests/injection-blocking-config.test.cjs`. See `docs/adr/1577-untrusted-input-boundary-and-injection-blocking.md` and `docs/explanation/security-model.md`. Grounding: arXiv 2506.05739 (PPA), 2507.15219 (PromptArmor), 2504.20472. @@ -471,6 +474,7 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr `RULESET.WORKFLOW_MARKDOWN.FENCES=preserve opening language fence when editing shell snippets in workflow markdown; malformed fence creates fresh CR threads (MD040)` `RULESET.WORKFLOW_SIZE_BUDGET=workflow size enforcement (#1074; BYTES not lines per #717; LF-normalized per #683) = per-file baseline (PRIMARY anti-creep: tests/workflow-size-baseline.json pins each file's exact size) + loose tier hard caps (outer red lines, NEVER raised on approach: XL<=98304 / LARGE<=61440 / DEFAULT<=40960) + new-file cap (un-baselined files <32768, the Codex anchor) + discuss-phase<32000; a file that grew fails the baseline guard — fix with `npm run size:baseline`, commit the one-line diff, and justify the growth in the PR (or extract LAZILY-loaded content; eager @-imports don't reduce loaded context); crossing a hard cap means EXTRACT, not bump` `RULESET.AGENT_SIZE_BUDGET=agent-size-budget (#1074; sibling of WORKFLOW_SIZE_BUDGET; BYTES not lines per #717/#683, rebased from lines in PR 3/3) = per-file baseline (PRIMARY anti-creep: tests/agent-size-baseline.json pins each agents/gsd-*.md exact byte size) + loose tier hard caps (red lines, never raised on approach: XL<=57344 / LARGE<=49152 / DEFAULT<=24576); net-new agents are DEFAULT-tier (no separate new-file cap). One 'npm run size:baseline' regenerates BOTH workflow and agent baselines via the shared scripts/workflow-size.cjs measureMdFiles(dir,predicate) counter. A grown agent fails the baseline guard — regenerate + justify, or extract LAZILY to gsd-core/references/. DISTINCT from DEFECT.AGENT-FILE-SIZE-CAP-BREACH (a separate 45K-CHAR extraction-evidence threshold on gsd-planner via planner-decomposition/reachability tests): that guard proves mode-sections were extracted; this one bounds total agent bytes. Two guards, two units (chars vs bytes), two purposes` +`RULESET.EMITTED_ATTRIBUTION=the emitted-artifact family (ADR-2719, epic #2719) = tests/fixtures/golden-install-parity/*.json (19 path→hash manifests) + tests/workflow-size-baseline.json + tests/agent-size-baseline.json. All three are PURE FUNCTIONS of the source tree, so their correct merge is ALWAYS "recompute", which git's ours/theirs interface cannot express — 140 of 143 conflicted-file instances across the open PR queue are these files, and 3 of 7 conflicting PRs have ZERO overlapping keys (pure line-adjacency false conflicts in a sorted map). Amplification: gsd-core/workflows|references|templates|contexts is copied to every host, so ONE source edit rewrites the same hash in all 19 manifests (19× the change). REGENERATE with `npm run regen:derived` (single command replacing seven invocations, dependency-ordered, gen:golden LAST because it hashes installed output); never hand-merge. It covers TWELVE generators, not the eleven #2721 enumerates: `sync-manifest-versions` is folded in because `lint:generated-sync` checks it too, and a command that claims to regenerate everything must not leave that gate red. Phase 1 (#2721) relief = `merge=gsd-regen` in .gitattributes + scripts/git-merge-regen-driver.cjs, registered per-clone via `npm run setup:merge-driver`; the driver accepts OURS and does NOT regenerate, because at merge-driver time neither the working tree nor the index reflects the merge (proven) — regenerating there would emit a plausible-but-wrong manifest. It removes the labour, NOT the GitHub CONFLICTING label (drivers live in .git/config; forks lack it, github.com never runs them). BRIDGE ONLY: #2724 deletes the artifacts and retires the driver. tests/fixtures/install-tree/*.json is DELIBERATELY EXCLUDED (ADR-2719 §7): it conflicts on 0 of 7, its diffs are readable, and it preserves "the installer stopped shipping X" as a hard absolute failure — capturing it would convert that absolute into an attribution-free auto-resolve. cf `RULESET.WORKFLOW_SIZE_BUDGET`, `RULESET.AGENT_SIZE_BUDGET`; see `### Emitted Artifact Provenance`` `RULESET.WORKFLOW_FILE_NAMES=workflow files use hyphens; XML attributes must match (extract-learnings not extract_learnings); tests should pin exact hyphenated name` `RULESET.WORKFLOW_EXECUTION_CONTEXT=@-ref in commands/gsd/*.md must resolve to an existing file on disk; regression test in tests/docs-update.test.cjs (folds former \`bug-3135-capture-backlog-workflow\`, consolidation epic #1969); INVENTORY.md row + INVENTORY-MANIFEST.json families.workflows must stay in sync; "Invoked by" attribution must move when a flag absorbs a micro-skill` `RULESET.WORKFLOW_EXECUTE_END_TO_END=standard for single-workflow commands is "Execute end-to-end." (no bolded **Follow the X workflow** fragments); flag-dispatch routing uses "execute the X workflow end-to-end." in routing bullets — convention verified live across ~20 commands/gsd/*.md files; no ADR currently documents this specific phrasing rule (ADR-0002 covers the adjacent but distinct command-contract/@-ref-resolution seam, not this convention)` diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 243dc13be..15e2fa71d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -763,6 +763,27 @@ npm run check:alias-drift This verifies generated alias artifacts are in sync with manifest source-of-truth. +### Generated artifacts that conflict on every merge + +If your PR conflicts on `tests/fixtures/golden-install-parity/*.json`, +`tests/workflow-size-baseline.json`, or `tests/agent-size-baseline.json`, **do not +hand-resolve them.** They are pure functions of the source tree, so neither "ours" +nor "theirs" is correct — the only correct value is recomputed. + +```bash +npm run setup:merge-driver # once per clone: conflicts resolve to your copy +npm run regen:derived # then recompute, before committing +``` + +`regen:derived` replaces the seven separate invocations this used to take (`npm run +build` plus six more), and runs them in dependency order — `gen:golden` last, because it +hashes installed output. It covers twelve generators: the eleven that produce committed +artifacts, plus `sync-manifest-versions`, which `npm run lint:generated-sync` also checks — +without it, the command that claims to regenerate everything could still leave that gate red. +Full guide, including what the driver deliberately does not do: +[docs/TESTING-SUITES.md](docs/TESTING-SUITES.md) → "the baselines or golden fixtures +conflict on merge". + Optional local pre-commit hook entry (Git-native): ```bash diff --git a/docs/TESTING-SUITES.md b/docs/TESTING-SUITES.md index 32911f440..831e4fd2a 100644 --- a/docs/TESTING-SUITES.md +++ b/docs/TESTING-SUITES.md @@ -117,12 +117,56 @@ the workflow and agent guards). To resolve: If a hard cap (not the baseline) is what failed, regeneration will **not** help — that is the signal to extract, per step 3. +### How-to: the baselines or golden fixtures conflict on merge + +`tests/workflow-size-baseline.json`, `tests/agent-size-baseline.json` and +`tests/fixtures/golden-install-parity/*.json` are **Emitted Artifact Provenance** +files (`CONTEXT.md` → `RULESET.EMITTED_ATTRIBUTION`): pure functions of the source +tree. Their correct merge is always *recompute*, which git's ours/theirs interface +cannot express — so a conflict here is never something to hand-resolve. + +Register the merge driver once per clone: + +```bash +npm run setup:merge-driver +``` + +Afterwards a conflicting merge, rebase or cherry-pick keeps your branch's copy and +prints a one-line notice. Recompute the artifacts before committing: + +```bash +npm run regen:derived +``` + +That one command runs every generator in dependency order (`gen:golden` last, +because it hashes installed output). On an unmodified tree it produces no diff. + +Two things it deliberately does **not** do: + +- **It does not clear GitHub's `CONFLICTING` label.** Merge drivers live in + `.git/config`, so forks do not have one and github.com's own merge never runs a + custom driver. The driver removes the labour, not the label. +- **It does not regenerate during the merge.** At the moment git invokes a merge + driver, neither the working tree nor the index reflects the merge yet — so + regenerating there would compute the artifact from the *pre-merge* tree and write + a confidently wrong answer. Running `regen:derived` afterwards is what makes it + correct. + +This driver is a bridge introduced by [#2721](https://github.com/open-gsd/gsd-core/issues/2721) +and retired by [#2724](https://github.com/open-gsd/gsd-core/issues/2724), which +replaces these committed artifacts with a computed attribution check (ADR-2719). +`tests/fixtures/install-tree/*.json` is deliberately excluded and keeps normal merge +semantics — its diffs are readable and it must stay an absolute "the installer +stopped shipping X" failure. + ### Reference | Artifact | Role | |---|---| | `scripts/workflow-size.cjs` | Single source of truth — LF-normalized byte counter (`lfByteCount`) + generic `measureMdFiles(dir, predicate)` (backs both workflows and agents) + workflow enumeration (`listWorkflowStems`, `measureWorkflows`). Imported by **both** the guards and the generator so they can never measure differently. | | `scripts/update-size-baseline.cjs` (`npm run size:baseline`) | Regenerates **both** `tests/workflow-size-baseline.json` and `tests/agent-size-baseline.json` — sorted keys, trailing newline, idempotent. | +| `npm run regen:derived` | Runs every generator in dependency order (build → registry → ADR index → capability matrix → inventory manifest → manifest versions → size baselines → golden fixtures). Use it instead of remembering which generator owns which artifact. | +| `scripts/git-merge-regen-driver.cjs` (`npm run setup:merge-driver`) | Registers the `gsd-regen` merge driver in this clone. Keeps your branch's copy of a conflicting generated artifact and points you at `regen:derived`. Bridge for #2721; retired by #2724. | | `tests/workflow-size-baseline.json` | The committed per-workflow snapshot (one entry per workflow). | | `tests/agent-size-baseline.json` | The committed per-agent snapshot (one entry per `gsd-*` agent). | | `tests/workflow-size-budget.test.cjs` | The three workflow guards above, plus the `discuss-phase` progressive-disclosure checks. | diff --git a/docs/contributor-standards.md b/docs/contributor-standards.md index 286126639..a58501b05 100644 --- a/docs/contributor-standards.md +++ b/docs/contributor-standards.md @@ -18,23 +18,23 @@ These apply to every PR — fix, enhancement, or feature. They are part of the m `CONTEXT.md` is the single source of truth for domain vocabulary. It defines: -- **Domain terms** — canonical Module names, seam vocabulary, and Interface names (e.g. Dispatch Policy Module, Command Contract Validation Module, Planning Workspace Module) +- **Domain modules and seams** — canonical Module names, seam vocabulary, and Interface names (e.g. Dispatch Policy Module, Command Contract Validation Module, Planning Workspace Module) - **Recurring PR mistakes** — CodeRabbit findings that recur; covers tests, shell guards, changesets, docs - **Workflow learnings** — patterns distilled from triage + PR cycles ### Format -`CONTEXT.md` is written as flat named sections under `## Domain terms` (for Modules/seams) and `##` sections for recurring rules. Machine-oriented predicates use `KEY.SUBKEY=value` flat format in code blocks under `## AI Ops Memory`. +`CONTEXT.md` is written as flat named sections under `## Glossary — Domain modules and seams` (for Modules/seams) and `##` sections for recurring rules. Machine-oriented predicates use `KEY.SUBKEY=value` flat format, grouped into the `##` section that owns the topic — `## Test rules and lint`, `## CodeRabbit + repo-process guards (machine-oriented predicates)`, or `## Workspace seams (machine-oriented predicates)`. Adding a new Module or seam: -- Add a `### ` entry under `## Domain terms`. +- Add a `### ` entry under `## Glossary — Domain modules and seams`. - Write one paragraph. State what the Module owns. Be concrete — list the Interface names and policy boundaries it covers. - Do not add synonyms; pick one name and use it everywhere. Extending an existing predicate: -- Add a `KEY.SUBKEY=value` line inside the relevant `## AI Ops Memory` block. +- Add a `KEY.SUBKEY=value` line inside the relevant predicate section — for a test or lint rule that is `## Test rules and lint`. - Do not create a new top-level section for a variation on an existing concept. When to add a new predicate vs extend an existing one: diff --git a/package.json b/package.json index 9c4d1dcfe..d06c586f0 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,8 @@ "gen:capability-registry": "node scripts/gen-capability-registry.cjs --write", "gen:registry": "node scripts/gen-registry.cjs --write", "gen:golden": "node scripts/gen-golden-install-parity-zcode.cjs && node scripts/gen-install-tree-fixtures.cjs", + "regen:derived": "npm run build && npm run gen:registry && node scripts/gen-adr-index.cjs --write && node scripts/gen-capability-matrix.cjs --write && node scripts/gen-inventory-manifest.cjs --write && node scripts/sync-manifest-versions.cjs && npm run size:baseline && npm run gen:golden", + "setup:merge-driver": "node scripts/git-merge-regen-driver.cjs --install", "validate:registry": "node scripts/validate-registry.cjs", "prepack": "npm run build:lib", "prepare": "npm run build:lib", diff --git a/scripts/git-merge-regen-driver.cjs b/scripts/git-merge-regen-driver.cjs new file mode 100644 index 000000000..bab9e2784 --- /dev/null +++ b/scripts/git-merge-regen-driver.cjs @@ -0,0 +1,367 @@ +#!/usr/bin/env node +'use strict'; + +/** + * git-merge-regen-driver.cjs — the `gsd-regen` git merge driver (#2721, ADR-2719 Phase 1). + * + * ## Why + * + * `tests/fixtures/golden-install-parity/*.json` and the two size baselines are pure + * functions of the source tree. Git offers ours or theirs; both are wrong, because the + * only correct value is recomputed from the merged tree. 140 of 143 conflicted-file + * instances across the open PR queue are these artifacts (ADR-2719). + * + * ## What this driver does — and deliberately does NOT do + * + * It does **not** regenerate. That is not implementable, and the constraint is git's, not + * a design preference: at the moment git invokes a merge driver, **neither the working + * tree nor the index reflects the merge**. Both hold the ours side; a file added by + * theirs does not exist yet; `.git/MERGE_HEAD` has not been written. A driver that shelled + * out to the generators there would read the ours-side tree and write a + * plausible-but-wrong hash manifest — strictly worse than a conflict, because a conflict + * is visible and a wrong manifest is not. Git also invokes the driver once **per + * conflicted path** (20 in this repo), so a regenerating driver would run the full build + * plus 19 installer spawns up to twenty times per merge. + * + * So it resolves deterministically and without content knowledge: it accepts `%A` (which + * already holds ours verbatim), runs **zero subprocesses**, records the resolved paths + * under the git dir, and prints **one** notice per operation pointing at + * `npm run regen:derived`. Staleness is caught where it already was — by + * `tests/golden-install-parity.test.cjs` in CI. + * + * Every failure path degrades toward *today's* behavior (a normal conflict), never toward + * a silent wrong resolution. + * + * ## Bridge, not a destination + * + * This driver is explicitly temporary. #2724 deletes the artifacts it guards and retires + * it. Keeping it past that point would preserve the problem it exists to relieve. + * + * node scripts/git-merge-regen-driver.cjs --install # register in .git/config + * node scripts/git-merge-regen-driver.cjs --uninstall + * node scripts/git-merge-regen-driver.cjs --status + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const cp = require('node:child_process'); + +const { runMain, ExitError } = require('./lib/cli-exit.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const MARKER_NAME = 'gsd-regen-pending.json'; + +/** Bounded per the repo's unbounded-subprocess rule (5-30s for git). */ +const GIT_TIMEOUT_MS = 15_000; + +/** + * A single git operation's driver invocations land milliseconds apart, so this window is + * ~4 orders of magnitude wider than it needs to be. It exists only so a *later* operation + * does not inherit the previous one's silence. + */ +const NOTICE_WINDOW_MS = 60_000; + +const ACTION = Object.freeze({ + ACCEPT_OURS: 'accept_ours', + DECLINE: 'decline', +}); + +const REASON = Object.freeze({ + OK_RESOLVED: 'ok_resolved', + FAIL_BAD_ARGV: 'fail_bad_argv', + FAIL_OURS_UNREADABLE: 'fail_ours_unreadable', +}); + +const GITDIR_SOURCE = Object.freeze({ + DIRECTORY: 'directory', + GITFILE: 'gitfile', + UNRESOLVED: 'unresolved', +}); + +/** + * Locate the git dir from `cwd` without spawning git — the driver runs up to twenty times + * per merge, so a subprocess per invocation is not affordable. + * + * Handles both shapes: `.git` as a directory, and `.git` as a pointer file + * (`gitdir: `) in a linked worktree or submodule. Never throws; an unresolvable git + * dir is a degraded-but-correct state, not an error. + * + * @param {string} cwd + * @returns {{gitDir: string|null, source: string}} + */ +function resolveGitDir(cwd) { + const unresolved = { gitDir: null, source: GITDIR_SOURCE.UNRESOLVED }; + const dotGit = path.join(cwd, '.git'); + + let stat; + try { + stat = fs.statSync(dotGit); + } catch { + return unresolved; + } + if (stat.isDirectory()) return { gitDir: dotGit, source: GITDIR_SOURCE.DIRECTORY }; + + let raw; + try { + raw = fs.readFileSync(dotGit, 'utf8'); + } catch { + return unresolved; + } + + // Anchored /m with an explicit trailing-whitespace eat: `$` under /m sits before the + // \n, so a CRLF checkout would otherwise carry the \r into the path. + const match = /^gitdir:\s*(.*?)\s*$/m.exec(raw); + if (!match || match[1] === '') return unresolved; + return { gitDir: path.resolve(cwd, match[1]), source: GITDIR_SOURCE.GITFILE }; +} + +/** + * Read the pending-resolution marker, treating anything unusable as absent. + * + * Valid JSON is not the same as a usable marker: `0`, `"str"`, `[]`, `null` and `true` all + * parse. So does an object whose `startedAt` or `count` is a string. Every one of those + * means "no previous invocation I can trust" — reset, do not throw. + * + * @returns {{startedAt: number, count: number}|null} + */ +function readMarker(markerPath) { + let parsed; + try { + parsed = JSON.parse(fs.readFileSync(markerPath, 'utf8')); + } catch { + return null; + } + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) return null; + if (!Number.isFinite(parsed.startedAt)) return null; + if (!Number.isFinite(parsed.count) || parsed.count < 0) return null; + return { startedAt: parsed.startedAt, count: parsed.count }; +} + +function decline(reason) { + return { + action: ACTION.DECLINE, + reason, + exitCode: 1, + notice: false, + pendingCount: 0, + }; +} + +/** + * Decide how to resolve one conflicted path, and record it. + * + * Deliberately NOT named `plan*` like `planInstall`: that prefix promises purity, and this + * function reads and writes the marker file. The name says both halves out loud. + * + * @param {object} opts + * @param {string[]} opts.argv exactly what git supplies: [%O, %A, %B, %L]. Any further + * entry is ignored — see planInstall on why `%P` is deliberately not registered. + * @param {string|null} opts.gitDir from resolveGitDir; null means marker-less (degraded) + * @param {number} opts.now injected clock — never Date.now() inline, so tests are + * deterministic and never assert on elapsed wall-clock + * @returns {{action: string, reason: string, exitCode: number, notice: boolean, + * pendingCount: number}} + */ +function resolveAndRecord({ argv, gitDir, now }) { + if (!Array.isArray(argv) || argv.length < 3) return decline(REASON.FAIL_BAD_ARGV); + + const oursPath = argv[1]; + if (typeof oursPath !== 'string' || oursPath.trim() === '') { + return decline(REASON.FAIL_BAD_ARGV); + } + // %A is the one input the driver's contract depends on. If it is not there, we do not + // know what "ours" is, so we hand the conflict back to git rather than inventing one. + try { + fs.statSync(oursPath); + } catch { + return decline(REASON.FAIL_OURS_UNREADABLE); + } + + const markerPath = gitDir ? path.join(gitDir, MARKER_NAME) : null; + const previous = markerPath ? readMarker(markerPath) : null; + const sameOperation = previous !== null && now - previous.startedAt <= NOTICE_WINDOW_MS; + + const startedAt = sameOperation ? previous.startedAt : now; + const pendingCount = sameOperation ? previous.count + 1 : 1; + + if (markerPath) { + // A diagnostic must never fail a merge: a read-only .git degrades to a repeated + // notice, which is noisy but correct. + try { + fs.writeFileSync(markerPath, `${JSON.stringify({ startedAt, count: pendingCount })}\n`); + } catch { + /* degraded, not failed */ + } + } + + return { + action: ACTION.ACCEPT_OURS, + reason: REASON.OK_RESOLVED, + exitCode: 0, + notice: !sameOperation, + pendingCount, + }; +} + +/** + * The `.git/config` entries that register this driver. + * + * ## Why `%P` is NOT passed — do not "helpfully" add it back + * + * Git does **not** invoke a merge driver with an argv array. It substitutes `%O %A %B %L + * %P` textually into this string and runs the whole thing through a shell. Quoting a + * placeholder does not make it safe: inside POSIX double quotes `$(…)` and backticks still + * execute, and a `"` in the value ends the quoting outright. + * + * `%O`, `%A` and `%B` are git-generated temp names (`.merge_file_XXXXXX`) and `%L` is an + * integer, so none of them are attacker-controlled. **`%P` is the file's own path**, which + * any contributor chooses freely. A branch that renames a covered fixture to + * `evil$(touch PWNED).json` would execute that command on the machine of every maintainer + * who merges it — silently, since the merge still reports success. Verified by reproduction + * against a real `git merge`, not by inspection. + * + * So the driver takes no attacker-controlled argument at all, and records a count rather + * than path names. A filter would have been a guess about shell grammar; passing nothing is + * a property. + * + * Paths are normalized to forward slashes **unconditionally** — a backslash path can reach a + * config value on any platform, and git shells this command everywhere including Windows. + * + * @param {{repoRoot: string}} opts + * @returns {{entries: Array<{key: string, value: string}>}} + */ +function planInstall({ repoRoot }) { + const script = path + .join(repoRoot, 'scripts', 'git-merge-regen-driver.cjs') + .replace(/\\/g, '/'); + return { + entries: [ + { + key: 'merge.gsd-regen.name', + value: 'gsd-regen — keep ours for generated artifacts; regenerate with npm run regen:derived', + }, + { + key: 'merge.gsd-regen.driver', + value: `node "${script}" "%O" "%A" "%B" "%L"`, + }, + ], + }; +} + +function gitConfig(args, cwd = REPO_ROOT) { + const r = cp.spawnSync('git', ['config', ...args], { + cwd, + encoding: 'utf8', + timeout: GIT_TIMEOUT_MS, + }); + if (r.error && r.error.code === 'ETIMEDOUT') { + throw new ExitError(1, `git config timed out after ${GIT_TIMEOUT_MS}ms`); + } + return r; +} + +function runInstall({ repoRoot = REPO_ROOT } = {}) { + const { entries } = planInstall({ repoRoot }); + for (const { key, value } of entries) { + const r = gitConfig([key, value], repoRoot); + if (r.status !== 0) throw new ExitError(1, `git config ${key} failed: ${r.stderr}`); + } + process.stdout.write( + 'Registered the gsd-regen merge driver in this clone.\n' + + 'Conflicts on the generated parity manifests and size baselines now resolve to your\n' + + 'branch’s copy; run `npm run regen:derived` afterwards to recompute them.\n' + + 'This is a bridge for #2721 and is retired by #2724.\n', + ); + return 0; +} + +function runUninstall({ repoRoot = REPO_ROOT } = {}) { + for (const key of ['merge.gsd-regen.driver', 'merge.gsd-regen.name']) { + // exit 5 == "was not set"; uninstalling something absent is success here + gitConfig(['--unset-all', key], repoRoot); + } + process.stdout.write('Removed the gsd-regen merge driver from this clone.\n'); + return 0; +} + +/** + * @returns {{registered: boolean, pendingCount: number}} the same object it prints, so + * callers and tests consume the structure rather than re-parsing the rendered JSON. + * A count, not path names: the driver is never handed the conflicted path, by design + * (see planInstall). + */ +function statusOf({ repoRoot = REPO_ROOT } = {}) { + const registered = gitConfig(['--get', 'merge.gsd-regen.driver'], repoRoot).status === 0; + const { gitDir } = resolveGitDir(repoRoot); + const marker = gitDir ? readMarker(path.join(gitDir, MARKER_NAME)) : null; + return { registered, pendingCount: marker ? marker.count : 0 }; +} + +function runStatus({ repoRoot = REPO_ROOT } = {}) { + process.stdout.write(JSON.stringify(statusOf({ repoRoot }), null, 2) + '\n'); + return 0; +} + +function runDriver(argv) { + const { gitDir } = resolveGitDir(process.cwd()); + const plan = resolveAndRecord({ argv, gitDir, now: Date.now() }); + + if (plan.action === ACTION.DECLINE) { + process.stderr.write( + `gsd-regen: declined (${plan.reason}) — leaving this path as a normal conflict.\n`, + ); + return plan.exitCode; + } + if (plan.notice) { + process.stderr.write( + 'gsd-regen: kept your branch’s copy of the generated parity/size artifacts.\n' + + 'gsd-regen: these files cannot be line-merged — their only correct value is recomputed.\n' + + 'gsd-regen: run `npm run regen:derived` before committing.\n' + + 'gsd-regen: (bridge for #2721; retired by #2724)\n', + ); + } + return plan.exitCode; +} + +function main() { + const argv = process.argv.slice(2); + const flags = argv.filter((a) => a.startsWith('--')); + + if (flags.length > 0) { + if (flags.length > 1 || argv.length > 1) { + throw new ExitError(2, 'usage: git-merge-regen-driver.cjs [--install|--uninstall|--status]'); + } + switch (flags[0]) { + case '--install': + return runInstall(); + case '--uninstall': + return runUninstall(); + case '--status': + return runStatus(); + default: + throw new ExitError( + 2, + `unknown flag ${flags[0]}\nusage: git-merge-regen-driver.cjs [--install|--uninstall|--status]`, + ); + } + } + + return runDriver(argv); +} + +module.exports = { + ACTION, + REASON, + GITDIR_SOURCE, + NOTICE_WINDOW_MS, + MARKER_NAME, + resolveGitDir, + resolveAndRecord, + planInstall, + runInstall, + runUninstall, + runStatus, + statusOf, +}; + +if (require.main === module) runMain(main); diff --git a/scripts/workflow-size.cjs b/scripts/workflow-size.cjs index 990359d74..7cfa3848f 100644 --- a/scripts/workflow-size.cjs +++ b/scripts/workflow-size.cjs @@ -19,11 +19,16 @@ const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); /** * Byte size of a file, counted as on an LF (Unix) checkout. * - * The size budget is calibrated against `wc -c` on a Unix (LF) checkout, but - * these `.md` files have no `eol=lf` in `.gitattributes`, so Windows checks - * them out as CRLF. Counting raw on-disk bytes there adds one byte per line, - * a Windows-only false positive that diverges from the LF calibration basis + * The size budget is calibrated against `wc -c` on a Unix (LF) checkout. + * Counting raw on-disk bytes on a CRLF checkout adds one byte per line, a + * Windows-only false positive that diverges from the LF calibration basis * (issue #683). Stripping CR yields the same LF byte count on every platform. + * + * `.gitattributes:2` (`* text=auto eol=lf`, added in #1088) now normalizes these + * files to LF on checkout everywhere, so the CRLF case should not arise from a + * normal clone — but this stays unconditional because it also covers a working + * tree produced some other way (an unpacked archive, an editor that rewrites + * line endings, a checkout predating that attribute). * This is still a raw byte count (not a trailing-newline-stripping line count). * * @param {string} filePath - Absolute or relative path to the file. diff --git a/tests/contributor-standards.test.cjs b/tests/contributor-standards.test.cjs index 532e53525..97a67a624 100644 --- a/tests/contributor-standards.test.cjs +++ b/tests/contributor-standards.test.cjs @@ -81,6 +81,139 @@ describe('docs/contributor-standards.md', () => { }); }); +/** + * Parity: docs/contributor-standards.md tells contributors which CONTEXT.md sections to + * write into. If it names a heading CONTEXT.md does not have, the instruction is + * unfollowable — and that is not hypothetical: on 2026-07-27 it named `## Domain terms` + * and `## AI Ops Memory`, neither of which has ever existed (#2721). Two surfaces, one + * truth; this asserts they cannot diverge again. + */ +const CONTEXT_MD = path.join(REPO_ROOT, 'CONTEXT.md'); + +/** + * The body of one `## ` section, exclusive of the next `## `. Split on /\r?\n/ so a CRLF + * checkout parses identically. Shared by every assertion below — two copies of the same + * section parser is exactly the silent divergence RULESET.SHARED-HELPERS-LINT-VS-TEST warns of. + */ +function sectionBody(content, heading) { + const lines = content.split(/\r?\n/); + const start = lines.findIndex((l) => l.trim() === heading); + if (start === -1) return null; + const rest = lines.slice(start + 1); + const end = rest.findIndex((l) => /^##\s/.test(l)); + return (end === -1 ? rest : rest.slice(0, end)).join('\n'); +} + +/** + * Backticked heading references that the standards doc attributes to CONTEXT.md. + * + * Scoped to the doc's own `## CONTEXT.md` section on purpose. The doc also names headings + * belonging to *other* documents — `## Decision` and `## Consequences` describe an ADR + * body, `## Standards followed` describes an issue/PR body. Extracting doc-wide would + * demand CONTEXT.md grow headings that have nothing to do with it. + * + * `` templates like `### ` are skipped: they are shapes to + * follow, not headings to find. + */ +function extractContextHeadingRefs(standardsContent) { + const scope = sectionBody(standardsContent, '## CONTEXT.md'); + if (scope === null) return []; + const refs = new Set(); + for (const m of scope.matchAll(/`(#{2,6}\s+[^`]+)`/g)) { + const heading = m[1].trim(); + if (heading.includes('<')) continue; + refs.add(heading); + } + return [...refs]; +} + +/** Headings actually present in CONTEXT.md. Anchored /m — CRLF-safe without a `\n` split. */ +function actualHeadings(contextContent) { + return new Set([...contextContent.matchAll(/^#{2,6}\s+.*$/gm)].map((m) => m[0].trim())); +} + +describe('docs/contributor-standards.md ↔ CONTEXT.md heading parity', () => { + test('everyContextHeadingNamedByTheStandardsDocExistsInContextMd', () => { + const refs = extractContextHeadingRefs(readStandardsDoc()); + const actual = actualHeadings(fs.readFileSync(CONTEXT_MD, 'utf-8')); + + assert.ok(refs.length > 0, 'the standards doc must name at least one CONTEXT.md heading'); + const missing = refs.filter((r) => !actual.has(r)); + assert.deepEqual( + missing, + [], + `docs/contributor-standards.md directs contributors to heading(s) that do not exist in ` + + `CONTEXT.md: ${JSON.stringify(missing)}. Fix the standards doc (or add the heading).` + ); + }); + + test('contextMdStillHasTheGlossaryHeadingTheStandardsDocNamed', () => { + const actual = actualHeadings(fs.readFileSync(CONTEXT_MD, 'utf-8')); + assert.ok( + actual.has('## Glossary — Domain modules and seams'), + 'the glossary heading is the one the standards doc points Module authors at' + ); + }); + + // Negative space for the extractor itself. The RED run of this suite flagged + // `## Decision`, `## Consequences` and `## Standards followed` — all headings the doc + // attributes to an ADR body or a PR body, not to CONTEXT.md. A doc-wide extractor would + // demand CONTEXT.md sprout headings that do not belong to it. + test('doesNotTreatAdrOrPrBodyHeadingsAsContextMdClaims', () => { + const refs = extractContextHeadingRefs(readStandardsDoc()); + for (const foreign of ['## Decision', '## Consequences', '## Standards followed']) { + assert.ok( + !refs.includes(foreign), + `${foreign} describes another document's structure and must not be read as a CONTEXT.md claim` + ); + } + }); + + test('matchesAHeadingReferenceRegardlessOfLineEndingStyle', () => { + const crlf = '## Test rules and lint\r\n\r\n### Emitted Artifact Provenance\r\n'; + const found = actualHeadings(crlf); + assert.ok(found.has('## Test rules and lint'), 'a CRLF checkout must not defeat the match'); + assert.ok(found.has('### Emitted Artifact Provenance')); + }); +}); + +/** + * The Emitted Artifact Provenance naming deliverable (#2721). Without these, the artifact + * family that half the open PR queue collides on still has no name a contributor can look + * up — which ADR-2719 identifies as a direct cause of the problem. + */ +describe('CONTEXT.md names the emitted-artifact family', () => { + test('emittedAttributionRulesetIsUnderTheTestRulesAndLintSection', () => { + const body = sectionBody(fs.readFileSync(CONTEXT_MD, 'utf-8'), '## Test rules and lint'); + assert.ok(body, 'CONTEXT.md must have a `## Test rules and lint` section'); + assert.ok( + body.includes('RULESET.EMITTED_ATTRIBUTION='), + 'RULESET.EMITTED_ATTRIBUTION must be a sibling of the other test rules, not floating elsewhere' + ); + }); + + test('emittedArtifactProvenanceIsRegisteredInTheGlossary', () => { + const body = sectionBody( + fs.readFileSync(CONTEXT_MD, 'utf-8'), + '## Glossary — Domain modules and seams' + ); + assert.ok(body, 'CONTEXT.md must have the glossary section'); + assert.ok( + body.includes('### Emitted Artifact Provenance'), + 'the emitted-artifact family must be registered in the glossary' + ); + }); + + test('emittedArtifactProvenanceIsAConceptNotAModule', () => { + const headings = actualHeadings(fs.readFileSync(CONTEXT_MD, 'utf-8')); + assert.ok(headings.has('### Emitted Artifact Provenance')); + assert.ok( + !headings.has('### Emitted Artifact Provenance Module'), + 'it ships nothing, so it takes no `Module` suffix — follows the `### Resolution Provenance` precedent' + ); + }); +}); + describe('CONTRIBUTING.md links contributor-standards.md', () => { test('CONTRIBUTING.md contains link to contributor-standards.md', () => { let contributing; diff --git a/tests/git-merge-regen-driver.test.cjs b/tests/git-merge-regen-driver.test.cjs new file mode 100644 index 000000000..6a0acbade --- /dev/null +++ b/tests/git-merge-regen-driver.test.cjs @@ -0,0 +1,879 @@ + +/** + * Tests for the `gsd-regen` merge driver (#2721, epic #2719, ADR-2719 Phase 1). + * + * Design + behavior table: .gsd/phase/feat-2721-merge-driver-and-regen-derived/40-design.md + * Test matrix: .gsd/phase/feat-2721-merge-driver-and-regen-derived/50-test-matrix.md + */ + +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const cp = require('node:child_process'); + +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const DRIVER_PATH = path.join(REPO_ROOT, 'scripts', 'git-merge-regen-driver.cjs'); + +const { + ACTION, + REASON, + GITDIR_SOURCE, + NOTICE_WINDOW_MS, + resolveGitDir, + resolveAndRecord, + planInstall, +} = require(DRIVER_PATH); + +// --- helpers --------------------------------------------------------------- + +const GIT_TIMEOUT_MS = 30_000; + +function git(cwd, args) { + return cp.spawnSync('git', args, { + cwd, + encoding: 'utf8', + timeout: GIT_TIMEOUT_MS, + env: { ...process.env, GIT_CONFIG_NOSYSTEM: '1', HOME: cwd, GIT_TERMINAL_PROMPT: '0' }, + }); +} + +/** + * `git check-attr -- ` in the real repo. Returns the attribute value. + * + * `-c safe.directory=…` is required, not incidental: the test container checks this repo + * out at a path its user does not own, and git then refuses every command with + * "detected dubious ownership in repository at '/work'". `check-attr` is a pure read of + * `.gitattributes` — no hooks, no filters — so scoping the exemption to this one + * invocation is safe. It is deliberately NOT applied to the driver's own production + * `git config` calls, which run in the user's own clone and should keep the protection. + * Forward slashes unconditionally: git wants them in this value on every platform. + */ +function checkAttr(attr, relPath) { + const safeRoot = REPO_ROOT.replace(/\\/g, '/'); + const r = cp.spawnSync('git', ['-c', `safe.directory=${safeRoot}`, 'check-attr', attr, '--', relPath], { + cwd: REPO_ROOT, + encoding: 'utf8', + timeout: GIT_TIMEOUT_MS, + }); + assert.equal(r.status, 0, `git check-attr failed: ${r.stderr}`); + // Format: ": : " — the value is everything after the last ": ". + const line = String(r.stdout).trim(); + const idx = line.lastIndexOf(': '); + return idx === -1 ? '' : line.slice(idx + 2); +} + +/** Replace an fs method with a throwing stub for the duration of `fn`, then restore. */ +function withFsFailure(method, fn) { + const original = fs[method]; + fs[method] = () => { + throw Object.assign(new Error('injected'), { code: 'EACCES' }); + }; + try { + return fn(); + } finally { + fs[method] = original; + } +} + +/** Write an "ours" temp file and return its path — production always receives a real %A. */ +function writeOurs(dir, content = 'ours-content\n') { + const p = path.join(dir, '.merge_file_OURS'); + fs.writeFileSync(p, content); + return p; +} + +/** + * The argv shape git actually supplies under the registered command: [%O, %A, %B, %L]. + * `%P` is deliberately NOT registered — see planInstall's comment on shell interpolation. + * `extra` lets one test prove a stray 5th entry (an old registration) is ignored. + */ +function gitArgv(dir, ...extra) { + const o = path.join(dir, '.merge_file_ANC'); + const b = path.join(dir, '.merge_file_THEIRS'); + fs.writeFileSync(o, ''); + fs.writeFileSync(b, 'theirs-content\n'); + return [o, writeOurs(dir), b, '7', ...extra]; +} + +function readMarker(gitDir) { + return JSON.parse(fs.readFileSync(path.join(gitDir, 'gsd-regen-pending.json'), 'utf8')); +} + +function seedMarker(gitDir, value) { + fs.mkdirSync(gitDir, { recursive: true }); + fs.writeFileSync( + path.join(gitDir, 'gsd-regen-pending.json'), + typeof value === 'string' ? value : JSON.stringify(value), + ); +} + +const GOLDEN_DIR = path.join(REPO_ROOT, 'tests', 'fixtures', 'golden-install-parity'); +const INSTALL_TREE_DIR = path.join(REPO_ROOT, 'tests', 'fixtures', 'install-tree'); + +function jsonFixturesIn(dir) { + return fs + .readdirSync(dir) + .filter((f) => f.endsWith('.json')) + .sort(); +} + +// --- .gitattributes scoping (rows 1-10) ------------------------------------ + +describe('.gitattributes declares the gsd-regen driver for exactly the churning artifacts', () => { + test('goldenParityFixturesDeclareGsdRegenMergeDriver', () => { + assert.equal( + checkAttr('merge', 'tests/fixtures/golden-install-parity/claude.json'), + 'gsd-regen', + ); + }); + + test('allNineteenGoldenFixturesDeclareTheDriver', () => { + const fixtures = jsonFixturesIn(GOLDEN_DIR); + assert.ok(fixtures.length > 0, 'expected golden-install-parity fixtures to exist'); + for (const f of fixtures) { + assert.equal( + checkAttr('merge', `tests/fixtures/golden-install-parity/${f}`), + 'gsd-regen', + `${f} must declare merge=gsd-regen`, + ); + } + }); + + test('workflowSizeBaselineDeclaresTheDriver', () => { + assert.equal(checkAttr('merge', 'tests/workflow-size-baseline.json'), 'gsd-regen'); + }); + + test('agentSizeBaselineDeclaresTheDriver', () => { + assert.equal(checkAttr('merge', 'tests/agent-size-baseline.json'), 'gsd-regen'); + }); + + // NEGATIVE SPACE — ADR-2719 §7 keeps install-tree committed precisely so that + // "the installer stopped shipping X" stays an absolute failure. Capturing it + // with the driver would silently convert that absolute into an auto-resolve. + test('installTreeFixturesAreNotCapturedByTheDriver', () => { + assert.equal( + checkAttr('merge', 'tests/fixtures/install-tree/claude.json'), + 'unspecified', + 'install-tree must keep normal merge semantics (ADR-2719 §7)', + ); + }); + + test('noInstallTreeFixtureIsCapturedByTheDriver', () => { + const fixtures = jsonFixturesIn(INSTALL_TREE_DIR); + assert.ok(fixtures.length > 0, 'expected install-tree fixtures to exist'); + for (const f of fixtures) { + assert.equal( + checkAttr('merge', `tests/fixtures/install-tree/${f}`), + 'unspecified', + `${f} must NOT be captured by the driver`, + ); + } + }); + + test('installTreeFixturesAreNotMarkedLinguistGenerated', () => { + assert.equal( + checkAttr('linguist-generated', 'tests/fixtures/install-tree/claude.json'), + 'unspecified', + 'ADR-2719 §7 keeps install-tree because its diffs are readable', + ); + }); + + test('goldenParityFixturesAreMarkedLinguistGenerated', () => { + assert.equal( + checkAttr('linguist-generated', 'tests/fixtures/golden-install-parity/claude.json'), + 'true', + ); + }); + + test('sizeBaselineDeclarationsAreExactPathsNotAGlob', () => { + assert.equal( + checkAttr('merge', 'tests/other-size-baseline.json'), + 'unspecified', + 'the two baselines are declared by exact path, never by a tests/*-size-baseline.json glob', + ); + }); + + test('driverPatternDoesNotCrossADirectorySeparator', () => { + assert.equal( + checkAttr('merge', 'tests/fixtures/golden-install-parity/sub/nested.json'), + 'unspecified', + ); + }); +}); + +// --- resolveGitDir (rows 11-19) -------------------------------------------- + +describe('resolveGitDir', () => { + test('resolvesGitDirWhenDotGitIsADirectory', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + fs.mkdirSync(path.join(dir, '.git')); + + const r = resolveGitDir(dir); + assert.equal(r.source, GITDIR_SOURCE.DIRECTORY); + assert.equal(r.gitDir, path.join(dir, '.git')); + }); + + test('resolvesGitDirFromAWorktreePointerFile', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const target = path.join(dir, 'real-gitdir'); + fs.mkdirSync(target); + fs.writeFileSync(path.join(dir, '.git'), `gitdir: ${target}\n`); + + const r = resolveGitDir(dir); + assert.equal(r.source, GITDIR_SOURCE.GITFILE); + assert.equal(r.gitDir, target); + }); + + test('resolvesARelativeWorktreePointerAgainstCwd', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + fs.mkdirSync(path.join(dir, 'nested')); + fs.writeFileSync(path.join(dir, '.git'), 'gitdir: ./nested\n'); + + const r = resolveGitDir(dir); + assert.equal(r.source, GITDIR_SOURCE.GITFILE); + assert.equal(r.gitDir, path.resolve(dir, './nested')); + }); + + test('parsesAWorktreePointerWrittenWithCrlf', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const target = path.join(dir, 'real-gitdir'); + fs.mkdirSync(target); + fs.writeFileSync(path.join(dir, '.git'), `gitdir: ${target}\r\n`); + + const r = resolveGitDir(dir); + assert.equal(r.source, GITDIR_SOURCE.GITFILE); + assert.equal(r.gitDir, target, 'a trailing CR must not become part of the path'); + }); + + test('parsesAWorktreePointerWithNoSpaceAfterTheColon', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const target = path.join(dir, 'real-gitdir'); + fs.mkdirSync(target); + fs.writeFileSync(path.join(dir, '.git'), `gitdir:${target}\n`); + + assert.equal(resolveGitDir(dir).gitDir, target); + }); + + test('treatsAnEmptyPointerFileAsUnresolved', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + fs.writeFileSync(path.join(dir, '.git'), ''); + + const r = resolveGitDir(dir); + assert.equal(r.source, GITDIR_SOURCE.UNRESOLVED); + assert.equal(r.gitDir, null); + }); + + test('treatsAGarbagePointerFileAsUnresolved', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + fs.writeFileSync(path.join(dir, '.git'), 'this is not a gitdir pointer\n'); + + assert.equal(resolveGitDir(dir).source, GITDIR_SOURCE.UNRESOLVED); + }); + + test('treatsAMissingDotGitAsUnresolved', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + + const r = resolveGitDir(dir); + assert.equal(r.source, GITDIR_SOURCE.UNRESOLVED); + assert.equal(r.gitDir, null); + }); + + test('treatsAnUnreadablePointerFileAsUnresolved', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + fs.writeFileSync(path.join(dir, '.git'), 'gitdir: somewhere\n'); + + const r = withFsFailure('readFileSync', () => resolveGitDir(dir)); + assert.equal(r.source, GITDIR_SOURCE.UNRESOLVED, 'must degrade, never throw'); + }); +}); + +// --- resolveAndRecord: happy & boundary (rows 20-29) ------------------------- + +describe('resolveAndRecord — resolution and the notice window', () => { + test('acceptsOursAndNoticesOnTheFirstResolution', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const r = resolveAndRecord({ argv: gitArgv(dir), gitDir, now: 1_000_000 }); + assert.equal(r.action, ACTION.ACCEPT_OURS); + assert.equal(r.reason, REASON.OK_RESOLVED); + assert.equal(r.exitCode, 0); + assert.equal(r.notice, true); + assert.equal(r.pendingCount, 1); + }); + + test('suppressesTheNoticeForASubsequentPathInTheSameOperation', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + seedMarker(gitDir, { startedAt: 999_999, count: 1 }); + + const r = resolveAndRecord({ argv: gitArgv(dir), gitDir, now: 1_000_000 }); + assert.equal(r.notice, false); + assert.equal(r.pendingCount, 2, 'the second conflicted path in the same operation'); + }); + + test('treatsAMarkerJustInsideTheWindowAsTheSameOperation', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + const now = 5_000_000; + seedMarker(gitDir, { startedAt: now - (NOTICE_WINDOW_MS - 1), count: 1 }); + + assert.equal(resolveAndRecord({ argv: gitArgv(dir), gitDir, now }).notice, false); + }); + + test('treatsAMarkerAtExactlyTheWindowAsTheSameOperation', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + const now = 5_000_000; + seedMarker(gitDir, { startedAt: now - NOTICE_WINDOW_MS, count: 1 }); + + assert.equal( + resolveAndRecord({ argv: gitArgv(dir), gitDir, now }).notice, + false, + 'the window is inclusive at the limit', + ); + }); + + test('resetsAndRenoticesForAMarkerJustOutsideTheWindow', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + const now = 5_000_000; + seedMarker(gitDir, { startedAt: now - (NOTICE_WINDOW_MS + 1), count: 7 }); + + const r = resolveAndRecord({ argv: gitArgv(dir), gitDir, now }); + assert.equal(r.notice, true, 'a later operation must not inherit the previous silence'); + assert.equal(r.pendingCount, 1, 'the stale count must reset, not accumulate'); + }); + + test('noticesOnceAcrossAllTwentyArtifactResolutions', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const notices = []; + for (let i = 0; i < 20; i += 1) { + const r = resolveAndRecord({ argv: gitArgv(dir), gitDir, now: 2_000_000 + i }); + notices.push(r.notice); + } + assert.equal(notices.filter(Boolean).length, 1, 'exactly one notice for the whole operation'); + assert.equal(readMarker(gitDir).count, 20, 'this repo conflicts on 20 artifacts'); + }); + + test('countsEveryResolutionInTheOperation', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + resolveAndRecord({ argv: gitArgv(dir), gitDir, now: 3_000_000 }); + const r = resolveAndRecord({ argv: gitArgv(dir), gitDir, now: 3_000_001 }); + assert.equal(r.pendingCount, 2); + }); + + test('resolvesAnAddAddConflictWhereTheAncestorIsEmpty', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const argv = gitArgv(dir); + fs.writeFileSync(argv[0], ''); // %O is a 0-byte file in the add/add case + const r = resolveAndRecord({ argv, gitDir, now: 4_000_000 }); + assert.equal(r.action, ACTION.ACCEPT_OURS); + assert.equal(r.exitCode, 0); + }); + + test('resolvesOnTheMinimumThreeArgumentForm', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const [o, a, b] = gitArgv(dir); + const r = resolveAndRecord({ argv: [o, a, b], gitDir, now: 4_100_000 }); + assert.equal(r.action, ACTION.ACCEPT_OURS); + assert.equal(r.pendingCount, 1); + }); + + // An old registration (or a hand-edited .git/config) may still pass %P. It must be + // inert data, never consumed — the driver's contract does not depend on it. + test('ignoresAStrayFifthArgumentFromAnOldRegistration', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const r = resolveAndRecord({ + argv: gitArgv(dir, 'tests/workflow-size-baseline.json'), + gitDir, + now: 4_200_000, + }); + assert.equal(r.action, ACTION.ACCEPT_OURS); + assert.equal(r.pendingCount, 1); + assert.equal(r.realPath, undefined, 'no path is read from argv at all'); + }); +}); + +// --- resolveAndRecord: negative & hostile (rows 30-46) ----------------------- + +describe('resolveAndRecord — degrades toward a normal conflict, never toward a wrong resolution', () => { + const badArgvCases = [ + ['declinesRatherThanGuessingWhenArgvIsTooShort', (dir) => gitArgv(dir).slice(0, 2)], + ['declinesOnEmptyArgv', () => []], + ]; + for (const [name, build] of badArgvCases) { + test(name, (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const r = resolveAndRecord({ argv: build(dir), gitDir, now: 6_000_000 }); + assert.equal(r.action, ACTION.DECLINE); + assert.equal(r.reason, REASON.FAIL_BAD_ARGV); + assert.equal(r.exitCode, 1, 'a non-zero exit gives git a normal conflict — today’s behavior'); + }); + } + + test('declinesWhenTheOursSideIsMissing', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const [ancestor, , theirs] = gitArgv(dir); + const neverWritten = path.join(dir, '.merge_file_NEVER_WRITTEN'); + const r = resolveAndRecord({ + argv: [ancestor, neverWritten, theirs, '7', 'tests/workflow-size-baseline.json'], + gitDir, + now: 6_100_000, + }); + assert.equal(r.action, ACTION.DECLINE); + assert.equal(r.reason, REASON.FAIL_OURS_UNREADABLE); + assert.equal(r.exitCode, 1); + }); + + const blankOursCases = [ + ['declinesOnAnEmptyOursPath', ''], + ['declinesOnAWhitespaceOnlyOursPath', ' '], + ]; + for (const [name, oursPath] of blankOursCases) { + test(name, (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const [o, , b] = gitArgv(dir); + const r = resolveAndRecord({ argv: [o, oursPath, b], gitDir, now: 6_200_000 }); + assert.equal(r.action, ACTION.DECLINE); + assert.equal(r.reason, REASON.FAIL_BAD_ARGV); + }); + } + + // Valid JSON that is not a usable marker object. Each must be treated as absent + // (reset + notice) rather than throwing or being read as state. + const hostileMarkers = [ + ['treatsANumericMarkerAsAbsent', '0'], + ['treatsAStringMarkerAsAbsent', '"str"'], + ['treatsAnArrayMarkerAsAbsent', '[]'], + ['treatsANullMarkerAsAbsent', 'null'], + ['treatsABooleanMarkerAsAbsent', 'true'], + ['treatsAnEmptyMarkerFileAsAbsent', ''], + ['treatsACorruptMarkerAsAbsent', '{not json at all'], + ['treatsANonNumericStartedAtAsAbsent', '{"startedAt":"yesterday","count":1}'], + ['treatsANonFiniteStartedAtAsAbsent', '{"startedAt":1e999,"count":1}'], + ['treatsANonNumericCountAsAbsent', '{"startedAt":1,"count":"three"}'], + ['treatsANonFiniteCountAsAbsent', '{"startedAt":1,"count":1e999}'], + ['treatsANegativeCountAsAbsent', '{"startedAt":1,"count":-5}'], + ['treatsAMissingCountAsAbsent', '{"startedAt":1}'], + ]; + for (const [name, raw] of hostileMarkers) { + test(name, (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + seedMarker(gitDir, raw); + + const r = resolveAndRecord({ argv: gitArgv(dir), gitDir, now: 7_000_000 }); + assert.equal(r.action, ACTION.ACCEPT_OURS, 'a bad marker must never block the merge'); + assert.equal(r.notice, true); + assert.equal(r.pendingCount, 1, 'an unusable marker resets rather than accumulating'); + }); + } + + test('stillResolvesWhenTheMarkerCannotBeWritten', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + const gitDir = path.join(dir, '.git'); + fs.mkdirSync(gitDir); + + const argv = gitArgv(dir); + const r = withFsFailure('writeFileSync', () => + resolveAndRecord({ argv, gitDir, now: 8_000_000 }), + ); + assert.equal(r.action, ACTION.ACCEPT_OURS); + assert.equal(r.exitCode, 0, 'a diagnostic must never fail a merge'); + }); + + test('resolvesAndNoticesEveryTimeWhenTheGitDirIsUnknown', (t) => { + const dir = createTempDir('gsd-regen-'); + t.after(() => cleanup(dir)); + + const first = resolveAndRecord({ argv: gitArgv(dir), gitDir: null, now: 9_000_000 }); + const second = resolveAndRecord({ argv: gitArgv(dir), gitDir: null, now: 9_000_001 }); + assert.equal(first.action, ACTION.ACCEPT_OURS); + assert.equal(first.exitCode, 0); + assert.equal(first.notice, true); + assert.equal(second.notice, true, 'without a marker there is nothing to dedupe against'); + }); +}); + +// --- planInstall (rows 47-50) ---------------------------------------------- + +describe('planInstall', () => { + test('plansBothMergeDriverConfigEntries', () => { + const keys = planInstall({ repoRoot: '/repo' }).entries.map((e) => e.key); + assert.deepEqual(keys, ['merge.gsd-regen.name', 'merge.gsd-regen.driver']); + }); + + test('normalizesTheDriverCommandToForwardSlashesOnEveryPlatform', () => { + const { entries } = planInstall({ repoRoot: 'C:\\Users\\dev\\gsd-core' }); + const driver = entries.find((e) => e.key === 'merge.gsd-regen.driver').value; + assert.ok(!driver.includes('\\'), `driver command must contain no backslash: ${driver}`); + }); + + // `driverCommandPassesEveryPlaceholderGitProvides` lived here and asserted that the + // command carried %P. That was the vulnerable behaviour, so the test has been deleted + // rather than relaxed — a test that asserts the old, now-wrong behaviour is worse than + // no test. Its useful half (the four git-generated placeholders ARE passed) is folded + // into `registeredDriverCommandNeverPassesThePlaceholderForTheFilePath`, which asserts + // both directions in one place. + + test('plansIdenticalEntriesOnRepeatedInvocation', () => { + assert.deepEqual(planInstall({ repoRoot: '/repo' }), planInstall({ repoRoot: '/repo' })); + }); +}); + +// --- CLI dispatch (rows 62-73) --------------------------------------------- +// CONTRIBUTING.md → "QA Matrix Requirements" / "CLI and command routing" requires a +// negative-input matrix for any command dispatcher: unknown subcommands, duplicate and +// conflicting flags, plus assertions on exit status and the absence of a stack trace. + +describe('CLI dispatch', () => { + /** Run the driver as a real child process — its output never touches this test's stdout. */ + function runCli(args, cwd = REPO_ROOT) { + return cp.spawnSync(process.execPath, [DRIVER_PATH, ...args], { + cwd, + encoding: 'utf8', + timeout: GIT_TIMEOUT_MS, + }); + } + + /** + * Swap process.stdout.write for the duration of `fn`. Same monkeypatch-and-restore-in- + * finally shape as withFsFailure — the run* functions print, and leaking that into the + * runner's stream is how a reporter ends up parsing a driver banner as a test result. + */ + function withStdoutSilenced(fn) { + const original = process.stdout.write; + process.stdout.write = () => true; + try { + return fn(); + } finally { + process.stdout.write = original; + } + } + + /** A stack frame looks like a line beginning with whitespace + "at ". */ + function hasStackTrace(text) { + return /^\s+at\s/m.test(String(text)); + } + + const rejectedInvocations = [ + ['rejectsAnUnknownFlag', ['--bogus']], + ['rejectsTwoConflictingFlags', ['--install', '--status']], + ['rejectsADuplicatedFlag', ['--install', '--install']], + ['rejectsAFlagCombinedWithAPositionalArgument', ['--status', 'extra']], + ]; + for (const [name, args] of rejectedInvocations) { + test(name, () => { + const r = runCli(args); + assert.equal(r.status, 2, `${args.join(' ')} must exit 2, got ${r.status}: ${r.stderr}`); + assert.ok( + !hasStackTrace(r.stderr), + `usage errors must not print a stack trace, got: ${r.stderr}`, + ); + }); + } + + test('driverModeWithNoArgumentsDeclinesRatherThanCrashing', () => { + const r = runCli([]); + assert.equal(r.status, 1, 'too-few-args declines, which git reads as a normal conflict'); + assert.ok(!hasStackTrace(r.stderr), `expected no stack trace, got: ${r.stderr}`); + }); + + test('statusEmitsParseableJsonWithTheDocumentedShape', () => { + const r = runCli(['--status']); + assert.equal(r.status, 0); + const report = JSON.parse(r.stdout); + assert.equal(typeof report.registered, 'boolean'); + assert.equal(typeof report.pendingCount, 'number'); + }); + + /** A scratch repo so registration never touches the developer's own .git/config. */ + function scratchRepo(t) { + const dir = createTempDir('gsd-regen-cli-'); + t.after(() => cleanup(dir)); + git(dir, ['init', '-q', '.']); + return dir; + } + + test('installRegistersBothConfigEntriesAndStatusReportsIt', (t) => { + const dir = scratchRepo(t); + const { statusOf, runInstall } = require(DRIVER_PATH); + + assert.equal(statusOf({ repoRoot: dir }).registered, false, 'precondition: not registered'); + assert.equal(withStdoutSilenced(() => runInstall({ repoRoot: dir })), 0); + assert.equal(statusOf({ repoRoot: dir }).registered, true); + + const driver = git(dir, ['config', '--get', 'merge.gsd-regen.driver']).stdout.trim(); + assert.equal(driver, planInstall({ repoRoot: dir }).entries[1].value); + }); + + test('installIsIdempotent', (t) => { + const dir = scratchRepo(t); + const { statusOf, runInstall } = require(DRIVER_PATH); + + withStdoutSilenced(() => runInstall({ repoRoot: dir })); + const first = git(dir, ['config', '--get-all', 'merge.gsd-regen.driver']).stdout; + assert.equal(withStdoutSilenced(() => runInstall({ repoRoot: dir })), 0); + const second = git(dir, ['config', '--get-all', 'merge.gsd-regen.driver']).stdout; + + assert.equal(second, first, 'a second install must not append a duplicate value'); + assert.equal(statusOf({ repoRoot: dir }).registered, true); + }); + + test('uninstallRemovesTheRegistration', (t) => { + const dir = scratchRepo(t); + const { statusOf, runInstall, runUninstall } = require(DRIVER_PATH); + + withStdoutSilenced(() => runInstall({ repoRoot: dir })); + assert.equal(withStdoutSilenced(() => runUninstall({ repoRoot: dir })), 0); + assert.equal(statusOf({ repoRoot: dir }).registered, false); + }); + + test('uninstallOnACleanRepoSucceedsRatherThanFailing', (t) => { + const dir = scratchRepo(t); + const { runUninstall, statusOf } = require(DRIVER_PATH); + + assert.equal(withStdoutSilenced(() => runUninstall({ repoRoot: dir })), 0); + assert.equal(statusOf({ repoRoot: dir }).registered, false); + }); + + /** + * REGRESSION — arbitrary command execution via `%P` (isolated security review, #2721). + * + * Git does not invoke a merge driver with an argv array: it substitutes the placeholders + * textually into the configured string and runs the whole thing through a shell. Quoting + * does not save you — `$(…)` executes inside POSIX double quotes. `%O`/`%A`/`%B` are + * git-generated temp names and `%L` is an integer, but `%P` is the file's own path, which + * any contributor names freely. Registering `"%P"` let a branch that renamed a covered + * fixture to `evil$(touch PWNED).json` run that command on the machine of every maintainer + * who merged it — and the merge still reported success, so nothing looked wrong. + * + * The structural assertion is the real guard: it is platform-independent and fails the + * moment someone re-adds the placeholder. + */ + test('registeredDriverCommandNeverPassesThePlaceholderForTheFilePath', () => { + const { entries } = planInstall({ repoRoot: REPO_ROOT }); + const driver = entries.find((e) => e.key === 'merge.gsd-regen.driver').value; + assert.ok( + !driver.includes('%P'), + 'git shell-interpolates %P — passing it is arbitrary command execution. Do not re-add it.', + ); + for (const safe of ['%O', '%A', '%B', '%L']) { + assert.ok(driver.includes(safe), `${safe} is git-generated and must still be passed`); + } + }); + + test('aFilenameCarryingShellSubstitutionCannotExecuteDuringAMerge', (t) => { + const dir = createTempDir('gsd-regen-inject-'); + t.after(() => cleanup(dir)); + + git(dir, ['init', '-q', '.']); + git(dir, ['config', 'user.email', 'test@example.com']); + git(dir, ['config', 'user.name', 'test']); + // Register the REAL production command string — a hand-rolled one would not regress. + for (const { key, value } of planInstall({ repoRoot: REPO_ROOT }).entries) { + git(dir, ['config', key, value]); + } + + fs.writeFileSync(path.join(dir, '.gitattributes'), 'evil*.json merge=gsd-regen\n'); + // Written with fs, so this shell never expands it — the payload is the literal name. + const evil = 'evil$(touch PWNED_SENTINEL).json'; + const sentinel = path.join(dir, 'PWNED_SENTINEL'); + const write = (v) => fs.writeFileSync(path.join(dir, evil), `{"v":${v}}\n`); + + write(0); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'base']); + const base = git(dir, ['rev-parse', 'HEAD']).stdout.trim(); + + git(dir, ['checkout', '-qb', 'ours']); + write(1); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'ours']); + + git(dir, ['checkout', '-q', base]); + git(dir, ['checkout', '-qb', 'theirs']); + write(2); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'theirs']); + + git(dir, ['checkout', '-q', 'ours']); + git(dir, ['merge', 'theirs', '-m', 'merge']); + + assert.equal( + fs.existsSync(sentinel), + false, + 'a filename containing $(...) must never execute — see the regression note above', + ); + }); +}); + +// --- real-git end-to-end (rows 51-55) — #2721 AC1 -------------------------- + +describe('gsd-regen driver under real git operations', () => { + /** + * Build a repo whose `derived.json` is a stand-in for a golden fixture: both + * branches edit a DIFFERENT workflow file and both regenerate the artifact, so + * the artifact conflicts while the sources do not. That is exactly the #2721 + * scenario (7 of 7 conflicting PRs collide on the identical artifact set). + */ + function buildScenario(t, { register }) { + const dir = createTempDir('gsd-regen-e2e-'); + t.after(() => cleanup(dir)); + + git(dir, ['init', '-q', '.']); + git(dir, ['config', 'user.email', 'test@example.com']); + git(dir, ['config', 'user.name', 'test']); + if (register) { + // Register the REAL production entries. A hand-rolled command string would let the + // end-to-end tests keep passing while planInstall drifted — and would have kept + // registering the %P form these tests exist to prove is gone. + for (const { key, value } of planInstall({ repoRoot: REPO_ROOT }).entries) { + git(dir, ['config', key, value]); + } + } + + fs.mkdirSync(path.join(dir, 'workflows')); + fs.writeFileSync(path.join(dir, '.gitattributes'), 'derived.json merge=gsd-regen\n'); + fs.writeFileSync(path.join(dir, 'workflows', 'a.md'), 'A0\n'); + fs.writeFileSync(path.join(dir, 'workflows', 'b.md'), 'B0\n'); + fs.writeFileSync(path.join(dir, 'derived.json'), '{"a":"A0","b":"B0"}\n'); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'base']); + const base = git(dir, ['rev-parse', 'HEAD']).stdout.trim(); + + git(dir, ['checkout', '-qb', 'ours']); + fs.writeFileSync(path.join(dir, 'workflows', 'a.md'), 'A1\n'); + fs.writeFileSync(path.join(dir, 'derived.json'), '{"a":"A1","b":"B0"}\n'); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'ours edits workflow a']); + + git(dir, ['checkout', '-q', base]); + git(dir, ['checkout', '-qb', 'theirs']); + fs.writeFileSync(path.join(dir, 'workflows', 'b.md'), 'B1\n'); + fs.writeFileSync(path.join(dir, 'derived.json'), '{"a":"A0","b":"B1"}\n'); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'theirs edits workflow b']); + + git(dir, ['checkout', '-q', 'ours']); + return dir; + } + + test('twoBranchesEditingDifferentWorkflowsMergeCleanlyWithTheDriver', (t) => { + const dir = buildScenario(t, { register: true }); + + const merge = git(dir, ['merge', 'theirs', '-m', 'merge']); + assert.equal(merge.status, 0, `merge should succeed: ${merge.stdout}${merge.stderr}`); + assert.equal(git(dir, ['ls-files', '-u']).stdout.trim(), '', 'no unmerged index entries'); + assert.ok( + !fs.readFileSync(path.join(dir, 'derived.json'), 'utf8').includes('<<<<<<<'), + 'the artifact must carry no conflict markers', + ); + assert.equal( + fs.readFileSync(path.join(dir, 'workflows', 'b.md'), 'utf8'), + 'B1\n', + 'the source side of the merge must still be applied normally', + ); + }); + + // Control: proves the test observes the DRIVER, not a trivially-mergeable artifact. + test('theSameTwoBranchesConflictWithoutTheDriver', (t) => { + const dir = buildScenario(t, { register: false }); + + const merge = git(dir, ['merge', 'theirs', '-m', 'merge']); + assert.notEqual(merge.status, 0, 'without the driver this scenario must conflict'); + assert.notEqual(git(dir, ['ls-files', '-u']).stdout.trim(), ''); + }); + + test('resolvesUnderRebaseNotJustMerge', (t) => { + const dir = buildScenario(t, { register: true }); + git(dir, ['checkout', '-q', 'theirs']); + + const rebase = git(dir, ['rebase', 'ours']); + assert.equal(rebase.status, 0, `rebase should succeed: ${rebase.stdout}${rebase.stderr}`); + assert.equal(git(dir, ['ls-files', '-u']).stdout.trim(), ''); + }); + + test('leavesAOneSidedChangeToGitsTrivialMerge', (t) => { + const dir = buildScenario(t, { register: true }); + git(dir, ['checkout', '-q', '-b', 'sideways', 'ours']); + fs.writeFileSync(path.join(dir, 'workflows', 'c.md'), 'C1\n'); + git(dir, ['add', '-A']); + git(dir, ['commit', '-qm', 'unrelated']); + + const merge = git(dir, ['merge', 'ours', '-m', 'merge']); + assert.equal(merge.status, 0); + assert.equal( + fs.readFileSync(path.join(dir, 'derived.json'), 'utf8'), + '{"a":"A1","b":"B0"}\n', + 'a one-sided change is git’s trivial merge — the driver must not be involved', + ); + }); + + test('resolvedArtifactIsExactlyTheOursSide', (t) => { + const dir = buildScenario(t, { register: true }); + const ours = fs.readFileSync(path.join(dir, 'derived.json'), 'utf8'); + + git(dir, ['merge', 'theirs', '-m', 'merge']); + assert.equal( + fs.readFileSync(path.join(dir, 'derived.json'), 'utf8'), + ours, + 'the driver invents nothing — it takes ours verbatim and defers to regen:derived', + ); + }); +});