fix(v5): harden roadmap sync per PR #2729 bot-review findings

.hermes/SOUL.md

@@ -15,9 +15,9 @@ Genie v5 is task-backed and zero-daemon by default:
 - `genie launch <wish>` opens isolated execution lanes when requested.
 - `genie omni` handles the optional Omni bridge and approval inbox.
 
-Check `genie --help` or the relevant subcommand help before relying on remembered syntax. Never use obsolete
-`serve`, `ls`, `task board`, `spawn`, or `--no-tui` surfaces, and never assume the retired
-`/home/genie/workspace/agents` path.
+Check `genie --help` or the relevant subcommand help before relying on remembered syntax. Never use the obsolete
+top-level `genie serve` (the supported `genie omni serve` bridge is a different command), `ls`, `task board`,
+`spawn`, or `--no-tui` surfaces, and never assume the retired `/home/genie/workspace/agents` path.
 
 ## Operating law
 

.husky/post-checkout

@@ -0,0 +1,12 @@
+# Reconcile the local genie.db with the canonical board snapshot
+# (.genie/roadmap.json) after a branch checkout swapped the snapshot —
+# post-merge/post-rewrite do not fire here, so without this hook the shared db
+# keeps exposing the previous branch's board until the next sync. Same
+# three-way `task sync` contract: fail-open, refuses on divergence, skipped in
+# linked worktrees and for file (non-branch) checkouts ($3 = 0).
+[ "$3" = "1" ] || exit 0
+git_dir=$(git rev-parse --path-format=absolute --git-dir 2>/dev/null)
+git_common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null)
+if [ "$git_dir" = "$git_common" ] && [ -f src/genie.ts ] && [ -f .genie/roadmap.json ]; then
+  bun src/genie.ts task sync || true
+fi

.husky/pre-commit

@@ -21,8 +21,14 @@ fi
 git_dir=$(git rev-parse --path-format=absolute --git-dir 2>/dev/null)
 git_common=$(git rev-parse --path-format=absolute --git-common-dir 2>/dev/null)
 if [ "$git_dir" = "$git_common" ] && [ -f src/genie.ts ]; then
+  # Stage roadmap.json only when the sync itself rewrote it (export) — never
+  # absorb pre-existing unstaged edits into an unrelated commit.
+  roadmap_before=$( [ -f .genie/roadmap.json ] && git hash-object .genie/roadmap.json || echo absent )
   if bun src/genie.ts task sync; then
-    [ -f .genie/roadmap.json ] && git add .genie/roadmap.json
+    roadmap_after=$( [ -f .genie/roadmap.json ] && git hash-object .genie/roadmap.json || echo absent )
+    if [ "$roadmap_after" != "$roadmap_before" ] && [ "$roadmap_after" != "absent" ]; then
+      git add .genie/roadmap.json
+    fi
   else
     echo "warn: board snapshot not refreshed (see message above) — committing without it" >&2
   fi

scripts/release-docs.test.ts

