Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
09d73b5
feat(renderer): export renderString helper (#11)
breferrari Apr 19, 2026
06de8e2
feat: core/drift.ts + core/differ.ts — three-way merge engine (#11)
breferrari Apr 19, 2026
290bdcc
docs(roadmap): tick merge engine; defer orphan detection to v0.2 (#11…
breferrari Apr 19, 2026
c905719
refactor: dedupe ENOENT dance, parallelize drift reads, share state f…
breferrari Apr 19, 2026
31c9e04
fix(differ): normalize CRLF in three-way merge inputs
breferrari Apr 19, 2026
3bb4623
ci: run matrix on ubuntu + windows + macos
breferrari Apr 19, 2026
e4e9195
chore(gitignore): exclude Claude Code's ScheduleWakeup lock file
breferrari Apr 19, 2026
ae0d818
refactor: tighten merge engine per self-review
breferrari Apr 19, 2026
b5462e3
refactor(errors): type-safe ErrorCode union for ShardMindError
breferrari Apr 19, 2026
e5304dd
feat(drift): land orphan detection in v0.1 (closes #47)
breferrari Apr 19, 2026
79d4651
refactor: review-pass cleanup (vault-paths consts, LINE_SPLIT, inline…
breferrari Apr 19, 2026
5a06b8f
refactor: collapse review-pass dismissals after pushback
breferrari Apr 19, 2026
350d25f
test(merge): edge-case fixtures — empty file, UTF-8, frontmatter merge
breferrari Apr 19, 2026
cabe217
docs(spec): §4.8 and §4.9 reflect implementation reality
breferrari Apr 19, 2026
03248fe
feat: property-based invariants + fix node-diff3 prototype-key crash
breferrari Apr 19, 2026
d900d42
hardening: turn the merge engine upside down, fix 3 real bugs
breferrari Apr 19, 2026
45337c0
chore(gitignore): exclude local fork clones (e.g. node-diff3)
breferrari Apr 19, 2026
3de06de
Revert "chore(gitignore): exclude local fork clones (e.g. node-diff3)"
breferrari Apr 19, 2026
430c2d2
docs: track the node-diff3 workaround removal as a followup (#49)
breferrari Apr 19, 2026
df54469
fix: Copilot review round 2 — nine real findings
breferrari Apr 19, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ Ship the core: install, update, status. Prove that vault template upgrades work
### Milestone 3: Merge Engine (Day 3)

- [x] Write all 17 merge fixture directories — fixtures before code ([#10](https://github.com/breferrari/shardmind/issues/10))
- [ ] `core/drift.ts` + `core/differ.ts` — three-way merge engine ([#11](https://github.com/breferrari/shardmind/issues/11))
- [ ] Iterate until all 17 scenarios pass
- [x] `core/drift.ts` + `core/differ.ts` — three-way merge engine ([#11](https://github.com/breferrari/shardmind/issues/11))
- [x] Iterate until all 17 scenarios pass
- [ ] Add edge case fixtures: frontmatter merge, empty file, binary-identical, encoding

### Milestone 4: Update Command + Status (Day 4)
Expand Down Expand Up @@ -111,6 +111,7 @@ Deferred items surfaced during the v0.1 polish-pass architecture audit. None are
- [ ] Alternate registry configurability (GHE, private, custom URL) ([#39](https://github.com/breferrari/shardmind/issues/39))
- [ ] Encode state-schema migration rules (uses v0.1 framework) ([#40](https://github.com/breferrari/shardmind/issues/40))
- [ ] Re-evaluate `@inkjs/ui` dependency (upstream frozen; shim at `source/components/ui.ts` keeps swap cheap) ([#43](https://github.com/breferrari/shardmind/issues/43))
- [ ] `core/drift.ts` — orphan detection (files on disk under tracked paths but not in state) ([#47](https://github.com/breferrari/shardmind/issues/47))

---

Expand Down
185 changes: 185 additions & 0 deletions source/core/differ.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/**
* Three-way merge engine.
*
* Given the old template, the new template, and the file on disk,
* `computeMergeAction` decides whether to skip, silently overwrite,
* auto-merge, or surface a conflict. When a merge is needed, the heavy
* lifting is delegated to node-diff3's Khanna–Myers algorithm (same
* approach git uses).
*
* See docs/IMPLEMENTATION.md §4.9 for the spec.
*/

import { diff3MergeRegions, type IRegion } from 'node-diff3';
import type {
MergeAction,
MergeResult,
ConflictRegion,
RenderContext,
} from '../runtime/types.js';
import { ShardMindError } from '../runtime/types.js';
import { sha256 } from './fs-utils.js';
import { renderString } from './renderer.js';

const CONFLICT_START = '<<<<<<< yours';
const CONFLICT_SEPARATOR = '=======';
const CONFLICT_END = '>>>>>>> shard update';

export interface ComputeMergeActionInput {
path: string;
ownership: 'managed' | 'modified';
oldTemplate: string;
newTemplate: string;
oldValues: Record<string, unknown>;
newValues: Record<string, unknown>;
actualContent: string;
renderContext: RenderContext;
}

export async function computeMergeAction(
input: ComputeMergeActionInput,
): Promise<MergeAction> {
const base = renderString(
input.oldTemplate,
{ ...input.renderContext, values: input.oldValues },
input.path,
);
const ours = renderString(
input.newTemplate,
{ ...input.renderContext, values: input.newValues },
input.path,
);

if (sha256(base) === sha256(ours)) {
return { type: 'skip', reason: 'no upstream change' };
}

if (input.ownership === 'managed') {
return { type: 'overwrite', content: ours };
}

// ownership === 'modified'
const theirs = input.actualContent;
let merge: ReturnType<typeof threeWayMerge>;
try {
merge = threeWayMerge(base, theirs, ours);
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
throw new ShardMindError(
`Three-way merge failed for ${input.path}: ${message}`,
'MERGE_FAILED',
'Re-run with --verbose for the full trace, then report at github.com/breferrari/shardmind/issues.',
);
}

if (merge.conflicts.length === 0) {
return {
type: 'auto_merge',
content: merge.content,
stats: {
linesUnchanged: merge.stats.linesUnchanged,
linesAutoMerged: merge.stats.linesAutoMerged,
},
};
}

const result: MergeResult = {
content: merge.content,
hasConflicts: true,
conflicts: merge.conflicts,
stats: merge.stats,
};
return { type: 'conflict', result };
}

interface ThreeWayMergeResult {
content: string;
conflicts: ConflictRegion[];
stats: MergeResult['stats'];
}

/**
* Line-based three-way merge. `a` is theirs (user on disk), `o` is base
* (rendered from old template + old values), `b` is ours (rendered from
* new template + new values). Convention matches diff3MergeRegions and
* the git conflict-marker vocabulary (`<<<<<<< yours` wraps theirs,
* `>>>>>>> shard update` wraps ours).
*/
export function threeWayMerge(
base: string,
theirs: string,
ours: string,
): ThreeWayMergeResult {
const baseLines = base.split('\n');
const theirsLines = theirs.split('\n');
const oursLines = ours.split('\n');
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

threeWayMerge splits input on "\n" only. If the on-disk file uses CRLF ("\r\n"), each line in theirsLines will retain a trailing "\r", which can create spurious diffs/conflicts and inaccurate merge stats when compared against base/ours rendered with LF. Normalize line endings (e.g., convert CRLF→LF for all three inputs or split on /\r?\n/) before diff3 to make merges stable across platforms/editors.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — valid concern, fixed in 31c9e04.

threeWayMerge now splits all three inputs on /\r?\n/, so a CRLF theirs on Windows no longer fights LF base / ours (renderer output is always LF). Merged output stays LF; a comment on the split documents the choice and flags that callers should convert to platform-native line endings at the write boundary if ever needed.

Covered by tests/unit/differ-line-endings.test.ts — one test for identity (CRLF theirs matching LF base/ours → no conflict) and one for a genuine conflict that only CRLF-normalization could reveal.


const regions: IRegion<string>[] = diff3MergeRegions(
theirsLines,
baseLines,
oursLines,
);

const merged: string[] = [];
const conflicts: ConflictRegion[] = [];
let linesUnchanged = 0;
let linesAutoMerged = 0;
let linesConflicted = 0;

for (const region of regions) {
if (region.stable) {
merged.push(...region.bufferContent);
linesUnchanged += region.bufferContent.length;
continue;
}

// Unstable region — classify as auto-merge or true conflict.
const theirsUnchanged = arraysEqual(region.aContent, region.oContent);
const oursUnchanged = arraysEqual(region.bContent, region.oContent);
const bothSame = arraysEqual(region.aContent, region.bContent);

if (theirsUnchanged) {
// User left base alone; shard changed → take ours.
merged.push(...region.bContent);
linesAutoMerged += region.bContent.length;
} else if (oursUnchanged) {
// Shard left base alone; user changed → take theirs.
merged.push(...region.aContent);
linesAutoMerged += region.aContent.length;
} else if (bothSame) {
// False conflict — both sides made the same change.
merged.push(...region.aContent);
linesAutoMerged += region.aContent.length;
} else {
const lineStart = merged.length + 1;
merged.push(CONFLICT_START);
merged.push(...region.aContent);
merged.push(CONFLICT_SEPARATOR);
merged.push(...region.bContent);
merged.push(CONFLICT_END);
const lineEnd = merged.length;
conflicts.push({
lineStart,
lineEnd,
base: region.oContent.join('\n'),
theirs: region.aContent.join('\n'),
ours: region.bContent.join('\n'),
});
linesConflicted += region.aContent.length + region.bContent.length;
}
}

return {
content: merged.join('\n'),
conflicts,
stats: { linesUnchanged, linesAutoMerged, linesConflicted },
};
}

function arraysEqual<T>(a: T[], b: T[]): boolean {
if (a.length !== b.length) return false;
for (let i = 0; i < a.length; i += 1) {
if (a[i] !== b[i]) return false;
}
return true;
}
94 changes: 94 additions & 0 deletions source/core/drift.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* Ownership drift detection.
*
* `detectDrift` walks every file recorded in `state.files` and classifies it
* by comparing the on-disk sha256 against the hash stored at install/update
* time. See docs/IMPLEMENTATION.md §4.8.
*
* The output buckets are consumed by the update command to decide, per file,
* whether to overwrite silently (`managed`), run a three-way merge
* (`modified`), skip (`volatile`), re-render fresh (`missing`), or surface
* for user attention (`orphaned`).
*/

import fsp from 'node:fs/promises';
import path from 'node:path';
import type { ShardState, DriftReport, DriftEntry } from '../runtime/types.js';
import { sha256 } from './fs-utils.js';

export async function detectDrift(
vaultRoot: string,
state: ShardState,
): Promise<DriftReport> {
const managed: DriftEntry[] = [];
const modified: DriftEntry[] = [];
const volatile: DriftEntry[] = [];
const missing: DriftEntry[] = [];

for (const [relPath, file] of Object.entries(state.files)) {
// FileState.ownership uses the literal 'user' for volatile files (what the
// install engine writes to state.json); DriftEntry.ownership reports it as
// 'volatile' because that is the reporting-layer vocabulary used by the
// update command. Intentional naming gap, not a bug.
if (file.ownership === 'user') {
volatile.push({
path: relPath,
template: file.template,
renderedHash: file.rendered_hash,
actualHash: null,
ownership: 'volatile',
});
continue;
}

const absPath = path.join(vaultRoot, relPath);
let content: string;
try {
content = await fsp.readFile(absPath, 'utf-8');
} catch (err) {
if (isEnoent(err)) {
missing.push({
path: relPath,
template: file.template,
renderedHash: file.rendered_hash,
actualHash: null,
ownership: file.ownership === 'modified' ? 'modified' : 'managed',
});
continue;
}
throw err;
}

const actualHash = sha256(content);
const entry: DriftEntry = {
path: relPath,
template: file.template,
renderedHash: file.rendered_hash,
actualHash,
ownership: actualHash === file.rendered_hash ? 'managed' : 'modified',
};

if (entry.ownership === 'managed') {
managed.push(entry);
} else {
modified.push(entry);
}
}

// Orphan detection (files on disk under tracked paths but not in state) is
// deferred to v0.2. "Tracked paths" is under-specified once _each iterators
// have exploded one template into N per-item files, and no v0.1 flow needs
// the information. See #47.
const orphaned: string[] = [];

return { managed, modified, volatile, missing, orphaned };
}

function isEnoent(err: unknown): boolean {
return (
err !== null &&
typeof err === 'object' &&
'code' in err &&
(err as { code?: string }).code === 'ENOENT'
);
}
41 changes: 36 additions & 5 deletions source/core/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,43 @@ import { ShardMindError } from '../runtime/types.js';
const VOLATILE_MARKER = '{# shardmind: volatile #}';
const FRONTMATTER_REGEX = /^---\r?\n([\s\S]*?)\r?\n---\r?\n([\s\S]*)$/;

const NUNJUCKS_OPTS = {
autoescape: false,
trimBlocks: true,
lstripBlocks: true,
} as const;

export function createRenderer(templateDir: string): nunjucks.Environment {
return nunjucks.configure(templateDir, {
autoescape: false,
trimBlocks: true,
lstripBlocks: true,
});
return nunjucks.configure(templateDir, NUNJUCKS_OPTS);
}

/**
* Isolated env for rendering a template from a string (no filesystem loader).
* Lazily constructed so the `nunjucks.Environment` is only built when needed
* and never pollutes the module's global `nunjucks.configure()` state.
*/
let defaultStringEnv: nunjucks.Environment | undefined;

function getDefaultStringEnv(): nunjucks.Environment {
if (!defaultStringEnv) {
defaultStringEnv = new nunjucks.Environment(null, NUNJUCKS_OPTS);
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getDefaultStringEnv constructs new nunjucks.Environment(null, ...). Passing null as the loader list is risky because Environment expects undefined, a loader, or an array of loaders; null can behave differently across Nunjucks versions and may throw if internal code assumes an array/object.

Suggested fix: pass undefined (omit the first argument) or [] as the loaders argument to explicitly represent “no loaders”, keeping the env truly string-only and avoiding potential runtime incompatibilities.

Suggested change
defaultStringEnv = new nunjucks.Environment(null, NUNJUCKS_OPTS);
defaultStringEnv = new nunjucks.Environment([], NUNJUCKS_OPTS);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair — swapped to new nunjucks.Environment([], NUNJUCKS_OPTS) in df54469. Added a comment noting the empty loader array explicitly represents 'no filesystem resolution', which is the intent for in-memory string rendering.

}
return defaultStringEnv;
}

/**
* Render a template provided as a string, with the same frontmatter-aware
* split/render/YAML-normalize/recombine pipeline that `renderFile` uses.
* Used by the merge engine (`differ.ts`) where the old/new templates live
* in memory (cached or freshly downloaded), not on disk.
*/
export function renderString(
source: string,
context: RenderContext,
filePath: string,
env: nunjucks.Environment = getDefaultStringEnv(),
): string {
return renderContent(source, context, env, filePath);
}

/**
Expand Down
Loading
Loading