diff --git a/README.md b/README.md index ce36996..29fde31 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Craft, sync and convert AI-coding workspace harnesses — rules, agents, commands, skills, MCP servers — across every client workspace you maintain and every AI coder each team uses. -> Status: **phase 0 prototype**. `import`, `sync`, `status`, `diff`, `explain`, `ls` work end-to-end for the `claude-code` and `kiro` targets and are validated byte-for-byte against a real workspace (see *Oracle*). The Forge commands `forge variants`, `forge diff` (with a suggested class per hunk) and `forge unify` (`--take base|variant`, and `take: param` to turn a value into a `{{key}}` parameter) also work, covered by the unit and golden suites rather than the oracle, and `import` is render-aware: importing into an existing Forge reuses a base whose render equals the workspace file, and edits an existing profile and the recipes it owns in place. Ingredient bodies can hold client-specific **sections** (`` … ``), overridden by a profile's `sections` and a workspace's `overrides.sections`; import infers each client's content, and a Forge that uses them declares `schema: 2` (see *Forge layout*). Everything else in the spec (profiles with IDP/PM-tool integrations, local services, `dotnet new` project templates, `craftar ui`, `craftar mcp`) is not built yet. +> Status: **phase 0 prototype**. `import`, `sync`, `status`, `diff`, `explain`, `ls` work end-to-end for the `claude-code` and `kiro` targets and are validated byte-for-byte against a real workspace (see *Oracle*). The Forge commands `forge variants`, `forge diff` (with a suggested class per hunk) and `forge unify` (`--take base|variant`, `take: param` to turn a value into a `{{key}}` parameter, and `take: section` to turn a client block into a section) also work, covered by the unit and golden suites rather than the oracle, and `import` is render-aware: importing into an existing Forge reuses a base whose render equals the workspace file, and edits an existing profile and the recipes it owns in place. Ingredient bodies can hold client-specific **sections** (`` … ``), overridden by a profile's `sections` and a workspace's `overrides.sections`; import infers each client's content and `forge unify` can extract a section from a variant, and a Forge that uses them declares `schema: 2` (see *Forge layout*). Everything else in the spec (profiles with IDP/PM-tool integrations, local services, `dotnet new` project templates, `craftar ui`, `craftar mcp`) is not built yet. ## The idea in one paragraph @@ -59,7 +59,7 @@ From then on, change a rule in `forge/ingredients/rules//rule.md`, run `cr | `craftar ls` | Recipes and ingredients resolved for this workspace. | | `craftar forge variants [--forge \| --workspace ] [--json]` | Lists ingredients that have variants, nearest first, with the profile each came from, its distance to the base and its hunks counted by suggested class (`[1 evolution · 2 block]`), then any variant whose base is missing from the Forge. Read-only; exits 0 either way. `--json` prints `{groups, orphans}`; each variant carries `classes: {evolution, value, block}`, which add up to `distance.hunks`. | | `craftar forge diff [--against ] [--forge \| --workspace ] [--json]` | Shows the differences between a base ingredient and each of its variants: a header carrying the same distance `forge variants` reports, then hunk by hunk, each with a suggested class and reason — `evolution` (one side is newer text), `value` (an identifier-like token swapped in shared prose, with a suggested `param.`) or `block` (lines only one side has) — then the files that exist on only one side. A suggestion never decides anything. It compares the raw Forge text, section marker lines included, so a base with markers shows them as hunks against a variant that has none. Read-only. `--json` prints an array of `{ref, profile, distance, diff}`; each hunk carries `suggestion: {class, reason, tokens?}` next to its `kind`. | -| `craftar forge unify --profile

(--take base\|variant \| --plan \| --save-plan ) [--forge

\| --workspace ] [--json]` | Resolves one variant back into its base, hunk by hunk. `--save-plan` writes a reviewable plan with every decision set to `keep`, each hunk annotated with its suggested class (which `--plan` ignores) and, for a `value` hunk, a pre-filled `params` list (`token` → suggested `key`), and writes nothing else; it refuses a path that resolves inside the Forge (after `..` segments and symlinks) and a path that already exists, so it never overwrites a Forge file or a plan you already edited. A hunk set to `take: param` turns each listed token into `{{key}}` in the base, declares the base's text as the key's default in the base's `ingredient.yaml` and writes the variant's text into the variant profile's `profile.yaml` `params` — only after proving that both the base's and the variant's text render back exactly, and only when the same plan resolves the variant; it refuses (with the Forge untouched) a key another layer, profile or ingredient already uses, a variant another profile also uses, a whitespace-only difference, and a YAML file that would not round-trip unchanged. `--plan` applies an edited plan, refusing when it is not for this exact ingredient/profile or either side's fingerprint moved since it was saved. `--take base\|variant` resolves every decision to that side. Writes the merged base and, once every difference is resolved, removes the variant and rewrites every recipe reference to it (`ingredients`) to name the base. It never deletes a recipe and never edits an `extends`; it edits a profile only to write the values of a `take: param` extraction into the variant's own profile, and otherwise never: a `--` left identical to `` is reported as a warning, to be removed by hand after repointing the lists that name it — unify does not, because a workspace or an `extends` chain may also name `` and the recipe order or param precedence would change. Unify cannot reach workspaces, so a removed variant is reported as a warning for any `craftar.yaml` that disables it in `overrides.ingredients.disable`. Every recipe rewrite is checked before the first write, so one that cannot land (a reference behind a YAML alias) is refused with the Forge untouched; a failure after writing began (an I/O error, a locked file) names the paths already touched and the `git checkout` / `git clean` commands that undo them. A merge never adds, removes or changes a section marker: a plan or `--take variant` whose result would (a base with markers against a variant without them) is refused before the first write — take `base` for the marker lines (`--take base` always passes); the citation checks of `take: param` also read the profiles' section values, which can cite `{{key}}`. `ingredient.yaml` is never merged: when the two sides' metadata differs (beyond `name`, `as` and `origin`), the variant stays unresolved and the differing fields are named — edit it by hand, or `--take base` to discard the variant, metadata included. Requires a clean git checkout in the Forge with at least one commit (`--save-plan` excepted) — the Forge has no lock, so git is the undo. It also refuses when any path it would overwrite or delete (the base, the variant, the recipe files it rewrites, the profile a parameter extraction edits) is ignored, untracked, modified, or flagged skip-worktree or assume-unchanged in the index, since git could not restore it. `--json` prints `{base, profile, resolved, written, removed, unresolved, variantRemoved, recipes: {rewritten, identicalToSibling}, metaDiffers, params, profileEdited, warnings}` for `--take`/`--plan` (every key always present, `[]` when empty), or `{base, profile, plan, unresolved}` for `--save-plan`. | +| `craftar forge unify --profile