@@ -990,6 +990,7 @@ describe('Group E release and documentation contracts', () => {
     expect(lifecycle).toContain('Security-sensitive changes do not bypass by default');
     expect(lifecycle).toContain('Related existing work always resumes through its persisted state');
     expect(metadata).toContain('otherwise bypass it with a one-line notice');
+    expect(metadata).toContain('Security-sensitive work does not bypass by default.');
   });
 
   test('wizard discloses init MCP writes and owner-qualified lifecycle order', () => {

src/lib/v5/roadmap-sync.ts

@@ -107,6 +107,17 @@ export function recordSyncBaseline(db: Database, cwd?: string): void {
  * on pull (post-merge / post-rewrite) and before commit (pre-commit).
  */
 export function syncRoadmap(db: Database, cwd?: string): SyncResult {
+  // BEGIN IMMEDIATE for the whole compare-and-act sequence: the db-side hash
+  // must not go stale between comparison and a replace-import, or a task write
+  // committed in that window would be silently destroyed. Holding the write
+  // lock up front also serializes two concurrent syncs (git hook vs interactive
+  // command) through busy_timeout, so their file read-decide-write sequences
+  // cannot interleave. importState's inner transaction nests as a savepoint.
+  const locked = db.transaction(() => syncRoadmapLocked(db, cwd));
+  return locked.immediate() as SyncResult;
+}
+
+function syncRoadmapLocked(db: Database, cwd?: string): SyncResult {
   const filePath = resolveRoadmapPath(cwd);
   const markerPath = resolveSyncMarkerPath(cwd);
   const dbState = roadmapSnapshot(db);

src/lib/v5/task-state.test.ts

@@ -43,6 +43,7 @@ import {
   getTaskLane,
   getWishGroups,
   hireAgent,
+  importState,
   listBoards,
   listHires,
   listTasks,
@@ -751,6 +752,39 @@ try {
   }, 30_000);
 });
 
+describe('importState — replace is a full wipe even without operational rows', () => {
+  test('stale meta keys do not survive a --replace into a metadata-only db', () => {
+    // Source db: one task, then export. Its meta carries the backfill marker.
+    const source = openDb({ path: join(dir, 'source.db') });
+    createTask(source, { title: 'canonical card' });
+    const snapshot = exportState(source);
+    source.close();
+
+    // Destination db: NO operational rows, but a stale meta key (e.g. a
+    // wish_sig left behind after its rows were cleared elsewhere).
+    db.query('INSERT INTO meta (key, value) VALUES (?, ?)').run('wish_sig:ghost', 'deadbeef');
+
+    importState(db, snapshot, { replace: true });
+
+    const keys = (db.query('SELECT key FROM meta ORDER BY key').all() as Array<{ key: string }>).map((r) => r.key);
+    expect(keys).toEqual(snapshot.meta.map((m) => m.key).sort());
+    expect(keys).not.toContain('wish_sig:ghost');
+    // The export is now the exact snapshot — lossless replacement.
+    expect(exportState(db)).toEqual(snapshot);
+  });
+
+  test('a malformed row that passes shape validation fails as SnapshotFormatError and rolls back', () => {
+    const before = exportState(db);
+    const bad = {
+      ...before,
+      // title NOT NULL in schema — null must not surface as a raw SQLite error.
+      tasks: [{ id: 't_bad', title: null, status: 'ready', created_at: 1, updated_at: 1 }],
+    };
+    expect(() => importState(db, bad as unknown, { replace: true })).toThrow(/Snapshot rows could not be imported/);
+    expect(exportState(db)).toEqual(before); // transaction rolled back
+  });
+});
+
 // ---------------------------------------------------------------------------
 // Multi-PROCESS checkout race: N concurrent bun processes claim the same ready
 // task. Exactly one must win; every loser must get a typed conflict. This is

src/lib/v5/task-state.ts

@@ -1572,11 +1572,13 @@ export function importState(db: Database, snapshot: unknown, opts: ImportOptions
   const state = validateSnapshot(db, snapshot);
   const hires = opts.preserveHireRoster ? [] : state.hire_roster;
   const apply = db.transaction(() => {
-    if (hasOperationalState(db)) {
-      if (!opts.replace) throw new NonEmptyImportError();
-      // Children before parents so FK cascades never fire mid-wipe. Meta is
-      // wiped too: replace means EXACT snapshot state, and stale local
-      // wish_sig/backfill markers would misdescribe the imported rows.
+    if (hasOperationalState(db) && !opts.replace) throw new NonEmptyImportError();
+    if (opts.replace) {
+      // Wipe unconditionally under replace — a db with only meta rows (e.g. the
+      // backfill marker a fresh open stamps) must not merge stale keys into the
+      // snapshot's meta: replace means EXACT snapshot state, and a retained
+      // wish_sig marker would misdescribe the imported rows as drifted.
+      // Children before parents so FK cascades never fire mid-wipe.
       const wipe = ['task_events', 'stage_log', 'task_dependencies', 'tasks', 'wish_groups', 'boards', 'meta'];
       if (!opts.preserveHireRoster) wipe.splice(4, 0, 'hire_roster');
       for (const table of wipe) {
@@ -1585,7 +1587,17 @@ export function importState(db: Database, snapshot: unknown, opts: ImportOptions
     }
     insertSnapshotRows(db, { ...state, hire_roster: hires });
   });
-  apply();
+  try {
+    apply();
+  } catch (err) {
+    if (err instanceof SnapshotFormatError || err instanceof NonEmptyImportError) throw err;
+    // A malformed row that passed shape validation (roadmap.json survives git
+    // merges) surfaces as an opaque SQLite bind error — rethrow it in the same
+    // actionable class as the structural checks. The transaction rolled back.
+    throw new SnapshotFormatError(
+      `Snapshot rows could not be imported: ${err instanceof Error ? err.message : String(err)}. The database was left unchanged; re-export the snapshot or repair the malformed rows.`,
+    );
+  }
   return {
     boards: state.boards.length,
     tasks: state.tasks.length,

src/term-commands/v5-task.test.ts

@@ -443,6 +443,41 @@ describe('roadmap.json canonical sync', () => {
     expect(hires).toEqual([{ wish: 'w', worktree: '/tmp/wt' }]);
   });
 
+  test('explicit relative canonical path behaves like the default; custom-file exports stay lossless', async () => {
+    const db = openDb({ cwd: repo });
+    createTask(db, { title: 'card' });
+    hireAgent(db, { wish: 'w', agentAdapterId: 'claude', worktree: '/tmp/wt' });
+    db.close();
+
+    // Custom-file --write carries the COMPLETE state (hire_roster included), so
+    // a backup.json → import --replace round-trip cannot silently drop hires.
+    const backup = await cli(repo, 'export', '--write', 'backup.json');
+    expect(backup.code).toBe(0);
+    const backupState = JSON.parse(readFileSync(join(repo, 'backup.json'), 'utf-8')) as StateExport;
+    expect(backupState.hire_roster).toHaveLength(1);
+
+    // The canonical path spelled explicitly (relative) still emits the roadmap
+    // slice and still counts as canonical on import: local hires preserved.
+    const w = await cli(repo, 'export', '--write', '.genie/roadmap.json');
+    expect(w.code).toBe(0);
+    const snap = JSON.parse(snapshotOf(repo)) as StateExport;
+    expect(snap.hire_roster).toEqual([]);
+
+    const r = await cli(repo, 'import', '.genie/roadmap.json', '--replace');
+    expect(r.code).toBe(0);
+    const db2 = openDb({ cwd: repo });
+    const hires = db2.query('SELECT wish, worktree FROM hire_roster').all() as Array<{
+      wish: string;
+      worktree: string;
+    }>;
+    db2.close();
+    expect(hires).toEqual([{ wish: 'w', worktree: '/tmp/wt' }]);
+
+    // The explicit spelling also recorded the sync baseline: no divergence.
+    const settled = await cli(repo, 'sync');
+    expect(settled.code).toBe(0);
+  });
+
   test('pulled snapshot imports on sync; local mutation exports; divergence is refused then resolvable', async () => {
     // Machine A (repo): publish F1, then F2 with one more card.
     const db = openDb({ cwd: repo });

src/term-commands/v5-task.ts

@@ -18,6 +18,7 @@
 
 import type { Database } from 'bun:sqlite';
 import { existsSync, readFileSync, writeFileSync } from 'node:fs';
+import { resolve } from 'node:path';
 import type { Command } from 'commander';
 import { color, formatTimestamp, padRight, truncate } from '../lib/term-format.js';
 import { livenessBadge } from '../lib/v5/card-render.js';
@@ -428,16 +429,18 @@ function handleExport(opts: ExportOptions): void {
   run(() => {
     const db = openDb();
     try {
-      // --write publishes the roadmap slice (no hire_roster — machine-local
-      // worktree paths); the plain stdout dump stays the complete database.
-      const state = opts.write ? roadmapSnapshot(db) : exportState(db);
+      // Only the canonical roadmap.json gets the roadmap slice (no hire_roster —
+      // machine-local worktree paths). Custom-file writes and the plain stdout
+      // dump stay the complete database, so a backup file round-trips lossless.
+      const target = opts.write ? (typeof opts.write === 'string' ? resolve(opts.write) : resolveRoadmapPath()) : null;
+      const canonical = target === resolveRoadmapPath();
+      const state = canonical ? roadmapSnapshot(db) : exportState(db);
       const json = `${JSON.stringify(state, null, 2)}\n`;
-      if (opts.write) {
-        const target = typeof opts.write === 'string' ? opts.write : resolveRoadmapPath();
+      if (target) {
         writeFileSync(target, json);
         // Writing the canonical snapshot declares "this pair is intentional" —
         // it is the keep-the-local-board resolution for a diverged sync.
-        if (target === resolveRoadmapPath()) recordSyncBaseline(db);
+        if (canonical) recordSyncBaseline(db);
         out(`Wrote board snapshot to ${target}.`);
         return;
       }
@@ -467,7 +470,9 @@ function handleSync(): void {
 
 function handleImport(file: string | undefined, opts: ImportOptions): void {
   run(() => {
-    const source = file ?? resolveRoadmapPath();
+    // Normalize before the canonical comparison: an explicit relative spelling
+    // of the roadmap path must behave exactly like omitting it.
+    const source = file ? resolve(file) : resolveRoadmapPath();
     if (!existsSync(source)) {
       fail(`Snapshot not found: ${source}. Generate one with \`genie task export --write\` and commit it.`);
     }