From 640eaee16e08bc5426cec570871cc0503a6c7d4c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 1 Aug 2026 12:12:20 -0400 Subject: [PATCH] chore(#2930): fragmentize execute-phase.md and prove per-runtime composed emission (#2972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#2930): fragmentize plan-phase.md workflow into per-runtime-composed sections Adds src/workflow-fragments.cts (in-file marker parser/composer, ADR-1671 epic #1671 Phase 3), wires it into bin/install.js's copyWithPathReplacement emission path, and pilots the marker grammar on gsd-core/workflows/plan-phase.md. Bookkeeping ripple for the new src/*.cts module: .gitignore, eslint.config.mjs, docs/INVENTORY.md + docs/INVENTORY-MANIFEST.json, and a CONTEXT.md glossary entry. Amends ADR-1671 with open questions 1 and 2 resolutions and records the closed when= applicability grammar. Adds docs/reference/workflow-fragments.md and an ARCHITECTURE.md section documenting the marker authoring model. * fix(#2930): put allow-test-rule issue ref on the same line as the marker lint-allow-test-rule-refs.cjs requires the #NNN issue reference on the same source line as `allow-test-rule:`; it was one line below and read as an unreferenced novel exemption. * docs(#2930): link the orphaned gate-predicates reference from the docs index Found while adding the workflow-fragments reference doc: docs/reference/gate-predicates.md shipped without an entry in docs/README.md, so it was unreachable from the docs index. Fixed inline rather than deferred. Co-Authored-By: Claude Opus 5 (1M context) * fix(#2930): scope composition to workflows, add typed failure reasons Review findings from two orthogonal passes: - Scope composeWorkflow to gsd-core/workflows/ only. It previously ran on every .md the installer copied, so a future agent/command/reference doc documenting the marker syntax with an unfenced example would have been mis-parsed and silently stripped — a lossy drop the phase forbids. - Add a frozen REASON enum; failures attach a typed .reason and tests assert on it instead of matching free-form message text (CONTRIBUTING.md:635-694). - Derive the property generator's when= values from WHEN_VOCABULARY instead of duplicating them (DEFECT.GENERATIVE-FIX). - Add adversarial parser fixtures: Unicode headings, NUL, U+FFFD, BOM, fence-within-fence, tilde and indented fences, lone-CR marker line. - Document why --mvp is structurally unmarkable: its content is interleaved, not sectioned, so the whole-line grammar cannot reach it. Also fixes two stale tests on this branch, each reproduced on the unmodified tree before correction. Co-Authored-By: Claude Opus 5 (1M context) * fix(#2930): retarget the pilot from plan-phase to execute-phase The full remote matrix went red on both Linux lanes. Root cause was ours: tests/phase6-capstone-conformance.test.cjs holds a PRE_PHASE6 ceiling of 94519 bytes for plan-phase.md, asserting an ADR-857 Phase-6 completion property. That is a third size gate beyond the tier caps and the differential ratchet, and it left plan-phase.md just 36 bytes of headroom rather than the 3821 computed from the XL cap. The 330 marker bytes overran it by 294. Raising the ceiling is not an option: it is a red line certifying another ADR's completion. plan-phase.md is reverted to byte-identical origin/next and the pilot moves to execute-phase.md, which has 728 bytes of headroom under its own ceiling and lands at 93147 with 3 marker pairs. The vocabulary narrows to the atoms actually used: always, flag:--wave, state:gap-closure-phase, state:has-prior-phases. Recorded in the ADR: every branch the epic names lives in plan-phase.md, which cannot be fragmentized until caps move from source to emitted bytes. That is direct evidence for the epic's premise and may reorder phases 3-4. Co-Authored-By: Claude Opus 5 (1M context) * chore(#2930): backfill changeset PR number (#2972) * fix(#2930): make the emission install tests portable on Windows The windows-latest lane went red on three tests in the new install suite; Linux was green. Both causes were in the test harness, not the module. Root normalization: the opencode converter always embeds the install root forward-slashed, but the tests stripped it with the native-separator string from mkdtemp. On Windows that never matched, so the root leaked through unstripped — and because the real and stub install roots have different prefix lengths, that length difference landed directly in the byte-delta assertion (344 observed vs 275 expected). Normalize both text and root to one separator form before stripping. @-ref resolution: the helper stripped only the @~/ and @$HOME/ forms, so a Windows absolute ref (@C:/Users/...) fell through and was joined onto the root, producing ...\@C:\Users\... Strip the @ first, then detect absoluteness from the token's own shape (POSIX, drive-letter, or UNC) with no platform branching, so every OS takes the same path. Neither assertion was weakened; the exact-equality byte check is the point of the test and still holds. Co-Authored-By: Claude Opus 5 (1M context) * docs(#2930): document every REASON member and guard the doc/enum parity Code review found the reference doc's 'Fails closed' list covering 10 of the 11 frozen REASON members — MALFORMED_ATTRIBUTES (parseAttrs rejects malformed key="value" syntax) had no bullet, and it is distinct from UNRECOGNIZED_ATTRIBUTE, which is valid syntax with an unknown key. Two parallel surfaces sharing one constant with nothing asserting they agree is the DEFECT.GENERATIVE-FIX class, so the same commit adds the parity assertion: the test derives the enum side from the built module and the doc side by parsing the reference page, keyed on the reason IDENTIFIER rather than prose so a reworded bullet does not break it, and reports set differences in both directions by name. Proven non-vacuous: removing the MALFORMED_ATTRIBUTES bullet turns the suite red naming that exact member; restoring it returns 44/44. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: sim Co-authored-by: Claude Opus 5 (1M context) --- .changeset/wise-goats-roam.md | 5 + .gitignore | 1 + CONTEXT.md | 4 + bin/install.js | 29 + docs/ARCHITECTURE.md | 26 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 1 + docs/README.md | 2 + ...671-dynamic-context-management-platform.md | 5 + docs/reference/workflow-fragments.md | 175 ++++ eslint.config.mjs | 2 + gsd-core/workflows/execute-phase.md | 6 + src/workflow-fragments.cts | 461 +++++++++++ ...930-fragmentize-execute-phase-markers.json | 6 + ...rkflow-fragments-emission.install.test.cjs | 484 +++++++++++ tests/workflow-fragments.property.test.cjs | 198 +++++ tests/workflow-fragments.test.cjs | 776 ++++++++++++++++++ 17 files changed, 2182 insertions(+) create mode 100644 .changeset/wise-goats-roam.md create mode 100644 docs/reference/workflow-fragments.md create mode 100644 src/workflow-fragments.cts create mode 100644 tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json create mode 100644 tests/workflow-fragments-emission.install.test.cjs create mode 100644 tests/workflow-fragments.property.test.cjs create mode 100644 tests/workflow-fragments.test.cjs diff --git a/.changeset/wise-goats-roam.md b/.changeset/wise-goats-roam.md new file mode 100644 index 000000000..e5d574a63 --- /dev/null +++ b/.changeset/wise-goats-roam.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 2972 +--- +**Workflow markdown can now fragmentize into per-runtime-composed sections.** Authors can mark sections of a workflow file with in-file `` markers; per-runtime emission strips the markers and composes the marked sections back byte-identical-or-smaller, piloted on `execute-phase.md`. (#2930) diff --git a/.gitignore b/.gitignore index 7cf309367..851484eb1 100644 --- a/.gitignore +++ b/.gitignore @@ -86,6 +86,7 @@ build/ /gsd-core/bin/lib/cli-skew-check.cjs /gsd-core/bin/lib/context-composer.cjs /gsd-core/bin/lib/context-predicates.cjs +/gsd-core/bin/lib/workflow-fragments.cjs /gsd-core/bin/lib/capability-loader.cjs /gsd-core/bin/lib/capability-source.cjs /gsd-core/bin/lib/capability-ledger.cjs diff --git a/CONTEXT.md b/CONTEXT.md index 5ae7ca12f..43997714a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -197,6 +197,10 @@ Module owning which skills and agents are written to runtime config directories Shared, pure, no-I/O seam owning priority-ordered composition of content fragments within a measured budget (ADR-1671 Decision item 2; extracted from `prompt-budget` by #2929, epic #1671 Phase 2). Interface: `composeWithinBudget({fragments, budget, measure, options}) → {fragments, metadata}`, plus the relocated `headShrink`/`tailTruncate` primitives. **The composer decides; the caller renders** — it returns a plan of surviving fragments with their trimmed content and never emits a rendered string, because `prompt-budget`'s assembly is prompt-shaped (`## Roadmap`, `### `, note in position two) and per-runtime emission renders differently; owning rendering would prevent one seam from serving both. **Budget unit is injected** via `measure(text) → number` (`prompt-budget` passes `estimateTokens`, chars/4; emission passes a byte counter per ADR-1671's "bytes for emission caps"), with `charsPerUnit` as `measure`'s inverse for the proportional-truncate step — the prior code hardcoded `* 4`, which silently assumed the token estimator. **Shrink strategies, not a cutoff:** ADR-1671's literal contract says "priority + binary-search cutoff", which cannot express head-shrink, proportional-truncate-with-floor, or never-droppable classes; the closed strategy set is `verbatim | head-shrink | proportional-truncate(floorChars) | drop`, and a cutoff strategy joins that set when per-runtime emission needs it (Phases 3-4). Ordering is **declaration order**, not a numeric priority field. A `flexReserve`-style floor is a per-fragment guarantee, NOT a budget cap — it may deliberately push the total above the proportional share. The note reserve is deducted **only under pressure** (`baseline > effectiveBudget`, strict): deducting unconditionally drops sections `reserve` units early, which is the PR #3708 regression recorded at `LEARNING.prompt-budget.boundary-gap`. Presence is a **truthy** test, so an empty-string fragment is indistinguishable from an absent one — characterized, not designed. Source of truth: `gsd-core/bin/lib/context-composer.cjs` (generated from `src/context-composer.cts`). Test anchors: `tests/prompt-budget-parity.test.cjs`, `tests/fixtures/prompt-budget-parity/corpus.json`. +### Workflow Fragments Module + +Pure, no-I/O seam owning in-file `` / `` marker parsing and composition for GSD workflow markdown (ADR-1671 Decision item 1 + migration step 4 + open questions 1 & 2; epic #1671 Phase 3, #2930). `parseWorkflowSections` partitions a document into explicit (marked) and gap (unmarked, `explicit: false`) sections in document order — a marker line is removed in full (text + its own terminator), so an unmarked workflow (88 of 89 today) parses to exactly one implicit gap fragment and round-trips byte-identical. `toFragments` maps sections to Context Composer Module fragments, every one `{kind: 'verbatim'}` — non-lossiness in this phase is a structural guarantee of the strategy set, never a large-budget trick. `composeWorkflow` is the emission entry point: parse → `toFragments` → `composeWithinBudget` → `renderFragments`, run BEFORE the per-runtime converters so a marker attribute is stripped before any path-rewrite regex can reach it. **The grammar is deliberately CLOSED** (Greenspun's Tenth Rule): `when=` takes exactly one atom from the frozen `WHEN_VOCABULARY` — `always`, `flag:--wave`, `state:gap-closure-phase`, `state:has-prior-phases` — with no boolean operators, negation, or nesting; an unknown `when=` value throws rather than being silently dropped, and widening the vocabulary requires an ADR amendment, not an organic edit. Fence and HTML-comment interleaving is scanned in one left-to-right pass with two mutually exclusive states, reusing the discipline from the Context Predicates module's fence/comment scan (the two-pass design that caused #2928's silent-skip-to-EOF defect). `when=` is parsed and validated but not yet acted on — applicability selection is Phase 5; this phase lands the authoring model and proves the seam on one pilot workflow (`execute-phase.md`; retargeted from `plan-phase.md`, which sits 36 B under the ADR-857 `PRE_PHASE6` gate and cannot absorb marker overhead). Source of truth: `gsd-core/bin/lib/workflow-fragments.cjs` (generated from `src/workflow-fragments.cts`). Test anchors: `tests/workflow-fragments.test.cjs`, `tests/workflow-fragments.property.test.cjs`, `tests/workflow-fragments-emission.install.test.cjs`. + ### Runtime Artifact Layout Module Module owning the per-runtime mapping from artifact kind to filesystem placement. ADR-3660 defines the typed `kinds` per runtime (`commands`, `agents`, `skills`) with destination subpath, prefix, and stage adapter (with per-runtime converters in `bin/install.js`: `convertClaudeCommandToClaudeSkill`, `…CodexSkill`, `…CopilotSkill`, `…AntigravitySkill`). Owns the per-runtime `nested` skill-bundle decision (#69): a `skillsKind` flag in `src/runtime-artifact-layout.cts` drives whether a runtime receives the nested router layout (6 `gsd-ns-*` routers + concrete skills under `/skills//`) or the flat `skills/gsd-/` layout; the evidence/doc-link matrix is recorded in a comment above `resolveRuntimeArtifactLayout`. Phase 1 applies this seam to the Runtime Surface Module (`surface.cjs:applySurface`); as of #813, `applySurface` applies the same per-runtime skill-body path rewrites as `installRuntimeArtifacts` for `skills` kinds — re-surfacing no longer overwrites installed SKILL.md bodies with converter-default `~/.claude` paths. Per ADR-1508 / #1511 the former `getInstallExports`/`loadInstallExports` relay (a `GSD_TEST_MODE`-guarded `require('bin/install.js')` by which `surface.cjs` reached `computePathPrefix`/`applyRuntimeContentRewritesInPlace`) was DELETED from this module; content rewriting now lives in the Runtime Artifact Conversion Module and `surface.cjs:applySurface` calls its `rewriteStagedSkillBodies` directly. The resolved `scope` is still carried on the `Layout` object so `applySurface` derives the same `pathPrefix` (global `$HOME` form vs. absolute) as a fresh install. Phase 2 is planned to migrate install/uninstall in `bin/install.js` so all lifecycle sites iterate one shared layout table instead of re-encoding runtime layout logic. This design is intended to remove the #3659 class of omissions. Migrations remain under the Installer Migration Module (ADR-0008). The `.gsd-source` marker (#1477) is a two-party provisioning contract that lets source resolution succeed on the Claude global skills layout, which ships `gsd-core/{bin,contexts,references,templates,workflows}` but no `commands/gsd` source tree for `findInstallSourceRoot` to walk up to: the writer is `bin/install.js`, which writes `/.gsd-source` (content: the absolute path to its own `commands/gsd`, terminated by a newline) when `runtime === 'claude' && isGlobal`, guarded by `fs.existsSync` so a half-published package never writes a dangling marker; the reader is `findInstallSourceRoot(configDir)`, which prefers the marker over its walk-up but falls through to the walk-up if the marker is absent, dangling, or empty/whitespace-only. See ADR-3660. diff --git a/bin/install.js b/bin/install.js index bc2ad0085..f44d55a4d 100755 --- a/bin/install.js +++ b/bin/install.js @@ -45,6 +45,9 @@ const { } = require('../gsd-core/bin/lib/worktree-base-ref.cjs'); const { resolveInstallPlan } = require('../gsd-core/bin/lib/runtime-config-adapter-registry.cjs'); const { createImperativeAdapter } = require('../gsd-core/bin/lib/adapter-imperative.cjs'); +// #2930 (epic #1671 Phase 3): strips `` markers from +// workflow .md content at emit time, before any per-runtime rewrite runs. +const { composeWorkflow } = require('../gsd-core/bin/lib/workflow-fragments.cjs'); const runtimeArtifactConversion = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); // Canonical set of hook files shipped to users. Imported here so writeManifest() // records exactly the same set that build-hooks.js copies to hooks/dist/, making @@ -7786,6 +7789,32 @@ function copyWithPathReplacement(srcDir, destDir, pathPrefix, runtime, isCommand // Replace ~/.claude/ and $HOME/.claude/ and ./.claude/ with runtime-appropriate paths // Skip generic replacement for Copilot/Antigravity — their converters handle all paths let content = fs.readFileSync(srcPath, 'utf8'); + + // #2930 (epic #1671 Phase 3): strip `` markers + // BEFORE any per-runtime rewrite so a `.claude/` -> `.windsurf/` regex + // (or any other converter below) never reaches inside a marker + // attribute and corrupts it. composeWorkflow is a no-op (byte-identical + // return) for the 88+ workflows and every non-workflow .md that carries + // no markers, and for a malformed marker it throws loudly naming + // srcPath — never emit a half-composed workflow. + // + // Scoped to gsd-core/workflows/ ONLY (two independent reviewers, + // chore/2930): copyWithPathReplacement is the emit path for every .md + // under gsd-core/, skills/, and commands/ (see the three call sites), + // not just workflows. A doc that merely DOCUMENTS the marker syntax + // with an unfenced example (docs/reference/workflow-fragments.md is + // the live instance of this class, though not under the install tree + // today) would otherwise get silently mis-parsed as a real marker and + // that line lossily dropped — a file class issue #2930 never scoped + // to. Path is normalized UNCONDITIONALLY (backslash paths arrive on + // Linux too — CONTEXT.md path-separator rule) and checked as a + // path-segment match so the recursive descent (srcPath may be several + // directory levels below gsd-core/workflows/) is still caught. + const normalizedSrcPath = srcPath.replace(/\\/g, '/'); + if (/(?:^|\/)gsd-core\/workflows\//.test(normalizedSrcPath)) { + content = composeWorkflow(content, { sourcePath: srcPath }); + } + if (!dispatch.mdSkipGenericRewrite) { const globalClaudeRegex = /~\/\.claude\//g; const globalClaudeHomeRegex = /\$HOME\/\.claude\//g; diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index beb2e5bf5..98370f7a3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -355,6 +355,32 @@ The `CONTEXT.md` predicate fact-store — every backtick-wrapped `CLASS.subkey=v The committed index intentionally carries **no `line` field** for any predicate (ADR-1671 open question 4, resolved by #2928) — committed-but-uncompared metadata goes silently stale, the same defect class the drift-guard exists to catch, with the alarm removed. The live `gsd-tools query context-predicates` parse still returns `line`/`section` for callers that want to cite a source location. See [ADR-1671](adr/1671-dynamic-context-management-platform.md) and [CLI Tools Reference](CLI-TOOLS.md#query-context-predicates). +### Workflow Fragmentization and Emission (`src/workflow-fragments.cts`, ADR-1671) + +Workflow markdown under `gsd-core/workflows/*.md` can mark one or more sections with an +in-file `` / `` pair. A +compiled parser/composer seam (generated to `gsd-core/bin/lib/workflow-fragments.cjs` per +ADR-457) partitions a marked document into fragments and recomposes them through the shared +`context-composer.cjs` budget seam (ADR-1671, #2929) before any per-runtime converter sees the +text — so a marker attribute can never be corrupted by a `.claude/` → `.windsurf/`-style +path-rewrite regex. `bin/install.js`'s `copyWithPathReplacement` calls `composeWorkflow` on +every workflow file at emit time; an unmarked file (88 of the 89 shipped workflows today) +parses to a single implicit fragment and round-trips byte-identical, so this is a no-op for +every workflow that hasn't opted in yet. + +Every fragment in this phase carries the `verbatim` strategy, so composition is structurally +non-lossy — nothing is trimmed regardless of budget. Fence and HTML-comment interleaving +reuses the same LOCAL, single-pass, mutually-suppressing scan discipline as +`context-predicates.cts` (see above), so a marker-shaped line inside a fenced code block or an +unrelated comment is never misread as structural. Markers are **stripped at emit** — the +installed artifact carries no build metadata and is smaller than the source by exactly the +stripped marker bytes. + +See [Reference: Workflow fragments](reference/workflow-fragments.md) for the full marker +grammar, the frozen `when=` vocabulary, and fail-closed authoring rules, and +[ADR-1671](adr/1671-dynamic-context-management-platform.md) (open questions 1 and 2) for why +in-file markers were chosen over separate fragment files or a sidecar manifest. + ### CLI Tools (`gsd-core/bin/`) Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core/bin/lib/` (see [`docs/INVENTORY.md`](INVENTORY.md#cli-modules) for the authoritative roster): diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 0e7870dbd..586f9cf22 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -466,6 +466,7 @@ "verification.cjs", "verify-command-router.cjs", "verify.cjs", + "workflow-fragments.cjs", "workstream-inventory-builder.cjs", "workstream-inventory.cjs", "workstream-name-policy.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index f7bd3981b..25e81d891 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -549,6 +549,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `verification.cjs` | Verification-status routing — consolidates pass/gaps_found/human_needed status from phase verifier-emitted VERIFICATION.md frontmatter (#651) | | `verify-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools verify` | | `verify.cjs` | Plan structure, phase completeness, reference, commit validation | +| `workflow-fragments.cjs` | In-file `` marker parser/composer for GSD workflow markdown (ADR-1671, #2930) — `parseWorkflowSections` (fence/HTML-comment-aware document partition into explicit/gap sections, fail-closed on malformed/unclosed/nested/duplicate markers or an unknown `when=`), `toFragments` (maps sections to `context-composer.cjs` `verbatim` fragments — non-lossy by construction), and `renderFragments`/`composeWorkflow` (compose-within-budget then join, run BEFORE per-runtime converters so a marker attribute never reaches a path-rewrite regex). `WHEN_VOCABULARY` is a frozen 4-atom applicability set (`always`, `flag:--wave`, `state:gap-closure-phase`, `state:has-prior-phases`); widening it is an ADR amendment, not an organic edit. Compiled from `src/workflow-fragments.cts` | | `workstream-inventory-builder.cjs` | Pure workstream inventory projection builder | | `workstream-inventory.cjs` | Shared workstream inventory projection: state fields, phase/plan/summary counts, roadmap phase count, and active marker — thin orchestrator that delegates pure projection to `workstream-inventory-builder.cjs` | | `workstream-name-policy.cjs` | Canonical workstream name validation (`isValidActiveWorkstreamName`, `hasInvalidPathSegment`, `validateWorkstreamName`) and slug normalization (`toWorkstreamSlug`) | diff --git a/docs/README.md b/docs/README.md index 76aa7f4f3..54183964c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -61,9 +61,11 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md) - [PLAN.md schema](reference/plan-md.md) — field-by-field reference for `.planning/phases//PLAN.md` - [Planning artifacts](reference/planning-artifacts.md) — all `.planning/` files and their roles - [Review and verification capabilities](reference/review-verification-capabilities.md) — code review, security, and Nyquist capability ownership and hook contracts +- [Gate predicates](reference/gate-predicates.md) — canonical specification of the phase-gate predicate vocabulary - [Capability matrix](reference/capability-matrix.md) — generated catalogue of every capability's role, tier, extension points, hook kinds, and `engines.gsd` - [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 - [Reviewer Lane Registry](registries/reviewer-registry.md) — generated catalogue of third-party reviewer lanes, with their flags, transport, and install commands --- diff --git a/docs/adr/1671-dynamic-context-management-platform.md b/docs/adr/1671-dynamic-context-management-platform.md index 13c602ea0..4447608cb 100644 --- a/docs/adr/1671-dynamic-context-management-platform.md +++ b/docs/adr/1671-dynamic-context-management-platform.md @@ -66,6 +66,7 @@ Pure Agent Skills (A alone) and pure MCP (D alone) were rejected as the foundati - **Composer contract:** an ordered list of fragments, each carrying a *shrink strategy*; the closed set is `verbatim`, `head-shrink`, `proportional-truncate` (with a per-fragment floor), and `drop`. `flexReserve`-style floors for load-bearing fragments (`META.RULE` citation rules, contribution gates, closing-keyword rules) generalize the existing per-plan 1024-byte floor. A byte-stable canonical prefix (``) is kept identical across runtimes to preserve KV-cache warmth and keep launcher-parity tests green. **Amended by #2929 (Phase 2).** This ADR originally specified the contract as "priority + binary-search cutoff to a per-runtime budget". Implementing Phase 2 established that a cutoff alone **cannot express the function this platform generalizes**: `prompt-budget.applyBudget` is not a cutoff but a fixed five-step ladder in which each section carries its own shrink strategy, and only three of its eight sections are ever droppable — `PROJECT.md` is head-shrunk to N lines and plans are proportionally tail-truncated with a per-plan floor, while instructions and roadmap are never trimmed at all. A cutoff composer sorts by priority and discards the tail; it has no way to say "shrink this one", "truncate that one but never below its floor", or "these three are the only droppables, in this order". Building to the literal wording and routing `prompt-budget` through it would have silently changed review-prompt output. Shrink strategies are therefore the core abstraction, and **binary-search cutoff becomes one strategy among them** — the right one for per-runtime emission in Phases 3-4, not for this ladder. Ordering is declaration order rather than a numeric priority field. This is an elaboration of the decision's intent, not a reversal of it. +- **Applicability grammar (added by #2930, Phase 3).** The fragment unit's `when=` attribute is deliberately a CLOSED grammar: exactly one atom from a frozen vocabulary — `always`, `flag:--wave`, `state:gap-closure-phase`, `state:has-prior-phases` — with no boolean operators, negation, or nesting, and an unknown `when=` value throws rather than being ignored. This is a Greenspun's-Tenth-Rule guard: left open-ended, `when=` acquires `&&`/`!`/precedence/runtime-capability predicates and becomes an ad-hoc, informally-specified predicate language grown one condition at a time. Widening the vocabulary requires a coordinated ADR amendment, not an organic edit. `when=` is parsed and validated in Phase 3 but not yet acted on; applicability selection is Phase 5. - **Budget unit:** bytes for emission caps (matches `lfByteCount`, deterministic, offline-safe); a token estimate for run-time selection. - **Determinism + drift-guard:** every generated artifact follows the universal `--check`/`--write` idiom and is committed; any constant shared between two surfaces gets a `DEFECT.GENERATIVE-FIX` parity assertion. Caps are asserted on **emitted per-runtime bytes** via real spawn-install tests (engine-direct tests are false-green for install behavior). - **Boundary coverage:** the composer's budget logic is tested at `cap-1 / cap / cap+1` per `RULESET.TESTS.boundary-coverage`. @@ -132,6 +133,10 @@ Prototype scope notes: the parser is intentionally self-contained for the exampl **Resolved by other work — not carried as open.** A fourth question was proposed in review (#1671, 2026-06-25): *what populates the eval-gate assertion set, and is it graded exogenously?* Since that review, the answer has landed as first-class predicate classes rather than remaining a design gap: `PROBE.principle` (`verifier-reach-equals-spec-reach`), `PROBE.family` (edge-probe + prohibition-probe + ui-consideration-probe), `PROBE.protocol` (recall → precision), and `PROHIB.judgment-tier` (exogenous grading) — see ADR-550 D4/D7 and ADR-1606. The `PROHIB.*` predicates live in the same `CONTEXT.md` store this ADR formalizes, which is the single-store property that review asked for. +**Resolved by #2930 (Phase 3) — fragment unit: in-file `` markers.** Question 1 asked separate files vs in-file section markers. Confirmed with the maintainer: separate files are eliminated by this phase's own acceptance criterion — "emitted output byte-identical-or-smaller" — because splitting a workflow into files changes the emitted tree's *shape*, which is neither identical nor smaller, it is different; it also multiplies INVENTORY rows and `@`-ref contract surface for no Phase-3 benefit. A sidecar fragment manifest keyed on heading anchors was also rejected: zero source growth, but it creates a second surface that drifts from the workflow — the exact multi-surface edit pain the epic exists to remove (`DEFECT.GENERATIVE-FIX`), and directly against the epic's "one fragment, not 4 surfaces" thesis. The shipped answer is in-file markers, stripped at emit so the installed artifact carries no build metadata and shrinks; markers are self-anchoring (no line-number keying — Open question 4 already rejected that for the predicate index, and the same reasoning applies here), and the existing `` block at `plan-phase.md:1` is in-repo precedent for the form. Production landed under `src/workflow-fragments.cts` → `gsd-core/bin/lib/workflow-fragments.cjs` (ADR-457 build-at-publish), piloted on `execute-phase.md`. **The pilot was retargeted from `plan-phase.md` mid-phase, and the reason is itself the most important finding here.** The branches the epic names as motivating (`--prd`, `--ingest`, `--mvp`, `--reviews`) all live in `plan-phase.md` — but `plan-phase.md` sits only 36 B under an independent, pre-existing size gate (`tests/phase6-capstone-conformance.test.cjs`'s `PRE_PHASE6`, an ADR-857 Phase-6 completion property that this ADR's own Blast-radius analysis did not enumerate against, catching only the XL cap). It cannot absorb even the smallest marker overhead, so **it could not be fragmentized at all under this phase's grammar**, independent of any shape limitation. The pilot instead proves the mechanism on state- and flag-gated `` blocks in `execute-phase.md` (`partial-wave`/`flag:--wave`, `gap-closure-artifacts`/`state:gap-closure-phase`, `regression-gate`/`state:has-prior-phases`), which has 728 B of real headroom under its own `PRE_PHASE6` gate. This is direct evidence for the epic's premise that fragmentization pays off, but it also means **Phase 4 (moving size caps from source bytes to emitted bytes) may need to land before `plan-phase.md` itself can be fragmentized.** Separately, and independent of the size-gate finding: the marker grammar addresses SECTION-shaped branches only — a whole-line, non-nesting comment pair around a contiguous block — and `--mvp`'s content in `plan-phase.md` is INTERLEAVED rather than sectioned (`MVP_MODE` resolution shares a bash block with `--tdd`/`--no-tracer`/`--no-reversibility-gates` at `plan-phase.md:125-158`, and is inline `${MVP_MODE === 'true' ? ... }` template interpolation at `:794-803`), so `--mvp` would remain unmarkable by this grammar even if the size gate allowed it. Phase 6 must either accept that gap or introduce a finer-grained (sub-line) mechanism for interleaved branches. + +**Resolved by #2930 (Phase 3) — build-time emission is the primary surface; per-workflow cutover, no double-write.** Question 2 asked build-time emission vs run-time assembly as the primary surface during migration, and whether that requires a double-write period. Because markers are stripped at emit, an unmarked workflow parses to exactly one implicit fragment and composes back byte-identical by construction — that structural guarantee is what makes a per-workflow cutover safe file-by-file, with no double-write period and no flag day: a workflow can gain markers on its own schedule without touching any other workflow's emission path. Phase 5's run-time selection is planned to consume a build-derived manifest, not markers read at run time, keeping the run-time surface decoupled from the authoring surface. + ## Related - ADR-0002 — Command Contract Validation Module (the stub `` @-ref contract this platform's emission must keep satisfying). diff --git a/docs/reference/workflow-fragments.md b/docs/reference/workflow-fragments.md new file mode 100644 index 000000000..d91db84cc --- /dev/null +++ b/docs/reference/workflow-fragments.md @@ -0,0 +1,175 @@ +# Workflow fragments (reference) + +> **Diátaxis quadrant:** Reference. This is the canonical specification of the +> in-file `` marker grammar used to fragmentize GSD workflow +> markdown for per-runtime emission. For the surrounding seam (why it exists and +> how it composes with the shared budget composer), see +> [Architecture: Workflow Fragmentization and Emission](../ARCHITECTURE.md#workflow-fragmentization-and-emission-srcworkflow-fragmentscts-adr-1671) +> and [ADR-1671](../adr/1671-dynamic-context-management-platform.md) (open +> questions 1 and 2). + +Workflow authors can mark one or more sections of a `gsd-core/workflows/*.md` file +so that `bin/install.js`'s emission path can compose them per runtime. Today this +is an authoring model with no run-time effect yet — see +[Not acted on yet](#not-acted-on-yet) below. + +## Marker syntax + +An open marker is a line whose only content (after trimming leading/trailing +whitespace) is: + +```html + +``` + +A close marker is a line whose only content is: + +```html + +``` + +- Attribute order is free and inner spacing around `=` and between attributes + is flexible. +- `id` must match `/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/` and must be unique + within one file. +- `when` must be exactly one entry of the frozen vocabulary below — no + operators, no negation, no nesting. +- Both `id` and `when` are **required** on every open marker; a marker missing + either attribute fails closed (see [Fails closed](#fails-closed)). + +Text between an open marker and its matching close marker is that section's +body, byte-for-byte (including its own line terminators). Text outside any +marker pair becomes an implicit "gap" fragment — the file's ordinary, +unmarked content — so a workflow with no markers at all parses to exactly one +gap fragment and composes back byte-identical to its source. + +## The frozen `when=` vocabulary + +`when=` takes exactly one of: + +| Value | Meaning | +|---|---| +| `always` | Section is always applicable. | +| `flag:--wave` | Applicable when the workflow runs with `--wave`. | +| `state:gap-closure-phase` | Applicable when the phase number is a gap-closure phase (has a decimal, e.g. `4.1`). | +| `state:has-prior-phases` | Applicable when prior phases (and their `VERIFICATION.md` files) exist. | + +This list is **closed by design** (Greenspun's Tenth Rule): left open-ended, +`when=` would acquire boolean operators, negation, precedence, and +runtime/capability predicates one edit at a time, becoming an ad-hoc, +informally-specified applicability language. Widening the vocabulary is a +coordinated ADR amendment to ADR-1671, never an organic edit to the parser. + +## Fails closed + +An authoring mistake throws at parse time, naming the source file and 1-based +line number, rather than being silently dropped or swallowed to end-of-file: + +- Missing `id=` or `when=` attribute (`MISSING_ID`, `MISSING_WHEN`). +- `when=` value not in the frozen vocabulary above, including any boolean + operator or negation form (`UNKNOWN_WHEN`). +- `id=` value that does not match the id grammar (`MALFORMED_ID`). +- Malformed attribute syntax on an open marker — the attribute text is not a + run of well-formed `key="value"` tokens (e.g. an unterminated quote or a + duplicate attribute key) (`MALFORMED_ATTRIBUTES`). +- An unrecognized attribute on an open marker (`UNRECOGNIZED_ATTRIBUTE`). +- A close marker carrying attributes (`CLOSE_WITH_ATTRIBUTES`). +- An unmatched close marker, i.e. close with no open (`UNMATCHED_CLOSE`). +- A nested marker, i.e. open marker while already inside an open section + (`NESTED_SECTION`). +- A duplicate `id=` within one file (`DUPLICATE_ID`). +- An open marker with no matching close before end of file + (`UNCLOSED_SECTION`). + +An unrecognized `when=` is treated as an authoring instruction that must never +be silently ignored, not as a value to fail open on — this is deliberately +asymmetric with the marker *formatting* tolerance above (free attribute order, +flexible spacing), which is liberal by design. + +## Markers are stripped at emit + +Composition runs `parseWorkflowSections` → map sections to fragments → the +shared `context-composer.cjs` budget seam (every fragment uses the `verbatim` +strategy, so nothing is trimmed) → re-join fragment bodies in document order. +The marker lines themselves are never part of any fragment body, so the +composed output — and therefore every installed runtime artifact — contains +no `gsd:section` markers at all. An unmarked file composes to itself exactly; +a marked file composes to itself minus the marker line bytes. + +Composition runs **before** the per-runtime converters (the `.claude/` → +`.windsurf/`-style path and reference rewrites), so a marker's `id`/`when` +attribute text is never exposed to a rewrite regex. + +## Fenced and commented lookalikes are literal + +A ``-shaped line inside a fenced code block (three or +more backticks or tildes, CommonMark-style) is **not** a marker — it is +literal fence content, because workflows document their own marker syntax in +fenced examples (as in this page and in the workflow files themselves). The +same applies to a `gsd:section` mention inside an unrelated HTML comment, or +in prose/backtick text that never opens a real one-line comment. Fence and +comment detection run as a single interleaved left-to-right scan, mirroring +the discipline used by the `CONTEXT.md` predicate parser +(`src/context-predicates.cts`): while a fence is open, only a matching closer +can end it; while a comment is open, only `-->` can end it; an unclosed fence +running to end of file is not an error — everything after it is simply +literal. + +The pre-existing `` marker family (consumed by +`scripts/gen-loop-host-contract.cjs`) is a different, already-established +marker and is never treated as a `gsd:section` marker. + +## Not acted on yet + +`when=` is parsed and validated today, but applicability selection — actually +choosing which sections apply to a given invocation — is not implemented in +this phase. Every fragment composes into the output regardless of its `when=` +value; only the marker lines are stripped. Run-time selection is planned for +a later phase of ADR-1671's epic. + +## Piloted on one workflow so far + +Only `gsd-core/workflows/execute-phase.md` carries markers today. The marker +grammar and composer seam are general-purpose across any workflow file, but +rollout to other LARGE/XL workflows is intentionally sequenced as later work, +not part of this phase. + +The pilot marks three `` blocks: `partial-wave` (`flag:--wave`), +`gap-closure-artifacts` (`state:gap-closure-phase`), and `regression-gate` +(`state:has-prior-phases`). + +**The pilot was retargeted from `plan-phase.md` mid-phase.** Issue #2930's +own motivating mutually-exclusive branches (`--prd`, `--ingest`, `--mvp`, +`--reviews`) all live in `plan-phase.md`, not `execute-phase.md`. But +`plan-phase.md` sits only 36 B under an independent, pre-existing size gate +(`tests/phase6-capstone-conformance.test.cjs`'s `PRE_PHASE6`, an ADR-857 +Phase-6 completion property) and cannot absorb any marker overhead at all — +so it could not be fragmentized under this phase's grammar regardless of +branch shape. This is direct evidence for the epic's premise that +fragmentization pays off, and it also means Phase 4 (moving size caps from +source bytes to emitted bytes) may need to land before `plan-phase.md` +itself can be fragmentized. Separately, and independent of the size-gate +finding, `--mvp` would remain unmarkable by this grammar even if the size +gate allowed it: its content in `plan-phase.md` is INTERLEAVED with other +flags rather than living in its own contiguous section (`MVP_MODE` +resolution shares a single bash block with `--tdd`, `--no-tracer`, and +`--no-reversibility-gates` handling at `plan-phase.md:125-158`, and +elsewhere it is inline `${MVP_MODE === 'true' ? ... }` template +interpolation embedded inside the planner prompt at `plan-phase.md:794-803`) +— the marker grammar is closed, non-nesting, and whole-line (see +[Marker syntax](#marker-syntax) above), with no way to wrap part of a line +or split a shared conditional block without either corrupting the +conditional or bundling unrelated flags into one section. See +[ADR-1671](../adr/1671-dynamic-context-management-platform.md) open +question 1's resolution for the full record, and Phase 6 (LARGE/XL rollout) +for how both limits get addressed. + +## Related + +- [ADR-1671](../adr/1671-dynamic-context-management-platform.md) — the + platform decision record, including open questions 1 (fragment unit) and 2 + (build-time vs. run-time emission), both resolved by this phase. +- [Architecture: Workflow Fragmentization and Emission](../ARCHITECTURE.md#workflow-fragmentization-and-emission-srcworkflow-fragmentscts-adr-1671). +- `src/workflow-fragments.cts` — the compiled parser/composer source. +- `src/context-composer.cts` — the shared budget-composition seam consumed by + `composeWorkflow`. diff --git a/eslint.config.mjs b/eslint.config.mjs index 6f126a87c..0da9e9a21 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -238,6 +238,8 @@ export default tseslint.config( 'gsd-core/bin/lib/context-predicates.cjs', // #2929: tsc-generated runtime artifact — lint the src/context-composer.cts source. 'gsd-core/bin/lib/context-composer.cjs', + // ADR-1671 (#2930): tsc-generated runtime artifact — lint the src/workflow-fragments.cts source. + 'gsd-core/bin/lib/workflow-fragments.cjs', ], }, diff --git a/gsd-core/workflows/execute-phase.md b/gsd-core/workflows/execute-phase.md index 3b55624b9..8e69bdeee 100644 --- a/gsd-core/workflows/execute-phase.md +++ b/gsd-core/workflows/execute-phase.md @@ -1171,6 +1171,7 @@ If an active secure-phase step hook exists AND SECURITY.md exists: check frontma ``` + If `WAVE_FILTER` was used, re-run plan discovery after execution: @@ -1201,6 +1202,7 @@ Selected wave finished successfully. This phase still has incomplete plans, so p - continue with the normal phase-level verification and completion flow below - this means the selected wave happened to be the last remaining work in the phase + **This step is REQUIRED to evaluate the capability hook.** When the code-review capability is active, auto-invoke code review on the phase's source changes. Advisory only — never blocks execution flow. Also dispatches advisory execute:post gate hooks (e.g. tdd.review-checkpoint). @@ -1255,6 +1257,7 @@ Resolve and re-run /gsd execute-phase, or override with /gsd execute-phase {phas **Proceed rule:** If `MVP_MODE && TDD_MODE && GATE_RESULT.block == true` for `tdd.review-checkpoint`: STOP — do NOT proceed to `close_parent_artifacts`, `regression_gate`, `verify_phase_goal`, or `phase.complete`. Otherwise proceed normally. + **For decimal/polish phases only (X.Y pattern):** Close the feedback loop by resolving parent UAT and debug artifacts. @@ -1304,7 +1307,9 @@ mv .planning/debug/{slug}.md .planning/debug/resolved/ gsd_run query commit "docs(phase-${PARENT_PHASE}): resolve UAT gaps and debug sessions after ${PHASE_NUMBER} gap closure" --files .planning/phases/*${PARENT_PHASE}*/*-UAT.md .planning/debug/resolved/*.md ``` + + Run prior phases' test suites to catch cross-phase regressions BEFORE verification. @@ -1353,6 +1358,7 @@ Options: If `TEXT_MODE` is true, present as a plain-text numbered list and ask the user to type their choice number. Otherwise, use AskUserQuestion to present the options. + Verify phase achieved its GOAL, not just completed tasks. diff --git a/src/workflow-fragments.cts b/src/workflow-fragments.cts new file mode 100644 index 000000000..79646efb9 --- /dev/null +++ b/src/workflow-fragments.cts @@ -0,0 +1,461 @@ +/** + * Workflow Fragments — in-file `` marker parser/composer + * for GSD workflow markdown files (ADR-1671 epic #1671, Phase 3 / issue #2930, + * `.gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md`). + * + * Pure module: no I/O, no dependency beyond node built-ins and the shared + * budget-trim seam `context-composer.cjs` (issue #2929). Emission order is + * `parseWorkflowSections` -> `toFragments` -> `composeWithinBudget` -> + * `renderFragments` (= `composeWorkflow`), run BEFORE the per-runtime + * converters so a marker attribute never reaches a path-rewrite regex. + * + * ## Marker grammar (CLOSED) + * + * Open: a line whose only content (after trimming leading/trailing + * whitespace) is ``. + * Close: a line whose only content is ``. + * + * Attribute order is free and inner spacing is flexible (Postel on FORMAT); + * `id` and `when` VALUES are validated strictly and fail closed (Postel is + * deliberately NOT applied to semantics — an unrecognized `when` is an + * authoring instruction that must never be silently dropped). `id` matches + * `/^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/`; `when` must be `===` exactly one + * entry of the frozen {@link WHEN_VOCABULARY} — no operators, no negation, + * no nesting (Greenspun's Tenth Rule: extending the vocabulary is a + * coordinated ADR amendment, never an organic edit). + * + * ## Partition invariant + * + * `parseWorkflowSections` returns sections that PARTITION the document: + * every byte that is not part of a marker LINE belongs to exactly one + * section, in document order. Text outside any marker pair becomes a + * synthesized gap section (`explicit: false`, id `gap-`, n from 0). A + * marker line is removed IN FULL — text and its original line terminator — + * so an unmarked document (88 of 89 workflows today) parses to exactly one + * implicit gap fragment and composes back byte-identical. + * + * Line splitting is CRLF-aware per line (not `content.split('\n')`, which + * would leave a stray `\r` glued to `.text` and cannot express a mixed + * CRLF-marker/LF-body document): {@link splitLinesPreservingEol} records + * each line's own terminator (`''`, `'\n'`, or `'\r\n'`) so reassembly is + * exact regardless of line-ending mixture. + * + * ## Fence + comment interleaving (the highest-risk code here) + * + * A marker is structural only when it is NOT inside a fenced code block and + * NOT inside an unrelated HTML comment (`` is a + * different marker family entirely and is left untouched by construction — + * it does not match the `gsd:section` token). Fences and comments are + * scanned in ONE left-to-right interleaved pass with two mutually exclusive + * states (`fence`, `inComment`), copying the discipline documented in + * `src/context-predicates.cts`'s module comment (DEFECT.CONTEXT-PREDICATES- + * COMMENT-FENCE-BLIND, #2928): while a fence is open, only a matching closer + * can end it (a `` token on a fenced line is fence content, never + * a comment boundary); while a comment is open, only a `-->` token can end + * it (a fence delimiter inside it is comment content, never a fence + * boundary); when neither is open, a comment opener is checked BEFORE a + * fence opener (HTML comments are lexically outermost). A two-pass design + * (mask one construct, then scan for the other) resolves this wrongly in + * one direction and silently skips to EOF — that is the exact defect this + * module avoids by construction. An unclosed fence at EOF does NOT throw; + * everything after it is simply literal. + * + * One deliberate refinement beyond a naive "does the trimmed line START + * WITH ``" check: whether a comment PERSISTS past + * the current line is decided by `.includes('-->')` (does a close token + * appear anywhere on the line), not by `.endsWith('-->')`. A line like + * ` some trailing prose` closes its comment on the same + * line and must not swallow the rest of the document — it is simply not a + * `gsd:section` marker (a marker's grammar requires the comment to be the + * line's ONLY content), and is left as ordinary content in whichever + * section/gap contains it. + * + * Known inherited limitation (shared with `context-predicates.cts`, not a + * regression introduced here): comment-open detection is anchored to the + * start of the trimmed line. An HTML comment that opens *mid-line* (prose + * followed by an unclosed `` open marker's line for an explicit section, or the first line of the gap for a synthesized one. */ + readonly startLine: number; +} + +/** One line of source content plus its ORIGINAL terminator, individually. */ +interface LineRecord { + readonly text: string; + readonly eol: '' | '\n' | '\r\n'; +} + +/** + * Split `content` into per-line records that each carry their OWN original + * terminator, so CRLF/LF mixes and a missing trailing terminator reassemble + * byte-for-byte via `record.text + record.eol` concatenation. See the + * module doc comment's "Line splitting is CRLF-aware" note for why a bare + * `content.split('\n')` cannot serve this. + * + * @param content - full source document text + */ +function splitLinesPreservingEol(content: string): LineRecord[] { + const lines: LineRecord[] = []; + let i = 0; + while (i < content.length) { + const nlIdx = content.indexOf('\n', i); + if (nlIdx === -1) { + lines.push({ text: content.slice(i), eol: '' }); + break; + } + const hasCr = content[nlIdx - 1] === '\r'; + const end = hasCr ? nlIdx - 1 : nlIdx; + lines.push({ text: content.slice(i, end), eol: hasCr ? '\r\n' : '\n' }); + i = nlIdx + 1; + } + return lines; +} + +// Fence delimiter line matcher — mirrors `context-predicates.cts`'s (itself +// mirroring `markdown-sectionizer.cts`'s `scanFencedBlocks`) exactly: >=3 +// backticks/tildes, <=3-space indent tolerance. This is a single-line +// fence-OPENER/CLOSER probe, not a multiline fence-block-strip regex — it +// does not trip `local/no-adhoc-markdown-parsing`'s fenceRegex fingerprint +// (no `[\s\S]` multiline body in the pattern). +const FENCE_DELIM_RE = /^( {0,3})(`{3,}|~{3,})(.*)$/; + +const OPEN_TAG_RE = /^gsd:section(?=\s|$)/; +const CLOSE_TAG = '/gsd:section'; +const ID_RE = /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/; + +/** + * Parse a candidate marker's attribute string (everything after `gsd:section`, + * already trimmed) into a `key -> value` map, or `null` if it does not + * consist entirely of zero-or-more `key="value"` tokens (attribute order is + * free; spacing around `=` and between tokens is flexible — Postel on + * FORMAT). Returns `null` on a duplicate attribute key too. + * + * @param attrsPart - the marker's attribute text, e.g. `id="x" when="always"` + */ +function parseAttrs(attrsPart: string): Map | null { + const attrs = new Map(); + let remaining = attrsPart; + const ATTR_RE = /^\s*([A-Za-z][A-Za-z0-9_-]*)\s*=\s*"([^"]*)"/; + while (remaining.length > 0) { + const m = ATTR_RE.exec(remaining); + if (!m) return null; + const [full, key, value] = m; + if (attrs.has(key)) return null; + attrs.set(key, value); + remaining = remaining.slice(full.length); + } + return attrs; +} + +/** A `TypeError` carrying a stable {@link REASON} code alongside the human-readable message. */ +export interface WorkflowFragmentsError extends TypeError { + readonly reason: string; +} + +/** + * Throws a `TypeError` naming `sourcePath` (when given) and the 1-based + * `line`, carrying `reason` (one of {@link REASON}) as a typed property so + * callers/tests never need to pattern-match the message prose. + */ +function fail(sourcePath: string | undefined, line: number, reason: string, message: string): never { + const loc = sourcePath ? `${sourcePath}:${line}` : `line ${line}`; + const err = new TypeError(`workflow-fragments: ${message} (${loc})`) as TypeError & { reason: string }; + err.reason = reason; + throw err; +} + +/** Result of classifying a complete one-line HTML comment's inner text. */ +type MarkerClassification = + | { readonly kind: 'open'; readonly id: string; readonly when: string } + | { readonly kind: 'close' } + | { readonly kind: 'none' }; + +/** + * Classify a complete one-line HTML comment's inner text (already stripped + * of `` and trimmed) as a `gsd:section` open attempt, a close + * marker, or "not a marker at all" — including `gsd:loop-host` and any + * other unrelated comment, which never match the `gsd:section` token and + * fall through to `{kind: 'none'}` untouched. Throws on any STRUCTURAL + * violation of a recognized open/close attempt (fail-closed grammar). + * + * @param inner - the comment's inner text, e.g. `gsd:section id="x" when="always"` + * @param sourcePath - optional file path named in thrown errors + * @param lineNo - 1-based line number named in thrown errors + */ +function classifyMarker(inner: string, sourcePath: string | undefined, lineNo: number): MarkerClassification { + if (inner === CLOSE_TAG) { + return { kind: 'close' }; + } + if (inner.startsWith(CLOSE_TAG) && /^\s/.test(inner.slice(CLOSE_TAG.length))) { + fail(sourcePath, lineNo, REASON.CLOSE_WITH_ATTRIBUTES, 'close marker must not carry attributes'); + } + if (!OPEN_TAG_RE.test(inner)) { + return { kind: 'none' }; + } + + const attrsPart = inner.slice('gsd:section'.length).trim(); + const attrs = parseAttrs(attrsPart); + if (attrs === null) { + fail(sourcePath, lineNo, REASON.MALFORMED_ATTRIBUTES, 'malformed section marker attributes'); + } + const extraKeys = [...attrs.keys()].filter((k) => k !== 'id' && k !== 'when'); + if (extraKeys.length > 0) { + fail(sourcePath, lineNo, REASON.UNRECOGNIZED_ATTRIBUTE, `unrecognized attribute "${extraKeys[0]}" on section marker`); + } + const id = attrs.get('id'); + const when = attrs.get('when'); + if (id === undefined) { + fail(sourcePath, lineNo, REASON.MISSING_ID, 'section marker missing required "id" attribute'); + } + if (when === undefined) { + fail(sourcePath, lineNo, REASON.MISSING_WHEN, 'section marker missing required "when" attribute'); + } + if (!ID_RE.test(id)) { + fail(sourcePath, lineNo, REASON.MALFORMED_ID, `section marker "id" value "${id}" does not match ${ID_RE}`); + } + if (!WHEN_VOCABULARY.includes(when)) { + fail(sourcePath, lineNo, REASON.UNKNOWN_WHEN, `section marker "when" value "${when}" is not in the frozen WHEN_VOCABULARY`); + } + return { kind: 'open', id, when }; +} + +/** + * Parse a workflow document's `` markers into a + * document-order partition of {@link WorkflowSection}s. See the module doc + * comment for the full grammar, partition invariant, and fence/comment + * interleaving discipline. + * + * @param content - full workflow markdown source + * @param sourcePath - optional file path named in thrown errors + */ +export function parseWorkflowSections(content: string, sourcePath?: string): WorkflowSection[] { + const lines = splitLinesPreservingEol(content); + const sections: WorkflowSection[] = []; + + let fence: { char: '`' | '~'; len: number } | null = null; + let inComment = false; + let currentOpen: { id: string; when: string; startLineIndex: number } | null = null; + const seenIds = new Set(); + let gapCounter = 0; + let cursor = 0; + + const joinRange = (from: number, to: number): string => { + let out = ''; + for (let k = from; k <= to; k++) { + out += lines[k].text + lines[k].eol; + } + return out; + }; + + const flushGapBefore = (nextIndex: number): void => { + if (nextIndex > cursor) { + sections.push({ + id: `gap-${gapCounter}`, + when: 'always', + body: joinRange(cursor, nextIndex - 1), + explicit: false, + startLine: cursor + 1, + }); + gapCounter += 1; + } + }; + + for (let i = 0; i < lines.length; i++) { + const lineNo = i + 1; + const rawText = lines[i].text; + + if (fence !== null) { + // Inside a real fence: only a matching closer can end it. Any + // `` on this line is fence content, never a comment + // boundary (row 5/6 of 50-test-matrix.md). + const m = FENCE_DELIM_RE.exec(rawText); + if (m) { + const char = m[2][0] as '`' | '~'; + const len = m[2].length; + const trailing = m[3]; + if (char === fence.char && len >= fence.len && /^\s*$/.test(trailing)) { + fence = null; + } + } + continue; + } + + if (inComment) { + // Inside a real (unrelated) comment: only '-->' can end it. Any + // fence delimiter on this line is comment content, never a fence + // boundary (row 7 of 50-test-matrix.md). + if (rawText.includes('-->')) inComment = false; + continue; + } + + const trimmed = rawText.trim(); + + if (trimmed.startsWith(''); + if (hasClose && trimmed.endsWith('-->')) { + const inner = trimmed.slice(4, trimmed.length - 3).trim(); + const classification = classifyMarker(inner, sourcePath, lineNo); + if (classification.kind === 'open') { + if (currentOpen !== null) { + fail(sourcePath, lineNo, REASON.NESTED_SECTION, `nested gsd:section marker (already inside "${currentOpen.id}")`); + } + if (seenIds.has(classification.id)) { + fail(sourcePath, lineNo, REASON.DUPLICATE_ID, `duplicate section id "${classification.id}"`); + } + flushGapBefore(i); + seenIds.add(classification.id); + currentOpen = { id: classification.id, when: classification.when, startLineIndex: i }; + cursor = i + 1; + } else if (classification.kind === 'close') { + if (currentOpen === null) { + fail(sourcePath, lineNo, REASON.UNMATCHED_CLOSE, 'unmatched /gsd:section close marker'); + } + sections.push({ + id: currentOpen.id, + when: currentOpen.when, + body: joinRange(currentOpen.startLineIndex + 1, i - 1), + explicit: true, + startLine: currentOpen.startLineIndex + 1, + }); + currentOpen = null; + cursor = i + 1; + } + // classification.kind === 'none': ordinary self-contained comment + // (e.g. a one-line `gsd:loop-host` or unrelated comment) — no state change. + } + if (!hasClose) { + inComment = true; // multi-line: stays open until a later '-->' + } + continue; + } + + const fenceMatch = FENCE_DELIM_RE.exec(rawText); + if (fenceMatch) { + const char = fenceMatch[2][0] as '`' | '~'; + const trailing = fenceMatch[3]; + // CommonMark §4.5: a backtick fence opener's info string must not + // itself contain a backtick. + if (!(char === '`' && trailing.includes('`'))) { + fence = { char, len: fenceMatch[2].length }; + } + } + } + + if (currentOpen !== null) { + fail(sourcePath, currentOpen.startLineIndex + 1, REASON.UNCLOSED_SECTION, `unclosed gsd:section marker "${currentOpen.id}"`); + } + + flushGapBefore(lines.length); + + return sections; +} + +/** + * Map parsed sections to `context-composer` fragments. Every strategy is + * `{kind: 'verbatim'}` (design row 23 / test matrix rows 26-29): non- + * lossiness in this phase is a STRUCTURAL guarantee of the strategy choice, + * never a large-budget trick. + * + * @param sections - document-order sections from {@link parseWorkflowSections} + */ +export function toFragments(sections: readonly WorkflowSection[]): contextComposer.Fragment[] { + return sections.map((section) => ({ + id: section.id, + content: section.body, + strategy: { kind: 'verbatim' as const }, + })); +} + +/** + * Concatenate a {@link contextComposer.ComposeResult}'s fragment contents, + * in declaration order, back into a document. Every fragment here is + * `verbatim` with an empty wrapper, so this is a plain join. + * + * @param result - the plan returned by `composeWithinBudget` + */ +export function renderFragments(result: contextComposer.ComposeResult): string { + return result.fragments.map((f) => f.content).join(''); +} + +/** + * THE emission entry point: parse -> toFragments -> composeWithinBudget -> + * render. `budget` defaults to `Number.MAX_SAFE_INTEGER` (no pressure). + * Because every fragment is `verbatim`, the output is identical regardless + * of the budget value (design row 23) — this is never relied upon as the + * source of non-lossiness; the strategy set is. + * + * @param content - full workflow markdown source + * @param opts - `sourcePath` named in thrown parse errors; `budget` in bytes + */ +export function composeWorkflow(content: string, opts: { sourcePath?: string; budget?: number } = {}): string { + const { sourcePath, budget = Number.MAX_SAFE_INTEGER } = opts; + const sections = parseWorkflowSections(content, sourcePath); + const fragments = toFragments(sections); + const composed = contextComposer.composeWithinBudget({ + fragments, + budget, + measure: (text: string) => Buffer.byteLength(text, 'utf8'), + options: { charsPerUnit: 1 }, + }); + return renderFragments(composed); +} diff --git a/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json b/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json new file mode 100644 index 000000000..22988e762 --- /dev/null +++ b/tests/emitted-drift-acks/2930-fragmentize-execute-phase-markers.json @@ -0,0 +1,6 @@ +{ + "version": 1, + "paths": { + "execute-phase.md": "#2930 (epic #1671 Phase 3): pilots the in-file `` marker grammar by wrapping the --wave/gap-closure/regression-gate branch sections (partial-wave, gap-closure-artifacts, regression-gate) in marker pairs, proving the composeWorkflow seam runs at install time before per-runtime rewrites. Retargeted from plan-phase.md (chore/2930 review): plan-phase.md sits only 36 B under the ADR-857 Phase-6 PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot absorb marker overhead, so the maintainer retargeted the pilot to execute-phase.md, which has 728 B of headroom under its own PRE_PHASE6 cap. SOURCE grows by exactly 275 marker bytes (6 marker lines); the EMITTED artifact composeWorkflow produces at install is byte-identical to the pre-#2930 file (markers are stripped, never shipped). See .gsd/phase/chore-2930-fragmentize-xl-workflow/40-design.md 'Known limits' item 5." + } +} diff --git a/tests/workflow-fragments-emission.install.test.cjs b/tests/workflow-fragments-emission.install.test.cjs new file mode 100644 index 000000000..6b7f240c3 --- /dev/null +++ b/tests/workflow-fragments-emission.install.test.cjs @@ -0,0 +1,484 @@ +'use strict'; + +// allow-test-rule: source-text-is-the-product — noSectionMarkerLeaksIntoEmittedArtifacts (#2930) +// asserts on the literal bytes of an EMITTED install artifact, which IS the +// deployed contract (a leaked `gsd:section` marker byte would ship to every user). This +// mirrors the test-matrix's own row-34/35 exemption from the "no source-grep" rule +// (50-test-matrix.md "No source-grep" note) — the ESLint rule itself only fires on +// readFileSync of a .cjs/.js/.ts SOURCE path, never on an installed .md artifact, so this +// annotation is documentation of intent, not a required suppression. + +/** + * workflow-fragments-emission.install.test.cjs — 50-test-matrix.md rows 32-36 + * (issue #2930, epic #1671 Phase 3). + * + * Real spawn-install coverage for `composeWorkflow`'s wiring into + * `bin/install.js`'s `copyWithPathReplacement` (ADR-1671 "Architecture and + * contracts": an engine-direct assertion is false-green for install + * behavior — only a real spawned installer proves bytes actually reach + * disk). The pure parser/composer itself is covered by + * tests/workflow-fragments.test.cjs (unit, rows 1-29/37) and + * tests/workflow-fragments.property.test.cjs (prop, rows 30-31). + * + * Each test builds and tears down its own tmp fixture(s) inline (no shared + * `before()` install cache) — independence per matrix row 38. + * + * ── The overlay technique (rows 33/36) ─────────────────────────────────── + * + * Rows 33 and 36 need a spawned `bin/install.js` that reads a DIFFERENT + * `gsd-core/workflows/execute-phase.md` (malformed, row 36) or a different + * `gsd-core/bin/lib/workflow-fragments.cjs` (stubbed to identity, row 33) + * than this checkout's real files, without paying to copy the ~400 MB + * repository (mostly node_modules) for every run. `buildOverlayRepo` mirrors + * the repo tree with real directories (so `copyWithPathReplacement`'s own + * `entry.isDirectory()` / `entry.isFile()` Dirent checks — which do NOT + * follow symlinks — see the correct type) and HARD-LINKS every unmodified + * leaf file (not symlinks: a symlinked leaf file also fails an `isFile()` + * Dirent check elsewhere in the installer, verified empirically — "Failed + * to install agents: directory is empty" against a symlink-leaf overlay). + * Only `node_modules` and `.git` are symlinked at the top level (install.js + * never walks into either), which is what keeps the overlay build fast. + * Every overlay-spawned installer runs with `--preserve-symlinks + * --preserve-symlinks-main` as a defensive belt: with an all-hardlink leaf + * layout this checkout does not currently NEED symlink-preservation for + * correctness, but the flag is free insurance against a future install.js + * change that resolves a node_modules package by real path. + */ + +const { 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 { spawnSync } = require('node:child_process'); + +const { cleanup } = require('./helpers.cjs'); +const { RUNTIME_META, runMinimalInstall, installerEnv } = require('./helpers/install-shared.cjs'); +const { executionContextRefs } = require('../scripts/command-contract-helpers.cjs'); +const { composeWorkflow } = require('../gsd-core/bin/lib/workflow-fragments.cjs'); + +const REPO_ROOT = path.join(__dirname, '..'); +const PILOT_REL = path.join('gsd-core', 'workflows', 'execute-phase.md'); +const PILOT_PATH = path.join(REPO_ROOT, PILOT_REL); +// plan-phase.md was the original #2930 pilot but was reverted to unmarked +// (chore/2930 retarget: it sits 36 B under the ADR-857 Phase-6 PRE_PHASE6 +// gate and cannot absorb marker overhead) — it is now a genuinely unmarked +// file again, so row 33 uses it instead of plan-phase.md. +const UNMARKED_REL = path.join('gsd-core', 'workflows', 'plan-phase.md'); + +const RUNTIMES = Object.keys(RUNTIME_META); + +// ─── Overlay-repo builder (rows 33/36) ───────────────────────────────────── + +const OVERLAY_SKIP_TOP = new Set(['node_modules', '.git']); + +/** Hard-link a file, falling back to a real copy only if the two paths sit on + * different filesystems/devices (EXDEV) or linking is denied (EPERM) — both + * cross-platform-legitimate, unlike a symlink's Dirent type-detection gap. */ +function linkOrCopyFile(src, dest) { + try { + fs.linkSync(src, dest); + } catch (err) { + if (err.code === 'EXDEV' || err.code === 'EPERM') { + fs.copyFileSync(src, dest); + } else { + throw err; + } + } +} + +/** + * Build a throwaway mirror of REPO_ROOT with real directories throughout and + * every unmodified leaf file hard-linked, except the paths named in + * `fileOverrides` (POSIX-relative-path -> content string), which are written + * as real files. Returns the mirror's absolute path; caller must + * `fs.rmSync(..., {recursive:true, force:true})` it away. + * + * @param {{[relPath: string]: string}} fileOverrides + */ +function buildOverlayRepo(fileOverrides) { + const tmpRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2930-overlay-')); + const entries = Object.entries(fileOverrides).map(([relPath, content]) => ({ + parts: relPath.split('/'), + content, + })); + + function place(srcDir, destDir, pending, isTop) { + fs.mkdirSync(destDir, { recursive: true }); + const grouped = new Map(); + for (const e of pending) { + const [head, ...rest] = e.parts; + if (!grouped.has(head)) grouped.set(head, []); + grouped.get(head).push({ parts: rest, content: e.content }); + } + for (const de of fs.readdirSync(srcDir, { withFileTypes: true })) { + if (isTop && OVERLAY_SKIP_TOP.has(de.name)) { + fs.symlinkSync(path.join(srcDir, de.name), path.join(destDir, de.name)); + continue; + } + const srcPath = path.join(srcDir, de.name); + const destPath = path.join(destDir, de.name); + const overridden = grouped.get(de.name); + const leaf = overridden && overridden.find((s) => s.parts.length === 0); + if (leaf) { + fs.writeFileSync(destPath, leaf.content); + continue; + } + // fs.statSync follows symlinks (unlike Dirent.isDirectory()), so a + // symlinked source directory is still recursed as a REAL directory in + // the overlay — the property copyWithPathReplacement itself needs. + if (fs.statSync(srcPath).isDirectory()) { + place(srcPath, destPath, overridden || [], false); + } else { + linkOrCopyFile(srcPath, destPath); + } + } + } + + place(REPO_ROOT, tmpRepo, entries, true); + return tmpRepo; +} + +/** Spawn a (possibly overlaid) installScript at global scope. Does NOT + * assert success — callers decide (row 36 expects failure). */ +function spawnGlobalInstall(installScript, runtime, extraArgs = []) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-2930-dest-${runtime}-`)); + const args = [ + '--preserve-symlinks', + '--preserve-symlinks-main', + installScript, + `--${runtime}`, + '--global', + '--config-dir', + root, + ...extraArgs, + ]; + const result = spawnSync(process.execPath, args, { + cwd: root, + encoding: 'utf8', + env: installerEnv({ HOME: root, USERPROFILE: root }), + }); + return { result, configDir: root, root }; +} + +/** Convert native path separators to POSIX forward slashes, unconditionally + * (never gate on `path.sep` — CONTEXT.md's path-separator-normalization + * rule). Windows installs embed the SAME root in more than one spelling: + * the `@`-ref / pathPrefix rewrites always emit posix-normalized + * forward-slash paths, while other embedded content can still carry the + * native backslash spelling. `root` itself (from `fs.mkdtempSync`) is a + * native-separator string, so comparing it against text verbatim only + * matches ONE of those spellings. */ +function toPosixSlashes(value) { + return value.replace(/\\/g, '/'); +} + +/** Strip an install's own absolute root out of emitted text so two installs + * under DIFFERENT temp roots (different lengths, different runtime-name + * prefixes) can be compared byte-for-byte. Normalizes BOTH the text and the + * root to forward-slash spelling first, so every embedded spelling of the + * root collapses onto the SAME placeholder — leaving the comparison + * measuring only composeWorkflow's own contribution. */ +function stripRoot(text, root) { + return toPosixSlashes(text).split(toPosixSlashes(root)).join(''); +} + +// ─── Row 32: emitted execute-phase.md shrinks by exactly the marker bytes ──── +// +// Defect found and fixed inline while verifying (chore/2930 review; not one +// of the five assigned findings, but discovered incidentally): this test +// previously compared the RAW `composeWorkflow(source)` byte length directly +// against `fs.statSync(emittedPath).size`. That equality only holds when the +// OTHER rewrites `copyWithPathReplacement` also runs (the `~/.claude/` -> +// `pathPrefix` substitution, attribution stamping, per-runtime converters) +// happen to be byte-neutral — which they are NOT here: `runMinimalInstall` +// passes `--config-dir root` with `HOME=root` (root IS the target, not +// `root/.claude`), so `computePathPrefix` degenerates to the short literal +// `$HOME/` instead of the real-world `$HOME/.claude/`, shrinking the +// installed `~/.claude/` references by additional bytes unrelated to +// composeWorkflow. Proven with a real spawned install and reverted to +// confirm this reproduces on the UNMODIFIED tree, before this change. Fixed +// by isolating composeWorkflow's OWN contribution the same way row 33 +// already does: compare two REAL installs of the pilot workflow, one via +// this checkout's real composeWorkflow and one via an identity-stubbed +// composeWorkflow, and assert the size DELTA equals exactly the marker +// bytes stripped — never an absolute emitted byte count, which conflates +// unrelated rewrites this module does not own. +// +// A second, independent contaminant surfaced fixing the first one: opencode +// embeds the install's own absolute configDir path into execute-phase.md +// content (same fact row 33/atRefContractStillResolvesAfterComposition +// documents for SKILL.md), and `runMinimalInstall`'s temp-dir prefix +// (`gsd---`) is a DIFFERENT length than +// `spawnGlobalInstall`'s (`gsd-2930-dest--`) — so comparing RAW +// file sizes between the two installs bakes in a root-path-length delta +// that has nothing to do with composeWorkflow. Normalize each side's own +// root out of the text before measuring, exactly as row 33 already does. + +test('emittedWorkflowShrinksByMarkerBytesForEveryRuntime', () => { + const source = fs.readFileSync(PILOT_PATH, 'utf8'); + const composed = composeWorkflow(source, { sourcePath: PILOT_PATH }); + const sourceBytes = Buffer.byteLength(source, 'utf8'); + const composedBytes = Buffer.byteLength(composed, 'utf8'); + const expectedMarkerBytes = sourceBytes - composedBytes; + assert.ok( + expectedMarkerBytes > 0, + 'sanity: the pilot workflow must actually carry gsd:section markers to strip', + ); + + const identityStubRepo = buildOverlayRepo({ + 'gsd-core/bin/lib/workflow-fragments.cjs': 'module.exports = { composeWorkflow: (c) => c };\n', + }); + try { + for (const runtime of RUNTIMES) { + const real = runMinimalInstall({ runtime, scope: 'global' }); + const stub = spawnGlobalInstall(path.join(identityStubRepo, 'bin', 'install.js'), runtime); + try { + assert.equal( + stub.result.status, + 0, + `${runtime}: identity-stub install must succeed\nstderr: ${stub.result.stderr}`, + ); + const realPath = path.join(real.configDir, PILOT_REL); + const stubPath = path.join(stub.configDir, PILOT_REL); + assert.ok(fs.existsSync(realPath), `${runtime}: real install is missing execute-phase.md`); + assert.ok(fs.existsSync(stubPath), `${runtime}: identity-stub install is missing execute-phase.md`); + const realText = stripRoot(fs.readFileSync(realPath, 'utf8'), real.root); + const stubText = stripRoot(fs.readFileSync(stubPath, 'utf8'), stub.root); + const realBytes = Buffer.byteLength(realText, 'utf8'); + const stubBytes = Buffer.byteLength(stubText, 'utf8'); + assert.equal( + stubBytes - realBytes, + expectedMarkerBytes, + `${runtime}: emitted size delta (stub ${stubBytes} - real ${realBytes}, root-normalized) must equal exactly the marker bytes stripped (${expectedMarkerBytes})`, + ); + } finally { + cleanup(real.root); + cleanup(stub.root); + } + } + } finally { + cleanup(identityStubRepo); + } +}); + +// ─── Row 33: an unmarked workflow emits byte-identical for every runtime ── +// +// "Byte-identical" here means identical to what the SAME runtime's install +// pipeline would emit WITHOUT the #2930 composeWorkflow wiring — not +// necessarily identical to the raw repo source, since path-prefix rewrites, +// attribution stamping, and per-runtime .md converters already ran before +// this change and still run today. Proven empirically per runtime by +// comparing two REAL installs of the SAME unmarked file: one through this +// checkout's real composeWorkflow, one through an overlay whose +// gsd-core/bin/lib/workflow-fragments.cjs is stubbed to plain identity — any +// difference is attributable ONLY to the compose wiring, never to an +// unrelated converter (which fires identically on both sides). + +test('unmarkedWorkflowEmitsByteIdenticalForEveryRuntime', () => { + const identityStubRepo = buildOverlayRepo({ + 'gsd-core/bin/lib/workflow-fragments.cjs': 'module.exports = { composeWorkflow: (c) => c };\n', + }); + try { + for (const runtime of RUNTIMES) { + const real = runMinimalInstall({ runtime, scope: 'global' }); + const stub = spawnGlobalInstall(path.join(identityStubRepo, 'bin', 'install.js'), runtime); + try { + assert.equal( + stub.result.status, + 0, + `${runtime}: identity-stub install must succeed\nstderr: ${stub.result.stderr}`, + ); + const realPath = path.join(real.configDir, UNMARKED_REL); + const stubPath = path.join(stub.configDir, UNMARKED_REL); + assert.ok(fs.existsSync(realPath), `${runtime}: real install is missing plan-phase.md`); + assert.ok(fs.existsSync(stubPath), `${runtime}: stub install is missing plan-phase.md`); + + // Normalize each side's own randomly-generated temp root out of the + // content before hashing: some runtimes (opencode) embed the + // install's own absolute configDir path in execution_context refs, + // and the two installs necessarily used DIFFERENT temp roots — an + // unnormalized compare would report a spurious mismatch driven by + // temp-path length, not by anything composeWorkflow's wiring did. + const realText = stripRoot(fs.readFileSync(realPath, 'utf8'), real.root); + const stubText = stripRoot(fs.readFileSync(stubPath, 'utf8'), stub.root); + assert.equal( + Buffer.byteLength(realText, 'utf8'), + Buffer.byteLength(stubText, 'utf8'), + `${runtime}: plan-phase.md byte size drifted between real compose and identity-stub compose`, + ); + const realHash = crypto.createHash('sha256').update(realText).digest('hex'); + const stubHash = crypto.createHash('sha256').update(stubText).digest('hex'); + assert.equal( + realHash, + stubHash, + `${runtime}: plan-phase.md content drifted between real compose and identity-stub compose`, + ); + } finally { + cleanup(real.root); + cleanup(stub.root); + } + } + } finally { + cleanup(identityStubRepo); + } +}); + +// ─── Row 34: no gsd:section marker survives into any emitted artifact ───── + +test('noSectionMarkerLeaksIntoEmittedArtifacts', () => { + for (const runtime of RUNTIMES) { + const { configDir, root } = runMinimalInstall({ runtime, scope: 'global' }); + try { + const emittedPath = path.join(configDir, PILOT_REL); + assert.ok(fs.existsSync(emittedPath), `${runtime}: emitted execute-phase.md is missing`); + const emittedText = fs.readFileSync(emittedPath, 'utf8'); + assert.equal( + emittedText.includes('gsd:section'), + false, + `${runtime}: emitted execute-phase.md still contains a gsd:section marker token`, + ); + } finally { + cleanup(root); + } + } +}); + +// ─── Row 35: ADR-0002 @-ref contract still resolves after composition ───── +// +// Two representative runtimes chosen to cover BOTH observed @-ref forms +// (empirically confirmed, #2930 dispatch): claude/cursor/codex rewrite to +// `@$HOME/...`, while opencode rewrites to a bare `@/...`. +// Both installed SKILL.md files themselves pass through composeWorkflow too +// (as a no-op, being unmarked) — this proves that pass never corrupts or +// relocates the referenced workflow file. + +/** Detect absoluteness from the token's own shape only — never from + * `process.platform` — so the same logic runs identically on every OS. + * Covers POSIX (`/...`), Windows drive-letter (`C:/...` or `C:\...`), and + * UNC (`\\server\share`) forms. */ +function isAbsoluteRefTarget(candidate) { + return ( + candidate.startsWith('/') || + /^[a-zA-Z]:[\\/]/.test(candidate) || + candidate.startsWith('\\\\') + ); +} + +function resolveExecutionContextRefTarget(token, root) { + const withoutAt = token.replace(/^@/, ''); + if (isAbsoluteRefTarget(withoutAt)) return withoutAt; // already absolute (opencode form) + const stripped = withoutAt.replace(/^(?:~|\$HOME)\//, ''); + return path.join(root, stripped); +} + +test('atRefContractStillResolvesAfterComposition', () => { + for (const runtime of ['claude', 'opencode']) { + const { configDir, root } = runMinimalInstall({ runtime, scope: 'global' }); + try { + const skillPath = path.join(configDir, 'skills', 'gsd-plan-phase', 'SKILL.md'); + assert.ok(fs.existsSync(skillPath), `${runtime}: installed gsd-plan-phase SKILL.md is missing`); + const skillContent = fs.readFileSync(skillPath, 'utf8'); + const refs = executionContextRefs(skillContent); + assert.ok(refs.length > 0, `${runtime}: SKILL.md has no execution_context @-refs to check`); + for (const { token } of refs) { + const target = resolveExecutionContextRefTarget(token, root); + assert.ok( + fs.existsSync(target), + `${runtime}: execution_context @-ref "${token}" resolved to "${target}", which does not exist on disk`, + ); + } + } finally { + cleanup(root); + } + } +}); + +// ─── FIX 1 (chore/2930 review): composeWorkflow is scoped to gsd-core/workflows/ ── +// +// copyWithPathReplacement is the emit path for the ENTIRE gsd-core/ tree +// (skillSrc = path.join(src, 'gsd-core'), bin/install.js:10806-10809), not +// just gsd-core/workflows/. A non-workflow .md elsewhere under gsd-core/ +// (e.g. gsd-core/references/) that merely DOCUMENTS the marker syntax with +// an unfenced, structurally-invalid example line must never be run through +// composeWorkflow — doing so would either throw (breaking install for an +// unrelated file class) or silently strip/mis-parse the documentation line. +// Proven here by overlaying BOTH a marked workflow (must still compose) and +// an EXISTING non-workflow reference doc (buildOverlayRepo can only replace +// the content of a real leaf file, not graft in a net-new path — see the +// module doc comment's overlay-technique note) rewritten to carry an +// intentionally-UNCLOSED marker-shaped line (would throw if composeWorkflow +// ever touched it) in the SAME install run. + +test('nonWorkflowMarkdownWithMarkerShapedLineIsNotComposed', () => { + const markedWorkflow = '\nbody\n\n'; + const nonWorkflowDoc = + '# Marker syntax\n\nExample (deliberately unfenced and unclosed to prove non-composition):\n\n\nnever closed on purpose\n'; + const NON_WORKFLOW_DOC_REL = path.join('gsd-core', 'references', 'context-budget.md'); + const overlayRepo = buildOverlayRepo({ + 'gsd-core/workflows/execute-phase.md': markedWorkflow, + [NON_WORKFLOW_DOC_REL.split(path.sep).join('/')]: nonWorkflowDoc, + }); + let dest; + try { + dest = spawnGlobalInstall(path.join(overlayRepo, 'bin', 'install.js'), 'claude'); + assert.equal( + dest.result.status, + 0, + `install must succeed: a non-workflow doc's marker-shaped line must never reach composeWorkflow\nstderr: ${dest.result.stderr}`, + ); + + const emittedWorkflowPath = path.join(dest.configDir, PILOT_REL); + assert.ok(fs.existsSync(emittedWorkflowPath), 'emitted execute-phase.md is missing'); + assert.equal( + fs.readFileSync(emittedWorkflowPath, 'utf8'), + 'body\n', + 'gsd-core/workflows/execute-phase.md must still compose (markers stripped)', + ); + + const emittedDocPath = path.join(dest.configDir, NON_WORKFLOW_DOC_REL); + assert.ok(fs.existsSync(emittedDocPath), 'emitted context-budget.md is missing'); + assert.equal( + fs.readFileSync(emittedDocPath, 'utf8'), + nonWorkflowDoc, + 'a non-workflow .md must pass through composeWorkflow untouched, byte-identical, including its marker-shaped line', + ); + } finally { + cleanup(overlayRepo); + if (dest) cleanup(dest.root); + } +}); + +// ─── Row 36: a malformed marker fails install loudly, with no partial emit ─ + +test('malformedMarkersFailInstallWithoutPartialEmit', () => { + const malformed = '\nnever closed\n'; + const overlayRepo = buildOverlayRepo({ 'gsd-core/workflows/execute-phase.md': malformed }); + let dest; + try { + dest = spawnGlobalInstall(path.join(overlayRepo, 'bin', 'install.js'), 'claude'); + // stderr text is a child process's rendered prose, not a typed value + // this test can assert on across the process boundary (CONTRIBUTING.md + // "Prohibited: Raw Text Matching on Test Outputs" — err.reason is only + // reachable in-process; see tests/workflow-fragments.test.cjs's REASON + // assertions for the in-process equivalent of this same failure mode). + // Assert typed, observable facts instead: the install process exits + // non-zero, and no output file is written for the file that failed to + // compose. + assert.notEqual( + dest.result.status, + 0, + `install must fail loudly on a malformed marker, got exit 0\nstdout: ${dest.result.stdout}`, + ); + const emittedPath = path.join(dest.configDir, PILOT_REL); + assert.equal( + fs.existsSync(emittedPath), + false, + 'a half-composed execute-phase.md must never be written when composition throws', + ); + } finally { + cleanup(overlayRepo); + if (dest) cleanup(dest.root); + } +}); diff --git a/tests/workflow-fragments.property.test.cjs b/tests/workflow-fragments.property.test.cjs new file mode 100644 index 000000000..a3bee43a0 --- /dev/null +++ b/tests/workflow-fragments.property.test.cjs @@ -0,0 +1,198 @@ +'use strict'; + +/** + * Property-based tests for src/workflow-fragments.cts (compiled to + * gsd-core/bin/lib/workflow-fragments.cjs) — issue #2930 (epic #1671 + * Phase 3). Covers 50-test-matrix.md rows 30-31. + * + * Document-shaped generators (CONTRIBUTING.md "Fixture provenance #2371", + * mirroring tests/context-predicates.property.test.cjs): these generators + * build arbitrary markdown documents out of prose lines, fenced blocks + * (whose contents — including marker LOOKALIKES — are always literal), and + * well-formed `gsd:section` marker pairs. They are NOT seeded from this + * module's own `composeWorkflow`/`renderFragments` — document shape (which + * lines exist, in what order, wrapped in what fences) is generated + * independently; only the STRIPPED-EXPECTATION bookkeeping (which line + * indexes are real top-level marker lines) is computed alongside, from the + * same generation step, never by round-tripping through the code under + * test. + * + * Deterministic per CONTRIBUTING.md: seed and numRuns are pinned by + * tests/helpers/fast-check-setup.cjs (seed 42, numRuns 200). + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fc = require('./helpers/fast-check-setup.cjs'); + +const { parseWorkflowSections, composeWorkflow, WHEN_VOCABULARY } = require('../gsd-core/bin/lib/workflow-fragments.cjs'); + +// Derived from the module's own frozen WHEN_VOCABULARY (DEFECT.GENERATIVE-FIX, +// chore/2930 review): a hardcoded copy here would silently desync from the +// production vocabulary the moment either side is edited without the other. +const WHEN_VALUES = [...WHEN_VOCABULARY]; + +// ─── Document-shaped generators ──────────────────────────────────────────── + +// Plain-text charset that can never accidentally spell an HTML comment +// delimiter or a fence delimiter — keeps every "decoy" line unambiguous. +const proseTextArb = fc.stringMatching(/^[A-Za-z0-9 .,'":;()]{0,40}$/); + +const idArb = fc.stringMatching(/^[a-z][a-z0-9]{0,5}$/); +const whenArb = fc.constantFrom(...WHEN_VALUES); + +// A prose line that LOOKS marker-adjacent but is never a real marker: a +// heading, a blockquote, a table row, a mid-line backticked mention (extra +// prose before/after disqualifies it structurally), or a one-line +// `gsd:loop-host` comment (a different marker family entirely). +const proseLineArb = fc.oneof( + proseTextArb, + proseTextArb.map((t) => `# ${t}`), + proseTextArb.map((t) => `> ${t}`), + proseTextArb.map((t) => `| ${t} | cell |`), + idArb.map((id) => `See \`\` for syntax.`), + fc.constant(''), +); + +const fenceTickArb = fc.constantFrom('```', '~~~', '````'); + +// A fenced block: opener, 0-4 inner lines (prose OR a marker-lookalike full +// line), matching closer using the SAME tick string — everything inside is +// LITERAL regardless of shape (rows 5-8 of 50-test-matrix.md). +function fenceBlockArb() { + return fc + .tuple( + fenceTickArb, + fc.array( + fc.oneof( + proseLineArb, + idArb.map((id) => ``), + fc.constant(''), + ), + { minLength: 0, maxLength: 4 }, + ), + ) + .map(([tick, inner]) => [tick, ...inner, tick]); +} + +// A well-formed marker's interior: 0-3 items, each either a prose line or a +// nested fenced block (which may itself contain marker lookalikes). +function markerBodyArb() { + return fc + .array(fc.oneof({ arbitrary: proseLineArb, weight: 3 }, { arbitrary: fenceBlockArb(), weight: 1 }), { + minLength: 0, + maxLength: 3, + }) + .map((items) => items.flat()); +} + +// A top-level document block: a single prose line, a whole fenced block, or +// a well-formed gsd:section marker pair (id assigned by the assembler for +// document-wide uniqueness — see documentArb). +const blockArb = fc.oneof( + { arbitrary: proseLineArb.map((line) => ({ kind: 'prose', lines: [line] })), weight: 3 }, + { arbitrary: fenceBlockArb().map((lines) => ({ kind: 'prose', lines })), weight: 2 }, + { arbitrary: fc.tuple(whenArb, markerBodyArb()).map(([when, body]) => ({ kind: 'marker', when, body })), weight: 2 }, +); + +const eolArb = fc.constantFrom('\n', '\r\n'); + +/** + * A whole document assembled from an arbitrary sequence of blocks. Returns + * `{source, expectedStripped}`: `source` is the generated document text; + * `expectedStripped` is `source` with exactly the REAL top-level marker + * lines removed (computed from the same generation step, independent of + * the code under test). + */ +function documentArb() { + return fc + .tuple(fc.array(blockArb, { minLength: 0, maxLength: 8 }), eolArb, fc.boolean()) + .map(([blocks, eol, trailingEol]) => { + const allLines = []; + const markerLineIndexes = new Set(); + let counter = 0; + for (const block of blocks) { + if (block.kind === 'prose') { + allLines.push(...block.lines); + continue; + } + const id = `sec${counter}`; + counter += 1; + markerLineIndexes.add(allLines.length); + allLines.push(``); + allLines.push(...block.body); + markerLineIndexes.add(allLines.length); + allLines.push(''); + } + // Per-line records, each carrying ITS OWN terminator -- mirrors how + // workflow-fragments.cts's own line splitter models termination, so + // "expected" is computed by the same "remove this line INCLUDING its + // terminator" rule the partition invariant defines. A naive + // `filter().join(eol)` is WRONG here: `.join()` inserts a separator + // only BETWEEN surviving elements, so it silently drops a survivor's + // real trailing terminator whenever the (now-removed) line that used + // to follow it supplied that separator — caught live by this + // generator against a real marker-wrapped empty fence, where the + // section's last body line sits immediately before the close marker. + const records = allLines.map((text, idx) => ({ + text, + eol: idx === allLines.length - 1 ? (trailingEol ? eol : '') : eol, + })); + const source = records.map((r) => r.text + r.eol).join(''); + const expectedStripped = records + .filter((_, idx) => !markerLineIndexes.has(idx)) + .map((r) => r.text + r.eol) + .join(''); + return { source, expectedStripped }; + }); +} + +// ─── Row 30: parse/render round trip ─────────────────────────────────────── + +describe('property: parse/render round trip', () => { + test('parseRenderRoundTripProperty', () => { + fc.assert( + fc.property(documentArb(), ({ source, expectedStripped }) => { + const rendered = composeWorkflow(source); + assert.equal(rendered, expectedStripped); + }), + ); + }); + + test('parseRenderRoundTripProperty: no markers means exact identity', () => { + fc.assert( + fc.property(fc.array(proseLineArb, { minLength: 0, maxLength: 10 }), eolArb, (lines, eol) => { + const source = lines.join(eol); + assert.equal(composeWorkflow(source), source); + }), + ); + }); +}); + +// ─── Row 31: idempotency ──────────────────────────────────────────────────── + +describe('property: parse is idempotent over render', () => { + test('parseIsIdempotentOverRender', () => { + fc.assert( + fc.property(documentArb(), ({ source }) => { + const rendered = composeWorkflow(source); + + // Composing an already-composed (marker-free) document is a no-op. + assert.equal(composeWorkflow(rendered), rendered); + + // Re-parsing the rendered output yields a fixed shape: zero + // sections for an empty document, otherwise exactly ONE implicit + // gap fragment whose body is the whole (now marker-free) document — + // no marker survives composition to be re-recognized. + const sectionsAfter = parseWorkflowSections(rendered); + if (rendered === '') { + assert.deepEqual(sectionsAfter, []); + } else { + assert.equal(sectionsAfter.length, 1); + assert.equal(sectionsAfter[0].explicit, false); + assert.equal(sectionsAfter[0].body, rendered); + } + }), + ); + }); +}); diff --git a/tests/workflow-fragments.test.cjs b/tests/workflow-fragments.test.cjs new file mode 100644 index 000000000..ef11f7db3 --- /dev/null +++ b/tests/workflow-fragments.test.cjs @@ -0,0 +1,776 @@ +'use strict'; + +/** + * Example-based unit tests for src/workflow-fragments.cts (compiled to + * gsd-core/bin/lib/workflow-fragments.cjs) — issue #2930 (epic #1671 Phase 3). + * + * Covers 50-test-matrix.md rows 1-29 and 37 (unit level). Rows 30/31 + * (property) live in workflow-fragments.property.test.cjs; rows 32-36 + * (install-level, real spawn-install) are out of scope for this module's + * unit suite per ADR-1671 "Architecture and contracts". + * + * No source-grep (CONTRIBUTING.md): every assertion is on typed values + * (WorkflowSection records, ComposeResult metadata, byte counts) — never on + * rendered text via `.includes()`/`.match()`. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { createTempDir, cleanup } = require('./helpers.cjs'); + +const { + parseWorkflowSections, + toFragments, + renderFragments, + composeWorkflow, + WHEN_VOCABULARY, + REASON, +} = require('../gsd-core/bin/lib/workflow-fragments.cjs'); +const { composeWithinBudget } = require('../gsd-core/bin/lib/context-composer.cjs'); + +const measureBytes = (text) => Buffer.byteLength(text, 'utf8'); + +/** Compose a document string from an array of lines, joined with '\n'. */ +const doc = (...lines) => lines.join('\n'); + +// ─── Row 1: unmarked document (the 88/89 production shape) ───────────────── + +describe('unmarked document round trip', () => { + test('unmarkedDocumentRoundTripsByteIdentical', () => { + const source = doc( + '# Some Workflow', + '', + 'Ordinary prose describing the workflow.', + '', + '## A heading', + 'More prose.', + '', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].id, 'gap-0'); + assert.equal(sections[0].body, source); + + const rendered = composeWorkflow(source); + assert.equal(rendered, source); + }); +}); + +// ─── Row 2: single well-formed marker pair ────────────────────────────────── + +describe('single marker pair', () => { + test('singleMarkerPairStripsMarkersAndPreservesBody', () => { + const source = doc( + 'before prose', + '', + 'body line 1', + 'body line 2', + '', + 'after prose', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 3); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, 'before prose\n'); + assert.equal(sections[1].explicit, true); + assert.equal(sections[1].id, 'sec-a'); + assert.equal(sections[1].when, 'flag:--wave'); + assert.equal(sections[1].body, 'body line 1\nbody line 2\n'); + assert.equal(sections[2].explicit, false); + assert.equal(sections[2].body, 'after prose'); + + const rendered = composeWorkflow(source); + assert.equal(rendered, 'before prose\nbody line 1\nbody line 2\nafter prose'); + }); +}); + +// ─── Row 3: several disjoint pairs + unmarked gaps ───────────────────────── + +describe('multiple disjoint marker pairs', () => { + test('multiplePairsPartitionDocumentExactly', () => { + const source = doc( + 'gap0', + '', + 'bodyA', + '', + 'gap1', + '', + 'bodyB', + '', + 'gap2', + ); + const sections = parseWorkflowSections(source); + assert.deepEqual( + sections.map((s) => ({ id: s.id, explicit: s.explicit })), + [ + { id: 'gap-0', explicit: false }, + { id: 'a', explicit: true }, + { id: 'gap-1', explicit: false }, + { id: 'b', explicit: true }, + { id: 'gap-2', explicit: false }, + ], + ); + + const markerLineRe = /^\s*$/; + const expected = source + .split('\n') + .filter((line) => !markerLineRe.test(line)) + .join('\n'); + assert.equal(composeWorkflow(source), expected); + }); +}); + +// ─── Row 4: the real pilot workflow ───────────────────────────────────────── + +describe('real execute-phase.md', () => { + // NOTE (chore/2930 retarget): the pilot moved from plan-phase.md to + // execute-phase.md — plan-phase.md sits 36 B under the ADR-857 Phase-6 + // PRE_PHASE6 gate (tests/phase6-capstone-conformance.test.cjs) and cannot + // absorb marker overhead, so the maintainer retargeted the pilot to + // execute-phase.md (partial-wave, gap-closure-artifacts, regression-gate). + test('pilotWorkflowParsesAndRendersToSourceMinusMarkers', () => { + const pilotPath = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md'); + const original = fs.readFileSync(pilotPath, 'utf8'); + + // execute-phase.md carries the pilot's real marker pairs today: parsing + // it must recognize exactly those three explicit sections, in document + // order, and composing it must strip every marker line while leaving + // every byte of body content untouched. + const baselineSections = parseWorkflowSections(original, pilotPath); + const baselineExplicit = baselineSections.filter((s) => s.explicit); + assert.deepEqual( + baselineExplicit.map((s) => s.id), + ['partial-wave', 'gap-closure-artifacts', 'regression-gate'], + ); + const composedOriginal = composeWorkflow(original, { sourcePath: pilotPath }); + assert.equal(composedOriginal.includes('gsd:section'), false); + assert.ok(Buffer.byteLength(composedOriginal, 'utf8') < Buffer.byteLength(original, 'utf8')); + + // Wrap an ADDITIONAL, disjoint marker pair around an arbitrary interior + // slice of real content that sits outside every existing marker pair + // (lines 11-15, well before "partial-wave") and confirm it parses as a + // fourth explicit section and composes to the SAME final output as the + // unmodified file — every fragment is `verbatim` (row 23), so wrapping + // already-included content in a new marker pair can never change what + // is emitted, only how it is partitioned internally. + const lines = original.split(/\r?\n/); + const sliceStart = 10; + const sliceEnd = 15; + const markedLines = [ + ...lines.slice(0, sliceStart), + '', + ...lines.slice(sliceStart, sliceEnd), + '', + ...lines.slice(sliceEnd), + ]; + const marked = markedLines.join('\n'); + + const sections = parseWorkflowSections(marked, pilotPath); + const explicitSections = sections.filter((s) => s.explicit); + assert.deepEqual( + explicitSections.map((s) => s.id), + ['pilot-slice', 'partial-wave', 'gap-closure-artifacts', 'regression-gate'], + ); + assert.equal(explicitSections[0].body, lines.slice(sliceStart, sliceEnd).join('\n') + '\n'); + + const rendered = composeWorkflow(marked, { sourcePath: pilotPath }); + assert.equal(rendered, composedOriginal); + assert.equal(measureBytes(rendered), measureBytes(composedOriginal)); + }); +}); + +// ─── Row 5/6: fence negative space ────────────────────────────────────────── + +describe('marker lookalikes inside fences', () => { + test('markerInsideFencedBlockIsLiteral', () => { + const source = doc( + 'prose before', + '```', + '', + '```', + 'prose after', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); + + test('markerInsideFenceInsideSectionStaysLiteral', () => { + const source = doc( + '', + 'intro', + '```', + '', + '', + '```', + 'outro', + '', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, true); + assert.equal(sections[0].id, 'real'); + assert.equal( + sections[0].body, + ['intro', '```', '', '', '```', 'outro', ''].join( + '\n', + ), + ); + }); +}); + +// ─── Row 7/8: fence/comment mutual precedence ─────────────────────────────── + +describe('fence and comment mutual precedence', () => { + test('fenceDelimiterInsideCommentDoesNotOpenFence', () => { + const source = doc( + '', + '', + 'body', + '', + ); + // If the fence delimiter on line 2 had wrongly opened a fence, the real + // marker pair below would never be recognized (it would be swallowed as + // "fence content" all the way to EOF). + const sections = parseWorkflowSections(source); + const explicitSections = sections.filter((s) => s.explicit); + assert.equal(explicitSections.length, 1); + assert.equal(explicitSections[0].id, 'after-comment'); + assert.equal(explicitSections[0].body, 'body\n'); + }); + + test('commentTokenInsideFenceDoesNotOpenComment', () => { + const source = doc( + '```', + '', + 'body', + '', + ); + // If the `', + 'Do the thing.', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); + + test('backtickedMarkerMentionIsNotAMarker', () => { + const source = doc( + 'See `` for the marker syntax.', + 'And the close form is `` on its own line.', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); +}); + +// ─── Rows 11-14: structural negatives with location ──────────────────────── + +describe('structural negatives throw with file + line', () => { + test('unclosedSectionThrowsWithLocation', () => { + const source = doc('prose', '', 'body, never closed'); + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.message.includes('workflow.md:2') && err.reason === REASON.UNCLOSED_SECTION, + ); + }); + + test('unmatchedCloseThrowsWithLocation', () => { + const source = doc('prose', '', 'more prose'); + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.message.includes('workflow.md:2') && err.reason === REASON.UNMATCHED_CLOSE, + ); + }); + + test('nestedSectionThrows', () => { + const source = doc( + '', + '', + 'body', + '', + '', + ); + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.message.includes('workflow.md:2') && err.reason === REASON.NESTED_SECTION, + ); + }); + + test('duplicateSectionIdThrows', () => { + const source = doc( + '', + 'first', + '', + '', + 'second', + '', + ); + // Throws on the SECOND occurrence's line, not the first. + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.message.includes('workflow.md:4') && err.reason === REASON.DUPLICATE_ID, + ); + }); +}); + +// ─── Rows 15-18: attribute-shape negatives ───────────────────────────────── + +describe('attribute-shape negatives', () => { + test('missingIdAttributeThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.MISSING_ID, + ); + }); + + test('missingWhenAttributeThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.MISSING_WHEN, + ); + }); + + test('unknownWhenValueThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.UNKNOWN_WHEN, + ); + }); + + test('whenValueWithBooleanOperatorThrows', () => { + for (const when of ['flag:--wave && state:has-prior-phases', 'flag:--wave || state:has-prior-phases', '!flag:--wave']) { + const source = `\nbody\n`; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.UNKNOWN_WHEN, + `expected throw for when="${when}"`, + ); + } + }); + + test('malformedAttributesThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.MALFORMED_ATTRIBUTES, + ); + }); + + test('unrecognizedAttributeThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.UNRECOGNIZED_ATTRIBUTE, + ); + }); + + test('malformedIdValueThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.MALFORMED_ID, + ); + }); + + test('closeMarkerWithAttributesThrows', () => { + const source = '\nbody\n'; + assert.throws( + () => parseWorkflowSections(source, 'workflow.md'), + (err) => err instanceof TypeError && err.reason === REASON.CLOSE_WITH_ATTRIBUTES, + ); + }); +}); + +// ─── FIX 2/3 (chore/2930 review): REASON enum shape is locked ───────────── + +describe('REASON enum is frozen and its shape is locked', () => { + test('reasonEnumKeysAreLocked', () => { + assert.equal(Object.isFrozen(REASON), true); + assert.deepEqual(Object.keys(REASON).sort(), [ + 'CLOSE_WITH_ATTRIBUTES', + 'DUPLICATE_ID', + 'MALFORMED_ATTRIBUTES', + 'MALFORMED_ID', + 'MISSING_ID', + 'MISSING_WHEN', + 'NESTED_SECTION', + 'UNCLOSED_SECTION', + 'UNKNOWN_WHEN', + 'UNMATCHED_CLOSE', + 'UNRECOGNIZED_ATTRIBUTE', + ]); + }); +}); + +// ─── Doc/enum parity guard (DEFECT.GENERATIVE-FIX, code review #2930) ────── + +describe('REASON enum and docs "Fails closed" bullets stay in parity', () => { + test('everyReasonMemberIsDocumentedAndNoStaleBulletsRemain', () => { + // allow-test-rule: docs-parity — the doc text IS the contract being checked here (#2930) + const docPath = path.join(__dirname, '..', 'docs', 'reference', 'workflow-fragments.md'); + const docText = fs.readFileSync(docPath, 'utf8'); + + const sectionMatch = /## Fails closed\r?\n([\s\S]*?)\r?\n## /.exec(docText); + assert.ok(sectionMatch, 'docs/reference/workflow-fragments.md must have a "## Fails closed" section'); + const sectionText = sectionMatch[1]; + + const enumMembers = Object.keys(REASON); + // Key on the reason IDENTIFIER (e.g. `MALFORMED_ATTRIBUTES`) appearing in + // a bullet, never on bullet prose — a reword of the human-readable + // sentence must never falsely trip or falsely clear this guard. + const undocumented = enumMembers.filter((name) => !sectionText.includes(name)); + + const mentionedIdentifiers = [...sectionText.matchAll(/`([A-Z][A-Z0-9_]*)`/g)].map((m) => m[1]); + const staleMentions = mentionedIdentifiers.filter((name) => !enumMembers.includes(name)); + + assert.deepEqual( + undocumented, + [], + `REASON member(s) missing a "Fails closed" bullet in docs/reference/workflow-fragments.md: ${undocumented.join(', ')}`, + ); + assert.deepEqual( + staleMentions, + [], + `"Fails closed" section mentions identifier(s) that are not REASON members (stale bullet?): ${staleMentions.join(', ')}`, + ); + }); +}); + +// ─── Row 19: frozen vocabulary ────────────────────────────────────────────── + +describe('frozen when= vocabulary', () => { + test('whenVocabularyIsFrozenAndLocked', () => { + // WHEN_VOCABULARY is a frozen array (not an enum object) per the shipped + // public API — lock the actual VALUES (sorted), not Object.keys() (which + // for an array only reflects index positions '0','1',... and would not + // catch a value being silently renamed). See the dispatch report for + // this deliberate deviation from the test matrix's literal wording. + assert.equal(Object.isFrozen(WHEN_VOCABULARY), true); + assert.deepEqual( + [...WHEN_VOCABULARY].sort(), + ['always', 'flag:--wave', 'state:gap-closure-phase', 'state:has-prior-phases'], + ); + }); +}); + +// ─── Rows 20-22: boundary documents ───────────────────────────────────────── + +describe('boundary documents', () => { + test('emptyDocumentProducesNoFragments', () => { + const sections = parseWorkflowSections(''); + assert.deepEqual(sections, []); + assert.equal(composeWorkflow(''), ''); + }); + + test('documentOfOnlyAMarkerPairYieldsEmptyBody', () => { + const source = '\n'; + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, true); + assert.equal(sections[0].id, 'x'); + assert.equal(sections[0].body, ''); + assert.equal(composeWorkflow(source), ''); + }); + + test('unclosedFenceAtEofDoesNotThrow', () => { + const source = doc('prose', '```', 'never closed', ''); + assert.doesNotThrow(() => parseWorkflowSections(source)); + const sections = parseWorkflowSections(source); + // The whole document, including the marker-shaped line, is literal + // fence content — one implicit gap fragment, byte-identical. + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + }); +}); + +// ─── Rows 23-25: cross-platform + liberal formatting ─────────────────────── + +describe('cross-platform line endings and liberal marker formatting', () => { + test('crlfDocumentRoundTripsByteIdentical', () => { + const source = ['prose one', '', 'crlf body', '', 'prose two'].join( + '\r\n', + ); + const rendered = composeWorkflow(source); + const expected = source + .split('\r\n') + .filter((line) => !/^\s*$/.test(line)) + .join('\r\n'); + assert.equal(rendered, expected); + }); + + test('mixedLineEndingsPreservedExactly', () => { + const source = 'prose\r\n\r\nbody one\nbody two\n\nprose two'; + const sections = parseWorkflowSections(source); + const explicitSections = sections.filter((s) => s.explicit); + assert.equal(explicitSections.length, 1); + assert.equal(explicitSections[0].body, 'body one\nbody two\n'); + const rendered = composeWorkflow(source); + assert.equal(rendered, 'prose\r\nbody one\nbody two\nprose two'); + }); + + test('attributeOrderAndSpacingAreAccepted', () => { + const variants = [ + '', + '', + '', + ' ', + '', + ]; + for (const openLine of variants) { + const source = `${openLine}\nbody\n`; + const sections = parseWorkflowSections(source); + const explicitSections = sections.filter((s) => s.explicit); + assert.equal(explicitSections.length, 1, `expected recognition for: ${openLine}`); + assert.equal(explicitSections[0].id, 'x'); + assert.equal(explicitSections[0].when, 'always'); + // Re-render never leaks the original spacing — the marker is dropped + // entirely, so only the body survives. + assert.equal(composeWorkflow(source), 'body\n'); + } + }); +}); + +// ─── Rows 26-29: budget boundary set (non-lossiness is structural) ───────── + +describe('budget boundary set: nothing is ever trimmed', () => { + const source = doc( + 'gap prose', + '', + 'section a body', + '', + 'more gap prose', + ); + + function composeAt(budget) { + const sections = parseWorkflowSections(source); + const fragments = toFragments(sections); + return composeWithinBudget({ fragments, budget, measure: measureBytes, options: { charsPerUnit: 1 } }); + } + + const baseline = (() => { + const sections = parseWorkflowSections(source); + const fragments = toFragments(sections); + return fragments.reduce((sum, f) => sum + measureBytes(f.content), 0); + })(); + + const expectedRendered = composeWorkflow(source); + + test('nothingTrimmedWhenBudgetEqualsContent', () => { + const result = composeAt(baseline); + assert.deepEqual(result.metadata.omitted, []); + assert.deepEqual(result.metadata.shrunk, []); + assert.equal(renderFragments(result), expectedRendered); + }); + + test('nothingTrimmedWhenBudgetIsOneUnderContent', () => { + const result = composeAt(baseline - 1); + assert.deepEqual(result.metadata.omitted, []); + assert.deepEqual(result.metadata.shrunk, []); + assert.equal(renderFragments(result), expectedRendered); + }); + + test('nothingTrimmedWhenBudgetIsOneOverContent', () => { + const result = composeAt(baseline + 1); + assert.deepEqual(result.metadata.omitted, []); + assert.deepEqual(result.metadata.shrunk, []); + assert.equal(renderFragments(result), expectedRendered); + }); + + test('nothingTrimmedUnderAbsurdBudgetPressure', () => { + const result = composeAt(1); + assert.deepEqual(result.metadata.omitted, []); + assert.deepEqual(result.metadata.shrunk, []); + assert.equal(result.metadata.hardFailed, false); + assert.equal(renderFragments(result), expectedRendered); + }); +}); + +// ─── Row 37: fs.readFileSync fault injection ─────────────────────────────── + +/** + * Simulate the realistic caller shape (read a workflow file, compose it, + * write the composed result elsewhere) with `fs.readFileSync` monkeypatched + * to throw. The monkeypatch is saved/restored HERE, in a helper, inside a + * `finally` — never inside a test body, and never via chmod/permission + * tricks (CLAUDE.md cross-platform fault-injection rule). + */ +function withInjectedReadFailure(fn) { + const original = fs.readFileSync; + fs.readFileSync = () => { + throw new Error('injected read failure'); + }; + try { + return fn(); + } finally { + fs.readFileSync = original; + } +} + +// ─── FIX 4 (chore/2930 review): adversarial parser-input fixtures ───────── +// CONTRIBUTING.md:484-513 requires adversarial fixtures for a new parser's +// inputs. Each case here either round-trips byte-identical or produces the +// correct typed REASON — never a message-text match. + +describe('adversarial content bytes', () => { + test('unicodeHeadingRoundTripsByteIdentical', () => { + const source = doc( + '# 見出し — Ünïcödé Hëading 🚀', + '', + 'body with 中文, кириллица, emoji 🎉', + '', + 'trailing プロース', + ); + const expected = doc('# 見出し — Ünïcödé Hëading 🚀', 'body with 中文, кириллица, emoji 🎉', 'trailing プロース'); + const rendered = composeWorkflow(source); + assert.equal(rendered, expected); + assert.equal(measureBytes(rendered), measureBytes(expected)); + }); + + test('nulByteInBodyRoundTripsByteIdentical', () => { + const source = `prose\0more\n\nbody\0with\0nul\n\nafter\0`; + const sections = parseWorkflowSections(source); + const explicitSections = sections.filter((s) => s.explicit); + assert.equal(explicitSections.length, 1); + assert.equal(explicitSections[0].body, 'body\0with\0nul\n'); + const rendered = composeWorkflow(source); + assert.equal(rendered, 'prose\0more\nbody\0with\0nul\nafter\0'); + }); + + test('unicodeReplacementCharacterRoundTripsByteIdentical', () => { + const source = `prose � end\n\nbody ��\n\nafter �`; + const rendered = composeWorkflow(source); + assert.equal(rendered, 'prose � end\nbody ��\nafter �'); + }); + + test('leadingByteOrderMarkRoundTripsByteIdentical', () => { + const source = '# Heading\n\nbody\n\ntail'; + const sections = parseWorkflowSections(source); + const gaps = sections.filter((s) => !s.explicit); + // The BOM is ordinary content of the leading gap — never stripped or + // otherwise special-cased by this parser. + assert.equal(gaps[0].body, '# Heading\n'); + const rendered = composeWorkflow(source); + assert.equal(rendered, '# Heading\nbody\ntail'); + }); +}); + +describe('adversarial fence shapes', () => { + test('fenceWithinFenceStaysLiteralUntilOuterCloser', () => { + const source = doc( + 'prose before', + '````', + '```', + '', + '```', + '````', + 'prose after', + ); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); + + test('tildeFenceHidesMarkerLookalike', () => { + const source = doc('prose', '~~~', '', '~~~', 'prose after'); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); + + test('indentedFenceUpToThreeSpacesHidesMarkerLookalike', () => { + const source = doc('prose', ' ```', '', ' ```', 'prose after'); + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); +}); + +describe('adversarial line-ending shapes', () => { + test('markerLineTerminatedByLoneCrIsNotRecognizedAsAMarker', () => { + // A bare `\r` with no accompanying `\n` anywhere in the document is not + // an EOL this grammar recognizes (only '' / '\n' / '\r\n' — see the + // module doc comment). The marker-shaped text is therefore never on its + // "own line" and must be left as ordinary literal content, not parsed + // as an open marker. + const source = '\rbody, never a real line break'; + const sections = parseWorkflowSections(source); + assert.equal(sections.length, 1); + assert.equal(sections[0].explicit, false); + assert.equal(sections[0].body, source); + assert.equal(composeWorkflow(source), source); + }); +}); + +describe('fs.readFileSync fault injection mid-compose', () => { + test('readFailureDuringCompositionLeavesNoPartialArtifact', (t) => { + const tmpDir = createTempDir('gsd-wf-fault-'); + t.after(() => cleanup(tmpDir)); + + const srcPath = path.join(tmpDir, 'source.md'); + const destPath = path.join(tmpDir, 'composed.md'); + fs.writeFileSync(srcPath, '\nbody\n\n'); + + function readComposeWrite() { + const content = fs.readFileSync(srcPath, 'utf8'); + const result = composeWorkflow(content, { sourcePath: srcPath }); + fs.writeFileSync(destPath, result); + return result; + } + + assert.throws( + () => withInjectedReadFailure(() => readComposeWrite()), + (err) => err instanceof Error && err.message === 'injected read failure', + ); + assert.equal(fs.existsSync(destPath), false, 'no partial artifact must be written when the read fails'); + + // Restored correctly: a subsequent real call succeeds and DOES write. + const result = readComposeWrite(); + assert.equal(fs.existsSync(destPath), true); + assert.equal(result, 'body\n'); + }); +});