diff --git a/.changeset/quick-tunas-wander.md b/.changeset/quick-tunas-wander.md new file mode 100644 index 000000000..67e1f790a --- /dev/null +++ b/.changeset/quick-tunas-wander.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 4497 +--- +**`workflow.compact_content` splits now have a real CI guard.** Any workflow spine + `detail/*.md` split is enforced forever: completeness once at split time, disjointness and registration on every PR, and protected content (guardrails, output-format contracts, few-shot examples, security language, machine-parsed headings) that can never leave the spine, moved or not. Ordinary content moves between spine and detail need a `Boundary-Move-Declared` commit trailer naming the spine, mirroring ADR-3942's emitted-drift-ack trailers. The partition rule and the protected-content list live in one place, `docs/PARTITION-RULES.md`. (#4403) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d45e43c00..43ec1bfd4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1110,6 +1110,27 @@ explanation holds, and only you can say which. There is no "which source owns th question underneath it, because there is no shared file for two sources to own — to change an acknowledgment, amend the commit carrying it. +**Splitting a workflow file into a spine + `detail/*.md` parts (ADR-4139, `workflow.compact_content`)** +follows the same trailer idiom for one more case. See `docs/PARTITION-RULES.md` for the +full rule set — the short version: a split moves text, it never restates it, so there is +no drift-parity check to satisfy, only a guard (`tests/compact-content-partition-guard.test.cjs`) +that a moved sentence stay moved and a protected sentence never move at all. When the guard +reports an ordinary (non-protected) line that moved from a spine into its own detail part +without a declaration, add: + +``` +Boundary-Move-Declared: gsd-core/workflows/plan-phase.md — condensed the filesystem-fallback banner into one summary paragraph +``` + +Same range (`git log $(git merge-base HEAD)..HEAD`), same fail-closed behavior on an +uncomputable range, same silent dedupe of identical declarations across a rebase, same hard +error on two conflicting reasons for the same spine — it is a second key space alongside +`Emitted-Drift-Ack-Hash`/`-Growth` above, not a different mechanism. Content on the +protected-content list (guardrails, output-format contracts, few-shot examples the +workflow's own steps depend on, security language, machine-parsed structural headings) has +no trailer escape hatch: it may not leave the spine, moved or not, and the guard fails +regardless of what the trailer says. + `npm run regen:derived` still exists for the artifacts that ARE committed and derived — `sync-manifest-versions`, the ADR index, the capability matrix, the inventory manifest, the registry, and `tests/fixtures/install-tree/*.json` (`npm run gen:install-tree`, the diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index f331486b7..a79fac258 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -214,7 +214,6 @@ "checkpoints.md", "common-bug-patterns.md", "compact-content-gate.md", - "compact-content-protected-content.md", "context-budget.md", "continuation-format.md", "debugger-bug-taxonomy.md", @@ -670,6 +669,13 @@ "update/steps/channel-banner.md", "verify-work/steps/automated-ui-verification.md", "verify-work/steps/mvp-uat-framing.md" + ], + "workflow_detail": [ + "plan-phase/detail/elaboration.md" + ], + "workflow_templates": [ + "discuss-phase/templates/context.md", + "discuss-phase/templates/discussion-log.md" ] } } diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index fb09dc1df..c54f24805 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -273,19 +273,21 @@ Full roster at `gsd-core/workflows/*.md`. Workflows are thin orchestrators that ### Workflow Sub-Files -A workflow may own two kinds of sub-file. Both live under `gsd-core/workflows//` and -neither is separately invocable — the parent workflow reaches them. +A workflow may own four kinds of sub-file. All live under `gsd-core/workflows//` and +none is separately invocable — the parent workflow reaches them. | Subdirectory | What it holds | Manifest family | Roster | |---|---|---|---| | `/steps/*.md` | Gated section bodies extracted by the fragment model (ADR-1671, epic #1671 Phases 6.1–6.3). The parent carries a `section_manifest`-gated stub; `gsd-core/workflows/section-manifest.json` names which step a given invocation reads. | `workflow_steps` | See `docs/INVENTORY-MANIFEST.json` for the authoritative per-file list | | `/modes/*.md` | Progressive-disclosure mode files (#717). The parent dispatches to exactly one; `discuss-phase/modes/` is the canonical example. | `workflow_modes` | `discuss-phase`, `help` | +| `/detail/*.md` | Elaboration content deferred from a workflow spine, read at runtime only when `workflow.compact_content` is `false` (ADR-4139; epic #4139 Phase 2 #4402 established the first example, Phase 3 #4403 added the CI guard). | `workflow_detail` | `plan-phase` | +| `/templates/*.md` | Fill-in template bodies the parent workflow renders at runtime; also referenced as a `FRAGMENT_DIRS` entry in `scripts/lint-response-language-coverage.cjs`. | `workflow_templates` | `discuss-phase` | -Both families are keyed by `//.md` rather than a bare filename, because two -workflows may each own a step of the same name — `families.workflows` uses bare basenames and +All four families are keyed by `//.md` rather than a bare filename, because +two workflows may each own a step of the same name — `families.workflows` uses bare basenames and cannot represent these without collision. -**Adding a step or mode file requires no hand-written row here.** Run +**Adding a step, mode, detail, or template file requires no hand-written row here.** Run `node scripts/gen-inventory-manifest.cjs --write` (after `build:lib`) and the manifest picks it up; `tests/inventory-manifest-sync.test.cjs` fails if you forget. The per-file roster deliberately lives in `docs/INVENTORY-MANIFEST.json` rather than being duplicated in this table — 60 rows that must be @@ -388,7 +390,6 @@ Full roster at `gsd-core/references/*.md`. References are shared knowledge docum | `mvp-concepts.md` | Cross-reference index for the six MVP-related reference files; maps each file to its purpose and which workflow loads it. | | `verify-mvp-mode.md` | UAT framing rules for MVP-mode phases — user-flow-first ordering, deferred technical checks, user-story-format guard. | | `compact-content-gate.md` | Shared compact-content gate (ADR-4139 Decision 3/4) — the `workflow.compact_content` check and detail-file resolution rule every compact-split workflow spine references, stated once. | -| `compact-content-protected-content.md` | Compact-content protected-content list (ADR-4139 Decision 5) — the five categories a workflow spine must never move to a `detail/*.md` part, and the `` sentinel syntax that marks them; drafted here for Phase 3 (#4403) to relocate unchanged. | ### Sketch References diff --git a/docs/PARTITION-RULES.md b/docs/PARTITION-RULES.md new file mode 100644 index 000000000..7e84820dc --- /dev/null +++ b/docs/PARTITION-RULES.md @@ -0,0 +1,106 @@ +# Partition rules for compact-content splits + +(ADR-4139 Decision 5, epic [#4139](https://github.com/open-gsd/gsd-core/issues/4139), +Phase 3 [#4403](https://github.com/open-gsd/gsd-core/issues/4403).) These are the rules a +`workflow.compact_content` split — a workflow file broken into a spine plus one or more +`detail/*.md` parts — must obey, and the CI guard (`tests/compact-content-partition-guard.test.cjs`) +that enforces them. See `docs/adr/4139-compact-content-seam.md` for the full design rationale; +this document is the operational reference for anyone performing a split. + +## The rule + +**A split moves text. It does not restate it.** The spine and its detail parts are pieces of +one document, not two. There is exactly one copy of each sentence, so there is no stale twin +that can exist — which is what replaces the drift-parity check an earlier design of this +feature would have needed forever. + +Rewriting for terseness is permitted **within** one half and is never a way to move a +sentence into both. Where a split cannot be made by moving text alone without breaking the +spine's ability to run the workflow correctly on its own, the correct answer is a different +split point, not a duplicated paragraph. + +## The protected-content list + +Content in this list may never leave a workflow spine during a split — it stays directly in +the eagerly-loaded spine file, never moved to a `detail/*.md` part, regardless of how much it +would shrink the spine: + +1. **Negative instructions and guardrails** — any "do not X" / "never X" instruction that + changes what the orchestrator must refuse to do (e.g. "Never call `ScheduleWakeup`... to + literalize this wait"). +2. **Output-format contracts** — any block defining the literal shape of output another + system consumes: a prompt template handed to a subagent, a JSON/XML schema, a + `` or `` checklist. +3. **Few-shot examples the workflow's own steps depend on** — a worked example whose absence + would leave a later instruction ambiguous (e.g. a ``/`` XML pair a + planner prompt's own rule depends on). +4. **Security and prompt-injection language** — any text establishing a security boundary or + defending against injected instructions. +5. **Machine-parsed structural headings** — a heading or marker another tool locates by exact + text (a `## PLANNING COMPLETE`-style return marker, a `` directive, a + ``/`` boundary). + +### Marking + +A sentinel comment declares protection at authoring time. The guard checks for the +sentinel-wrapped content's continued presence in the spine, never for category membership — +a guard cannot judge prose category on its own, so protection is declared, not inferred: + +```markdown + +… one protected block … + + +… a protected region spanning several blocks … + +``` + +The categories above are authoring guidance for *where* to place a sentinel when splitting a +file — they are never what the automated guard evaluates; only the sentinel-wrapped content's +continued presence in the spine is. + +## The five checks + +The guard discovers registered splits by scanning `gsd-core/workflows/**` for any +`/detail/*.md` path and pairing it with `gsd-core/workflows/.md`. There is no +separate registry to maintain — a pair is registered by existing on disk. + +1. **Completeness — once, at split time.** Fires only on the PR that introduces a new + `/detail/*.md` path (i.e. the PR performing the split). The union of the new spine + and its new detail parts, whitespace-normalized, must contain every non-trivial line the + old spine carried at the merge-base. This is what makes a split reviewable; it never fires + again for that pair afterward. +2. **Disjointness — ongoing.** No non-trivial line may appear in both a spine and any of its + detail parts, checked on every PR against every registered pair regardless of what the PR + touched. This is the invariant that keeps duplication from creeping back in. +3. **Registration — ongoing.** A `/detail/*.md` with no `.md` spine, or a spine + whose prose names a detail path that does not exist on disk, fails and names the orphan. +4. **Protected content — ongoing, and cannot be excused.** For every registered spine a PR's + diff touches, every line that sat inside a `` sentinel at the + merge-base must still be physically present in the spine. Deleting it or moving it into a + detail part both fail — naming the sentinel's first line and, for a move, the destination + path. **No `Boundary-Move-Declared` trailer excuses this one**: protected content is + categorically barred from leaving the spine, not merely required to be declared when it + does. +5. **Boundary moves are declared — ongoing.** For ordinary (non-protected) content: if a + non-trivial line is removed from a registered spine and the identical line appears newly + added in that spine's own detail parts in the same diff, one of the PR's own commits + (`merge-base..HEAD`) must carry: + + ``` + Boundary-Move-Declared: gsd-core/workflows/.md — + ``` + + A missing trailer fails and names the spine and the moved line. This mirrors + [ADR-3942](adr/3942-emitted-drift-ack-commit-trailer.md)'s `Emitted-Drift-Ack-Hash`/`-Growth` + trailers exactly — same `git log $(git merge-base HEAD)..HEAD` range, same + fail-closed behavior when that range is uncomputable (a shallow clone throws, it never + silently reports "no violation"), same de-duplication of identical trailers across a + rebase, same hard error when two commits declare the same spine with different reasons. + See `CONTRIBUTING.md`'s "Editing shipped content" section for the trailer mechanism's + general shape. + +Checks 1 and 4–5 read `merge-base..HEAD`, never `base..HEAD` (two-dot) — the same correction +ADR-3942 made for its own trailer range, for the same reason: a two-dot range would let the +set of commits being checked and the set of files being diffed disagree about what "this PR" +means. diff --git a/docs/README.md b/docs/README.md index 2d4692ac7..090cc32b4 100644 --- a/docs/README.md +++ b/docs/README.md @@ -103,6 +103,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [Capability manifest](reference/capability-manifest.md) — the full `capability.json` schema and validation rules - [`gsd capability` command](reference/gsd-capability-command.md) — install / update / remove / list reference for third-party capabilities - [Workflow fragments](reference/workflow-fragments.md) — in-file `` marker grammar for fragmentizing workflow markdown at emission time +- [Partition rules for compact-content splits](PARTITION-RULES.md) — the protected-content list, sentinel syntax, and the five CI checks a `workflow.compact_content` spine/detail split must obey - [Reviewer Lane Registry](registries/reviewer-registry.md) — generated catalogue of third-party reviewer lanes, with their flags, transport, and install commands --- diff --git a/gsd-core/references/compact-content-protected-content.md b/gsd-core/references/compact-content-protected-content.md deleted file mode 100644 index e7bdefa0b..000000000 --- a/gsd-core/references/compact-content-protected-content.md +++ /dev/null @@ -1,26 +0,0 @@ -# Compact Content — Protected Content List - -(ADR-4139 Decision 5.) Content in this list may never leave a workflow spine during a compact-content split — it stays directly in the eagerly-loaded spine file, never moved to a `detail/*.md` part, regardless of how much it would shrink the spine. - -## The categories - -1. **Negative instructions and guardrails** — any "do not X" / "never X" instruction that changes what the orchestrator must refuse to do (e.g. "Never call `ScheduleWakeup`... to literalize this wait"). -2. **Output-format contracts** — any block defining the literal shape of output another system consumes: a prompt template handed to a subagent, a JSON/XML schema, a `` or `` checklist. -3. **Few-shot examples the workflow's own steps depend on** — a worked example whose absence would leave a later instruction ambiguous (e.g. a ``/`` XML pair a planner prompt's own rule depends on). -4. **Security and prompt-injection language** — any text establishing a security boundary or defending against injected instructions. -5. **Machine-parsed structural headings** — a heading or marker another tool locates by exact text (a `## PLANNING COMPLETE`-style return marker, a `` directive, a ``/`` boundary). - -## Marking - -A sentinel comment declares protection at authoring time — the guard (Phase 3, #4403) checks for the sentinel's continued presence, never for category membership, because a guard cannot judge prose category on its own: - -```markdown - -… one protected block … - - -… a protected region spanning several blocks … - -``` - -The rule is mechanical: a sentinel present in the canonical file at the parent commit must be present in the spine afterward, and every line it covers must be in the spine. The categories above are authoring guidance for *where* to place a sentinel when splitting a file — they are never what an automated guard evaluates; only the sentinel's presence is. diff --git a/scripts/gen-inventory-manifest.cjs b/scripts/gen-inventory-manifest.cjs index 094e97023..ba17bf0c1 100644 --- a/scripts/gen-inventory-manifest.cjs +++ b/scripts/gen-inventory-manifest.cjs @@ -91,6 +91,18 @@ const NESTED_FAMILIES = [ subdir: 'steps', filter: (f) => f.endsWith('.md'), }, + { + name: 'workflow_detail', + root: path.join(ROOT, 'gsd-core', 'workflows'), + subdir: 'detail', + filter: (f) => f.endsWith('.md'), + }, + { + name: 'workflow_templates', + root: path.join(ROOT, 'gsd-core', 'workflows'), + subdir: 'templates', + filter: (f) => f.endsWith('.md'), + }, ]; /** diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs index 157747b9a..33e1a6a3c 100644 --- a/scripts/lint-docs-guard-registration.exempt-baseline.cjs +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -46,6 +46,7 @@ const DOCS_GUARD_EXEMPT_BASELINE = [ 'codebuddy-upgrades.test.cjs', 'commands.test.cjs', 'commit-docs-bypass.test.cjs', + 'compact-content-partition-guard.test.cjs', 'complexity-trigger.test.cjs', 'concurrency-safety.test.cjs', 'cursor-imperative-reference.test.cjs', @@ -135,6 +136,10 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = { 'commit-docs-bypass.test.cjs': [ 'docs/40-design.md', 'docs/CONFIGURATION.md', 'docs/readme.md', 'docs/tracked-var-mentioning', ], + // #4403: cites docs/PARTITION-RULES.md as a "see" pointer in the module docstring + // describing the operational spec this guard implements; the file never reads that + // (or any) docs/ file. + 'compact-content-partition-guard.test.cjs': ['docs/PARTITION-RULES.md'], 'complexity-trigger.test.cjs': ['docs/readme.md'], // #3884: cites docs/CLI-TOOLS.md:736 in an explanatory comment describing // the real `frontmatter get [--field key]` CLI shape; the file diff --git a/scripts/lint-response-language-coverage.cjs b/scripts/lint-response-language-coverage.cjs index 95e590d09..8688689fa 100644 --- a/scripts/lint-response-language-coverage.cjs +++ b/scripts/lint-response-language-coverage.cjs @@ -22,8 +22,10 @@ * explanations" describes the gap rather than closing it. Coverage therefore * requires a narration-class token as well — see `NARRATION_CLASS_RE`. * - * A workflow FRAGMENT (`//.md`, the shape - * the #1671 fragment epic extracts) additionally passes when its parent workflow + * A workflow FRAGMENT (`//.md`, the + * shape the #1671 fragment epic extracts — `detail/` is the fourth such + * subdirectory kind, added by the #4403/ADR-4139 spine+detail split) additionally + * passes when its parent workflow * names that exact fragment path and is itself covered — see * `inheritsParentCoverage`, which proves the inheritance per file instead of * granting it to a directory. @@ -124,7 +126,11 @@ const EXACT_INLINE_DIRECTIVE_WORKFLOWS = new Set([ // failure for a quiet assumption; as it stands, extracting a fragment-of-a- // fragment without pinning it turns the lint RED, which is the correct answer // and names the file to fix. -const FRAGMENT_DIRS = new Set(['modes', 'steps', 'templates']); +// `detail` (ADR-4139 §6 Decision 6 / #4403) is the fourth fragment-directory kind, +// alongside modes/steps/templates from #1671: a spine's `detail/.md` is +// reached the same way -- a `read and execute` stub in the top-level parent -- so +// it inherits coverage through the exact same mechanism, not a parallel one. +const FRAGMENT_DIRS = new Set(['modes', 'steps', 'templates', 'detail']); const DIRECTIVE_ACTION_RE = /\b(?:apply|present|render|respond|translate|use|write|must|should)\b/i; const USER_OUTPUT_RE = /\b(?:explanations?|language|narration|outputs?|prompts?|prose|questions?|templates?|user-facing)\b/i; // The defect #2529 reports is NARRATION, not the question/answer surface: a diff --git a/scripts/run-tests.cjs b/scripts/run-tests.cjs index 092207a38..e39b32aa4 100644 --- a/scripts/run-tests.cjs +++ b/scripts/run-tests.cjs @@ -658,6 +658,47 @@ function packChunks(files, { weightOf, maxWeight, maxChars, fixedOverhead }) { } } +// 2026-09-07 (PR #4497 CI, Windows full test shard 2/3, chunk 3/8): the +// Windows-only budget cut in main() (60 -> 40, 2026-09-06 / PR #4428) still +// was not enough — codex-config.test.cjs (weight 17.87, genuinely measured) +// was packed alongside 39 other files and the chunk still exceeded the 600s +// backstop. Two documented incidents in as many days, at two different +// Windows budget settings, both centered on this one file: it is not a +// "this chunk got unlucky today" case, it is this file's weight being +// disproportionate enough (~45% of the post-cut Windows budget alone) that +// ANY companion files sharing its chunk are gambling with the remaining +// headroom — and per .github/workflows/test.yml's own note on this lane, +// "adding one test file reshuffled 115 of 268 unit files between shards", so +// which files end up as that gamble's companions is not something a future +// PR can predict or control. +// +// Isolating it into its own chunk, unconditionally, on every platform, +// removes the gamble at its source rather than tuning the shared budget a +// third time around a moving target: no other file's packing changes (this +// file simply never enters the shared pool `packChunks` balances), and no +// future single-file addition can silently reintroduce this exact failure by +// landing in its chunk. If a future profiling pass genuinely speeds up +// codex-config.test.cjs itself, this isolation can be revisited — this is a +// packing-side mitigation for a KNOWN file's cost, not a statement that the +// cost is irreducible. +const ISOLATED_HEAVY_FILES = new Set(['codex-config.test.cjs']); + +/** + * Split `files` (absolute or repo-relative paths) into `{isolated, packable}` + * by basename membership in `ISOLATED_HEAVY_FILES`. Pure and order-preserving + * within each half, so it is unit-testable without spawning `main()` as a + * subprocess. `isolated` files are meant to become their own single-file + * chunk each; `packable` files are meant to go through `packChunks` as before. + */ +function partitionIsolatedFiles(files) { + const isolated = []; + const packable = []; + for (const f of files) { + (ISOLATED_HEAVY_FILES.has(f.split(/[\\/]/).pop()) ? isolated : packable).push(f); + } + return { isolated, packable }; +} + function parseArgs(argv) { let suite = null; let seen = false; @@ -1406,12 +1447,17 @@ function main() { const reporterOverhead = reporterArgs.reduce((sum, a) => sum + a.length + 1, 0); const FIXED_OVERHEAD = process.execPath.length + '--test'.length + concurrency.length + (forceExit ? '--test-force-exit'.length + 1 : 0) + reporterOverhead + 8; - const chunks = packChunks(selected, { - weightOf: fileWeightOf(), - maxWeight: MAX_FILES_PER_CHUNK, - maxChars: MAX_CMDLINE_CHARS, - fixedOverhead: FIXED_OVERHEAD, - }); + + const { isolated: isolatedFiles, packable: packableFiles } = partitionIsolatedFiles(selected); + const chunks = [ + ...isolatedFiles.map((f) => [f]), + ...packChunks(packableFiles, { + weightOf: fileWeightOf(), + maxWeight: MAX_FILES_PER_CHUNK, + maxChars: MAX_CMDLINE_CHARS, + fixedOverhead: FIXED_OVERHEAD, + }), + ]; // A chunk that still hangs (a leak the backstop somehow misses, or a wedged // subprocess) must fail loudly rather than silently burn the job's wall-clock @@ -1661,6 +1707,10 @@ module.exports = { loadTestTimings, makeFileWeigher, packChunks, + // 2026-09-07 (PR #4497): the codex-config.test.cjs chunk-isolation fix — + // see the comment above their definitions. + ISOLATED_HEAVY_FILES, + partitionIsolatedFiles, analyzeChunkEvents, DEFAULT_TIMINGS_PATH, // Exported so callers (tests/ci-test-scope.test.cjs) can assert the diff --git a/tests/compact-content-partition-guard.test.cjs b/tests/compact-content-partition-guard.test.cjs new file mode 100644 index 000000000..1e49630f1 --- /dev/null +++ b/tests/compact-content-partition-guard.test.cjs @@ -0,0 +1,838 @@ +'use strict'; + +// docs-guard-exempt: mentions docs/PARTITION-RULES.md only as a "see" pointer in the +// module docstring below, never reads its content — the checks this file implements are +// verified against docs/PARTITION-RULES.md's own prose by a human reviewer at authoring +// time, not by this test re-parsing that document at runtime. + +/** + * tests/compact-content-partition-guard.test.cjs — ADR-4139 Decision 5, epic #4139, + * Phase 3 #4403. This is the general-purpose CI guard PARTITION-RULES.md describes: it + * implements the five checks against every `/detail/*.md` split registered under + * `gsd-core/workflows/`, and supersedes the one-off `tests/plan-phase-compact-split.test.cjs` + * pilot (deleted by this same change). + * + * See `docs/PARTITION-RULES.md` for the operational spec — it is the source of truth for + * what each check does; this file is the mechanics plus the failing-first fixtures that + * prove each check can actually fail (per this repo's rule that a guard nobody has seen go + * red is not yet a guard). + * + * Layout: + * - checks 2/3 (disjointness, registration) run unconditionally against the real repo — + * they need no PR diff, just the files on disk. + * - checks 1/4/5 (completeness, protected content, boundary moves) are PR-diff-scoped: + * they compare the resolved base ref (origin/next, mirroring the base-ref-resolution + * idiom `tests/helpers/emitted-runtime.cjs`'s `resolveBase` already implements) against + * HEAD. When no base ref resolves (a fresh clone, a shallow/detached CI context), they + * skip cleanly — this is the ambient-run guard the dispatch brief calls out, and it is + * deliberately asymmetric with `readBoundaryMoveTrailers`'s own contract: THAT function + * throws on an uncomputable range because it is answering "did THIS PR declare its + * moves", and silently passing would disarm it; the ambient guard here is answering "is + * there even a PR diff to look at", which is a different, structurally prior question. + * - each of the 5 checks gets a fixture `describe` block with a RED (deliberately broken) + * and a GREEN (fixed) sibling test, built against synthetic temp files/repos — never + * against this repo's own real splits, so the fixture is independent of whatever the + * real tree happens to contain. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const crypto = require('node:crypto'); +const fc = require('fast-check'); + +const { cleanup } = require('./helpers.cjs'); +const { gitOrThrow, GIT_FIXTURE_TIMEOUT_MS } = require('./helpers/git-fixture.cjs'); +const { runGit, OUTCOME } = require('./helpers/process-seam.cjs'); +const { REPO_ROOT, GIT_TIMEOUT_MS, safeDirArgs, resolveBase } = require('./helpers/emitted-runtime.cjs'); +const { ACK_TRAILER_DELIM } = require('./helpers/emitted-diff.cjs'); +const { + DEFAULT_WORKFLOWS_DIR, + discoverRegisteredSplits, + normalizeNonTrivialLines, + extractProtectedBlocks, + readBoundaryMoveTrailers, + parseBoundaryMoveTrailerValues, + checkDisjointness, + checkRegistration, + checkDetailFileSizeCap, + NEW_FILE_CAP, +} = require('./helpers/compact-content-split.cjs'); + +// ─── git plumbing local to this guard (checks 1/4/5's PR-diff orchestration) ────────── + +/** ``-relative, forward-slash path, matching the trailer-key / reference shape + * PARTITION-RULES.md and `checkRegistration` both use. */ +function toRepoRelative(absPath) { + return path.relative(REPO_ROOT, absPath).split(path.sep).join('/'); +} + +/** `git merge-base HEAD`, or `null` on any failure (ambient guard: this is a + * "nothing to compare" signal, never a hard error, for the reasons in the file header). */ +function computeMergeBaseSha(baseRef) { + const result = runGit([...safeDirArgs(REPO_ROOT), 'merge-base', baseRef, 'HEAD'], { + cwd: REPO_ROOT, + timeoutMs: GIT_TIMEOUT_MS, + }); + if (result.outcome !== OUTCOME.EXITED || result.exitCode !== 0) return null; + const sha = result.stdout.trim(); + return /^[0-9a-f]{40}$/.test(sha) ? sha : null; +} + +/** `git diff --name-status ...HEAD -- gsd-core/workflows`, parsed into + * `{status, path}` rows (`path` is always the CURRENT path — the second field of an + * `R###\told\tnew` rename row, or the single field of every other status). `null` on + * any git failure (ambient guard). */ +function diffNameStatusForWorkflows(mergeBaseSha) { + const result = runGit( + [...safeDirArgs(REPO_ROOT), 'diff', '--name-status', `${mergeBaseSha}...HEAD`, '--', 'gsd-core/workflows'], + { cwd: REPO_ROOT, timeoutMs: GIT_TIMEOUT_MS }, + ); + if (result.outcome !== OUTCOME.EXITED || result.exitCode !== 0) return null; + return result.stdout + .split('\n') + .map((l) => l.trim()) + .filter(Boolean) + .map((line) => { + const fields = line.split('\t'); + return { status: fields[0][0], path: fields[fields.length - 1] }; + }); +} + +/** `git show :`, or `null` if the path is absent at `ref` (or on any other + * git failure — see the file header on why this check family never throws for that). */ +function showAtRefOrNull(ref, relPath) { + const result = runGit([...safeDirArgs(REPO_ROOT), 'show', `${ref}:${relPath}`], { + cwd: REPO_ROOT, + timeoutMs: GIT_TIMEOUT_MS, + }); + return result.outcome === OUTCOME.EXITED && result.exitCode === 0 ? result.stdout : null; +} + +// ─── pure check logic (the "lower-level functions" the fixtures call directly) ──────── + +/** + * Check 1 (completeness): the union of the new spine + new detail parts must be a + * superset of the OLD spine's non-trivial lines, and the new spine must be smaller. + * `oldSpineContent === null` (no comparison base — a wholly new split with nothing to be + * complete relative to) short-circuits to no violations at all, per PARTITION-RULES.md. + */ +function checkCompletenessForPair({ splitName, oldSpineContent, newSpineContent, newDetailContents }) { + if (oldSpineContent === null) return []; + const violations = []; + const oldLines = normalizeNonTrivialLines(oldSpineContent); + const unionSet = new Set([ + ...normalizeNonTrivialLines(newSpineContent), + ...newDetailContents.flatMap((c) => normalizeNonTrivialLines(c)), + ]); + for (const line of oldLines) { + if (!unionSet.has(line)) violations.push({ kind: 'incomplete_split', splitName, line }); + } + const oldSize = Buffer.byteLength(oldSpineContent, 'utf8'); + const newSize = Buffer.byteLength(newSpineContent, 'utf8'); + if (!(newSize < oldSize)) { + violations.push({ kind: 'spine_not_smaller', splitName, oldSize, newSize }); + } + return violations; +} + +/** + * All non-blank, trimmed lines of `content`, WITHOUT `normalizeNonTrivialLines`'s + * boilerplate filter. Check 4 needs this distinct membership set: a protected block can + * legitimately enclose an `isTrivial`-shaped line (most commonly a code-fence delimiter, + * e.g. a `` example's own ```` ``` ````), and that line is + * just as much "content the block covers" as any prose line — dropping it from the + * membership test would report a byte-identical spine as having "deleted" its own fence, + * a false positive caught by this guard's own real-repo sanity run against plan-phase.md + * before this fix (the fence lines were being tested for presence in a + * `normalizeNonTrivialLines`-filtered set, which strips exactly that shape). + */ +function allNonBlankLines(content) { + return content.split(/\r?\n/).map((l) => l.trim()).filter((l) => l.length > 0); +} + +/** + * Check 4 (protected content): every line inside a `` sentinel at + * the OLD spine must still be physically present in the NEW spine. A missing line that + * now appears in one of the spine's detail parts is reported as `protected_relocated` + * (named destination); any other missing line is `protected_deleted`. No trailer can ever + * excuse either — this function does not even look for one. + */ +function checkProtectedContentForPair({ splitName, oldSpineContent, newSpineContent, detailPathToContent }) { + if (oldSpineContent === null) return []; + const violations = []; + const oldBlocks = extractProtectedBlocks(oldSpineContent).blocks; + const newSpineLines = new Set(allNonBlankLines(newSpineContent)); + for (const block of oldBlocks) { + for (const line of block.lines) { + if (newSpineLines.has(line)) continue; + let relocatedTo = null; + for (const [relPath, content] of detailPathToContent) { + if (allNonBlankLines(content).includes(line)) { + relocatedTo = relPath; + break; + } + } + violations.push({ + kind: relocatedTo ? 'protected_relocated' : 'protected_deleted', + splitName, + blockFirstLine: block.firstLine, + line, + relocatedTo, + }); + } + } + return violations; +} + +/** + * Check 5's moved-line detector: non-trivial lines removed from the spine (present at the + * old ref, absent now) that are simultaneously newly present in one of the spine's OWN + * detail parts (absent from that detail part's old content — or the part is brand new — + * present now). `excludeLines` drops anything already counted as a check-4 protected + * violation: a protected line moving is ALWAYS a hard failure regardless of a trailer + * (check 4's job), so check 5 only concerns itself with ordinary lines. + */ +function findMovedLines({ oldSpineContent, newSpineContent, oldDetailContentByPath, newDetailContentByPath, excludeLines }) { + const oldSpineLines = new Set(normalizeNonTrivialLines(oldSpineContent)); + const newSpineLines = new Set(normalizeNonTrivialLines(newSpineContent)); + const removedFromSpine = new Set([...oldSpineLines].filter((l) => !newSpineLines.has(l))); + const moved = new Set(); + for (const [relPath, newContent] of newDetailContentByPath) { + const oldContent = oldDetailContentByPath.has(relPath) ? oldDetailContentByPath.get(relPath) : null; + const oldDetailLines = new Set(oldContent === null ? [] : normalizeNonTrivialLines(oldContent)); + for (const line of normalizeNonTrivialLines(newContent)) { + if (removedFromSpine.has(line) && !oldDetailLines.has(line) && !excludeLines.has(line)) { + moved.add(line); + } + } + } + return [...moved]; +} + +/** + * Check 5 (boundary moves declared): wraps `findMovedLines` with the trailer lookup. When + * there is nothing moved, `readBoundaryMoveTrailers` is never even called — no PR-shaped + * question to ask. When there IS a moved line, the trailer read is NOT guarded: a failure + * there must throw and propagate, per PARTITION-RULES.md check 5 / the file header's + * asymmetry note. + */ +function checkBoundaryMovesForSplit({ + splitName, + spineRelPath, + oldSpineContent, + newSpineContent, + oldDetailContentByPath, + newDetailContentByPath, + excludeLines, + baseRef, + cwd, +}) { + const moved = findMovedLines({ + oldSpineContent, + newSpineContent, + oldDetailContentByPath, + newDetailContentByPath, + excludeLines, + }); + if (moved.length === 0) return []; + const { declarations } = readBoundaryMoveTrailers({ baseRef, cwd }); + if (declarations.has(spineRelPath)) return []; + return moved.map((line) => ({ kind: 'undeclared_boundary_move', splitName, spinePath: spineRelPath, line })); +} + +/** + * Full PR-diff-scoped orchestration for checks 1, 4, 5 against the REAL repo. Returns + * `{skipped: true, reason}` when there is genuinely nothing to compare (ambient guard); + * otherwise `{skipped: false, completeness, protectedContent, boundaryMoves}`. + */ +function runPrDiffScopedChecks() { + const base = resolveBase(); + if (!base) return { skipped: true, reason: 'no resolvable base ref (origin/next unavailable)' }; + + const mergeBaseSha = computeMergeBaseSha(base.ref); + if (!mergeBaseSha) return { skipped: true, reason: `merge-base unresolvable for "${base.ref}..HEAD"` }; + + const diff = diffNameStatusForWorkflows(mergeBaseSha); + if (diff === null) return { skipped: true, reason: 'git diff --name-status failed' }; + + const splits = discoverRegisteredSplits().filter((s) => s.spineExists); + const completeness = []; + const protectedContent = []; + const boundaryMoves = []; + + // Check 1: only for detail paths newly ADDED by this diff, grouped by split name. + const newlySplitNames = new Set(); + for (const { status, path: p } of diff) { + if (status !== 'A') continue; + const m = /^gsd-core\/workflows\/([^/]+)\/detail\/[^/]+\.md$/.exec(p); + if (m) newlySplitNames.add(m[1]); + } + for (const name of newlySplitNames) { + const split = splits.find((s) => s.name === name); + if (!split) continue; + const spineRel = toRepoRelative(split.spinePath); + const oldSpineContent = showAtRefOrNull(mergeBaseSha, spineRel); + const newSpineContent = fs.readFileSync(split.spinePath, 'utf8'); + const newDetailContents = split.detailPaths.map((p) => fs.readFileSync(p, 'utf8')); + completeness.push( + ...checkCompletenessForPair({ splitName: name, oldSpineContent, newSpineContent, newDetailContents }), + ); + } + + // Checks 4 + 5: every registered split NOT already counted as newly-split above (an + // untouched spine trivially produces zero violations either way, since old === new + // content — see the file header). + // + // A split in `newlySplitNames` is excluded here, not merely redundant with check 1. + // Checks 4/5 both premise an OLD spine that already carried the content in question — + // "did an EXISTING split shed protected content or move something undeclared". A + // split whose detail path did not exist at the resolved base has no such premise: it + // is brand new relative to that base, which is check 1's domain alone. This also + // makes checks 4/5 robust to a base ref that resolves further back than expected (a + // known class of gotcha `resolveBase()`'s own doc comment describes — no `origin/*` + // remote-tracking refs inside the gsd-test sandbox): a stale/older base makes an + // already-merged, already-reviewed split look "newly introduced" from that base's + // vantage point, and retroactively demanding a Boundary-Move-Declared trailer for a + // split that predates the trailer mechanism's own existence is exactly the false + // positive this exclusion prevents. Verified against the real repo: plan-phase's + // split (Phase 2, #4402) landed before this PR (#4403) introduces Boundary-Move-Declared + // at all, so it must never be checked against that requirement retroactively. + for (const split of splits) { + if (newlySplitNames.has(split.name)) continue; + const spineRel = toRepoRelative(split.spinePath); + const oldSpineContent = showAtRefOrNull(mergeBaseSha, spineRel); + if (oldSpineContent === null) continue; // no old spine to compare against at this ref + const newSpineContent = fs.readFileSync(split.spinePath, 'utf8'); + const detailPathToContent = new Map(split.detailPaths.map((p) => [toRepoRelative(p), fs.readFileSync(p, 'utf8')])); + + const protectedViolations = checkProtectedContentForPair({ + splitName: split.name, + oldSpineContent, + newSpineContent, + detailPathToContent, + }); + protectedContent.push(...protectedViolations); + + const excludeLines = new Set(protectedViolations.map((v) => v.line)); + const oldDetailContentByPath = new Map(); + for (const relPath of detailPathToContent.keys()) { + oldDetailContentByPath.set(relPath, showAtRefOrNull(mergeBaseSha, relPath)); + } + boundaryMoves.push( + ...checkBoundaryMovesForSplit({ + splitName: split.name, + spineRelPath: spineRel, + oldSpineContent, + newSpineContent, + oldDetailContentByPath, + newDetailContentByPath: detailPathToContent, + excludeLines, + baseRef: base.ref, + cwd: REPO_ROOT, + }), + ); + } + + return { skipped: false, completeness, protectedContent, boundaryMoves }; +} + +// ─── real-repo assertions (checks 2/3 unconditional; 1/4/5 PR-diff-scoped) ──────────── + +describe('compact-content partition guard — real repo state (ADR-4139 Decision 5, #4403)', () => { + test('check 2 (disjointness): no non-trivial line duplicated between any spine and its detail parts', () => { + const splits = discoverRegisteredSplits().filter((s) => s.spineExists); + const violations = checkDisjointness(splits); + assert.deepStrictEqual(violations, [], `disjointness violations: ${JSON.stringify(violations, null, 2)}`); + }); + + test('check 3 (registration): every split is paired, referenced, and has no dangling reference', () => { + const splits = discoverRegisteredSplits(); + const violations = checkRegistration(splits, DEFAULT_WORKFLOWS_DIR); + assert.deepStrictEqual(violations, [], `registration violations: ${JSON.stringify(violations, null, 2)}`); + }); + + test('check 3 (size cap): every detail file is under NEW_FILE_CAP', () => { + const splits = discoverRegisteredSplits().filter((s) => s.spineExists); + const violations = checkDetailFileSizeCap(splits); + assert.deepStrictEqual(violations, [], `size-cap violations: ${JSON.stringify(violations, null, 2)}`); + }); + + test('checks 1, 4, 5: PR-diff-scoped checks against the resolved base ref (skips cleanly when unresolvable)', () => { + const result = runPrDiffScopedChecks(); + if (result.skipped) { + // Ambient guard: nothing to compare (no origin/next, no computable merge-base, no + // readable diff). Not a failure — see the file header for why this is different + // from check 5's own uncomputable-range throw. + return; + } + assert.deepStrictEqual( + result.completeness, [], + `completeness (check 1) violations: ${JSON.stringify(result.completeness, null, 2)}`, + ); + assert.deepStrictEqual( + result.protectedContent, [], + `protected-content (check 4) violations: ${JSON.stringify(result.protectedContent, null, 2)}`, + ); + assert.deepStrictEqual( + result.boundaryMoves, [], + `undeclared boundary-move (check 5) violations: ${JSON.stringify(result.boundaryMoves, null, 2)}`, + ); + }); + + test('plan-phase spine references the shared compact-content gate exactly once (real-repo sanity, folded in from the retired pilot test)', () => { + const spine = fs.readFileSync(path.join(DEFAULT_WORKFLOWS_DIR, 'plan-phase.md'), 'utf8'); + const matches = spine.match(/compact-content-gate\.md/g) || []; + assert.strictEqual(matches.length, 1, `expected exactly one reference to compact-content-gate.md, found ${matches.length}`); + }); + + test('every registered spine has no malformed gsd:protected sentinel', () => { + const splits = discoverRegisteredSplits().filter((s) => s.spineExists); + for (const split of splits) { + const { malformed } = extractProtectedBlocks(fs.readFileSync(split.spinePath, 'utf8')); + assert.deepStrictEqual( + malformed, [], + `${split.spinePath} has malformed gsd:protected sentinel(s): ${JSON.stringify(malformed, null, 2)}`, + ); + } + }); + + test('plan-phase spine: protected-block count and the four named categories are represented (real-repo sanity, folded in from the retired pilot test)', () => { + const spinePath = path.join(DEFAULT_WORKFLOWS_DIR, 'plan-phase.md'); + const spine = fs.readFileSync(spinePath, 'utf8'); + const { blocks, malformed } = extractProtectedBlocks(spine); + assert.deepStrictEqual(malformed, []); + const paired = blocks.filter((b) => b.kind === 'paired'); + const single = blocks.filter((b) => b.kind === 'single'); + assert.ok(paired.length >= 4, `expected at least 4 paired protected blocks, found ${paired.length}`); + assert.ok(single.length >= 2, `expected at least 2 single-line protected markers, found ${single.length}`); + + // Output-format contracts: + assert.match(spine, /\s*/, 'quality_gate output-format contract must be protected'); + assert.match(spine, /\s*/, 'success_criteria output-format contract must be protected'); + assert.match(spine, /\s*/, 'downstream_consumer output-format contract must be protected'); + // Few-shot example the workflow's own steps depend on: + assert.match(spine, /\s*/, 'failing_direction_contract few-shot example must be protected'); + // Negative instruction / guardrail: + const guardrailCount = (spine.match(/\s*> \*\*ORCHESTRATOR RULE[^]*?Never call `ScheduleWakeup`/g) || []).length; + assert.strictEqual(guardrailCount, 2, `expected 2 protected ScheduleWakeup guardrail paragraphs, found ${guardrailCount}`); + }); +}); + +// ─── failing-first fixtures: one describe block per check, RED then GREEN ───────────── + +describe('failing-first fixture: check 1 (completeness)', () => { + test('RED — a line from the old spine is missing from the new spine+detail union, and the spine did not shrink', () => { + const oldSpineContent = 'Intro line.\n\nCritical instruction not carried over.\n\nClosing line.\n'; + const newSpineContent = 'Intro line.\n\nClosing line.\n'; + const newDetailContents = ['Some unrelated detail content.\n']; + const violations = checkCompletenessForPair({ + splitName: 'broken-split', oldSpineContent, newSpineContent, newDetailContents, + }); + assert.ok(violations.length > 0, 'expected at least one completeness violation'); + assert.ok( + violations.some((v) => v.kind === 'incomplete_split' && v.line === 'Critical instruction not carried over.'), + `expected the missing line to be named: ${JSON.stringify(violations)}`, + ); + }); + + test('GREEN — the union carries every old line and the spine shrank', () => { + const oldSpineContent = 'Intro line.\n\nCritical instruction not carried over.\n\nClosing line.\n'; + const newSpineContent = 'Intro line.\n\nClosing line.\n'; + const newDetailContents = ['Critical instruction not carried over.\n']; + const violations = checkCompletenessForPair({ + splitName: 'fixed-split', oldSpineContent, newSpineContent, newDetailContents, + }); + assert.deepStrictEqual(violations, []); + }); +}); + +describe('failing-first fixture: check 2 (disjointness)', () => { + test('RED — a non-trivial line appears in both the spine and a detail part', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-disjoint-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'dup', 'detail'), { recursive: true }); + const duplicatedLine = 'This exact sentence must never appear twice across the split.'; + fs.writeFileSync(path.join(workflowsDir, 'dup.md'), `Spine intro.\n\n${duplicatedLine}\n\ngsd-core/workflows/dup/detail/part.md\n`); + fs.writeFileSync(path.join(workflowsDir, 'dup', 'detail', 'part.md'), `Detail intro.\n\n${duplicatedLine}\n`); + + const splits = discoverRegisteredSplits(workflowsDir).filter((s) => s.spineExists); + const violations = checkDisjointness(splits); + assert.ok(violations.length > 0, 'expected at least one disjointness violation'); + assert.ok( + violations.some((v) => v.line === duplicatedLine), + `expected the duplicated line to be named: ${JSON.stringify(violations)}`, + ); + } finally { + cleanup(tmpRoot); + } + }); + + test('GREEN — the same line, rewritten distinctly on each side, is disjoint', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-disjoint-ok-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'dup', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'dup.md'), 'Spine intro.\n\nSpine-only sentence.\n\ngsd-core/workflows/dup/detail/part.md\n'); + fs.writeFileSync(path.join(workflowsDir, 'dup', 'detail', 'part.md'), 'Detail intro.\n\nDetail-only sentence.\n'); + + const splits = discoverRegisteredSplits(workflowsDir).filter((s) => s.spineExists); + const violations = checkDisjointness(splits); + assert.deepStrictEqual(violations, []); + } finally { + cleanup(tmpRoot); + } + }); +}); + +describe('failing-first fixture: check 3 (registration)', () => { + test('RED — a detail/*.md exists with no matching spine (orphan)', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-orphan-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'orphan', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'orphan', 'detail', 'part.md'), 'Orphaned detail content.\n'); + + const splits = discoverRegisteredSplits(workflowsDir); + const violations = checkRegistration(splits, workflowsDir); + assert.ok(violations.length > 0, 'expected at least one registration violation'); + assert.ok( + violations.some((v) => v.kind === 'orphan_detail' && v.name === 'orphan'), + `expected the orphan split to be named: ${JSON.stringify(violations)}`, + ); + } finally { + cleanup(tmpRoot); + } + }); + + test('GREEN — a spine is added that references the detail part by its full repo-relative path', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-orphan-ok-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'orphan', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'orphan', 'detail', 'part.md'), 'Orphaned detail content.\n'); + fs.writeFileSync( + path.join(workflowsDir, 'orphan.md'), + 'Spine intro.\n\nSee gsd-core/workflows/orphan/detail/part.md for elaboration.\n', + ); + + const splits = discoverRegisteredSplits(workflowsDir); + const violations = checkRegistration(splits, workflowsDir); + assert.deepStrictEqual(violations, []); + } finally { + cleanup(tmpRoot); + } + }); + + test('RED (size cap) — a new detail file at or over NEW_FILE_CAP', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-sizecap-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'big', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'big.md'), 'Spine.\n\ngsd-core/workflows/big/detail/part.md\n'); + fs.writeFileSync(path.join(workflowsDir, 'big', 'detail', 'part.md'), 'x'.repeat(NEW_FILE_CAP)); + + const splits = discoverRegisteredSplits(workflowsDir).filter((s) => s.spineExists); + const violations = checkDetailFileSizeCap(splits); + assert.ok(violations.length > 0, 'expected at least one size-cap violation'); + assert.ok( + violations.some((v) => v.detailPath.endsWith(path.join('big', 'detail', 'part.md'))), + `expected the oversized file to be named: ${JSON.stringify(violations)}`, + ); + } finally { + cleanup(tmpRoot); + } + }); + + test('GREEN (size cap) — the same detail file kept just under NEW_FILE_CAP', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-sizecap-ok-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'big', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'big.md'), 'Spine.\n\ngsd-core/workflows/big/detail/part.md\n'); + fs.writeFileSync(path.join(workflowsDir, 'big', 'detail', 'part.md'), 'x'.repeat(NEW_FILE_CAP - 1)); + + const splits = discoverRegisteredSplits(workflowsDir).filter((s) => s.spineExists); + const violations = checkDetailFileSizeCap(splits); + assert.deepStrictEqual(violations, []); + } finally { + cleanup(tmpRoot); + } + }); + + test('RED (size cap) — a new detail file one byte OVER NEW_FILE_CAP (the third boundary point)', () => { + // Boundary coverage requires limit-1 / limit / limit+1 (CLAUDE.md TEST RULES). + // The two tests above already cover limit-1 (GREEN) and limit exactly (RED, + // ">= NEW_FILE_CAP" boundary); this is the third point, proving the cap does + // not silently stop firing past its own threshold. + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-sizecap-over-')); + try { + const workflowsDir = path.join(tmpRoot, 'gsd-core', 'workflows'); + fs.mkdirSync(path.join(workflowsDir, 'big', 'detail'), { recursive: true }); + fs.writeFileSync(path.join(workflowsDir, 'big.md'), 'Spine.\n\ngsd-core/workflows/big/detail/part.md\n'); + fs.writeFileSync(path.join(workflowsDir, 'big', 'detail', 'part.md'), 'x'.repeat(NEW_FILE_CAP + 1)); + + const splits = discoverRegisteredSplits(workflowsDir).filter((s) => s.spineExists); + const violations = checkDetailFileSizeCap(splits); + assert.ok(violations.length > 0, 'expected at least one size-cap violation'); + assert.ok( + violations.some((v) => v.detailPath.endsWith(path.join('big', 'detail', 'part.md')) && v.size === NEW_FILE_CAP + 1), + `expected the oversized file to be named with its actual size: ${JSON.stringify(violations)}`, + ); + } finally { + cleanup(tmpRoot); + } + }); +}); + +describe('failing-first fixture: check 4 (protected content)', () => { + const oldSpineWithProtection = + 'Intro.\n\n\n> Never do the dangerous thing.\n\nMiddle prose.\n'; + + test('RED — a protected line is deleted from the spine entirely', () => { + const newSpineContent = 'Intro.\n\nMiddle prose.\n'; + const violations = checkProtectedContentForPair({ + splitName: 'broken-split', + oldSpineContent: oldSpineWithProtection, + newSpineContent, + detailPathToContent: new Map(), + }); + assert.ok(violations.length > 0, 'expected at least one protected-content violation'); + const v = violations.find((x) => x.line === '> Never do the dangerous thing.'); + assert.ok(v, `expected the deleted protected line to be named: ${JSON.stringify(violations)}`); + assert.strictEqual(v.kind, 'protected_deleted'); + }); + + test('RED (relocated) — a protected line moved into a detail part is reported distinctly, not merely "deleted"', () => { + const newSpineContent = 'Intro.\n\nMiddle prose.\n'; + const detailPathToContent = new Map([ + ['gsd-core/workflows/broken/detail/part.md', 'Detail intro.\n\n> Never do the dangerous thing.\n'], + ]); + const violations = checkProtectedContentForPair({ + splitName: 'broken-split', + oldSpineContent: oldSpineWithProtection, + newSpineContent, + detailPathToContent, + }); + const v = violations.find((x) => x.line === '> Never do the dangerous thing.'); + assert.ok(v, `expected the relocated protected line to be named: ${JSON.stringify(violations)}`); + assert.strictEqual(v.kind, 'protected_relocated'); + assert.strictEqual(v.relocatedTo, 'gsd-core/workflows/broken/detail/part.md'); + }); + + test('GREEN — the protected line is still physically present in the spine', () => { + const newSpineContent = 'Intro.\n\n\n> Never do the dangerous thing.\n\nMiddle prose.\n'; + const violations = checkProtectedContentForPair({ + splitName: 'fixed-split', + oldSpineContent: oldSpineWithProtection, + newSpineContent, + detailPathToContent: new Map(), + }); + assert.deepStrictEqual(violations, []); + }); +}); + +describe('failing-first fixture: check 5 (boundary moves declared)', () => { + /** A throwaway git repo, mirroring the makeTempRepo idiom in + * tests/emitted-ack-trailer.test.cjs (this module's `readBoundaryMoveTrailers` sibling + * under tests/helpers/compact-content-split.cjs is a direct port of that same + * mechanism, so the fixture idiom mirrors it byte-for-byte). */ + function makeTempRepo(prefix) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + gitOrThrow(['init', '-q', '-b', 'main'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + gitOrThrow(['config', 'user.email', 'partition-guard-fixture@example.com'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + gitOrThrow(['config', 'user.name', 'Partition Guard Fixture'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + return dir; + } + + /** Commit from a message written to a file (never `-m`), so a trailer block lands where + * git's own trailer parser expects it — same reasoning as emitted-ack-trailer.test.cjs. */ + function commitMessage(dir, message) { + const msgFile = path.join(os.tmpdir(), `gsd-partition-guard-msg-${crypto.randomBytes(6).toString('hex')}.txt`); + fs.writeFileSync(msgFile, message); + try { + gitOrThrow(['add', '-A'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + gitOrThrow(['commit', '-q', '-F', msgFile], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + } finally { + cleanup(msgFile); + } + } + + function headSha(dir) { + return gitOrThrow(['rev-parse', 'HEAD'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }).trim(); + } + + const SPINE_KEY = 'gsd-core/workflows/spine.md'; // the trailer key our fixture declares against + + test('RED — a line moved from spine to detail with no Boundary-Move-Declared trailer', (t) => { + const dir = makeTempRepo('gsd-partition-boundary-red-'); + t.after(() => cleanup(dir)); + + fs.writeFileSync(path.join(dir, 'spine.md'), 'Line A.\n\nOrdinary line that will move.\n\nLine C.\n'); + fs.writeFileSync(path.join(dir, 'detail.md'), 'Line D.\n'); + commitMessage(dir, 'init\n\nbaseline, no move yet\n'); + const baseSha = headSha(dir); + + const oldSpineContent = fs.readFileSync(path.join(dir, 'spine.md'), 'utf8'); + const oldDetailContent = fs.readFileSync(path.join(dir, 'detail.md'), 'utf8'); + + fs.writeFileSync(path.join(dir, 'spine.md'), 'Line A.\n\nLine C.\n'); + fs.writeFileSync(path.join(dir, 'detail.md'), 'Line D.\n\nOrdinary line that will move.\n'); + commitMessage(dir, 'move a line without declaring it\n\nno trailer here\n'); + + const newSpineContent = fs.readFileSync(path.join(dir, 'spine.md'), 'utf8'); + const newDetailContent = fs.readFileSync(path.join(dir, 'detail.md'), 'utf8'); + + const violations = checkBoundaryMovesForSplit({ + splitName: 'spine', + spineRelPath: SPINE_KEY, + oldSpineContent, + newSpineContent, + oldDetailContentByPath: new Map([['detail.md', oldDetailContent]]), + newDetailContentByPath: new Map([['detail.md', newDetailContent]]), + excludeLines: new Set(), + baseRef: baseSha, + cwd: dir, + }); + + assert.ok(violations.length > 0, 'expected an undeclared boundary-move violation'); + const v = violations.find((x) => x.line === 'Ordinary line that will move.'); + assert.ok(v, `expected the moved line to be named: ${JSON.stringify(violations)}`); + assert.strictEqual(v.kind, 'undeclared_boundary_move'); + assert.strictEqual(v.spinePath, SPINE_KEY); + }); + + test('GREEN — the same move, with a Boundary-Move-Declared trailer naming the spine', (t) => { + const dir = makeTempRepo('gsd-partition-boundary-green-'); + t.after(() => cleanup(dir)); + + fs.writeFileSync(path.join(dir, 'spine.md'), 'Line A.\n\nOrdinary line that will move.\n\nLine C.\n'); + fs.writeFileSync(path.join(dir, 'detail.md'), 'Line D.\n'); + commitMessage(dir, 'init\n\nbaseline, no move yet\n'); + const baseSha = headSha(dir); + + const oldSpineContent = fs.readFileSync(path.join(dir, 'spine.md'), 'utf8'); + const oldDetailContent = fs.readFileSync(path.join(dir, 'detail.md'), 'utf8'); + + fs.writeFileSync(path.join(dir, 'spine.md'), 'Line A.\n\nLine C.\n'); + fs.writeFileSync(path.join(dir, 'detail.md'), 'Line D.\n\nOrdinary line that will move.\n'); + commitMessage( + dir, + `move a line and declare it\n\nBoundary-Move-Declared: ${SPINE_KEY} — moved to detail deliberately, #4403\n`, + ); + + const newSpineContent = fs.readFileSync(path.join(dir, 'spine.md'), 'utf8'); + const newDetailContent = fs.readFileSync(path.join(dir, 'detail.md'), 'utf8'); + + const violations = checkBoundaryMovesForSplit({ + splitName: 'spine', + spineRelPath: SPINE_KEY, + oldSpineContent, + newSpineContent, + oldDetailContentByPath: new Map([['detail.md', oldDetailContent]]), + newDetailContentByPath: new Map([['detail.md', newDetailContent]]), + excludeLines: new Set(), + baseRef: baseSha, + cwd: dir, + }); + + assert.deepStrictEqual(violations, []); + }); + + test('uncomputable range still throws through checkBoundaryMovesForSplit (never silently passes)', (t) => { + const origin = makeTempRepo('gsd-partition-boundary-origin-'); + t.after(() => cleanup(origin)); + fs.writeFileSync(path.join(origin, 'spine.md'), 'Line A.\n\nOrdinary line that will move.\n'); + commitMessage(origin, 'origin A\n\nfirst commit\n'); + const shaA = headSha(origin); + fs.writeFileSync(path.join(origin, 'spine.md'), 'Line A.\n\nOrdinary line that will move.\n\nLine B.\n'); + commitMessage(origin, 'origin B\n\nsecond commit\n'); + + const clone = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-partition-boundary-clone-')); + t.after(() => cleanup(clone)); + gitOrThrow(['clone', '-q', '--depth', '1', `file://${origin}`, clone], { cwd: origin, timeoutMs: GIT_FIXTURE_TIMEOUT_MS }); + + assert.throws( + () => checkBoundaryMovesForSplit({ + splitName: 'spine', + spineRelPath: SPINE_KEY, + oldSpineContent: 'Line A.\n\nOrdinary line that will move.\n', + newSpineContent: 'Line A.\n', + oldDetailContentByPath: new Map(), + newDetailContentByPath: new Map([['detail.md', 'Ordinary line that will move.\n']]), + excludeLines: new Set(), + baseRef: shaA, + cwd: clone, + }), + /./, + 'a genuinely uncomputable merge-base must throw, never return an empty result', + ); + }); +}); + +// ─── property tests (CLAUDE.md TEST RULES: parsers/bijective contracts need fast-check) ─ + +describe('property: compact-content-split.cjs parsers', () => { + // Same alphabets as the ADR-3942 sibling this trailer mechanism ports from + // (tests/emitted-ack-trailer.test.cjs's KEY_ALPHABET/REASON_ALPHABET) — neither + // alphabet can produce ACK_TRAILER_DELIM itself, so a generated key/reason pair + // never accidentally embeds a second delimiter and corrupts its own round trip. + const KEY_ALPHABET = 'abcdefghijklmnopqrstuvwxyz0123456789_./-'.split(''); + const REASON_ALPHABET = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 ,.#-'.split(''); + + const keyArb = fc.array(fc.constantFrom(...KEY_ALPHABET), { minLength: 1, maxLength: 40 }) + .map((chars) => chars.join('')); + const reasonArb = fc.array(fc.constantFrom(...REASON_ALPHABET), { minLength: 1, maxLength: 60 }) + .map((chars) => chars.join('').trim()) + .filter((s) => s.length > 0); + + test('prop: Boundary-Move-Declared trailer render/parse is bijective', () => { + fc.assert( + fc.property(keyArb, reasonArb, (key, reason) => { + const rendered = `${key}${ACK_TRAILER_DELIM}${reason}`; + const { declarations, errors } = parseBoundaryMoveTrailerValues([rendered]); + assert.deepStrictEqual(errors, []); + assert.strictEqual(declarations.get(key)?.reason, reason); + }), + { seed: 4403, numRuns: 300 }, + ); + }); + + test('prop: two identical (key, reason) declarations always dedupe to exactly one entry, never an error', () => { + fc.assert( + fc.property(keyArb, reasonArb, (key, reason) => { + const rendered = `${key}${ACK_TRAILER_DELIM}${reason}`; + const { declarations, errors } = parseBoundaryMoveTrailerValues([rendered, rendered]); + assert.deepStrictEqual(errors, []); + assert.strictEqual(declarations.size, 1); + assert.strictEqual(declarations.get(key)?.reason, reason); + }), + { seed: 4403, numRuns: 200 }, + ); + }); + + // Body lines deliberately exclude anything sentinel-shaped or blank, so the + // generated fixture can never accidentally produce a SECOND sentinel or an + // early terminator inside what is meant to be one continuous protected span. + const bodyLineArb = fc.array(fc.constantFrom(...'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 .,'.split('')), { minLength: 1, maxLength: 30 }) + .map((chars) => chars.join('').trim()) + .filter((s) => s.length > 0 && !s.includes('gsd:protected')); + + test('prop: any well-formed span round-trips through extractProtectedBlocks with zero malformed entries', () => { + fc.assert( + fc.property(fc.array(bodyLineArb, { minLength: 1, maxLength: 8 }), (lines) => { + const content = ['Intro prose.', '', '', ...lines, '', '', 'Closing prose.'].join('\n'); + const { blocks, malformed } = extractProtectedBlocks(content); + assert.deepStrictEqual(malformed, []); + const paired = blocks.filter((b) => b.kind === 'paired'); + assert.strictEqual(paired.length, 1); + assert.deepStrictEqual(paired[0].lines, lines); + }), + { seed: 4403, numRuns: 200 }, + ); + }); +}); diff --git a/tests/fixtures/install-tree/antigravity.json b/tests/fixtures/install-tree/antigravity.json index 412122bdd..f0ac237e5 100644 --- a/tests/fixtures/install-tree/antigravity.json +++ b/tests/fixtures/install-tree/antigravity.json @@ -169,7 +169,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/augment.json b/tests/fixtures/install-tree/augment.json index ec30b21ce..30a73a900 100644 --- a/tests/fixtures/install-tree/augment.json +++ b/tests/fixtures/install-tree/augment.json @@ -241,7 +241,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/claude-local.json b/tests/fixtures/install-tree/claude-local.json index c1c261b33..f69fbedeb 100644 --- a/tests/fixtures/install-tree/claude-local.json +++ b/tests/fixtures/install-tree/claude-local.json @@ -134,7 +134,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/claude.json b/tests/fixtures/install-tree/claude.json index db1babe3f..4cc0fb0c5 100644 --- a/tests/fixtures/install-tree/claude.json +++ b/tests/fixtures/install-tree/claude.json @@ -169,7 +169,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/cline.json b/tests/fixtures/install-tree/cline.json index 854ba7124..d560221b9 100644 --- a/tests/fixtures/install-tree/cline.json +++ b/tests/fixtures/install-tree/cline.json @@ -171,7 +171,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/codebuddy.json b/tests/fixtures/install-tree/codebuddy.json index b53440eab..0c11b577d 100644 --- a/tests/fixtures/install-tree/codebuddy.json +++ b/tests/fixtures/install-tree/codebuddy.json @@ -241,7 +241,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/codex.json b/tests/fixtures/install-tree/codex.json index 67478dbf3..078b79408 100644 --- a/tests/fixtures/install-tree/codex.json +++ b/tests/fixtures/install-tree/codex.json @@ -205,7 +205,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/copilot.json b/tests/fixtures/install-tree/copilot.json index 844063746..b7cbaeab8 100644 --- a/tests/fixtures/install-tree/copilot.json +++ b/tests/fixtures/install-tree/copilot.json @@ -170,7 +170,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/cursor.json b/tests/fixtures/install-tree/cursor.json index d94dfa9d3..00a5d48af 100644 --- a/tests/fixtures/install-tree/cursor.json +++ b/tests/fixtures/install-tree/cursor.json @@ -169,7 +169,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/hermes.json b/tests/fixtures/install-tree/hermes.json index b9a7c327f..e8ae05694 100644 --- a/tests/fixtures/install-tree/hermes.json +++ b/tests/fixtures/install-tree/hermes.json @@ -169,7 +169,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/kilo.json b/tests/fixtures/install-tree/kilo.json index b18cda0be..8f1946035 100644 --- a/tests/fixtures/install-tree/kilo.json +++ b/tests/fixtures/install-tree/kilo.json @@ -241,7 +241,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/kimi-code.json b/tests/fixtures/install-tree/kimi-code.json index 4148e94b7..7db79bd6e 100644 --- a/tests/fixtures/install-tree/kimi-code.json +++ b/tests/fixtures/install-tree/kimi-code.json @@ -170,7 +170,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/kimi.json b/tests/fixtures/install-tree/kimi.json index 1c14051a9..777917122 100644 --- a/tests/fixtures/install-tree/kimi.json +++ b/tests/fixtures/install-tree/kimi.json @@ -206,7 +206,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/opencode.json b/tests/fixtures/install-tree/opencode.json index 77225f799..91056c1d6 100644 --- a/tests/fixtures/install-tree/opencode.json +++ b/tests/fixtures/install-tree/opencode.json @@ -241,7 +241,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/pi.json b/tests/fixtures/install-tree/pi.json index 9034e8167..364f12ac4 100644 --- a/tests/fixtures/install-tree/pi.json +++ b/tests/fixtures/install-tree/pi.json @@ -29,7 +29,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/qwen.json b/tests/fixtures/install-tree/qwen.json index e071e177e..95bb924cf 100644 --- a/tests/fixtures/install-tree/qwen.json +++ b/tests/fixtures/install-tree/qwen.json @@ -169,7 +169,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/trae.json b/tests/fixtures/install-tree/trae.json index 1171c16cf..d8e20be28 100644 --- a/tests/fixtures/install-tree/trae.json +++ b/tests/fixtures/install-tree/trae.json @@ -169,7 +169,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/windsurf.json b/tests/fixtures/install-tree/windsurf.json index 19da7c414..3a39db393 100644 --- a/tests/fixtures/install-tree/windsurf.json +++ b/tests/fixtures/install-tree/windsurf.json @@ -97,7 +97,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/fixtures/install-tree/zcode.json b/tests/fixtures/install-tree/zcode.json index a42364f38..16458449d 100644 --- a/tests/fixtures/install-tree/zcode.json +++ b/tests/fixtures/install-tree/zcode.json @@ -241,7 +241,6 @@ "gsd-core/references/checkpoints.md", "gsd-core/references/common-bug-patterns.md", "gsd-core/references/compact-content-gate.md", - "gsd-core/references/compact-content-protected-content.md", "gsd-core/references/context-budget.md", "gsd-core/references/continuation-format.md", "gsd-core/references/debugger-bug-taxonomy.md", diff --git a/tests/helpers/compact-content-split.cjs b/tests/helpers/compact-content-split.cjs new file mode 100644 index 000000000..20cd3db21 --- /dev/null +++ b/tests/helpers/compact-content-split.cjs @@ -0,0 +1,556 @@ +'use strict'; + +/** + * Shared library for the compact-content partition guard (ADR-4139 Decision 5, + * epic #4139, Phase 3 #4403). See `docs/PARTITION-RULES.md` for the operational + * spec this module implements — it is the source of truth for behavior; this + * file is the mechanics. + * + * A "registered split" is any `gsd-core/workflows//detail/*.md` path + * paired with the spine at `gsd-core/workflows/.md`. There is no + * separate registry — a pair is registered by existing on disk + * (`discoverRegisteredSplits`). Everything else here is one of the five + * checks PARTITION-RULES.md describes, or a primitive those checks share: + * + * 1. Completeness — NOT implemented here (fires once, at split time, and + * needs the merge-base spine content the way the pilot test already + * reads it; `normalizeNonTrivialLines` is the shared primitive it needs). + * 2. Disjointness — `checkDisjointness`. + * 3. Registration — `checkRegistration`. + * 4. Protected content — `extractProtectedBlocks` extracts the sentinel- + * wrapped spans; the guard test compares their presence itself. + * 5. Boundary moves — `readBoundaryMoveTrailers`. + * + * `checkDetailFileSizeCap` is the pilot's size-cap assertion, generalized. + * + * This module only reads (filesystem + `git log`/`git merge-base`, both + * read-only). No writes, no network. + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +const { + REPO_ROOT, + GIT_TIMEOUT_MS, + safeDirArgs, +} = require('./emitted-runtime.cjs'); +const { NEW_FILE_CAP, ACK_TRAILER_DELIM, normalizeAckReason } = require('./emitted-diff.cjs'); +const { runGit, OUTCOME } = require('./process-seam.cjs'); + +/** Default scan root: `gsd-core/workflows/` at repo root. */ +const DEFAULT_WORKFLOWS_DIR = path.join(__dirname, '..', '..', 'gsd-core', 'workflows'); + +/** + * Discover every registered spine/detail split under `workflowsDir`. + * + * A split is registered by a `//detail/` DIRECTORY (never + * a `/detail.md` FILE — only an actual `detail/` subdirectory counts) + * containing at least one `*.md` file, where `` is exactly ONE path + * segment directly under `workflowsDir` — a `detail/` dir nested two or more + * segments deep (e.g. `/sub/detail/`) is walked (so its own children + * are still found) but is never itself registered, per PARTITION-RULES.md's + * "no nested split names" contract. + * + * Pure enumeration: a missing spine is reported here as `spineExists: false`, + * never thrown — deciding that's a failure is check 3's (registration) job, + * not discovery's. + * + * @param {string} [workflowsDir] + * @returns {Array<{name: string, spinePath: string, detailPaths: string[], spineExists: boolean}>} + */ +function discoverRegisteredSplits(workflowsDir = DEFAULT_WORKFLOWS_DIR) { + const found = new Map(); // name -> split record, first registration wins + + function walk(dir) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; // unreadable directory: nothing to discover under it + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const full = path.join(dir, entry.name); + + if (entry.name === 'detail') { + let mdFiles = []; + try { + mdFiles = fs.readdirSync(full, { withFileTypes: true }) + .filter((e) => e.isFile() && e.name.endsWith('.md')) + .map((e) => e.name); + } catch { + mdFiles = []; + } + if (mdFiles.length > 0) { + // `dir` is detail/'s PARENT — the segment(s) between workflowsDir + // and dir are the candidate ``. Only a single segment counts. + const segments = path.relative(workflowsDir, dir).split(path.sep).filter(Boolean); + if (segments.length === 1) { + const name = segments[0]; + if (!found.has(name)) { + const spinePath = path.join(workflowsDir, `${name}.md`); + const detailPaths = mdFiles.map((f) => path.join(full, f)).sort(); + found.set(name, { name, spinePath, detailPaths, spineExists: fs.existsSync(spinePath) }); + } + } + } + } + + walk(full); + } + } + + walk(workflowsDir); + return [...found.values()].sort((a, b) => a.name.localeCompare(b.name)); +} + +/** + * "Trivial" lines (fences, `---` rules, bare headings, bare `Label:` lines) + * are boilerplate that legitimately repeats throughout any markdown file with + * code blocks — excluding them from the completeness/disjointness checks is + * what keeps those checks from flaring on structure rather than content. + * Ported verbatim from the pilot's `tests/plan-phase-compact-split.test.cjs`, + * which this module supersedes as Phase 3's generalized version of the same + * check. + */ +function isTrivial(line) { + if (/^`{3,}/.test(line)) return true; + if (/^-{3,}$/.test(line)) return true; + if (/^#+\s*$/.test(line)) return true; + if (/^[A-Za-z][A-Za-z ]*:$/.test(line)) return true; // bare label lines like "Options:" + return false; +} + +/** + * Normalize file content into the comparable unit the completeness and + * disjointness checks both operate on: trimmed, non-empty, non-trivial + * lines, in order, duplicates preserved (callers choose Set vs array + * depending whether they need membership or a count). + * + * @param {string} content + * @returns {string[]} + */ +function normalizeNonTrivialLines(content) { + return content + .split(/\r?\n/) + .map((l) => l.trim()) + .filter((l) => l.length > 0 && !isTrivial(l)); +} + +/** + * Is `line` the canonical `gsd_run` launcher bootstrap preamble (see + * `gsd-core/workflows/_runtime-launcher.snippet.sh`, + * `tests/runtime-launcher-parity.test.cjs`)? + * + * That other guard's OWN contract mandates exactly one inlined copy in every + * workflow/detail file that calls `gsd_run` — spine and detail both call it, + * so both legitimately carry their own copy. That is sanctioned + * cross-file duplication, not something the disjointness check should ever + * flag, so it is filtered out wherever this module compares lines across a + * spine/detail pair. + * + * @param {string} line + * @returns {boolean} + */ +function isCanonicalLauncherPreamble(line) { + return line.startsWith('_GSD_SHIM_NAME="gsd-tools.cjs";'); +} + +const PROTECTED_START = ''; +const PROTECTED_END = ''; +const PROTECTED_SINGLE = ''; + +/** + * Extract every `` (single-line) and + * ``/`` (paired) + * sentinel-wrapped block from `content`. + * + * Never throws on a malformed sentinel (unclosed start, orphan end, a single + * marker with no following content, a paired block enclosing nothing) — + * those are reported best-effort in `blocks` (covering whatever content is + * actually present, or an empty span) AND named explicitly in `malformed`, + * mirroring the well-formedness rules `tests/plan-phase-compact-split.test.cjs`'s + * `'every gsd:protected sentinel...'` test already enforced for the one + * existing split. The check-4 caller decides what to do with a malformed + * sentinel; this function's job is only to describe the file accurately. + * + * Single-marker coverage: from the marker line, skip blank lines, then + * collect (trimmed) lines until a blank line, another sentinel line, or EOF. + * Paired coverage: every non-blank (trimmed) line strictly between the start + * and end sentinel lines. + * + * @param {string} content + * @returns {{ + * blocks: Array<{kind: 'single'|'paired', lines: string[], firstLine: string}>, + * malformed: Array<{line: number, message: string}>, + * }} + */ +function extractProtectedBlocks(content) { + const lines = content.split(/\r?\n/); + const blocks = []; + const malformed = []; + const singleMarkerLines = []; + let openStart = -1; + + for (let i = 0; i < lines.length; i++) { + const trimmed = lines[i].trim(); + if (trimmed === PROTECTED_START) { + if (openStart !== -1) { + // A new start while one is already open: the earlier open span is + // abandoned (best-effort — it never gets a `blocks` entry) and this + // one becomes the tracked open start. + malformed.push({ line: i + 1, message: `nested/unclosed gsd:protected:start at line ${i + 1}` }); + } + openStart = i; + } else if (trimmed === PROTECTED_END) { + if (openStart === -1) { + malformed.push({ line: i + 1, message: `gsd:protected:end with no matching start at line ${i + 1}` }); + continue; + } + const covered = lines.slice(openStart + 1, i).map((l) => l.trim()).filter((l) => l.length > 0); + if (covered.length === 0) { + malformed.push({ + line: openStart + 1, + message: `protected block at lines ${openStart + 1}-${i + 1} encloses no content`, + }); + } + blocks.push({ kind: 'paired', lines: covered, firstLine: covered[0] || '' }); + openStart = -1; + } else if (trimmed === PROTECTED_SINGLE) { + singleMarkerLines.push(i); + } + } + + if (openStart !== -1) { + malformed.push({ line: openStart + 1, message: 'a gsd:protected:start sentinel was never closed' }); + const covered = lines.slice(openStart + 1).map((l) => l.trim()).filter((l) => l.length > 0); + blocks.push({ kind: 'paired', lines: covered, firstLine: covered[0] || '' }); + } + + for (const idx of singleMarkerLines) { + let j = idx + 1; + while (j < lines.length && lines[j].trim() === '') j++; + if (j >= lines.length || lines[j].trim().length === 0) { + malformed.push({ line: idx + 1, message: `single protected marker at line ${idx + 1} has no following content` }); + blocks.push({ kind: 'single', lines: [], firstLine: '' }); + continue; + } + const covered = []; + for (let k = j; k < lines.length; k++) { + const trimmed = lines[k].trim(); + if (trimmed === '' || trimmed === PROTECTED_START || trimmed === PROTECTED_END || trimmed === PROTECTED_SINGLE) break; + covered.push(trimmed); + } + blocks.push({ kind: 'single', lines: covered, firstLine: covered[0] || '' }); + } + + return { blocks, malformed }; +} + +/** Trailer key for check 5 (boundary moves), PARTITION-RULES.md check 5. */ +const ACK_TRAILER_BOUNDARY_MOVE = 'Boundary-Move-Declared'; + +// Record/value separators for the `git log --format` trailer extraction +// below — same ASCII control characters, same escaped-hex-in-`separator=` +// gotcha, as `readAckTrailers` (`tests/helpers/emitted-runtime.cjs`) and +// `ship.md:312`'s own `%(trailers:...)` extraction. Only one trailer key +// space here (unlike the hash/growth pair `readAckTrailers` reads), so no +// field separator is needed — one placeholder per commit record. +const BOUNDARY_RECORD_SEP = '\x1e'; // between commits +const BOUNDARY_VALUE_SEP = '\x1d'; // between multiple values of the SAME trailer key on one commit + +/** + * Bounded git invocation for the boundary-move trailer reader, built on the + * never-throws process seam (`runGit`) so a PER-CALL `timeoutMs` override is + * honored (mirrors `emitted-runtime.cjs`'s private `ackTrailerGit`, which + * this reimplements locally since it is not exported). A git failure THROWS + * — never an empty result — same "a git failure is a hard error" law as + * every other git-touching helper in this test suite. + */ +function boundaryTrailerGit(args, { cwd = REPO_ROOT, timeoutMs = GIT_TIMEOUT_MS } = {}) { + const result = runGit([...safeDirArgs(cwd), ...args], { cwd, timeoutMs }); + if (result.outcome === OUTCOME.EXITED && result.exitCode === 0) { + return result.stdout; + } + throw new Error( + `boundary-move-trailer: \`git ${args.join(' ')}\` failed — outcome=${result.outcome} ` + + `exitCode=${result.exitCode} stderr=${(result.stderr || '').trim()}`, + ); +} + +/** + * Parse raw `Boundary-Move-Declared` trailer VALUES (no git I/O) into the + * declarations map, applying the SAME rules `parseAckTrailers` + * (`tests/helpers/emitted-diff.cjs`) applies to its two spaces, narrowed to + * one: empty key -> error, empty reason -> error, two declarations of the + * SAME key with the SAME reason (after `normalizeAckReason`) -> silent + * dedupe (keep the first), two declarations of the same key with DIFFERENT + * reasons -> error and drop the key entirely (never guess a winner). + * + * Keys here are deliberately NOT validated against `<`/`>`/whitespace or the + * `__proto__`/`constructor`/`prototype` reserved set the ack-trailer parser + * rejects — those defenses exist because `parseAckTrailers`' output keys + * plain OBJECTS (`ackHash`/`ackGrowth` consumers). `declarations` here is a + * `Map`, which has no prototype-pollution surface, so narrowing to only the + * rules PARTITION-RULES.md's check 5 actually specifies avoids inventing + * validation the spec never asked for. + * + * @param {string[]} values + * @returns {{declarations: Map, errors: string[]}} + */ +function parseBoundaryMoveTrailerValues(values) { + const errors = []; + const declarations = new Map(); + const conflicted = new Set(); // keys already reported ambiguous — never resurrected + + for (const raw of values) { + const delimIndex = raw.indexOf(ACK_TRAILER_DELIM); + if (delimIndex === -1) { + errors.push( + `${ACK_TRAILER_BOUNDARY_MOVE}: trailer value ${JSON.stringify(raw)} has no ` + + `"${ACK_TRAILER_DELIM}" delimiter — expected " — "`, + ); + continue; + } + const key = raw.slice(0, delimIndex).trim(); + const reason = raw.slice(delimIndex + ACK_TRAILER_DELIM.length).trim(); + + if (key === '') { + errors.push(`${ACK_TRAILER_BOUNDARY_MOVE}: trailer value ${JSON.stringify(raw)} has an empty key`); + continue; + } + if (reason === '') { + errors.push( + `${ACK_TRAILER_BOUNDARY_MOVE}: trailer value ${JSON.stringify(raw)} has an empty reason — ` + + 'name it and say why', + ); + continue; + } + if (conflicted.has(key)) continue; + + const existing = declarations.get(key); + if (existing === undefined) { + declarations.set(key, { reason }); + } else if (normalizeAckReason(existing.reason) === normalizeAckReason(reason)) { + // Identical after normalization — dedupe silently, keep the first declaration. + } else { + errors.push( + `${ACK_TRAILER_BOUNDARY_MOVE}: trailer key "${key}" is declared twice with ambiguous, ` + + 'conflicting reasons — an ambiguous declaration cannot silently pick a winner', + ); + declarations.delete(key); + conflicted.add(key); + } + } + + return { declarations, errors }; +} + +/** + * Read `Boundary-Move-Declared` commit trailers over `..` + * (never `..` — a two-dot range would let the diff and the + * commit range being checked disagree about what "this PR" means, the same + * correction ADR-3942 made for its own ack trailers). + * + * Fails closed: an uncomputable merge-base (shallow clone, unrelated + * histories, a bad ref) THROWS — never returns an empty Map, which would + * silently read as "no boundary moves to excuse" and disarm check 5 exactly + * the way an empty `readAckTrailers` result would disarm ADR-3942's gate. + * + * @param {{baseRef: string, headRef?: string, cwd?: string, timeoutMs?: number}} opts + * @returns {{declarations: Map, errors: string[]}} + */ +function readBoundaryMoveTrailers({ baseRef, headRef = 'HEAD', cwd = REPO_ROOT, timeoutMs = GIT_TIMEOUT_MS } = {}) { + let mergeBaseOut; + try { + mergeBaseOut = boundaryTrailerGit(['merge-base', baseRef, headRef], { cwd, timeoutMs }); + } catch (err) { + throw new Error( + `boundary-move-trailer: could not compute a merge-base range for "${baseRef}..${headRef}" ` + + '(a bad ref, a shallow clone with no common ancestor, or another git failure): ' + + err.message, + ); + } + const mergeBase = mergeBaseOut.trim(); + if (!/^[0-9a-f]{40}$/.test(mergeBase)) { + throw new Error( + `boundary-move-trailer: git merge-base for "${baseRef}..${headRef}" returned no usable ` + + `commit (${JSON.stringify(mergeBase)}) — the range is structurally uncomputable ` + + '(possibly a shallow clone with no common ancestor).', + ); + } + + const valueSepHex = `%x${BOUNDARY_VALUE_SEP.codePointAt(0).toString(16).padStart(2, '0')}`; + const format = `${BOUNDARY_RECORD_SEP}%(trailers:key=${ACK_TRAILER_BOUNDARY_MOVE},valueonly,separator=${valueSepHex})`; + + let raw; + try { + raw = boundaryTrailerGit(['log', `${mergeBase}..${headRef}`, `--format=${format}`], { cwd, timeoutMs }); + } catch (err) { + throw new Error(`boundary-move-trailer: could not read commit trailers over the range: ${err.message}`); + } + + const normalized = raw.replace(/\r/g, ''); // a CRLF commit message must parse identically to LF + const values = []; + // index 0 is the (empty) text before the FIRST record separator — every + // real record starts with one, by construction of the `--format` string. + const records = normalized.split(BOUNDARY_RECORD_SEP).slice(1); + for (const record of records) { + const field = record.replace(/\n+$/, ''); // git's own between-commit newline + for (const v of field.split(BOUNDARY_VALUE_SEP)) if (v !== '') values.push(v); + } + + return parseBoundaryMoveTrailerValues(values); +} + +/** + * Check 2 (disjointness): no non-trivial line may appear in both a spine and + * any of its detail parts. Checked on every registered pair regardless of + * what a given PR touched — this is what keeps duplication from creeping + * back in after a split is made. + * + * The canonical `gsd_run` launcher preamble is excluded from both sides + * before comparing (`isCanonicalLauncherPreamble`) — it is sanctioned + * cross-file duplication under a different guard's contract, not a + * violation of this one. + * + * Capped at the first 20 reported violations PER SPLIT, to avoid a + * runaway report on a badly-drifted pair; this is a reporting cap only, + * not a detection cap — `ok` (computed by the caller from array length) is + * unaffected either way since any violation at all fails the check. + * + * @param {ReturnType} splits - already + * filtered by the caller to `spineExists: true` entries. + * @returns {Array<{splitName: string, detailPath: string, line: string}>} + */ +function checkDisjointness(splits) { + const violations = []; + const PER_SPLIT_CAP = 20; + + for (const split of splits) { + const spineLines = normalizeNonTrivialLines(fs.readFileSync(split.spinePath, 'utf8')) + .filter((l) => !isCanonicalLauncherPreamble(l)); + const spineSet = new Set(spineLines); + + let reported = 0; + for (const detailPath of split.detailPaths) { + if (reported >= PER_SPLIT_CAP) break; + const detailLines = normalizeNonTrivialLines(fs.readFileSync(detailPath, 'utf8')) + .filter((l) => !isCanonicalLauncherPreamble(l)); + for (const line of detailLines) { + if (reported >= PER_SPLIT_CAP) break; + if (spineSet.has(line)) { + violations.push({ splitName: split.name, detailPath, line }); + reported++; + } + } + } + } + + return violations; +} + +/** A detail-path-shaped substring in spine prose: `/detail/.md`. */ +const DETAIL_REFERENCE_PATTERN = /[\w./-]+\/detail\/[\w./-]+\.md/g; + +/** + * Check 3 (registration): a `/detail/*.md` with no `.md` spine + * fails as an orphan; a spine that never mentions one of its own detail + * paths fails as unreferenced; and any detail-path-SHAPED substring in the + * spine's prose that does not resolve to a real file fails as a dangling + * reference — three independent ways the spine/detail pairing can rot. + * + * Runs uncritically over every split `discoverRegisteredSplits` found, + * including `spineExists: false` entries — that is exactly the orphan case + * this check exists to name. + * + * @param {ReturnType} splits + * @param {string} workflowsDir + * @returns {Array} violations, one of: + * `{kind: 'orphan_detail', name, detailPaths}` + * `{kind: 'unreferenced_detail', name, detailPath}` + * `{kind: 'dangling_reference', name, referencedPath}` + */ +function checkRegistration(splits, workflowsDir) { + const violations = []; + + for (const split of splits) { + if (!split.spineExists) { + violations.push({ kind: 'orphan_detail', name: split.name, detailPaths: split.detailPaths }); + continue; + } + + const spineContent = fs.readFileSync(split.spinePath, 'utf8'); + + for (const detailPath of split.detailPaths) { + const rel = path.relative(workflowsDir, detailPath).split(path.sep).join('/'); + if (!spineContent.includes(rel)) { + violations.push({ kind: 'unreferenced_detail', name: split.name, detailPath }); + } + } + + // The prose spells these out in full — e.g. `gsd-core/workflows/plan-phase/ + // detail/elaboration.md` — i.e. relative to the REPO ROOT (`workflowsDir`'s + // grandparent: `workflowsDir` = `/gsd-core/workflows`), not relative + // to `workflowsDir` itself (verified against plan-phase.md's real + // references; resolving against `workflowsDir` directly double-prefixes + // `gsd-core/workflows/` and false-positives every real reference). + const repoRoot = path.join(workflowsDir, '..', '..'); + const referenced = new Set(spineContent.match(DETAIL_REFERENCE_PATTERN) || []); + for (const referencedPath of referenced) { + const resolved = path.join(repoRoot, referencedPath); + // Containment check: DETAIL_REFERENCE_PATTERN allows `.` and `/` freely, so + // spine prose shaped like `../../../etc/detail/passwd.md` would otherwise + // resolve outside repoRoot and turn this existence check into a path-traversal + // oracle (security review finding, #4403). A resolved path outside repoRoot + // can never be a real detail file this repo ships, so it is reported the same + // as any other dangling reference rather than probed on disk. + const relFromRoot = path.relative(repoRoot, resolved); + const escapesRoot = relFromRoot === '' || relFromRoot.startsWith('..') || path.isAbsolute(relFromRoot); + if (escapesRoot || !fs.existsSync(resolved)) { + violations.push({ kind: 'dangling_reference', name: split.name, referencedPath }); + } + } + } + + return violations; +} + +/** + * Every detail file is a NEW shipped file (it did not exist before its + * split), so it must stay under the same `NEW_FILE_CAP` (32768 bytes, + * `tests/helpers/emitted-diff.cjs`, ADR-1610 Decision point 3) any other + * brand-new workflow/agent file is held to — generalizes the pilot's + * `tests/plan-phase-compact-split.test.cjs` size assertion to every split. + * + * @param {ReturnType} splits + * @returns {Array<{splitName: string, detailPath: string, size: number}>} + */ +function checkDetailFileSizeCap(splits) { + const violations = []; + for (const split of splits) { + for (const detailPath of split.detailPaths) { + const size = fs.statSync(detailPath).size; + if (size >= NEW_FILE_CAP) { + violations.push({ splitName: split.name, detailPath, size }); + } + } + } + return violations; +} + +module.exports = { + DEFAULT_WORKFLOWS_DIR, + discoverRegisteredSplits, + normalizeNonTrivialLines, + isCanonicalLauncherPreamble, + extractProtectedBlocks, + ACK_TRAILER_BOUNDARY_MOVE, + readBoundaryMoveTrailers, + parseBoundaryMoveTrailerValues, + checkDisjointness, + checkRegistration, + checkDetailFileSizeCap, + NEW_FILE_CAP, +}; diff --git a/tests/helpers/inventory-roster.cjs b/tests/helpers/inventory-roster.cjs index 940c1715b..8d4eb1e8d 100644 --- a/tests/helpers/inventory-roster.cjs +++ b/tests/helpers/inventory-roster.cjs @@ -22,10 +22,10 @@ * `cli_modules`, `hooks`. `docs/INVENTORY.md:7` says the file "enumerates every * shipped surface across all six families", and each has its own `##` section. * - * OUT: the two NESTED families — `workflow_steps`, `workflow_modes`. - * `docs/INVENTORY.md` §"Workflow Sub-Files" is an explicit shipped decision that - * these carry no hand-written per-file rows: "Adding a step or mode file requires no - * hand-written row here … The per-file roster deliberately lives in + * OUT: the four NESTED families — `workflow_steps`, `workflow_modes`, `workflow_detail`, + * `workflow_templates`. `docs/INVENTORY.md` §"Workflow Sub-Files" is an explicit shipped + * decision that these carry no hand-written per-file rows: "Adding a step, mode, detail, + * or template file requires no hand-written row here … The per-file roster deliberately lives in * `docs/INVENTORY-MANIFEST.json` rather than being duplicated in this table — 60 * rows that must be hand-maintained in lockstep with a generated artifact is the * drift this file exists to catch." Demanding rows there would override that @@ -59,7 +59,7 @@ /** * Manifest family name → the level-2 heading in `docs/INVENTORY.md` that rosters it. - * The two nested families are absent BY DESIGN (see above). + * The four nested families are absent BY DESIGN (see above). */ const ROSTER_SECTIONS = Object.freeze({ agents: 'Agents', diff --git a/tests/plan-phase-compact-split.test.cjs b/tests/plan-phase-compact-split.test.cjs deleted file mode 100644 index 7b2e9df08..000000000 --- a/tests/plan-phase-compact-split.test.cjs +++ /dev/null @@ -1,172 +0,0 @@ -'use strict'; - -/** - * Issue #4402 (ADR-4139 Decision 5): verifies the plan-phase.md spine / - * plan-phase/detail/ split is complete (nothing lost), disjoint (nothing - * duplicated), size-capped, and preserves the protected-content sentinels - * this pilot draws from gsd-core/references/compact-content-protected-content.md. - * - * Scoped to this one split — Phase 3 (#4403) owns the generalized guard that - * runs this class of check against every future split. - */ - -const { describe, test } = require('node:test'); -const assert = require('node:assert/strict'); -const fs = require('fs'); -const path = require('path'); -const { execFileSync } = require('node:child_process'); -const { GIT_TIMEOUT_MS } = require('./helpers/timeouts.cjs'); - -const ROOT = path.join(__dirname, '..'); -// The commit immediately before this branch split plan-phase.md (PR #4441's merge to next). -const PARENT_SHA = 'e54d3aa159810b2308cd777047b9de9c04de418a'; -const SPINE_REL = 'gsd-core/workflows/plan-phase.md'; -const DETAIL_REL = 'gsd-core/workflows/plan-phase/detail/elaboration.md'; - -/** Trivial lines (fences, rules, bare headings, bare labels) are excluded from - * both the completeness and disjointness checks — they are boilerplate that - * legitimately repeats throughout any markdown file with code blocks, not - * content that could be silently lost or duplicated in a meaningful sense. - * A blanket short-line length cutoff would swallow real content (e.g. the - * 14-char `` sentinel), so short lines are trivial only when - * they match a specific boilerplate shape rather than by length alone. */ -function isTrivial(line) { - if (/^`{3,}/.test(line)) return true; - if (/^-{3,}$/.test(line)) return true; - if (/^#+\s*$/.test(line)) return true; - if (/^[A-Za-z][A-Za-z ]*:$/.test(line)) return true; - return false; -} - -/** The canonical gsd_run launcher preamble (see gsd-core/workflows/_runtime-launcher.snippet.sh, - * tests/runtime-launcher-parity.test.cjs). runtime-launcher-parity mandates exactly one inlined - * copy in EVERY workflow/agent .md file that calls gsd_run — spine and detail both call gsd_run, - * so both are required to carry their own copy. That is sanctioned duplication by a different - * contract, not content this split lost or copy-pasted; exclude it from the disjointness check - * the same way trivial fences/headings are excluded. */ -function isCanonicalLauncherPreamble(line) { - return line.startsWith('_GSD_SHIM_NAME="gsd-tools.cjs";'); -} - -function normalizeNonTrivialLines(content) { - return content - .split(/\r?\n/) - .map((l) => l.trim()) - .filter((l) => l.length > 0 && !isTrivial(l)); -} - -function readParentSpine() { - // -c safe.directory= (scoped to this one invocation, not global config): - // a sandboxed test runner can mount the repo under a UID that doesn't own the - // checkout, and git refuses every operation with "detected dubious ownership" - // until the directory is trusted. Passing it per-call avoids mutating shared - // git config for a check this test alone needs. - return execFileSync('git', ['-c', `safe.directory=${ROOT}`, 'show', `${PARENT_SHA}:${SPINE_REL}`], { - cwd: ROOT, - encoding: 'utf-8', - timeout: GIT_TIMEOUT_MS, - }); -} - -function readCurrent(relPath) { - return fs.readFileSync(path.join(ROOT, relPath), 'utf-8'); -} - -describe('plan-phase compact-content spine/detail split (#4402, ADR-4139 Decision 5)', () => { - test('union of spine + detail contains every non-trivial line the parent commit carried', () => { - const parentLines = normalizeNonTrivialLines(readParentSpine()); - const spineLines = normalizeNonTrivialLines(readCurrent(SPINE_REL)); - const detailLines = normalizeNonTrivialLines(readCurrent(DETAIL_REL)); - const unionSet = new Set([...spineLines, ...detailLines]); - - const missing = parentLines.filter((l) => !unionSet.has(l)); - assert.deepStrictEqual( - missing, - [], - `${missing.length} line(s) from the parent commit are missing from spine+detail:\n${missing.slice(0, 15).join('\n')}${missing.length > 15 ? `\n(+${missing.length - 15} more)` : ''}`, - ); - }); - - test('no non-trivial line appears in both spine and detail', () => { - const spineLines = normalizeNonTrivialLines(readCurrent(SPINE_REL)); - const detailLines = normalizeNonTrivialLines(readCurrent(DETAIL_REL)); - const spineSet = new Set(spineLines); - const duplicated = detailLines - .filter((l) => spineSet.has(l)) - .filter((l) => !isCanonicalLauncherPreamble(l)); - assert.deepStrictEqual( - duplicated, - [], - `${duplicated.length} line(s) appear in both spine and detail:\n${duplicated.slice(0, 15).join('\n')}`, - ); - }); - - test('detail.md is a new shipped file under the NEW_FILE_CAP (32768 bytes, tests/helpers/emitted-diff.cjs)', () => { - const size = fs.statSync(path.join(ROOT, DETAIL_REL)).size; - assert.ok(size < 32768, `plan-phase/detail/elaboration.md is ${size} bytes; NEW_FILE_CAP is 32768`); - }); - - test('the spine is smaller than the parent commit\'s file (eager-window byte reduction is real)', () => { - const parentSize = Buffer.byteLength(readParentSpine(), 'utf-8'); - const spineSize = fs.statSync(path.join(ROOT, SPINE_REL)).size; - assert.ok( - spineSize < parentSize, - `spine (${spineSize}B) is not smaller than the parent commit's plan-phase.md (${parentSize}B)`, - ); - }); - - test('the spine references the shared compact-content gate exactly once', () => { - const spine = readCurrent(SPINE_REL); - const matches = spine.match(/compact-content-gate\.md/g) || []; - assert.strictEqual(matches.length, 1, `expected exactly one reference to compact-content-gate.md, found ${matches.length}`); - }); - - test('every gsd:protected sentinel in the spine is well-formed (start/end paired, or a single-line marker followed by content)', () => { - const spine = readCurrent(SPINE_REL); - const lines = spine.split(/\r?\n/); - let openStart = -1; - const singleMarkers = []; - const pairedBlocks = []; - for (let i = 0; i < lines.length; i++) { - const line = lines[i].trim(); - if (line === '') { - assert.strictEqual(openStart, -1, `nested/unclosed gsd:protected:start at line ${i + 1}`); - openStart = i; - } else if (line === '') { - assert.notStrictEqual(openStart, -1, `gsd:protected:end with no matching start at line ${i + 1}`); - pairedBlocks.push({ start: openStart, end: i }); - openStart = -1; - } else if (line === '') { - singleMarkers.push(i); - } - } - assert.strictEqual(openStart, -1, 'a gsd:protected:start sentinel was never closed'); - assert.ok(pairedBlocks.length >= 4, `expected at least 4 paired protected blocks, found ${pairedBlocks.length}`); - assert.ok(singleMarkers.length >= 2, `expected at least 2 single-line protected markers, found ${singleMarkers.length}`); - - // Each paired block must actually enclose non-trivial content (not an empty/decorative wrap). - for (const block of pairedBlocks) { - const enclosed = lines.slice(block.start + 1, block.end).join('\n').trim(); - assert.ok(enclosed.length > 0, `protected block at lines ${block.start + 1}-${block.end + 1} encloses no content`); - } - // Each single marker must be immediately followed by non-trivial content on the next non-empty line. - for (const idx of singleMarkers) { - let j = idx + 1; - while (j < lines.length && lines[j].trim() === '') j++; - assert.ok(j < lines.length && lines[j].trim().length > 0, `single protected marker at line ${idx + 1} has no following content`); - } - }); - - test('the four protected-content categories named in gsd-core/references/compact-content-protected-content.md are represented among the spine\'s protected blocks', () => { - const spine = readCurrent(SPINE_REL); - // Output-format contracts: - assert.match(spine, /\s*/, 'quality_gate output-format contract must be protected'); - assert.match(spine, /\s*/, 'success_criteria output-format contract must be protected'); - assert.match(spine, /\s*/, 'downstream_consumer output-format contract must be protected'); - // Few-shot example the workflow's own steps depend on: - assert.match(spine, /\s*/, 'failing_direction_contract few-shot example must be protected'); - // Negative instruction / guardrail: - const guardrailCount = (spine.match(/\n> \*\*ORCHESTRATOR RULE[^]*?Never call `ScheduleWakeup`/g) || []).length; - assert.strictEqual(guardrailCount, 2, `expected 2 protected ScheduleWakeup guardrail paragraphs, found ${guardrailCount}`); - }); -}); diff --git a/tests/response-language-coverage.test.cjs b/tests/response-language-coverage.test.cjs index e4c5d988b..6a06699fb 100644 --- a/tests/response-language-coverage.test.cjs +++ b/tests/response-language-coverage.test.cjs @@ -181,6 +181,88 @@ describe('response-language workflow coverage lint (#2529)', () => { ); }); + // #4403 / ADR-4139 §6 — `detail/` is the fourth fragment-directory kind + // (spine + detail split), added to FRAGMENT_DIRS alongside modes/steps/ + // templates. It inherits through the exact same per-file proof as the other + // three: the parent must dispatch it from a read/execute context and be + // itself covered. These pin the same three cases the modes/steps/templates + // tests above pin, for `detail/` specifically, so the extension cannot + // silently become an exemption instead of inheritance. + function detailFragmentFixture({ parentCovered = true, parentNamesFragment = true } = {}) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-response-language-detail-')); + tempDirs.push(root); + fs.mkdirSync(path.join(root, 'autonomous', 'detail'), { recursive: true }); + fs.writeFileSync( + path.join(root, 'autonomous.md'), + [ + parentCovered + ? '@~/.claude/gsd-core/references/response-language-directive.md' + : '# No directive here', + parentNamesFragment + ? 'read and execute `gsd-core/workflows/autonomous/detail/converge-detail.md`' + : 'read and execute `gsd-core/workflows/autonomous/detail/something-else.md`', + ].join('\n') + '\n', + ); + fs.writeFileSync( + path.join(root, 'autonomous', 'detail', 'converge-detail.md'), + 'Extended convergence prose.\n', + ); + return root; + } + + test('a detail/ file inherits coverage from the parent that names it', () => { + const root = detailFragmentFixture(); + assert.strictEqual( + inheritsParentCoverage(root, 'autonomous/detail/converge-detail.md'), + true, + ); + assert.deepStrictEqual(findViolations(root), []); + }); + + test('a detail/ file with an uncovered or non-dispatching parent still fails (inheritance, not an exemption)', () => { + const uncoveredParent = detailFragmentFixture({ parentCovered: false }); + assert.deepStrictEqual( + findViolations(uncoveredParent).map((file) => path.relative(uncoveredParent, file).replaceAll(path.sep, '/')), + ['autonomous.md', 'autonomous/detail/converge-detail.md'], + ); + + const unreferenced = detailFragmentFixture({ parentNamesFragment: false }); + assert.deepStrictEqual( + findViolations(unreferenced).map((file) => path.relative(unreferenced, file).replaceAll(path.sep, '/')), + ['autonomous/detail/converge-detail.md'], + ); + }); + + test('a detail/ file carrying its own inline directive is accepted independently of parent inheritance', () => { + const root = detailFragmentFixture({ parentCovered: false }); + fs.writeFileSync( + path.join(root, 'autonomous', 'detail', 'converge-detail.md'), + 'Apply response_language to all user-facing prose, narration between tool calls included.\n', + ); + // Own directive covers the file even though the parent is uncovered — the + // same behavior modes/steps/templates already get, because + // hasResponseLanguageCoverage is checked before inheritsParentCoverage is + // ever consulted (findViolations). Not double-flagged as redundant either. + assert.deepStrictEqual( + findViolations(root).map((file) => path.relative(root, file).replaceAll(path.sep, '/')), + ['autonomous.md'], + ); + }); + + test('the shipped plan-phase/detail/elaboration.md fixture is covered in the real catalog (#4403)', () => { + // One real example exists today (ADR-4139): prove the recognizer actually + // reaches it, not just a synthetic fixture. + const relative = 'plan-phase/detail/elaboration.md'; + assert.ok( + fs.existsSync(path.join(WORKFLOWS_DIR, relative)), + `expected shipped detail file missing: ${relative}`, + ); + assert.strictEqual(inheritsParentCoverage(WORKFLOWS_DIR, relative), true); + const violations = findViolations(WORKFLOWS_DIR) + .map((file) => path.relative(WORKFLOWS_DIR, file).replaceAll(path.sep, '/')); + assert.ok(!violations.includes(relative), `${relative} must not be a violation in the real catalog`); + }); + // #2558 round 10, Minor D. `inheritsParentCoverage` used to prove the parent // "is the way in" with a bare substring test, so any mention of the fragment // path — a changelog line, a deprecation note, a sentence about the file — diff --git a/tests/run-tests-harness.test.cjs b/tests/run-tests-harness.test.cjs index c27913aef..64a2927a3 100644 --- a/tests/run-tests-harness.test.cjs +++ b/tests/run-tests-harness.test.cjs @@ -2865,3 +2865,60 @@ describe('analyzeChunkEvents (#3889)', () => { assert.strictEqual(result.sawAnyEvent, false); }); }); + +// ─── partitionIsolatedFiles (#4497 codex-config.test.cjs chunk isolation) ─── +// +// 2026-09-07: codex-config.test.cjs (weight 17.87, genuinely measured — see +// scripts/run-tests.cjs's ISOLATED_HEAVY_FILES comment) is pulled out of the +// weight-balanced packing pool and given its own dedicated chunk, on every +// platform, so no future single-file addition can reshuffle a companion into +// its chunk and retrigger the per-chunk timeout two prior incidents already +// hit. These tests pin partitionIsolatedFiles directly — the pure split, not +// the chunk-execution loop around it. +const { ISOLATED_HEAVY_FILES, partitionIsolatedFiles } = require('../scripts/run-tests.cjs'); + +describe('partitionIsolatedFiles (#4497 codex-config.test.cjs chunk isolation)', () => { + test('an isolated-heavy file is split out, in its own bucket, everything else stays packable', () => { + const files = [ + '/repo/tests/a.test.cjs', + '/repo/tests/codex-config.test.cjs', + '/repo/tests/b.test.cjs', + ]; + const { isolated, packable } = partitionIsolatedFiles(files); + assert.deepStrictEqual(isolated, ['/repo/tests/codex-config.test.cjs']); + assert.deepStrictEqual(packable, ['/repo/tests/a.test.cjs', '/repo/tests/b.test.cjs']); + }); + + test('matches by BASENAME, so it isolates regardless of platform path separator or directory prefix', () => { + const files = [ + 'C:\\repo\\tests\\codex-config.test.cjs', + '/repo/tests/subdir/codex-config.test.cjs', + 'codex-config.test.cjs', + ]; + const { isolated, packable } = partitionIsolatedFiles(files); + assert.deepStrictEqual(isolated, files, 'every path ending in the isolated basename must be isolated, regardless of prefix/separator'); + assert.deepStrictEqual(packable, []); + }); + + test('a file with a similar but not exactly matching name is NOT isolated (exact basename match only)', () => { + const files = ['/repo/tests/codex-config-extra.test.cjs', '/repo/tests/my-codex-config.test.cjs']; + const { isolated, packable } = partitionIsolatedFiles(files); + assert.deepStrictEqual(isolated, []); + assert.deepStrictEqual(packable, files); + }); + + test('no isolated-heavy files present: everything is packable, order preserved', () => { + const files = ['/repo/tests/z.test.cjs', '/repo/tests/a.test.cjs']; + const { isolated, packable } = partitionIsolatedFiles(files); + assert.deepStrictEqual(isolated, []); + assert.deepStrictEqual(packable, files); + }); + + test('an empty file list produces two empty buckets', () => { + assert.deepStrictEqual(partitionIsolatedFiles([]), { isolated: [], packable: [] }); + }); + + test('ISOLATED_HEAVY_FILES currently names exactly codex-config.test.cjs (documents the set the fix scoped to)', () => { + assert.deepStrictEqual([...ISOLATED_HEAVY_FILES], ['codex-config.test.cjs']); + }); +}); diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index 9c11c5baf..2f5465acf 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -187,6 +187,71 @@ describe('SIZE: workflow tier hard caps (issue #1074)', () => { // extract rather than tier in — a disclosed, deliberate simplification. }); +describe('SIZE: detail/ subtree is excluded from tier classification (#4403, ADR-4139 §6)', () => { + // ADR-4139 §6 (the `NEW_FILE_CAP` row): a spine's `/detail/.md` + // files are governed SOLELY by the hard, non-waivable NEW_FILE_CAP (32768 bytes, + // exported from tests/helpers/emitted-diff.cjs) -- NEVER by the XL/LARGE/DEFAULT + // tier caps above, because NEW_FILE_CAP carries no per-tier escape hatch the way + // the old pre-#2724 per-file baseline did. + // + // This already holds TRUE BY CONSTRUCTION: measureWorkflows() / listWorkflowStems() + // (scripts/workflow-size.cjs) do a plain, non-recursive fs.readdirSync() over the + // top-level workflows directory, so a `detail/` subdirectory (like the + // modes/steps/templates subdirectories before it) is never walked and never + // contributes a key to SIZES or a stem to ALL_WORKFLOWS -- it is never a candidate + // for capFor()/XL_CAP/LARGE_CAP/DEFAULT_CAP at all. These tests make that explicit + // rather than true-by-omission, and lock it as a regression guard: a future switch + // to a recursive scan must not start tier-classifying detail files. + test('measureWorkflows()/listWorkflowStems() do not recurse into any /detail/ subdirectory', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-size-detail-')); + try { + fs.writeFileSync(path.join(dir, 'sample.md'), 'top-level spine\n'); + const detailDir = path.join(dir, 'sample', 'detail'); + fs.mkdirSync(detailDir, { recursive: true }); + fs.writeFileSync(path.join(detailDir, 'part.md'), 'detail part\n'); + + const sizes = measureWorkflows(dir); + assert.deepEqual( + Object.keys(sizes), ['sample.md'], + 'measureWorkflows() must only key the top-level spine, never a nested detail/ file' + ); + + const stems = listWorkflowStems(dir); + assert.deepEqual( + stems, ['sample'], + 'listWorkflowStems() must not surface a stem for anything under detail/' + ); + } finally { + cleanup(dir); + } + }); + + test('a near-NEW_FILE_CAP-sized detail/ file is excluded from SIZES and never tier-classified', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-size-detail-')); + try { + fs.writeFileSync(path.join(dir, 'sample.md'), 'top-level spine\n'); + const detailDir = path.join(dir, 'sample', 'detail'); + fs.mkdirSync(detailDir, { recursive: true }); + // Sized just under the NEW_FILE_CAP anchor (32768 bytes, + // tests/helpers/emitted-diff.cjs) -- the cap this file is ACTUALLY governed + // by -- and comfortably below every tier cap in this file (DEFAULT_CAP alone + // is 40960), so a regression that started tier-classifying it would still + // pass on size and only be caught by the key-shape assertion below. + const largeBody = 'x'.repeat(32760); + fs.writeFileSync(path.join(detailDir, 'large-part.md'), largeBody); + + const sizes = measureWorkflows(dir); + assert.deepEqual( + Object.keys(sizes), ['sample.md'], + 'a large detail/ file must not appear in SIZES -- it is governed solely by ' + + 'NEW_FILE_CAP (tests/helpers/emitted-diff.cjs), never by XL/LARGE/DEFAULT tiering' + ); + } finally { + cleanup(dir); + } + }); +}); + // A prior "SIZE: per-file workflow baseline (issue #1074)" describe block lived here, // asserting every workflow file's exact byte count against the committed // `tests/workflow-size-baseline.json` snapshot. #2724 (ADR-2719 Phase 4) deletes that