From b0572c0108bd3effd64ee0374bceb275fd347899 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 2 Sep 2026 07:33:42 -0400 Subject: [PATCH] feat(#3674): extract shared file-overlap wave partitioner (#4166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#3674): characterize existing wave-dispatch output and add tests for the extracted partitioner Pins resolveWaveDispatch's and emitWorkflowScript's current, unextracted output (chain-overlap, disjoint-empty-set, and a multi-wave/multi-stage golden script) as a regression safety net ahead of extracting partitionStages into a standalone module. Also adds the new module's unit and property tests (test matrix rows 1-11) against its expected public API, which does not exist yet and is added in the next commit. * feat(#3674): extract file-overlap partitioner into a shared, generic module Moves partitionStages' greedy first-fit file-overlap algorithm into a new, dependency-free src/file-overlap-partitioner.cts module (partitionByFileOverlap), generalized over a plain {id, files}[] shape rather than claude-orchestration.cts's Plan/Wave interfaces. partitionStages becomes a thin adapter mapping its own Plan[] shape onto the generic input and back — behavior-preserving, no dependency ordering, no path normalization, no filesystem access moved or added. Enables a future consumer (quick-batch, #3675 / ADR-1239) to reuse the same primitive without pulling in orchestration internals. * docs(#3674): register the file-overlap-partitioner module bookkeeping New src/*.cts -> bin/lib/*.cjs modules need four hand-maintained registrations beyond the code itself: .gitignore (compiled artifact), eslint.config.mjs (ADR-457: lint the .cts, not the emitted .cjs), docs/INVENTORY.md's CLI Modules roster row (regenerated via gen-inventory-manifest.cjs --write), and a CONTEXT.md glossary entry matching the convention set by similarly-scoped leaf modules (text-lines.cts, plan-dependency-graph.cts, spec-section.cts). * fix(#3674): alphabetize INVENTORY.md row, manifest regen no-op, fast-check import already correct - docs/INVENTORY.md: move file-overlap-partitioner.cjs row to alphabetical position - docs/INVENTORY-MANIFEST.json: regenerated via gen-inventory-manifest.cjs --write, produced no diff (manifest is keyed by content, not row order) - tests/claude-orchestration.test.cjs's direct require('fast-check') is correct as-is: tests/helpers/fast-check-setup.cjs's own docstring scopes the shared-seed wrapper to "every *.property.test.cjs file"; claude-orchestration.test.cjs is not a .property.test.cjs file, and every .property.test.cjs file sampled uses the wrapper consistently. No outlier. * fix(#3674): constrain the no-overlap property test to unique ids, fixing an ambiguous duplicate-id reconstruction The `no two plans in the same stage share a modified file` property reconstructs which physical item produced each output id via `remaining.findIndex(r => r.id === id)`. Under duplicate ids (an explicitly-supported input shape for `partitionByFileOverlap`) that reconstruction can pick the wrong physical occurrence, producing a false-positive overlap failure (observed counterexample: p0(f1), p208(f1), p208([]) — correctly staged as [[p0,p208#2],[p208#1]], but misread by id-order as [[p0,p208#1],...], which do overlap). Properties (a) determinism and (b) totality already exercise duplicate ids correctly and are left unchanged; only this property's generated items are now constrained to unique ids via `fc.uniqueArray`, where the reconstruction is unambiguous. --------- Co-authored-by: sim --- .gitignore | 2 + CONTEXT.md | 3 + docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 1 + eslint.config.mjs | 2 + src/claude-orchestration.cts | 32 ++-- src/file-overlap-partitioner.cts | 77 +++++++++ tests/claude-orchestration.test.cjs | 160 ++++++++++++++++++ ...file-overlap-partitioner.property.test.cjs | 103 +++++++++++ tests/file-overlap-partitioner.test.cjs | 113 +++++++++++++ 10 files changed, 473 insertions(+), 21 deletions(-) create mode 100644 src/file-overlap-partitioner.cts create mode 100644 tests/file-overlap-partitioner.property.test.cjs create mode 100644 tests/file-overlap-partitioner.test.cjs diff --git a/.gitignore b/.gitignore index bca92bf41..fe420bd69 100644 --- a/.gitignore +++ b/.gitignore @@ -254,6 +254,8 @@ build/ /gsd-core/bin/lib/federated-config.cjs /gsd-core/bin/lib/phase-locator.cjs /gsd-core/bin/lib/plan-dependency-graph.cjs +# #3674: compiled from src/file-overlap-partitioner.cts (ADR-457 build-at-publish). +/gsd-core/bin/lib/file-overlap-partitioner.cjs /gsd-core/bin/lib/phase-estimation.cjs /gsd-core/bin/lib/estimate-cli.cjs /gsd-core/bin/lib/roadmap-parser.cjs diff --git a/CONTEXT.md b/CONTEXT.md index 71d7ffc43..6098d1239 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -43,6 +43,9 @@ Since #3882 (ADR-3473 §8.2) the module also owns **`listAllPhaseDirs(phasesDir, ### Plan Dependency Graph Module Module owning the single halt-propagation engine over a plan's `depends_on` DAG (#2830). **Domain term: _halted_** — a plan that reached a designed stop (a gate failure, a spike concluding without expanding, or any other intentional non-completion) and wrote a SUMMARY recording that fact via `status: halted` in its frontmatter, as opposed to `status: complete` (ordinary finish) or no SUMMARY at all (not yet attempted). **Domain term: _blocked_** — a plan whose `depends_on` chain reaches a halted plan, directly or transitively; distinct from merely _incomplete_ (no SUMMARY yet) — an ordinary in-progress/not-yet-started dependency does not block. `computeHaltPropagation(nodes: {id, resolvedDependsOn, halted}[])` performs exactly one Kahn's-algorithm topological pass and returns `{order, visited, blockedBy}`, where `blockedBy` maps a plan id to the de-duplicated set of halted plan ids transitively upstream of it (diamond-safe, any depth). This is the SHARED engine both of the two independent "which plans are incomplete" readers call — `phase.cts`'s wave-grouping (`cmdPhasePlanIndex`) and `phase-locator.cts`'s phase-location primitive (`searchPhaseInDir`) — so the two-implementation divergence that caused #2830 (one parsed `depends_on` for waves only, the other never parsed it at all) cannot recur: each caller resolves its own raw `depends_on` tokens to canonical ids before calling in, but the graph traversal itself exists in exactly one place. Pure — no I/O, no config; each caller does its own file reads (a plan's frontmatter, a completed plan's SUMMARY `status`) and fails open (treats an unreadable/malformed file as "not halted"/"no deps") rather than throwing. Source of truth: `gsd-core/bin/lib/plan-dependency-graph.cjs` (generated from `src/plan-dependency-graph.cts`). +### File Overlap Partitioner Module +Generic, dependency-free greedy first-fit file-overlap partitioner (#3674), extracted from `claude-orchestration.cts`'s `partitionStages` (#1143) as a behavior-preserving move — same algorithm, same output, no improvement. `partitionByFileOverlap(items: {id, files}[]) → string[][]` places each item into the earliest stage where it does not share a `files` entry with any item already there (in input order); an item with an empty `files` array overlaps nothing and coalesces into stage 0. Deliberately narrow scope, matching the ADR-1239 "Quick-batch binding" design lock: no dependency-DAG ordering (a caller resolves dependency order before calling in), no path normalization (`Foo.ts` vs `foo.ts`, or a forward-slash path vs its backslash-separator equivalent, compare as distinct files by exact string equality), and no filesystem access. Not guaranteed optimal — greedy first-fit can leave a smaller packing on the table for chain-overlap inputs (A∩B, B∩C, A∌C), which is `partitionStages`' own long-standing, intentional trade-off, preserved rather than "fixed" during extraction. `claude-orchestration.cts`'s `partitionStages` is now a thin adapter mapping its own `Plan[]` shape onto this module's generic `{id, files}[]` input and back, so `emitWorkflowScript`'s `files_modified` overlap → separate sequential stages behavior is unchanged. Exists so a future consumer (quick-batch, #3675) can partition its own planned-path items without pulling in orchestration internals. Pure, zero external dependencies, never throws. Source of truth: `gsd-core/bin/lib/file-overlap-partitioner.cjs` (generated from `src/file-overlap-partitioner.cts`). Tests: `tests/file-overlap-partitioner.test.cjs`, `tests/file-overlap-partitioner.property.test.cjs`. + ### Runtime Identity Module Module owning this package's runtime identity surface and the resolver preference that keeps a shipped workflow off a foreign handler (#3146). **Domain term: _colliding bin_** — a binary name published by more than one package with different semantics behind it; here `gsd-tools`, published by both this package and the predecessor `get-shit-done-cc` (verified against 1.42.3: `bin.gsd-tools → bin/gsd-sdk.js`), whose `phases.clear` **deletes** where this package's **archives**, both printing success-shaped output against a gitignored `.planning/` (#3129). **The fix is resolution, not detection.** `_runtime-launcher.snippet.sh`'s PATH branch resolves **`gsd_run`** — published only by this package, and self-locating via its own symlink chain to the `gsd-tools.cjs` beside it — instead of the colliding `gsd-tools`, so a foreign handler is unreachable from PATH; when no `gsd_run` is reachable the resolver fails closed through its remaining path-based branches rather than falling back to an arbitrary `gsd-tools`. `unset -f gsd_run` leading that branch is load-bearing: on a second source of the preamble `command -v gsd_run` finds the shell FUNCTION and returns the bare string `gsd_run`, which would otherwise define the function in terms of itself. An `[ -x ]` guard was tried here instead and REMOVED — it rejected the bare name, fell through every branch, and reached the resolver's `exit 1`, which kills a SOURCED caller's shell. **Resolution is not the whole fix (#3841).** The path-based branches — a project-local install, a runtime config directory — trust their configured location and have no structural guarantee, so the preamble ASSERTS identity once after resolving and before any verb runs: it probes `runtime-identity --raw` and matches **anchored at BOTH ends** of the compact payload — the `IDENTITY_RAW_PREFIX` opener, then anything, then a literal `}` — because an unanchored substring match verifies the decoy `{"packageName":"get-shit-done-cc","note":"@opengsd/gsd-core"}`, and an opener-only match verifies a TRUNCATED payload. Closing on `}` is safe for any future additive field: a JSON object's own brace is always the last character, whatever the last value's type. **The trailing `}` is also load-bearing for a reason unrelated to security, and removing it turns a distant test red.** The preamble is inlined into 112 shipped files, and several downstream guards balance braces over RAW TEXT with no awareness of shell quoting — `tests/new-project-mvp-prompt.test.cjs`'s #3784 brace guard scans `new-project.md` plus `new-project/steps/`, which carry one preamble copy EACH, so an off-by-one snippet reports a combined net depth of 2 in a test naming neither the launcher nor this issue (verified: that is exactly how #3841 first went red). `tests/runtime-launcher-parity.test.cjs` (F0) now pins brace balance at the snippet so the next edit fails on the file it broke. **Domain term: _identity status_** — the two-valued `GSD_IDENTITY_STATUS` (`ok`/`unverified`, frozen as `IDENTITY_STATUS`, bridged from the five-way reason by `statusForVerdict`) the preamble exports so the gate is asserted on a VALUE, never on its warning prose. The rollout is warn-then-fail: `unverified` prints one line and continues, because `no_identity_verb` cannot tell a foreign package from an `@opengsd/gsd-core` older than the verb, and at rollout the old-version case is the common one. **The byte budget was the blocker, and folding the resolver is what cleared it:** the preamble is inlined into 113 shipped files, several of which sat within single-digit bytes of frozen ceilings (`agents/gsd-verifier.md` had 16 bytes; `gsd-executor.md` 33; `execute-phase.md` 234), and a first attempt broke five of them. Collapsing the twenty near-identical `elif [ -f … ]` arms into one candidate-list helper (`_gsd_at`) buys far more than the assertion costs — net **1,876 bytes SMALLER** per inlined file. Editing this preamble is still a ceiling hazard; measure before adding. The module exports the identity surface backing the `runtime-identity` verb: `classifyIdentityProbe` (pure, total — `(stdout, exitCode, spawnFailed, timedOut)` → `ok`/`identity_mismatch`/`no_identity_verb`/`unparseable`/`probe_failed`; strict because `JSON.parse` admits `0`/`"str"`/`[]`/`null`/`true` and a truthiness test would verify `[]`), `buildIdentityPayload` over baked `package-identity.cjs` coordinates plus `readHostVersion()`, and `explainVerdict`. That verb is a manual diagnostic, not an automatic gate. Source of truth: `gsd-core/bin/lib/runtime-identity.cjs` (generated from `src/runtime-identity.cts`). diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index cc567c85d..51dd56989 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -379,6 +379,7 @@ "external-job.cjs", "fallow-runner.cjs", "federated-config.cjs", + "file-overlap-partitioner.cjs", "frontmatter.cjs", "gap-checker.cjs", "gate-predicate-evaluator.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index bd09aa99e..572b6abf7 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -529,6 +529,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`. | `external-job.cjs` | Produces scheduler manifests for asynchronous external jobs; SLURM is the first backend (#1164) | | `fallow-runner.cjs` | Fallow audit adapter for `/gsd-code-review`: binary resolution (`node_modules/.bin` then `PATH`), actionable missing-binary errors, and structural findings normalization | | `federated-config.cjs` | Defensive merge of capability-declared config slices into the loadConfig return value — ADR-857 phase 3b; exports `mergeFederatedConfig({ configSchema, isCentralKey, userConfig })` → `{ values, validKeys, warnings }`; live for migrated Capability keys that are atomically removed from the central config schema | +| `file-overlap-partitioner.cjs` | Generic greedy first-fit file-overlap partitioner (#3674) — `partitionByFileOverlap(items: {id, files}[]) → string[][]`, extracted from `claude-orchestration.cjs`'s `partitionStages` behavior-preserving; no `Plan`/`Wave` dependency, no path normalization, no dependency-graph ordering. Compiled from `src/file-overlap-partitioner.cts` | | `frontmatter.cjs` | YAML frontmatter CRUD operations | | `gap-checker.cjs` | Post-planning gap analysis (#2493): unified REQUIREMENTS.md + CONTEXT.md decisions vs PLAN.md coverage report (`gsd-tools gap-analysis`) | | `gate-predicate-evaluator.cjs` | Evaluates capability gate predicates — `command-exit-zero` and `artifact-frontmatter` (#2008) | diff --git a/eslint.config.mjs b/eslint.config.mjs index 370dd5656..35ab00a4e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -253,6 +253,8 @@ export default tseslint.config( 'gsd-core/bin/lib/config-loader.cjs', 'gsd-core/bin/lib/phase-locator.cjs', 'gsd-core/bin/lib/plan-dependency-graph.cjs', + // #3674: tsc-generated runtime artifact — lint the src/file-overlap-partitioner.cts source, not this. + 'gsd-core/bin/lib/file-overlap-partitioner.cjs', 'gsd-core/bin/lib/roadmap-parser.cjs', 'gsd-core/bin/lib/drift.cjs', 'gsd-core/bin/lib/cjs-command-router-adapter.cjs', diff --git a/src/claude-orchestration.cts b/src/claude-orchestration.cts index 1dc98c375..9782d2079 100644 --- a/src/claude-orchestration.cts +++ b/src/claude-orchestration.cts @@ -43,6 +43,10 @@ * Zero external dependencies. Pure functions. Never throws on bad input. */ +// eslint-disable-next-line @typescript-eslint/no-require-imports -- file-overlap-partitioner.cjs is an export= CommonJS module +import fileOverlapPartitionerMod = require('./file-overlap-partitioner.cjs'); +const { partitionByFileOverlap } = fileOverlapPartitionerMod; + // ─── Constants ──────────────────────────────────────────────────────────────── /** @@ -330,29 +334,15 @@ interface EmitErr { * * This is the same overlap rule execute-phase applies inline — the only difference * is the execution vehicle (Workflow `parallel()` vs one-agent-per-message). + * + * #3674: the algorithm itself now lives in the generic, dependency-free + * `file-overlap-partitioner.cts` module (`partitionByFileOverlap`) — this is a + * thin adapter mapping this module's own `Plan[]` shape onto that module's + * generic `{ id, files }[]` input and back. Behavior-preserving extraction: + * same greedy first-fit algorithm, same output, only the implementation moved. */ function partitionStages(plans: Plan[]): string[][] { - const stages: { plans: Plan[]; files: Set }[] = []; - for (const plan of plans) { - const fileSet = new Set(plan.files_modified); - let placed = false; - for (const stage of stages) { - let overlap = false; - for (const f of fileSet) { - if (stage.files.has(f)) { overlap = true; break; } - } - if (!overlap) { - stage.plans.push(plan); - for (const f of fileSet) stage.files.add(f); - placed = true; - break; - } - } - if (!placed) { - stages.push({ plans: [plan], files: new Set(fileSet) }); - } - } - return stages.map((s) => s.plans.map((p) => p.id)); + return partitionByFileOverlap(plans.map((p) => ({ id: p.id, files: p.files_modified }))); } /** diff --git a/src/file-overlap-partitioner.cts b/src/file-overlap-partitioner.cts new file mode 100644 index 000000000..484e61b7b --- /dev/null +++ b/src/file-overlap-partitioner.cts @@ -0,0 +1,77 @@ +/** + * File-overlap partitioner — shared greedy first-fit stage assignment (#3674). + * + * Extracted from `claude-orchestration.cts`'s `partitionStages` (#1143), which + * emits sequential Workflow `parallel()` stage barriers so that no two plans + * sharing a `files_modified` entry ever cohabit a stage. This module is the + * SAME algorithm, generalized: it depends on no `Plan`/`Wave` interface from + * `claude-orchestration.cts` (or any other caller-specific shape), so a future + * consumer (quick-batch, #3675 / ADR-1239 "Quick-batch binding") can partition + * its own planned-path items without pulling in orchestration internals. + * + * This is a pure, behavior-preserving extraction (#3674's acceptance bar is + * byte-identical output for `claude-orchestration.cts`'s existing callers — + * not an improvement). Explicitly OUT of scope, per the #3674 design lock: + * - dependency-DAG ordering — this module only ever sees a flat item list + * and file-overlap; a caller resolves dependency order before calling in + * (same contract `partitionStages` already had); + * - path normalization — file entries are compared by exact string equality + * only. `Foo.ts` vs `foo.ts`, or `src/a.ts` vs `src\a.ts`, are treated as + * DISTINCT files. This is deliberate, not a gap to "fix" during extraction; + * - filesystem access — this module never reads a path off disk. + * + * Zero external dependencies. Pure function. Never throws on well-typed input. + */ + +/** A generic overlap-partitioned item: an id plus the files it touches. */ +interface OverlapItem { + id: string; + files: string[]; +} + +/** + * Partition `items` into a near-minimal number of sequential stages (via + * greedy first-fit — not guaranteed optimal for arbitrary overlap graphs, but + * correct: no two items sharing a file ever cohabit a stage) such that no two + * items in the same stage share a file. Each item goes into the earliest + * stage where it does not overlap any item already there, in input order. + * + * An item with an EMPTY `files` array declares no files; it overlaps nothing + * and coalesces into stage 0 (mirrors `partitionStages`' original behavior — + * this module cannot guard against undeclared concurrent writes; a caller + * must declare `files` accurately). + * + * File comparison is EXACT STRING EQUALITY — no path normalization, no + * case-folding, no separator canonicalization. Duplicate `id`s in the input + * are NOT deduplicated; each item is placed independently, in input order. + * + * Deterministic: identical input (including input order) always yields an + * identical partition. + */ +function partitionByFileOverlap(items: OverlapItem[]): string[][] { + const stages: { items: OverlapItem[]; files: Set }[] = []; + for (const item of items) { + const fileSet = new Set(item.files); + let placed = false; + for (const stage of stages) { + let overlap = false; + for (const f of fileSet) { + if (stage.files.has(f)) { overlap = true; break; } + } + if (!overlap) { + stage.items.push(item); + for (const f of fileSet) stage.files.add(f); + placed = true; + break; + } + } + if (!placed) { + stages.push({ items: [item], files: new Set(fileSet) }); + } + } + return stages.map((s) => s.items.map((i) => i.id)); +} + +export = { + partitionByFileOverlap, +}; diff --git a/tests/claude-orchestration.test.cjs b/tests/claude-orchestration.test.cjs index 158b6de6e..2e7c7c670 100644 --- a/tests/claude-orchestration.test.cjs +++ b/tests/claude-orchestration.test.cjs @@ -601,6 +601,166 @@ describe('emitWorkflowScript — per-plan use_worktree (#2772 / #2285 finding 1) }); }); +// ─── 3.6. #3674 — pre-extraction characterization (golden output pins) ──────── +// +// `partitionStages` is being extracted into a standalone, generic module +// (`file-overlap-partitioner.cts`). The extraction's acceptance bar is +// byte-identical output from `resolveWaveDispatch` and `emitWorkflowScript` +// (#3674's own callers) — NOT improved output. These tests pin the ACTUAL +// current production output (captured from the unmodified, pre-extraction +// code) for representative multi-plan fixtures, so the extraction cannot +// silently change what either function returns. They must pass BOTH before +// AND after the extraction lands. + +describe('#3674 — pre-extraction characterization (golden output pins)', () => { + + test('characterization: chain-overlap plans (A∩B, B∩C, A∌C) produce the greedy, non-optimal stage assignment', () => { + // A and B share f2; B and C share f3; A and C share nothing. A minimal + // packing could put A and C together after B — greedy first-fit does NOT + // find that; it processes in input order and is not required to be optimal. + const r = emitWorkflowScript({ + phaseDir: '.planning/phases/01-chain', + runId: 'run-chain-3674', + waves: [{ + id: 'w1', + plans: [ + { id: 'A', brief: 'Plan A', files_modified: ['f1.ts', 'f2.ts'] }, + { id: 'B', brief: 'Plan B', files_modified: ['f2.ts', 'f3.ts'] }, + { id: 'C', brief: 'Plan C', files_modified: ['f3.ts', 'f4.ts'] }, + ], + }], + }); + assert.strictEqual(r.ok, true); + assert.deepStrictEqual(r.summary.stagesByWave, [[['A', 'C'], ['B']]]); + }); + + test('characterization: two plans with empty files_modified coalesce into stage 0 without colliding', () => { + const r = emitWorkflowScript({ + phaseDir: '.planning/phases/01-empty', + runId: 'run-empty-3674', + waves: [{ + id: 'w1', + plans: [ + { id: 'p1', brief: 'Plan with no files', files_modified: [] }, + { id: 'p2', brief: 'Another empty plan', files_modified: [] }, + ], + }], + }); + assert.strictEqual(r.ok, true); + assert.deepStrictEqual(r.summary.stagesByWave, [[['p1', 'p2']]]); + }); + + test('characterization: resolveWaveDispatch output is captured before extraction and is byte-identical after (multi-wave, multi-stage fixture)', () => { + const CAPABLE_HOST_3674 = { dispatch: { namedDispatch: true, nested: true, background: true, backgroundDispatch: false } }; + const input = { + runtimeId: 'claude', + hostIntegration: CAPABLE_HOST_3674, + agentSdkVersion: '1.2.0', + config: { 'claude_orchestration.enabled': true, 'claude_orchestration.execution_backend': 'workflow' }, + phaseDir: '.planning/phases/01-resolve3674', + runId: 'run-resolve-3674', + waves: [ + { id: 'w1', plans: [ + { id: 'p1', brief: 'Plan P1', files_modified: ['src/shared.cts', 'src/a.cts'] }, + { id: 'p2', brief: 'Plan P2', files_modified: ['src/shared.cts', 'src/b.cts'] }, + { id: 'p3', brief: 'Plan P3', files_modified: ['src/c.cts'] }, + ] }, + { id: 'w2', plans: [ + { id: 'p4', brief: 'Plan P4', files_modified: [] }, + ] }, + ], + }; + + const r = resolveWaveDispatch(input); + + assert.strictEqual(r.backend, 'workflow'); + assert.strictEqual(r.reason, 'workflow_backend_active'); + assert.deepStrictEqual(r.summary, { + waves: 2, + plans: 4, + worktreePlans: 4, + stagesByWave: [[['p1', 'p3'], ['p2']], [['p4']]], + resumeRunId: 'run-resolve-3674', + budgetTokens: null, + }); + + // Golden script captured verbatim from the unmodified pre-extraction + // production code (`node -e` against `gsd-core/bin/lib/claude-orchestration.cjs` + // before any source under this phase was touched). Any divergence here is + // an observable behavior change in the emitted Workflow script. + const GOLDEN_SCRIPT_3674 = [ + 'export const meta = {', + ' name: "gsd-execute-run-resolve-3674",', + ' description: "GSD wave dispatch for .planning/phases/01-resolve3674",', + ' phases: [', + ' { title: "Wave w1", detail: "3 plan(s)" },', + ' { title: "Wave w2", detail: "1 plan(s)" },', + ' ],', + '}', + '', + '// GSD Workflow script — generated by the claude-orchestration capability (#1143)', + '// phase: .planning/phases/01-resolve3674', + '// BETA: preview-grade; on any failure the orchestrator falls back to inline dispatch.', + '// Composes the SAME gsd-executor agent as the inline path, so artifacts (SUMMARY.md)', + '// and commits are produced identically. Worktree isolation is per-plan (use_worktree)', + '// and mirrors execute-phase.md step 2.5\'s submodule gate exactly (#2772 / #2285).', + '// model: none applied — resolved to "inherit"/empty, so each agent inherits the', + '// orchestrator model (#2517: emitting an empty model 404s on some runtimes).', + '//', + '// resume: pass "run-resolve-3674" as the Workflow tool\'s resumeFromRunId input', + '// (it is a tool parameter, NOT a script function).', + '', + '// #3302: extract the executor-returned JSON so the', + '// orchestrator can record it into WAVE_WORKTREE_MANIFEST after the run', + '// (worktree.record-agent -> worktree.cleanup-wave, the same manifest-scoped', + '// merge chain inline dispatch feeds). null = absent/unparseable/interrupted.', + 'function gsdWorktreeMetadata(agentResult) {', + ' if (typeof agentResult !== \'string\') return null;', + ' const m = agentResult.match(/([\\s\\S]*?)<\\/worktree_metadata>/);', + ' if (m === null) return null;', + ' try {', + ' const parsed = JSON.parse(m[1]);', + ' return (parsed !== null && typeof parsed === \'object\') ? parsed : null;', + ' } catch (e) {', + ' return null;', + ' }', + '}', + 'const gsdAgentOutcomes = [];', + '', + '// Wave w1', + 'phase("Wave w1")', + '// Stage 0', + 'const gsdStage_0_0 = await parallel([', + ' () => agent("Plan P1", { agentType: "gsd-executor", isolation: "worktree" }),', + ' () => agent("Plan P3", { agentType: "gsd-executor", isolation: "worktree" }),', + '])', + 'gsdAgentOutcomes.push(', + ' { plan: "p1", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_0_0[0]) },', + ' { plan: "p3", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_0_0[1]) }', + ')', + '// Stage 1 (sequential — files_modified overlap)', + 'const gsdStage_0_1 = await parallel([', + ' () => agent("Plan P2", { agentType: "gsd-executor", isolation: "worktree" }),', + '])', + 'gsdAgentOutcomes.push(', + ' { plan: "p2", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_0_1[0]) }', + ')', + '', + '// Wave w2', + 'phase("Wave w2")', + 'const gsdStage_1_0 = await parallel([', + ' () => agent("Plan P4", { agentType: "gsd-executor", isolation: "worktree" }),', + '])', + 'gsdAgentOutcomes.push(', + ' { plan: "p4", expects_worktree: true, metadata: gsdWorktreeMetadata(gsdStage_1_0[0]) }', + ')', + 'return gsdAgentOutcomes', + ].join('\n'); + + assert.strictEqual(r.script, GOLDEN_SCRIPT_3674); + }); +}); + // ─── 4. Capability declaration validation ───────────────────────────────────── describe('capability declaration (capabilities/claude-orchestration/capability.json)', () => { diff --git a/tests/file-overlap-partitioner.property.test.cjs b/tests/file-overlap-partitioner.property.test.cjs new file mode 100644 index 000000000..3af3da967 --- /dev/null +++ b/tests/file-overlap-partitioner.property.test.cjs @@ -0,0 +1,103 @@ +'use strict'; + +/** + * Property-based tests for file-overlap-partitioner.cjs (#3674) + * + * Module: gsd-core/bin/lib/file-overlap-partitioner.cjs + * Exported: partitionByFileOverlap(items: { id: string; files: string[] }[]) -> string[][] + * + * Properties tested (test matrix rows 9-11): + * (a) determinism — two runs on the same input produce an identical partition + * (b) totality — every input item appears in exactly one output stage, + * never lost or duplicated + * (c) the core invariant — no two items in the same stage share a file + * + * Duplicate `id`s are a real, intentionally-supported input shape (see + * `partitionByFileOverlap`'s doc comment) and properties (a) and (b) verify + * it correctly — (a) needs no per-item identity at all, and (b) only compares + * the sorted id multiset, so it never needs to pick out *which* physical item + * a given output id refers to. Property (c) is different: checking "do these + * two co-staged items' file sets overlap" requires reconstructing which + * physical item (files included) produced each id in the output, and under + * duplicate ids that reconstruction is ambiguous — a stage's "p208" could be + * either physical p208 occurrence, and picking the wrong one produces a false + * failure (see #3674 regression: `p0(f1)`, `p208(f1)`, `p208([])` staged + * correctly as `[[p0,p208#2],[p208#1]]`, misread as `[[p0,p208#1],...]` by an + * id-order reconstruction). So (c) alone constrains its generated items to + * unique ids, where the reconstruction is unambiguous by construction. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fc = require('./helpers/fast-check-setup.cjs'); + +const { partitionByFileOverlap } = require('../gsd-core/bin/lib/file-overlap-partitioner.cjs'); + +/** A single overlap item: a small id plus a small set of file tokens (some shared, some not). */ +const itemArb = fc.record({ + id: fc.integer({ min: 0, max: 999 }).map((n) => 'p' + n), + files: fc.array(fc.constantFrom('f0', 'f1', 'f2', 'f3', 'f4', 'f5', 'f6', 'f7'), { maxLength: 4 }), +}); + +describe('partitionByFileOverlap: properties', () => { + + test('property: repeated runs on the same input are deterministic', () => { + fc.assert(fc.property( + fc.array(itemArb, { minLength: 0, maxLength: 60 }), + (items) => { + const a = partitionByFileOverlap(items); + const b = partitionByFileOverlap(items); + assert.deepStrictEqual(a, b); + }, + )); + }); + + test('property: every input plan appears in exactly one output stage', () => { + fc.assert(fc.property( + fc.array(itemArb, { minLength: 0, maxLength: 60 }), + (items) => { + const stages = partitionByFileOverlap(items); + const flat = stages.flatMap((s) => s); + assert.strictEqual(flat.length, items.length, 'no item lost or duplicated across stages'); + // Positional identity: nth occurrence in flattened output must match + // input order's ids, one-to-one (id alone is not unique under + // duplicates, so compare the full multiset via sorted copies). + assert.deepStrictEqual(flat.slice().sort(), items.map((i) => i.id).sort()); + }, + )); + }); + + test('property: no two plans in the same stage share a modified file', () => { + fc.assert(fc.property( + // Unique ids only: this property reconstructs which physical item + // produced each output id, and that reconstruction is ambiguous when + // ids duplicate (see the module doc comment above). Properties (a) + // and (b) still fuzz duplicate ids via the shared `itemArb`. + fc.uniqueArray(itemArb, { minLength: 0, maxLength: 60, selector: (it) => it.id }), + (items) => { + const stages = partitionByFileOverlap(items); + // Positional lookup (not strictly required once ids are unique, but + // kept for symmetry with the reconstruction shape and to tolerate + // any future relaxation of the uniqueness constraint above). + const remaining = items.map((i) => ({ id: i.id, files: new Set(i.files) })); + for (const stageIds of stages) { + const stageItems = stageIds.map((id) => { + const idx = remaining.findIndex((r) => r.id === id); + const found = remaining[idx]; + remaining.splice(idx, 1); + return found; + }); + for (let i = 0; i < stageItems.length; i++) { + for (let j = i + 1; j < stageItems.length; j++) { + const a = stageItems[i].files; + const b = stageItems[j].files; + let overlap = false; + for (const f of a) if (b.has(f)) { overlap = true; break; } + assert.ok(!overlap, `stage-mates ${stageItems[i].id} and ${stageItems[j].id} must not share a file`); + } + } + } + }, + )); + }); +}); diff --git a/tests/file-overlap-partitioner.test.cjs b/tests/file-overlap-partitioner.test.cjs new file mode 100644 index 000000000..139d81881 --- /dev/null +++ b/tests/file-overlap-partitioner.test.cjs @@ -0,0 +1,113 @@ +'use strict'; + +/** + * file-overlap-partitioner.test.cjs — Behavioral tests for the shared + * file-overlap wave partitioner (#3674), extracted from + * `claude-orchestration.cts`'s `partitionStages` (#1143). + * + * Module: gsd-core/bin/lib/file-overlap-partitioner.cjs + * Exported: partitionByFileOverlap(items: { id: string; files: string[] }[]) -> string[][] + * + * These tests pin the generic module's own contract, independent of any + * `Plan`/`Wave` shape from `claude-orchestration.cts`. Several test names + * carry a "(characterization)" suffix per the #3674 test matrix (rows 5, 6): + * they characterize `partitionStages`' original algorithm — traced from its + * source, since duplicate-id and chain-overlap inputs were never reachable + * through `emitWorkflowScript`'s public API (it validates plan ids unique + * before ever calling the partitioner) — now pinned against the extracted, + * generic entry point. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { partitionByFileOverlap } = require('../gsd-core/bin/lib/file-overlap-partitioner.cjs'); + +describe('partitionByFileOverlap', () => { + + test('partitions overlapping plans into separate stages', () => { + const stages = partitionByFileOverlap([ + { id: 'p1', files: ['shared.ts'] }, + { id: 'p2', files: ['shared.ts'] }, + { id: 'p3', files: ['shared.ts'] }, + ]); + assert.strictEqual(stages.length, 3, 'every plan shares the same file -> each gets its own stage'); + assert.deepStrictEqual(stages, [['p1'], ['p2'], ['p3']]); + }); + + test('coalesces disjoint plans into stage 0', () => { + const stages = partitionByFileOverlap([ + { id: 'p1', files: ['a.ts'] }, + { id: 'p2', files: ['b.ts'] }, + { id: 'p3', files: ['c.ts'] }, + ]); + assert.strictEqual(stages.length, 1, 'fully disjoint plans coalesce into one stage'); + assert.deepStrictEqual(stages[0].slice().sort(), ['p1', 'p2', 'p3']); + }); + + test('an empty file set joins the first stage', () => { + const stages = partitionByFileOverlap([ + { id: 'p1', files: [] }, + ]); + assert.deepStrictEqual(stages, [['p1']]); + }); + + test('two plans with empty file sets do not collide', () => { + const stages = partitionByFileOverlap([ + { id: 'p1', files: [] }, + { id: 'p2', files: [] }, + ]); + assert.strictEqual(stages.length, 1, 'two empty file sets never overlap each other'); + assert.deepStrictEqual(stages[0], ['p1', 'p2']); + }); + + test('duplicate plan ids are not deduplicated by the partitioner (characterization)', () => { + // Same id, overlapping files: each occurrence is placed independently by + // the greedy first-fit walk (id is never used as a dedup/merge key) — the + // second occurrence overlaps the first's file set and lands in stage 1. + const overlapping = partitionByFileOverlap([ + { id: 'p1', files: ['a.ts'] }, + { id: 'p1', files: ['a.ts'] }, + ]); + assert.deepStrictEqual(overlapping, [['p1'], ['p1']], 'both occurrences of the duplicate id are preserved, one per stage'); + + // Same id, disjoint files: both occurrences independently coalesce into + // stage 0 (files are what is checked for overlap, never the id). + const disjoint = partitionByFileOverlap([ + { id: 'p1', files: ['a.ts'] }, + { id: 'p1', files: ['b.ts'] }, + ]); + assert.deepStrictEqual(disjoint, [['p1', 'p1']], 'duplicate ids with disjoint files both land in stage 0, not merged'); + }); + + test('chain-overlap plans produce the greedy, non-optimal assignment (characterization)', () => { + // A∩B (share f2), B∩C (share f3), A∌C (no shared file). A minimal + // assignment could place A and C together after B, but greedy first-fit + // processes in input order: A -> stage0; B overlaps A -> stage1; C does + // NOT overlap stage0 (A's files are f1,f2; C's are f3,f4) -> stage0. + const stages = partitionByFileOverlap([ + { id: 'A', files: ['f1', 'f2'] }, + { id: 'B', files: ['f2', 'f3'] }, + { id: 'C', files: ['f3', 'f4'] }, + ]); + assert.deepStrictEqual(stages, [['A', 'C'], ['B']], 'greedy first-fit, not optimal bin-packing'); + }); + + test('path casing is compared by exact string, not normalized', () => { + const stages = partitionByFileOverlap([ + { id: 'p1', files: ['Foo.ts'] }, + { id: 'p2', files: ['foo.ts'] }, + ]); + assert.strictEqual(stages.length, 1, '"Foo.ts" and "foo.ts" are treated as distinct files -> no forced split'); + assert.deepStrictEqual(stages[0], ['p1', 'p2']); + }); + + test('backslash and forward-slash paths are not normalized to the same file', () => { + const stages = partitionByFileOverlap([ + { id: 'p1', files: ['src\\foo.ts'] }, + { id: 'p2', files: ['src/foo.ts'] }, + ]); + assert.strictEqual(stages.length, 1, 'no separator normalization -> the two strings never compare equal'); + assert.deepStrictEqual(stages[0], ['p1', 'p2']); + }); +});