From 8f9345a8bc62ec31ebc6f4779a7abd3fb8f29cf1 Mon Sep 17 00:00:00 2001 From: Brenno Ferrari Date: Tue, 2 Jun 2026 00:04:36 +0200 Subject: [PATCH 1/6] fix(merge): don't render copy-origin files in three-way merge (#132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `shardmind update` ran computeMergeAction over every modified managed file and rendered both sides through Nunjucks (differ.ts). But in v6 only `.njk` files are templates; copy-origin files (scripts, .test.ts, JSON) are verbatim. Rendering them (a) crashes on a literal `{{` that isn't a valid expression (the reported crash) and (b) silently substitutes any real `{{ expr }}` they contain as data. Add a `literal` flag to ComputeMergeActionInput; when set the merge runs on the raw bytes (base=oldTemplate, ours=newTemplate, no renderString). update-planner passes literal=copyFromSourcePath!==undefined — copy-origin actions already carry that marker, so no new plumbing. The renderer stays strict, so genuine `.njk` authoring errors keep failing loudly (unlike a renderer-level catch, which would mask them). Fixtures-first: new merge fixture 21 (copy-origin file with `garbage{{` + `{{ user_name }}`) authored + confirmed RED (rendered → RENDER_TEMPLATE_ERROR) before the fix; the harness threads scenario.copy_origin → literal and bumps the fixture count to 21. Unit tests: literal merges raw + preserves both literals, literal still detects real conflicts, and a `.njk` syntax error STILL throws RENDER_TEMPLATE_ERROR (regression guard). Root cause reported via #129. Binary copy files through the utf-8 merge remain tracked by #63. Co-Authored-By: Claude Opus 4.8 (1M context) --- source/core/differ.ts | 37 ++++++-- source/core/update-planner.ts | 4 + .../actual-file.md | 5 + .../expected-output.md | 6 ++ .../new-template.md.njk | 5 + .../new-values.yaml | 1 + .../old-template.md.njk | 4 + .../old-values.yaml | 1 + .../scenario.yaml | 17 ++++ tests/unit/drift.test.ts | 9 +- tests/unit/three-way-merge.test.ts | 94 ++++++++++++++++++- 11 files changed, 171 insertions(+), 12 deletions(-) create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/actual-file.md create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/expected-output.md create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/new-template.md.njk create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/new-values.yaml create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/old-template.md.njk create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/old-values.yaml create mode 100644 tests/fixtures/merge/21-copy-origin-literal-braces/scenario.yaml diff --git a/source/core/differ.ts b/source/core/differ.ts index f97fc0a..b683df4 100644 --- a/source/core/differ.ts +++ b/source/core/differ.ts @@ -68,6 +68,16 @@ export interface ComputeMergeActionInput { readonly newValues: Record; readonly actualContent: string; readonly renderContext: RenderContext; + /** + * Copy-origin file (not a `.njk` template): `oldTemplate` / `newTemplate` + * are verbatim bytes, NOT Nunjucks sources. Skip rendering them — a copy + * file is never templated, so rendering it (a) crashes on a literal `{{` + * that isn't a valid expression (e.g. `garbage{{` in a test fixture) and + * (b) silently substitutes any real `{{ expr }}` the file contains as + * data. The three-way merge runs on the raw bytes instead. Default + * `false` keeps the rendered path for `.njk` templates. See #132. + */ + readonly literal?: boolean; } export interface ThreeWayMergeResult { @@ -79,16 +89,23 @@ export interface ThreeWayMergeResult { export async function computeMergeAction( input: ComputeMergeActionInput, ): Promise { - const base = renderString( - input.oldTemplate, - { ...input.renderContext, values: input.oldValues }, - input.path, - ); - const ours = renderString( - input.newTemplate, - { ...input.renderContext, values: input.newValues }, - input.path, - ); + // Copy-origin files are verbatim — never run Nunjucks over them (see + // `literal` on the input type). Template files render old/new values so the + // diff3 below sees only the user's manual edits, not value churn. + const base = input.literal + ? input.oldTemplate + : renderString( + input.oldTemplate, + { ...input.renderContext, values: input.oldValues }, + input.path, + ); + const ours = input.literal + ? input.newTemplate + : renderString( + input.newTemplate, + { ...input.renderContext, values: input.newValues }, + input.path, + ); if (sha256(base) === sha256(ours)) { return { type: 'skip', reason: 'no upstream change' }; diff --git a/source/core/update-planner.ts b/source/core/update-planner.ts index 8616e24..adb46b2 100644 --- a/source/core/update-planner.ts +++ b/source/core/update-planner.ts @@ -451,6 +451,10 @@ export async function planUpdate(input: PlanUpdateInput): Promise { newValues, actualContent, renderContext: newRenderContext, + // Copy-origin files (no `.njk` suffix) are verbatim, not templates. + // Rendering them through Nunjucks crashes on a literal `{{` and would + // substitute any real `{{ expr }}` as data, so merge raw bytes (#132). + literal: target.copyFromSourcePath !== undefined, }); switch (mergeAction.type) { diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/actual-file.md b/tests/fixtures/merge/21-copy-origin-literal-braces/actual-file.md new file mode 100644 index 0000000..8a43d6b --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/actual-file.md @@ -0,0 +1,5 @@ +header +const greeting = "Hi {{ user_name }}"; +const broken = "garbage{{"; +my personal line +footer diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/expected-output.md b/tests/fixtures/merge/21-copy-origin-literal-braces/expected-output.md new file mode 100644 index 0000000..91402dc --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/expected-output.md @@ -0,0 +1,6 @@ +header +shard added a line +const greeting = "Hi {{ user_name }}"; +const broken = "garbage{{"; +my personal line +footer diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/new-template.md.njk b/tests/fixtures/merge/21-copy-origin-literal-braces/new-template.md.njk new file mode 100644 index 0000000..1aae36a --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/new-template.md.njk @@ -0,0 +1,5 @@ +header +shard added a line +const greeting = "Hi {{ user_name }}"; +const broken = "garbage{{"; +footer diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/new-values.yaml b/tests/fixtures/merge/21-copy-origin-literal-braces/new-values.yaml new file mode 100644 index 0000000..8059494 --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/new-values.yaml @@ -0,0 +1 @@ +user_name: Alice diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/old-template.md.njk b/tests/fixtures/merge/21-copy-origin-literal-braces/old-template.md.njk new file mode 100644 index 0000000..b2f5433 --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/old-template.md.njk @@ -0,0 +1,4 @@ +header +const greeting = "Hi {{ user_name }}"; +const broken = "garbage{{"; +footer diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/old-values.yaml b/tests/fixtures/merge/21-copy-origin-literal-braces/old-values.yaml new file mode 100644 index 0000000..8059494 --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/old-values.yaml @@ -0,0 +1 @@ +user_name: Alice diff --git a/tests/fixtures/merge/21-copy-origin-literal-braces/scenario.yaml b/tests/fixtures/merge/21-copy-origin-literal-braces/scenario.yaml new file mode 100644 index 0000000..f91b3ff --- /dev/null +++ b/tests/fixtures/merge/21-copy-origin-literal-braces/scenario.yaml @@ -0,0 +1,17 @@ +name: "Copy-origin file with literal {{ — merged verbatim, never rendered" +description: > + A modified copy-origin file (not a .njk template) containing a literal `{{` + that is not a valid Nunjucks expression ("garbage{{"), plus a real-looking + `{{ user_name }}`. The three-way merge must NOT render it: rendering would + crash on `garbage{{` and would substitute `{{ user_name }}` as data. With + copy_origin the merge runs on raw bytes — the user's add and the shard's add + combine non-conflicting, and both literals survive untouched. + Reported via #129; root cause #132. + +copy_origin: true +ownership_before: modified +user_edited: true +template_changed: true +values_changed: false +conflict_expected: false +expected_action: auto_merge diff --git a/tests/unit/drift.test.ts b/tests/unit/drift.test.ts index 2146ca4..66d297f 100644 --- a/tests/unit/drift.test.ts +++ b/tests/unit/drift.test.ts @@ -69,6 +69,12 @@ interface Scenario { partial_update?: boolean; module_change?: 'newly_included' | 'newly_excluded'; module?: string; + /** + * Copy-origin file (not a `.njk` template). The merge must NOT render it — + * `oldTemplate`/`newTemplate` are verbatim bytes. Threads to + * `computeMergeAction`'s `literal` flag. See #132. + */ + copy_origin?: boolean; } interface FixtureFiles { @@ -154,7 +160,7 @@ function makeRenderContext(scenario: Scenario): RenderContext { }; } -const EXPECTED_FIXTURE_COUNT = 20; +const EXPECTED_FIXTURE_COUNT = 21; describe('merge engine (fixture-driven)', () => { it(`discovers all ${EXPECTED_FIXTURE_COUNT} scenarios`, () => { @@ -229,6 +235,7 @@ async function assertStandardMerge( newValues: files.newValues, actualContent: files.actualContent, renderContext: makeRenderContext(scenario), + literal: scenario.copy_origin ?? false, }); expect(action.type).toBe(scenario.expected_action); diff --git a/tests/unit/three-way-merge.test.ts b/tests/unit/three-way-merge.test.ts index 9ecd611..10ba021 100644 --- a/tests/unit/three-way-merge.test.ts +++ b/tests/unit/three-way-merge.test.ts @@ -6,7 +6,16 @@ */ import { describe, it, expect } from 'vitest'; -import { threeWayMerge } from '../../source/core/differ.js'; +import { threeWayMerge, computeMergeAction } from '../../source/core/differ.js'; +import type { RenderContext } from '../../source/runtime/types.js'; + +const RENDER_CTX: RenderContext = { + values: {}, + included_modules: [], + shard: { name: 'test-shard', version: '0.1.0' }, + install_date: '2026-04-01', + year: '2026', +}; describe('threeWayMerge — stats accounting', () => { it('counts every unchanged line when base === theirs === ours', () => { @@ -107,3 +116,86 @@ describe('threeWayMerge — stats accounting', () => { expect(result.content).toContain('user added'); }); }); + +describe('computeMergeAction — literal (copy-origin) inputs (#132)', () => { + it('literal: merges raw bytes without rendering — literal {{ survives, no crash', async () => { + // A copy file containing a literal `{{` that is not a valid Nunjucks + // expression AND a real-looking `{{ user_name }}`. Rendering (the bug) + // would crash on the former and substitute the latter. literal=true + // merges the raw bytes; both survive. + const old = 'a\nconst s = "garbage{{";\nconst g = "{{ user_name }}";\n'; + const neu = 'a\nshard line\nconst s = "garbage{{";\nconst g = "{{ user_name }}";\n'; + const actual = 'a\nconst s = "garbage{{";\nconst g = "{{ user_name }}";\nmine\n'; + const action = await computeMergeAction({ + path: 'scripts/foo.test.ts', + ownership: 'modified', + oldTemplate: old, + newTemplate: neu, + oldValues: { user_name: 'Alice' }, + newValues: { user_name: 'Alice' }, + actualContent: actual, + renderContext: RENDER_CTX, + literal: true, + }); + expect(action.type).toBe('auto_merge'); + if (action.type === 'auto_merge') { + expect(action.content).toContain('garbage{{'); // not crashed + expect(action.content).toContain('{{ user_name }}'); // NOT substituted to "Alice" + expect(action.content).not.toContain('Alice'); + expect(action.content).toContain('mine'); // user edit kept + expect(action.content).toContain('shard line'); // shard edit kept + } + }); + + it('literal: a genuine divergence still produces a conflict on raw bytes', async () => { + const action = await computeMergeAction({ + path: 'data.json', + ownership: 'modified', + oldTemplate: 'a\nbase\nc\n', + newTemplate: 'a\nshard{{\nc\n', + oldValues: {}, + newValues: {}, + actualContent: 'a\nmine{{\nc\n', + renderContext: RENDER_CTX, + literal: true, + }); + expect(action.type).toBe('conflict'); + }); + + it('non-literal (.njk template) with a real syntax error STILL throws — no #129 masking', async () => { + // Regression guard: the fix must NOT make `.njk` authoring errors silent. + // A broken template (`{{ user_name }` — missing brace) on the rendered + // path must still surface RENDER_TEMPLATE_ERROR, not pass through. + await expect( + computeMergeAction({ + path: 'note.md.njk', + ownership: 'modified', + oldTemplate: 'ok\n', + newTemplate: 'Hi {{ user_name }\n', // genuine Nunjucks syntax error + oldValues: { user_name: 'Alice' }, + newValues: { user_name: 'Alice' }, + actualContent: 'ok\nedited\n', + renderContext: RENDER_CTX, + literal: false, + }), + ).rejects.toMatchObject({ code: 'RENDER_TEMPLATE_ERROR' }); + }); + + it('non-literal default: omitting literal renders as before (back-compat)', async () => { + // No `literal` field → renders, substituting `{{ user_name }}`. + const action = await computeMergeAction({ + path: 'note.md.njk', + ownership: 'managed', + oldTemplate: 'Hi {{ user_name }}\n', + newTemplate: 'Hello {{ user_name }}\n', + oldValues: { user_name: 'Alice' }, + newValues: { user_name: 'Alice' }, + actualContent: 'Hi Alice\n', + renderContext: RENDER_CTX, + }); + expect(action.type).toBe('overwrite'); + if (action.type === 'overwrite') { + expect(action.content).toBe('Hello Alice\n'); // substituted + } + }); +}); From 8075050aa0c94f289f4d0e323a56ba964b8dadf2 Mon Sep 17 00:00:00 2001 From: Brenno Ferrari Date: Tue, 2 Jun 2026 00:06:54 +0200 Subject: [PATCH 2/6] test(update): copy-origin file with literal {{ merges end-to-end (#132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integration test through the real install → modify → detect drift → plan → runUpdate pipeline: a shard ships a non-`.njk` copy file containing `garbage{{` + `{{ user_name }}`; the user edits it; the new shard version edits a different region. planUpdate no longer crashes (it did before #132), the file merges on raw bytes, and both literals survive — `{{ user_name }}` is NOT substituted to "Alice", proving the merge never rendered it. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/integration/update.test.ts | 76 ++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/tests/integration/update.test.ts b/tests/integration/update.test.ts index 7ad1de3..fddc753 100644 --- a/tests/integration/update.test.ts +++ b/tests/integration/update.test.ts @@ -1000,6 +1000,82 @@ describe('update pipeline (against examples/minimal-shard)', () => { await fsp.rm(shardDir, rmOpts); } }, 45_000); + + it('merges a modified copy-origin file with a literal {{ without rendering it (#132)', async () => { + // A copy-origin file (no `.njk` suffix) carrying a literal `{{` that is + // not a valid Nunjucks expression, plus a real-looking `{{ user_name }}`. + // Before #132 the merge rendered both sides → crashed on `garbage{{` and + // would have substituted `{{ user_name }}`. Now it merges raw bytes. + const oldShard = path.join(os.tmpdir(), `shardmind-update-oldshard-${crypto.randomUUID()}`); + try { + await copyShard(MINIMAL_SHARD, oldShard); + const probeBase = + 'line A\nconst s = "garbage{{";\nconst g = "{{ user_name }}";\nline B\n'; + await fsp.writeFile(path.join(oldShard, 'notes.txt'), probeBase, 'utf-8'); + + await installBaseline(vault, oldShard, 'sha-0.1.0'); + // Sanity: the copy file installed verbatim (no substitution at install). + const installed = await fsp.readFile(path.join(vault, 'notes.txt'), 'utf-8'); + expect(installed).toBe(probeBase); + + // User edits the copy file → drift classifies it `modified`. + const probeUser = + 'line A\nconst s = "garbage{{";\nconst g = "{{ user_name }}";\nmy line\nline B\n'; + await fsp.writeFile(path.join(vault, 'notes.txt'), probeUser, 'utf-8'); + + // New shard version edits a different region of the copy file. + await copyShard(oldShard, newShard); + await bumpVersion(newShard, '0.2.0'); + const probeShard = + 'line A\nshard line\nconst s = "garbage{{";\nconst g = "{{ user_name }}";\nline B\n'; + await fsp.writeFile(path.join(newShard, 'notes.txt'), probeShard, 'utf-8'); + + const state = (await readState(vault)) as ShardState; + const oldValues = parseYaml( + await fsp.readFile(path.join(vault, 'shard-values.yaml'), 'utf-8'), + ) as Record; + const newManifest = await parseManifest(path.join(newShard, '.shardmind', 'shard.yaml')); + const newSchema = await parseSchema(path.join(newShard, '.shardmind', 'shard-schema.yaml')); + + const migration = applyMigrations(oldValues, state.version, newManifest.version, newSchema.migrations); + const selections = mergeModuleSelections(state.modules, newSchema, {}); + const drift = await detectDrift(vault, state); + const renderCtx = buildRenderContext(newManifest, migration.values, selections); + + // Before #132 this call threw RENDER_TEMPLATE_ERROR on `garbage{{`. + const plan = await planUpdate({ + vault: { root: vault, state, drift }, + values: { old: oldValues, new: migration.values }, + newShard: { schema: newSchema, selections, tempDir: newShard, renderContext: renderCtx }, + removedFileDecisions: {}, + }); + expect(plan.pendingConflicts).toEqual([]); + + const result = await runUpdate({ + vaultRoot: vault, + plan, + conflictResolutions: {}, + currentState: state, + newManifest, + newSchema, + newValues: migration.values, + newSelections: selections, + resolved: { ...RESOLVED, version: newManifest.version }, + tarballSha256: 'sha-0.2.0', + newTempDir: newShard, + }); + expect(result.state.version).toBe('0.2.0'); + + const merged = await fsp.readFile(path.join(vault, 'notes.txt'), 'utf-8'); + expect(merged).toContain('garbage{{'); // no crash, literal intact + expect(merged).toContain('{{ user_name }}'); // NOT substituted + expect(merged).not.toContain('Alice'); // proof it was never rendered + expect(merged).toContain('my line'); // user edit kept + expect(merged).toContain('shard line'); // shard edit kept + } finally { + await fsp.rm(oldShard, { recursive: true, force: true }); + } + }, 45_000); }); // Shard copy helper for the hook integration test. Mirrors the one in From cb45852f78cd1b26e660abb20001d1e270837fb0 Mon Sep 17 00:00:00 2001 From: Brenno Ferrari Date: Tue, 2 Jun 2026 00:08:45 +0200 Subject: [PATCH 3/6] docs(merge): document copy-origin literal merge (#132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IMPLEMENTATION §4.9: `literal` input + a "Copy-origin files" note (merge raw bytes, skip render; renderer stays strict). - SHARD-LAYOUT: the update three-way merge honors the .njk/copy split — copy files merged on raw bytes, never re-rendered. - CHANGELOG: Fixed entry, crediting the #129 report. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 10 ++++++++++ docs/IMPLEMENTATION.md | 8 ++++++-- docs/SHARD-LAYOUT.md | 2 +- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fbabfe1..515b7d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,16 @@ Between releases: see `git log` for merged work and [`ROADMAP.md`](ROADMAP.md) f ## [Unreleased] +### Fixed (update merge rendered copy-origin files — #132) + +Closes [#132](https://github.com/breferrari/shardmind/issues/132); root cause **reported via [#129](https://github.com/breferrari/shardmind/issues/129)** (thanks!). + +- **`shardmind update` no longer crashes** when a modified copy-origin file (any non-`.njk` file — scripts, `.test.ts`, JSON) contains a literal `{{` that isn't a valid Nunjucks expression. The three-way merge (`source/core/differ.ts`) was rendering both sides of *every* modified file through Nunjucks; in v6 only `.njk` files are templates, so copy files were being rendered when they shouldn't be — which crashed on a literal `{{` and **silently substituted** any real `{{ expr }}` a copy file legitimately contained. + +- **Fix:** `computeMergeAction` takes a `literal` flag; `update-planner` sets it for copy-origin files (those carrying `copyFromSourcePath`). When set, the merge runs on the raw bytes (`base = oldTemplate`, `ours = newTemplate`, no render). The renderer stays strict, so genuine `.njk` template syntax errors still fail loudly with `RENDER_TEMPLATE_ERROR` — a renderer-level "swallow the parse error" approach would have masked them. Binary copy files through the utf-8 merge remain tracked by [#63](https://github.com/breferrari/shardmind/issues/63). + +- **Tests:** merge fixture 21 (`tests/fixtures/merge/21-copy-origin-literal-braces/`, authored RED-first) proving the copy file merges without rendering and both literals survive; unit tests for `literal` raw-merge + conflict and a regression guard that a broken `.njk` still throws; an end-to-end `update` integration test. **Docs:** `docs/IMPLEMENTATION.md §4.9`, `docs/SHARD-LAYOUT.md`. + ### Added (adopt batch operations — #120) Closes [#120](https://github.com/breferrari/shardmind/issues/120). `shardmind adopt` against a divergent vault asked for a decision on *every* differing file (the flagship obsidian-mind run had 35). A top-level mode picker now resolves the whole set at once, with per-file prompting kept as a mode. diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 8d26005..9ff03fa 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -734,6 +734,7 @@ computeMergeAction(input: { newValues: Record; actualContent: string; // File on disk renderContext: RenderContext; + literal?: boolean; // copy-origin file → merge raw bytes, don't render (#132) }): Promise type MergeAction = @@ -749,9 +750,12 @@ interface MergeStats { ``` **Algorithm**: -1. Render old template with old values → `base` -2. Render new template with new values → `ours` +1. Render old template with old values → `base` (**unless `literal`** — see below) +2. Render new template with new values → `ours` (**unless `literal`**) 3. `theirs` = `actualContent` (what's on disk) + +**Copy-origin files (`literal: true`, #132)**: in v6 only `.njk` files are templates; everything else is copied verbatim (`modules.ts`). The update planner sets `literal` for copy-origin files (those carrying `copyFromSourcePath`). When set, steps 1–2 **skip rendering** — `base = oldTemplate`, `ours = newTemplate` — and the three-way merge runs on the raw bytes. Rendering a copy file would (a) crash on a literal `{{` that isn't a valid expression and (b) silently substitute any real `{{ expr }}` it contains as data. The renderer itself stays strict, so genuine `.njk` authoring errors still throw `RENDER_TEMPLATE_ERROR`. + 4. If `sha256(base) === sha256(ours)` → no upstream change → `{ type: 'skip' }` 5. If ownership is `managed` (base === theirs) → `{ type: 'overwrite', content: ours }` 6. If ownership is `modified`: diff --git a/docs/SHARD-LAYOUT.md b/docs/SHARD-LAYOUT.md index 9911a1f..4c05611 100644 --- a/docs/SHARD-LAYOUT.md +++ b/docs/SHARD-LAYOUT.md @@ -91,7 +91,7 @@ Three mechanisms. 1. **Module / agent selection.** Wizard values gate which files ship. Default wizard state is **all modules enabled, all agent files shipped** — per VISION's "ships complete" posture and Invariant 1. User deselects what they don't want. -2. **`.njk` Nunjucks rendering** (author-explicit opt-in by suffix). Any file ending in `.njk` is rendered with user values and the suffix is stripped on install. Author convention is to keep `.njk` to **dotfolder configs** the user doesn't see — `.claude/settings.json.njk`, `.mcp.json.njk` — so the clone-UX cost stays zero. The engine doesn't enforce that convention because iterator templates (`/_each..njk`) and other legitimate uses produce vault-visible output. Vault-visible `{{ values.X }}` *without* the `.njk` suffix is the deferred `rendered_files` opt-in tracked under [#86](https://github.com/breferrari/shardmind/issues/86). +2. **`.njk` Nunjucks rendering** (author-explicit opt-in by suffix). Any file ending in `.njk` is rendered with user values and the suffix is stripped on install. Author convention is to keep `.njk` to **dotfolder configs** the user doesn't see — `.claude/settings.json.njk`, `.mcp.json.njk` — so the clone-UX cost stays zero. The engine doesn't enforce that convention because iterator templates (`/_each..njk`) and other legitimate uses produce vault-visible output. Vault-visible `{{ values.X }}` *without* the `.njk` suffix is the deferred `rendered_files` opt-in tracked under [#86](https://github.com/breferrari/shardmind/issues/86). The three-way merge on `update` honors the same split: copy-origin (non-`.njk`) files are merged on their raw bytes and are **never** re-rendered, so a literal `{{` in a script or test fixture neither crashes the merge nor gets substituted ([#132](https://github.com/breferrari/shardmind/issues/132)). 3. **Lifecycle hooks.** Shard-author TypeScript split across three named slots with engine-enforced write boundaries — `bootstrap` (unmanaged-path setup: QMD bootstrap, `git init`), `personalize` (managed-file edits like `brain/North Star.md`, only when the user supplied non-default values), `post-update` (additive managed-file edits on update). Bound by Invariants 2 + 3 + 4 below. See [§Hook lifecycle](#hook-lifecycle-state-and-re-hash-semantics). From 7cddfd9c4afc86e6c5055c631e02ff2938461fff Mon Sep 17 00:00:00 2001 From: Brenno Ferrari Date: Tue, 2 Jun 2026 00:12:49 +0200 Subject: [PATCH 4/6] =?UTF-8?q?refactor(merge):=20simplify=20pass=20?= =?UTF-8?q?=E2=80=94=20share=20the=20render-or-literal=20branch=20(#132)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /simplify: collapse the two parallel `literal ? template : renderString(...)` branches into one local `sideContent` helper. Agents confirmed the seam (literal on computeMergeAction), the copyFromSourcePath signal, and the status.ts deferral are all the right altitude. Skipped: extracting a shared test RenderContext factory (test-infra, outside this diff); reusing setUpUpdate in the integration test (explicit setup reads clearer for a one-off copy-file scenario). Co-Authored-By: Claude Opus 4.8 (1M context) --- source/core/differ.ts | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/source/core/differ.ts b/source/core/differ.ts index b683df4..345e7b8 100644 --- a/source/core/differ.ts +++ b/source/core/differ.ts @@ -92,20 +92,12 @@ export async function computeMergeAction( // Copy-origin files are verbatim — never run Nunjucks over them (see // `literal` on the input type). Template files render old/new values so the // diff3 below sees only the user's manual edits, not value churn. - const base = input.literal - ? input.oldTemplate - : renderString( - input.oldTemplate, - { ...input.renderContext, values: input.oldValues }, - input.path, - ); - const ours = input.literal - ? input.newTemplate - : renderString( - input.newTemplate, - { ...input.renderContext, values: input.newValues }, - input.path, - ); + const sideContent = (template: string, values: Record): string => + input.literal + ? template + : renderString(template, { ...input.renderContext, values }, input.path); + const base = sideContent(input.oldTemplate, input.oldValues); + const ours = sideContent(input.newTemplate, input.newValues); if (sha256(base) === sha256(ours)) { return { type: 'skip', reason: 'no upstream change' }; From fb5bf40f41db18f6f1349cdaaddce874dafcbabb Mon Sep 17 00:00:00 2001 From: Brenno Ferrari Date: Tue, 2 Jun 2026 00:19:26 +0200 Subject: [PATCH 5/6] =?UTF-8?q?test(merge):=20harden=20literal=20path=20?= =?UTF-8?q?=E2=80=94=20CRLF=20+=20no-trailing-newline=20(#132)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /harden (focused adversarial pass on the merge boundary; it confirmed the fix correct — copyFromSourcePath is set ONLY on copy[] never render[], so a .njk file is never wrongly skipped; add/restore/overwrite byte-copy copy files; .njk errors still throw). Add the two cheap raw-byte edge cases it flagged: literal merge preserves the user file's CRLF line endings, and a copy file with no trailing newline merges without inventing one. status.ts:573 renders templates for its drift base too (same root cause, but try/catch-guarded so it degrades gracefully) — tracked as a follow-up on #132, out of scope for this crash fix. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/three-way-merge.test.ts | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/unit/three-way-merge.test.ts b/tests/unit/three-way-merge.test.ts index 10ba021..a27bdc4 100644 --- a/tests/unit/three-way-merge.test.ts +++ b/tests/unit/three-way-merge.test.ts @@ -147,6 +147,45 @@ describe('computeMergeAction — literal (copy-origin) inputs (#132)', () => { } }); + it('literal: preserves the user file\'s CRLF line endings on raw merge', async () => { + const action = await computeMergeAction({ + path: 'win.txt', + ownership: 'modified', + oldTemplate: 'a\nb\n', + newTemplate: 'a\nshard\nb\n', + oldValues: {}, + newValues: {}, + actualContent: 'a\r\nb\r\nmine\r\n', // user file is CRLF + renderContext: RENDER_CTX, + literal: true, + }); + expect(action.type).toBe('auto_merge'); + if (action.type === 'auto_merge') { + expect(action.content).toContain('\r\n'); // user's CRLF honored + expect(action.content).toContain('shard'); + expect(action.content).toContain('mine'); + } + }); + + it('literal: copy file with no trailing newline merges without inventing one', async () => { + const action = await computeMergeAction({ + path: 'no-eol.txt', + ownership: 'modified', + oldTemplate: 'a\nb', + newTemplate: 'a\nc', + oldValues: {}, + newValues: {}, + actualContent: 'a\nb', + renderContext: RENDER_CTX, + literal: true, + }); + // base→ours changed b→c, user unchanged → managed-style overwrite to ours. + expect(['overwrite', 'auto_merge']).toContain(action.type); + if (action.type === 'overwrite' || action.type === 'auto_merge') { + expect(action.content.endsWith('\n')).toBe(false); + } + }); + it('literal: a genuine divergence still produces a conflict on raw bytes', async () => { const action = await computeMergeAction({ path: 'data.json', From fac9522672665c268723f49bb2ad76d342e90dc8 Mon Sep 17 00:00:00 2001 From: Brenno Ferrari Date: Tue, 2 Jun 2026 00:27:52 +0200 Subject: [PATCH 6/6] test(merge): tighten no-trailing-newline assertion to auto_merge (#132) Copilot review: with ownership 'modified' the action can never be 'overwrite' (that's the managed-only branch), so accepting it weakened the test. Assert auto_merge specifically. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/three-way-merge.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/three-way-merge.test.ts b/tests/unit/three-way-merge.test.ts index a27bdc4..f7aaa86 100644 --- a/tests/unit/three-way-merge.test.ts +++ b/tests/unit/three-way-merge.test.ts @@ -179,9 +179,10 @@ describe('computeMergeAction — literal (copy-origin) inputs (#132)', () => { renderContext: RENDER_CTX, literal: true, }); - // base→ours changed b→c, user unchanged → managed-style overwrite to ours. - expect(['overwrite', 'auto_merge']).toContain(action.type); - if (action.type === 'overwrite' || action.type === 'auto_merge') { + // ownership 'modified' → three-way merge (never the managed overwrite + // branch). base→ours changed b→c, user unchanged → clean auto_merge to ours. + expect(action.type).toBe('auto_merge'); + if (action.type === 'auto_merge') { expect(action.content.endsWith('\n')).toBe(false); } });