Backups: stable Theme Bridge ownership, isolated damaged targets, reconciled interrupted writes (D2–D4)

docs/development/native-appearance-welcome/BACKUPS_AND_RESET.md

@@ -36,3 +36,17 @@ Settings saves keep their merge-preserving behavior and take at most one daily c
 Focused tests cover journal read-back, corruption, bounds, permissions, symbolic links, live writer exclusion, backup failure, settings coexistence, stale restore, reversible restore, owned-edit preservation, shared-file reset planning, command routing, modal ownership, wrong-word cancellation, all wizard stages and responsive rendering under NO_COLOR. Existing adapter, host, cursor, tmux, prompt persistence, Ask, palette, setup and routing tests were also run. Logs and plain-text/ASCII panel snapshots are in the task evidence folder; the final report names authoritative runs. No full-suite run or physical terminal validation is claimed.
 
 Manual terminal checks still needed: open /changes, review and restore a harmless config change, confirm the extra warning after a later user edit, then undo the restore. In /nmsh reset select one installed area, inspect every diff and backup path, cancel, reopen and complete with the exact word. Verify personal lines and newline style survive, result scrolling works at small sizes, and /reset only explains the guarded command. Test in the supported terminal hosts and with NO_COLOR. Automatic host reload is intentionally left to the lead's integration at `TerminalApp.afterHostFilesChanged(paths)`; the panels currently explain that a reload may be needed.
