From 3625d30113e2e59f0dd263a239f1ed60dd3f4bf7 Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:09:59 +0530 Subject: [PATCH 1/4] Backups: stable Theme Bridge ownership, damaged targets isolated, interrupted writes reconciled - Theme Bridge: re-applying unchanged colors no longer rewrites ledger.json; every real record change is journaled; a restore reconciles the record per tool so an older backup never forgets files NMSh manages for other tools. - A damaged backup target (directory replaced by a file, symlink, wrong type, inaccessible parent) blocks only its own restore, with the exact entry, path and reason. Unreadable entries are kept, counted, never pruned, and block writes only to the files they name. - Unfinished entries are classified from on-disk hashes (not applied, partially applied, applied but unfinalized, conflicted, reverted, ...), shown in /changes, and can be rolled back with an undo checkpoint or finalized only with proof. In-process failures record reverted/rollback-incomplete; unresolved entries are never pruned. Older builds still read every manifest. --- .../BACKUPS_AND_RESET.md | 14 + src/appearance/hostFiles.ts | 39 ++- src/backups/ChangesPanel.ts | 48 ++- src/backups/describe.ts | 20 ++ src/backups/journal.ts | 131 ++++++++- src/backups/restore.ts | 109 ++++++- src/themeBridge/artifacts.ts | 46 ++- src/themeBridge/runtime.ts | 5 +- tests/backupsIntegrity.test.ts | 277 ++++++++++++++++++ 9 files changed, 640 insertions(+), 49 deletions(-) create mode 100644 src/backups/describe.ts create mode 100644 tests/backupsIntegrity.test.ts diff --git a/docs/development/native-appearance-welcome/BACKUPS_AND_RESET.md b/docs/development/native-appearance-welcome/BACKUPS_AND_RESET.md index c603077f..4f5a903b 100644 --- a/docs/development/native-appearance-welcome/BACKUPS_AND_RESET.md +++ b/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: ` 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. diff --git a/src/appearance/hostFiles.ts b/src/appearance/hostFiles.ts index 90fee1d3..e18bbb89 100644 --- a/src/appearance/hostFiles.ts +++ b/src/appearance/hostFiles.ts @@ -17,11 +17,22 @@ 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++) { @@ -29,12 +40,12 @@ export function checkPath(path: string, homeRoot: string, writable = false): voi 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'); + 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.'}`); } } diff --git a/src/backups/ChangesPanel.ts b/src/backups/ChangesPanel.ts index ccf20497..4a9db5e6 100644 --- a/src/backups/ChangesPanel.ts +++ b/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; + journal:Journal;ownershipPaths:string[];entries:JournalEntry[];statuses:Map;states:Map; 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,14 @@ 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()==='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 +83,25 @@ 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]:[]),['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.'); + lines.push(...report.reasons); + return lines; } diff --git a/src/backups/describe.ts b/src/backups/describe.ts new file mode 100644 index 00000000..e0834642 --- /dev/null +++ b/src/backups/describe.ts @@ -0,0 +1,20 @@ +import type {TransactionState} from './journal.js'; + +/** Plain words for a reconciled transaction state; the same wording in history, details and restore review. */ +export function describeState(state: TransactionState): string { + switch (state) { + case 'applied': return 'applied'; + case 'in-progress': return 'still being written by another NMSh process'; + case 'not-applied': return 'interrupted before any file was written (nothing changed)'; + case 'partially-applied': return 'interrupted part-way: some files were written, others were not'; + case 'applied-unfinalized': return 'interrupted after every file was written, before it was marked done'; + case 'conflicted': return 'interrupted, and a file was changed by something else since'; + case 'reverted': return 'rolled back (files returned to their backed-up state)'; + case 'rollback-incomplete': return 'failed, and its rollback was incomplete'; + case 'partial': return 'partly applied (some areas failed or were skipped)'; + case 'indeterminate': return 'unfinished, and a file cannot be read safely to check it'; + } +} + +/** Whether an entry needs the user's review before it can be trusted as finished. */ +export const needsReview = (state: TransactionState) => !['applied', 'reverted', 'in-progress'].includes(state); diff --git a/src/backups/journal.ts b/src/backups/journal.ts index 041cd864..785d1e49 100644 --- a/src/backups/journal.ts +++ b/src/backups/journal.ts @@ -3,12 +3,30 @@ import {constants, closeSync, fchmodSync, fsyncSync, mkdirSync, openSync, readdi import {randomUUID} from 'node:crypto'; import {join, dirname} from 'node:path'; import {nmshConfigDirectory} from '../configuration/paths.js'; -import {checkPath, digest, snapshot, unchanged, type FileChange} from '../appearance/hostFiles.js'; +import {checkContained, checkPath, digest, snapshot, unchanged, type FileChange} from '../appearance/hostFiles.js'; export interface JournalOptions {home: string; directory?: string; maxEntries?: number; maxBytes?: number} export interface JournalFile {path: string; existed: boolean; mode: number; beforeHash: string; afterHash: string; bytes: number; afterBytes?: number; afterMode?: number} -export interface JournalEntry {version: 1; id: string; time: string; source: string; label: string; files: JournalFile[]; outcome: 'prepared' | 'applied'} -export interface JournalMetadata {source: string; label: string; directory?: string} +/** + * How an unfinished write ended. `outcome` keeps only 'prepared' | 'applied' so older NMSh builds can still read every entry; + * the resolution is extra evidence they ignore. + */ +export interface JournalResolution {state: 'reverted' | 'rollback-incomplete' | 'finalized' | 'partial'; at: string; detail?: string} +export interface JournalEntry {version: 1; id: string; time: string; source: string; label: string; files: JournalFile[]; outcome: 'prepared' | 'applied'; resolution?: JournalResolution; group?: string} +/** What the files on disk say about an entry. Computed from bytes and hashes only; never assumed. */ +export type TransactionState = 'applied' | 'in-progress' | 'not-applied' | 'partially-applied' | 'applied-unfinalized' | 'conflicted' | 'reverted' | 'rollback-incomplete' | 'partial' | 'indeterminate'; +export type FileEvidence = 'before' | 'after' | 'neither' | 'unsafe'; +export interface TransactionReport {state: TransactionState; files: FileEvidence[]; reasons: string[]; writer?: number} +export interface FileInspection {status: 'unchanged since' | 'edited since' | 'missing' | 'unsafe'; reason?: string} +const RESOLUTIONS = ['reverted', 'rollback-incomplete', 'finalized', 'partial']; +const message = (error: unknown) => error instanceof Error ? error.message : String(error); +export interface JournalMetadata { + source: string; label: string; directory?: string; + /** Links an entry to the reset checkpoint it is a stage of. */ + group?: string; + /** Test seam: called at each transaction boundary ('recorded', 'written:', 'before-complete'). */ + fault?: (stage: string) => void; +} const protectedSource = (source: string) => ['reset', 'restore', 'manual'].includes(source); export const ABSENT_HASH = 'absent'; const hash = (text: string | undefined) => text === undefined ? ABSENT_HASH : digest(text); @@ -47,11 +65,16 @@ export class Journal { const value = JSON.parse(this.file(join(base, 'manifest.json'))) as JournalEntry; if (value.version !== 1 || value.id !== id || !['prepared','applied'].includes(value.outcome) || typeof value.source !== 'string' || !/^[a-z][a-z0-9-]{0,63}$/u.test(value.source) || typeof value.label !== 'string' || value.label.length>512 || typeof value.time !== 'string' || !Array.isArray(value.files) || value.files.length > 512) throw new Error('Backup manifest failed verification.'); + if (value.resolution !== undefined && (typeof value.resolution !== 'object' || value.resolution === null || !RESOLUTIONS.includes(value.resolution.state) + || typeof value.resolution.at !== 'string' || (value.resolution.detail !== undefined && (typeof value.resolution.detail !== 'string' || value.resolution.detail.length > 2048)))) throw new Error('Backup resolution failed verification.'); + if (value.group !== undefined && (typeof value.group !== 'string' || !/^\d{8}T\d{9}Z-[a-f0-9-]{36}$/u.test(value.group))) throw new Error('Backup group failed verification.'); value.files.forEach((file, index) => { if (typeof file.path !== 'string' || typeof file.existed !== 'boolean' || !Number.isInteger(file.mode) || file.mode < 0 || file.mode > 0o777 || (file.afterMode!==undefined&&(!Number.isInteger(file.afterMode)||file.afterMode<0||file.afterMode>0o777)) || typeof file.afterHash !== 'string' || !/^(?:absent|[a-f0-9]{64})$/u.test(file.afterHash)) throw new Error('Backup file metadata failed verification.'); - this.checked(file.path); + // Only the recorded path's shape is checked here. Its current filesystem state belongs to whoever reads or writes that file, + // so a damaged target never makes the journal (and every unrelated configuration write) unreadable. + checkContained(file.path, this.options.home); const content = this.file(join(base, 'files', String(index))); if (file.afterBytes !== undefined) { const after = this.file(join(base,'after',String(index))); @@ -62,8 +85,18 @@ export class Journal { return value; } readonly unreadable: string[] = []; + /** Why each unreadable entry failed verification, by id. Such entries are kept for review and never pruned automatically. */ + readonly unreadableReasons = new Map(); + /** Paths an unreadable entry's manifest still names (best effort), so writes to those files can fail closed. */ + readonly unreadablePaths = new Map(); + private damagedPaths(id: string): string[] { + try { + const manifest = JSON.parse(this.file(join(this.directory, id, 'manifest.json'))) as {files?: Array<{path?: unknown}>}; + return Array.isArray(manifest.files) ? manifest.files.flatMap(file => typeof file?.path === 'string' ? [file.path] : []).slice(0, 512) : []; + } catch { return []; } + } list(options: {strict?: boolean} = {}): JournalEntry[] { - this.unreadable.length = 0; + this.unreadable.length = 0; this.unreadableReasons.clear(); this.unreadablePaths.clear(); this.checked(this.directory); let names: string[]; try { names = readdirSync(this.directory); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return []; throw error; } @@ -71,7 +104,7 @@ export class Journal { const entries: JournalEntry[] = []; for (const name of names.filter(name => /^\d{8}T/u.test(name)).sort().reverse()) { try { entries.push(this.read(name)); } - catch (error) { if (options.strict) throw error; this.unreadable.push(name); } + catch (error) { if (options.strict) throw new Error(`Backup ${name} needs review: ${message(error)}`); this.unreadable.push(name); this.unreadableReasons.set(name, message(error)); this.unreadablePaths.set(name, this.damagedPaths(name)); } } return entries; } @@ -91,6 +124,61 @@ export class Journal { const current = snapshot(file.path, this.options.home); return current.content === undefined ? 'missing' : hash(current.content) === file.afterHash && (file.afterMode===undefined||current.mode===file.afterMode) ? 'unchanged since' : 'edited since'; } + /** Like status, but an unsafe or unreadable path is reported with its exact reason instead of throwing. */ + inspect(file: JournalFile): FileInspection { + try { return {status: this.status(file)}; } catch (error) { return {status: 'unsafe', reason: message(error)}; } + } + private evidence(file: JournalFile): FileEvidence { + let current; + try { current = snapshot(file.path, this.options.home); } catch { return 'unsafe'; } + const now = hash(current.content); + const modeMatches = (mode: number | undefined) => current.content === undefined || mode === undefined || current.mode === mode; + if (now === file.afterHash && modeMatches(file.afterMode)) return 'after'; + if (now === file.beforeHash && (!file.existed || current.mode === file.mode)) return 'before'; + return 'neither'; + } + /** The live writer of an unfinished entry, if its recorded process still runs. This process never has one in flight between calls. */ + private writer(entry: JournalEntry): number | undefined { + let owner: {pid?: number} = {}; + try { owner = JSON.parse(snapshot(join(this.directory, entry.id, 'owner.json'), this.options.home).content ?? '{}'); } catch { return undefined; } + const pid = owner.pid; + if (!Number.isSafeInteger(pid) || pid! <= 0 || pid === process.pid) return undefined; + try { process.kill(pid!, 0); return pid; } catch (error) { return (error as NodeJS.ErrnoException).code === 'EPERM' ? pid : undefined; } + } + /** + * Reconcile an entry against the bytes on disk. A prepared entry is never assumed to have succeeded: it is classified from each + * file's hash as not applied, partially applied, applied but not finalized, or conflicted (edited by something else since). + */ + transactionState(entry: JournalEntry): TransactionReport { + const files = entry.files.map(file => this.evidence(file)); + const reasons = entry.files.flatMap((file, index) => files[index] === 'unsafe' ? [`${file.path}: ${this.inspect(file).reason ?? 'unreadable'}`] : []); + if (entry.resolution) return {state: entry.resolution.state === 'finalized' ? 'applied' : entry.resolution.state, files, reasons}; + if (entry.outcome === 'applied') return {state: 'applied', files, reasons}; + const writer = this.writer(entry); + if (writer !== undefined) return {state: 'in-progress', files, reasons, writer}; + if (files.includes('unsafe')) return {state: 'indeterminate', files, reasons}; + if (files.includes('neither')) return {state: 'conflicted', files, reasons}; + // A file whose before and after are identical matches both; it never decides the state alone. + const same = entry.files.map(file => file.beforeHash === file.afterHash && (file.afterMode === undefined || file.afterMode === file.mode)); + const decisive = files.filter((_, index) => !same[index]); + if (!decisive.length || decisive.every(value => value === 'after')) return {state: 'applied-unfinalized', files, reasons}; + if (decisive.every(value => value === 'before')) return {state: 'not-applied', files, reasons}; + return {state: 'partially-applied', files, reasons}; + } + /** Record how an unfinished entry ended. Only the manifest changes; before- and after-images stay as evidence. */ + resolve(entry: JournalEntry, resolution: Omit): JournalEntry { + const verified = this.read(entry.id); + const next: JournalEntry = {...verified, ...(resolution.state === 'finalized' ? {outcome: 'applied' as const} : {}), + resolution: {state: resolution.state, at: new Date().toISOString(), ...(resolution.detail ? {detail: resolution.detail.slice(0, 2048)} : {})}}; + this.atomic(join(this.directory, entry.id, 'manifest.json'), `${JSON.stringify(next, null, 2)}\n`); + return this.read(entry.id); + } + /** Mark an interrupted entry finished, only when every file verifiably holds the bytes it was meant to write. */ + finalize(entry: JournalEntry): JournalEntry { + const report = this.transactionState(this.read(entry.id)); + if (report.state !== 'applied-unfinalized') throw new Error(`Only an entry whose files all hold the written bytes can be finalized (this one is ${report.state}).`); + return this.resolve(entry, {state: 'finalized', detail: 'Every file held the bytes this change wrote; finalized after an interruption.'}); + } private prunedKeys():Set { const text=snapshot(join(this.directory,'.pruned.json'),this.options.home).content; if(text===undefined)return new Set(); @@ -109,7 +197,7 @@ export class Journal { return !this.prunedKeys().has(digest(`${source}:${path}`)); } /** Serialize writers across frontend processes; a crashed writer's verified lock can be recovered. */ - record(changes: readonly FileChange[], source: string, label: string): JournalEntry { + record(changes: readonly FileChange[], source: string, label: string, options: {group?: string} = {}): JournalEntry { this.checked(this.directory,true);mkdirSync(this.directory,{recursive:true,mode:0o700});this.checked(this.directory,true); if((lstatSync(this.directory).mode&0o077)!==0)throw new Error('Backup directory must be private (0700); configuration was not changed.'); const path=join(this.directory,'.write-lock');const token=randomUUID(); @@ -129,10 +217,10 @@ export class Journal { renameSync(path, stale); try { fd=acquire(); } finally { rmSync(stale); } } - try {writeFileSync(fd,content);fsyncSync(fd);return this.recordLocked(changes,source,label);} + try {writeFileSync(fd,content);fsyncSync(fd);return this.recordLocked(changes,source,label,options.group);} finally {closeSync(fd);try{if(this.file(path)===content)rmSync(path);}catch{/* preserve an unexpected replacement */}} } - private incompleteUsage():{bytes:number;count:number} { + private incompleteUsage(damaged: readonly string[] = []):{bytes:number;count:number} { const pending=readdirSync(this.directory).filter(name=>name.startsWith('.pending-')); let files=0; const size=(path:string,depth:number):number=>{ @@ -159,9 +247,15 @@ export class Journal { } bytes+=usage;count++; } + // Entries that failed verification stay where they are for review: they count toward the limits and are never pruned. + for (const name of damaged) { + const path=join(this.directory,name); + try {bytes+=size(path,0);} catch {/* an unmeasurable entry still counts once */} + count++; + } return {bytes,count}; } - private recordLocked(changes: readonly FileChange[], source: string, label: string): JournalEntry { + private recordLocked(changes: readonly FileChange[], source: string, label: string, group?: string): JournalEntry { if(!/^[a-z][a-z0-9-]{0,63}$/u.test(source)||label.length>512)throw new Error('Backup source or label is invalid.'); if (changes.length > 512 || new Set(changes.map(change => change.before.path)).size !== changes.length) throw new Error('Backup file limit or duplicate path.'); for (const change of changes) { @@ -170,7 +264,8 @@ export class Journal { } const now = new Date(); const id = `${now.toISOString().replace(/[-:.]/gu, '')}-${randomUUID()}`; - const entry: JournalEntry = {version: 1, id, time: now.toISOString(), source, label, outcome: 'prepared', files: changes.map(change => ({ + if (group !== undefined && !/^\d{8}T\d{9}Z-[a-f0-9-]{36}$/u.test(group)) throw new Error('Backup group is invalid.'); + const entry: JournalEntry = {version: 1, id, time: now.toISOString(), source, label, outcome: 'prepared', ...(group ? {group} : {}), files: changes.map(change => ({ path: change.before.path, existed: change.before.content !== undefined, mode: change.before.mode, beforeHash: hash(change.before.content), afterHash: hash(change.after), bytes: Buffer.byteLength(change.before.content ?? ''), ...(change.after===undefined?{}:{afterBytes:Buffer.byteLength(change.after),afterMode:change.afterMode??change.before.mode}), @@ -182,9 +277,17 @@ export class Journal { const maxEntries = this.options.maxEntries ?? 200; if (size > maxBytes || maxEntries < 1) throw new Error('Backup capacity limit reached; configuration was not changed.'); this.checked(this.directory, true); - const existing = this.list({strict:true}); + // A damaged entry is kept for review; it must not block unrelated configuration writes. + const existing = this.list(); + const damaged = [...this.unreadable]; + // Fail closed only for the file a damaged backup names: its history needs review before NMSh changes it again. + for (const name of damaged) { + const path = changes.map(change => change.before.path).find(candidate => this.unreadablePaths.get(name)!.includes(candidate)); + if (path) throw new Error(`Backup ${name} of ${path} failed verification (${this.unreadableReasons.get(name)}); review it in /changes. ` + + `To continue, move ${join(this.directory, name)} out of the backups folder. Other files can still be changed.`); + } const cost = (item: JournalEntry) => Buffer.byteLength(snapshot(join(this.directory,item.id,'owner.json'),this.options.home).content??'') + Buffer.byteLength(this.file(join(this.directory,item.id,'manifest.json'))) + item.files.reduce((n,f)=>n+f.bytes+(f.afterBytes??0),0); - const incomplete=this.incompleteUsage(); + const incomplete=this.incompleteUsage(damaged); const pruned=this.prunedKeys(); const oldMarker=Buffer.byteLength(snapshot(join(this.directory,'.pruned.json'),this.options.home).content??''); const previousCount=this.prunedCount(); @@ -192,6 +295,8 @@ export class Journal { let count = existing.length + incomplete.count; const victims: JournalEntry[] = []; const keep = new Set(existing.filter(item=>protectedSource(item.source)).slice(0,protectedSource(source)?19:20).map(item=>item.id)); + // An unfinished write is evidence for recovery (or still in flight in another process): never pruned automatically. + for (const item of existing) if (item.outcome==='prepared'&&!item.resolution) keep.add(item.id); // The incoming entry anchors its source/paths; retain the newest anchor for every other pair. const anchored = new Set(changes.map(change=>`${source}:${change.before.path}`)); for (const item of existing) for(const file of item.files) { diff --git a/src/backups/restore.ts b/src/backups/restore.ts index 73928e44..6445f398 100644 --- a/src/backups/restore.ts +++ b/src/backups/restore.ts @@ -1,19 +1,98 @@ import {dirname,join} from 'node:path'; import {Journal, type JournalEntry} from './journal.js'; -import {digest, snapshot, unchanged, transact, type FileChange} from '../appearance/hostFiles.js'; +import {digest, snapshot, unchanged, transact, type FileChange, type FileSnapshot} from '../appearance/hostFiles.js'; import {changeDiff} from './diff.js'; +import {describeState} from './describe.js'; export interface RestorePlan {entry: JournalEntry; changes: FileChange[]; edited: string[]; preview: string[]; guidance: string} const prepared = new WeakSet(); +const message = (error: unknown) => error instanceof Error ? error.message : String(error); + +/** A restore never writes through a damaged path: it names the entry, the path and the reason, and nothing changes. */ +function checkedSnapshot(entry: JournalEntry, path: string, home: string): FileSnapshot { + try { return snapshot(path, home, true); } + catch (error) { + throw new Error(`Cannot restore "${entry.label}" (${entry.id}): ${path} is not safe to change. ${message(error)} Nothing was changed. ` + + `Repair or move that path yourself (NMSh never writes through it), then review this restore again; other backups are unaffected.`); + } +} + +type RawRecord = Record & {artifactPath?: unknown; hook?: {configPath?: unknown}}; +const isRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null && !Array.isArray(value); +function rawLedger(text: string | undefined): {root: Record; entries: Record} { + let parsed: unknown; + try { parsed = text === undefined ? {} : JSON.parse(text); } catch { parsed = {}; } + const root = isRecord(parsed) ? parsed : {}; + const entries = isRecord(root.entries) ? root.entries as Record : {}; + return {root, entries}; +} + +/** + * The Theme Bridge ownership record covers several tools. Restoring an older copy wholesale would forget files NMSh wrote later + * for tools this backup never touched (their files stay on disk, but NMSh could no longer prove it owns them). So the record is + * reconciled 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, which still matches its untouched files. + */ +function reconcileBridgeLedger(journal: Journal, entry: JournalEntry, changes: FileChange[]): string[] { + const path = join(dirname(journal.directory), 'theme-bridge', 'ledger.json'); + const change = changes.find(item => item.before.path === path); + if (!change) return []; + const restoredPaths = new Set(entry.files.map(file => file.path)); + const backup = rawLedger(change.after), current = rawLedger(change.before.content); + const merged: Record = {}; + const notes: string[] = []; + const touches = (record: RawRecord | undefined, field: 'artifact' | 'hook') => { + const value = field === 'artifact' ? record?.artifactPath : isRecord(record?.hook) ? record!.hook!.configPath : undefined; + return typeof value === 'string' && restoredPaths.has(value); + }; + for (const target of [...new Set([...Object.keys(current.entries), ...Object.keys(backup.entries)])].sort()) { + const old = backup.entries[target], now = current.entries[target]; + const artifact = touches(old, 'artifact') || touches(now, 'artifact'); + const hook = touches(old, 'hook') || touches(now, 'hook'); + let next: RawRecord | undefined; + if (artifact && hook) next = old; + else if (!artifact && !hook) next = now; + else if (artifact) { + // The backup's file record, with the include NMSh records now (this backup does not restore the include's file). + const keptHook = now?.hook; + if (old) { next = {...old}; if (keptHook) next.hook = keptHook; else delete next.hook; } + else if (keptHook) next = {...now!, sha256: digest(''), hook: keptHook}; + } else { + const restoredHook = old?.hook; + if (now) { next = {...now}; if (restoredHook) next.hook = restoredHook; else delete next.hook; } + else if (restoredHook) next = old; + } + if (next) merged[target] = next; + if (!artifact && !hook && JSON.stringify(old) !== JSON.stringify(now)) notes.push(`Theme Bridge: keeps NMSh's current ownership record for ${target}; this backup does not restore its files.`); + } + const after = `${JSON.stringify({...current.root, version: 1, entries: merged}, null, 2)}\n`; + if (after === change.after) return []; + change.after = after; + return notes.length ? notes : ['Theme Bridge: the ownership record is reconciled with files this backup does not restore.']; +} + export function planRestore(journal: Journal, id: string): RestorePlan { const entry = journal.read(id); - const changes = entry.files.map((file,index) => ({before:snapshot(file.path,journal.options.home,true),after:journal.content(entry,index),afterMode:file.mode})); - const edited = changes.filter((change,index)=>(change.before.content === undefined ? 'absent' : digest(change.before.content)) !== entry.files[index]!.afterHash || (entry.files[index]!.afterMode!==undefined && change.before.content!==undefined && change.before.mode!==entry.files[index]!.afterMode)).map(change=>change.before.path); + const home = journal.options.home; + const changes: FileChange[] = entry.files.map((file,index) => ({before:checkedSnapshot(entry,file.path,home),after:journal.content(entry,index),afterMode:file.mode})); + // Only a file this restore would actually change can lose later edits: one already holding its backed-up bytes is not edited. + const edited = changes.filter((change,index)=>{ + const file=entry.files[index]!;const now=change.before.content === undefined ? 'absent' : digest(change.before.content); + if (change.after === change.before.content && (change.before.content === undefined || change.before.mode === file.mode)) return false; + return now !== file.afterHash || (file.afterMode!==undefined && change.before.content!==undefined && change.before.mode!==file.afterMode); + }).map(change=>change.before.path); const settings=join(dirname(journal.directory),'config.json'); - if(!changes.some(change=>change.before.path===settings)){const before=snapshot(settings,journal.options.home,true);changes.push({before,after:before.content,afterMode:before.mode});} - const plan: RestorePlan = {entry,changes,edited,preview:[ + if(!changes.some(change=>change.before.path===settings)){const before=checkedSnapshot(entry,settings,home);changes.push({before,after:before.content,afterMode:before.mode});} + const reconciled = reconcileBridgeLedger(journal, entry, changes); + const ledger = join(dirname(journal.directory), 'theme-bridge', 'ledger.json'); + // The reconciled ownership record is NMSh's own bookkeeping, not a user edit that would be lost. + const warnings = reconciled.length ? edited.filter(path => path !== ledger) : edited; + const state = journal.transactionState(entry); + const plan: RestorePlan = {entry,changes,edited:warnings,preview:[ `${entry.time} · ${entry.source} · ${entry.label}`, - ...entry.files.map(file=>`${file.path}: ${journal.status(file)}`), - ...edited.map(path=>`WARNING: your later edits to ${path} will be lost.`), + ...(state.state === 'applied' ? [] : [`This change is ${describeState(state.state)}. Restoring returns its files to their backed-up state (a rollback).`]), + ...entry.files.map(file=>{const seen=journal.inspect(file);return `${file.path}: ${seen.status}${seen.reason?` (${seen.reason})`:''}`;}), + ...warnings.map(path=>`WARNING: your later edits to ${path} will be lost.`), + ...reconciled, ...changes.flatMap(change=>changeDiff(change).lines)], guidance:'Restore these exact files. Current contents are backed up first, so this restore can itself be undone. Terminal configuration may need a reload.'}; prepared.add(plan); return plan; @@ -24,11 +103,21 @@ export function restore(journal: Journal, plan: RestorePlan, confirmation: {revi for (const change of plan.changes) unchanged(change.before,journal.options.home,true); const undo = transact(plan.changes,[],journal.options.home,{source:'restore',label:`Restore ${plan.entry.label}`,directory:journal.directory}); if (!undo) throw new Error('Restore checkpoint was not created.'); + // Rolling back an unfinished write settles it: history then shows it as reverted, naming the undo checkpoint. + const current = journal.read(plan.entry.id); + if (current.outcome === 'prepared' && !current.resolution) { + try { journal.resolve(current, {state: 'reverted', detail: `Rolled back by restore ${undo.id}.`}); } catch { /* the restore itself succeeded and is recorded */ } + } prepared.delete(plan); return undo; } -/** Includes known journal paths and caller-discovered ownership paths; reading/planning alone writes nothing. */ -export function manualSnapshot(journal: Journal, ownershipPaths: readonly string[]): JournalEntry { +/** Includes known journal paths and caller-discovered ownership paths; reading/planning alone writes nothing. Unsafe paths are skipped and named. */ +export function manualSnapshot(journal: Journal, ownershipPaths: readonly string[], skipped: string[] = []): JournalEntry { const paths = [...new Set([...journal.list().flatMap(entry=>entry.files.map(file=>file.path)),...ownershipPaths])]; - const changes = paths.map(path=>{const before=snapshot(path,journal.options.home);return {before,after:before.content};}); + const changes: FileChange[] = []; + for (const path of paths) { + try { const before=snapshot(path,journal.options.home); changes.push({before,after:before.content}); } + catch (error) { skipped.push(`${path}: ${message(error)}`); } + } + if (!changes.length) throw new Error(`No managed file can be read safely; nothing was saved. ${skipped.join(' ')}`); const entry = journal.record(changes,'manual','Manual snapshot of NMSh-managed files'); journal.complete(entry); return entry; } diff --git a/src/themeBridge/artifacts.ts b/src/themeBridge/artifacts.ts index 1325b6b2..28b0a446 100644 --- a/src/themeBridge/artifacts.ts +++ b/src/themeBridge/artifacts.ts @@ -90,8 +90,14 @@ const TARGETS: readonly ManagedTarget[] = ['tmux', 'neovim', 'vim', 'helix', 'ba /** A malformed or stale ledger yields no ownership (so nothing can be deleted on its say-so). */ export function loadLedger(env: NodeJS.ProcessEnv = process.env): Ledger { + let text: string; + try { text = readFileSync(ledgerPath(env), 'utf8'); } catch { return {version: 1, entries: {}}; } + return loadLedgerText(text, env); +} + +export function loadLedgerText(text: string, env: NodeJS.ProcessEnv = process.env): Ledger { let parsed: unknown; - try { parsed = JSON.parse(readFileSync(ledgerPath(env), 'utf8')); } catch { return {version: 1, entries: {}}; } + try { parsed = JSON.parse(text); } catch { return {version: 1, entries: {}}; } const entries: Ledger['entries'] = {}; const raw = isRecord(parsed) && isRecord(parsed.entries) ? parsed.entries : {}; for (const target of TARGETS) { @@ -123,6 +129,30 @@ function ledgerChange(ledger:Ledger,env:NodeJS.ProcessEnv):FileChange { return {before:snapshot(ledgerPath(env),env.HOME || homedir(),true),after:`${JSON.stringify(ledger,null,2)}\n`}; } +/** + * The one way to change the ownership record on its own: nothing is written when the record already says this, and a real change + * goes through the verified journal like every other configuration write (so /changes can restore it and later entries stay + * "unchanged since"). + */ +export function commitLedger(ledger: Ledger, env: NodeJS.ProcessEnv = process.env, label = 'Update Theme Bridge ownership record'): void { + const home = env.HOME || homedir(); + const change = ledgerChange(ledger, env); + if (change.before.content === change.after) return; + if (change.before.content !== undefined && equalLedgers(change.before.content, ledger)) return; + transact([change], [], home, {source: 'theme-bridge', directory: journalDirectory(home, env), label}); +} + +/** Same ownership facts, ignoring formatting, unknown fields and key order. */ +function equalLedgers(text: string, ledger: Ledger): boolean { + let parsed: unknown; + try { parsed = JSON.parse(text); } catch { return false; } + if (!isRecord(parsed) || !isRecord(parsed.entries) || Object.keys(parsed.entries).some(key => !TARGETS.includes(key as ManagedTarget))) return false; + const stable = (value: unknown): unknown => Array.isArray(value) ? value.map(stable) + : isRecord(value) ? Object.fromEntries(Object.keys(value).sort().map(key => [key, stable(value[key])])) : value; + // Any extra, dropped or normalized field counts as a change, so the stored record always ends up exactly what NMSh trusts. + return JSON.stringify(stable(parsed)) === JSON.stringify(stable(JSON.parse(JSON.stringify(ledger)))); +} + export type Ownership = 'absent' | 'owned' | 'modified' | 'unknown'; /** Owned: the ledger recorded this exact content. Modified/unknown files are never touched. */ @@ -155,7 +185,8 @@ export function writeArtifact(target: ManagedTarget, content: string, validate: const hash = sha256(content); const previous = ledger.entries[target]; const changed = !previous || previous.sha256 !== hash || state === 'absent'; - ledger.entries[target] = {...record, target, artifactPath: path, sha256: hash, adapterVersion: ADAPTER_VERSION, generatedAt: now.toISOString(), + // An unchanged artifact keeps its generation time: re-applying the same colors on every launch must not rewrite the record. + ledger.entries[target] = {...record, target, artifactPath: path, sha256: hash, adapterVersion: ADAPTER_VERSION, generatedAt: changed || !previous ? now.toISOString() : previous.generatedAt, ...(previous?.hook ? {hook: previous.hook} : {}), ...(previous?.cacheApproved ? {cacheApproved: true} : {}), ...(previous?.cacheBuiltFor ? {cacheBuiltFor: previous.cacheBuiltFor} : {})}; if(changed){ @@ -164,7 +195,10 @@ export function writeArtifact(target: ManagedTarget, content: string, validate: transact([{before:snapshot(path,home,true),after:content},ledgerChange(ledger,env)],[],home, {source:target==='tmuxConfig'?'tmux':'theme-bridge',directory:journalDirectory(home,env),label:`Generate ${target}`}); }catch(error){return {ok:false,error:`Writing ${path} failed: ${error instanceof Error?error.message:String(error)}. The previous file was kept.`};} - }else saveLedger(ledger,env); + }else{ + try {commitLedger(ledger,env,`Update ${target} record`);} + catch(error){return {ok:false,error:`Updating the Theme Bridge record for ${path} failed: ${error instanceof Error?error.message:String(error)}.`};} + } return {ok: true, path, changed}; } @@ -183,7 +217,9 @@ export function removeArtifact(target: ManagedTarget, env: NodeJS.ProcessEnv = p transact([{before:snapshot(path,home,true)},ledgerChange(ledger,env)],[],home, {source:'theme-bridge',directory:journalDirectory(home,env),label:`Remove ${target}`}); }catch(error){return {ok:false,error:error instanceof Error?error.message:String(error)};} - }else saveLedger(ledger,env); + }else{ + try {commitLedger(ledger,env,`Forget ${target} record`);}catch(error){return {ok:false,error:error instanceof Error?error.message:String(error)};} + } return {ok: true, removed: state === 'owned'}; } @@ -309,7 +345,7 @@ export function applyHookRemoval(target: HookTarget, plan: FileEditPlan | undefi if(plan){ const result=applyPlan(plan,{home:env.HOME || homedir(),source:'theme-bridge',directory:journalDirectory(env.HOME || homedir(),env),label:plan.reason,companions:[ledgerChange(ledger,env)]}); if(!result.ok)return {ok:false,error:result.reason}; - }else if(entry)saveLedger(ledger,env); + }else if(entry)commitLedger(ledger,env,`Forget ${target} include record`); }catch(error){return {ok:false,error:error instanceof Error?error.message:String(error)};} return {ok: true}; } diff --git a/src/themeBridge/runtime.ts b/src/themeBridge/runtime.ts index e5ef9a73..9562afb1 100644 --- a/src/themeBridge/runtime.ts +++ b/src/themeBridge/runtime.ts @@ -14,7 +14,7 @@ import { } from './model.js'; import {bridgeEnvPath, writeEnvironmentFiles, type BridgeEnvironment, type ListingWrapper} from './environment.js'; import { - artifactPath, batConfigDirectory, helixConfigDirectory, hookSpec, recordedHook, ledgerPath, loadLedger, ownership, removeArtifact, saveLedger, sha256, writeArtifact, + artifactPath, batConfigDirectory, helixConfigDirectory, hookSpec, recordedHook, ledgerPath, loadLedger, ownership, removeArtifact, commitLedger, sha256, writeArtifact, type HookTarget, type ManagedTarget, } from './artifacts.js'; import {loadTmuxModel} from '../tools/config/tmux.js'; @@ -493,7 +493,8 @@ export async function buildBatCache(env: NodeJS.ProcessEnv = process.env, approv if (!list.stdout.split('\n').some(line => line.trim() === BAT_THEME_NAME)) return {ok: false, message: 'bat rebuilt its cache but does not list the NMSh theme; BAT_THEME stays unset.'}; const fresh = loadLedger(env); fresh.entries.bat = {...fresh.entries.bat!, cacheBuiltFor: fresh.entries.bat!.sha256, ...(approve || fresh.entries.bat!.cacheApproved ? {cacheApproved: true} : {})}; - saveLedger(fresh, env); + try { commitLedger(fresh, env, 'Record bat theme cache'); } + catch (error) { return {ok: false, message: `bat rebuilt its cache, but NMSh could not record it: ${error instanceof Error ? error.message : String(error)}`}; } return {ok: true, message: 'bat rebuilt its theme cache and lists nmsh-bridge; NMSh shells use it from their next prompt.'}; } diff --git a/tests/backupsIntegrity.test.ts b/tests/backupsIntegrity.test.ts new file mode 100644 index 00000000..b9940b6f --- /dev/null +++ b/tests/backupsIntegrity.test.ts @@ -0,0 +1,277 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import {spawn, spawnSync} from 'node:child_process'; +import {chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, symlinkSync, writeFileSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import {dirname, join, resolve} from 'node:path'; +import {fileURLToPath, pathToFileURL} from 'node:url'; +import {Journal} from '../src/backups/journal.js'; +import {snapshot, transact} from '../src/appearance/hostFiles.js'; +import {planRestore, restore, manualSnapshot} from '../src/backups/restore.js'; +import {createChangesPanel, renderChangesPanel} from '../src/backups/ChangesPanel.js'; +import {ledgerPath, loadLedger, ownership, writeArtifact, applyHook, hookSpec, planHook, removeArtifact} from '../src/themeBridge/artifacts.js'; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +type T = {after(fn: () => void): void}; +function box(t: T) { + const home = realpathSync(mkdtempSync(join(tmpdir(), 'integrity-'))); + t.after(() => { try { spawnSync('chmod', ['-R', 'u+rwx', home]); } catch { /* best effort */ } rmSync(home, {recursive: true, force: true}); }); + const env = {...process.env, HOME: home, XDG_CONFIG_HOME: join(home, '.config')}; + const journal = new Journal({home, directory: join(home, '.config/nmsh/backups')}); + return {home, env, journal}; +} +const meta = (themeRef = 'one') => ({mode: 'follow' as const, themeRef, format: 'vim', formatVersion: 1}); +const write = (path: string, text: string) => { mkdirSync(dirname(path), {recursive: true}); writeFileSync(path, text); }; +const plainRender = (lines: string[]) => lines.join('\n').replace(/\x1b\[[0-9;]*m/gu, ''); + +// ---- D2: Theme Bridge ledger -------------------------------------------------------------------------------------------- + +test('D2: ordinary launches re-applying the same colors never rewrite the Theme Bridge ledger or add backups', t => { + const b = box(t); + assert.equal(writeArtifact('vim', 'colors', () => true, meta(), b.env, new Date('2026-10-01T00:00:00Z')).ok, true); + const before = readFileSync(ledgerPath(b.env), 'utf8'); const inode = statSync(ledgerPath(b.env)).ino; const entries = b.journal.list().length; + for (let launch = 1; launch <= 5; launch++) { + const result = writeArtifact('vim', 'colors', () => true, meta(), b.env, new Date(Date.UTC(2026, 9, 1 + launch))); + assert.deepEqual(result, {ok: true, path: join(b.home, '.config/nmsh/theme-bridge/vim/colors/nmsh-bridge.vim'), changed: false}); + } + assert.equal(readFileSync(ledgerPath(b.env), 'utf8'), before, 'ledger bytes unchanged'); + assert.equal(statSync(ledgerPath(b.env)).ino, inode, 'ledger was not replaced'); + assert.equal(b.journal.list().length, entries, 'no extra backups'); + assert.ok(b.journal.list().every(entry => entry.files.every(file => b.journal.status(file) === 'unchanged since'))); +}); + +test('D2: a real record change with the same file content is journaled and restorable', t => { + const b = box(t); + writeArtifact('vim', 'colors', () => true, meta('one'), b.env); + assert.equal(writeArtifact('vim', 'colors', () => true, meta('two'), b.env).ok, true); + const [latest] = b.journal.list(); + assert.equal(latest!.source, 'theme-bridge'); assert.match(latest!.label, /vim record/u); + assert.deepEqual(latest!.files.map(file => file.path), [ledgerPath(b.env)]); + assert.equal(loadLedger(b.env).entries.vim!.themeRef, 'two'); +}); + +test('D2: restoring an older Theme Bridge backup never forgets a managed Vim file written later', t => { + const b = box(t); + assert.equal(writeArtifact('neovim', '-- lua', () => true, {...meta(), format: 'lua'}, b.env).ok, true); + const first = b.journal.list()[0]!; + assert.equal(writeArtifact('vim', '" vim', () => true, meta(), b.env).ok, true); + assert.equal(ownership('vim', loadLedger(b.env), b.env), 'owned'); + const plan = planRestore(b.journal, first.id); + assert.deepEqual(plan.edited, [], 'NMSh bookkeeping is reconciled, not reported as a user edit'); + assert.match(plan.preview.join('\n'), /keeps NMSh's current ownership record for vim/u); + restore(b.journal, plan, {reviewed: true, overwrite: false}); + const ledger = loadLedger(b.env); + assert.equal(ownership('vim', ledger, b.env), 'owned', 'vim stays managed'); + assert.equal(ownership('neovim', ledger, b.env), 'absent', 'the restored neovim file is gone, and so is its record'); + assert.equal(ledger.entries.neovim, undefined); + assert.equal(removeArtifact('vim', b.env).ok, true, 'NMSh can still remove the vim file it wrote'); +}); + +test('D2: restoring an include change keeps the newer artifact record and restores only the include part', t => { + const b = box(t); + writeArtifact('vim', 'v1', () => true, meta(), b.env); + const spec = hookSpec('vim', b.env, b.home); + assert.ok(!('error' in spec), JSON.stringify(spec)); + const planned = planHook(spec, b.home); + assert.ok('plan' in planned, JSON.stringify(planned)); + assert.deepEqual(applyHook('vim', planned.plan, spec, b.env), {ok: true}); + const include = b.journal.list()[0]!; + assert.equal(writeArtifact('vim', 'v2', () => true, meta(), b.env).ok, true); + restore(b.journal, planRestore(b.journal, include.id), {reviewed: true, overwrite: true}); + const ledger = loadLedger(b.env); + assert.equal(ownership('vim', ledger, b.env), 'owned', 'the regenerated v2 file stays owned'); + assert.equal(ledger.entries.vim!.hook, undefined, 'the include record is restored to before the include'); +}); + +// ---- D3: one damaged target ------------------------------------------------------------------------------------------- + +function damagedFixture(t: T) { + const b = box(t); + const kitty = join(b.home, '.config/kitty/kitty.conf'); + write(kitty, 'font_size 12\n'); + transact([{before: snapshot(kitty, b.home), after: 'font_size 13\n'}], [], b.home, {source: 'appearance', label: 'Kitty appearance', directory: b.journal.directory}); + const entry = b.journal.list()[0]!; + const other = join(b.home, '.config/ghostty/config'); + write(other, 'theme = a\n'); + return {...b, kitty, entry, other}; +} +const writeOther = (f: ReturnType, text: string) => + transact([{before: snapshot(f.other, f.home, true), after: text}], [], f.home, {source: 'appearance', label: 'Ghostty appearance', directory: f.journal.directory}); + +test('D3: a recorded directory replaced by a regular file blocks only its own restore, with the exact path and reason', t => { + const f = damagedFixture(t); + rmSync(dirname(f.kitty), {recursive: true}); writeFileSync(dirname(f.kitty), 'not a directory'); + writeOther(f, 'theme = b\n'); + assert.equal(readFileSync(f.other, 'utf8'), 'theme = b\n', 'an unrelated write succeeds'); + assert.equal(f.journal.list({strict: true}).length, 2, 'the journal itself stays readable'); + const seen = f.journal.inspect(f.entry.files[0]!); + assert.equal(seen.status, 'unsafe'); assert.match(seen.reason!, new RegExp(`not a directory: ${dirname(f.kitty)} is a regular file`, 'u')); + assert.throws(() => planRestore(f.journal, f.entry.id), (error: Error) => + error.message.includes(f.entry.id) && error.message.includes(f.kitty) && /Kitty appearance/u.test(error.message) && /Nothing was changed/u.test(error.message)); + assert.equal(readFileSync(dirname(f.kitty), 'utf8'), 'not a directory', 'never written through'); + const panel = createChangesPanel(f.journal, []); panel.selected = 1; + assert.match(plainRender(renderChangesPanel(panel, 160, 30)), /unsafe: A configuration parent is not a directory/u); +}); + +test('D3: symlinks, file↔directory swaps, inaccessible parents and missing files stay isolated', t => { + const f = damagedFixture(t); + // A file where a directory was recorded: the recorded file path is now a directory. + rmSync(f.kitty); mkdirSync(f.kitty); + writeOther(f, 'theme = c\n'); + assert.match(f.journal.inspect(f.entry.files[0]!).reason!, /is a directory/u); + assert.throws(() => planRestore(f.journal, f.entry.id), /not safe to change/u); + // A symbolic link in the parent chain. + rmSync(dirname(f.kitty), {recursive: true}); mkdirSync(join(f.home, 'elsewhere')); symlinkSync(join(f.home, 'elsewhere'), dirname(f.kitty)); + writeOther(f, 'theme = d\n'); + assert.match(f.journal.inspect(f.entry.files[0]!).reason!, /symbolic link/u); + assert.throws(() => planRestore(f.journal, f.entry.id), /symbolic link/u); + // An inaccessible parent. + rmSync(dirname(f.kitty)); mkdirSync(dirname(f.kitty)); writeFileSync(f.kitty, 'x'); chmodSync(dirname(f.kitty), 0o000); + writeOther(f, 'theme = e\n'); + if (process.getuid?.() !== 0) assert.equal(f.journal.inspect(f.entry.files[0]!).status, 'unsafe'); + chmodSync(dirname(f.kitty), 0o700); + // Missing: reported as missing, and a reviewed restore recreates the backed-up file. + rmSync(f.kitty); + assert.equal(f.journal.status(f.entry.files[0]!), 'missing'); + restore(f.journal, planRestore(f.journal, f.entry.id), {reviewed: true, overwrite: true}); + assert.equal(readFileSync(f.kitty, 'utf8'), 'font_size 12\n'); + const skipped: string[] = []; + rmSync(dirname(f.other), {recursive: true}); writeFileSync(dirname(f.other), 'file'); + manualSnapshot(f.journal, [], skipped); + assert.ok(skipped.some(line => line.startsWith(f.other)), 'manual snapshot names the unsafe path it skipped'); +}); + +test('D3: a damaged backup entry is kept for review, never pruned, and does not block other writes', t => { + const b = box(t); + const file = join(b.home, 'a.conf'); write(file, 'a0'); + const small = new Journal({home: b.home, directory: b.journal.directory, maxEntries: 4}); + transact([{before: snapshot(file, b.home), after: 'a1'}], [], b.home, {source: 'cursor', label: 'first', directory: small.directory}); + const damaged = small.list()[0]!; + rmSync(join(small.directory, damaged.id, 'files', '0')); + assert.throws(() => small.record([{before: snapshot(file, b.home), after: 'a2'}], 'cursor', 'same file'), + (error: Error) => error.message.includes(damaged.id) && error.message.includes(file) && /Other files can still be changed/u.test(error.message)); + const other = join(b.home, 'b.conf'); write(other, 'b0'); + for (let index = 2; index < 12; index++) { + const entry = small.record([{before: snapshot(other, b.home), after: `b${index}`}], 'cursor', `write ${index}`); + writeFileSync(other, `b${index}`); small.complete(entry); + } + assert.ok(existsSync(join(small.directory, damaged.id)), 'kept for review'); + small.list(); + assert.match(small.unreadableReasons.get(damaged.id)!, /journal file is missing/u); + assert.ok(small.list().length <= 3, 'it still counts toward the entry cap'); + const panel = createChangesPanel(small, []); + assert.match(panel.message, new RegExp(`${damaged.id}: Backup verify failed`, 'u')); +}); + +// ---- D4: interrupted writes --------------------------------------------------------------------------------------------- + +/** Runs the real transact in a separate process and kills it with SIGKILL at one transaction boundary. */ +async function killAt(home: string, directory: string, stage: string, files: Array<[string, string]>): Promise { + const script = join(home, `kill-${stage.replace(/\W/gu, '-')}.mts`); + writeFileSync(script, ` +import {snapshot, transact} from ${JSON.stringify(pathToFileURL(join(root, 'src/appearance/hostFiles.ts')).href)}; +const files = ${JSON.stringify(files)}; +transact(files.map(([path, after]) => ({before: snapshot(path, ${JSON.stringify(home)}, true), after})), [], ${JSON.stringify(home)}, + {source: 'appearance', label: 'Interrupted write', directory: ${JSON.stringify(directory)}, + fault: stage => { if (stage === ${JSON.stringify(stage)}) process.kill(process.pid, 'SIGKILL'); }}); +process.exit(3); +`); + const child = spawn(process.execPath, ['--import=tsx', script], {cwd: root, env: {...process.env, HOME: home, XDG_CONFIG_HOME: join(home, '.config')}, stdio: 'ignore'}); + const signal = await new Promise(done => child.on('exit', (_code, sig) => done(sig))); + assert.equal(signal, 'SIGKILL', `the writer was killed at ${stage}`); +} +function twoFiles(t: T) { + const b = box(t); + const one = join(b.home, 'one.conf'), two = join(b.home, 'two.conf'); + write(one, 'one-before\n'); write(two, 'two-before\n'); + return {...b, one, two, files: [[one, 'one-after\n'], [two, 'two-after\n']] as Array<[string, string]>}; +} + +test('D4: a write killed at each boundary is classified from the bytes on disk, never assumed applied', async t => { + const cases: Array<[string, string, string, string]> = [ + ['recorded', 'not-applied', 'one-before\n', 'two-before\n'], + ['written:0', 'partially-applied', 'one-after\n', 'two-before\n'], + ['before-complete', 'applied-unfinalized', 'one-after\n', 'two-after\n'], + ]; + for (const [stage, expected, one, two] of cases) { + const f = twoFiles(t); + await killAt(f.home, f.journal.directory, stage, f.files); + assert.equal(readFileSync(f.one, 'utf8'), one); assert.equal(readFileSync(f.two, 'utf8'), two); + const [entry] = f.journal.list(); + assert.equal(entry!.outcome, 'prepared', `${stage}: never marked applied`); + assert.equal(f.journal.transactionState(entry!).state, expected, stage); + const panel = createChangesPanel(f.journal, []); + assert.match(panel.message, /1 unfinished change needs review/u); + } +}); + +test('D4: a later external edit to an interrupted write is reported as a conflict and needs explicit overwrite', async t => { + const f = twoFiles(t); + await killAt(f.home, f.journal.directory, 'written:0', f.files); + writeFileSync(f.one, 'user edit after the crash\n'); + const [entry] = f.journal.list(); + assert.equal(f.journal.transactionState(entry!).state, 'conflicted'); + const plan = planRestore(f.journal, entry!.id); + assert.deepEqual(plan.edited, [f.one], 'only the externally edited file needs the second confirmation'); + assert.throws(() => restore(f.journal, plan, {reviewed: true, overwrite: false}), /later edits/u); + assert.equal(readFileSync(f.one, 'utf8'), 'user edit after the crash\n'); +}); + +test('D4: rollback of a partial write takes an undo checkpoint and settles the entry; finalize needs proof', async t => { + const f = twoFiles(t); + await killAt(f.home, f.journal.directory, 'written:0', f.files); + const [entry] = f.journal.list(); + assert.throws(() => f.journal.finalize(entry!), /partially-applied/u); + const plan = planRestore(f.journal, entry!.id); + assert.deepEqual(plan.edited, [], 'the written file is the one this change wrote; the other already holds its backup'); + assert.match(plan.preview.join('\n'), /interrupted part-way/u); + const undo = restore(f.journal, plan, {reviewed: true, overwrite: false}); + assert.equal(readFileSync(f.one, 'utf8'), 'one-before\n'); assert.equal(readFileSync(f.two, 'utf8'), 'two-before\n'); + const settled = f.journal.read(entry!.id); + assert.equal(settled.outcome, 'prepared', 'older builds still read it'); + assert.equal(settled.resolution?.state, 'reverted'); assert.match(settled.resolution!.detail!, new RegExp(undo.id, 'u')); + assert.equal(f.journal.transactionState(settled).state, 'reverted'); + // The undo checkpoint returns the partial state if the user wants it back. + restore(f.journal, planRestore(f.journal, undo.id), {reviewed: true, overwrite: false}); + assert.equal(readFileSync(f.one, 'utf8'), 'one-after\n'); +}); + +test('D4: an interrupted write whose files all landed can be finalized without touching them', async t => { + const f = twoFiles(t); + await killAt(f.home, f.journal.directory, 'before-complete', f.files); + const [entry] = f.journal.list(); + const stamp = statSync(f.one).mtimeMs; + const done = f.journal.finalize(entry!); + assert.equal(done.outcome, 'applied'); assert.equal(done.resolution?.state, 'finalized'); + assert.equal(statSync(f.one).mtimeMs, stamp); + assert.equal(f.journal.transactionState(done).state, 'applied'); +}); + +test('D4: a write that fails in-process is rolled back and recorded as reverted; a live writer is in progress', t => { + const f = twoFiles(t); + assert.throws(() => transact(f.files.map(([path, after]) => ({before: snapshot(path, f.home, true), after})), [], f.home, + {source: 'appearance', label: 'Failing write', directory: f.journal.directory, fault: stage => { if (stage === 'written:0') throw new Error('disk full'); }}), /disk full.*rolled back/u); + assert.equal(readFileSync(f.one, 'utf8'), 'one-before\n'); + const [entry] = f.journal.list(); + assert.equal(entry!.resolution?.state, 'reverted'); assert.equal(f.journal.transactionState(entry!).state, 'reverted'); + // Another process that is still running owns a prepared entry: never reconciled behind its back. + const sleeper = spawn('sleep', ['30']); t.after(() => sleeper.kill()); + const pending = f.journal.record([{before: snapshot(f.two, f.home), after: 'x'}], 'appearance', 'Live writer'); + writeFileSync(join(f.journal.directory, pending.id, 'owner.json'), JSON.stringify({owner: 'nmsh-journal', version: 1, pid: sleeper.pid})); + assert.equal(f.journal.transactionState(pending).state, 'in-progress'); +}); + +test('D4: unresolved interrupted entries are never pruned, even under a tiny cap', t => { + const b = box(t); + const file = join(b.home, 'c.conf'); write(file, 'c0'); + const small = new Journal({home: b.home, directory: b.journal.directory, maxEntries: 5}); + // The file's original backup comes first, so the interrupted entry is protected only for being unfinished. + const first = small.record([{before: snapshot(file, b.home), after: 'c-first'}], 'cursor', 'original'); writeFileSync(file, 'c-first'); small.complete(first); + const interrupted = small.record([{before: snapshot(file, b.home), after: 'never written'}], 'cursor', 'interrupted'); + for (let index = 1; index < 30; index++) { + const entry = small.record([{before: snapshot(file, b.home, true), after: `c${index}`}], 'cursor', `w${index}`); + writeFileSync(file, `c${index}`); small.complete(entry); + } + assert.ok(small.list().some(entry => entry.id === interrupted.id)); + assert.ok(readdirSync(small.directory).filter(name => /^\d{8}T/u.test(name)).length <= 5); +}); From 48a5976b7e18851985e6185d8c8dba8b434c4c0c Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:39:27 +0530 Subject: [PATCH 2/4] Backups: settle raced and reviewed entries, refuse restores of live writes From independent review: - An edit that lands while the backup is written now settles the entry as reverted (the recheck runs inside the transaction), so it never lingers as an unprunable 'prepared' entry. - /changes: D sets an unfinished entry aside as reviewed without touching files; it then no longer needs review and can be pruned. Rollback-incomplete entries stay protected. - A restore of an entry another live NMSh process is writing is refused; a 'live' writer of an entry older than 10 minutes is treated as a reused PID. - A damaged older copy of NMSh's own JSON records no longer blocks writes that re-capture them; user files named by a damaged entry still fail closed. - Record-only Theme Bridge entries restore their record (only where it still describes the same file bytes); restoring the first record removes it. - The unjournaled saveLedger is gone. --- src/appearance/hostFiles.ts | 6 ++-- src/backups/ChangesPanel.ts | 6 +++- src/backups/describe.ts | 5 ++-- src/backups/journal.ts | 24 +++++++++++---- src/backups/restore.ts | 19 ++++++++---- src/themeBridge/artifacts.ts | 8 ----- tests/backupsIntegrity.test.ts | 55 +++++++++++++++++++++++++++++++++- tests/themeBridge.test.ts | 4 +-- 8 files changed, 100 insertions(+), 27 deletions(-) diff --git a/src/appearance/hostFiles.ts b/src/appearance/hostFiles.ts index e18bbb89..dbe7cb1e 100644 --- a/src/appearance/hostFiles.ts +++ b/src/appearance/hostFiles.ts @@ -97,12 +97,12 @@ export function transact(changes: readonly FileChange[], dependencies: readonly let entry:JournalEntry; 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 { 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; diff --git a/src/backups/ChangesPanel.ts b/src/backups/ChangesPanel.ts index 4a9db5e6..581fa5d7 100644 --- a/src/backups/ChangesPanel.ts +++ b/src/backups/ChangesPanel.ts @@ -41,6 +41,9 @@ export function changesKey(state:ChangesPanel,key:Key):{close?:true;paths?:strin 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.`; @@ -91,7 +94,7 @@ export function renderChangesPanel(state:ChangesPanel,columns:number,height:numb 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'],...(entry&&state.states.get(entry.id)?.state==='applied-unfinalized'?[['F','mark applied'] as const]:[]),['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[] { @@ -102,6 +105,7 @@ function stateDetails(state:ChangesPanel,entry:JournalEntry):string[] { 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; } diff --git a/src/backups/describe.ts b/src/backups/describe.ts index e0834642..042c6cdb 100644 --- a/src/backups/describe.ts +++ b/src/backups/describe.ts @@ -5,7 +5,7 @@ export function describeState(state: TransactionState): string { switch (state) { case 'applied': return 'applied'; case 'in-progress': return 'still being written by another NMSh process'; - case 'not-applied': return 'interrupted before any file was written (nothing changed)'; + case 'not-applied': return 'interrupted before any file was written (the files still hold their backed-up bytes)'; case 'partially-applied': return 'interrupted part-way: some files were written, others were not'; case 'applied-unfinalized': return 'interrupted after every file was written, before it was marked done'; case 'conflicted': return 'interrupted, and a file was changed by something else since'; @@ -13,8 +13,9 @@ export function describeState(state: TransactionState): string { case 'rollback-incomplete': return 'failed, and its rollback was incomplete'; case 'partial': return 'partly applied (some areas failed or were skipped)'; case 'indeterminate': return 'unfinished, and a file cannot be read safely to check it'; + case 'reviewed': return 'reviewed and set aside (files left as they were)'; } } /** Whether an entry needs the user's review before it can be trusted as finished. */ -export const needsReview = (state: TransactionState) => !['applied', 'reverted', 'in-progress'].includes(state); +export const needsReview = (state: TransactionState) => !['applied', 'reverted', 'in-progress', 'reviewed'].includes(state); diff --git a/src/backups/journal.ts b/src/backups/journal.ts index 785d1e49..96e6e88c 100644 --- a/src/backups/journal.ts +++ b/src/backups/journal.ts @@ -11,14 +11,16 @@ export interface JournalFile {path: string; existed: boolean; mode: number; befo * How an unfinished write ended. `outcome` keeps only 'prepared' | 'applied' so older NMSh builds can still read every entry; * the resolution is extra evidence they ignore. */ -export interface JournalResolution {state: 'reverted' | 'rollback-incomplete' | 'finalized' | 'partial'; at: string; detail?: string} +export interface JournalResolution {state: 'reverted' | 'rollback-incomplete' | 'finalized' | 'partial' | 'reviewed'; at: string; detail?: string} export interface JournalEntry {version: 1; id: string; time: string; source: string; label: string; files: JournalFile[]; outcome: 'prepared' | 'applied'; resolution?: JournalResolution; group?: string} /** What the files on disk say about an entry. Computed from bytes and hashes only; never assumed. */ -export type TransactionState = 'applied' | 'in-progress' | 'not-applied' | 'partially-applied' | 'applied-unfinalized' | 'conflicted' | 'reverted' | 'rollback-incomplete' | 'partial' | 'indeterminate'; +export type TransactionState = 'applied' | 'in-progress' | 'not-applied' | 'partially-applied' | 'applied-unfinalized' | 'conflicted' | 'reverted' | 'rollback-incomplete' | 'partial' | 'indeterminate' | 'reviewed'; export type FileEvidence = 'before' | 'after' | 'neither' | 'unsafe'; export interface TransactionReport {state: TransactionState; files: FileEvidence[]; reasons: string[]; writer?: number} export interface FileInspection {status: 'unchanged since' | 'edited since' | 'missing' | 'unsafe'; reason?: string} -const RESOLUTIONS = ['reverted', 'rollback-incomplete', 'finalized', 'partial']; +const RESOLUTIONS = ['reverted', 'rollback-incomplete', 'finalized', 'partial', 'reviewed']; +/** A journal write takes well under a second; a "live" writer of an older entry is a reused process id, not the writer. */ +const LIVE_WRITER_MS = 10 * 60 * 1000; const message = (error: unknown) => error instanceof Error ? error.message : String(error); export interface JournalMetadata { source: string; label: string; directory?: string; @@ -143,6 +145,7 @@ export class Journal { try { owner = JSON.parse(snapshot(join(this.directory, entry.id, 'owner.json'), this.options.home).content ?? '{}'); } catch { return undefined; } const pid = owner.pid; if (!Number.isSafeInteger(pid) || pid! <= 0 || pid === process.pid) return undefined; + if (!(Date.now() - Date.parse(entry.time) < LIVE_WRITER_MS)) return undefined; try { process.kill(pid!, 0); return pid; } catch (error) { return (error as NodeJS.ErrnoException).code === 'EPERM' ? pid : undefined; } } /** @@ -173,6 +176,13 @@ export class Journal { this.atomic(join(this.directory, entry.id, 'manifest.json'), `${JSON.stringify(next, null, 2)}\n`); return this.read(entry.id); } + /** Set an unfinished entry aside after review: files are left exactly as they are, the evidence stays, and it becomes prunable. */ + dismiss(entry: JournalEntry): JournalEntry { + const report = this.transactionState(this.read(entry.id)); + if (report.state === 'in-progress') throw new Error('Another NMSh process is still writing this change; wait for it to finish.'); + if (report.state === 'applied' || report.state === 'reverted' || report.state === 'reviewed') throw new Error('This change is already settled.'); + return this.resolve(entry, {state: 'reviewed', detail: `Reviewed and set aside while ${report.state}; files were left as they were.`}); + } /** Mark an interrupted entry finished, only when every file verifiably holds the bytes it was meant to write. */ finalize(entry: JournalEntry): JournalEntry { const report = this.transactionState(this.read(entry.id)); @@ -281,8 +291,12 @@ export class Journal { const existing = this.list(); const damaged = [...this.unreadable]; // Fail closed only for the file a damaged backup names: its history needs review before NMSh changes it again. + // NMSh's own JSON records (settings, ownership) ride along with many writes; they are always re-captured in full, so a damaged + // older copy of one never blocks them. Each named user file does fail closed. + const own = dirname(this.directory); + const bookkeeping = (path: string) => path.endsWith('.json') && path.startsWith(`${own}/`); for (const name of damaged) { - const path = changes.map(change => change.before.path).find(candidate => this.unreadablePaths.get(name)!.includes(candidate)); + const path = changes.map(change => change.before.path).find(candidate => !bookkeeping(candidate) && this.unreadablePaths.get(name)!.includes(candidate)); if (path) throw new Error(`Backup ${name} of ${path} failed verification (${this.unreadableReasons.get(name)}); review it in /changes. ` + `To continue, move ${join(this.directory, name)} out of the backups folder. Other files can still be changed.`); } @@ -296,7 +310,7 @@ export class Journal { const victims: JournalEntry[] = []; const keep = new Set(existing.filter(item=>protectedSource(item.source)).slice(0,protectedSource(source)?19:20).map(item=>item.id)); // An unfinished write is evidence for recovery (or still in flight in another process): never pruned automatically. - for (const item of existing) if (item.outcome==='prepared'&&!item.resolution) keep.add(item.id); + for (const item of existing) if ((item.outcome==='prepared'&&!item.resolution)||item.resolution?.state==='rollback-incomplete') keep.add(item.id); // The incoming entry anchors its source/paths; retain the newest anchor for every other pair. const anchored = new Set(changes.map(change=>`${source}:${change.before.path}`)); for (const item of existing) for(const file of item.files) { diff --git a/src/backups/restore.ts b/src/backups/restore.ts index 6445f398..b3ce9f9d 100644 --- a/src/backups/restore.ts +++ b/src/backups/restore.ts @@ -37,6 +37,9 @@ function reconcileBridgeLedger(journal: Journal, entry: JournalEntry, changes: F const change = changes.find(item => item.before.path === path); if (!change) return []; const restoredPaths = new Set(entry.files.map(file => file.path)); + // A record-only change (bat cache, a theme reference, a forgotten include) touches no tool file: restore each tool's record + // where it still describes the same file bytes, so ownership stays exact. + const recordOnly = entry.files.every(file => file.path === path || file.path === join(dirname(journal.directory), 'config.json')); const backup = rawLedger(change.after), current = rawLedger(change.before.content); const merged: Record = {}; const notes: string[] = []; @@ -50,7 +53,8 @@ function reconcileBridgeLedger(journal: Journal, entry: JournalEntry, changes: F const hook = touches(old, 'hook') || touches(now, 'hook'); let next: RawRecord | undefined; if (artifact && hook) next = old; - else if (!artifact && !hook) next = now; + else if (!artifact && !hook) next = recordOnly && old && now && old.sha256 === now.sha256 && old.artifactPath === now.artifactPath + ? (() => { const kept = {...old}; if (now.hook) kept.hook = now.hook; else delete kept.hook; return kept; })() : now; else if (artifact) { // The backup's file record, with the include NMSh records now (this backup does not restore the include's file). const keptHook = now?.hook; @@ -62,10 +66,12 @@ function reconcileBridgeLedger(journal: Journal, entry: JournalEntry, changes: F else if (restoredHook) next = old; } if (next) merged[target] = next; - if (!artifact && !hook && JSON.stringify(old) !== JSON.stringify(now)) notes.push(`Theme Bridge: keeps NMSh's current ownership record for ${target}; this backup does not restore its files.`); + if (!artifact && !hook && JSON.stringify(next) !== JSON.stringify(old)) notes.push(`Theme Bridge: keeps NMSh's current ownership record for ${target}; this backup does not restore its files.`); } - const after = `${JSON.stringify({...current.root, version: 1, entries: merged}, null, 2)}\n`; + // A backup from before the record existed, with nothing left to own, removes the record again. + const after = change.after === undefined && !Object.keys(merged).length ? undefined : `${JSON.stringify({...current.root, version: 1, entries: merged}, null, 2)}\n`; if (after === change.after) return []; + if (recordOnly && JSON.stringify(merged) === JSON.stringify(current.entries)) throw new Error(`Restoring "${entry.label}" would change nothing: every tool's current record describes files this backup does not restore. Nothing was changed.`); change.after = after; return notes.length ? notes : ['Theme Bridge: the ownership record is reconciled with files this backup does not restore.']; } @@ -73,6 +79,8 @@ function reconcileBridgeLedger(journal: Journal, entry: JournalEntry, changes: F export function planRestore(journal: Journal, id: string): RestorePlan { const entry = journal.read(id); const home = journal.options.home; + const live = journal.transactionState(entry); + if (live.state === 'in-progress') throw new Error(`NMSh process ${live.writer} is still writing "${entry.label}"; wait for it to finish, then review again. Nothing was changed.`); const changes: FileChange[] = entry.files.map((file,index) => ({before:checkedSnapshot(entry,file.path,home),after:journal.content(entry,index),afterMode:file.mode})); // Only a file this restore would actually change can lose later edits: one already holding its backed-up bytes is not edited. const edited = changes.filter((change,index)=>{ @@ -81,12 +89,13 @@ export function planRestore(journal: Journal, id: string): RestorePlan { return now !== file.afterHash || (file.afterMode!==undefined && change.before.content!==undefined && change.before.mode!==file.afterMode); }).map(change=>change.before.path); const settings=join(dirname(journal.directory),'config.json'); - if(!changes.some(change=>change.before.path===settings)){const before=checkedSnapshot(entry,settings,home);changes.push({before,after:before.content,afterMode:before.mode});} + // The settings checkpoint is a courtesy: an unreadable settings file is left out rather than blocking an unrelated restore. + if(!changes.some(change=>change.before.path===settings)){try{const before=snapshot(settings,home,true);changes.push({before,after:before.content,afterMode:before.mode});}catch{/* not part of this backup */}} const reconciled = reconcileBridgeLedger(journal, entry, changes); const ledger = join(dirname(journal.directory), 'theme-bridge', 'ledger.json'); // The reconciled ownership record is NMSh's own bookkeeping, not a user edit that would be lost. const warnings = reconciled.length ? edited.filter(path => path !== ledger) : edited; - const state = journal.transactionState(entry); + const state = live; const plan: RestorePlan = {entry,changes,edited:warnings,preview:[ `${entry.time} · ${entry.source} · ${entry.label}`, ...(state.state === 'applied' ? [] : [`This change is ${describeState(state.state)}. Restoring returns its files to their backed-up state (a rollback).`]), diff --git a/src/themeBridge/artifacts.ts b/src/themeBridge/artifacts.ts index 28b0a446..6b40c202 100644 --- a/src/themeBridge/artifacts.ts +++ b/src/themeBridge/artifacts.ts @@ -117,14 +117,6 @@ export function loadLedgerText(text: string, env: NodeJS.ProcessEnv = process.en return {version: 1, entries}; } -export function saveLedger(ledger: Ledger, env: NodeJS.ProcessEnv = process.env): void { - const path = ledgerPath(env); - mkdirSync(dirname(path), {recursive: true, mode: 0o700}); - const staged = `${path}.${process.pid}.tmp`; - writeFileSync(staged, `${JSON.stringify(ledger, null, 2)}\n`, {encoding: 'utf8', mode: 0o600}); - renameSync(staged, path); -} - function ledgerChange(ledger:Ledger,env:NodeJS.ProcessEnv):FileChange { return {before:snapshot(ledgerPath(env),env.HOME || homedir(),true),after:`${JSON.stringify(ledger,null,2)}\n`}; } diff --git a/tests/backupsIntegrity.test.ts b/tests/backupsIntegrity.test.ts index b9940b6f..a7c4d8b9 100644 --- a/tests/backupsIntegrity.test.ts +++ b/tests/backupsIntegrity.test.ts @@ -8,7 +8,7 @@ import {fileURLToPath, pathToFileURL} from 'node:url'; import {Journal} from '../src/backups/journal.js'; import {snapshot, transact} from '../src/appearance/hostFiles.js'; import {planRestore, restore, manualSnapshot} from '../src/backups/restore.js'; -import {createChangesPanel, renderChangesPanel} from '../src/backups/ChangesPanel.js'; +import {createChangesPanel, changesKey, renderChangesPanel} from '../src/backups/ChangesPanel.js'; import {ledgerPath, loadLedger, ownership, writeArtifact, applyHook, hookSpec, planHook, removeArtifact} from '../src/themeBridge/artifacts.js'; const root = resolve(dirname(fileURLToPath(import.meta.url)), '..'); @@ -275,3 +275,56 @@ test('D4: unresolved interrupted entries are never pruned, even under a tiny cap assert.ok(small.list().some(entry => entry.id === interrupted.id)); assert.ok(readdirSync(small.directory).filter(name => /^\d{8}T/u.test(name)).length <= 5); }); + +// ---- Review follow-ups ------------------------------------------------------------------------------------------------ + +test('an edit that lands while the backup is written settles the entry as reverted, never as an orphan', t => { + const f = twoFiles(t); + assert.throws(() => transact(f.files.map(([path, after]) => ({before: snapshot(path, f.home, true), after})), [], f.home, + {source: 'appearance', label: 'Raced', directory: f.journal.directory, fault: stage => { if (stage === 'recorded') writeFileSync(f.one, 'user typed this\n'); }}), /changed since the preview/u); + assert.equal(readFileSync(f.one, 'utf8'), 'user typed this\n'); + const [entry] = f.journal.list(); + assert.equal(entry!.resolution?.state, 'reverted'); assert.match(entry!.resolution!.detail!, /No file was changed/u); +}); + +test('a conflicted interrupted entry can be set aside without touching files; a live writer blocks restore', async t => { + const f = twoFiles(t); + await killAt(f.home, f.journal.directory, 'written:0', f.files); + writeFileSync(f.one, 'later edit\n'); + const [entry] = f.journal.list(); + const panel = createChangesPanel(f.journal, []); + assert.match(plainRender(renderChangesPanel(panel, 160, 30)), /D set aside/u); + changesKey(panel, {kind: 'text', value: 'd'}); + assert.equal(f.journal.transactionState(f.journal.read(entry!.id)).state, 'reviewed'); + assert.equal(readFileSync(f.one, 'utf8'), 'later edit\n'); assert.equal(readFileSync(f.two, 'utf8'), 'two-before\n'); + assert.doesNotMatch(createChangesPanel(f.journal, []).message, /unfinished/u); + const sleeper = spawn('sleep', ['30']); t.after(() => sleeper.kill()); + const pending = f.journal.record([{before: snapshot(f.two, f.home), after: 'x'}], 'appearance', 'Live writer'); + writeFileSync(join(f.journal.directory, pending.id, 'owner.json'), JSON.stringify({owner: 'nmsh-journal', version: 1, pid: sleeper.pid})); + assert.throws(() => planRestore(f.journal, pending.id), new RegExp(`process ${sleeper.pid} is still writing`, 'u')); +}); + +test('a damaged backup of NMSh settings never blocks later settings or ownership writes', t => { + const b = box(t); + const config = join(b.home, '.config/nmsh/config.json'); write(config, '{}\n'); + transact([{before: snapshot(config, b.home), after: '{"a":1}\n'}], [], b.home, {source: 'settings', label: 'settings', directory: b.journal.directory}); + const damaged = b.journal.list()[0]!; + rmSync(join(b.journal.directory, damaged.id, 'files', '0')); + transact([{before: snapshot(config, b.home, true), after: '{"a":2}\n'}], [], b.home, {source: 'settings', label: 'again', directory: b.journal.directory}); + assert.equal(readFileSync(config, 'utf8'), '{"a":2}\n'); + assert.equal(writeArtifact('vim', 'v', () => true, meta(), b.env).ok, true); +}); + +test('restoring a record-only Theme Bridge change restores that record; restoring the first record removes it', t => { + const b = box(t); + writeArtifact('vim', 'colors', () => true, meta('one'), b.env); + const created = b.journal.list()[0]!; + writeArtifact('vim', 'colors', () => true, meta('two'), b.env); + const recordOnly = b.journal.list()[0]!; + restore(b.journal, planRestore(b.journal, recordOnly.id), {reviewed: true, overwrite: false}); + assert.equal(loadLedger(b.env).entries.vim!.themeRef, 'one'); + assert.equal(ownership('vim', loadLedger(b.env), b.env), 'owned'); + restore(b.journal, planRestore(b.journal, created.id), {reviewed: true, overwrite: true}); + assert.equal(existsSync(ledgerPath(b.env)), false, 'no empty record is left behind'); + assert.equal(existsSync(join(b.home, '.config/nmsh/theme-bridge/vim/colors/nmsh-bridge.vim')), false); +}); diff --git a/tests/themeBridge.test.ts b/tests/themeBridge.test.ts index 9b1e76b8..a2bc771b 100644 --- a/tests/themeBridge.test.ts +++ b/tests/themeBridge.test.ts @@ -17,7 +17,7 @@ import { validateVimColorscheme, vimColorscheme, withFzfTheme, TMUX_STYLE_OPTIONS, } from '../src/themeBridge/targets.js'; import { - applyHook, applyHookRemoval, artifactPath, hookSpec, ledgerPath, loadLedger, ownership, planHook, planHookRemoval, removeArtifact, saveLedger, writeArtifact, + applyHook, applyHookRemoval, artifactPath, hookSpec, ledgerPath, loadLedger, ownership, planHook, planHookRemoval, removeArtifact, commitLedger, writeArtifact, } from '../src/themeBridge/artifacts.js'; import {resolveCommand} from '../src/providers/providers.js'; import {execFileSync} from 'node:child_process'; @@ -414,7 +414,7 @@ test('delta: never managed; its status says whether git config pins the syntax t assert.ok(written.ok); const ledger = loadLedger(box.env); ledger.entries.bat = {...ledger.entries.bat!, cacheBuiltFor: ledger.entries.bat!.sha256}; - saveLedger(ledger, box.env); + commitLedger(ledger, box.env); report = delta(follow); assert.equal(report.readiness, 'Syntax via bat', 'BAT_THEME=nmsh-bridge reaches NMSh shells, and delta reads it'); assert.match(report.notes.join(' '), /BAT_THEME[\s\S]*bat's theme cache[\s\S]*different bat version/u); From 70fa963ddea5d3f03e305db99cb191deee302a82 Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:03:03 +0530 Subject: [PATCH 3/4] Backups test: match the damaged-target notice regardless of line wrapping --- tests/backupsIntegrity.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/backupsIntegrity.test.ts b/tests/backupsIntegrity.test.ts index a7c4d8b9..e3d72178 100644 --- a/tests/backupsIntegrity.test.ts +++ b/tests/backupsIntegrity.test.ts @@ -22,7 +22,7 @@ function box(t: T) { } const meta = (themeRef = 'one') => ({mode: 'follow' as const, themeRef, format: 'vim', formatVersion: 1}); const write = (path: string, text: string) => { mkdirSync(dirname(path), {recursive: true}); writeFileSync(path, text); }; -const plainRender = (lines: string[]) => lines.join('\n').replace(/\x1b\[[0-9;]*m/gu, ''); +const plainRender = (lines: string[]) => lines.join('\n').replace(/\x1b\[[0-9;]*m/gu, '').replace(/\s+/gu, ' '); // ---- D2: Theme Bridge ledger -------------------------------------------------------------------------------------------- From b0b473ba39f7e5241300d56e98b1053b8581c9be Mon Sep 17 00:00:00 2001 From: raiseCatError <315733358+raiseCatError@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:19:00 +0530 Subject: [PATCH 4/4] Backups test: check the damaged-target status directly, not across side-by-side columns --- tests/backupsIntegrity.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/backupsIntegrity.test.ts b/tests/backupsIntegrity.test.ts index e3d72178..3faa0118 100644 --- a/tests/backupsIntegrity.test.ts +++ b/tests/backupsIntegrity.test.ts @@ -109,8 +109,10 @@ test('D3: a recorded directory replaced by a regular file blocks only its own re assert.throws(() => planRestore(f.journal, f.entry.id), (error: Error) => error.message.includes(f.entry.id) && error.message.includes(f.kitty) && /Kitty appearance/u.test(error.message) && /Nothing was changed/u.test(error.message)); assert.equal(readFileSync(dirname(f.kitty), 'utf8'), 'not a directory', 'never written through'); + // List and details render side by side, so check the reported status itself plus the row marker (wrapping varies with path length). const panel = createChangesPanel(f.journal, []); panel.selected = 1; - assert.match(plainRender(renderChangesPanel(panel, 160, 30)), /unsafe: A configuration parent is not a directory/u); + assert.match(panel.statuses.get(`${f.entry.id}:${f.kitty}`)!, /^unsafe: A configuration parent is not a directory: /u); + assert.match(plainRender(renderChangesPanel(panel, 160, 30)), /unsafe file/u); }); test('D3: symlinks, file↔directory swaps, inaccessible parents and missing files stay isolated', t => {