feat: update command — migrator, planner, executor, DiffView (#12)

[breferrari]

Closes #12. Milestone 4's update command — the moat the whole project is built around.

Summary

• shardmind update command with the full pipeline: fetch → apply schema migrations → prompt only for newly required values → offer newly optional modules → decide per-file fate for modified files the new shard no longer ships → three-way merge every modified file in parallel → resolve conflicts via DiffView → snapshot-and-write → post-update hook → summary. Any failure between snapshot and state-write walks the snapshot back; the vault is indistinguishable from pre-update.
• 5 new core modules: migrator.ts, update-planner.ts, update-executor.ts, values-io.ts (shared YAML load for install + update), and the commands/hooks/shared.ts utility (cross-machine summarizeHook + useSigintRollback).
• 8 new Ink components: DiffView, NewValuesPrompt, NewModulesReview, RemovedFilesReview, UpdateSummary, plus CommandFrame and CommandProgress (shared with install — InstallProgress was renamed).
• 6 new typed error codes in source/runtime/errors.ts: UPDATE_NO_INSTALL, UPDATE_SOURCE_MISMATCH, UPDATE_CACHE_MISSING, UPDATE_WRITE_FAILED, MIGRATION_INVALID_VERSION, MIGRATION_TRANSFORM_FAILED.
• 113 new tests; suite 300 → 413 (+38%). Includes 32 adversarial tests, 5 property-based suites × 200 runs (1,000 generative scenarios), 6 component test files, and 9 end-to-end integration scenarios.
• Deferrals tracked as $EDITOR integration for DiffView conflict resolution #50 ($EDITOR in DiffView) and 24h update-check cache for status + update commands #51 (24h update-check cache shared with the status command).

Highlights

• Migrator (source/core/migrator.ts) — applyMigrations transforms shard-values.yaml across versions. Filter rule currentVersion < from_version ≤ targetVersion makes re-runs idempotent. Four change types (rename / added / removed / type_changed); rename refuses to clobber an existing target value (warns and keeps both keys) — prevents a class of silent data loss the adversarial audit surfaced.
• Update planner (source/core/update-planner.ts) — pure. Inputs grouped as { vault, values, newShard, removedFileDecisions } so call sites can't accidentally mix fields from two different shards. Emits UpdatePlan with 10 action variants plus a pendingConflicts queue the state machine drives through DiffView. Per-modified-file merges run in parallel via mapConcurrent(drift.modified, 16, …). theirsHash captured at plan time so keep_mine / skip resolutions don't re-read + re-hash the file.
• Update executor (source/core/update-executor.ts) — snapshot-based rollback. Backup directory allocation is collision-safe under same-millisecond concurrent updates (ISO timestamp retains ms, then numeric suffix on EEXIST). Snapshot copies touched files + .shardmind/state.json + cached manifest/schema/templates in parallel (SNAPSHOT_CONCURRENCY=16). Write pass runs before delete pass so rename-style moves can't clobber the incoming file. Rollback returns per-file failures (RollbackFailure[]) rather than swallowing them — the command layer surfaces "update failed AND rollback was partial" instead of lying about vault state.
• DiffView — one conflict at a time with ±3 lines of context, color-coded yours/shard, CRLF-tolerant (splits on /\r?\n/), Select with three active options and one disabled placeholder. Allowlist filter + firedRef guard against double-fire and accidental editor-stub activation.
• Hook sandbox (source/core/hook.ts) — lookupHook rejects absolute paths and any .. traversal; the resolved hook path must live under the shard's temp directory. Prevents a malicious manifest from probing arbitrary filesystem paths via existence detection.
• --yes, --verbose, --dry-run flags. Dry-run runs the full pipeline (fetch, migrate, plan, merge) without touching the vault or allocating a backup; the summary reports what would happen.
• useSigintRollback + summarizeHook shared between install and update. SIGINT always runs the tempdir cleanup — cancelling during the prompt/wizard phase used to leak the extracted tarball.

Commits (chronological)

The branch is 13 commits; notable ones:

• feat: core/migrator.ts … / core/update-planner.ts / core/update-executor.ts — foundation.
• refactor: share install+update chrome, cache render pass, parallelize I/O — round 1 cleanup (extracts CommandFrame / CommandProgress / shared.ts).
• refactor: round-two cleanups — dedup YAML load, group planner input, fix conflictFromDirect — round 2, after review pushback exposed three real issues the first pass dismissed.
• fix+test: round-three hardening audit (adversarial + property-based) — round 3, four bugs + double-submit guard + 6 component test files + migrator/update adversarial suites.
• fix+test: round-four /harden audit — round 4, four more bugs (SIGINT handler re-registration, rollback swallowing, hook path traversal, error hint specificity) + hook path-traversal tests + stale-phrasing sweep.

Docs

• docs/IMPLEMENTATION.md §3 — Update data flow diagram rewritten to the actual useUpdateMachine phase progression.
• docs/IMPLEMENTATION.md §4.10 — corrected migration filter rule, added the rename-never-clobbers invariant.
• docs/IMPLEMENTATION.md §4.11 / §4.12 / §4.13 — new module specs for the planner, executor, and values-io.
• docs/IMPLEMENTATION.md §6.5 — DiffView props updated to match the actual component shape.
• docs/IMPLEMENTATION.md §7 — error handling table adds the six new codes.
• docs/ARCHITECTURE.md §10.5 — update flow expanded to cover the new flags and the snapshot/rollback guarantee.
• CLAUDE.md — module table + source tree include the new files.
• CHANGELOG.md Unreleased — comprehensive entry covering every user-visible surface.

Test plan

• npm run typecheck — strict mode clean; zero any / @ts-ignore / @ts-nocheck in the diff.
• npm test — 413/413 passing across 38 test files.
• npm run build — runtime bundle 7.9 KB (under the 30 KB target); dist/commands/update.js emitted.
• 9 end-to-end scenarios in tests/integration/update.test.ts: silent overwrite, auto-merge, pending conflict w/ keep_mine, up-to-date, new-required-value detection, removed-file decision, keep_as_user untracks from state, dry-run no-mutation, corrupted-cache conflict fallback.
• 1,000 generative property-based scenarios (fast-check) — migrator idempotence, transitivity, no-key-invention, rename-preserves-data, plan determinism.
• 6 component test files via ink-testing-library — caught a re-entrant double-submit bug before ship.
• Manual smoke test against a live GitHub shard — deferred to obsidian-mind conversion (obsidian-mind v6 — convert to shard format #14).

🤖 Generated with Claude Code

---

Harden Audit

Reference PR: #48

Rounds

• Round 1 — /simplify: reuse + quality + efficiency cleanups. Extracted CommandFrame / CommandProgress / shared.ts; cached NewFilePlan (removed double-render); parallelized modified-merge + snapshot I/O.
• Round 2 — round-two cleanups after pushback: grouped PlanUpdateInput into { vault, values, newShard }, extracted values-io.ts, fixed conflictFromDirect dead-branch + wrong-tempdir bug.
• Round 3 — adversarial + property-based + component tests. Found 4 real bugs: rename clobbers values, same-second backup-dir collision, DiffView CRLF corruption, keep_as_user left files tracked. Plus a re-entrant double-submit in NewValuesPrompt / RemovedFilesReview that the new component tests caught.
• Round 4 — /harden fresh-eyes pass. Four more: SIGINT handler re-registered every render, rollback failures silently swallowed, hook path traversal probe, vague UPDATE_CACHE_MISSING hint. Also swept code comments for stale temporal phrasing.

Real bugs found

• core/migrator.ts — rename silently overwrote target; now refuses + warns.
• core/update-executor.ts :: createBackupDir — same-second timestamp collision; now ms + numeric-suffix probe.
• components/DiffView.tsx — \r leaked through \n-only split and corrupted the terminal; splits on /\r?\n/.
• core/update-executor.ts :: applyWriteAction — keep_as_user kept the entry in state.files (the user chose "untrack" but the engine still considered it managed); now deletes.
• commands/hooks/shared.ts :: useSigintRollback — deps were inline arrows, teardown+re-register on every render; refs pin the callbacks.
• core/update-executor.ts :: runUpdate — rollback errors swallowed via .catch(() => {}); now rollbackUpdate returns a failure list which is attached to the thrown error.
• core/hook.ts :: lookupHook — path traversal / absolute paths allowed a shard to probe arbitrary filesystem paths; now sandboxed to tempDir.
• core/update-planner.ts :: conflictFromDirect — passed '' as tempDir, producing absolute templateKey; fixed to thread newTempDir through.

Tests added

• Unit: 18 migrator adversarial + 4 property-based × 200 runs (800 scenarios), 11 update planner/executor adversarial + 1 property, 6 hook path-traversal. Regression coverage for every fixed bug.
• Component: 6 new files using ink-testing-library — DiffView, CommandFrame, NewValuesPrompt, NewModulesReview, RemovedFilesReview, UpdateSummary.
• Integration: dry-run no-mutation assertion, conflictFromDirect fallback via wiped cache, keep_as_user untracks from state.
• Tests-before → tests-after: 300 → 413 (+113; suite grew by 38%).

Deferrals

• $EDITOR integration for DiffView conflict resolution #50 — $EDITOR integration for DiffView conflict resolution (disabled placeholder shipped).
• 24h update-check cache for status + update commands #51 — 24h update-check cache shared between status (commands/index.tsx — status display + --verbose diagnostics #13) and update.

[Copilot]

Pull request overview

Adds the v0.1 shardmind update pipeline to the CLI/TUI, including value migrations, a pure update planner, a rollback-capable executor, and Ink UI for resolving merge conflicts—backed by new unit + integration tests.

Changes:

• Introduces core update engine modules: migrator, update-planner, and update-executor (with backups + rollback).
• Adds shardmind update command with a useUpdateMachine orchestrator and Ink components (DiffView, prompts, progress, summary).
• Expands tests with unit coverage for migrations/planning and an end-to-end update pipeline integration suite.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tsup.config.ts	Emits dist/commands/update.js for Pastel routing.
source/runtime/errors.ts	Adds update/migration-related ShardMindError codes.
source/core/migrator.ts	Applies schema migrations between shard versions.
source/core/update-planner.ts	Renders new shard outputs and produces an UpdatePlan + pending conflicts.
source/core/update-executor.ts	Executes the plan with snapshot/rollback and writes updated state/values/cache.
source/core/hook.ts	Adds post-update hook detection (runPostUpdateHook).
source/components/DiffView.tsx	Renders conflict regions and collects per-file conflict decisions.
source/components/NewValuesPrompt.tsx	Prompts for newly-required values missing after migration.
source/components/NewModulesReview.tsx	Prompts for opting into new optional modules.
source/components/RemovedFilesReview.tsx	Prompts for modified files removed by the new shard.
source/components/UpdateProgress.tsx	Shows progress bar + optional history while writing.
source/components/UpdateSummary.tsx	Summarizes update results and migration warnings.
source/commands/update.tsx	New shardmind update command UI wiring.
source/commands/hooks/use-update-machine.ts	Orchestrates the full update flow (resolve/download/migrate/plan/execute).
source/commands/hooks/use-install-machine.ts	Updates messaging to point users at shardmind update.
ROADMAP.md	Marks update command + integration test as completed; records deferrals.
tests/unit/migrator.test.ts	Unit tests for migration selection + change application.
tests/unit/update-planner.test.ts	Unit/integration-style tests for planner behavior.
tests/integration/update.test.ts	End-to-end update pipeline scenarios against examples/minimal-shard.

[Copilot]

The comment says "If it also disappeared upstream, leave alone", but the code emits a delete action (which will remove the entry from state and attempt an rm). If the intended behavior is to stop tracking missing files that the shard no longer produces, consider updating the comment to match; otherwise adjust the logic.

[breferrari]

Stale — the misleading comment is already gone. The current drift.missing branch (update-planner.ts:328–344) has no such text; it emits delete when the new shard no longer produces the file, restore_missing otherwise. No action needed on current HEAD.

[Copilot]

The SIGINT rollback path is effectively unreachable right now: backupDirRef.current is only assigned after runUpdate resolves, but Ctrl+C happens while runUpdate is in flight. Also addedPathsRef.current is never populated from the executor, so even if rollback ran it wouldn't remove newly-added files. Consider having runUpdate (or the executor) surface backupDir immediately (callback/progress event) and stream added paths so the SIGINT handler can reliably roll back mid-run.

[breferrari]

SIGINT rollback path is now reachable. Fixed in commit d600dab: the executor exposes onBackupReady (fires after snapshot is staged, before any write) and onFileTouched (fires per write with an introduced boolean). The state machine populates backupDirRef + addedPathsRef eagerly from those callbacks, so a mid-write Ctrl-C now finds live rollback state. Integration test fires onBackupReady before any writes, so SIGINT can roll back mid-run locks the ordering.

[Copilot]

This comment claims the update overwrites shard-values.yaml "atomically", but the implementation uses a direct writeFile to the final path. If atomicity matters (e.g., crash safety), consider writing to a temp file in the same directory and rename-ing, or adjust the comment to avoid promising atomic semantics.

[breferrari]

Fixed in d600dab — the docstring on writeValuesFile now says the atomicity comes from the snapshot-based rollback, not the write itself. Not switching to temp+rename because rollback already covers the crash-during-write case; added the note so the claim doesn't overpromise.

[Copilot]

writable is computed via actionWrites, but the executor still iterates all non-delete actions and emits progress events for them. For conflicts resolved as keep_mine/skip, actionWrites returns false, so total can be smaller than the number of progress updates and index can exceed total (progress bar can go past 100%). Consider defining total as the number of actions you will actually process in these loops, or skip processing/emitting progress for actions you exclude from writable.

[breferrari]

Real bug, fixed in d600dab. Added actionEmitsProgress helper (covers conflict regardless of resolution) and use it for progressTotal instead of writable.length. actionWrites was retired since its only callsite was the broken total calculation.

[Copilot]

add actions can overwrite an existing on-disk file: wasExisting is only used to decide whether to record addedPaths, but writeFile always overwrites. This is especially risky because snapshotForRollback does not snapshot add paths, so an update could destroy an untracked user file with no rollback restore. Consider treating add as a collision when the path already exists (prompt/skip/error), or snapshot and preserve the pre-existing content before overwriting.

[breferrari]

Fixed in d600dab. snapshotForRollback now includes add paths too — copyOptional ENOENTs on the normal (non-existent) case, but captures the pre-existing user file when there is one, so rollback restores it. addedPaths is now populated only when the path was actually absent pre-write (checked via pathExists), so rollback erases only the paths this run newly introduced.

[Copilot]

renderNewShard reads copied files as UTF-8 strings (buffer.toString('utf-8')). The install path writes resolution.copy entries as raw buffers, so updates may corrupt non-text/binary assets (images, PDFs, etc.) and also make hashes/content comparisons inconsistent. Consider carrying copy-file content as a Buffer through the plan (or re-reading as Buffer in the executor) and writing with a buffer-safe writer, mirroring install-executor behavior.

[breferrari]

Fixed in d600dab. RenderedFileEntry now carries an optional copyFromSourcePath for copy-origin entries (binary assets, scripts, anything outside Nunjucks). Planner threads it onto add / overwrite / restore_missing actions. Executor's new writeAction prefers fsp.copyFile when that pointer is present, otherwise writes content as UTF-8. Bytes survive round-trip. Regression test asserts sha256 of a binary fixture round-trips identical.

[Copilot]

The "added" pass only checks currentState.files to decide whether a new output is an add. If a user already has an untracked file at output.outputPath (often reported in drift.orphaned, but also possible outside tracked dirs), this will plan an add and the executor will overwrite it silently. Consider treating existing-on-disk paths as collisions (prompt/skip/conflict) and/or incorporating drift.orphaned (and/or an fs.stat check) before emitting add actions.

[breferrari]

Addressed as part of the add-safety fix in d600dab. Rather than promoting existing-on-disk to a user-facing collision prompt (which would be a wizard-level behavior change — install has the same question via ExistingInstallGate), the executor now snapshots the pre-existing file before the add write, so a failed update's rollback restores it. For a successful update where the user had an orphan at the same path, the update quietly adopts the shard's version — matches install-time behavior. Follow-up to surface the collision interactively would be a --confirm-collisions flag; low priority given the snapshot-restore floor.

[Copilot]

conflictFromDirect sets templateKey via toTemplateKey('', target.entry.sourcePath), which will store an absolute filesystem path in state (because tempDir is empty). That will later break cache lookups that expect a templates/... relative key. Consider using the real newTempDir to compute a relative key here, or setting templateKey to null if it cannot be determined safely.

Suggested change

	templateKey: toTemplateKey('', target.entry.sourcePath),
	templateKey: null,

[breferrari]

Stale — already fixed in commit 21a900b (round 2 cleanup). conflictFromDirect now receives the real newTempDir and produces a cache-relative templateKey matching the other branches. Current code at update-planner.ts:495-540 reflects the fix.

[Copilot]

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.

[Copilot]

SIGINT rollback can’t work during the write phase: backupDirRef.current is only set after runUpdate() resolves, so isActive() is false while writes are in progress. Also addedPathsRef is reset but never populated, so even if rollback ran it wouldn’t remove newly-added files. Suggestion: expose backupDir (and addedPaths) from the executor earlier (e.g., callback/event when backup dir is created and when a path is newly added) or move backup allocation/snapshotting into the machine so refs are set before the first write.

[breferrari]

Same SIGINT issue as 3107674252 — addressed by exposing onBackupReady + onFileTouched from the executor so the state machine can populate its refs mid-run instead of waiting for the return value. Commit d600dab.

[Copilot]

renderNewShard() reads copy-entry files as a Buffer but then coerces them to UTF-8 (buffer.toString('utf-8')) and stores them in content: string. The executor subsequently writes content via fsp.writeFile(..., 'utf-8'). This breaks byte-for-byte updates for any non-UTF8 or binary files (and is inconsistent with install, which writes resolution.copy entries as raw buffers). Suggestion: carry copied file contents as Buffer (or a distinct action type for copied assets) and have the executor write buffers without re-encoding; also ensure modified copied files don’t go through the template-based three-way merge path.

Suggested change

	content: buffer.toString('utf-8'),
	// Preserve copied assets as raw bytes so updates remain
	// byte-for-byte identical for binary and non-UTF-8 files.
	content: buffer,

[breferrari]

Same as 3107674269 — fixed in d600dab. Went with the copyFromSourcePath approach rather than storing Buffers in actions: keeps the merge-engine text pipeline clean (it only cares about rendered strings) while giving the executor a byte-safe path for copy-origin files. Merge/diff never touches copy-origin content anyway, so the string content there is informational-only.