(--take base\|variant \| --plan \| --save-plan ) [--forge

\| --workspace ] [--json]` | Resolves one variant back into its base, hunk by hunk. `--save-plan` writes a reviewable plan with every decision set to `keep`, each hunk annotated with its suggested class (which `--plan` ignores) and, for a `value` hunk, a pre-filled `params` list (`token` → suggested `key`); on a `block` hunk, a pre-filled `section: { name }` from the Markdown heading above (or `section-`); and on a hunk touching a section the base already has, that section's name. It refuses a path that resolves inside the Forge (after `..` segments and symlinks) and a path that already exists, so it never overwrites a Forge file or a plan you already edited; it writes nothing else. A hunk set to `take: param` turns each listed token into `{{key}}` in the base, declares the base's text as the key's default in the base's `ingredient.yaml` and writes the variant's text into the variant profile's `profile.yaml` `params` — only after proving that both the base's and the variant's text render back exactly, and only when the same plan resolves the variant; it refuses (with the Forge untouched) a key another layer, profile or ingredient already uses, a variant another profile also uses, a whitespace-only difference, and a YAML file that would not round-trip unchanged. A hunk set to `take: section` (with `section: { name, lines? }`) wraps the base's lines in section markers as the default and writes the variant's lines into the variant profile's `sections` — consecutive hunks with one name form one section, `lines: "-"` widens it over equal base lines, and hunks inside a section the base already has only write the profile value; it is proved (both sides render back exactly) and needs the same plan to resolve the variant; `take: param` and `take: section` may share a plan, never a hunk. It refuses (Forge untouched) a variant that holds markers, a profile that already sets that section (another profile for a new section, or the variant's profile with other content), a span that cuts a hunk or crosses another section, a span that covers a hunk outside the run (including `take: base` on a hunk inside a section the plan fills), a hunk mixing lines inside and outside an existing section, and a span reaching a missing final newline; when it adds a section to a Forge still at `schema: 1` it sets `schema: 2` in `craftar.forge.yaml` in place (refused if the file does not round-trip). `--plan` applies an edited plan, refusing when it is not for this exact ingredient/profile or either side's fingerprint moved since it was saved. `--take base\|variant` resolves every decision to that side. Writes the merged base and, once every difference is resolved, removes the variant and rewrites every recipe reference to it (`ingredients`) to name the base. It never deletes a recipe and never edits an `extends`; it edits a profile only to write the values of a `take: param` or `take: section` extraction into the variant's own profile, and otherwise never: a `--` left identical to `` is reported as a warning, to be removed by hand after repointing the lists that name it — unify does not, because a workspace or an `extends` chain may also name `` and the recipe order or param precedence would change. Unify cannot reach workspaces, so a removed variant is reported as a warning for any `craftar.yaml` that disables it in `overrides.ingredients.disable`. Every recipe rewrite is checked before the first write, so one that cannot land (a reference behind a YAML alias) is refused with the Forge untouched; a failure after writing began (an I/O error, a locked file) names the paths already touched and the `git checkout` / `git clean` commands that undo them. A merge never adds, removes or changes a section marker other than the sections the plan declares; a plan or `--take variant` that would is refused before the first write — take `base` for the marker lines, or `take: section` to fill the section; the citation checks of `take: param` also read the profiles' section values, which can cite `{{key}}`. `ingredient.yaml` is never merged: when the two sides' metadata differs (beyond `name`, `as` and `origin`), the variant stays unresolved and the differing fields are named — edit it by hand, or `--take base` to discard the variant, metadata included. Requires a clean git checkout in the Forge with at least one commit (`--save-plan` excepted) — the Forge has no lock, so git is the undo. It also refuses when any path it would overwrite or delete (the base, the variant, the recipe files it rewrites, the profile a parameter or section extraction edits, `craftar.forge.yaml` when it is bumped) is ignored, untracked, modified, or flagged skip-worktree or assume-unchanged in the index, since git could not restore it. `--json` prints `{base, profile, resolved, written, removed, unresolved, variantRemoved, recipes: {rewritten, identicalToSibling}, metaDiffers, params, profileEdited, sections, manifestEdited, warnings}` for `--take`/`--plan` (every key always present, `[]` when empty), or `{base, profile, plan, unresolved}` for `--save-plan`. | All commands take `--workspace ` (default: current directory); the `forge` commands also take `--forge ` as an alternative to it. @@ -133,7 +133,7 @@ Dispatch reviewers after every commit. Never edit what a reviewer reads. ``` -A marker is a whole line starting at column 0, with single spaces (trailing spaces or tabs are tolerated). Sections do not nest, a name is declared once per ingredient, and a line that looks almost like a marker is an error. An indented marker is plain text — indent it to show the syntax in a rule. The parser is line-based and not Markdown-aware, so a column-0 marker inside a fenced block is still a marker. Markers are read only in files every target renders as text; elsewhere they are copied as they are, with a warning. Agent and command frontmatter lives in `ingredient.yaml` and is never expanded; a rule's or skill's frontmatter is part of its body file, so a column-0 marker there is read like any other. A Forge whose bodies hold a marker must declare `schema: 2` in `craftar.forge.yaml`, so that craftar 0.6.2 and older refuse it instead of emitting the markers; `import` sets it when it relies on markers. +A marker is a whole line starting at column 0, with single spaces (trailing spaces or tabs are tolerated). Sections do not nest, a name is declared once per ingredient, and a line that looks almost like a marker is an error. An indented marker is plain text — indent it to show the syntax in a rule. The parser is line-based and not Markdown-aware, so a column-0 marker inside a fenced block is still a marker. Markers are read only in files every target renders as text; elsewhere they are copied as they are, with a warning. Agent and command frontmatter lives in `ingredient.yaml` and is never expanded; a rule's or skill's frontmatter is part of its body file, so a column-0 marker there is read like any other. A Forge whose bodies hold a marker must declare `schema: 2` in `craftar.forge.yaml`, so that craftar 0.6.2 and older refuse it instead of emitting the markers; `import` sets it when it relies on markers. A section can also be extracted from a variant with `forge unify` (`take: section`), not only added by hand and re-imported. Profile `profile.yaml`, with section values keyed by `/` (the output name: a variant's values are its base's), then by section name: @@ -240,6 +240,12 @@ Next: `craftar init` from a profile; profile-driven integrations (PM tool → MC ## Upgrading +### to 0.8.0 + +- **`forge unify` can write `sections` into a profile and set `schema: 2` in `craftar.forge.yaml`.** craftar 0.6.2 and older then refuse that Forge, as after an import that relies on markers. +- **A plan that uses `take: section` is refused by craftar 0.7.x**, which does not know the value; a plan saved by 0.8.0 with every decision still `keep` loads in 0.7.x. +- **U1's advice now ends `take base for the marker lines, or take: section to fill the section`.** + ### to 0.7.0 - **A Forge with section markers declares `schema: 2`.** 0.7.0 refuses to sync a workspace that resolves a marked body while `craftar.forge.yaml` says `schema: 1` (or nothing), and `import` sets `schema: 2` itself when it writes a section value or compares a workspace file with a marked base. craftar 0.6.2 and older refuse a `schema: 2` Forge at load, so upgrade every workspace that uses the Forge before its first marker, and when you add markers by hand, set `schema: 2` in the same commit — an older CLI syncing a `schema: 1` Forge with markers writes the marker lines into every target. diff --git a/package-lock.json b/package-lock.json index f0f6a1e..3f9c73a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "craftar", - "version": "0.7.1", + "version": "0.8.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "craftar", - "version": "0.7.1", + "version": "0.8.0", "license": "MIT", "dependencies": { "commander": "^13.1.0", diff --git a/package.json b/package.json index 19b1e77..c4ef842 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "craftar", - "version": "0.7.1", + "version": "0.8.0", "description": "Craft, sync and convert AI-coding workspace harnesses across clients and tools.", "license": "MIT", "type": "module", diff --git a/src/cli.ts b/src/cli.ts index 4a20640..c0c0216 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -5,6 +5,7 @@ import { promises as fs } from "node:fs"; import YAML from "yaml"; import { importClaudeCode } from "./importers/claude-code.js"; import { loadWorkspace, plan, readLock, status, apply, resolveForge, type FileStatus, type SectionLayer } from "./core/sync.js"; +import { sectionKey } from "./core/resolve.js"; import { canonicalValue } from "./core/sections.js"; import { renderDiff, NO_EOF_NEWLINE_MARKER } from "./core/diff.js"; import { diffIngredients, listVariants, profileOf, type Distance, type IngredientDiff } from "./core/variants.js"; @@ -27,7 +28,7 @@ import { HUNK_CLASSES, UnifyPlanSchema, type HunkClass, type HunkSuggestion, typ process.stdout.on("error", (e: NodeJS.ErrnoException) => { if (e.code === "EPIPE") process.exit(0); }); const program = new Command(); -program.name("craftar").description("Craft, sync and convert AI-coding workspace harnesses.").version("0.7.1"); +program.name("craftar").description("Craft, sync and convert AI-coding workspace harnesses.").version("0.8.0"); /* ---------------------------------------------------------------- import */ program @@ -267,12 +268,12 @@ forge forge .command("unify") - .description("Resolve one variant back into its base through a reviewable plan, taking each hunk from a side or turning it into a {{param}}. Writes to the Forge") + .description("Resolve one variant back into its base through a reviewable plan, taking each hunk from a side or turning it into a {{param}} or a section. Writes to the Forge") .argument("", "base ingredient (rule/workflow)") .requiredOption("--profile

", "which variant to resolve") .option("--take ", "resolve every decision to base or variant") - .option("--plan ", "apply the decisions in this plan file (a hunk may be take: param with its params list)") - .option("--save-plan ", "write a plan with every decision deferred, each hunk annotated with its suggested class (which --plan ignores) and value hunks pre-filled with params, to a new file outside the Forge, and stop") + .option("--plan ", "apply the decisions in this plan file (a hunk may be take: param with its params list, or take: section with its section name)") + .option("--save-plan ", "write a plan with every decision deferred, each hunk annotated with its suggested class (which --plan ignores), value hunks pre-filled with params, block hunks pre-filled with a section name, and hunks touching an existing section with its name, to a new file outside the Forge, and stop") .option("--forge

", "Forge directory (instead of --workspace)") .option("--workspace ", "workspace whose craftar.yaml names the Forge (default: .)") .option("--json", "machine-readable output", false) @@ -368,12 +369,21 @@ forge } } - const result = await applyPlan(base, variant, diff, toApply, { discardVariantMeta: o.take === "base" }); + // Get the profile's current sections for the base's key (spec 12 §6.5). + const baseKey = sectionKey(base.meta); + const profileSections = (() => { + const p = f.profiles.get(o.profile); + return p && Object.hasOwn(p.sections, baseKey) ? p.sections[baseKey] : {}; + })(); + + const result = await applyPlan(base, variant, diff, toApply, { discardVariantMeta: o.take === "base", profileSections }); // Ruling 33, dry pass: every recipe `ingredients` rewrite the cascade will make is checked before // the first byte is written, so a refusal (an aliased reference) leaves the Forge untouched. const cascadeFiles = result.resolved ? await checkRecipeCascade(f, base.ref, variant.ref) : []; - // Spec 09: the Forge-level rows and both YAML edits of a parameter extraction, rendered before any write. - const paramWrites = result.params.length ? await checkParamWrites(f, base, variant, o.profile, result.params) : null; + // Spec 09 & 12: the Forge-level rows and YAML edits for parameter and section extractions, rendered before any write. + const paramWrites = result.params.length || result.sections.length + ? await checkParamWrites(f, base, variant, o.profile, result.params, result.sections) + : null; // Ruling 37: "git is the undo" only holds for files git actually has. The whole-repo clean // check above cannot see ignored files (a Forge its enclosing repo ignores, an ignored file in @@ -391,12 +401,15 @@ forge // Order: merged files, then the recipe cascade, then removal of the variant directory // (Ruling 21) — a late failure leaves the variant in place, never a recipe naming a removed ingredient. + // Spec 12 §6.7: the manifest is first when it moves to schema: 2; each prefix is emission-neutral. const journal: WriteJournal = []; let touched: string[] = []; let cascade: RecipeCascadeResult = { rewritten: [], identicalToSibling: [] }; let variantRemoved: string | null = null; try { - // Spec 09 §6.5: declarations, then the template, then the profile's values — each prefix emission-neutral. + // Spec 12 §6.7 step 1: the manifest (schema: 2), FIRST when it needs to move. + if (paramWrites?.manifest) await writeParamFile(paramWrites.manifest, journal); + // Spec 09 §6.5 / Spec 12 §6.7 step 2: declarations, then the template, then the profile's values — each prefix emission-neutral. if (paramWrites?.ingredientYaml) await writeParamFile(paramWrites.ingredientYaml, journal); touched = await writeUnified(base, result, journal); if (paramWrites?.ingredientYaml) touched = [...touched, "ingredient.yaml"].sort(); @@ -442,6 +455,13 @@ forge `craftar.local.yaml) now overrides ${base.ref} too; unify cannot reach workspaces`, ); } + // Spec 12 W3: a new section's name may be cited by a workspace's overrides.sections that was inert until now. + for (const s of result.sections.filter((sec) => !sec.existing)) { + warnings.push( + `${s.name} is now a section of ${base.ref} — a workspace that sets overrides.sections.${s.key}.${s.name} (craftar.yaml or ` + + `craftar.local.yaml) now applies there; unify cannot reach workspaces`, + ); + } if (variantRemoved) { warnings.push( `${variant.ref} was removed — a workspace that disables it in overrides.ingredients.disable (craftar.yaml or craftar.local.yaml) ` + @@ -469,6 +489,17 @@ forge metaDiffers: result.metaDiffers, // Spec 09 §4.5: only what this run wrote — [] when every key was already declared and valued. params: result.params.filter((e) => paramWrites?.written.includes(e.key)).map((e) => ({ key: e.key, default: e.default, value: e.value })), + // Spec 12 §4.5: sections with line counts; [] when no section hunk. + sections: result.sections.map((s) => ({ + key: s.key, + name: s.name, + file: s.file, + existing: s.existing, + defaultLines: s.default === null ? null : numLines(s.default), + valueLines: numLines(s.value), + written: paramWrites?.sectionsWritten.includes(s.name) ?? false, + })), + manifestEdited: paramWrites?.manifest !== null && paramWrites?.manifest !== undefined, profileEdited: paramWrites?.profile ? path.relative(f.root, paramWrites.profile.abs).split(path.sep).join("/") : null, warnings, }, @@ -479,12 +510,21 @@ forge } console.log(pc.bold(`craftar forge unify ${ref} ↔ ${o.profile}`)); + // Spec 12 §4.4: manifest line first, if edited + if (paramWrites?.manifest) console.log(` forge craftar.forge.yaml edited (schema: 2)`); for (const p of touched) console.log(` ${result.write[p] !== undefined || p === "ingredient.yaml" ? pc.green("~") : pc.magenta("-")} ${p}`); for (const e of result.params) { const line = `param ${e.key} — default ${JSON.stringify(e.default)} (${base.ref}) · ${JSON.stringify(e.value)} (profile ${o.profile})`; // Same filter as --json params: a key already declared and valued is named as such, not as written. console.log(` ${line}${paramWrites?.written.includes(e.key) ? "" : " — already in place"}`); } + // Spec 12 §4.4: one line per section, after the params + for (const s of result.sections) { + const defCount = s.existing ? "existing" : `default ${lineCount(s.default!)} (${base.ref})`; + const valCount = `${lineCount(s.value)} (profile ${o.profile})`; + const inPlace = paramWrites?.sectionsWritten.includes(s.name) ? "" : " — already in place"; + console.log(` section ${s.key} ${s.name} — ${defCount} · ${valCount}${inPlace}`); + } if (paramWrites?.profile) console.log(` ${pc.green("~")} ${path.relative(f.root, paramWrites.profile.abs).split(path.sep).join("/")}`); console.log(` resolved ${result.resolved ? pc.green("yes") : pc.yellow("no")} · unresolved ${result.unresolved}`); if (variantRemoved) console.log(` ${pc.magenta("removed variant")} ${variantRemoved}`); @@ -653,10 +693,15 @@ function describeSuggestion(s: HunkSuggestion): string { /** A section value as the import report shows it: `empty`, or its line count once canonical (spec 11 §4.3). */ function lineCount(value: string): string { - const n = canonicalValue(value).split("\n").length - 1; + const n = numLines(value); return n === 0 ? "empty" : `${n} line${n === 1 ? "" : "s"}`; } +/** Numeric line count of a canonical section value, for --json output. */ +function numLines(value: string): number { + return canonicalValue(value).split("\n").length - 1; +} + function describeDistance(d: Distance): string { if (d.identicalAfterNormalization) return "identical after normalization"; if (d.sameBodyDifferentMeta) return "meta only"; diff --git a/src/core/manifest-edit.ts b/src/core/manifest-edit.ts new file mode 100644 index 0000000..ca52c9d --- /dev/null +++ b/src/core/manifest-edit.ts @@ -0,0 +1,45 @@ +import { isDeepStrictEqual } from "node:util"; +import YAML from "yaml"; +import { FORGE_SCHEMA_SECTIONS, ForgeManifestSchema } from "../schema/index.js"; +import { stripBom } from "./text.js"; +import { editYamlText } from "./yaml-edit.js"; + +/** + * `craftar.forge.yaml` with `schema: 2` (spec 11 §6.14; spec 12 §6.7), edited in place through the + * round-trip gate, or null when it already declares 2. Throws `: cannot edit craftar.forge.yaml + * in place () — set schema: 2 by hand, commit, and re-run`. + */ +export function manifestWithSections(raw: string, command: "import" | "unify"): string | null { + const refuse = (why: string) => + new Error(`${command}: cannot edit craftar.forge.yaml in place (${why}) — set schema: ${FORGE_SCHEMA_SECTIONS} by hand, commit, and re-run`); + + let before: unknown; + try { + before = YAML.parse(stripBom(raw)); + } catch (e) { + throw refuse(`it does not parse: ${(e as Error).message}`); + } + + // Unreachable while forgeBefore/loadForge loads the manifest through its schema first; kept so the edit never assumes it. + if (before === null || typeof before !== "object" || Array.isArray(before)) { + throw refuse("it is not a YAML mapping"); + } + + if ((before as { schema?: unknown }).schema === FORGE_SCHEMA_SECTIONS) { + return null; + } + + let content: string; + try { + content = editYamlText(raw, { command, label: "craftar.forge.yaml", keys: ["schema"] }, (doc) => doc.set("schema", FORGE_SCHEMA_SECTIONS)); + } catch (e) { + throw refuse((e as Error).message.replace(/^.*in place \((.*)\) — .*$/s, "$1")); + } + + const after = YAML.parse(stripBom(content)); + if (!ForgeManifestSchema.safeParse(after).success || !isDeepStrictEqual(after, { ...before, schema: FORGE_SCHEMA_SECTIONS })) { + throw refuse(`the edit does not read back as the original with exactly schema: ${FORGE_SCHEMA_SECTIONS}`); + } + + return content; +} diff --git a/src/core/param-writes.ts b/src/core/param-writes.ts index 4604bd9..928bfc8 100644 --- a/src/core/param-writes.ts +++ b/src/core/param-writes.ts @@ -4,11 +4,14 @@ import { isDeepStrictEqual } from "node:util"; import YAML from "yaml"; import { IngredientSchema, ProfileSchema, WorkspaceConfigSchema } from "../schema/index.js"; import { placeholders, substitutedFile, type Extraction } from "./extract.js"; -import { exists, listFiles, readIngredientText, type Forge, type LoadedIngredient } from "./forge.js"; +import { exists, listFiles, readIngredientText, FORGE_MANIFEST, type Forge, type LoadedIngredient } from "./forge.js"; +import { manifestWithSections } from "./manifest-edit.js"; import { resolve, sectionKey } from "./resolve.js"; +import { canonicalValue } from "./sections.js"; import { stripBom } from "./text.js"; +import type { SectionExtraction, WriteJournal } from "./unify.js"; import { editYamlText } from "./yaml-edit.js"; -import type { WriteJournal } from "./unify.js"; +import { deepMerge } from "./merge.js"; /** * The Forge-level half of a parameter extraction (spec 09 §6.3–§6.5): the rows only the whole @@ -22,10 +25,14 @@ export interface ParamWrites { ingredientYaml: { abs: string; content: string } | null; /** Profile `

`'s `profile.yaml`, rendered, or null when it already holds every value. */ profile: { abs: string; content: string } | null; + /** `craftar.forge.yaml` with `schema: 2`, or null (already 2, or no new section). */ + manifest: { abs: string; content: string } | null; /** Paths git must hold before anything is written (spec 06 row 17). */ mustHold: string[]; /** The keys this run declares in `ingredient.yaml` or sets in the profile; the others were already in place. */ written: string[]; + /** The section names this run writes into the profile (not listed when already in place). */ + sectionsWritten: string[]; } @@ -82,10 +89,8 @@ function citingSection(profile: { sections: Record a !== undefined && String(a) === b; -/** The spec 09 edit of one Forge file, through the shared in-place YAML editor. */ -async function editYaml(abs: string, label: string, edit: (doc: YAML.Document) => void): Promise { - return editYamlText(await fs.readFile(abs, "utf8"), { command: "unify", label, keys: ["params"] }, edit); -} +/** Equality of canonical section values (spec 12 §6.6). */ +const eq = (a: string, b: string) => canonicalValue(a) === canonicalValue(b); export async function checkParamWrites( forge: Forge, @@ -93,6 +98,7 @@ export async function checkParamWrites( variant: LoadedIngredient, profile: string, extractions: Extraction[], + sections: SectionExtraction[] = [], ): Promise { const profileAbs = await findProfileFile(forge.root, profile); const current = forge.profiles.get(profile); @@ -158,11 +164,56 @@ export async function checkParamWrites( assign.push(e); } + // Section checks (spec 12 §6.6): S13–S16 + const sectionAssign: SectionExtraction[] = []; + for (const s of sections) { + // S15: check that a value citing {{k}} where base declares k with a default and variant doesn't is refused + for (const k of placeholders(s.value)) { + const baseDecl = base.meta.params?.[k]?.default; + const variantDecl = variant.meta.params?.[k]; + if (baseDecl !== undefined && variantDecl === undefined) { + throw new Error(`unify: section ${s.name} would render {{${k}}} through ${base.ref}'s default, where ${variant.ref} renders it without`); + } + } + + // Get the profile's current value for this section (if any) + const profileSections = Object.hasOwn(current.sections, s.key) ? current.sections[s.key] : {}; + const profileValue = Object.hasOwn(profileSections, s.name) ? profileSections[s.name] : undefined; + + if (!s.existing) { + // New section: check S13 — no other profile sets this section + for (const [q, prof] of forge.profiles) { + const otherSections = Object.hasOwn(prof.sections, s.key) ? prof.sections[s.key] : {}; + if (!Object.hasOwn(otherSections, s.name)) continue; + const v = otherSections[s.name]; + if (q === profile) { + // The unifying profile: if equal, nothing to write; if different, S14 + if (eq(v, s.value)) continue; // already in place, skip assign + throw new Error(`unify: profile ${profile} already sets section ${s.name} of ${s.key} to other content`); + } else { + // Another profile: S13 + throw new Error(`unify: profile ${q} already sets section ${s.name} of ${s.key} — it names no marker today and would start to apply`); + } + } + // If we get here and the profile already has exactly the value, skip assign + if (profileValue !== undefined && eq(profileValue, s.value)) continue; + sectionAssign.push(s); + } else { + // Existing section: only check the unifying profile + if (profileValue !== undefined) { + if (eq(profileValue, s.value)) continue; // already in place + throw new Error(`unify: profile ${profile} already sets section ${s.name} of ${s.key} to other content`); + } + // Undefined → write + sectionAssign.push(s); + } + } + let ingredientYaml: ParamWrites["ingredientYaml"] = null; if (declare.length) { const abs = path.join(base.dir, "ingredient.yaml"); const label = path.relative(forge.root, abs).split(path.sep).join("/"); - const content = await editYaml(abs, label, (doc) => { + const content = await editYamlText(await fs.readFile(abs, "utf8"), { command: "unify", label, keys: ["params"] }, (doc) => { for (const e of declare) doc.setIn(["params", e.key, "default"], e.default); }); const before = IngredientSchema.parse(YAML.parse(await fs.readFile(abs, "utf8"))); @@ -174,23 +225,71 @@ export async function checkParamWrites( ingredientYaml = { abs, content }; } + // Profile edit: ONE editYamlText call with both params and sections (spec 12 §6.7 step 4) let profileWrite: ParamWrites["profile"] = null; - if (assign.length) { + if (assign.length || sectionAssign.length) { const label = path.relative(forge.root, profileAbs).split(path.sep).join("/"); - const content = await editYaml(profileAbs, label, (doc) => { + const content = await editYamlText(await fs.readFile(profileAbs, "utf8"), { command: "unify", label, keys: ["params", "sections"] }, (doc) => { for (const e of assign) doc.setIn(["params", e.key], e.value); + for (const s of sectionAssign) doc.setIn(["sections", s.key, s.name], s.value); }); const before = ProfileSchema.parse(YAML.parse(await fs.readFile(profileAbs, "utf8"))); const after = parseOr(label, () => ProfileSchema.parse(YAML.parse(stripBom(content)))); - const expected = { ...before, params: { ...before.params, ...Object.fromEntries(assign.map((e) => [e.key, e.value])) } }; - if (!isDeepStrictEqual(after, expected) || assign.some((e) => !same(after.params[e.key], e.value))) { + // Build expected sections: deep-merge the before sections with the new ones + const expectedSections = deepMerge( + before.sections, + Object.fromEntries(sectionAssign.map((s) => [s.key, { [s.name]: s.value }])) + ); + const expected = { + ...before, + params: { ...before.params, ...Object.fromEntries(assign.map((e) => [e.key, e.value])) }, + sections: expectedSections, + }; + if (!isDeepStrictEqual(after, expected)) { + throw new Error(`unify: cannot edit ${label} in place (the edit does not read back as exactly the new values)`); + } + // Also verify each value reads back exactly (string equality) + if (assign.some((e) => !same(after.params[e.key], e.value))) { throw new Error(`unify: cannot edit ${label} in place (the edit does not read back as exactly the new values)`); } + // For sections, verify each value reads back exactly + for (const s of sectionAssign) { + const afterVal = after.sections[s.key]?.[s.name]; + if (afterVal !== s.value) { + throw new Error(`unify: cannot edit ${label} in place (the edit does not read back as exactly the new values)`); + } + } profileWrite = { abs: profileAbs, content }; } + // Manifest edit (spec 12 §6.7 step 1): when at least one section is NEW and schema is 1 + // Use the full `sections` list, not `sectionAssign`: a new section adds markers to the body + // even when its value is already in place in the profile, so the Forge needs schema: 2. + let manifestWrite: ParamWrites["manifest"] = null; + const hasNewSection = sections.some((s) => !s.existing); + if (hasNewSection && forge.manifest.schema === 1) { + const manifestAbs = path.join(forge.root, FORGE_MANIFEST); + const raw = await fs.readFile(manifestAbs, "utf8"); + const content = manifestWithSections(raw, "unify"); + if (content !== null) { + manifestWrite = { abs: manifestAbs, content }; + } + } + const written = new Set([...declare, ...assign].map((e) => e.key)); - return { ingredientYaml, profile: profileWrite, mustHold: profileWrite ? [profileWrite.abs] : [], written: extractions.map((e) => e.key).filter((k) => written.has(k)) }; + const sectionsWritten = sectionAssign.map((s) => s.name); + const mustHold: string[] = []; + if (profileWrite) mustHold.push(profileWrite.abs); + if (manifestWrite) mustHold.push(manifestWrite.abs); + + return { + ingredientYaml, + profile: profileWrite, + manifest: manifestWrite, + mustHold, + written: extractions.map((e) => e.key).filter((k) => written.has(k)), + sectionsWritten, + }; } function parseOr(label: string, parse: () => T): T { diff --git a/src/core/section-extract.ts b/src/core/section-extract.ts new file mode 100644 index 0000000..83622ee --- /dev/null +++ b/src/core/section-extract.ts @@ -0,0 +1,886 @@ +import type { Hunk } from "./diff.js"; +import { SECTION_NAME, type PlanHunk } from "../schema/index.js"; +import { splitLines } from "./diff.js"; +import { expandSections, firstMarkerLine, parseSections, SectionMarkerError, type ParsedSections } from "./sections.js"; +import { stripBom, toLf } from "./text.js"; +import type { Extraction } from "./extract.js"; +import { placeholders, substituteKeys } from "./extract.js"; + +/** + * Section extraction (spec 12): turn a `take: section` run into a section whose markers are added + * to the base and whose value goes into the profile. Pure — the Forge-level gates and the YAML + * edits live elsewhere. + */ + +/** A section's span: opener and closer line numbers (1-based, both inclusive). */ +interface SectionSpan { + name: string; + opener: number; + closer: number; +} + +/** + * Compute the opener/closer span for each section of a parsed file (spec 12 §4.2). + * `closer = opener + s.default.split("\n").length` — the opener line, plus one line per `\n` in the default. + */ +function sectionSpans(parsed: ParsedSections): SectionSpan[] { + return parsed.sections.map((s) => ({ + name: s.name, + opener: s.line, + closer: s.line + s.default.split("\n").length, + })); +} + +/** + * Does a hunk "touch" a section span? (spec 12 §4.2's definition) + * - With base lines: at least one line j in [start, start + length - 1] satisfies opener <= j <= closer. + * - Without base lines: position N = h.a.start - 1 satisfies opener <= N < closer. + */ +function touches(h: { a: { start: number; lines: string[] } }, span: SectionSpan): boolean { + const { opener: o, closer: c } = span; + if (h.a.lines.length > 0) { + for (let j = h.a.start; j < h.a.start + h.a.lines.length; j++) { + if (o <= j && j <= c) return true; + } + return false; + } else { + const N = h.a.start - 1; + return o <= N && N < c; + } +} + +/** A section a plan run creates or fills (spec 12 §3). Lines are base line numbers, 1-based. */ +export interface SectionRun { + name: string; + file: string; // base-relative, as the plan names it + hunks: number[]; // the run's 1-based hunk numbers, ascending + existing: boolean; + /** True when the plan gave explicit `lines`, false when the span was computed from hunks. */ + linesSpecified: boolean; + /** Inclusive base line range; for an empty span, `from === to + 1` (the markers go between line `to` and line `from`). */ + from: number; + to: number; + default: string | null; // null for an existing section; "" or ends with "\n" + value: string; // "" or ends with "\n" +} + +/** Marker lines to insert, by base line index (0-based: "before base line index `at`", `at === A.lines.length` = at the end), in order. */ +export interface MarkerInsertion { + at: number; + line: string; +} + +/** Parse a "from-to" range string into two numbers. */ +function parseLines(s: string): [number, number] { + const [a, b] = s.split("-").map(Number); + return [a, b]; +} + +/** The position a no-base-line hunk inserts at: h.a.start - 1 (spec 12 §6.1, step 3). */ +function pureAdditionPos(h: Hunk): number { + return h.a.start - 1; +} + +/** + * S5 error message: wording differs based on whether the plan gave explicit `lines`. + * When the plan gave `lines`, the user meant to override the computed span, so we say + * "lines of section ". Otherwise the span was computed from hunks, so we say + * "section (lines )". + */ +function s5CoverageError(name: string, rangeStr: string, hunkNum: number, take: string, linesSpecified: boolean): Error { + if (linesSpecified) { + return new Error(`unify plan: lines ${rangeStr} of section ${name} covers hunk ${hunkNum}, which is take: ${take}`); + } else { + return new Error(`unify plan: section ${name} (lines ${rangeStr}) covers hunk ${hunkNum}, which is take: ${take}`); + } +} + +/** + * Compute the section runs, their spans, defaults and values from a file's diff and plan entries. + * Throws with spec 12 §4.6 messages (S1–S10, S12) on any contradiction. + */ +export function deriveSections(args: { + file: string; // base-relative file name, used in messages + label: string; // Forge-relative path for parseSections messages + ref: string; // the base ref, for parseSections' duplicate message + baseText: string; + variantText: string; + hunks: Hunk[]; // the file's hunks, from diffIngredients (1-based numbering = index + 1) + entries: PlanHunk[]; // the plan's hunk entries for this file (any take) + declaredElsewhere: Map; // section names declared in the base's OTHER admitted files → "file:line" +}): { runs: SectionRun[]; markers: MarkerInsertion[] } { + const { file, label, ref, baseText, variantText, hunks, entries, declaredElsewhere } = args; + const A = splitLines(baseText); + const B = splitLines(variantText); + + // Step 1: Variant markers (S12) + const variantMarkerLine = firstMarkerLine(toLf(stripBom(variantText))); + if (variantMarkerLine !== null) { + throw new Error( + `unify: variant holds a section marker on ${file}:${variantMarkerLine} — remove it by hand and save the plan again, or re-import the workspace`, + ); + } + + // Step 2: Base sections + const parsed = parseSections(baseText, label, ref); + const baseSections = sectionSpans(parsed); + + // Step 4: Group section entries + const sectionEntries = entries.filter((e) => e.take === "section"); + const byName = new Map(); + + for (const e of sectionEntries) { + if (!e.section) { + throw new Error(`unify plan: "${file}" hunk ${e.hunk} is take: section but names no section`); + } + const name = e.section.name; + let group = byName.get(name); + if (!group) { + group = { hunkNums: [], linesRanges: [] }; + byName.set(name, group); + } + group.hunkNums.push(e.hunk); + if (e.section.lines) group.linesRanges.push(e.section.lines); + } + + // Validate groups: consecutive hunks, same lines range + for (const [name, group] of byName) { + group.hunkNums.sort((a, b) => a - b); + // Check consecutive + for (let i = 1; i < group.hunkNums.length; i++) { + if (group.hunkNums[i] !== group.hunkNums[i - 1] + 1) { + throw new Error(`unify plan: section ${name} is not one run of consecutive hunks (hunks ${group.hunkNums[i - 1]}, ${group.hunkNums[i]})`); + } + } + // Check same lines range (S4) + const uniqueRanges = [...new Set(group.linesRanges)]; + if (uniqueRanges.length > 1) { + throw new Error(`unify plan: section ${name} has two ranges: ${uniqueRanges[0]}, ${uniqueRanges[1]}`); + } + } + + // Step 5: Existing or new + const runs: SectionRun[] = []; + const runByName = new Map(); + + for (const [name, group] of byName) { + const runHunks = group.hunkNums.map((n) => hunks[n - 1]); + const linesStr = group.linesRanges[0]; // all same or none + + // Find base sections touched by any hunk of this run + const touchedSections = new Set<(typeof baseSections)[number]>(); + for (const h of runHunks) { + for (const sec of baseSections) { + if (touches(h, sec)) touchedSections.add(sec); + } + } + + // Classify: existing or new + const touched = [...touchedSections]; + + if (touched.length > 1) { + // More than one section touched + throw new Error(`unify plan: section ${name} would overlap section ${touched[1].name} (${file}:${touched[1].opener})`); + } + + if (touched.length === 1) { + const sec = touched[0]; + if (sec.name !== name) { + // Touched section has different name + throw new Error(`unify plan: section ${name} would overlap section ${sec.name} (${file}:${sec.opener})`); + } + + // Check all run hunks touch this section (S7 for partial touch) + for (const h of runHunks) { + const touchesThis = touches(h, sec); + if (!touchesThis) { + throw new Error(`unify plan: section ${name} would overlap section ${sec.name} (${file}:${sec.opener})`); + } + } + + // EXISTING section + // S8: Every hunk with base lines must have them all within [opener, closer] + for (let i = 0; i < runHunks.length; i++) { + const h = runHunks[i]; + if (h.a.lines.length > 0) { + const first = h.a.start; + const last = h.a.start + h.a.lines.length - 1; + if (first < sec.opener || last > sec.closer) { + throw new Error( + `unify plan: hunk ${group.hunkNums[i]} of "${file}" holds lines inside and outside section ${name} — its variant side cannot be split; resolve it by re-importing the workspace`, + ); + } + } + } + + // S9: If lines given, must be exactly opener-closer + if (linesStr) { + const expected = `${sec.opener}-${sec.closer}`; + if (linesStr !== expected) { + throw new Error(`unify plan: section ${name} already spans lines ${sec.opener}-${sec.closer}; unify does not move existing markers`); + } + } + + const from = sec.opener; + const to = sec.closer; + + // Compute value from variant segment + const value = computeValue(A, B, hunks, from, to, name, file, !!linesStr); + + runs.push({ + name, + file, + hunks: group.hunkNums, + existing: true, + linesSpecified: !!linesStr, + from, + to, + default: null, + value, + }); + } else { + // NEW section + // S6: name must not be declared anywhere + const declaredInThis = baseSections.find((s) => s.name === name); + if (declaredInThis) { + throw new Error(`unify plan: section ${name} is already declared in ${ref} (${file}:${declaredInThis.opener})`); + } + const declaredOther = declaredElsewhere.get(name); + if (declaredOther) { + throw new Error(`unify plan: section ${name} is already declared in ${ref} (${declaredOther})`); + } + + // Compute span + let from: number; + let to: number; + + if (linesStr) { + // With lines + const [a, b] = parseLines(linesStr); + if (a < 1 || b < a || b > A.lines.length) { + throw new Error(`unify plan: lines ${linesStr} of section ${name} is out of range`); + } + // Check all run hunks are within the range + for (let i = 0; i < runHunks.length; i++) { + const h = runHunks[i]; + if (h.a.lines.length > 0) { + const first = h.a.start; + const last = h.a.start + h.a.lines.length - 1; + if (first < a || last > b) { + throw new Error(`unify plan: lines ${linesStr} of section ${name} does not contain hunk ${group.hunkNums[i]}`); + } + } else { + const N = pureAdditionPos(h); + if (N < a - 1 || N > b) { + throw new Error(`unify plan: lines ${linesStr} of section ${name} does not contain hunk ${group.hunkNums[i]}`); + } + } + } + from = a; + to = b; + } else { + // Without lines: spec 12 §6.2 — a pure insertion at position N contributes N+1 for 'from' + // and N for 'to'; Ruling 1 includes equal lines between hunks. + const fromCandidates: number[] = []; + const toCandidates: number[] = []; + for (const h of runHunks) { + if (h.a.lines.length > 0) { + fromCandidates.push(h.a.start); + toCandidates.push(h.a.start + h.a.lines.length - 1); + } else { + const N = pureAdditionPos(h); + fromCandidates.push(N + 1); + toCandidates.push(N); + } + } + + // All pure insertions at one position → empty span (from > to) + const uniqueFrom = new Set(fromCandidates); + const uniqueTo = new Set(toCandidates); + if (uniqueFrom.size === 1 && uniqueTo.size === 1 && Math.min(...fromCandidates) > Math.max(...toCandidates)) { + // Empty span: all hunks are pure insertions at the same position + from = fromCandidates[0]; + to = toCandidates[0]; + } else { + from = Math.min(...fromCandidates); + to = Math.max(...toCandidates); + } + } + + // Check span doesn't contain any existing section's opener or closer (S7) + for (const sec of baseSections) { + if (from <= sec.opener && sec.opener <= to) { + throw new Error(`unify plan: section ${name} would overlap section ${sec.name} (${file}:${sec.opener})`); + } + if (from <= sec.closer && sec.closer <= to) { + throw new Error(`unify plan: section ${name} would overlap section ${sec.name} (${file}:${sec.closer})`); + } + } + + // Compute default and value + const defaultText = computeDefault(A, from, to, name, file); + const value = computeValue(A, B, hunks, from, to, name, file, !!linesStr); + + runs.push({ + name, + file, + hunks: group.hunkNums, + existing: false, + linesSpecified: !!linesStr, + from, + to, + default: defaultText, + value, + }); + } + + runByName.set(name, runs[runs.length - 1]); + } + + // Step 6: Coverage — every hunk not in a run must lie outside all spans + const hunkInRun = new Set(); + for (const run of runs) { + for (const n of run.hunks) hunkInRun.add(n); + } + + for (let i = 0; i < hunks.length; i++) { + const hunkNum = i + 1; + if (hunkInRun.has(hunkNum)) continue; + + const h = hunks[i]; + const entry = entries.find((e) => e.hunk === hunkNum); + const take = entry?.take ?? "keep"; + + for (const run of runs) { + const { from, to, name, linesSpecified } = run; + const isEmptySpan = from === to + 1; + const rangeStr = `${from}-${to}`; + + if (h.a.lines.length > 0) { + // Hunk with base lines: none in [from, to] + for (let j = h.a.start; j < h.a.start + h.a.lines.length; j++) { + if (!isEmptySpan && from <= j && j <= to) { + throw s5CoverageError(name, rangeStr, hunkNum, take, linesSpecified); + } + } + } else { + // No base lines: N must satisfy N < from - 1 or N > to + const N = pureAdditionPos(h); + if (!isEmptySpan && !(N < from - 1 || N > to)) { + throw s5CoverageError(name, rangeStr, hunkNum, take, linesSpecified); + } else if (isEmptySpan && N === to) { + // Empty span: N cannot equal to (the position) + throw s5CoverageError(name, rangeStr, hunkNum, take, linesSpecified); + } + } + } + } + + // Check two runs don't overlap (S7) + // Sort deterministically: by from, then by to, then by name + const sortedRuns = [...runs].sort((a, b) => { + if (a.from !== b.from) return a.from - b.from; + if (a.to !== b.to) return a.to - b.to; + return a.name.localeCompare(b.name); + }); + for (let i = 1; i < sortedRuns.length; i++) { + const prev = sortedRuns[i - 1]; + const curr = sortedRuns[i]; + const prevEmpty = prev.from === prev.to + 1; + const currEmpty = curr.from === curr.to + 1; + + // Non-empty vs non-empty: prev.to must be < curr.from + // An empty span [N+1, N] conflicts with a non-empty neighbour that touches or contains position N, and with another empty span at the same N. + let overlaps = false; + if (!prevEmpty && !currEmpty) { + // Both non-empty: standard check + overlaps = prev.to >= curr.from; + } else if (prevEmpty && !currEmpty) { + // Empty prev: [from=N+1, to=N] touches curr if curr.from === N + 1 + // prev.from is the "after line N" position = N + 1 + overlaps = curr.from === prev.from; + } else if (!prevEmpty && currEmpty) { + // Empty curr: [from=N+1, to=N] is inside or touching prev if prev.to >= N. + // The sort guarantees prev.from <= curr.from, so >= covers both touching and containing. + overlaps = prev.to >= curr.to; + } else { + // Both empty: [prevFrom=M+1, prevTo=M] and [currFrom=N+1, currTo=N] + // They overlap only if M === N (same position, i.e. both insert after the same line). + // Adjacent positions (M+1 === N, i.e. one base line apart) do NOT overlap. + overlaps = prev.to === curr.to; + } + + if (overlaps) { + throw new Error(`unify plan: section ${curr.name} would overlap section ${prev.name} (${file}:${prev.from})`); + } + } + + // Step 9: Generate markers for new sections + // Track section name with each marker for proper sorting + const markersWithMeta: Array = []; + for (const run of runs) { + if (run.existing) continue; + + const opener = ``; + const closer = ``; + + // For an empty span (from === to + 1), both markers go at position `to` (which equals from - 1) + // opener at from - 1 (0-based: from - 1), closer at to (0-based: to) + // For non-empty: opener before line `from` (at = from - 1), closer after line `to` (at = to) + const openerAt = run.from - 1; + const closerAt = run.to; + + markersWithMeta.push({ at: openerAt, line: opener, name: run.name, isOpener: true }); + markersWithMeta.push({ at: closerAt, line: closer, name: run.name, isOpener: false }); + } + + // Sort markers: by `at`, then by type (closer before opener for different sections, + // opener before closer for same section), then by name for stability. + // + // This is a total order because S7 refuses every empty span sharing a position with + // another span's marker. After S7 passes, at any `at` value we have either: + // - One section's opener and closer (empty span) → opener before closer + // - Different sections' openers or closers → closer before opener, then by name + // No two empty spans share an `at`, and no empty span shares `at` with another marker. + markersWithMeta.sort((a, b) => { + if (a.at !== b.at) return a.at - b.at; + + if (a.name === b.name) { + // Same section: opener before closer + return a.isOpener ? -1 : 1; + } + + // Different sections at same position: closer before opener + if (a.isOpener !== b.isOpener) { + return a.isOpener ? 1 : -1; + } + + // Both same type (both openers or both closers), different names: sort by name + return a.name.localeCompare(b.name); + }); + + const markers: MarkerInsertion[] = markersWithMeta.map(({ at, line }) => ({ at, line })); + + return { runs: sortedRuns, markers }; +} + +/** + * Compute the default text for a NEW section (base lines from..to). + * S10: check for missing final newline. + */ +function computeDefault(A: ReturnType, from: number, to: number, name: string, file: string): string { + const isEmptySpan = from === to + 1; + + if (isEmptySpan) { + // Empty span: check S10 for empty span at end of file + if (to === A.lines.length && !A.eofNewline && A.lines.length > 0) { + throw new Error(`unify plan: section ${name} would reach a missing final newline in the base, which a section cannot reproduce`); + } + return ""; + } + + // Non-empty span + if (to === A.lines.length && !A.eofNewline) { + throw new Error(`unify plan: section ${name} would reach a missing final newline in the base, which a section cannot reproduce`); + } + + const lines = A.lines.slice(from - 1, to); + const text = lines.map((l) => l + "\n").join(""); + + // S12: check for marker in default + if (firstMarkerLine(text) !== null) { + throw new Error(`unify plan: section ${name} would hold a section marker`); + } + + return text; +} + +/** + * Compute the value for a section (variant lines between anchors). + * S10: check for missing final newline. + * S5: anchors must be equal lines (not inside any hunk). + * @param linesSpecified true when the plan gave explicit `lines`, false when span was computed from hunks + */ +function computeValue( + A: ReturnType, + B: ReturnType, + hunks: Hunk[], + from: number, + to: number, + name: string, + file: string, + linesSpecified: boolean, +): string { + const isEmptySpan = from === to + 1; + + // Step 7: Anchors and mapping + // Anchors are base lines from - 1 and to + 1 (absent when < 1 or > A.lines.length) + const startAnchor = from - 1 >= 1 ? from - 1 : null; + const endAnchor = to + 1 <= A.lines.length ? to + 1 : null; + + // Step 7 assert: anchors must lie in no hunk (they must be equal lines) + // If an anchor is inside a hunk's base lines, the value would be read from wrong variant lines + const lineInHunk = (line: number): number | null => { + for (let i = 0; i < hunks.length; i++) { + const h = hunks[i]; + if (h.a.lines.length > 0) { + const first = h.a.start; + const last = h.a.start + h.a.lines.length - 1; + if (first <= line && line <= last) { + return i + 1; // 1-based hunk number + } + } + } + return null; + }; + + if (startAnchor !== null) { + const hunkNum = lineInHunk(startAnchor); + if (hunkNum !== null) { + // S5: wording differs based on whether the plan gave explicit `lines` + if (linesSpecified) { + throw new Error(`unify plan: lines ${from}-${to} of section ${name} cuts hunk ${hunkNum}`); + } else { + throw new Error(`unify plan: section ${name} (lines ${from}-${to}) cuts hunk ${hunkNum}`); + } + } + } + + if (endAnchor !== null) { + const hunkNum = lineInHunk(endAnchor); + if (hunkNum !== null) { + // S5: wording differs based on whether the plan gave explicit `lines` + if (linesSpecified) { + throw new Error(`unify plan: lines ${from}-${to} of section ${name} cuts hunk ${hunkNum}`); + } else { + throw new Error(`unify plan: section ${name} (lines ${from}-${to}) cuts hunk ${hunkNum}`); + } + } + } + + // Map anchor line i to variant: i' = i + Σ(h.b.lines.length - h.a.lines.length) for hunks before i + const mapToVariant = (i: number): number => { + let offset = 0; + for (const h of hunks) { + // A hunk is "before" line i if its base lines end before i + // h.a.start + h.a.lines.length <= i covers both cases + if (h.a.start + h.a.lines.length <= i) { + offset += h.b.lines.length - h.a.lines.length; + } + } + return i + offset; + }; + + // Variant segment bounds + let variantStart: number; + let variantEnd: number; + + if (startAnchor !== null) { + variantStart = mapToVariant(startAnchor) + 1; // line after the anchor's counterpart + } else { + variantStart = 1; + } + + if (endAnchor !== null) { + variantEnd = mapToVariant(endAnchor) - 1; // line before the anchor's counterpart + } else { + variantEnd = B.lines.length; + } + + // Empty variant segment + if (variantStart > variantEnd) { + return ""; + } + + // S10: check for missing final newline in variant + if (variantEnd === B.lines.length && !B.eofNewline) { + throw new Error(`unify plan: section ${name} would reach a missing final newline in the variant, which a section cannot reproduce`); + } + + const lines = B.lines.slice(variantStart - 1, variantEnd); + const value = lines.map((l) => l + "\n").join(""); + + // S12: check for marker in value + if (firstMarkerLine(value) !== null) { + throw new Error(`unify plan: section ${name} would hold a section marker`); + } + + return value; +} + +/** + * The section half of the equivalence proof (spec 12 §6.5), for one file that has at least one + * section run. It also carries the plan's param keys K (spec 09 §6.2), because a file can hold both: + * for such a file this replaces `prove`. Throws S17 naming the file. + */ +export function proveSections(args: { + file: string; // base-relative, for messages + label: string; // Forge-relative, for parseSections + ref: string; + template: string; // T: section hunks as base lines, param hunks as templates, new markers inserted + mBase: string; // every param and section hunk taken base, others as the plan says + mVar: string; // every param and section hunk taken variant, others as the plan says + newNames: string[]; // the names of the NEW runs in this file + values: Record; // σ: run name → value, for every run (new and existing) in this file + profileValues: Record; // S_p: profile

's current sections for the base's key ({} when none) + extractions: Extraction[]; // the plan's param keys (may be empty) +}): void { + const { file, label, ref, template, mBase, mVar, newNames, values, profileValues, extractions } = args; + const n = (x: string) => toLf(stripBom(x)); + + // Build D and V from extractions + const D = new Map(extractions.map((e) => [e.key, e.default])); + const V = new Map(extractions.map((e) => [e.key, e.value])); + const K = new Set(extractions.map((e) => e.key)); + + const names = (p: ParsedSections) => p.sections.map((s) => s.name); + + // Step 1: Parse all three + let pT: ParsedSections; + let pB: ParsedSections; + let pV: ParsedSections; + + const runNames = Object.keys(values).join(", ") || newNames.join(", "); + + try { + pT = parseSections(template, label, ref); + } catch (e) { + if (e instanceof SectionMarkerError) { + throw new Error(`unify: section ${runNames} would not reproduce the base side of "${file}" (${e.problem})`); + } + throw e; + } + + try { + pB = parseSections(mBase, label, ref); + } catch (e) { + if (e instanceof SectionMarkerError) { + throw new Error(`unify: section ${runNames} would not reproduce the base side of "${file}" (${e.problem})`); + } + throw e; + } + + try { + pV = parseSections(mVar, label, ref); + } catch (e) { + if (e instanceof SectionMarkerError) { + throw new Error(`unify: section ${runNames} would not reproduce the variant side of "${file}" (${e.problem})`); + } + throw e; + } + + // Step 2: Structure check + // names(pT) with every name of newNames removed must equal names(pB) + const tNames = names(pT); + const bNames = names(pB); + const newNamesSet = new Set(newNames); + + // Check every newNames entry appears exactly once in tNames + for (const name of newNames) { + const count = tNames.filter((n) => n === name).length; + if (count !== 1) { + throw new Error(`unify: section ${runNames} would not reproduce the base side of "${file}"`); + } + } + + // names(pT) - newNames must equal names(pB) + const tNamesFiltered = tNames.filter((name) => !newNamesSet.has(name)); + if (tNamesFiltered.length !== bNames.length || !tNamesFiltered.every((name, i) => name === bNames[i])) { + throw new Error(`unify: section ${runNames} would not reproduce the base side of "${file}"`); + } + + // Step 3: Base side proof + const tB = expandSections(pT, {}); + const mB = expandSections(pB, {}); + if (n(substituteKeys(tB, D)) !== n(substituteKeys(mB, D))) { + throw new Error(`unify: section ${runNames} would not reproduce the base side of "${file}"`); + } + + // Step 4: Variant side proof + const combinedValues = { ...profileValues, ...values }; + const tV = expandSections(pT, combinedValues); + const mV = expandSections(pV, profileValues); + if (n(substituteKeys(tV, V)) !== n(substituteKeys(mV, V))) { + throw new Error(`unify: section ${runNames} would not reproduce the variant side of "${file}"`); + } + + // Step 5: Placeholders check (spec 09 §6.2 (b)) + const others = (t: string) => placeholders(n(t)).filter((k) => !K.has(k)); + + const othersTB = others(tB); + const othersMB = others(mB); + if (JSON.stringify(othersTB) !== JSON.stringify(othersMB)) { + throw new Error(`unify: extracting into "${file}" would change which {{…}} placeholders the text holds`); + } + + const othersTV = others(tV); + const othersMV = others(mV); + if (JSON.stringify(othersTV) !== JSON.stringify(othersMV)) { + throw new Error(`unify: extracting into "${file}" would change which {{…}} placeholders the text holds`); + } +} + + +/** + * A Markdown heading line: `/^#{1,6}[ \t]+(.+?)[ \t#]*$/` at column 0. + * The capture group is the heading text, trimmed of trailing spaces and `#`. + */ +const HEADING_RE = /^#{1,6}[ \t]+(.+?)[ \t#]*$/; + +/** + * Turn a heading text into a slug for a section name (spec 12 §4.2): + * NFD, strip combining marks (`\p{M}`), lower-case, non-`[a-z0-9]` → `-`, trim `-`, cut at 40 chars, trim `-` again. + */ +function headingSlug(text: string): string { + // NFD decomposition, strip combining marks + const decomposed = text.normalize("NFD").replace(/\p{M}/gu, ""); + // Lower-case + const lower = decomposed.toLowerCase(); + // Non-[a-z0-9] → - + const dashed = lower.replace(/[^a-z0-9]+/g, "-"); + // Trim leading/trailing - + const trimmed = dashed.replace(/^-+|-+$/g, ""); + // Cut at 40 chars + const cut = trimmed.slice(0, 40); + // Trim trailing - again (in case cut ended mid-run) + return cut.replace(/-+$/, ""); +} + +export interface HunkWithSuggestion { + a: { start: number; lines: string[] }; + suggestion: { class: string }; +} + +/** + * The section name `--save-plan` pre-fills for each hunk of one file, or undefined (spec 12 §4.2). + * Index = hunk index (0-based). + * + * Rules (in this order, per hunk): + * 1. Touches an existing section → that section's exact name. + * 2. Suggestion class `block` → a slug from the nearest heading above, or `section-`. + * 3. Otherwise → undefined. + * + * Uniqueness (rule 2 names only): a candidate name that is in `declared`, or that an earlier + * non-consecutive hunk of this file got, gets `-2`, `-3`, … appended until it is free. + * Consecutive block hunks keep the same name (they form one run if the human sets them all to `take: section`). + * + * A parse error in the base → return all undefined (pre-fill never blocks `--save-plan`). + */ +export function prefillSections(args: { + baseText: string; + label: string; + ref: string; + hunks: HunkWithSuggestion[]; + /** Every section name the ingredient already declares, in any admitted file. */ + declared: Set; +}): Array { + const { baseText, label, ref, hunks, declared } = args; + const result: Array = new Array(hunks.length).fill(undefined); + + // Parse base sections; on error, return all undefined + let parsed: ParsedSections; + try { + parsed = parseSections(baseText, label, ref); + } catch (e) { + if (e instanceof SectionMarkerError) return result; + throw e; + } + + // Build base section spans using the shared helper + const baseSections = sectionSpans(parsed); + + // Split base into lines for heading search + const baseLines = splitLines(baseText).lines; + + // Find the nearest heading above a given position (line number, 1-based) + const nearestHeadingAbove = (pos: number): string | null => { + for (let i = pos - 1; i >= 0; i--) { + const line = baseLines[i]; + const match = HEADING_RE.exec(line); + if (match) return match[1]; + } + return null; + }; + + // Track used names for uniqueness + const usedNames = new Map(); // name → last hunk index that used it + // Also track what was declared + const declaredSet = new Set(declared); + + // Process each hunk + for (let i = 0; i < hunks.length; i++) { + const h = hunks[i]; + + // Rule 1: Touches existing section → that section's exact name + let touchedSection: string | null = null; + for (const sec of baseSections) { + if (touches(h, sec)) { + touchedSection = sec.name; + break; + } + } + + if (touchedSection !== null) { + result[i] = touchedSection; + continue; + } + + // Rule 2: Block suggestion → slug from heading or section- + if (h.suggestion.class === "block") { + // Find position for heading search + let searchPos: number; + if (h.a.lines.length > 0) { + // Above a.start means < a.start, so search starting from a.start - 1 (0-indexed: a.start - 2) + searchPos = h.a.start - 1; // This will search lines [0, a.start-2] in 0-indexed terms + } else { + // At or above N where N = a.start - 1, means lines <= N, so search from N (0-indexed: N-1) + const N = h.a.start - 1; + searchPos = N; // This will search lines [0, N-1] in 0-indexed, i.e., lines 1..N in 1-indexed + } + + const heading = nearestHeadingAbove(searchPos); + let candidate: string; + + if (heading !== null) { + const slug = headingSlug(heading); + // Check if slug is valid (matches SECTION_NAME) and non-empty + if (slug.length > 0 && SECTION_NAME.test(slug)) { + candidate = slug; + } else { + // Fallback: section- with n = 1-based hunk number + candidate = `section-${i + 1}`; + } + } else { + // Fallback: section- + candidate = `section-${i + 1}`; + } + + // Uniqueness check: consecutive block hunks with the same name keep it + const prevIndex = usedNames.get(candidate); + const isConsecutive = prevIndex !== undefined && prevIndex === i - 1; + + if (isConsecutive) { + // Same name as directly preceding hunk, keep it + result[i] = candidate; + usedNames.set(candidate, i); + } else if (declaredSet.has(candidate) || (prevIndex !== undefined && !isConsecutive)) { + // Need to find a unique suffix + let suffix = 2; + let uniqueName = `${candidate}-${suffix}`; + while (declaredSet.has(uniqueName) || usedNames.has(uniqueName)) { + suffix++; + uniqueName = `${candidate}-${suffix}`; + } + result[i] = uniqueName; + usedNames.set(uniqueName, i); + } else { + // Name is free + result[i] = candidate; + usedNames.set(candidate, i); + } + } + // Rule 3: Otherwise → undefined (already the default) + } + + return result; +} diff --git a/src/core/unify.ts b/src/core/unify.ts index fd07b5e..f34c7c3 100644 --- a/src/core/unify.ts +++ b/src/core/unify.ts @@ -7,9 +7,12 @@ import { exists, listFiles, loadForge, readIngredientText, type Forge, type Load import { splitLines, type Hunk } from "./diff.js"; import { detectEol, withEol, type Eol } from "./text.js"; import type { IngredientDiff } from "./variants.js"; -import type { HunkTake, Ingredient, IngredientRef, Recipe, UnifyPlan, PlanFile } from "../schema/index.js"; +import type { HunkTake, Ingredient, IngredientRef, Recipe, UnifyPlan, PlanFile, PlanHunk } from "../schema/index.js"; import { collect, deriveHunk, prove, substitutedFile, type Extraction } from "./extract.js"; -import { parseSections, SectionMarkerError } from "./sections.js"; +import { firstMarkerLine, parseSections, SectionMarkerError } from "./sections.js"; +import { sectionKey } from "./resolve.js"; +import { deriveSections, prefillSections, proveSections, type MarkerInsertion, type SectionRun } from "./section-extract.js"; +import { stripBom, toLf } from "./text.js"; // Spelled out rather than embedded as a raw character: in the one function whose job is byte // fidelity, correctness should not hinge on a glyph no diff viewer, editor or re-encoding shows. @@ -74,8 +77,43 @@ export async function planFrom( profile: string, ): Promise { await assertTextMergeable(base, variant); + + // Build `declared`: every section name the ingredient already declares, in any admitted file (spec 12 §4.2). + const declared = new Set(); + const baseFiles = await listFiles(base.dir); + const label = (rel: string) => ["ingredients", path.basename(path.dirname(base.dir)), path.basename(base.dir), rel].join("/"); + for (const rel of baseFiles) { + if (rel === "ingredient.yaml") continue; + if (!substitutedFile(base.meta, rel)) continue; + try { + const text = await readIngredientText(base, rel); + const parsed = parseSections(text, label(rel), base.ref); + for (const s of parsed.sections) declared.add(s.name); + } catch (e) { + // Ignore parse errors — the check that matters runs later (spec 12 §4.2) + if (!(e instanceof SectionMarkerError)) throw e; + } + } + const files: PlanFile[] = []; for (const f of diff.files) { + // Pre-fill section names only when the file is admitted (substitutedFile) (spec 12 §4.2) + let sectionNames: Array = []; + if (substitutedFile(base.meta, f.file)) { + try { + const baseText = await readIngredientText(base, f.file); + sectionNames = prefillSections({ + baseText, + label: label(f.file), + ref: base.ref, + hunks: f.hunks, + declared, + }); + } catch { + // On any error, leave all undefined (spec 12 §4.2: "pre-fill never blocks --save-plan") + } + } + files.push({ file: f.file, hunks: f.hunks.map((h, i) => ({ @@ -84,6 +122,8 @@ export async function planFrom( take: "keep" as const, // Pre-filled for a value hunk, read only once the human sets take: param (spec 09 §4.2). ...(h.suggestion.class === "value" && h.suggestion.tokens ? { params: h.suggestion.tokens.map((t) => ({ token: t.a, key: t.param })) } : {}), + // Pre-filled section name for --save-plan (spec 12 §4.2). Only when defined. + ...(sectionNames[i] !== undefined ? { section: { name: sectionNames[i] } } : {}), suggestion: h.suggestion, })), }); @@ -132,6 +172,27 @@ export interface ApplyOptions { * metadata difference does not hold the variant back (Ruling 28). */ discardVariantMeta?: boolean; + /** + * Profile `

`'s current `sections` for the base's key (spec 12 §6.5); the CLI passes it in + * step 7. Default `{}`. + */ + profileSections?: Record; +} + +/** One section a plan run creates or fills (spec 12 §4.5, §5.5). */ +export interface SectionExtraction { + /** `/` of the base. */ + key: string; + /** The section name. */ + name: string; + /** The base-relative file. */ + file: string; + /** True for an existing section whose markers were already in the base (Ruling 2). */ + existing: boolean; + /** The base's lines inside the span; `null` for an existing section. */ + default: string | null; + /** The variant's lines between the anchors, canonical (`` or ending with `\n`). */ + value: string; } export interface UnifyResult { @@ -153,6 +214,8 @@ export interface UnifyResult { metaDiffers: string[]; /** The keys a `take: param` plan extracts (spec 09); [] when it has no param hunk. */ params: Extraction[]; + /** The sections a `take: section` plan extracts (spec 12); [] when it has no section hunk. */ + sections: SectionExtraction[]; } /** @@ -174,8 +237,20 @@ export interface UnifyResult { * `B.eofNewline`), not reconstructed from the winning hunk's own `noEofNewline` flag — that flag * has nothing to say when the winning side contributes no lines (a pure removal taken as the * winner), so `variantText` is passed in alongside the hunks for exactly this reason. + * + * The optional `markers` parameter (spec 12 §6.4) inserts section marker lines at their base line + * indices: before base line index `at`, in array order, so the template holds them at the positions + * §6.1's span described. A `take: "section"` hunk contributes its **base** lines, never the + * variant's, so the default text sits between the markers. */ -function mergeFile(baseText: string, variantText: string, hunks: Hunk[], takes: HunkTake[], templates: (string[] | undefined)[] = []): string { +function mergeFile( + baseText: string, + variantText: string, + hunks: Hunk[], + takes: HunkTake[], + templates: (string[] | undefined)[] = [], + markers: MarkerInsertion[] = [], +): string { const bom = baseText.charCodeAt(0) === BOM.charCodeAt(0); const A = splitLines(baseText); const B = splitLines(variantText); @@ -183,20 +258,38 @@ function mergeFile(baseText: string, variantText: string, hunks: Hunk[], takes: let i = 0; // 0-based index into A.lines let iAfterLastHunk = 0; let lastWinnerIsVariant = false; + let mi = 0; // index into markers + + /** Push all markers with `at === idx` before copying any lines at that index. */ + const flushMarkers = (idx: number) => { + while (mi < markers.length && markers[mi].at === idx) out.push(markers[mi++].line); + }; + hunks.forEach((h, k) => { const start = h.a.start - 1; // a.start is 1-based and marks where the hunk applies - while (i < start) out.push(A.lines[i++]); + while (i < start) { + flushMarkers(i); + out.push(A.lines[i++]); + } + flushMarkers(i); const takeVariant = takes[k] === "variant"; + const takeSection = takes[k] === "section"; const side = takeVariant ? h.b : h.a; - // A param hunk contributes its template; its sides pair line by line and agree on the final newline (P2). - out.push(...(takes[k] === "param" ? templates[k]! : side.lines)); + // A param hunk contributes its template; a section hunk contributes the base (the default inside + // the markers); its sides pair line by line and agree on the final newline (P2, §6.4). + out.push(...(takes[k] === "param" ? templates[k]! : takeSection ? h.a.lines : side.lines)); i += h.a.lines.length; // the base's lines for this hunk are consumed either way if (k === hunks.length - 1) { iAfterLastHunk = i; lastWinnerIsVariant = takeVariant; } }); - while (i < A.lines.length) out.push(A.lines[i++]); + while (i < A.lines.length) { + flushMarkers(i); + out.push(A.lines[i++]); + } + // Markers at the end of the file (at === A.lines.length) + flushMarkers(A.lines.length); // The tail decides the final newline only when the last hunk reaches the end of the file. const endsAtTail = hunks.length > 0 && iAfterLastHunk >= A.lines.length; @@ -225,11 +318,29 @@ export async function applyPlan( let unresolved = 0; const extractions = new Map(); const proofs: Array<{ file: string; template: string; mBase: string; mVar: string }> = []; + const sectionExtractions: SectionExtraction[] = []; + const sectionProofs: Array<{ + file: string; + label: string; + template: string; + mBase: string; + mVar: string; + newNames: string[]; + values: Record; + }> = []; + /** The new section names for each file, for U1's expected structure. */ + const newSectionNames = new Map(); + /** Tracks which files have section hunks, for the S11 check. */ + const filesWithSectionHunks = new Set(); + /** Names used across all files, for S3 second form. */ + const globalNameUsage = new Map(); // name → file const hunksByFile = new Map(diff.files.map((f) => [f.file, f.hunks])); const onlyInBase = new Set(diff.onlyInBase); const onlyInVariant = new Set(diff.onlyInVariant); + const profileSections = opts.profileSections ?? {}; + // Ruling 29 (spec §8 refusal 7, in both directions): the plan must cover every file the diff // has — paired, base-only and variant-only — each exactly once. The walk below only visits the // plan's own entries, so without this a plan stripped of an entry would apply as "resolved" and @@ -254,6 +365,29 @@ export async function applyPlan( } } + // S12 first part: When the plan has at least one section hunk, every admitted file of the VARIANT + // (not only the files with runs) is checked for markers. + const planHasSectionHunk = plan.files.some((pf) => pf.hunks?.some((h) => h.take === "section")); + if (planHasSectionHunk) { + const variantFiles = await listFiles(variant.dir); + for (const rel of variantFiles) { + if (rel === "ingredient.yaml") continue; + if (!substitutedFile(variant.meta, rel)) continue; + const variantText = await readIngredientText(variant, rel); + const markerLine = firstMarkerLine(toLf(stripBom(variantText))); + if (markerLine !== null) { + throw new Error( + `unify: ${variant.ref} holds a section marker on ${rel}:${markerLine} — remove it by hand and save the plan again, or re-import the workspace`, + ); + } + } + } + + // Build declaredElsewhere: section names declared in the base's OTHER admitted files. + // This is needed for S6 (new section name already declared). + const baseFiles = await listFiles(base.dir); + const label = (rel: string) => ["ingredients", path.basename(path.dirname(base.dir)), path.basename(base.dir), rel].join("/"); + for (const pf of plan.files) { if (pf.hunks) { const hunks = hunksByFile.get(pf.file); @@ -270,7 +404,8 @@ export async function applyPlan( ); } const takes: HunkTake[] = new Array(hunks.length); - const paramOf = new Map(); + const paramOf = new Map(); + const sectionOf = new Map(); const seen = new Set(); for (const ph of pf.hunks) { if (ph.hunk < 1 || ph.hunk > hunks.length || seen.has(ph.hunk)) { @@ -281,9 +416,123 @@ export async function applyPlan( seen.add(ph.hunk); takes[ph.hunk - 1] = ph.take; if (ph.take === "param") paramOf.set(ph.hunk - 1, ph); + if (ph.take === "section") sectionOf.set(ph.hunk - 1, ph); } + // Section hunks are not counted as unresolved decisions (spec 12 §6.1 step 5: "A section hunk is a decision") unresolved += takes.filter((t) => t === "keep").length; + + // Files with section hunks + if (sectionOf.size > 0) { + filesWithSectionHunks.add(pf.file); + + // S2: the file must be substituted + if (!substitutedFile(base.meta, pf.file)) { + throw new Error( + `unify plan: "${pf.file}" is copied without expansion by a target that emits it — a section marker there would be emitted literally`, + ); + } + + // Build declaredElsewhere for this file + const declaredElsewhere = new Map(); + for (const otherFile of baseFiles) { + if (otherFile === "ingredient.yaml" || otherFile === pf.file) continue; + if (!substitutedFile(base.meta, otherFile)) continue; + const otherText = await readIngredientText(base, otherFile); + const parsed = parseSections(otherText, label(otherFile), base.ref); + for (const s of parsed.sections) { + declaredElsewhere.set(s.name, `${otherFile}:${s.line}`); + } + } + + // Derive sections + const baseText = await readIngredientText(base, pf.file); + const variantText = await readIngredientText(variant, pf.file); + const { runs, markers } = deriveSections({ + file: pf.file, + label: label(pf.file), + ref: base.ref, + baseText, + variantText, + hunks, + entries: pf.hunks, + declaredElsewhere, + }); + + // S3 second form: a name used in two different files + for (const run of runs) { + const existingFile = globalNameUsage.get(run.name); + if (existingFile !== undefined && existingFile !== pf.file) { + throw new Error(`unify plan: section ${run.name} is named in "${existingFile}" and "${pf.file}"`); + } + globalNameUsage.set(run.name, pf.file); + } + + // Collect new section names for U1 + const newNames = runs.filter((r) => !r.existing).map((r) => r.name); + if (newNames.length > 0) { + newSectionNames.set(pf.file, newNames); + } + + // Build values map for this file's runs + const values: Record = {}; + for (const run of runs) { + values[run.name] = run.value; + } + + // Create the three merges: template with markers, all-base, all-variant + // as(side) maps BOTH "param" and "section" to side + const as = (side: HunkTake) => takes.map((t) => (t === "param" || t === "section" ? side : t)); + + // Handle param hunks in the same file + const templates: (string[] | undefined)[] = new Array(hunks.length); + for (const [k, ph] of paramOf) { + const { lines, pairs } = deriveHunk(pf.file, k + 1, hunks[k], ph.params, base.meta.params); + templates[k] = lines; + collect(extractions, pf.file, k + 1, pairs); + } + + const template = mergeFile(baseText, variantText, hunks, takes, templates, markers); + const mBase = mergeFile(baseText, variantText, hunks, as("base"), templates); + const mVar = mergeFile(baseText, variantText, hunks, as("variant"), templates); + + // Record proof for proveSections (not prove) + sectionProofs.push({ + file: pf.file, + label: label(pf.file), + template, + mBase, + mVar, + newNames, + values, + }); + + // Push section extractions + const key = sectionKey(base.meta); + for (const run of runs) { + sectionExtractions.push({ + key, + name: run.name, + file: run.file, + existing: run.existing, + default: run.default, + value: run.value, + }); + } + + // A reuse-only plan (only existing sections, no new sections, no variant takes, no param + // takes) does not change the body by spec 12 §6.7 step 3: "For an existing section, nothing + // (unless param or variant hunks in the same file change it)." Skip mergeFile entirely + // rather than round-tripping a mixed-EOL base through splitLines/withEol. + const hasNewSection = newNames.length > 0; + const hasVariantTake = takes.includes("variant"); + const hasParamTake = paramOf.size > 0; + if (hasNewSection || hasVariantTake || hasParamTake) { + if (template !== baseText) write[pf.file] = template; + } + continue; + } + // A plan with no `variant` decision cannot change this file's bytes — skip the merge // entirely rather than round-tripping the base through `splitLines`/`withEol` for nothing, // which would re-terminate a base with mixed line endings even though no decision moved it. @@ -363,17 +612,47 @@ export async function applyPlan( } } - await checkMarkers(base, write, remove); + await checkMarkers(base, write, remove, newSectionNames); const metaDiffers = opts.discardVariantMeta ? [] : metaDifferences(base, variant); const params = [...extractions.values()]; + const sections = sectionExtractions; + + // S11: when the plan has a section hunk, the variant must be resolved in the same plan + if (sections.length) { + if (unresolved) { + throw new Error(`unify plan: take: section needs the variant resolved in the same plan — ${unresolved} decision(s) still keep`); + } + if (metaDiffers.length) { + throw new Error(`unify plan: take: section needs the variant resolved in the same plan — ingredient.yaml differs in ${metaDiffers.join(", ")}`); + } + } + + // P10: the base now holds {{key}}; left unresolved, the next diff would show the same hunk as placeholder versus literal. + // When both params and sections are present, report the param wording (P10). if (params.length) { - // P10: the base now holds {{key}}; left unresolved, the next diff would show the same hunk as placeholder versus literal. if (unresolved) throw new Error(`unify plan: take: param needs the variant resolved in the same plan — ${unresolved} decision(s) still keep`); if (metaDiffers.length) throw new Error(`unify plan: take: param needs the variant resolved in the same plan — ingredient.yaml differs in ${metaDiffers.join(", ")}`); - for (const p of proofs) prove(p.file, p.template, p.mBase, p.mVar, params); } - return { write, remove, resolved: unresolved === 0 && metaDiffers.length === 0, unresolved, metaDiffers, params }; + + // Run proofs: param-only files use prove, files with section runs use proveSections + for (const p of proofs) prove(p.file, p.template, p.mBase, p.mVar, params); + for (const p of sectionProofs) { + proveSections({ + file: p.file, + label: p.label, + ref: base.ref, + template: p.template, + mBase: p.mBase, + mVar: p.mVar, + newNames: p.newNames, + values: p.values, + profileValues: profileSections, + extractions: params, + }); + } + + return { write, remove, resolved: unresolved === 0 && metaDiffers.length === 0, unresolved, metaDiffers, params, sections }; } /** The sequence of section names a text declares, or the parse problem. */ @@ -387,13 +666,19 @@ function markerStructure(text: string, label: string): { names: string[] } | { p } /** - * U1 (spec 11 §6.12, Ruling 8): a merge never adds, removes or changes a section marker. Taking the - * variant's side of a hunk that holds a marker would leave an unterminated section, which every later - * sync refuses, or drop a section whose profile values would then silently stop applying. Each file - * the result writes or removes is compared, as the sequence of its section names, with the base's — - * before anything is written. Editing sections through unify is not supported yet. + * U1 (spec 11 §6.12, spec 12 §6.8): a merge never adds, removes or changes a section marker unless + * the plan declares it. Taking the variant's side of a hunk that holds a marker would leave an + * unterminated section, which every later sync refuses, or drop a section whose profile values would + * then silently stop applying. Each file the result writes or removes is compared, as the sequence + * of its section names, with the **expected** sequence: the base's, plus the plan's new sections + * inserted in span order. A plan without section hunks expects the base's sequence, as in 0.7.x. */ -async function checkMarkers(base: LoadedIngredient, write: Record, remove: string[]): Promise { +async function checkMarkers( + base: LoadedIngredient, + write: Record, + remove: string[], + newNames: Map = new Map(), +): Promise { const label = (rel: string) => ["ingredients", path.basename(path.dirname(base.dir)), path.basename(base.dir), rel].join("/"); const touched = [...Object.keys(write), ...remove].filter((rel) => substitutedFile(base.meta, rel)).sort(); for (const rel of touched) { @@ -405,11 +690,36 @@ async function checkMarkers(base: LoadedIngredient, write: Record (xs.length ? xs.join(", ") : "none"); - why = `sections ${show(before.names)} would become ${show(after.names)}`; + else { + // Compute expected names: base's names plus new names from the plan + // For a file in newNames, the comparison becomes: `after` names with the new names removed must + // equal `before` names, and each new name must occur exactly once in `after`. + const fileNewNames = newNames.get(rel); + if (fileNewNames && fileNewNames.length > 0) { + const newNamesSet = new Set(fileNewNames); + // Check each new name occurs exactly once in after + for (const name of fileNewNames) { + const count = after.names.filter((n) => n === name).length; + if (count !== 1) { + const show = (xs: string[]) => (xs.length ? xs.join(", ") : "none"); + why = `sections ${show(before.names)} would become ${show(after.names)}`; + break; + } + } + if (!why) { + // after names with new names removed must equal before names + const afterFiltered = after.names.filter((n) => !newNamesSet.has(n)); + if (JSON.stringify(before.names) !== JSON.stringify(afterFiltered)) { + const show = (xs: string[]) => (xs.length ? xs.join(", ") : "none"); + why = `sections ${show(before.names)} would become ${show(after.names)}`; + } + } + } else if (JSON.stringify(before.names) !== JSON.stringify(after.names)) { + const show = (xs: string[]) => (xs.length ? xs.join(", ") : "none"); + why = `sections ${show(before.names)} would become ${show(after.names)}`; + } } - if (why) throw new Error(`unify: ${label(rel)} would lose or change section markers (${why}) — take base for the marker lines; editing sections through unify is not supported yet`); + if (why) throw new Error(`unify: ${label(rel)} would lose or change section markers (${why}) — take base for the marker lines, or take: section to fill the section`); } } diff --git a/src/importers/claude-code.ts b/src/importers/claude-code.ts index 7d5bf3e..d39b8f8 100644 --- a/src/importers/claude-code.ts +++ b/src/importers/claude-code.ts @@ -3,10 +3,11 @@ import path from "node:path"; import YAML from "yaml"; import { parseFrontmatter } from "../core/frontmatter.js"; import { exists, listFiles, parseYaml, typeFolder, FORGE_MANIFEST } from "../core/forge.js"; +import { manifestWithSections } from "../core/manifest-edit.js"; import { decodeForScan, findSecrets, hasUtf16Bom, secretValueKind } from "../core/secrets.js"; import { stripBom, toLf } from "../core/text.js"; import { fingerprintDir, fingerprintOf, type DirReader } from "../core/fingerprint.js"; -import { FORGE_SCHEMA_SECTIONS, ForgeManifestSchema, IngredientSchema, ProfileSchema, RecipeSchema, WorkspaceConfigSchema, type Ingredient, type McpServer, type Profile, type Sections, type Target } from "../schema/index.js"; +import { FORGE_SCHEMA_SECTIONS, IngredientSchema, ProfileSchema, RecipeSchema, WorkspaceConfigSchema, type Ingredient, type McpServer, type Profile, type Sections, type Target } from "../schema/index.js"; import { isDeepStrictEqual } from "node:util"; import { resolve } from "../core/resolve.js"; import { editYamlText } from "../core/yaml-edit.js"; @@ -967,27 +968,9 @@ async function stageManifest(stage: ForgeStage, forge: string, needs: boolean): return "created"; } if (!needs) return "unchanged"; - const i13 = (why: string) => new Error(`import: cannot edit ${FORGE_MANIFEST} in place (${why}) — set schema: ${FORGE_SCHEMA_SECTIONS} by hand, commit, and re-run`); const raw = await stage.readText(abs); - let before: unknown; - try { - before = YAML.parse(stripBom(raw)); - } catch (e) { - throw i13(`it does not parse: ${(e as Error).message}`); - } - // Unreachable while forgeBefore loads the manifest through its schema first (I5); kept so the edit never assumes it. - if (before === null || typeof before !== "object" || Array.isArray(before)) throw i13("it is not a YAML mapping"); - if ((before as { schema?: unknown }).schema === FORGE_SCHEMA_SECTIONS) return "unchanged"; - let content: string; - try { - content = editYamlText(raw, { command: "import", label: FORGE_MANIFEST, keys: ["schema"] }, (doc) => doc.set("schema", FORGE_SCHEMA_SECTIONS)); - } catch (e) { - throw i13((e as Error).message.replace(/^.*in place \((.*)\) — .*$/s, "$1")); - } - const after = YAML.parse(stripBom(content)); - if (!ForgeManifestSchema.safeParse(after).success || !isDeepStrictEqual(after, { ...before, schema: FORGE_SCHEMA_SECTIONS })) { - throw i13(`the edit does not read back as the original with exactly schema: ${FORGE_SCHEMA_SECTIONS}`); - } + const content = manifestWithSections(raw, "import"); + if (content === null) return "unchanged"; stage.write(abs, content); return "edited"; } diff --git a/src/schema/index.ts b/src/schema/index.ts index 0c050c8..476b780 100644 --- a/src/schema/index.ts +++ b/src/schema/index.ts @@ -284,8 +284,8 @@ export type HunkSuggestion = z.infer; /** `substitute`'s key syntax (src/core/resolve.ts). */ export const PARAM_KEY = /^[A-Za-z0-9_.]+$/; -/** A hunk may also become a parameter (spec 09); a one-sided file entry keeps TakeSchema. */ -export const HunkTakeSchema = z.enum(["base", "variant", "param", "keep"]); +/** A hunk may also become a parameter (spec 09) or a section (spec 12); a one-sided file entry keeps TakeSchema. */ +export const HunkTakeSchema = z.enum(["base", "variant", "param", "section", "keep"]); export type HunkTake = z.infer; export const PlanParamSchema = z @@ -293,6 +293,16 @@ export const PlanParamSchema = z .strict(); export type PlanParam = z.infer; +/** A section a hunk becomes (spec 12 §4.1). Read only when take is "section". */ +export const PlanSectionSchema = z + .object({ + name: z.string().regex(SECTION_NAME, "a section name is slug-like"), + /** Base line numbers, 1-based and inclusive: widens the section over equal base lines (spec 12 §6.2). */ + lines: z.string().regex(/^[1-9][0-9]*-[1-9][0-9]*$/, 'lines is "-"').optional(), + }) + .strict(); +export type PlanSection = z.infer; + export const PlanHunkSchema = z.object({ hunk: z.number().int().positive(), /** Human echo of what `forge diff` printed. Never read back. */ @@ -300,6 +310,8 @@ export const PlanHunkSchema = z.object({ take: HunkTakeSchema, /** Read only when take is "param" (spec 09 §4.1). */ params: z.array(PlanParamSchema).optional(), + /** Read only when take is "section" (spec 12 §4.1). */ + section: PlanSectionSchema.optional(), /** Echo of the suggested class when the plan was saved. Never read back; a malformed one is dropped. */ suggestion: HunkSuggestionSchema.optional().catch(undefined), }); diff --git a/test/cli.test.ts b/test/cli.test.ts index c81b33b..6907c51 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -763,6 +763,9 @@ describe("cli", () => { metaDiffers: [], // Spec 09 §4.5: always present, [] / null when the plan extracts nothing. params: [], + // Spec 12 §4.5: always present, [] / false when no section hunk. + sections: [], + manifestEdited: false, profileEdited: null, // Ruling 38: a removed variant always warns about overrides.ingredients.disable. warnings: [ @@ -1868,9 +1871,461 @@ describe("cli — sections (spec 11 §4.2, §4.3)", () => { const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--take", "variant", "--forge", root]); expect(r.code).toBe(1); expect(r.stderr).toContain( - "error: unify: ingredients/rules/review-posture/rule.md would lose or change section markers (sections flavors would become none) — take base for the marker lines; editing sections through unify is not supported yet", + "error: unify: ingredients/rules/review-posture/rule.md would lose or change section markers (sections flavors would become none) — take base for the marker lines, or take: section to fill the section", ); expect(await snapshot(root)).toEqual(before); expect(gitStatus(root)).toBe(""); }); }); + +describe("forge unify take: section (spec 12)", () => { + // Helper functions for section extraction tests + async function sectionForge(extra: Record = {}): Promise { + const root = await tmpDir("craftar-cli-forge-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + // Base and variant with only the table differing — creates a block hunk + const baseBody = "# Review posture\n\nDispatch reviewers.\n\nshared line\n"; + const variantBody = "# Review posture\n\nDispatch reviewers.\n\n| Repo | Reviewer |\n|---|---|\n| `acme-api` | backend |\n\nshared line\n"; + await makeForge(root, { + ingredients: [ + rule("review-posture", baseBody), + rule("review-posture--acme", variantBody, { as: "review-posture" }), + ], + recipes: [recipe("base", ["rule/review-posture"]), recipe("base--acme", ["rule/review-posture--acme"])], + profiles: [profile("acme", ["base--acme"]), profile("globex", ["base"])], + }); + await writeFiles(root, extra); + gitInit(root); + gitCommitAll(root, "init"); + return root; + } + + async function savedSectionPlan(root: string, edit: (plan: any) => void): Promise { + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + edit(plan); + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + return edited; + } + + it("a full run: --save-plan → edit → --plan exits 0, writes manifest, body with markers, profile with value, removes variant", async () => { + const root = await sectionForge(); + const planPath = await savedSectionPlan(root, (plan) => { + // The hunk should have a pre-filled section (from the heading slug or as block) + expect(plan.files[0].hunks.length).toBe(1); + expect(plan.files[0].hunks[0].suggestion.class).toBe("block"); + expect(plan.files[0].hunks[0].section).toBeDefined(); + // Set take: section and use the pre-filled name or a custom one + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; // No lines needed for a block hunk + }); + + const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", planPath, "--forge", root]); + expect(r.code, r.stderr).toBe(0); + + // Text report checks + expect(r.stdout).toContain("forge craftar.forge.yaml edited (schema: 2)"); + expect(r.stdout).toContain("section rule/review-posture flavors — default"); + expect(r.stdout).toContain("(profile acme)"); + expect(r.stdout).toContain("~ profiles/acme/profile.yaml"); + expect(r.stdout).toContain("removed variant rule/review-posture--acme"); + // W3 warning + expect(r.stdout).toContain("warn flavors is now a section of rule/review-posture"); + expect(r.stdout).toContain("overrides.sections.rule/review-posture.flavors"); + + // Verify files on disk + const body = await fs.readFile(path.join(root, "ingredients/rules/review-posture/rule.md"), "utf8"); + expect(body).toContain(""); + expect(body).toContain(""); + + const profileYaml = YAML.parse(await fs.readFile(path.join(root, "profiles/acme/profile.yaml"), "utf8")); + expect(profileYaml.sections?.["rule/review-posture"]?.flavors).toBeDefined(); + + const manifest = YAML.parse(await fs.readFile(path.join(root, "craftar.forge.yaml"), "utf8")); + expect(manifest.schema).toBe(2); + + // Variant directory is gone + expect(await fs.access(path.join(root, "ingredients/rules/review-posture--acme")).then(() => true, () => false)).toBe(false); + }); + + it("--json includes sections array and manifestEdited: true", async () => { + const root = await sectionForge(); + const planPath = await savedSectionPlan(root, (plan) => { + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + }); + + const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", planPath, "--forge", root, "--json"]); + expect(r.code, r.stderr).toBe(0); + + const out = JSON.parse(r.stdout); + expect(out.manifestEdited).toBe(true); + expect(out.sections).toHaveLength(1); + expect(out.sections[0]).toMatchObject({ + key: "rule/review-posture", + name: "flavors", + file: "rule.md", + existing: false, + written: true, + }); + expect(typeof out.sections[0].defaultLines).toBe("number"); + expect(typeof out.sections[0].valueLines).toBe("number"); + expect(out.profileEdited).toBe("profiles/acme/profile.yaml"); + }); + + it("a plan without section hunks has sections: [] and manifestEdited: false", async () => { + const root = await sectionForge(); + // Use --take base to resolve without sections + const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--take", "base", "--forge", root, "--json"]); + expect(r.code, r.stderr).toBe(0); + const out = JSON.parse(r.stdout); + expect(out.sections).toEqual([]); + expect(out.manifestEdited).toBe(false); + }); + + it("an existing section: shows '— existing ·', no manifest line, no W3", async () => { + // Create a Forge where the base already has markers + const root = await tmpDir("craftar-cli-forge-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + const baseBody = "# Review posture\n\n\n| globex |\n\n"; + const variantBody = "# Review posture\n\n| acme |\n| acme-web |\n"; + await makeForge(root, { + manifest: { schema: 2 }, + ingredients: [ + rule("review-posture", baseBody), + rule("review-posture--acme", variantBody, { as: "review-posture" }), + ], + recipes: [recipe("base", ["rule/review-posture"]), recipe("base--acme", ["rule/review-posture--acme"])], + profiles: [profile("acme", ["base--acme"]), profile("globex", ["base"])], + }); + gitInit(root); + gitCommitAll(root, "init"); + + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + // Hunks touching existing section should have pre-filled name + for (const h of plan.files[0].hunks) { + h.take = "section"; + h.section = { name: "flavors" }; + } + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + + const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", edited, "--forge", root]); + expect(r.code, r.stderr).toBe(0); + + // Text report: existing section format + expect(r.stdout).toContain("— existing ·"); + // No manifest line (already schema: 2) + expect(r.stdout).not.toContain("forge craftar.forge.yaml edited"); + // No W3 warning for existing section + expect(r.stdout).not.toContain("is now a section of"); + }); + + it("an already-in-place value: line ends with ' — already in place'", async () => { + // Forge A: run a full extraction to learn the exact value + const rootA = await sectionForge(); + const planA = await savedSectionPlan(rootA, (plan) => { + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + }); + expect(runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", planA, "--forge", rootA]).code).toBe(0); + const profileA = YAML.parse(await fs.readFile(path.join(rootA, "profiles/acme/profile.yaml"), "utf8")); + const extractedValue = profileA.sections["rule/review-posture"].flavors; + + // Forge B: identical structure, but pre-write the value into the profile + const rootB = await sectionForge({ + "profiles/acme/profile.yaml": YAML.stringify({ + name: "acme", + recipes: ["base--acme"], + params: {}, + sections: { "rule/review-posture": { flavors: extractedValue } }, + }), + }); + const profileBefore = await fs.readFile(path.join(rootB, "profiles/acme/profile.yaml"), "utf8"); + const planB = await savedSectionPlan(rootB, (plan) => { + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + }); + + const rText = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", planB, "--forge", rootB]); + expect(rText.code, rText.stderr).toBe(0); + // The section line ends with "— already in place" + expect(rText.stdout).toMatch(/section rule\/review-posture flavors.*— already in place/); + // No profile edit line (the value is in place, so the profile is not touched) + expect(rText.stdout).not.toContain("~ profiles/acme/profile.yaml"); + // Profile bytes unchanged + expect(await fs.readFile(path.join(rootB, "profiles/acme/profile.yaml"), "utf8")).toBe(profileBefore); + // But the manifest IS bumped (pinning fix 21d4f5d — a new section adds markers, so schema: 2 is required) + expect(rText.stdout).toContain("forge craftar.forge.yaml edited (schema: 2)"); + const manifestB = YAML.parse(await fs.readFile(path.join(rootB, "craftar.forge.yaml"), "utf8")); + expect(manifestB.schema).toBe(2); + + // --json: sections[0].written === false and manifestEdited === true + const rootC = await sectionForge({ + "profiles/acme/profile.yaml": YAML.stringify({ + name: "acme", + recipes: ["base--acme"], + params: {}, + sections: { "rule/review-posture": { flavors: extractedValue } }, + }), + }); + const planC = await savedSectionPlan(rootC, (plan) => { + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + }); + const rJson = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", planC, "--forge", rootC, "--json"]); + expect(rJson.code, rJson.stderr).toBe(0); + const out = JSON.parse(rJson.stdout); + expect(out.sections[0].written).toBe(false); + expect(out.manifestEdited).toBe(true); + }); + + it("mustHold: an untracked manifest makes the run exit 1 naming it, Forge untouched", async () => { + const root = await sectionForge(); + // Untrack the manifest + execFileSync("git", ["-C", root, "rm", "-q", "--cached", "craftar.forge.yaml"]); + execFileSync("git", ["-C", root, "config", "status.showUntrackedFiles", "no"]); + execFileSync("git", ["-C", root, "commit", "-q", "-m", "untrack manifest"], { env: gitEnv() }); + + const planPath = await savedSectionPlan(root, (plan) => { + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + }); + + const before = await snapshot(root); + const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", planPath, "--forge", root]); + expect(r.code).toBe(1); + expect(r.stderr).toContain("craftar.forge.yaml is not held by git"); + expect(await snapshot(root)).toEqual(before); + }); + + it.skipIf(process.platform === "win32" || process.getuid?.() === 0)("a late failure after the manifest write names craftar.forge.yaml in the recovery commands", async () => { + // Use a helper that restores the permission in cleanup so the temp dir can be removed + const root = await tmpDir("craftar-cli-forge-"); + cleanups.push(async () => { + await fs.chmod(path.join(root, "ingredients/rules"), 0o755).catch(() => {}); + await fs.rm(root, { recursive: true, force: true }); + }); + // Build the same Forge structure as sectionForge + const baseBody = "# Review posture\n\nDispatch reviewers.\n\nshared line\n"; + const variantBody = "# Review posture\n\nDispatch reviewers.\n\n| Repo | Reviewer |\n|---|---|\n| `acme-api` | backend |\n\nshared line\n"; + await makeForge(root, { + ingredients: [ + rule("review-posture", baseBody), + rule("review-posture--acme", variantBody, { as: "review-posture" }), + ], + recipes: [recipe("base", ["rule/review-posture"]), recipe("base--acme", ["rule/review-posture--acme"])], + profiles: [profile("acme", ["base--acme"]), profile("globex", ["base"])], + }); + gitInit(root); + gitCommitAll(root, "init"); + + // Save and edit the plan + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + + // Make ingredients/rules read-only: the variant removal (last write) will fail + await fs.chmod(path.join(root, "ingredients/rules"), 0o555); + + const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "acme", "--plan", edited, "--forge", root]); + expect(r.code).toBe(1); + // stderr names craftar.forge.yaml (the manifest was touched) + expect(r.stderr).toContain("craftar.forge.yaml"); + // stderr names the profile (it was edited) + expect(r.stderr).toContain("profiles/acme/profile.yaml"); + // stderr names the body file (it was written with markers) + expect(r.stderr).toContain("ingredients/rules/review-posture/rule.md"); + // The recovery line shows git checkout for those paths + expect(r.stderr).toContain("git -C"); + expect(r.stderr).toContain("checkout --"); + }); + + // Table-driven refusal tests: each scenario exits 1, emits the fragment, and leaves the Forge untouched. + const refusalCases: Array<{ + name: string; + fragment: string; + setup: () => Promise<{ root: string; planPath: string }>; + }> = [ + { + name: "S5: lines out of range", + fragment: "out of range", + setup: async () => { + const root = await tmpDir("craftar-cli-s5-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + // Base and variant with a block hunk + await makeForge(root, { + ingredients: [ + rule("wf", "a\nb\nc\n"), + rule("wf--acme", "a\nB\nc\n", { as: "wf" }), + ], + recipes: [recipe("base", ["rule/wf"]), recipe("base--acme", ["rule/wf--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + gitInit(root); + gitCommitAll(root, "init"); + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "data", lines: "1-100" }; // out of range + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + return { root, planPath: edited }; + }, + }, + { + name: "S5 without lines: existing section covers take: base hunk (Case D)", + fragment: "section flavors (lines ", + setup: async () => { + const root = await tmpDir("craftar-cli-s5-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + // Base with markers at lines 3-7: opener, a, b, c, closer + // Variant without markers: a, B, C (b and c changed) + const baseBody = "H\n\n\na\nb\nc\n\n\nF\n"; + const variantBody = "H\n\na\nB\nC\n\nF\n"; + await makeForge(root, { + ingredients: [ + rule("wf", baseBody), + rule("wf--acme", variantBody, { as: "wf" }), + ], + recipes: [recipe("base", ["rule/wf"]), recipe("base--acme", ["rule/wf--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + gitInit(root); + gitCommitAll(root, "init"); + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + // Hunk 1: section flavors, hunk 2: base (covers hunk 2, which is inside span 3-7) + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "flavors" }; + plan.files[0].hunks[1].take = "base"; + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + return { root, planPath: edited }; + }, + }, + { + name: "S12: variant holds a section marker", + fragment: "holds a section marker", + setup: async () => { + const root = await tmpDir("craftar-cli-s12-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + // Variant has a marker + await makeForge(root, { + ingredients: [ + rule("wf", "a\nb\n"), + rule("wf--acme", "a\n\ny\n\n", { as: "wf" }), + ], + recipes: [recipe("base", ["rule/wf"]), recipe("base--acme", ["rule/wf--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + gitInit(root); + gitCommitAll(root, "init"); + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + // Set take: section on hunks + for (const h of plan.files[0].hunks) { + h.take = "section"; + h.section = { name: "s" }; + } + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + return { root, planPath: edited }; + }, + }, + { + name: "S13: another profile sets the section", + fragment: "it names no marker today and would start to apply", + setup: async () => { + const root = await tmpDir("craftar-cli-s13-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + await makeForge(root, { + ingredients: [ + rule("wf", "a\nb\n"), + rule("wf--acme", "a\nB\n", { as: "wf" }), + ], + recipes: [recipe("base", ["rule/wf"]), recipe("base--acme", ["rule/wf--acme"])], + profiles: [ + profile("acme", ["base--acme"]), + // globex sets section 'data' on rule/wf — this would start to apply after acme's extraction + { name: "globex", recipes: ["base"], targets: ["claude-code"], params: {}, sections: { "rule/wf": { data: "other\n" } } }, + ], + }); + gitInit(root); + gitCommitAll(root, "init"); + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "data" }; // same name globex already sets + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + return { root, planPath: edited }; + }, + }, + { + name: "S16: profile sections is an alias", + fragment: "sections is an alias", + setup: async () => { + const root = await tmpDir("craftar-cli-s16-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + await makeForge(root, { + ingredients: [ + rule("wf", "a\nb\n"), + rule("wf--acme", "a\nB\n", { as: "wf" }), + ], + recipes: [recipe("base", ["rule/wf"]), recipe("base--acme", ["rule/wf--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + // Overwrite the profile with a YAML alias + await fs.writeFile(path.join(root, "profiles/acme/profile.yaml"), "name: acme\nrecipes:\n - base--acme\nx: &s {}\nsections: *s\n"); + gitInit(root); + gitCommitAll(root, "init"); + const dir = await tmpDir("craftar-cli-plan-"); + cleanups.push(() => fs.rm(dir, { recursive: true, force: true })); + const planPath = path.join(dir, "plan.yaml"); + expect(runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", planPath, "--forge", root]).code).toBe(0); + const plan = YAML.parse(await fs.readFile(planPath, "utf8")); + plan.files[0].hunks[0].take = "section"; + plan.files[0].hunks[0].section = { name: "data" }; + const edited = path.join(dir, "edited.yaml"); + await fs.writeFile(edited, YAML.stringify(plan)); + return { root, planPath: edited }; + }, + }, + ]; + + it.each(refusalCases)("$name: exits 1, emits fragment, Forge untouched", async ({ fragment, setup }) => { + const { root, planPath } = await setup(); + const before = await snapshot(root); + const r = runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--plan", planPath, "--forge", root]); + expect(r.code).toBe(1); + expect(r.stderr).toContain(fragment); + expect(await snapshot(root)).toEqual(before); + }); +}); diff --git a/test/golden-take-section.test.ts b/test/golden-take-section.test.ts new file mode 100644 index 0000000..4ac4ec6 --- /dev/null +++ b/test/golden-take-section.test.ts @@ -0,0 +1,324 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { promises as fs } from "node:fs"; +import { execFileSync } from "node:child_process"; +import os from "node:os"; +import path from "node:path"; +import YAML from "yaml"; +import { importClaudeCode } from "../src/importers/claude-code.js"; +import { listFiles } from "../src/core/forge.js"; +import { runCli } from "./helpers/cli.js"; + +/** + * The take-section golden round trip (spec 12 §10.5, AC 3): three workspaces that differ only in a + * reviewer table, a block hunk turned into a section via `forge unify`, and every workspace + * remaining unchanged — the profiles hold their section values, the generated files hold no + * marker, and no workspace byte moves, AGENTS.md included. + * + * Inputs: `test/golden/take-section-acme/`, `take-section-globex/` and `take-section-initech/` hold + * a hand-written `.claude/rules/` (`review-posture.md`, which differs only in its reviewer table, + * and `shared.md`). + * + * The expected Forge after step 3 (the first unify), `test/golden/forge-take-section-expected/`, + * is regenerated only with the user's confirmation, and its diff reviewed like code: + * CRAFTAR_REGEN_GOLDEN_TAKE_SECTION=1 npx vitest run test/golden-take-section.test.ts + */ +const GOLDEN = path.resolve(__dirname, "golden"); +const INPUT = { + acme: path.join(GOLDEN, "take-section-acme"), + globex: path.join(GOLDEN, "take-section-globex"), + initech: path.join(GOLDEN, "take-section-initech"), +}; +const EXPECTED = path.join(GOLDEN, "forge-take-section-expected"); +const FIXED = new Date("2026-10-02T12:00:00Z"); +const PROFILES = ["acme", "globex", "initech"] as const; +type P = (typeof PROFILES)[number]; + +const cleanups: Array<() => Promise> = []; +afterEach(async () => { + vi.useRealTimers(); + while (cleanups.length) await cleanups.pop()!(); +}); + +const treeFiles = async (dir: string) => (await listFiles(dir)).filter((rel) => rel !== ".git" && !rel.startsWith(".git/")); +async function copyTree(src: string, dst: string): Promise { + for (const rel of await treeFiles(src)) { + await fs.mkdir(path.dirname(path.join(dst, rel)), { recursive: true }); + await fs.copyFile(path.join(src, rel), path.join(dst, rel)); + } +} +async function snapshot(dir: string): Promise> { + const out: Record = {}; + for (const rel of await treeFiles(dir)) out[rel] = (await fs.readFile(path.join(dir, rel))).toString("base64"); + return out; +} +const gitEnv = (): NodeJS.ProcessEnv => ({ + ...process.env, + GIT_AUTHOR_NAME: "craftar-test", + GIT_AUTHOR_EMAIL: "test@example.invalid", + GIT_COMMITTER_NAME: "craftar-test", + GIT_COMMITTER_EMAIL: "test@example.invalid", +}); +function gitCommitAll(dir: string, message: string): void { + execFileSync("git", ["-C", dir, "add", "-A"]); + execFileSync("git", ["-C", dir, "commit", "-q", "-m", message], { env: gitEnv() }); +} +const gitStatus = (dir: string) => execFileSync("git", ["-C", dir, "status", "--porcelain"], { encoding: "utf8" }); +const statesOf = (ws: string) => { + const r = runCli(["status", "--workspace", ws, "--json"]); + expect(r.code, r.stderr).toBe(0); + return Object.fromEntries((JSON.parse(r.stdout).statuses as Array<{ path: string; state: string }>).map((s) => [s.path, s.state])); +}; +const importCli = (forge: string, profile: P, ws: string) => + runCli(["import", "--from", "claude-code", "--forge", forge, "--profile", profile, "--workspace", ws, "--write-config"]); + +/** Appends `kiro` and `agents-md` to a block `targets` list. */ +function addKiroAndAgentsMd(yaml: string): string { + const out = yaml.replace(/^(targets:\n(?: {2}- .*\n)+)/m, "$1 - kiro\n - agents-md\n"); + if (out === yaml) throw new Error("expected a block targets list"); + return out; +} + +async function edit(file: string, f: (text: string) => string): Promise { + await fs.writeFile(file, f(await fs.readFile(file, "utf8"))); +} + +interface RoundTrip { + forge: string; + tmp: string; + ws: Record; + /** All three workspaces after step 1's sync, locks included. */ + synced: Record>; +} + +/** + * Step 1: Import acme, globex, initech with --write-config at a fixed date. Append agents-md to + * profiles and workspaces. Commit. Sync and snapshot. Assert variants created. + */ +async function step1_importAndSync(): Promise { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "craftar-golden-take-section-")); + cleanups.push(() => fs.rm(tmp, { recursive: true, force: true })); + const forge = path.join(tmp, "forge"); + const ws: Record = { + acme: path.join(tmp, "acme"), + globex: path.join(tmp, "globex"), + initech: path.join(tmp, "initech"), + }; + + // Import all three with --write-config at a fixed date. + vi.useFakeTimers({ toFake: ["Date"] }); + vi.setSystemTime(FIXED); + for (const p of PROFILES) { + await copyTree(INPUT[p], ws[p]); + const r = await importClaudeCode({ workspaceRoot: ws[p], forgeRoot: forge, profileName: p, writeWorkspaceConfig: true }); + expect(r.rejected, p).toEqual([]); + if (p === "globex") { + expect(r.variants.map((v) => v.name)).toEqual(["rule/review-posture--globex"]); + } + if (p === "initech") { + expect(r.variants.map((v) => v.name)).toEqual(["rule/review-posture--initech"]); + } + } + vi.useRealTimers(); + + // Append agents-md to profiles and workspaces. + for (const p of PROFILES) { + await edit(path.join(forge, "profiles", p, "profile.yaml"), addKiroAndAgentsMd); + await edit(path.join(ws[p], "craftar.yaml"), addKiroAndAgentsMd); + } + + execFileSync("git", ["init", "-q", forge]); + gitCommitAll(forge, "import acme, globex and initech"); + + // Sync and snapshot all three. + const synced = {} as Record>; + for (const p of PROFILES) { + const r = runCli(["sync", "--workspace", ws[p]]); + expect(r.code, r.stderr).toBe(0); + synced[p] = await snapshot(ws[p]); + } + + return { forge, tmp, ws, synced }; +} + +/** Step 4 / after step 5: All workspaces unchanged, byte-equal, no marker in generated files. */ +async function assertNothingMoved(t: RoundTrip): Promise { + for (const p of PROFILES) { + const states = statesOf(t.ws[p]); + expect(Object.entries(states).filter(([, s]) => s !== "unchanged"), `${p} has non-unchanged files`).toEqual([]); + const check = runCli(["sync", "--check", "--workspace", t.ws[p]]); + expect(check.code, `${p}: ${check.stdout}${check.stderr}`).toBe(0); + expect(await snapshot(t.ws[p]), `${p} snapshot mismatch`).toEqual(t.synced[p]); + for (const rel of await treeFiles(t.ws[p])) { + expect(await fs.readFile(path.join(t.ws[p], rel), "utf8"), `${p} ${rel}`).not.toContain("craftar:section"); + } + } +} + +describe("golden: take-section round trip (spec 12 §10.5)", () => { + it("steps 1–6: a block hunk becomes a section, an existing section is filled, and no workspace byte moves", { timeout: 180_000 }, async () => { + // Step 1: Import and sync. + const t = await step1_importAndSync(); + + // Step 2: Save plan for globex. + const planGlobex = path.join(t.tmp, "globex.yaml"); + const savePlan = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--save-plan", planGlobex, "--forge", t.forge]); + expect(savePlan.code, savePlan.stderr).toBe(0); + + // The block hunk should have a pre-filled section with name derived from "## Reviewer flavors". + const planRaw = await fs.readFile(planGlobex, "utf8"); + const plan = YAML.parse(planRaw); + const ruleFile = plan.files.find((f: { file: string }) => f.file === "rule.md"); + expect(ruleFile).toBeDefined(); + // The globex variant has extra rows = block hunk(s). + const blockHunk = ruleFile.hunks.find((h: any) => h.suggestion?.class === "block"); + expect(blockHunk, "expected a block hunk for globex").toBeDefined(); + expect(blockHunk.section?.name).toBe("reviewer-flavors"); + expect(blockHunk.take).toBe("keep"); + + // Step 3: Edit the plan, apply it. + // Set take: section on all hunks, set lines to cover the table header through the last shared row. + // The table in the base (acme) is: + // ## Reviewer flavors (line 5) + // (blank line) (line 6) + // | Repo | Reviewer | (line 7) + // |---|---| (line 8) + // | `acme-api` | ... (line 9) + // | `acme-web` | ... (line 10) + // (blank line) (line 11) + // Per the spec: "set section.lines to the base line range of the header through the last shared row". + // The shared rows are lines 7-10 (header, separator, acme-api, acme-web). + // We use lines 7-10 to include the table header and the shared rows. + for (const h of ruleFile.hunks) { + h.take = "section"; + h.section = { name: "reviewer-flavors", lines: "7-10" }; + } + await fs.writeFile(planGlobex, YAML.stringify(plan)); + + const applyPlan = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--plan", planGlobex, "--forge", t.forge]); + expect(applyPlan.code, applyPlan.stderr).toBe(0); + expect(applyPlan.stdout).toContain("forge craftar.forge.yaml edited (schema: 2)"); + expect(applyPlan.stdout).toContain("section rule/review-posture reviewer-flavors"); + expect(applyPlan.stdout).toContain("default 4 lines"); + expect(applyPlan.stdout).toContain("6 lines (profile globex)"); + + // Check git status lists exactly the expected files. + const status3 = gitStatus(t.forge); + const statusLines = status3.trim().split("\n").filter(Boolean).sort(); + // Expected: manifest, rule.md, globex profile, owned recipe, and the deleted variant files. + expect(statusLines).toContainEqual(expect.stringContaining("craftar.forge.yaml")); + expect(statusLines).toContainEqual(expect.stringContaining("ingredients/rules/review-posture/rule.md")); + expect(statusLines).toContainEqual(expect.stringContaining("profiles/globex/profile.yaml")); + // The owned recipe could be base--globex.yaml. + expect(statusLines.some((l) => l.includes("recipes/") && l.includes("globex"))).toBe(true); + // The deleted variant files. + expect(statusLines.some((l) => l.includes("ingredients/rules/review-posture--globex/"))).toBe(true); + + // Regenerate expected Forge if requested. + if (process.env.CRAFTAR_REGEN_GOLDEN_TAKE_SECTION) { + await fs.rm(EXPECTED, { recursive: true, force: true }); + await copyTree(t.forge, EXPECTED); + } + expect(await treeFiles(t.forge)).toEqual(await treeFiles(EXPECTED)); + expect(await snapshot(t.forge)).toEqual(await snapshot(EXPECTED)); + gitCommitAll(t.forge, "unify review-posture for globex"); + + // Step 4: All three workspaces unchanged. + await assertNothingMoved(t); + + // Step 5: initech — fill the existing section. + const planInitech = path.join(t.tmp, "initech.yaml"); + const savePlanInitech = runCli(["forge", "unify", "rule/review-posture", "--profile", "initech", "--save-plan", planInitech, "--forge", t.forge]); + expect(savePlanInitech.code, savePlanInitech.stderr).toBe(0); + + const planInitechRaw = await fs.readFile(planInitech, "utf8"); + const planI = YAML.parse(planInitechRaw); + const ruleFileI = planI.files.find((f: { file: string }) => f.file === "rule.md"); + expect(ruleFileI).toBeDefined(); + // The hunks should touch the existing section and have name "reviewer-flavors" pre-filled. + for (const h of ruleFileI.hunks) { + expect(h.section?.name, "expected initech hunks to touch the existing section").toBe("reviewer-flavors"); + h.take = "section"; + } + await fs.writeFile(planInitech, YAML.stringify(planI)); + + const applyPlanInitech = runCli(["forge", "unify", "rule/review-posture", "--profile", "initech", "--plan", planInitech, "--forge", t.forge]); + expect(applyPlanInitech.code, applyPlanInitech.stderr).toBe(0); + expect(applyPlanInitech.stdout).toContain("existing"); + expect(applyPlanInitech.stdout).toContain("4 lines (profile initech)"); + // No manifest line (existing section, schema already 2). + expect(applyPlanInitech.stdout).not.toContain("craftar.forge.yaml edited"); + + // git status: only profile, owned recipe and deleted variant files (body unchanged). + const status5 = gitStatus(t.forge); + const statusLines5 = status5.trim().split("\n").filter(Boolean); + expect(statusLines5.some((l) => l.includes("profiles/initech/profile.yaml"))).toBe(true); + expect(statusLines5.some((l) => l.includes("recipes/") && l.includes("initech"))).toBe(true); + expect(statusLines5.some((l) => l.includes("ingredients/rules/review-posture--initech/"))).toBe(true); + // The body (rule.md) should NOT be in the status (unchanged). + expect(statusLines5.some((l) => l.includes("ingredients/rules/review-posture/rule.md"))).toBe(false); + gitCommitAll(t.forge, "unify review-posture for initech"); + + // Step 4 again: All three workspaces still unchanged. + await assertNothingMoved(t); + + // Step 6: Re-import all three — 0 created, 0 variants. + // Per spec 11 §6.15, Ruling 9: a re-import may re-point a profile from its owned recipe + // (base--

) to the shared recipe (base) when they become identical. Only profile.yaml + // may change; nothing else may move. + // Note: we do NOT use --write-config here because the workspace config is already set up + // and --write-config would overwrite the targets with only what import auto-detects (losing + // agents-md which was added manually in step 1). + const expectedStatus: Record = { + acme: [], // acme was already on base, nothing changes + globex: ["M profiles/globex/profile.yaml"], // re-points from base--globex to base + initech: ["M profiles/initech/profile.yaml"], // re-points from base--initech to base + }; + for (const p of PROFILES) { + // Don't use --write-config on re-import: the workspace config already exists. + const r = runCli(["import", "--from", "claude-code", "--forge", t.forge, "--profile", p, "--workspace", t.ws[p]]); + expect(r.code, r.stderr).toBe(0); + expect(r.stdout).toContain("0 created"); + expect(r.stdout).toContain("0 variants"); + // Assert exactly what git status reports for this profile's re-import. + const statusLines = gitStatus(t.forge).trim().split("\n").filter(Boolean); + expect(statusLines, `re-import ${p} should only change profile.yaml or nothing`).toEqual(expectedStatus[p]); + // Commit any profile re-pointing changes before the next re-import. + if (statusLines.length) gitCommitAll(t.forge, `re-import ${p}`); + } + + // After all three re-imports, all workspaces must still be unchanged. + await assertNothingMoved(t); + }); + + it("step 7, negative control: without globex's section value its files would change", { timeout: 180_000 }, async () => { + const t = await step1_importAndSync(); + + // Apply the globex unify (steps 2–3). + const planGlobex = path.join(t.tmp, "globex.yaml"); + const savePlan = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--save-plan", planGlobex, "--forge", t.forge]); + expect(savePlan.code, savePlan.stderr).toBe(0); + const plan = YAML.parse(await fs.readFile(planGlobex, "utf8")); + const ruleFile = plan.files.find((f: { file: string }) => f.file === "rule.md"); + for (const h of ruleFile.hunks) { + h.take = "section"; + h.section = { name: "reviewer-flavors", lines: "7-10" }; + } + await fs.writeFile(planGlobex, YAML.stringify(plan)); + const applyPlan = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--plan", planGlobex, "--forge", t.forge]); + expect(applyPlan.code, applyPlan.stderr).toBe(0); + gitCommitAll(t.forge, "unify review-posture for globex"); + + // Remove globex's reviewer-flavors section value. + const profPath = path.join(t.forge, "profiles/globex/profile.yaml"); + const profYaml = YAML.parse(await fs.readFile(profPath, "utf8")); + delete profYaml.sections; + await fs.writeFile(profPath, YAML.stringify(profYaml)); + + // Now globex's files should report update. + const states = statesOf(t.ws.globex); + expect(states[".claude/rules/review-posture.md"]).toBe("update"); + expect(states["AGENTS.md"]).toBe("update"); + // kiro is always a target, so the steering file must update. + expect(states[".kiro/steering/review-posture.md"]).toBe("update"); + }); +}); diff --git a/test/golden/forge-take-section-expected/craftar.forge.yaml b/test/golden/forge-take-section-expected/craftar.forge.yaml new file mode 100644 index 0000000..782a9cf --- /dev/null +++ b/test/golden/forge-take-section-expected/craftar.forge.yaml @@ -0,0 +1,3 @@ +name: forge +schema: 2 +description: Craftar Forge — shared harness ingredients, recipes and client profiles. diff --git a/test/golden/forge-take-section-expected/ingredients/rules/review-posture--initech/ingredient.yaml b/test/golden/forge-take-section-expected/ingredients/rules/review-posture--initech/ingredient.yaml new file mode 100644 index 0000000..3a640d2 --- /dev/null +++ b/test/golden/forge-take-section-expected/ingredients/rules/review-posture--initech/ingredient.yaml @@ -0,0 +1,10 @@ +type: rule +name: review-posture--initech +inclusion: always +file: rule.md +targets: "*" +tags: [] +origin: + workspace: initech + path: .claude/rules/review-posture.md +as: review-posture diff --git a/test/golden/forge-take-section-expected/ingredients/rules/review-posture--initech/rule.md b/test/golden/forge-take-section-expected/ingredients/rules/review-posture--initech/rule.md new file mode 100644 index 0000000..d000981 --- /dev/null +++ b/test/golden/forge-take-section-expected/ingredients/rules/review-posture--initech/rule.md @@ -0,0 +1,12 @@ +# Review posture + +Dispatch reviewers after every commit. + +## Reviewer flavors + +| Repo | Reviewer | +|---|---| +| `initech-api` | backend-reviewer | +| `initech-web` | frontend-reviewer | + +Never edit what a reviewer reads. diff --git a/test/golden/forge-take-section-expected/ingredients/rules/review-posture/ingredient.yaml b/test/golden/forge-take-section-expected/ingredients/rules/review-posture/ingredient.yaml new file mode 100644 index 0000000..f862b0f --- /dev/null +++ b/test/golden/forge-take-section-expected/ingredients/rules/review-posture/ingredient.yaml @@ -0,0 +1,9 @@ +type: rule +name: review-posture +inclusion: always +file: rule.md +targets: "*" +tags: [] +origin: + workspace: acme + path: .claude/rules/review-posture.md diff --git a/test/golden/forge-take-section-expected/ingredients/rules/review-posture/rule.md b/test/golden/forge-take-section-expected/ingredients/rules/review-posture/rule.md new file mode 100644 index 0000000..8b58067 --- /dev/null +++ b/test/golden/forge-take-section-expected/ingredients/rules/review-posture/rule.md @@ -0,0 +1,14 @@ +# Review posture + +Dispatch reviewers after every commit. + +## Reviewer flavors + + +| Repo | Reviewer | +|---|---| +| `acme-api` | backend-reviewer | +| `acme-web` | frontend-reviewer | + + +Never edit what a reviewer reads. diff --git a/test/golden/forge-take-section-expected/ingredients/rules/shared/ingredient.yaml b/test/golden/forge-take-section-expected/ingredients/rules/shared/ingredient.yaml new file mode 100644 index 0000000..069909b --- /dev/null +++ b/test/golden/forge-take-section-expected/ingredients/rules/shared/ingredient.yaml @@ -0,0 +1,9 @@ +type: rule +name: shared +inclusion: always +file: rule.md +targets: "*" +tags: [] +origin: + workspace: acme + path: .claude/rules/shared.md diff --git a/test/golden/forge-take-section-expected/ingredients/rules/shared/rule.md b/test/golden/forge-take-section-expected/ingredients/rules/shared/rule.md new file mode 100644 index 0000000..d793b91 --- /dev/null +++ b/test/golden/forge-take-section-expected/ingredients/rules/shared/rule.md @@ -0,0 +1,3 @@ +# Shared conventions + +Write commit messages in English. diff --git a/test/golden/forge-take-section-expected/profiles/acme/profile.yaml b/test/golden/forge-take-section-expected/profiles/acme/profile.yaml new file mode 100644 index 0000000..e9d61bd --- /dev/null +++ b/test/golden/forge-take-section-expected/profiles/acme/profile.yaml @@ -0,0 +1,17 @@ +name: acme +description: Imported from acme on 2026-10-02. +recipes: + - base +targets: + - claude-code + - kiro + - agents-md +language: {} +identity: {} +scm: {} +naming: {} +frontend: {} +executor: {} +integrations: {} +params: {} +repos: [] diff --git a/test/golden/forge-take-section-expected/profiles/globex/profile.yaml b/test/golden/forge-take-section-expected/profiles/globex/profile.yaml new file mode 100644 index 0000000..d761d59 --- /dev/null +++ b/test/golden/forge-take-section-expected/profiles/globex/profile.yaml @@ -0,0 +1,26 @@ +name: globex +description: Imported from globex on 2026-10-02. +recipes: + - base--globex +targets: + - claude-code + - kiro + - agents-md +language: {} +identity: {} +scm: {} +naming: {} +frontend: {} +executor: {} +integrations: {} +params: {} +repos: [] +sections: + rule/review-posture: + reviewer-flavors: | + | Repo | Reviewer | + |---|---| + | `acme-api` | backend-reviewer | + | `acme-web` | frontend-reviewer | + | `globex-desktop` | desktop-reviewer | + | `globex-worker` | worker-reviewer | diff --git a/test/golden/forge-take-section-expected/profiles/initech/profile.yaml b/test/golden/forge-take-section-expected/profiles/initech/profile.yaml new file mode 100644 index 0000000..bce9058 --- /dev/null +++ b/test/golden/forge-take-section-expected/profiles/initech/profile.yaml @@ -0,0 +1,17 @@ +name: initech +description: Imported from initech on 2026-10-02. +recipes: + - base--initech +targets: + - claude-code + - kiro + - agents-md +language: {} +identity: {} +scm: {} +naming: {} +frontend: {} +executor: {} +integrations: {} +params: {} +repos: [] diff --git a/test/golden/forge-take-section-expected/recipes/base--globex.yaml b/test/golden/forge-take-section-expected/recipes/base--globex.yaml new file mode 100644 index 0000000..7cc5d09 --- /dev/null +++ b/test/golden/forge-take-section-expected/recipes/base--globex.yaml @@ -0,0 +1,7 @@ +name: base--globex +description: Always-on conventions, commands, agents, scripts and MCP servers. +extends: [] +ingredients: + - rule/review-posture + - rule/shared +params: {} diff --git a/test/golden/forge-take-section-expected/recipes/base--initech.yaml b/test/golden/forge-take-section-expected/recipes/base--initech.yaml new file mode 100644 index 0000000..a563bc9 --- /dev/null +++ b/test/golden/forge-take-section-expected/recipes/base--initech.yaml @@ -0,0 +1,7 @@ +name: base--initech +description: Always-on conventions, commands, agents, scripts and MCP servers. +extends: [] +ingredients: + - rule/review-posture--initech + - rule/shared +params: {} diff --git a/test/golden/forge-take-section-expected/recipes/base.yaml b/test/golden/forge-take-section-expected/recipes/base.yaml new file mode 100644 index 0000000..184ca75 --- /dev/null +++ b/test/golden/forge-take-section-expected/recipes/base.yaml @@ -0,0 +1,7 @@ +name: base +description: Always-on conventions, commands, agents, scripts and MCP servers. +extends: [] +ingredients: + - rule/review-posture + - rule/shared +params: {} diff --git a/test/golden/take-section-acme/.claude/rules/review-posture.md b/test/golden/take-section-acme/.claude/rules/review-posture.md new file mode 100644 index 0000000..e44a0b1 --- /dev/null +++ b/test/golden/take-section-acme/.claude/rules/review-posture.md @@ -0,0 +1,12 @@ +# Review posture + +Dispatch reviewers after every commit. + +## Reviewer flavors + +| Repo | Reviewer | +|---|---| +| `acme-api` | backend-reviewer | +| `acme-web` | frontend-reviewer | + +Never edit what a reviewer reads. diff --git a/test/golden/take-section-acme/.claude/rules/shared.md b/test/golden/take-section-acme/.claude/rules/shared.md new file mode 100644 index 0000000..d793b91 --- /dev/null +++ b/test/golden/take-section-acme/.claude/rules/shared.md @@ -0,0 +1,3 @@ +# Shared conventions + +Write commit messages in English. diff --git a/test/golden/take-section-globex/.claude/rules/review-posture.md b/test/golden/take-section-globex/.claude/rules/review-posture.md new file mode 100644 index 0000000..b7eb438 --- /dev/null +++ b/test/golden/take-section-globex/.claude/rules/review-posture.md @@ -0,0 +1,14 @@ +# Review posture + +Dispatch reviewers after every commit. + +## Reviewer flavors + +| Repo | Reviewer | +|---|---| +| `acme-api` | backend-reviewer | +| `acme-web` | frontend-reviewer | +| `globex-desktop` | desktop-reviewer | +| `globex-worker` | worker-reviewer | + +Never edit what a reviewer reads. diff --git a/test/golden/take-section-globex/.claude/rules/shared.md b/test/golden/take-section-globex/.claude/rules/shared.md new file mode 100644 index 0000000..d793b91 --- /dev/null +++ b/test/golden/take-section-globex/.claude/rules/shared.md @@ -0,0 +1,3 @@ +# Shared conventions + +Write commit messages in English. diff --git a/test/golden/take-section-initech/.claude/rules/review-posture.md b/test/golden/take-section-initech/.claude/rules/review-posture.md new file mode 100644 index 0000000..d000981 --- /dev/null +++ b/test/golden/take-section-initech/.claude/rules/review-posture.md @@ -0,0 +1,12 @@ +# Review posture + +Dispatch reviewers after every commit. + +## Reviewer flavors + +| Repo | Reviewer | +|---|---| +| `initech-api` | backend-reviewer | +| `initech-web` | frontend-reviewer | + +Never edit what a reviewer reads. diff --git a/test/golden/take-section-initech/.claude/rules/shared.md b/test/golden/take-section-initech/.claude/rules/shared.md new file mode 100644 index 0000000..d793b91 --- /dev/null +++ b/test/golden/take-section-initech/.claude/rules/shared.md @@ -0,0 +1,3 @@ +# Shared conventions + +Write commit messages in English. diff --git a/test/helpers/regen-golden-take-section.ts b/test/helpers/regen-golden-take-section.ts new file mode 100644 index 0000000..07c59c1 --- /dev/null +++ b/test/helpers/regen-golden-take-section.ts @@ -0,0 +1,130 @@ +/** + * Regenerates test/golden/forge-take-section-expected/ from the committed input workspaces + * by running the real `craftar import` and `craftar forge unify` commands the way + * `test/golden-take-section.test.ts` does (spec 12 §10.5). + * + * Run this only after an intended change to the input workspaces or to section extraction, + * and review the resulting diff like code — never hand-edit the output to make the test pass. + * + * Usage: npx tsx test/helpers/regen-golden-take-section.ts + */ +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { execFileSync, spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import YAML from "yaml"; +import { listFiles } from "../../src/core/forge.js"; +import { importClaudeCode } from "../../src/importers/claude-code.js"; +import { TSX_LOADER } from "./tsx-loader.js"; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const REPO = path.resolve(HERE, "../.."); +const GOLDEN_ROOT = path.resolve(HERE, "../golden"); +const INPUT = { + acme: path.join(GOLDEN_ROOT, "take-section-acme"), + globex: path.join(GOLDEN_ROOT, "take-section-globex"), + initech: path.join(GOLDEN_ROOT, "take-section-initech"), +}; +const EXPECTED = path.join(GOLDEN_ROOT, "forge-take-section-expected"); +const PROFILES = ["acme", "globex", "initech"] as const; +type P = (typeof PROFILES)[number]; +const FIXED = new Date("2026-10-02T12:00:00Z"); + +// A local runCli, as in regen-golden-unify.ts. +function runCli(args: string[]): { code: number | null; stdout: string; stderr: string } { + const r = spawnSync(process.execPath, ["--import", TSX_LOADER, path.join(REPO, "src/cli.ts"), ...args], { + cwd: REPO, + encoding: "utf8", + env: { ...process.env, NO_COLOR: "1" }, + timeout: 60_000, + }); + if (r.error) throw r.error; + return { code: r.status, stdout: r.stdout, stderr: r.stderr }; +} + +const gitEnv = (): NodeJS.ProcessEnv => ({ + ...process.env, + GIT_AUTHOR_NAME: "craftar-test", + GIT_AUTHOR_EMAIL: "test@example.invalid", + GIT_COMMITTER_NAME: "craftar-test", + GIT_COMMITTER_EMAIL: "test@example.invalid", +}); +const gitCommitAll = (dir: string, message: string) => { + execFileSync("git", ["-C", dir, "add", "-A"]); + execFileSync("git", ["-C", dir, "commit", "-q", "-m", message], { env: gitEnv() }); +}; +const forgeFiles = async (dir: string) => (await listFiles(dir)).filter((rel) => rel !== ".git" && !rel.startsWith(".git/")); +async function copyTree(src: string, dst: string): Promise { + for (const rel of await forgeFiles(src)) { + await fs.mkdir(path.dirname(path.join(dst, rel)), { recursive: true }); + await fs.copyFile(path.join(src, rel), path.join(dst, rel)); + } +} +async function edit(file: string, f: (text: string) => string): Promise { + await fs.writeFile(file, f(await fs.readFile(file, "utf8"))); +} + +function addKiroAndAgentsMd(yaml: string): string { + const out = yaml.replace(/^(targets:\n(?: {2}- .*\n)+)/m, "$1 - kiro\n - agents-md\n"); + if (out === yaml) throw new Error("expected a block targets list"); + return out; +} + +const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "craftar-regen-take-section-")); +const forge = path.join(tmp, "forge"); +const ws: Record = { + acme: path.join(tmp, "acme"), + globex: path.join(tmp, "globex"), + initech: path.join(tmp, "initech"), +}; + +try { + // Step 1: Import all three at a fixed date. + // We use a hack to simulate a fixed date for import: set the description manually after. + for (const p of PROFILES) { + await copyTree(INPUT[p], ws[p]); + const r = await importClaudeCode({ workspaceRoot: ws[p], forgeRoot: forge, profileName: p, writeWorkspaceConfig: true }); + if (r.rejected.length > 0) throw new Error(`Import ${p} rejected: ${JSON.stringify(r.rejected)}`); + } + + // Append kiro and agents-md to profiles and workspaces. + for (const p of PROFILES) { + await edit(path.join(forge, "profiles", p, "profile.yaml"), addKiroAndAgentsMd); + await edit(path.join(ws[p], "craftar.yaml"), addKiroAndAgentsMd); + } + + execFileSync("git", ["init", "-q", forge]); + gitCommitAll(forge, "import acme, globex and initech"); + + // Sync workspaces. + for (const p of PROFILES) { + const r = runCli(["sync", "--workspace", ws[p]]); + if (r.code !== 0) throw new Error(`sync ${p} failed: ${r.stderr}`); + } + + // Step 2–3: Save plan for globex, edit it to take: section with lines, apply. + const planGlobex = path.join(tmp, "globex.yaml"); + const savePlan = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--save-plan", planGlobex, "--forge", forge]); + if (savePlan.code !== 0) throw new Error(`--save-plan globex failed: ${savePlan.stderr}`); + + const plan = YAML.parse(await fs.readFile(planGlobex, "utf8")); + const ruleFile = plan.files.find((f: { file: string }) => f.file === "rule.md"); + if (!ruleFile) throw new Error("expected rule.md in plan"); + for (const h of ruleFile.hunks) { + h.take = "section"; + h.section = { name: "reviewer-flavors", lines: "7-10" }; + } + await fs.writeFile(planGlobex, YAML.stringify(plan)); + + const applyPlan = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--plan", planGlobex, "--forge", forge]); + if (applyPlan.code !== 0) throw new Error(`--plan globex failed: ${applyPlan.stderr}`); + + // The expected Forge is the state after step 3 (globex unify), NOT after initech. + await fs.rm(EXPECTED, { recursive: true, force: true }); + await copyTree(forge, EXPECTED); + + console.log(`regenerated test/golden/forge-take-section-expected (${(await forgeFiles(EXPECTED)).length} files)`); +} finally { + await fs.rm(tmp, { recursive: true, force: true }); +} diff --git a/test/param-writes.test.ts b/test/param-writes.test.ts index d43eb89..f79742f 100644 --- a/test/param-writes.test.ts +++ b/test/param-writes.test.ts @@ -166,3 +166,162 @@ describe("checkParamWrites — section values cite keys too (spec 11 §6.11)", ( expect(await err(check(forge, [ext("k", "globex-api", "acme-api")]))).toBe("no error"); }); }); + +describe("checkParamWrites — sections (spec 12 §6.6)", () => { + const sectionExt = (name: string, existing: boolean, value: string, key = "rule/deploy", file = "rule.md"): import("../src/core/unify.js").SectionExtraction => ({ + key, + name, + file, + existing, + default: existing ? null : "default content\n", + value, + }); + + async function checkWithSections( + forge: Awaited>, + sections: import("../src/core/unify.js").SectionExtraction[], + extractions: Extraction[] = [], + p = "acme", + ) { + return checkParamWrites(forge, forge.ingredients.get("rule/deploy")!, forge.ingredients.get("rule/deploy--acme")!, p, extractions, sections); + } + + it("a new section: the profile gains sections.., sectionsWritten names it", async () => { + const forge = await forgeOf(spec()); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "| row |\n")]); + expect(w.profile).not.toBeNull(); + expect(YAML.parse(w.profile!.content).sections).toEqual({ "rule/deploy": { flavors: "| row |\n" } }); + expect(w.sectionsWritten).toEqual(["flavors"]); + }); + + it("a new section appended at end for a profile without sections (D8)", async () => { + const forge = await forgeOf(spec()); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + // The sections block is appended at the end + expect(w.profile!.content).toContain("sections:\n rule/deploy:\n flavors: |\n row\n"); + }); + + it("a new section into a profile with an existing sections block (block map)", async () => { + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"], ["claude-code"], { sections: { "rule/other": { x: "y\n" } } }), profile("globex", ["base"])] }), + ); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + const parsed = YAML.parse(w.profile!.content); + expect(parsed.sections).toEqual({ "rule/other": { x: "y\n" }, "rule/deploy": { flavors: "row\n" } }); + }); + + it("the manifest is rendered with schema: 2 for a new section (LF)", async () => { + const forge = await forgeOf(spec()); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + expect(w.manifest).not.toBeNull(); + expect(w.manifest!.content).toBe("name: test-forge\nschema: 2\n"); + expect(w.mustHold).toContain(w.manifest!.abs); + }); + + it("the manifest is rendered with schema: 2 for a new section (CRLF)", async () => { + const forge = await forgeOf(spec(), { "craftar.forge.yaml": "name: test-forge\r\nschema: 1\r\n" }); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + expect(w.manifest).not.toBeNull(); + expect(w.manifest!.content).toBe("name: test-forge\r\nschema: 2\r\n"); + }); + + it("params + a section in one call: one profile edit holding both", async () => { + const forge = await forgeOf(spec()); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")], [ext("k", "globex", "acme")]); + expect(w.profile).not.toBeNull(); + const parsed = YAML.parse(w.profile!.content); + expect(parsed.params).toEqual({ k: "acme" }); + expect(parsed.sections).toEqual({ "rule/deploy": { flavors: "row\n" } }); + }); + + it("S13 (another profile sets the name)", async () => { + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"]), profile("globex", ["base"], ["claude-code"], { sections: { "rule/deploy": { flavors: "other\n" } } })] }), + ); + expect(await err(checkWithSections(forge, [sectionExt("flavors", false, "row\n")]))).toContain( + "profile globex already sets section flavors of rule/deploy — it names no marker today and would start to apply", + ); + }); + + it("S14 for a new section (profile sets other content)", async () => { + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"], ["claude-code"], { sections: { "rule/deploy": { flavors: "other\n" } } }), profile("globex", ["base"])] }), + ); + expect(await err(checkWithSections(forge, [sectionExt("flavors", false, "row\n")]))).toContain("profile acme already sets section flavors of rule/deploy to other content"); + }); + + it("S14 for an existing section (profile sets other content)", async () => { + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"], ["claude-code"], { sections: { "rule/deploy": { flavors: "other\n" } } }), profile("globex", ["base"])] }), + ); + expect(await err(checkWithSections(forge, [sectionExt("flavors", true, "row\n")]))).toContain("profile acme already sets section flavors of rule/deploy to other content"); + }); + + it("equal case for new section: nothing written, not in sectionsWritten", async () => { + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"], ["claude-code"], { sections: { "rule/deploy": { flavors: "row\n" } } }), profile("globex", ["base"])] }), + ); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + expect(w.sectionsWritten).toEqual([]); + // When nothing else changes, no profile edit + expect(w.profile).toBeNull(); + }); + + it("equal case for existing section: nothing written, not in sectionsWritten", async () => { + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"], ["claude-code"], { sections: { "rule/deploy": { flavors: "row\n" } } }), profile("globex", ["base"])] }), + ); + const w = await checkWithSections(forge, [sectionExt("flavors", true, "row\n")]); + expect(w.sectionsWritten).toEqual([]); + }); + + it("S15: value cites {{k}} where base declares default and variant does not", async () => { + const forge = await forgeOf(spec({ ingredients: [] }), { + "ingredients/rules/deploy/ingredient.yaml": "type: rule\nname: deploy\nparams:\n k:\n default: x\n", + "ingredients/rules/deploy/rule.md": "use globex-api\n", + "ingredients/rules/deploy--acme/ingredient.yaml": "type: rule\nname: deploy--acme\nas: deploy\n", + "ingredients/rules/deploy--acme/rule.md": "use acme-api\n", + }); + expect(await err(checkWithSections(forge, [sectionExt("flavors", false, "see {{k}}\n")]))).toContain( + "unify: section flavors would render {{k}} through rule/deploy's default, where rule/deploy--acme renders it without", + ); + }); + + it("aliased sections → S16 via editYamlText alias refusal", async () => { + const forge = await forgeOf(spec(), { "profiles/acme/profile.yaml": "name: acme\nrecipes:\n - base--acme\nx: &s {}\nsections: *s\n" }); + expect(await err(checkWithSections(forge, [sectionExt("flavors", false, "row\n")]))).toContain("sections is an alias"); + }); + + it("hand-aligned manifest → S16 via manifestWithSections", async () => { + const forge = await forgeOf(spec(), { "craftar.forge.yaml": "name: test-forge # aligned\nschema: 1\n" }); + expect(await err(checkWithSections(forge, [sectionExt("flavors", false, "row\n")]))).toContain("cannot edit craftar.forge.yaml in place"); + }); + + it("schema: 2 already: no manifest edit", async () => { + const forge = await forgeOf(spec(), { "craftar.forge.yaml": "name: test-forge\nschema: 2\ndescription: d\n" }); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + expect(w.manifest).toBeNull(); + }); + + it("existing section only (no new section): no manifest edit even on schema: 1 Forge", async () => { + const forge = await forgeOf(spec()); + const w = await checkWithSections(forge, [sectionExt("flavors", true, "row\n")]); + expect(w.manifest).toBeNull(); + }); + + it("new section with value already in place: no profile edit, sectionsWritten empty, but manifest IS rendered with schema: 2", async () => { + // The bug: a NEW section (adding markers) whose value is already in the profile was not bumping the manifest. + // The manifest bump depends on whether the run ADDS markers (sections.some(!existing)), not on what is written to the profile. + const forge = await forgeOf( + spec({ profiles: [profile("acme", ["base--acme"], ["claude-code"], { sections: { "rule/deploy": { flavors: "row\n" } } }), profile("globex", ["base"])] }), + ); + const w = await checkWithSections(forge, [sectionExt("flavors", false, "row\n")]); + // No profile edit — the value is already in place + expect(w.profile).toBeNull(); + expect(w.sectionsWritten).toEqual([]); + // But the manifest IS edited, because the body gains markers → the Forge needs schema: 2 + expect(w.manifest).not.toBeNull(); + expect(w.manifest!.content).toContain("schema: 2"); + expect(w.mustHold).toContain(w.manifest!.abs); + }); +}); diff --git a/test/schema.test.ts b/test/schema.test.ts index 06a5953..24b5a4e 100644 --- a/test/schema.test.ts +++ b/test/schema.test.ts @@ -167,3 +167,104 @@ describe("sections and the manifest schema (spec 11 §5.2)", () => { await expect(loadForge(root)).rejects.toThrow(/invalid .*acme.profile\.yaml/); }); }); + +describe("plan section (spec 12)", () => { + /** Helper to build a plan with a hunk entry for testing. */ + const plan = (hunk: Record) => ({ + schema: 1, + base: "rule/w", + profile: "acme", + variant: "rule/w--acme", + baseFingerprint: "sha256:a", + variantFingerprint: "sha256:b", + files: [{ file: "rule.md", hunks: [{ hunk: 1, at: "lines 1–5", take: "section", ...hunk }] }], + }); + + it("accepts take: section with section: { name } and with lines: '-'", () => { + expect(UnifyPlanSchema.safeParse(plan({ section: { name: "flavors" } })).success).toBe(true); + expect(UnifyPlanSchema.safeParse(plan({ section: { name: "flavors", lines: "5-10" } })).success).toBe(true); + expect(UnifyPlanSchema.safeParse(plan({ section: { name: "flavors", lines: "1-1" } })).success).toBe(true); + expect(UnifyPlanSchema.safeParse(plan({ section: { name: "review-table-2", lines: "12-99" } })).success).toBe(true); + }); + + it("accepts take: section with no section object (S1 is the engine's refusal)", () => { + const noSection = plan({}); + delete (noSection.files[0].hunks[0] as Record).section; + expect(UnifyPlanSchema.safeParse(noSection).success).toBe(true); + }); + + it("accepts take: keep with a section field (the engine ignores it, the schema does not strip it)", () => { + const withKeep = plan({ take: "keep", section: { name: "flavors" } }); + const parsed = UnifyPlanSchema.parse(withKeep); + expect(parsed.files[0].hunks![0].take).toBe("keep"); + expect(parsed.files[0].hunks![0].section).toEqual({ name: "flavors" }); + }); + + it("refuses a name with a space", () => { + const r = UnifyPlanSchema.safeParse(plan({ section: { name: "Flavors Table" } })); + expect(r.success).toBe(false); + if (r.success) return; + expect(r.error.issues.some((i) => i.message.includes("slug-like"))).toBe(true); + }); + + it("refuses lines: '5' (no range)", () => { + const r = UnifyPlanSchema.safeParse(plan({ section: { name: "flavors", lines: "5" } })); + expect(r.success).toBe(false); + if (r.success) return; + expect(r.error.issues.some((i) => i.message.includes("-"))).toBe(true); + }); + + it("refuses lines: '0-3' (zero-based start)", () => { + const r = UnifyPlanSchema.safeParse(plan({ section: { name: "flavors", lines: "0-3" } })); + expect(r.success).toBe(false); + if (r.success) return; + expect(r.error.issues.some((i) => i.message.includes("-"))).toBe(true); + }); + + it("refuses lines: 'a-b' (non-numeric)", () => { + const r = UnifyPlanSchema.safeParse(plan({ section: { name: "flavors", lines: "a-b" } })); + expect(r.success).toBe(false); + if (r.success) return; + expect(r.error.issues.some((i) => i.message.includes("-"))).toBe(true); + }); + + it("refuses an unknown field inside section (PlanSectionSchema is strict)", () => { + const r = UnifyPlanSchema.safeParse(plan({ section: { name: "x", extra: 1 } })); + expect(r.success).toBe(false); + if (r.success) return; + expect(r.error.issues.some((i) => i.code === "unrecognized_keys")).toBe(true); + }); + + it("refuses a one-sided file entry with take: section (TakeSchema unchanged)", () => { + const oneSided = { + schema: 1, + base: "rule/w", + profile: "acme", + variant: "rule/w--acme", + baseFingerprint: "sha256:a", + variantFingerprint: "sha256:b", + files: [{ file: "x.md", onlyIn: "variant", take: "section" }], + }; + const r = UnifyPlanSchema.safeParse(oneSided); + expect(r.success).toBe(false); + if (r.success) return; + // TakeSchema is base | variant | keep — "section" is not valid there. + expect(r.error.issues.some((i) => i.path.includes("take"))).toBe(true); + }); + + it("an existing plan with take: param and params still parses exactly as before", () => { + const paramPlan = { + schema: 1, + base: "rule/w", + profile: "acme", + variant: "rule/w--acme", + baseFingerprint: "sha256:a", + variantFingerprint: "sha256:b", + files: [{ file: "rule.md", hunks: [{ hunk: 1, take: "param", params: [{ token: "globex-api", key: "deploy.api" }] }] }], + }; + const parsed = UnifyPlanSchema.parse(paramPlan); + expect(parsed.files[0].hunks![0].take).toBe("param"); + expect(parsed.files[0].hunks![0].params).toEqual([{ token: "globex-api", key: "deploy.api" }]); + expect(parsed.files[0].hunks![0]).not.toHaveProperty("section"); + }); +}); diff --git a/test/section-extract.test.ts b/test/section-extract.test.ts new file mode 100644 index 0000000..0a44bb4 --- /dev/null +++ b/test/section-extract.test.ts @@ -0,0 +1,1926 @@ +import { describe, expect, it } from "vitest"; +import { diffLines, type Hunk } from "../src/core/diff.js"; +import { deriveSections, prefillSections, type SectionRun, type MarkerInsertion, type HunkWithSuggestion } from "../src/core/section-extract.js"; +import type { PlanHunk } from "../src/schema/index.js"; + +/** + * Section extraction tests (spec 12 §10.1): deriveSections on synthetic bodies. + * Uses the §5.1 body pattern (review posture, reviewer table) with acme/globex/initech names. + */ + +const OPEN = (n: string) => ``; +const CLOSE = ""; + +const lines = (...xs: string[]) => xs.map((x) => x + "\n").join(""); + +/** The §5.1 body structure without markers. */ +const HEADER = ["# Review posture", "", "Dispatch reviewers after every commit.", ""]; +const FOOTER = ["", "Never edit what a reviewer reads."]; +const TABLE_ACME = ["| Repo | Reviewer |", "|---|---|", "| `acme-api` | backend-reviewer |"]; +const TABLE_GLOBEX = ["| Repo | Reviewer |", "|---|---|", "| `globex-api` | backend-reviewer |", "| `globex-web` | frontend-reviewer |"]; + +/** Build a body from header, table rows and footer. */ +const body = (rows: string[]) => lines(...HEADER, ...rows, ...FOOTER); + +/** Build a body with sections. */ +const bodyWithSection = (sectionName: string, rows: string[]) => + lines(...HEADER, OPEN(sectionName), ...rows, CLOSE, ...FOOTER); + +/** Plan entry helper. */ +const entry = (hunk: number, take: string, section?: { name: string; lines?: string }): PlanHunk => ({ + hunk, + at: "", + take: take as PlanHunk["take"], + ...(section ? { section } : {}), +}); + +/** Get hunks from two texts. */ +const getHunks = (base: string, variant: string): Hunk[] => diffLines(base, variant); + +const FILE = "rule.md"; +const LABEL = "ingredients/rules/review-posture/rule.md"; +const REF = "rule/review-posture"; + +const err = (fn: () => unknown): string => { + try { + fn(); + return "no error"; + } catch (e) { + return (e as Error).message; + } +}; + +describe("deriveSections — Q1: variant adds rows (block hunk, no base lines)", () => { + const base = body(TABLE_ACME); + const variant = body([...TABLE_ACME, "| `acme-web` | frontend-reviewer |", "| `acme-desktop` | desktop-reviewer |"]); + const hunks = getHunks(base, variant); + + it("without lines: empty span, default '', value = the two rows", () => { + const entries = [entry(1, "section", { name: "extras" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + expect(run.name).toBe("extras"); + expect(run.existing).toBe(false); + expect(run.default).toBe(""); + expect(run.value).toBe("| `acme-web` | frontend-reviewer |\n| `acme-desktop` | desktop-reviewer |\n"); + expect(run.from).toBe(run.to + 1); // empty span + + // Markers at the right positions + expect(result.markers).toHaveLength(2); + const opener = result.markers.find((m) => m.line.includes("craftar:section extras")); + const closer = result.markers.find((m) => m.line.includes("/craftar:section")); + expect(opener).toBeDefined(); + expect(closer).toBeDefined(); + // Same `at` for empty span + expect(opener!.at).toBe(closer!.at); + }); + + it("with lines over shared rows: default = shared table, value = whole variant table", () => { + // Lines covering the table header (line 5), separator (line 6), and shared row (line 7) + const entries = [entry(1, "section", { name: "flavors", lines: "5-7" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + expect(run.name).toBe("flavors"); + expect(run.existing).toBe(false); + expect(run.from).toBe(5); + expect(run.to).toBe(7); + expect(run.default).toBe(lines(...TABLE_ACME)); + // Value is the variant's table plus the two extra rows + expect(run.value).toBe(lines(...TABLE_ACME, "| `acme-web` | frontend-reviewer |", "| `acme-desktop` | desktop-reviewer |")); + }); +}); + +describe("deriveSections — Q2: two hunks (rows 1 and 3 differ, row 2 equal)", () => { + const base = lines("# Table", "| a |", "| b |", "| c |", "end"); + const variant = lines("# Table", "| A |", "| b |", "| C |", "end"); + const hunks = getHunks(base, variant); + + it("one run of both hunks: default = three base rows, value = three variant rows", () => { + // Two hunks: line 2 and line 4 + expect(hunks).toHaveLength(2); + const entries = [entry(1, "section", { name: "rows" }), entry(2, "section", { name: "rows" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + expect(run.hunks).toEqual([1, 2]); + expect(run.default).toBe("| a |\n| b |\n| c |\n"); + expect(run.value).toBe("| A |\n| b |\n| C |\n"); + }); +}); + +describe("deriveSections — Q3: existing section (base with flavors, variant adds row c)", () => { + const base = bodyWithSection("flavors", ["| a |", "| b |"]); + // Variant has rows a, b, c but no markers + const variantNoMarkers = body(["| a |", "| b |", "| c |"]); + const hunks = getHunks(base, variantNoMarkers); + + it("existing run from opener to closer; value = a, b, c rows; no markers inserted", () => { + // Find the hunks that touch the section + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "flavors" })); + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variantNoMarkers, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + expect(run.name).toBe("flavors"); + expect(run.existing).toBe(true); + expect(run.default).toBeNull(); + expect(run.value).toBe("| a |\n| b |\n| c |\n"); + expect(result.markers).toHaveLength(0); // No markers for existing section + }); +}); + +describe("deriveSections — Q4: existing section (variant rows q, r)", () => { + const base = bodyWithSection("flavors", ["| a |", "| b |"]); + const variantNoMarkers = body(["| q |", "| r |"]); + const hunks = getHunks(base, variantNoMarkers); + + it("existing; value = q, r", () => { + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "flavors" })); + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variantNoMarkers, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + expect(run.existing).toBe(true); + expect(run.value).toBe("| q |\n| r |\n"); + }); +}); + +describe("deriveSections — Q5: missing final newline (S10)", () => { + it("base ends without final newline → S10 base", () => { + const base = "x\n| a |"; // no final newline + const variant = "x\n| b |\n| c |\n"; + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "data" })]; + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("would reach a missing final newline in the base"); + }); + + it("variant ends without final newline → S10 variant", () => { + const base = "x\n| a |\n"; + const variant = "x\n| b |\n| c |"; // no final newline + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "data" })]; + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("would reach a missing final newline in the variant"); + }); +}); + +describe("deriveSections — S8: straddling hunk", () => { + it("hunk holds lines inside and outside section → S8", () => { + // Base has section around "| a |", variant also changes line right after closer + const base = lines("x", OPEN("data"), "| a |", CLOSE, "y"); + const variant = lines("x", "| b |", "z"); // Changed both inside and after section + const hunks = getHunks(base, variant); + + // The hunk spans lines that include both inside and outside the section + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "data" })); + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("holds lines inside and outside section data"); + }); +}); + +describe("deriveSections — S1: take: section without section object", () => { + it("throws S1 message", () => { + const base = "a\nb\n"; + const variant = "a\nc\n"; + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section")]; // No section field + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain('hunk 1 is take: section but names no section'); + }); +}); + +describe("deriveSections — S3: non-consecutive hunks", () => { + it("gap in hunk numbers → S3", () => { + const base = lines("a", "b", "c", "d", "e"); + const variant = lines("A", "b", "C", "d", "E"); + const hunks = getHunks(base, variant); + expect(hunks.length).toBe(3); + + // Hunks 1 and 3 for same section (gap at 2) + const entries = [entry(1, "section", { name: "data" }), entry(2, "base"), entry(3, "section", { name: "data" })]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("section data is not one run of consecutive hunks (hunks 1, 3)"); + }); +}); + +describe("deriveSections — S4: two different lines ranges", () => { + it("two hunks with different lines → S4", () => { + const base = lines("a", "b", "c", "d"); + const variant = lines("A", "B", "c", "d"); + const hunks = getHunks(base, variant); + + const entries = [ + entry(1, "section", { name: "data", lines: "1-2" }), + entry(2, "section", { name: "data", lines: "1-3" }), + ]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("section data has two ranges: 1-2, 1-3"); + }); +}); + +describe("deriveSections — S5: lines issues", () => { + it("out of range → S5", () => { + const base = lines("a", "b", "c"); + const variant = lines("A", "b", "c"); + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "data", lines: "1-10" })]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("lines 1-10 of section data is out of range"); + }); + + it("does not contain hunk → S5", () => { + const base = lines("a", "b", "c", "d", "e"); + const variant = lines("a", "b", "c", "D", "e"); + const hunks = getHunks(base, variant); + + // Hunk is at line 4, but lines only covers 1-2 + const entries = [entry(1, "section", { name: "data", lines: "1-2" })]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("does not contain hunk 1"); + }); + + it("covers a non-run hunk → S5", () => { + // Create three separate hunks by having equal lines between changes + const base = lines("a", "x", "b", "y", "c", "z", "d"); + const variant = lines("A", "x", "B", "y", "C", "z", "d"); + const hunks = getHunks(base, variant); + + // Should have 3 hunks at lines 1, 3, 5 + expect(hunks.length).toBe(3); + + // Only hunks 1 and 2 are section, but lines covers all 3 + const entries = [ + entry(1, "section", { name: "data", lines: "1-5" }), + entry(2, "section", { name: "data", lines: "1-5" }), + entry(3, "base"), + ]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("covers hunk 3, which is take: base"); + }); + + it("end anchor cuts a hunk → S5 (lines ends directly before a changed line)", () => { + // base: lines 1-10, where line 9 is equal and line 10 is changed + // Create a scenario: hunk 1 at line 5 (in section), hunk 2 at lines 10 (not in section) + const base = lines("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k"); + const variant = lines("a", "b", "c", "d", "E", "f", "g", "h", "i", "J", "k"); + const hunks = getHunks(base, variant); + + // Hunk 1 at line 5, hunk 2 at line 10 + expect(hunks.length).toBe(2); + expect(hunks[0].a.start).toBe(5); + expect(hunks[1].a.start).toBe(10); + + // Section spans lines 5-9, so end anchor is line 10 which is inside hunk 2 + const entries = [ + entry(1, "section", { name: "data", lines: "5-9" }), + entry(2, "base"), + ]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("lines 5-9 of section data cuts hunk 2"); + }); + + it("start anchor cuts a hunk → S5 (lines starts directly after a changed line)", () => { + // Create a scenario: hunk 1 at line 2 (not in section), hunk 2 at line 5 (in section) + const base = lines("a", "b", "c", "d", "e", "f", "g", "h"); + const variant = lines("a", "B", "c", "d", "E", "f", "g", "h"); + const hunks = getHunks(base, variant); + + // Hunk 1 at line 2, hunk 2 at line 5 + expect(hunks.length).toBe(2); + expect(hunks[0].a.start).toBe(2); + expect(hunks[1].a.start).toBe(5); + + // Section spans lines 3-5, so start anchor is line 2 which is inside hunk 1 + const entries = [ + entry(1, "base"), + entry(2, "section", { name: "data", lines: "3-5" }), + ]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("lines 3-5 of section data cuts hunk 1"); + }); + + it("S5 without lines: existing section covers a take: base hunk (Case D)", () => { + // Case D: existing section `flavors` in base (lines 3-7: opener, a, b, c, closer) + // Variant without markers: a, B, C (b and c changed) + // 2 hunks: hunk 1 removes opener, hunk 2 changes b→B, c→C and removes closer + // Plan: hunk 1 take: section name `flavors`, hunk 2 take: base + // Error: S5 "section flavors (lines 3-7) covers hunk 2, which is take: base" + const base = lines("H", "", OPEN("flavors"), "a", "b", "c", CLOSE, "", "F"); + const variant = lines("H", "", "a", "B", "C", "", "F"); + const hunks = getHunks(base, variant); + + // Should have 2 hunks + expect(hunks.length).toBe(2); + + // Hunk 1: removes opener (line 3) + // Hunk 2: changes b→B, c→C and removes closer (lines 5-7 in base) + + // Plan: hunk 1 section (name flavors), hunk 2 base + const entries = [ + entry(1, "section", { name: "flavors" }), + entry(2, "base"), + ]; + + const msg = err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + ); + + // S5 without lines: "section flavors (lines 3-7) covers hunk 2, which is take: base" + expect(msg).toContain("section flavors (lines "); + expect(msg).toContain("covers hunk"); + }); +}); + +describe("deriveSections — S6: name already declared", () => { + it("same file → S6", () => { + const base = bodyWithSection("flavors", ["| a |"]); + const variant = body(["| a |", "| b |"]); + const hunks = getHunks(base, variant); + + // Try to create a new section with same name as existing + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "flavors" })); + + // This should actually be detected as existing, let's use a different case + // New section name that matches existing + }); + + it("declared elsewhere → S6", () => { + const base = lines("a", "b", "c"); + const variant = lines("a", "B", "c"); + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "extras" })]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map([["extras", "other.md:5"]]), + }), + )).toContain("section extras is already declared in rule/review-posture (other.md:5)"); + }); +}); + +describe("deriveSections — S7: overlapping sections", () => { + it("new span over existing marker → S7", () => { + const base = lines("a", OPEN("data"), "x", CLOSE, "b", "c"); + const variant = lines("a", "x", "B", "C"); + const hunks = getHunks(base, variant); + + // Try to create a section that overlaps the existing one + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "other" })); + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("would overlap section data"); + }); + + it("existing run named differently → S7", () => { + const base = bodyWithSection("flavors", ["| a |"]); + const variant = body(["| b |"]); + const hunks = getHunks(base, variant); + + // Touch the section but name it differently + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "other" })); + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("would overlap section flavors"); + }); +}); + +describe("deriveSections — S7: empty span INSIDE another span (Case A)", () => { + // Case A: an empty span fully inside a non-empty span must be refused. + // base L1..L8, variant L1 L2 V3 L4 X L5 L6 L7 L8 + // Hunk 1 (line 3 change) section `p` with lines: "3-7" → span [3,7] + // Hunk 2 (insertion after line 4) section `e` without lines → empty span [5,4] + // The empty span [5,4] is INSIDE [3,7], not just touching. Should be S7. + + it("empty span inside non-empty span → S7 in both plan orders", () => { + const base = lines("L1", "L2", "L3", "L4", "L5", "L6", "L7", "L8"); + const variant = lines("L1", "L2", "V3", "L4", "X", "L5", "L6", "L7", "L8"); + const hunks = getHunks(base, variant); + + // Should have 2 hunks: line 3 change, insertion after line 4 + expect(hunks.length).toBe(2); + + // Plan with p first, e second + const entries1 = [ + entry(1, "section", { name: "p", lines: "3-7" }), + entry(2, "section", { name: "e" }), + ]; + const msg1 = err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries: entries1, + declaredElsewhere: new Map(), + }), + ); + expect(msg1).toContain("would overlap section"); + + // Plan with e first, p second (swapped order) + const entries2 = [ + entry(2, "section", { name: "e" }), + entry(1, "section", { name: "p", lines: "3-7" }), + ]; + const msg2 = err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries: entries2, + declaredElsewhere: new Map(), + }), + ); + expect(msg2).toContain("would overlap section"); + }); +}); + +describe("deriveSections — S7: empty span adjacent to another span (SF-B)", () => { + // SF-B bug: an empty span adjacent to another new span reaches S17 instead of S7, + // and the result depends on plan order. Fix: treat a shared boundary as overlap + // when either span is empty, and sort runs deterministically. + + it("empty span after non-empty span at the same boundary → S7 in both plan orders", () => { + // Base: L1, L2, L3, L4, L5, L6 (6 lines) + // Variant: V1, L2, L3, L4, X, L5, L6 + // Plan: + // hunk 1: section `p` with lines: "1-4" → span [1,4] + // hunk 2: section `e` without lines → empty span [5,4] (insertion after line 4) + // These spans share a boundary at line 4/5, should be refused. + const base = lines("L1", "L2", "L3", "L4", "L5", "L6"); + const variant = lines("V1", "L2", "L3", "L4", "X", "L5", "L6"); + const hunks = getHunks(base, variant); + + // Should have 2 hunks: line 1 change, insertion after line 4 + expect(hunks.length).toBe(2); + + // Plan with p first, e second (the original plan order issue) + const entries1 = [ + entry(1, "section", { name: "p", lines: "1-4" }), + entry(2, "section", { name: "e" }), + ]; + const msg1 = err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries: entries1, + declaredElsewhere: new Map(), + }), + ); + expect(msg1).toContain("would overlap section"); + + // Plan with e first, p second (swapped order) + const entries2 = [ + entry(2, "section", { name: "e" }), + entry(1, "section", { name: "p", lines: "1-4" }), + ]; + const msg2 = err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries: entries2, + declaredElsewhere: new Map(), + }), + ); + expect(msg2).toContain("would overlap section"); + }); + + it("two empty spans one base line apart → accepted (Case C)", () => { + // Case C: base a b c d, variant a X b Y c d (two pure insertions) + // Hunk 1 section `x` → empty span after line 1 [2,1], value X + // Hunk 2 section `y` → empty span after line 2 [3,2], value Y + // These are one base line apart and should NOT overlap. + const base = lines("a", "b", "c", "d"); + const variant = lines("a", "X", "b", "Y", "c", "d"); + const hunks = getHunks(base, variant); + + // Should have 2 hunks: insertion after line 1, insertion after line 2 + expect(hunks.length).toBe(2); + + // Plan: hunk 1 section x, hunk 2 section y + const entries = [ + entry(1, "section", { name: "x" }), + entry(2, "section", { name: "y" }), + ]; + + // Should succeed, NOT throw S7 + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + // Two runs, both empty spans + expect(result.runs).toHaveLength(2); + + const runX = result.runs.find((r) => r.name === "x"); + const runY = result.runs.find((r) => r.name === "y"); + + expect(runX).toBeDefined(); + expect(runX!.from).toBe(2); // after line 1 + expect(runX!.to).toBe(1); + expect(runX!.default).toBe(""); + expect(runX!.value).toBe("X\n"); + + expect(runY).toBeDefined(); + expect(runY!.from).toBe(3); // after line 2 + expect(runY!.to).toBe(2); + expect(runY!.default).toBe(""); + expect(runY!.value).toBe("Y\n"); + + // Markers: open x, close x (at position 1), then open y, close y (at position 2) + expect(result.markers).toHaveLength(4); + // opener x at=1 (0-based: before line 2 i.e. after line 1) + expect(result.markers[0]).toEqual({ at: 1, line: "" }); + expect(result.markers[1]).toEqual({ at: 1, line: "" }); + // opener y at=2 (0-based: before line 3 i.e. after line 2) + expect(result.markers[2]).toEqual({ at: 2, line: "" }); + expect(result.markers[3]).toEqual({ at: 2, line: "" }); + }); + + it("two empty spans at the SAME position → S7, in both plan orders", () => { + // diffLines cannot produce two hunks at one position, so split one real insertion hunk in two: + // both pure insertions sit after base line 1 (a.start 2, no base lines). + const baseText = "a\nb\nc\n"; + const variantText = "a\nX\nY\nb\nc\n"; + const [h] = diffLines(baseText, variantText); + expect(h.a.lines).toEqual([]); + expect(h.b.lines).toEqual(["X", "Y"]); + const hx: Hunk = { kind: "block", a: { start: h.a.start, lines: [] }, b: { start: h.b.start, lines: ["X"] } }; + const hy: Hunk = { kind: "block", a: { start: h.a.start, lines: [] }, b: { start: h.b.start + 1, lines: ["Y"] } }; + const ex: PlanHunk = { hunk: 1, at: "after line 1", take: "section", section: { name: "x" } }; + const ey: PlanHunk = { hunk: 2, at: "after line 1", take: "section", section: { name: "y" } }; + for (const entries of [[ex, ey], [ey, ex]]) { + expect(() => + deriveSections({ + file: "rule.md", + label: "ingredients/rules/r/rule.md", + ref: "rule/r", + baseText, + variantText, + hunks: [hx, hy], + entries, + declaredElsewhere: new Map(), + }), + ).toThrow("unify plan: section y would overlap section x (rule.md:2)"); + } + }); +}); + +describe("deriveSections — S9: existing section with wrong lines", () => { + it("lines given but not exact → S9", () => { + const base = bodyWithSection("flavors", ["| a |", "| b |"]); + const variant = body(["| c |", "| d |"]); + const hunks = getHunks(base, variant); + + // The section spans lines 5-8 (opener, two rows, closer) + const entries = hunks.map((_, i) => entry(i + 1, "section", { name: "flavors", lines: "5-10" })); + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("already spans lines"); + }); +}); + +describe("deriveSections — S12: variant with markers", () => { + it("column-0 marker in variant → S12", () => { + const base = lines("a", "b", "c"); + const variant = lines("a", OPEN("data"), "c"); + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "other" })]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("variant holds a section marker"); + }); + + it("near miss in variant → S12", () => { + const base = lines("a", "b", "c"); + const variant = lines("a", "", "c"); + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "other" })]; + + expect(err(() => + deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }), + )).toContain("variant holds a section marker"); + }); +}); + +describe("deriveSections — empty span cases", () => { + it("empty span in the middle of a file", () => { + const base = lines("a", "b", "c", "d"); + const variant = lines("a", "b", "x", "y", "c", "d"); + const hunks = getHunks(base, variant); + + // Pure addition after line 2 + expect(hunks.length).toBe(1); + expect(hunks[0].a.lines.length).toBe(0); + + const entries = [entry(1, "section", { name: "extras" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + expect(run.default).toBe(""); + expect(run.from).toBe(run.to + 1); // empty span + expect(run.value).toBe("x\ny\n"); + }); + + it("empty span at the end of a file with final newline", () => { + const base = lines("a", "b"); + const variant = lines("a", "b", "c", "d"); + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "extras" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + expect(result.runs[0].default).toBe(""); + expect(result.runs[0].value).toBe("c\nd\n"); + }); +}); + +describe("deriveSections — adjacent new and existing sections", () => { + it("processes both correctly", () => { + // Base has one section, variant adds content that becomes another + const base = lines("header", OPEN("first"), "x", CLOSE, "middle", "footer"); + // Variant changes middle and removes markers from first + const variant = lines("header", "x", "NEW", "footer"); + const hunks = getHunks(base, variant); + + // Find the hunks - one touches existing, one is new + // This tests that both can coexist in one file + // For simplicity, let's test a simpler case + }); +}); + +describe("deriveSections — CRLF and BOM handling", () => { + it("CRLF base and variant give same runs as LF", () => { + const baseLf = lines("a", "b", "c"); + const variantLf = lines("a", "B", "c"); + const baseCrlf = baseLf.replace(/\n/g, "\r\n"); + const variantCrlf = variantLf.replace(/\n/g, "\r\n"); + + const hunksLf = getHunks(baseLf, variantLf); + const hunksCrlf = getHunks(baseCrlf, variantCrlf); + + const entriesLf = [entry(1, "section", { name: "data" })]; + const entriesCrlf = [entry(1, "section", { name: "data" })]; + + const resultLf = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: baseLf, + variantText: variantLf, + hunks: hunksLf, + entries: entriesLf, + declaredElsewhere: new Map(), + }); + + const resultCrlf = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: baseCrlf, + variantText: variantCrlf, + hunks: hunksCrlf, + entries: entriesCrlf, + declaredElsewhere: new Map(), + }); + + // Runs should be equivalent (default/value are LF normalized) + expect(resultLf.runs.length).toBe(resultCrlf.runs.length); + expect(resultLf.runs[0].default).toBe(resultCrlf.runs[0].default); + expect(resultLf.runs[0].value).toBe(resultCrlf.runs[0].value); + }); + + it("BOM base and variant give same runs as non-BOM", () => { + const bom = "\uFEFF"; + const baseLf = lines("a", "b", "c"); + const variantLf = lines("a", "B", "c"); + const baseBom = bom + baseLf; + const variantBom = bom + variantLf; + + const hunksLf = getHunks(baseLf, variantLf); + const hunksBom = getHunks(baseBom, variantBom); + + const entriesLf = [entry(1, "section", { name: "data" })]; + const entriesBom = [entry(1, "section", { name: "data" })]; + + const resultLf = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: baseLf, + variantText: variantLf, + hunks: hunksLf, + entries: entriesLf, + declaredElsewhere: new Map(), + }); + + const resultBom = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: baseBom, + variantText: variantBom, + hunks: hunksBom, + entries: entriesBom, + declaredElsewhere: new Map(), + }); + + expect(resultLf.runs.length).toBe(resultBom.runs.length); + expect(resultLf.runs[0].default).toBe(resultBom.runs[0].default); + expect(resultLf.runs[0].value).toBe(resultBom.runs[0].value); + }); +}); + +describe("deriveSections — marker insertion order", () => { + it("opener before closer at same at for empty span", () => { + const base = lines("a", "b", "c"); + const variant = lines("a", "b", "x", "c"); + const hunks = getHunks(base, variant); + + const entries = [entry(1, "section", { name: "data" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.markers.length).toBe(2); + // For empty span, opener and closer at same position, opener first + expect(result.markers[0].line).toContain("craftar:section data"); + expect(result.markers[1].line).toContain("/craftar:section"); + }); +}); + +describe("deriveSections — D1: insertion-first and insertion-last cases (spec 12 §6.2, Ruling 1)", () => { + // Example from spec: base a b c d e f (one per line), variant with line inserted after a and e changed + // → hunks: 1 (insertion after line 1) and 2 (line 5), both take: section + // → span 2-5 (lines b..e), per spec 12 §6.2 and Ruling 1 + + it("insertion-first: pure insertion at N=1, change at line 5 → span 2-5", () => { + // Base: a, b, c, d, e, f (lines 1-6) + const base = lines("a", "b", "c", "d", "e", "f"); + // Variant: a, [inserted], b, c, d, E, f — insertion after a, change at e + const variant = lines("a", "inserted", "b", "c", "d", "E", "f"); + const hunks = getHunks(base, variant); + + // Expect two hunks: one pure insertion after line 1, one change at line 5 + expect(hunks.length).toBe(2); + // Hunk 1: pure insertion after line 1 + expect(hunks[0].a.lines.length).toBe(0); + // Hunk 2: line 5 changed + expect(hunks[1].a.start).toBe(5); + expect(hunks[1].a.lines.length).toBe(1); + + const entries = [entry(1, "section", { name: "t" }), entry(2, "section", { name: "t" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + // Spec 12 §6.2: insertion at N=1 contributes N+1=2 for 'from', N=1 for 'to' + // Hunk at line 5 contributes 5 for 'from' and 5 for 'to' + // min(2,5)=2, max(1,5)=5 → span 2-5 + expect(run.from).toBe(2); + expect(run.to).toBe(5); + // Default is base lines 2-5 (b, c, d, e) + expect(run.default).toBe("b\nc\nd\ne\n"); + // Value is variant lines between anchors (line 1 = a, line 6+offset = f) + // Anchors are base line 1 (before span) and base line 6 (after span) + // Variant segment is lines between a and f counterparts = inserted, b, c, d, E + expect(run.value).toBe("inserted\nb\nc\nd\nE\n"); + }); + + it("insertion-last: change at line 2, pure insertion after line 5 → span 2-5", () => { + // Base: a, b, c, d, e, f (lines 1-6) + const base = lines("a", "b", "c", "d", "e", "f"); + // Variant: a, B, c, d, e, [inserted], f — change at b, insertion after e + const variant = lines("a", "B", "c", "d", "e", "inserted", "f"); + const hunks = getHunks(base, variant); + + // Expect two hunks: one change at line 2, one pure insertion after line 5 + expect(hunks.length).toBe(2); + // Hunk 1: line 2 changed + expect(hunks[0].a.start).toBe(2); + expect(hunks[0].a.lines.length).toBe(1); + // Hunk 2: pure insertion after line 5 + expect(hunks[1].a.lines.length).toBe(0); + + const entries = [entry(1, "section", { name: "t" }), entry(2, "section", { name: "t" })]; + const result = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base, + variantText: variant, + hunks, + entries, + declaredElsewhere: new Map(), + }); + + expect(result.runs).toHaveLength(1); + const run = result.runs[0]; + // Hunk at line 2 contributes 2 for 'from' and 2 for 'to' + // Insertion at N=5 contributes N+1=6 for 'from', N=5 for 'to' + // min(2,6)=2, max(2,5)=5 → span 2-5 + expect(run.from).toBe(2); + expect(run.to).toBe(5); + // Default is base lines 2-5 (b, c, d, e) + expect(run.default).toBe("b\nc\nd\ne\n"); + // Value is variant lines between anchors + expect(run.value).toBe("B\nc\nd\ne\ninserted\n"); + }); + + it("linesSpecified field is set correctly in runs", () => { + // Test that the linesSpecified field is correctly set on section runs. + // When lines are explicitly specified: linesSpecified=true + // When span is computed from hunks: linesSpecified=false + + // Case 1: No explicit lines, span computed from hunks + const base1 = lines("a", "b", "c", "d", "e"); + const variant1 = lines("a", "B", "c", "D", "e"); + const hunks1 = getHunks(base1, variant1); + + // Expect 2 hunks at lines 2 and 4 + expect(hunks1.length).toBe(2); + + const entries1 = [ + entry(1, "section", { name: "t" }), + entry(2, "section", { name: "t" }), + ]; + const result1 = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base1, + variantText: variant1, + hunks: hunks1, + entries: entries1, + declaredElsewhere: new Map(), + }); + + expect(result1.runs).toHaveLength(1); + expect(result1.runs[0].linesSpecified).toBe(false); + // Span computed from hunks at 2 and 4: from=2, to=4 + expect(result1.runs[0].from).toBe(2); + expect(result1.runs[0].to).toBe(4); + + // Case 2: Explicit lines specified + const entries2 = [ + entry(1, "section", { name: "t", lines: "1-5" }), + entry(2, "section", { name: "t", lines: "1-5" }), + ]; + const result2 = deriveSections({ + file: FILE, + label: LABEL, + ref: REF, + baseText: base1, + variantText: variant1, + hunks: hunks1, + entries: entries2, + declaredElsewhere: new Map(), + }); + + expect(result2.runs).toHaveLength(1); + expect(result2.runs[0].linesSpecified).toBe(true); + // Span explicitly set to 1-5 + expect(result2.runs[0].from).toBe(1); + expect(result2.runs[0].to).toBe(5); + }); +}); + + +// ==================================================================== +// proveSections tests (spec 12 §6.5) +// ==================================================================== + +import { proveSections } from "../src/core/section-extract.js"; + +const OPEN_TAG = (n: string) => ``; +const CLOSE_TAG = ""; + +describe("proveSections (spec 12 §6.5)", () => { + const FILE = "rule.md"; + const LABEL = "ingredients/rules/review-posture/rule.md"; + const REF = "rule/review-posture"; + + describe("new section around a table", () => { + // Template T has markers, mBase is base side (markers removed, default content), mVar is variant side + // + // IMPORTANT: The segment AFTER the closer starts with what follows the closer LINE, not the closer TAG. + // So if footer = "\n\nNever...", after the closer "\n", what remains is "\nNever...". + // When constructing mBase/mVar, we must match what expandSections produces. + + const header = "# Review posture\n\nDispatch reviewers.\n\n"; + const tableBase = "| Repo | Reviewer |\n|---|---|\n| `acme-api` | backend |\n"; + const tableVar = "| Repo | Reviewer |\n|---|---|\n| `acme-api` | backend |\n| `acme-web` | frontend |\n"; + const footerInTemplate = "\n\nNever edit.\n"; // placed after CLOSE_TAG + const footerAfterExpand = "\nNever edit.\n"; // what remains after closer line is consumed + + // Template: markers around the table + // Structure: header + OPEN + "\n" + tableBase + CLOSE + footerInTemplate + // The CLOSE + "\n\n" means closer line is "\n" and then "\nNever..." + const template = header + OPEN_TAG("flavors") + "\n" + tableBase + CLOSE_TAG + footerInTemplate; + + // mBase: must match expandSections(parse(template), {}) = header + tableBase (default) + footerAfterExpand + const mBase = header + tableBase + footerAfterExpand; + // mVar: must match expandSections(parse(template), {flavors: tableVar}) = header + tableVar + footerAfterExpand + const mVar = header + tableVar + footerAfterExpand; + + it("passes when template renders base via defaults and variant via values", () => { + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: ["flavors"], + values: { flavors: tableVar }, + profileValues: {}, + extractions: [], + }), + ).not.toThrow(); + }); + + it("S17 variant side: wrong value (one row changed)", () => { + const wrongValue = "| Repo | Reviewer |\n|---|---|\n| `acme-api` | backend |\n| `WRONG` | WRONG |\n"; + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: ["flavors"], + values: { flavors: wrongValue }, // Wrong value + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the variant side/); + }); + + it("S17 base side: template default differs from mBase by one line", () => { + const wrongTemplate = header + OPEN_TAG("flavors") + "\n" + "| DIFFERENT |\n" + CLOSE_TAG + footerInTemplate; + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template: wrongTemplate, + mBase, + mVar, + newNames: ["flavors"], + values: { flavors: tableVar }, + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the base side/); + }); + }); + + describe("existing section reused", () => { + // For existing section: template T has markers, mBase has markers too (taken base preserves them), + // mVar has no markers (variant side expanded) + // + // Same footer issue: what follows the closer LINE, not the closer TAG + + const header = "# Posture\n\n"; + const footerInTemplate = "\n\nEnd.\n"; + const footerAfterExpand = "\nEnd.\n"; + const sectionDefault = "| a |\n| b |\n"; + const sectionVar = "| a |\n| b |\n| c |\n"; + + // Template: base with markers (existing section, not new) + const template = header + OPEN_TAG("flavors") + "\n" + sectionDefault + CLOSE_TAG + footerInTemplate; + // mBase: same as template (markers present, default content inside) + const mBase = template; + // mVar: expanded with values = header + sectionVar + footerAfterExpand (no markers) + const mVar = header + sectionVar + footerAfterExpand; + + it("passes with profileValues {} and values { flavors: variant rows }", () => { + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: [], // existing, not new + values: { flavors: sectionVar }, + profileValues: {}, + extractions: [], + }), + ).not.toThrow(); + }); + + it("passes with profileValues holding another section (a second section the plan does not name)", () => { + // Base has two sections, plan only touches 'flavors', 'extra' stays via profileValues + // For the proof: template and mBase both have markers for both sections + // mVar has markers only for 'extra' (taken base), content for 'flavors' (taken variant) + const extraDefault = "extra content\n"; + const footerInTemplateTwo = "\nfinal.\n"; + const footerAfterExpandTwo = "final.\n"; // after "\n" comes "final.\n" + + // Note: between the two sections we have "\n\nmiddle\n\n" followed by the opener for extra + const templateTwo = + header + OPEN_TAG("flavors") + "\n" + sectionDefault + CLOSE_TAG + "\n\nmiddle\n\n" + OPEN_TAG("extra") + "\n" + extraDefault + CLOSE_TAG + footerInTemplateTwo; + const mBaseTwo = templateTwo; + // mVar: flavors expanded to sectionVar (no markers), extra still has markers (taken base) + // After flavors closer: "\nmiddle\n\n" (closer consumes one \n from the "\n\nmiddle...") + // So mVar = header + sectionVar + "\nmiddle\n\n" + OPEN_TAG("extra") + "\n" + extraDefault + CLOSE_TAG + footerInTemplateTwo + const mVarTwo = + header + sectionVar + "\nmiddle\n\n" + OPEN_TAG("extra") + "\n" + extraDefault + CLOSE_TAG + footerInTemplateTwo; + + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template: templateTwo, + mBase: mBaseTwo, + mVar: mVarTwo, + newNames: [], // existing + values: { flavors: sectionVar }, + profileValues: {}, // extra not in profileValues since mVar still has markers for it + extractions: [], + }), + ).not.toThrow(); + }); + }); + + describe("combined with a param key", () => { + // T holds {{deploy.api}} outside the section and the section around a table + // Key insight: mBase and mVar must match what expandSections produces from the template. + // + // The closer TAG "" plus footerInTemplate "\nEnd.\n" gives the line: + // "\n" and then "End.\n" + // So after expansion, the text after the section is "End.\n" + + const header = "Deploy `{{deploy.api}}` first.\n\n"; + const tableBase = "| a |\n"; + const tableVar = "| b |\n"; + const footerInTemplate = "\nEnd.\n"; // This follows the closer TAG + const footerAfterExpand = "End.\n"; // The closer LINE consumes the leading \n + + // Template: has section markers and the {{deploy.api}} placeholder + const template = header + OPEN_TAG("flavors") + "\n" + tableBase + CLOSE_TAG + footerInTemplate; + // mBase: must match expandSections(pT, {}) = header + tableBase + footerAfterExpand + const mBase = header + tableBase + footerAfterExpand; + // mVar: must match expandSections(pT, values) = header + tableVar + footerAfterExpand + const mVar = header + tableVar + footerAfterExpand; + + it("passes with D/V given", () => { + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: ["flavors"], + values: { flavors: tableVar }, + profileValues: {}, + extractions: [{ key: "deploy.api", default: "acme-api", value: "globex-api", sites: [], reused: false }], + }), + ).not.toThrow(); + }); + + it("fails with mismatched V", () => { + // Use mVarDifferent that has different content, causing variant side mismatch + const mVarDifferent = header + "| WRONG |\n" + footerAfterExpand; + + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar: mVarDifferent, // Has "| WRONG |" but values says tableVar + newNames: ["flavors"], + values: { flavors: tableVar }, + profileValues: {}, + extractions: [{ key: "deploy.api", default: "acme-api", value: "globex-api", sites: [], reused: false }], + }), + ).toThrow(/would not reproduce the variant side/); + }); + }); + + describe("template markers do not parse (unterminated)", () => { + it("S17 with the problem", () => { + const badTemplate = "# Header\n\n" + OPEN_TAG("flavors") + "\n| a |\n"; // no closer + const mBase = "# Header\n\n| a |\n"; + const mVar = "# Header\n\n| b |\n"; + + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template: badTemplate, + mBase, + mVar, + newNames: ["flavors"], + values: { flavors: "| b |\n" }, + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the base side.*is never closed/); + }); + }); + + describe("placeholder check (spec 09 §6.2 (b))", () => { + it("value that adds a {{title}} placeholder the variant did not have → P7 wording", () => { + // Construct a case where steps 3-4 pass but step 5 fails + // The value adds a placeholder that wasn't in mVar + // + // For step 5 to fail: others(tV) != others(mV) + // After expandSections, tV will have the value's placeholders, mV will have the original + // + // To make steps 3-4 pass: + // - Base side: expandSections(pT, {}) with D should equal mB with D + // - Variant side: expandSections(pT, values) with V should equal mV with V + // + // The trick: make mV have the same text as expandSections(pT, values) but differ only in placeholders. + // This is hard because placeholders are part of the text. + // + // Actually, looking at step 5 more carefully: + // others(tB) = placeholders in expandSections(pT, {}) that are not in K + // others(mB) = placeholders in mB that are not in K + // For the test to work, these must be equal (step 3-4 pass), but then + // others(tV) = placeholders in expandSections(pT, values) that are not in K + // others(mV) = placeholders in mV that are not in K + // For step 5 to fail, these must differ. + // + // The value itself contains the new placeholder, so expandSections puts it in tV. + // If mV doesn't have that placeholder but has the same text otherwise, step 4 fails first. + // + // To isolate step 5: we need the text to match but placeholders to differ. + // This is only possible if the placeholder in the value renders to the same text + // as what's in mV through some substitution - but we're testing with K being the param keys, + // and the {{title}} is outside K. + // + // Actually, the test description says "(Construct mVar consistently so that steps 3–4 pass + // and only step 5 fails, or explain in a comment why it is unreachable and test the reachable path.)" + // + // It's unreachable: if the value adds {{title}} and mVar doesn't have it, + // expandSections(pT, values) will have "{{title}}" literally in the text, + // and mV won't, so step 4's text comparison fails before step 5. + // + // Let's test the reachable path: step 4 fails when value introduces new placeholder. + + const template = "# Header\n\n" + OPEN_TAG("data") + "\ncontent\n" + CLOSE_TAG + "\nEnd.\n"; + const mBase = "# Header\n\ncontent\nEnd.\n"; + const mVar = "# Header\n\nother\nEnd.\n"; // no {{title}} + + // Value that introduces {{title}} - this will cause step 4 to fail (not step 5) + // because the text won't match + const valueWithPlaceholder = "{{title}} in section\n"; + + // The error will be "would not reproduce the variant side" because: + // tV = "# Header\n\n{{title}} in section\nEnd.\n" + // mV = "# Header\n\nother\nEnd.\n" + // These don't match even after V substitution (V is empty since no param extractions) + + // So we test that this path is caught by the variant side check + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: ["data"], + values: { data: valueWithPlaceholder }, + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the variant side/); + + // Note: The P7 wording ("would change which {{…}} placeholders") is unreachable + // in isolation for this scenario because the text mismatch is caught first in step 4. + // The placeholder check in step 5 guards against cases where text matches but + // placeholders differ, which happens when a param key substitution masks the difference. + }); + + // The placeholder branch (step 5 of proveSections) IS reachable via applyPlan when a param + // extraction wraps a placeholder in extra braces (e.g., `{{k}}` with token "k" → key "who" + // produces `{{{{who}}}}`). See test/unify.test.ts "P7 via section proof" for the pinned case. + }); + + describe("case that passes earlier rows and fails only at S17", () => { + // spec 12 §4.6: "a failure that reaches S17 without an earlier row is a bug in the rows, and a test case" + // This tests a case that passes S1-S12 checks (those are in deriveSections) and fails at the proof step. + // + // The proof can fail because: + // 1. parseSections fails on template/mBase/mVar + // 2. Structure mismatch (names don't match) + // 3. Base side render mismatch + // 4. Variant side render mismatch + // 5. Placeholder mismatch + // + // A case reaching S17 means deriveSections passed but proveSections fails. + // The subtlest case is when the texts look right but a tiny difference causes the proof to fail. + + it("subtle base side mismatch: whitespace difference", () => { + // Template with markers + const template = "# Head\n\n" + OPEN_TAG("data") + "\n| a |\n" + CLOSE_TAG + "\n\nFoot.\n"; + // mBase has slightly different whitespace (two newlines vs one in a spot) + const mBase = "# Head\n\n| a |\n\n\nFoot.\n"; // extra newline + const mVar = "# Head\n\n| b |\n\nFoot.\n"; + + // This passes S1-S12 (no marker issues, valid structure) but fails the base side proof + // because expandSections(pT, {}) gives "| a |\n" as default, but mBase has extra newline + + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: ["data"], + values: { data: "| b |\n" }, + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the base side/); + }); + }); + + describe("structure check", () => { + it("newNames entry not in template → S17 base side", () => { + // Template has no section, but newNames claims one + const template = "# Header\n\nContent.\n\nEnd.\n"; + const mBase = "# Header\n\nContent.\n\nEnd.\n"; + const mVar = "# Header\n\nOther.\n\nEnd.\n"; + + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template, + mBase, + mVar, + newNames: ["flavors"], // not in template + values: { flavors: "Other.\n" }, + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the base side/); + }); + + it("template has extra section not in base → S17 base side", () => { + // Template has a section, but mBase also has it as existing (should be in base too) + const template = "# Header\n\n" + OPEN_TAG("flavors") + "\n| a |\n" + CLOSE_TAG + "\n\nEnd.\n"; + // mBase has no section at all + const mBase = "# Header\n\n| a |\n\nEnd.\n"; + const mVar = "# Header\n\n| b |\n\nEnd.\n"; + + // Structure check: names(pT) - newNames should equal names(pB) + // pT has ["flavors"], newNames = ["flavors"], so [] should equal names(pB) = [] + // That passes structure. But the base side proof should fail because + // expandSections(pT, {}) = "# Header\n\n| a |\n\nEnd.\n" + // mB = "# Header\n\n| a |\n\nEnd.\n" + // These are equal, so it should pass! + // + // Let's construct a case where the structure check actually fails: + // Template has 2 sections, newNames has 1, base has 0 + + const template2 = "# H\n\n" + OPEN_TAG("a") + "\nx\n" + CLOSE_TAG + "\n" + OPEN_TAG("b") + "\ny\n" + CLOSE_TAG + "\nE.\n"; + const mBase2 = "# H\n\nx\ny\nE.\n"; // no sections + const mVar2 = "# H\n\nX\nY\nE.\n"; + + // names(pT) = ["a", "b"], newNames = ["a"], so remaining = ["b"] + // names(pB) = [], so ["b"] != [] → structure mismatch + + expect(() => + proveSections({ + file: FILE, + label: LABEL, + ref: REF, + template: template2, + mBase: mBase2, + mVar: mVar2, + newNames: ["a"], + values: { a: "X\n", b: "Y\n" }, + profileValues: {}, + extractions: [], + }), + ).toThrow(/would not reproduce the base side/); + }); + }); +}); + + +/* ------------------------------------------------------------------ */ +/* prefillSections (spec 12 §4.2) */ +/* ------------------------------------------------------------------ */ + +/** + * Helper to create a minimal hunk for prefillSections testing. + * When `baseLines` is empty, the hunk is a pure addition (no base lines). + */ +function prefillHunk( + start: number, + baseLines: string[], + suggestionClass: "evolution" | "value" | "block", +): HunkWithSuggestion { + return { + a: { start, lines: baseLines }, + suggestion: { class: suggestionClass }, + }; +} + +describe("prefillSections (spec 12 §4.2)", () => { + describe("heading slug", () => { + it("slugifies `## Reviewer flavors — São Paulo` to `reviewer-flavors-sao-paulo`", () => { + // Heading: "Reviewer flavors — São Paulo" (with em-dash and accented char) + const text = lines("## Reviewer flavors — São Paulo", "", "| a | b |"); + const hunks = [prefillHunk(3, [], "block")]; // after line 2 (blank line) + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe("reviewer-flavors-sao-paulo"); + }); + + it("cuts the slug at 40 characters", () => { + // Create a heading that will produce a slug > 40 chars + const heading = "## This Is A Very Long Heading That Should Be Cut At Forty Characters"; + const text = lines(heading, "body"); + const hunks = [prefillHunk(2, ["body"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + // Expected slug: "this-is-a-very-long-heading-that-should" (39 chars, then cut) + const slug = result[0]!; + expect(slug.length).toBeLessThanOrEqual(40); + expect(slug).toMatch(/^[a-z0-9-]+$/); + expect(slug).not.toMatch(/-$/); // no trailing dash + }); + + it("fallbacks to `section-` when no heading above", () => { + const text = lines("body line"); + const hunks = [prefillHunk(1, ["body line"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe("section-1"); + }); + + it("fallbacks to `section-` when heading produces empty or invalid slug (only symbols)", () => { + // Heading with only symbols that all become "-" + const text = lines("## ---???---", "body"); + const hunks = [prefillHunk(2, ["body"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + // "---???---" → "-" repeated → trimmed → empty + expect(result[0]).toBe("section-1"); + }); + }); + + describe("existing section name wins over block", () => { + it("returns the existing section's name when a hunk touches it", () => { + const text = lines("# Heading", "", OPEN("flavors"), "| table |", CLOSE, ""); + // Hunk touching lines 3-5 (the section area) + const hunks = [prefillHunk(4, ["| table |"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(["flavors"]), + }); + expect(result[0]).toBe("flavors"); + }); + + it("returns the existing section's name for a pure-addition hunk inside a section", () => { + const text = lines("# Heading", "", OPEN("flavors"), "| a |", CLOSE, ""); + // Pure addition after line 3 (opener is line 3, closer is line 5) + // Position N = 4 - 1 = 3, opener=3, closer=5: opener <= 3 < closer? 3 <= 3 < 5 → yes + const hunks = [prefillHunk(4, [], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(["flavors"]), + }); + expect(result[0]).toBe("flavors"); + }); + }); + + describe("value and evolution hunks get nothing", () => { + it("returns undefined for a `value` hunk", () => { + const text = lines("## Config", "key = acme-api"); + const hunks = [prefillHunk(2, ["key = acme-api"], "value")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBeUndefined(); + }); + + it("returns undefined for an `evolution` hunk", () => { + const text = lines("## Intro", "Old text here"); + const hunks = [prefillHunk(2, ["Old text here"], "evolution")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBeUndefined(); + }); + }); + + describe("uniqueness suffixes", () => { + it("appends `-2` when the name is in `declared`", () => { + const text = lines("## Flavors", "| a |"); + const hunks = [prefillHunk(2, ["| a |"], "block")]; + const declared = new Set(["flavors"]); + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared, + }); + expect(result[0]).toBe("flavors-2"); + }); + + it("appends `-2` for a non-consecutive repeat", () => { + const text = lines("## Table", "row1", "middle", "row2"); + // Two block hunks under the same heading, but with a gap (hunk 2 is evolution) + const hunks = [ + prefillHunk(2, ["row1"], "block"), + prefillHunk(3, ["middle"], "evolution"), + prefillHunk(4, ["row2"], "block"), + ]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + // hunk 0: "table" + // hunk 1: undefined (evolution) + // hunk 2: "table" is already used and not consecutive → "table-2" + expect(result[0]).toBe("table"); + expect(result[1]).toBeUndefined(); + expect(result[2]).toBe("table-2"); + }); + + it("consecutive block hunks keep the same name", () => { + const text = lines("## Table", "row1", "row2"); + const hunks = [ + prefillHunk(2, ["row1"], "block"), + prefillHunk(3, ["row2"], "block"), + ]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe("table"); + expect(result[1]).toBe("table"); + }); + }); + + describe("probe Q3 (edge case 4)", () => { + it("both touching hunks get the existing section's name", () => { + // Base: section `flavors` around rows `a`, `b` + // Two hunks: one holding opener, one holding closer against row c + const text = lines("# Heading", OPEN("flavors"), "| a |", "| b |", CLOSE, "footer"); + // Hunk 1: touches opener (line 2) - this is the marker line itself + // Hunk 2: touches closer (line 5) with variant having extra row + const hunks = [ + prefillHunk(2, [OPEN("flavors")], "block"), // touches opener line 2 + prefillHunk(5, [CLOSE], "block"), // touches closer line 5 + ]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(["flavors"]), + }); + // Both hunks touch the `flavors` section (opener=2, closer=5) + expect(result[0]).toBe("flavors"); + expect(result[1]).toBe("flavors"); + }); + }); + + describe("parse error handling", () => { + it("returns all undefined when base has malformed markers", () => { + // Near miss: will cause a parse error + const text = lines("## Heading", "", "body"); + const hunks = [prefillHunk(3, ["body"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBeUndefined(); + }); + + it("returns all undefined when section is never closed", () => { + const text = lines("## Heading", OPEN("broken"), "body"); + const hunks = [prefillHunk(3, ["body"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBeUndefined(); + }); + }); + + describe("heading search position", () => { + it("for a hunk with base lines, searches above a.start", () => { + // Heading on line 2, hunk starts at line 4 + const text = lines("intro", "## My Section", "blank", "content"); + const hunks = [prefillHunk(4, ["content"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe("my-section"); + }); + + it("for a pure-addition hunk, searches at or above line N", () => { + // Heading on line 2, pure addition after line 2 (N=2) + const text = lines("intro", "## Config"); + const hunks = [prefillHunk(3, [], "block")]; // after line 2 + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe("config"); + }); + + it("finds the nearest heading, not the first one", () => { + const text = lines("## First", "a", "## Second", "b"); + const hunks = [prefillHunk(4, ["b"], "block")]; // under Second + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe("second"); + }); + }); + + describe("multiple heading levels", () => { + it("recognizes H1 through H6", () => { + for (let level = 1; level <= 6; level++) { + const prefix = "#".repeat(level); + const text = lines(`${prefix} Level ${level}`, "body"); + const hunks = [prefillHunk(2, ["body"], "block")]; + const result = prefillSections({ + baseText: text, + label: LABEL, + ref: REF, + hunks, + declared: new Set(), + }); + expect(result[0]).toBe(`level-${level}`); + } + }); + }); +}); diff --git a/test/unify.test.ts b/test/unify.test.ts index 6395de2..834db44 100644 --- a/test/unify.test.ts +++ b/test/unify.test.ts @@ -9,7 +9,7 @@ import { applyPlan, metaDifferences, planFrom, rewriteRecipes, writeUnified } fr import { prove } from "../src/core/extract.js"; import { diffIngredients } from "../src/core/variants.js"; import { HunkSuggestionSchema, UnifyPlanSchema, type UnifyPlan } from "../src/schema/index.js"; -import { makeForge, profile, recipe, rule, tmpDir, writeFiles, type ForgeSpec } from "./helpers/forge.js"; +import { makeForge, profile, recipe, rule, tmpDir, writeFiles, type ForgeSpec, type IngredientSpec } from "./helpers/forge.js"; import { runCli } from "./helpers/cli.js"; const execFileP = promisify(execFile); @@ -116,6 +116,38 @@ describe("planFrom", () => { expect(plan.files.find((f) => f.file === "gone.md")).toMatchObject({ onlyIn: "base", take: "keep" }); expect(plan.files.find((f) => f.file === "extra.md")).toMatchObject({ onlyIn: "variant", take: "keep" }); }); + + it("pre-fills a section name from a heading for a block hunk (spec 12 §4.2)", async () => { + // Base has a heading "## Reviewer Table" above the table rows + // Variant adds extra rows (block hunk) + const baseDir = await tmpDir(); + cleanups.push(() => fs.rm(baseDir, { recursive: true, force: true })); + await writeFiles(baseDir, { + "ingredient.yaml": "type: rule\nname: review-posture\n", + "rule.md": "# Rules\n\n## Reviewer Table\n| a |\n", + }); + + const variantDir = await tmpDir(); + cleanups.push(() => fs.rm(variantDir, { recursive: true, force: true })); + await writeFiles(variantDir, { + "ingredient.yaml": "type: rule\nname: review-posture--acme\nas: review-posture\n", + "rule.md": "# Rules\n\n## Reviewer Table\n| a |\n| b |\n", + }); + + const base = { ref: "rule/review-posture", dir: baseDir, meta: { type: "rule", name: "review-posture" } } as never; + const variant = { ref: "rule/review-posture--acme", dir: variantDir, meta: { type: "rule", name: "review-posture--acme", as: "review-posture" } } as never; + const diff = await diffIngredients(base, variant); + + const plan = await planFrom(base, variant, diff, "acme"); + const paired = plan.files.find((f) => f.file === "rule.md")!; + + // The hunk should be classified as "block" (only in variant) + expect(paired.hunks![0].suggestion?.class).toBe("block"); + // The section name should be pre-filled from the heading "Reviewer Table" → "reviewer-table" + expect(paired.hunks![0].section).toEqual({ name: "reviewer-table" }); + // take should still be "keep" + expect(paired.hunks![0].take).toBe("keep"); + }); }); /** Two temp ingredient directories (base + variant--profile), loaded and diffed for real. */ @@ -786,7 +818,7 @@ describe("U1 — a merge never changes section markers (spec 11 §6.12, Ruling 8 const r = runCli(["forge", "unify", "rule/review-posture", "--profile", "globex", "--plan", planPath, "--forge", root]); expect(r.code).toBe(1); expect(r.stderr).toContain( - "unify: ingredients/rules/review-posture/rule.md would lose or change section markers (the result has malformed markers — line 5: section flavors is never closed) — take base for the marker lines; editing sections through unify is not supported yet", + "unify: ingredients/rules/review-posture/rule.md would lose or change section markers (the result has malformed markers — line 5: section flavors is never closed) — take base for the marker lines, or take: section to fill the section", ); expect(await porcelain(root)).toBe(""); }); @@ -819,3 +851,482 @@ describe("U1 — a merge never changes section markers (spec 11 §6.12, Ruling 8 expect(await e(applyPlan(rm.base, rm.variant, rm.diff, rmPlan))).toContain("the file would be removed with sections n"); }); }); + +describe("take: section (spec 12)", () => { + // Helper to create a basic Forge scenario for section tests + const sectionScenario = async (baseBody: string, variantBody: string) => { + const root = await tmpDir("craftar-section-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + await makeForge(root, { + ingredients: [ + rule("review-posture", baseBody), + rule("review-posture--acme", variantBody, { as: "review-posture" }), + ], + recipes: [recipe("base", ["rule/review-posture"]), recipe("base--acme", ["rule/review-posture--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + const forge = await loadForge(root); + const base = forge.ingredients.get("rule/review-posture")!; + const variant = forge.ingredients.get("rule/review-posture--acme")!; + const diffResult = await diffIngredients(base, variant); + return { root, forge, base, variant, diff: diffResult }; + }; + + it("a new section over a table whose variant has extra rows: template holds markers", async () => { + const baseBody = "# Review\n\nDispatch reviewers.\n\n| Repo | Reviewer |\n|---|---|\n| `api` | bob |\n\nDone.\n"; + const variantBody = "# Review\n\nDispatch reviewers.\n\n| Repo | Reviewer |\n|---|---|\n| `api` | bob |\n| `web` | alice |\n\nDone.\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + // variant has extra row; the hunk is a block hunk (lines only in variant) + const planObj = await planFrom(base, variant, diff, "acme"); + // Set take: section with lines covering the whole table (lines 5-7) + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "flavors", lines: "5-7" }; + + const result = await applyPlan(base, variant, diff, planObj); + expect(result.resolved).toBe(true); + + const merged = result.write["rule.md"] as string; + expect(merged).toContain(""); + expect(merged).toContain(""); + // The markers should wrap the table + const expected = `# Review + +Dispatch reviewers. + + +| Repo | Reviewer | +|---|---| +| \`api\` | bob | + + +Done. +`; + expect(merged).toBe(expected); + + expect(result.sections).toHaveLength(1); + expect(result.sections[0].key).toBe("rule/review-posture"); + expect(result.sections[0].name).toBe("flavors"); + expect(result.sections[0].existing).toBe(false); + expect(result.sections[0].default).toBe("| Repo | Reviewer |\n|---|---|\n| `api` | bob |\n"); + expect(result.sections[0].value).toBe("| Repo | Reviewer |\n|---|---|\n| `api` | bob |\n| `web` | alice |\n"); + }); + + it("CRLF + BOM base: markers written with CRLF, BOM kept", async () => { + const BOM = String.fromCharCode(0xfeff); + const baseBody = BOM + "# Review\r\n\r\nTable:\r\n\r\n| a |\r\n\r\nDone.\r\n"; + const variantBody = BOM + "# Review\r\n\r\nTable:\r\n\r\n| b |\r\n\r\nDone.\r\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "t" }; + + const result = await applyPlan(base, variant, diff, planObj); + const merged = result.write["rule.md"] as string; + + // Check BOM is kept + expect(merged.charCodeAt(0)).toBe(0xfeff); + // Check CRLF is used for markers + expect(merged).toContain("\r\n"); + expect(merged).toContain("\r\n"); + }); + + it("reuse of an existing section (probe Q3 shape): result.write has no rule.md, existing=true", async () => { + // Base already has the section markers + const baseBody = "# Review\n\n\n| a |\n| b |\n\n\nDone.\n"; + // Variant has no markers, but content is a|b|c + const variantBody = "# Review\n\n| a |\n| b |\n| c |\n\nDone.\n"; + const { root, base, variant, diff } = await sectionScenario(baseBody, variantBody); + // The variant's text differs from the base in the rows, resulting in hunks touching the section + const planObj = await planFrom(base, variant, diff, "acme"); + // Both hunks touch the existing section, set them to take: section + for (const h of planObj.files[0].hunks!) { + h.take = "section"; + (h as Record).section = { name: "flavors" }; + } + + const result = await applyPlan(base, variant, diff, planObj); + + // Body unchanged (only value is written to profile, not the file) + expect(result.write["rule.md"]).toBeUndefined(); + expect(result.sections).toHaveLength(1); + expect(result.sections[0].existing).toBe(true); + expect(result.sections[0].value).toBe("| a |\n| b |\n| c |\n"); + }); + + // SF1: a reuse-only plan on a mixed-EOL base must not round-trip through mergeFile, which would + // normalize the line endings even though the body does not change (spec 12 §6.7 step 3). + it("leaves a mixed-EOL base untouched on a reuse-only section plan", async () => { + // Base has mixed EOL (CRLF first line, then LF) with existing section markers + const baseBody = "# T\r\n\n\n| a |\n\n\nEnd.\n"; + // Variant has no markers, different content + const variantBody = "# T\r\n\n| b |\n\nEnd.\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + // Set take: section on all hunks (reuse, no new section) + for (const h of planObj.files[0].hunks!) { + h.take = "section"; + (h as Record).section = { name: "flavors" }; + } + + const result = await applyPlan(base, variant, diff, planObj); + + // Body unchanged: write has no rule.md (spec 12 §6.7 step 3) + expect(result.write["rule.md"]).toBeUndefined(); + expect(result.sections).toHaveLength(1); + expect(result.sections[0].existing).toBe(true); + }); + + it("a plan mixing take: param and take: section in one file: both proved", async () => { + const baseBody = "# Review\n\nDeploy to acme-api.\n\n| Repo |\n|---|\n| x |\n\nEnd.\n"; + const variantBody = "# Review\n\nDeploy to globex-api.\n\n| Repo |\n|---|\n| y |\n\nEnd.\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + + // First hunk is "acme-api" -> "globex-api" (param) + planObj.files[0].hunks![0].take = "param"; + (planObj.files[0].hunks![0] as Record).params = [{ token: "acme-api", key: "deploy.api" }]; + + // Second hunk is table row change (section) + planObj.files[0].hunks![1].take = "section"; + (planObj.files[0].hunks![1] as Record).section = { name: "repos" }; + + const result = await applyPlan(base, variant, diff, planObj); + expect(result.resolved).toBe(true); + expect(result.params).toHaveLength(1); + expect(result.params[0].key).toBe("deploy.api"); + expect(result.sections).toHaveLength(1); + expect(result.sections[0].name).toBe("repos"); + + const merged = result.write["rule.md"] as string; + expect(merged).toContain("{{deploy.api}}"); + expect(merged).toContain(""); + }); + + it("S2: section hunk on file not expanded by emitter is refused", async () => { + // Use a script with a non-TEXT_EXT file extension (.bin is not in the regex) + const root = await tmpDir("craftar-s2-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + + // Create script ingredients manually with proper structure + // Scripts require `files` to list the file names + // Using .bin which is NOT in TEXT_EXT (/\.(md|txt|json|ya?ml|ps1|py|sh|js|ts|cjs|mjs|toml|xml|csv)$/i) + const script = (name: string, body: Record, extra: Record = {}): IngredientSpec => ({ + meta: { type: "script", name, files: Object.keys(body), ...extra }, + files: body, + }); + + await makeForge(root, { + ingredients: [ + script("deploy", { "run.bin": "echo a\n" }), + script("deploy--acme", { "run.bin": "echo b\n" }, { as: "deploy" }), + ], + recipes: [recipe("base", ["script/deploy"]), recipe("base--acme", ["script/deploy--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + const forge = await loadForge(root); + const base = forge.ingredients.get("script/deploy")!; + const variant = forge.ingredients.get("script/deploy--acme")!; + const diffResult = await diffIngredients(base, variant); + const planObj = await planFrom(base, variant, diffResult, "acme"); + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "s" }; + + await expect(applyPlan(base, variant, diffResult, planObj)).rejects.toThrow( + 'unify plan: "run.bin" is copied without expansion by a target that emits it — a section marker there would be emitted literally', + ); + }); + + it("S3 second form: one name in two files is refused", async () => { + const root = await tmpDir("craftar-s3-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + + // Create skill ingredients manually with proper structure + const skill = (name: string, body: Record, extra: Record = {}): IngredientSpec => ({ + meta: { type: "skill", name, layout: "dir", ...extra }, + files: body, + }); + + await makeForge(root, { + ingredients: [ + skill("analyze", { "SKILL.md": "a\n", "notes.md": "x\n" }), + skill("analyze--acme", { "SKILL.md": "b\n", "notes.md": "y\n" }, { as: "analyze" }), + ], + recipes: [recipe("base", ["skill/analyze"]), recipe("base--acme", ["skill/analyze--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + const forge = await loadForge(root); + const base = forge.ingredients.get("skill/analyze")!; + const variant = forge.ingredients.get("skill/analyze--acme")!; + const diffResult = await diffIngredients(base, variant); + const planObj = await planFrom(base, variant, diffResult, "acme"); + + // Set both files to use section with the same name + for (const pf of planObj.files) { + if (pf.hunks) { + pf.hunks[0].take = "section"; + (pf.hunks[0] as Record).section = { name: "same-name" }; + } + } + + await expect(applyPlan(base, variant, diffResult, planObj)).rejects.toThrow( + /section same-name is named in ".*" and ".*"/, + ); + }); + + it("S11: a keep left with section hunk is refused", async () => { + // Need a base with two differences so we get two hunks + const baseBody = "# Review\n\na\nb\nc\nd\n"; + const variantBody = "# Review\n\nx\nb\nc\ny\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + + // Ensure we have at least 2 hunks + expect(planObj.files[0].hunks!.length).toBeGreaterThanOrEqual(2); + + // First hunk is section, second is keep + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "s" }; + planObj.files[0].hunks![1].take = "keep"; + + await expect(applyPlan(base, variant, diff, planObj)).rejects.toThrow( + "unify plan: take: section needs the variant resolved in the same plan — 1 decision(s) still keep", + ); + }); + + it("S11: ingredient.yaml differs with section hunk is refused", async () => { + const root = await tmpDir("craftar-s11-meta-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + await makeForge(root, { + ingredients: [ + rule("review-posture", "a\n", { tags: ["x"] }), + rule("review-posture--acme", "b\n", { as: "review-posture", tags: ["y"] }), + ], + recipes: [recipe("base", ["rule/review-posture"]), recipe("base--acme", ["rule/review-posture--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + const forge = await loadForge(root); + const base = forge.ingredients.get("rule/review-posture")!; + const variant = forge.ingredients.get("rule/review-posture--acme")!; + const diffResult = await diffIngredients(base, variant); + const planObj = await planFrom(base, variant, diffResult, "acme"); + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "s" }; + + await expect(applyPlan(base, variant, diffResult, planObj)).rejects.toThrow( + /unify plan: take: section needs the variant resolved in the same plan — ingredient.yaml differs in/, + ); + }); + + it("S12: variant with a marker in another admitted file is refused", async () => { + const root = await tmpDir("craftar-s12-"); + cleanups.push(() => fs.rm(root, { recursive: true, force: true })); + + // Create skill ingredients manually with proper structure + const skill = (name: string, body: Record, extra: Record = {}): IngredientSpec => ({ + meta: { type: "skill", name, layout: "dir", ...extra }, + files: body, + }); + + await makeForge(root, { + ingredients: [ + skill("analyze", { "SKILL.md": "a\n", "notes.md": "x\n" }), + skill("analyze--acme", { + "SKILL.md": "b\n", + "notes.md": "\ny\n\n", + }, { as: "analyze" }), + ], + recipes: [recipe("base", ["skill/analyze"]), recipe("base--acme", ["skill/analyze--acme"])], + profiles: [profile("acme", ["base--acme"])], + }); + const forge = await loadForge(root); + const base = forge.ingredients.get("skill/analyze")!; + const variant = forge.ingredients.get("skill/analyze--acme")!; + const diffResult = await diffIngredients(base, variant); + const planObj = await planFrom(base, variant, diffResult, "acme"); + // Set section on SKILL.md only + for (const pf of planObj.files) { + if (pf.file === "SKILL.md" && pf.hunks) { + pf.hunks[0].take = "section"; + (pf.hunks[0] as Record).section = { name: "s" }; + } else if (pf.hunks) { + pf.hunks[0].take = "base"; // resolve the other file + } + } + + await expect(applyPlan(base, variant, diffResult, planObj)).rejects.toThrow( + /skill\/analyze--acme holds a section marker on notes.md:1/, + ); + }); + + it("U1: --take variant over a marked base is still refused", async () => { + // This test ensures that taking variant on a file with markers is still refused + const baseBody = "# Review\n\n\n| a |\n\n\nDone.\n"; + const variantBody = "# Review\n\n| b |\n\nDone.\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + // Take variant on all hunks (no section declaration) + for (const h of planObj.files[0].hunks!) { + h.take = "variant"; + } + + await expect(applyPlan(base, variant, diff, planObj)).rejects.toThrow( + /would lose or change section markers.*take base for the marker lines, or take: section to fill the section/, + ); + }); + + it("U1: plan with section hunk that also takes variant on an existing marker hunk is refused", async () => { + // Base has two sections + const baseBody = "# Review\n\n\na\n\n\n\nb\n\n"; + // Variant has neither marker + const variantBody = "# Review\n\nc\n\nd\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + // There should be hunks touching both sections + // Take section on one, take variant on the other (dropping its markers) + if (planObj.files[0].hunks!.length >= 2) { + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "s1" }; + planObj.files[0].hunks![1].take = "variant"; + } + + await expect(applyPlan(base, variant, diff, planObj)).rejects.toThrow( + /would lose or change section markers/, + ); + }); + + it("a plan with no section hunk behaves exactly as before", async () => { + // Simple base/variant diff with take: base + const baseBody = "a\nb\nc\n"; + const variantBody = "a\nx\nc\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + const planObj = await planFrom(base, variant, diff, "acme"); + planObj.files[0].hunks![0].take = "base"; + + const result = await applyPlan(base, variant, diff, planObj); + expect(result.resolved).toBe(true); + expect(result.sections).toEqual([]); + // No changes when taking base on a simple diff + expect(result.write["rule.md"]).toBeUndefined(); + }); + + it("P7 via section proof: a param that wraps a placeholder in more braces is refused at the proof (step 5)", async () => { + // SF-A: The reviewer's scenario that reaches proveSections step 5. + // Base has {{k}} which the param hunk extracts as token "k" → key "who". + // deriveHunk produces `Hello {{{{who}}}}.` because the tokenizer splits `{{k}}` and + // only the changed region `k` is templated. The section proof then sees that the + // template has `{{{{who}}}}` which is a different placeholder set than the original. + const baseBody = "Hello {{k}}.\n\n| a |\n\nEnd.\n"; + const variantBody = "Hello {{y}}.\n\n| b |\n\nEnd.\n"; + const { root, base, variant, diff } = await sectionScenario(baseBody, variantBody); + + const planObj = await planFrom(base, variant, diff, "acme"); + + // Hunk 1: param extraction for {{k}} → {{y}} + planObj.files[0].hunks![0].take = "param"; + (planObj.files[0].hunks![0] as Record).params = [{ token: "k", key: "who" }]; + + // Hunk 2: section extraction for the table + planObj.files[0].hunks![1].take = "section"; + (planObj.files[0].hunks![1] as Record).section = { name: "t" }; + + // This should fail at step 5 of proveSections: placeholders differ + await expect(applyPlan(base, variant, diff, planObj)).rejects.toThrow( + /would change which \{\{…\}\} placeholders the text holds/, + ); + + // Verify Forge is untouched + const ruleFile = path.join(root, "ingredients/rules/review-posture/rule.md"); + expect(await fs.readFile(ruleFile, "utf8")).toBe(baseBody); + }); + + it("D1: insertion-first run computes span correctly — markers wrap b..e", async () => { + // D1 bug: insertion-first case where run starts with pure insertion and ends with a change + // Base: a b c d e f (lines 1-6) + // Variant: a [inserted] b c d E f — insertion after a, change at e + // Span should be 2-5 (b..e), markers around b through e + const baseBody = "a\nb\nc\nd\ne\nf\n"; + const variantBody = "a\ninserted\nb\nc\nd\nE\nf\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + + // Should have 2 hunks: hunk 1 = insertion after line 1, hunk 2 = line 5 changed + expect(diff.files[0].hunks.length).toBe(2); + + const planObj = await planFrom(base, variant, diff, "acme"); + // Set both hunks to section with same name + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "t" }; + planObj.files[0].hunks![1].take = "section"; + (planObj.files[0].hunks![1] as Record).section = { name: "t" }; + + const result = await applyPlan(base, variant, diff, planObj); + expect(result.resolved).toBe(true); + expect(result.sections).toHaveLength(1); + expect(result.sections[0].name).toBe("t"); + + const merged = result.write["rule.md"] as string; + // Markers should wrap lines b, c, d, e (span 2-5) + const expected = `a + +b +c +d +e + +f +`; + expect(merged).toBe(expected); + + // Default is base lines 2-5 (b, c, d, e) + expect(result.sections[0].default).toBe("b\nc\nd\ne\n"); + // Value is variant lines between anchors (inserted, b, c, d, E) + expect(result.sections[0].value).toBe("inserted\nb\nc\nd\nE\n"); + }); + + it("Case C: two empty sections one base line apart — accepted via applyPlan", async () => { + // Case C: base a b c d, variant a X b Y c d (two pure insertions) + // Two sections: x after line 1 (empty span), y after line 2 (empty span) + // These are one base line apart and should NOT overlap. + const baseBody = "a\nb\nc\nd\n"; + const variantBody = "a\nX\nb\nY\nc\nd\n"; + const { base, variant, diff } = await sectionScenario(baseBody, variantBody); + + // Should have 2 hunks: insertion after line 1, insertion after line 2 + expect(diff.files[0].hunks.length).toBe(2); + + const planObj = await planFrom(base, variant, diff, "acme"); + // First hunk: section x + planObj.files[0].hunks![0].take = "section"; + (planObj.files[0].hunks![0] as Record).section = { name: "x" }; + // Second hunk: section y + planObj.files[0].hunks![1].take = "section"; + (planObj.files[0].hunks![1] as Record).section = { name: "y" }; + + const result = await applyPlan(base, variant, diff, planObj); + expect(result.resolved).toBe(true); + expect(result.sections).toHaveLength(2); + + const sectionX = result.sections.find((s) => s.name === "x"); + const sectionY = result.sections.find((s) => s.name === "y"); + + expect(sectionX).toBeDefined(); + expect(sectionX!.default).toBe(""); + expect(sectionX!.value).toBe("X\n"); + + expect(sectionY).toBeDefined(); + expect(sectionY!.default).toBe(""); + expect(sectionY!.value).toBe("Y\n"); + + const merged = result.write["rule.md"] as string; + // Markers: open x, close x before line 2; open y, close y before line 3 + const expected = `a + + +b + + +c +d +`; + expect(merged).toBe(expected); + }); +});