+
+## Integrity follow-ups (D2–D4, 2026-10-29)
+
+**Theme Bridge ownership record (D2).** Re-applying unchanged colors (every launch) no longer rewrites `theme-bridge/ledger.json`: an unchanged artifact keeps its `generatedAt`, and `commitLedger` writes nothing when the record already says the same thing. Every real record-only change (theme reference, include removal, bat cache) goes through the journal like any other configuration write; `saveLedger` is no longer used by NMSh itself. Restoring a backup that contains the record reconciles it per tool: a tool whose managed file or include this backup restores takes the backup's record for that part; every other tool keeps NMSh's current record (its files are untouched, so its ownership stays provable). The review lists each kept record, and the record is not reported as a "later user edit".
+
+**Damaged targets (D3).** A journal entry is verified from its own files; the recorded target path is checked only lexically (absolute, inside HOME or NMSh's directory). Reading or writing a target still uses the full `checkPath` guard (no symbolic links, real directories, owner, permissions), and its errors now name the exact path and why. Consequences:
+- a damaged target makes only that entry's restore fail, naming the entry, path and reason; nothing is written through it;
+- `/changes` shows `unsafe: <reason>` for that file and keeps every other entry restorable;
+- an entry whose own files fail verification is listed as unreadable with its reason, kept on disk, counted toward the limits and never pruned. Writes to a file that such an entry names are refused (fail closed for that file, with the folder to move aside); writes to other files continue;
+- a manual snapshot skips unsafe paths and names them.
+
+**Interrupted writes (D4).** `outcome` stays `prepared | applied` so older builds still read every entry; how an unfinished write ended is extra evidence in `resolution` (`reverted`, `rollback-incomplete`, `finalized`, `partial`). `Journal.transactionState` reconciles an entry from the bytes on disk, never from assumptions: `applied`, `in-progress` (another live process owns it), `not-applied`, `partially-applied`, `applied-unfinalized`, `conflicted` (a file now matches neither its before nor after image), `reverted`, `rollback-incomplete`, `partial`, `indeterminate` (a file cannot be read safely). An in-process failure records `reverted` or `rollback-incomplete`. In `/changes`, unfinished entries are counted in the notice and marked `!`; Enter reviews a rollback (a normal restore with its own undo checkpoint; only files whose bytes would change and differ from what NMSh wrote need the second confirmation), and `F` marks an entry applied only when every file holds the written bytes (no file is touched). A successful rollback marks the entry `reverted`, naming the undo checkpoint. Unresolved prepared entries are never pruned automatically. Tests kill a real writer process with SIGKILL after the entry is recorded, after the first of two files, and before finalization (`tests/backupsIntegrity.test.ts`).
+
+Limits: the per-host appearance and shader ownership records each describe one file group and are always restored together with that group, so they are not reconciled per tool. PID reuse can make a long-dead writer look `in-progress`; that state blocks nothing but reconciliation advice.

src/appearance/hostFiles.ts

@@ -17,24 +17,35 @@ const within = (root: string, path: string) => { const inside = relative(root, p
  * Reject every symbolic-link component. Missing descendants are allowed only below a verified directory. Paths must be inside the
  * home, or inside NMSh's own configuration directory (XDG_CONFIG_HOME may point outside the home); the same checks apply there.
  */
-export function checkPath(path: string, homeRoot: string, writable = false): void {
+function containingRoot(path: string, homeRoot: string): string {
   const own = nmshConfigDirectory();
   const home = isAbsolute(path) && !within(homeRoot, path) && within(own, path) ? own : homeRoot;
   const inside = relative(home, path);
-  if (!isAbsolute(path) || inside === '..' || inside.startsWith(`..${sep}`) || isAbsolute(inside)) throw new Error('The configuration path is outside your home; no changes are allowed.');
+  if (!isAbsolute(path) || inside === '..' || inside.startsWith(`..${sep}`) || isAbsolute(inside)) throw new Error(`The configuration path is outside your home; no changes are allowed: ${path}`);
+  return home;
+}
+
+/** The lexical half of checkPath: an absolute path inside the home (or NMSh's own directory). Reads nothing on disk. */
+export function checkContained(path: string, homeRoot: string): void {
+  if (typeof path !== 'string' || path.includes('\0')) throw new Error('A recorded configuration path is not a plain absolute path.');
+  containingRoot(path, homeRoot);
+}
+
+export function checkPath(path: string, homeRoot: string, writable = false): void {
+  const home = containingRoot(path, homeRoot);
   const parts = resolve(path).split(sep).filter(Boolean);
   let current: string = sep;
   for (let index = 0; index < parts.length; index++) {
     current = resolve(current, parts[index]!);
     let stat;
     try { stat = lstatSync(current); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return; throw error; }
     if (stat.isSymbolicLink()) throw new Error(`A symbolic link prevents safe configuration access: ${current}`);
-    if (index < parts.length - 1 && !stat.isDirectory()) throw new Error('A configuration parent is not a directory.');
+    if (index < parts.length - 1 && !stat.isDirectory()) throw new Error(`A configuration parent is not a directory: ${current} is a ${stat.isFile() ? 'regular file' : 'special file'} (needed for ${path}).`);
     // System ancestors need not belong to this user; the home and its descendants must.
     if (current === home || current.startsWith(`${home}${sep}`)) {
-      if (process.getuid && stat.uid !== process.getuid()) throw new Error('The configuration path is owned by another user.');
-      if (writable && (stat.mode & 0o222) === 0) throw new Error('The configuration path is read-only.');
-      if (writable && (stat.mode & 0o022) !== 0) throw new Error('The configuration path is writable by a group or everyone.');
+      if (process.getuid && stat.uid !== process.getuid()) throw new Error(`The configuration path is owned by another user: ${current}`);
+      if (writable && (stat.mode & 0o222) === 0) throw new Error(`The configuration path is read-only: ${current}`);
+      if (writable && (stat.mode & 0o022) !== 0) throw new Error(`The configuration path is writable by a group or everyone: ${current}`);
     }
   }
 }
@@ -49,7 +60,7 @@ export function snapshot(path: string, home: string, writable = false): FileSnap
   }
   try {
     const stat = fstatSync(fd);
-    if (!stat.isFile() || stat.size > MAX_FILE_BYTES) throw new Error('Configuration must be a bounded regular file (at most 2 MiB).');
+    if (!stat.isFile() || stat.size > MAX_FILE_BYTES) throw new Error(`Configuration must be a bounded regular file (at most 2 MiB): ${path} is ${stat.isDirectory() ? 'a directory' : stat.isFile() ? 'too large' : 'not a regular file'}.`);
     const bytes=readFileSync(fd);
     const content=bytes.toString('utf8');
     if(bytes.length>MAX_FILE_BYTES||!Buffer.from(content,'utf8').equals(bytes))throw new Error('Configuration must be bounded UTF-8 text; no changes are allowed.');
@@ -84,20 +95,23 @@ export function transact(changes: readonly FileChange[], dependencies: readonly
   if (!effective.length) return;
   const journal = new Journal({home, directory: metadata.directory});
   let entry:JournalEntry;
-  try {entry=journal.record(effective,metadata.source,metadata.label);}
+  try {entry=journal.record(effective,metadata.source,metadata.label,metadata.group?{group:metadata.group}:{});}
   catch(error){throw new Error(`Could not create a verified backup; configuration was not changed. ${error instanceof Error?error.message:String(error)}`);}
-  // Backup I/O may take time: recheck every dependency before the first mutation.
-  for (const dependency of dependencies) unchanged(dependency, home);
-  for (const change of changes) unchanged(change.before, home, true);
   const done: FileChange[] = [];
   try {
-    for (const change of changes) {
+    metadata.fault?.('recorded');
+    // Backup I/O may take time: recheck every dependency before the first mutation (a failure here settles the entry as reverted).
+    for (const dependency of dependencies) unchanged(dependency, home);
+    for (const change of changes) unchanged(change.before, home, true);
+    for (const [index, change] of changes.entries()) {
       unchanged(change.before, home, true);
       if (change.after === change.before.content && (change.afterMode===undefined||change.afterMode===change.before.mode)) continue;
       if (change.after === undefined) unlinkSync(change.before.path);
       else replace(change.before.path, change.after, change.afterMode ?? change.before.mode, home, change.before);
       done.push(change);
+      metadata.fault?.(`written:${index}`);
     }
+    metadata.fault?.('before-complete');
     journal.complete(entry);
     return entry;
   } catch (error) {
@@ -110,6 +124,11 @@ export function transact(changes: readonly FileChange[], dependencies: readonly
         else replace(change.before.path, change.before.content, change.before.mode, home, current);
       } catch { incomplete = true; }
     }
-    throw new Error(`${error instanceof Error ? error.message : String(error)}${incomplete ? ' Recovery is incomplete; preserve the files and inspect the preview before retrying.' : ' Applied file changes were rolled back.'}`);
+    const reason = error instanceof Error ? error.message : String(error);
+    // The entry records how the write ended, so /changes never shows a rolled-back write as pending or applied.
+    try { journal.resolve(entry, incomplete ? {state: 'rollback-incomplete', detail: `${reason} Some files could not be returned to their backed-up state.`}
+      : {state: 'reverted', detail: `${reason} ${done.length ? 'Applied file changes were rolled back.' : 'No file was changed.'}`}); }
+    catch { /* the write's own error stays authoritative; /changes classifies the entry from the files */ }
+    throw new Error(`${reason}${incomplete ? ' Recovery is incomplete; preserve the files and inspect the preview before retrying.' : ' Applied file changes were rolled back.'}`);
   }
 }

src/backups/ChangesPanel.ts

@@ -2,24 +2,32 @@ import {takeSettingsCheckpointFailure} from './settings.js';
 import {dirname,join} from 'node:path';
 import type {Key} from '../terminal/keys.js';
 import {renderStudioLayout, renderStudioReview, studioWrap, type StudioReview} from '../ui/StudioLayout.js';
-import {Journal, type JournalEntry} from './journal.js';
+import {Journal, type JournalEntry, type TransactionReport} from './journal.js';
+import {describeState, needsReview} from './describe.js';
 import {manualSnapshot, planRestore, restore, type RestorePlan} from './restore.js';
 export interface ChangesPanel {
-  journal:Journal;ownershipPaths:string[];entries:JournalEntry[];statuses:Map<string,string>;
+  journal:Journal;ownershipPaths:string[];entries:JournalEntry[];statuses:Map<string,string>;states:Map<string,TransactionReport>;
   selected:number;view:'list'|'review'|'confirm'|'result';review?:StudioReview;restore?:RestorePlan;message:string;shown:boolean;
 }
 export function createChangesPanel(journal:Journal,ownershipPaths:readonly string[]):ChangesPanel {
-  const state:ChangesPanel={journal,ownershipPaths:[...new Set([join(dirname(journal.directory),'config.json'),...ownershipPaths])],entries:[],statuses:new Map(),selected:0,view:'list',message:'History is capped at 200 entries / 64 MiB; newest 20 protected checkpoints and source/path anchors are kept.',shown:false};
+  const state:ChangesPanel={journal,ownershipPaths:[...new Set([join(dirname(journal.directory),'config.json'),...ownershipPaths])],entries:[],statuses:new Map(),states:new Map(),selected:0,view:'list',message:'History is capped at 200 entries / 64 MiB; newest 20 protected checkpoints and source/path anchors are kept.',shown:false};
   refresh(state);return state;
 }
 function refresh(state:ChangesPanel):void {
-  state.entries=state.journal.list();state.statuses.clear();
+  state.entries=state.journal.list();state.statuses.clear();state.states.clear();
   const notices:string[]=[];
-  if(state.journal.unreadable.length)notices.push(`${state.journal.unreadable.length} unreadable backup entries skipped.`);
+  // A damaged entry is kept on disk for review and named with its reason; it no longer blocks other backups or writes.
+  const damaged=[...state.journal.unreadableReasons];
+  if(damaged.length)notices.push(`${damaged.length} unreadable backup ${damaged.length===1?'entry needs':'entries need'} review and ${damaged.length===1?'is':'are'} not listed: ${damaged.slice(0,3).map(([id,reason])=>`${id}: ${reason}`).join('; ')}${damaged.length>3?'; …':''} (kept in ${state.journal.directory}).`);
   try {const count=state.journal.prunedCount();if(count)notices.push(`${count} old backup entries pruned.`);}catch{notices.push('Backup pruning record unreadable.');}
   const failure=takeSettingsCheckpointFailure(state.journal.directory);if(failure)notices.push(failure);
+  for(const entry of state.entries){
+    for(const file of entry.files){const seen=state.journal.inspect(file);state.statuses.set(`${entry.id}:${file.path}`,seen.reason?`unsafe: ${seen.reason}`:seen.status);}
+    state.states.set(entry.id,state.journal.transactionState(entry));
+  }
+  const unfinished=state.entries.filter(entry=>needsReview(state.states.get(entry.id)!.state)).length;
+  if(unfinished)notices.unshift(`${unfinished} unfinished ${unfinished===1?'change needs':'changes need'} review (marked !).`);
   if(notices.length)state.message=notices.join(' ');
-  for(const entry of state.entries)for(const file of entry.files){try{state.statuses.set(`${entry.id}:${file.path}`,state.journal.status(file));}catch{state.statuses.set(`${entry.id}:${file.path}`,'unreadable / unsafe path');}}
   state.selected=Math.max(0,Math.min(state.selected,state.entries.length-1));
 }
 export function changesKey(state:ChangesPanel,key:Key):{close?:true;paths?:string[]}|undefined {
@@ -28,7 +36,17 @@ export function changesKey(state:ChangesPanel,key:Key):{close?:true;paths?:strin
     if(state.view==='list'){
       if(key.kind==='up'||key.kind==='down')state.selected=Math.max(0,Math.min(state.entries.length-1,state.selected+(key.kind==='up'?-1:1)));
       if(key.kind==='text'&&key.value.toLowerCase()==='s'){
-        const entry=manualSnapshot(state.journal,state.ownershipPaths);refresh(state);state.message=`Snapshot ${entry.id} saved.`;
+        const skipped:string[]=[];
+        const entry=manualSnapshot(state.journal,state.ownershipPaths,skipped);refresh(state);
+        state.message=`Snapshot ${entry.id} saved.${skipped.length?` Skipped (unsafe to read): ${skipped.join('; ')}`:''}`;
+      }
+      const selected=state.entries[state.selected];
+      if(key.kind==='text'&&key.value.toLowerCase()==='d'&&selected&&needsReview(state.states.get(selected.id)!.state)){
+        state.journal.dismiss(selected);refresh(state);state.message=`Set ${selected.id} aside as reviewed; your files were not changed.`;
+      }
+      if(key.kind==='text'&&key.value.toLowerCase()==='f'&&selected){
+        // Finalize only records what the files already prove; it writes no configuration.
+        state.journal.finalize(selected);refresh(state);state.message=`Marked ${selected.id} as applied: every file holds the bytes it wrote.`;
       }
       if(key.kind==='enter'&&state.entries[state.selected]){
         state.restore=planRestore(state.journal,state.entries[state.selected]!.id);
@@ -68,10 +86,26 @@ export function renderChangesPanel(state:ChangesPanel,columns:number,height:numb
     entries:state.entries.length?state.entries.map(item=>({group:item.time.slice(0,10),label:item.label,
       render:width=>{
         const states=item.files.map(file=>state.statuses.get(`${item.id}:${file.path}`));
-        const summary=states.includes('edited since')?'edited since':states.includes('missing')?'missing':states.some(value=>value!=='unchanged since')?'unsafe file':'unchanged since';
+        const report=state.states.get(item.id);
+        const summary=report&&report.state!=='applied'?`${needsReview(report.state)?'! ':''}${describeState(report.state)}`
+          :states.includes('edited since')?'edited since':states.includes('missing')?'missing':states.some(value=>value!=='unchanged since')?'unsafe file':'unchanged since';
         return studioWrap(`${item.time.slice(11,16)} ${item.source} · ${summary} · ${item.files.length} files · ${item.label}`,width)[0]!;
       }})):[{group:'History',label:'No backups yet'}],
     selected:state.selected,notice:state.message,details:(width,budget)=>entry?[...studioWrap(`${entry.time} · ${entry.source} · ${entry.label}`,width),
+      ...stateDetails(state,entry).flatMap(line=>studioWrap(line,width)),
       ...entry.files.flatMap(file=>studioWrap(`${file.path}: ${state.statuses.get(`${entry.id}:${file.path}`)}`,width))].slice(0,budget):[],
-    controls:[['↑↓','select'],['Enter','review restore'],['S','snapshot now'],['Esc','close']]},columns,height);
+    controls:[['↑↓','select'],['Enter','review restore'],['S','snapshot now'],...(entry&&state.states.get(entry.id)?.state==='applied-unfinalized'?[['F','mark applied'] as const]:[]),...(entry&&needsReview(state.states.get(entry.id)?.state??'applied')?[['D','set aside'] as const]:[]),['Esc','close']]},columns,height);
+}
+
+function stateDetails(state:ChangesPanel,entry:JournalEntry):string[] {
+  const report=state.states.get(entry.id);
+  if(!report||report.state==='applied')return [];
+  const lines=[`State: ${describeState(report.state)}.`];
+  if(entry.resolution?.detail)lines.push(entry.resolution.detail);
+  if(report.writer)lines.push(`Writer process ${report.writer} is still running; wait for it before restoring.`);
+  if(report.state==='applied-unfinalized')lines.push('F marks it applied (nothing is written to your files). Enter reviews a rollback instead.');
+  else if(needsReview(report.state))lines.push('Enter reviews a rollback to the backed-up files; a newer edit needs an explicit second confirmation.');
+  if(needsReview(report.state))lines.push('D sets it aside as reviewed: files stay as they are and the entry can later be pruned.');
+  lines.push(...report.reasons);
+  return lines;
 }