feat: delete backup-less configs on remove and restore (revert #196)

AGENTS.md

@@ -110,9 +110,20 @@ The settings.json backup itself is independent and still happens at configure ti
 
 The install flow's "Skip configuration" auth choice routes Configure through `backupOnly(tool)` instead of the per-agent `configure*` functions: it runs the same `ensureBackup` logic for the agent's main config file (so any existing live config is snapshotted to `*.backup` exactly once) and then exits without writing CoDev's own config. `Configure` accepts `creds: Credentials | null`; `null` is the signal to take this backup-only path, and the finalize Phase reads the same `creds === null` signal to pick `backupClaudeAuth` over `resetClaudeAuth`.
 
-`restoreTool` returns `RestoreResult[]` — a length-1 array for single-file tools, length-3 for any Claude tool (settings.json + .claude.json + .credentials.json, in that order). Each file ends in one of three states: `restored` (a `*.backup` existed → swapped over the live file), `kept-live` (no backup, but a live file exists → **left untouched**; with no backup we can't know what preceded CoDev, so we don't destroy the current config), or `noop` (neither file exists). Callers iterate. `runRestoreOrKeep` (in `src/lib/remove.ts`) rolls Claude's three results into one aggregated step (`restored 2 files; kept 1 file (no backup)` style); `runRestore` / `runRestoreAll` (in `src/lib/restore.ts`) print one line per file. In the sweep, only a genuine `restored` counts as action — an all-`kept-live`/`noop` run exits 1 with "No backups found."
+`restoreTool` returns `RestoreResult[]` — a length-1 array for single-file tools, length-3 for any Claude tool (settings.json + .claude.json + .credentials.json, in that order). Each file ends in one of three states:
 
-`restoreTool` is invoked via `codevhub restore <agent>` (one tool) or bare `codevhub restore` (sweep all tools with a backup). The dispatcher accepts **launch names** — `claude`/`codex`/`opencode`/`codev` — and `toolForRestoreAgent` in `src/lib/restore.ts` maps them to the internal `Tool` type. Behavior splits on path: `runRestore` (single) treats a missing backup as an error and exits 1; `runRestoreAll` (sweep) skips tools without backups silently, only erroring when *every* tool was skipped. Keep that asymmetry — it's right for both contexts.
+- `restored` — a `*.backup` existed → swapped over the live file. Checked first, so a backup always wins.
+- `deleted-live` — no backup, but a live file exists → **the live file is deleted**. `ensureBackup` only skips the backup step when there was nothing to back up, so "no file" is a legitimate pre-CoDev state.
+- `noop` — neither file exists; already pre-CoDev.
+
+**The delete is unconditional, and that is the design.** Both entry points (`codevhub remove` and `codevhub restore`) mean "I want this machine without CoDev", so they are destructive by construction and no ownership check gates the deletion. Two consequences to keep in mind rather than "fix":
+
+- A config the user hand-wrote **after** install has no backup either, and is deleted too — including for a tool CoDev never configured, since `remove` and the bare `restore` sweep all five tools unconditionally.
+- Claude's `~/.claude.json` and `~/.claude/.credentials.json` ride along in the same length-3 bundle, so a backup-less one goes even though it may hold Claude's own project history or a login made after install. This is intended: `remove` prompts for confirmation first, and a leftover login is exactly the CoDev-era state the command exists to erase.
+
+Callers iterate. `runRestoreOrDelete` (in `src/lib/remove.ts`) rolls Claude's three results into one aggregated step (`restored 2 files; deleted 1 file (no backup)` style); `runRestore` / `runRestoreAll` (in `src/lib/restore.ts`) print one line per file. In the sweep, a `deleted-live` **counts as action just like a `restored`** — only an all-`noop` run exits 1 with "No backups found." Deleting seven configs and then reporting "nothing to restore" would be a lie, so keep `deleted-live` out of the noop bucket.
+
+`restoreTool` is invoked via `codevhub restore <agent>` (one tool) or bare `codevhub restore` (sweep all tools). The dispatcher accepts **launch names** — `claude`/`codex`/`opencode`/`codev` — and `toolForRestoreAgent` in `src/lib/restore.ts` maps them to the internal `Tool` type. `runRestore` (single) always returns 0 — a missing backup is not an error, it just reports what it deleted; `runRestoreAll` (sweep) returns 1 only when every result was a noop, or when a tool threw.
 
 ## Config refresh and upload self-healing
 

src/RemoveApp.tsx

@@ -36,7 +36,6 @@ export function RemoveApp({ skipConfirm = false }: RemoveAppProps) {
 						},
 					],
 					anyFailed: true,
-					keptPaths: [],
 				});
 				setPhase("done");
 			});
@@ -99,25 +98,6 @@ export function RemoveApp({ skipConfirm = false }: RemoveAppProps) {
 		</Text>
 	));
 
-	// Config files that had no backup were left in place rather than deleted.
-	// Point them out so the user can remove them by hand if they want a fully
-	// pre-CoDev state — they may still reference the removed ~/.codev-hub.
-	const keptHint =
-		result.keptPaths.length > 0 ? (
-			<Box flexDirection="column" marginTop={1}>
-				<Text color="yellow">
-					Left {result.keptPaths.length} config file
-					{result.keptPaths.length === 1 ? "" : "s"} in place (no backup to
-					restore from). Delete manually for a clean state:
-				</Text>
-				{result.keptPaths.map((p) => (
-					<Text key={p} dimColor>
-						- {p}
-					</Text>
-				))}
-			</Box>
-		) : null;
-
 	if (result.anyFailed) {
 		const failures = result.steps.filter((s) => s.status === "failed");
 		return (
@@ -129,7 +109,6 @@ export function RemoveApp({ skipConfirm = false }: RemoveAppProps) {
 						- {s.label}: {s.detail}
 					</Text>
 				))}
-				{keptHint}
 			</Box>
 		);
 	}
@@ -142,7 +121,6 @@ export function RemoveApp({ skipConfirm = false }: RemoveAppProps) {
 				<Text color="cyan">npm uninstall -g codev-ai</Text>
 				{" to remove the CoDev package. Restart your terminal to apply."}
 			</Text>
-			{keptHint}
 		</Box>
 	);
 }

src/lib/configure.ts

@@ -601,7 +601,7 @@ export function configureClaudeCode(creds: Credentials): ConfigureResult[] {
 	return [{ kind: "claude-settings", sourcePath, backupPath, created }];
 }
 
-export type RestoreStatus = "restored" | "kept-live" | "noop";
+export type RestoreStatus = "restored" | "deleted-live" | "noop";
 
 export interface RestoreResult {
 	status: RestoreStatus;
@@ -612,9 +612,14 @@ export interface RestoreResult {
 // "Make this file look pre-CoDev." Three terminal states:
 //   - backup present → swap it over the live file (the user's pre-CoDev
 //     state is reinstated).
-//   - no backup, but a live file exists → leave the live file untouched. With
-//     no backup we can't know what (if anything) preceded CoDev, so we don't
-//     destroy the current config; the user can remove it by hand if they want.
+//   - no backup, but a live file exists → delete the live file. Both callers
+//     (`codevhub remove` and `codevhub restore`) mean "I want this machine
+//     without CoDev", and "no file" is a valid pre-CoDev state — CoDev only
+//     skips the backup step when there was nothing to back up. Note this is
+//     unconditional: a config the user hand-wrote *after* install has no
+//     backup either and is deleted too. That is the deliberate trade-off —
+//     both commands are destructive by construction, and `remove` confirms
+//     before running.
 //   - neither file exists → noop; already at pre-CoDev state.
 function restoreKind(kind: BackupKind): RestoreResult {
 	const sourcePath = sourcePathOf(kind);
@@ -635,7 +640,8 @@ function restoreKind(kind: BackupKind): RestoreResult {
 	}
 
 	if (existsSync(sourcePath)) {
-		return log({ status: "kept-live", sourcePath, backupPath });
+		rmSync(sourcePath, { force: true });
+		return log({ status: "deleted-live", sourcePath, backupPath });
 	}
 
 	return log({ status: "noop", sourcePath, backupPath });

src/lib/remove.ts

@@ -17,25 +17,17 @@ export interface StepResult {
 	label: string;
 	detail: string;
 	status: StepStatus;
-	// Live config files left in place because they had no backup to restore
-	// from. Surfaced to the user so they can delete them by hand if they want a
-	// fully pre-CoDev state; these files may still point at the removed
-	// ~/.codev-hub.
-	keptPaths?: string[];
 }
 
 export interface RemoveResult {
 	steps: StepResult[];
 	anyFailed: boolean;
-	// Aggregated across all restore steps — the union of every step's keptPaths.
-	keptPaths: string[];
 }
 
 // We iterate one Tool per BackupKind. `vscode-continue` and
 // `jetbrains-continue` share ~/.continue/config.yaml, so including both
-// would restore the file once and then redundantly re-report it (the second
-// visit finds no backup left and reports keeping the file the first visit just
-// restored). Same for the Claude Code extension variants — they share
+// would have the second iteration delete the file the first iteration just
+// restored. Same for the Claude Code extension variants — they share
 // ~/.claude/settings.json with `claude-code`. Use `claude-code` and
 // `vscode-continue` as the canonical Tools for each shared kind.
 const TOOLS: Tool[] = [
@@ -77,15 +69,11 @@ export async function runRemove(): Promise<RemoveResult> {
 	steps.push(recordStep(runUnhook()));
 	steps.push(recordStep(await runCodegraphRemoval()));
 	for (const tool of TOOLS) {
-		steps.push(recordStep(runRestoreOrKeep(tool)));
+		steps.push(recordStep(runRestoreOrDelete(tool)));
 	}
 	steps.push(recordStep(runWipeCodevDir()));
 
-	return {
-		steps,
-		anyFailed: steps.some((s) => s.status === "failed"),
-		keptPaths: steps.flatMap((s) => s.keptPaths ?? []),
-	};
+	return { steps, anyFailed: steps.some((s) => s.status === "failed") };
 }
 
 // Diagnostic-log mirror of each step's TUI row, leveled by status.
@@ -167,7 +155,7 @@ function runUnhook(): StepResult {
 	}
 }
 
-function runRestoreOrKeep(tool: Tool): StepResult {
+function runRestoreOrDelete(tool: Tool): StepResult {
 	const label = TOOL_LABEL[tool];
 	try {
 		const results = restoreTool(tool);
@@ -186,36 +174,36 @@ function runRestoreOrKeep(tool: Tool): StepResult {
 						detail: `restored from ${result.backupPath}`,
 						status: "ok",
 					};
-				case "kept-live":
+				case "deleted-live":
 					return {
 						label,
-						detail: `no backup; kept ${result.sourcePath}`,
+						detail: `no backup; deleted ${result.sourcePath}`,
 						status: "ok",
-						keptPaths: [result.sourcePath],
 					};
 				case "noop":
 					return { label, detail: "nothing to restore", status: "noop" };
 			}
 		}
 		let restored = 0;
+		let deleted = 0;
 		let noop = 0;
-		const keptPaths: string[] = [];
 		for (const r of results) {
 			if (r.status === "restored") restored++;
-			else if (r.status === "kept-live") keptPaths.push(r.sourcePath);
+			else if (r.status === "deleted-live") deleted++;
 			else noop++;
 		}
-		const kept = keptPaths.length;
-		if (restored === 0 && kept === 0) {
+		if (restored === 0 && deleted === 0) {
 			return { label, detail: "nothing to restore", status: "noop" };
 		}
 		const parts: string[] = [];
 		if (restored > 0)
 			parts.push(`restored ${restored} file${restored === 1 ? "" : "s"}`);
-		if (kept > 0)
-			parts.push(`kept ${kept} file${kept === 1 ? "" : "s"} (no backup)`);
+		if (deleted > 0)
+			parts.push(
+				`deleted ${deleted} file${deleted === 1 ? "" : "s"} (no backup)`,
+			);
 		if (noop > 0) parts.push(`${noop} already clean`);
-		return { label, detail: parts.join("; "), status: "ok", keptPaths };
+		return { label, detail: parts.join("; "), status: "ok" };
 	} catch (err) {
 		return { label, detail: errorMessage(err), status: "failed" };
 	}

src/lib/restore.ts

@@ -36,9 +36,12 @@ function reportRestoreResult(result: RestoreResult): void {
 		case "restored":
 			console.log(`Restored ${result.sourcePath} from ${result.backupPath}.`);
 			return;
-		case "kept-live":
+		// Pre-#196 this printed only "No backup at <backup>." and never mentioned
+		// the deletion. Deleting a file is exactly the thing worth saying out
+		// loud, so name the path we removed.
+		case "deleted-live":
 			console.log(
-				`No backup at ${result.backupPath}; left ${result.sourcePath} in place.`,
+				`No backup at ${result.backupPath}; deleted ${result.sourcePath}.`,
 			);
 			return;
 		case "noop":
@@ -59,9 +62,8 @@ export function runRestore(tool: Tool): number {
 
 // One Tool per BackupKind. The extension variants (`vscode-claude-code`,
 // `jetbrains-claude-code`, `jetbrains-continue`) share their config file with
-// the canonical entry, so iterating them too would redundantly re-report the
-// same file (the second visit sees no backup left and reports keeping the file
-// the first visit just restored).
+// the canonical entry, so iterating them too would have the second visit see
+// no backup and then delete the file the first visit just restored.
 const SWEEP_TOOLS: Tool[] = [
 	"claude-code",
 	"codex",
@@ -71,11 +73,11 @@ const SWEEP_TOOLS: Tool[] = [
 ];
 
 // Bare `codevhub restore` — process every tool. Each result ends in one of
-// three states (restored / kept-live / noop); only `restored` actually reverts
-// a file. Counters aggregate across all results from all sweep tools
-// (claude-code contributes three results, the others one). Exit 1 if nothing
-// was restored (every result was kept-live or noop) or any tool threw;
-// otherwise 0.
+// three states (restored / deleted-live / noop). Counters aggregate across
+// all results from all sweep tools (claude-code contributes three results,
+// the others one). Exit 1 only if every result was noop or any tool threw;
+// otherwise 0 — a delete is a real reversal, so it counts as action just like
+// a restore.
 export function runRestoreAll(): number {
 	let acted = 0;
 	let failed = 0;
@@ -86,10 +88,8 @@ export function runRestoreAll(): number {
 			const results = restoreTool(tool);
 			for (const result of results) {
 				reportRestoreResult(result);
-				// Only a genuine restore counts as action; kept-live left the file
-				// untouched, so it falls in with noop for the "nothing restored" check.
-				if (result.status === "restored") acted++;
-				else noop++;
+				if (result.status === "noop") noop++;
+				else acted++;
 			}
 		} catch (err) {
 			const msg = err instanceof Error ? err.message : String(err);

tests/RemoveApp.test.tsx

@@ -56,7 +56,6 @@ const SUCCESS_RESULT: remove.RemoveResult = {
 		{ label: "~/.codev-hub", detail: "removed /x/.codev", status: "ok" },
 	],
 	anyFailed: false,
-	keptPaths: [],
 };
 
 const FAILED_RESULT: remove.RemoveResult = {
@@ -73,7 +72,6 @@ const FAILED_RESULT: remove.RemoveResult = {
 		{ label: "~/.codev-hub", detail: "permission denied", status: "failed" },
 	],
 	anyFailed: true,
-	keptPaths: [],
 };
 
 describe("RemoveApp", () => {
@@ -147,7 +145,6 @@ describe("RemoveApp", () => {
 				},
 			],
 			anyFailed: false,
-			keptPaths: [],
 		});
 		const { frames } = render(<RemoveApp skipConfirm />);
 		await waitForFrame(frames, "Removed successfully.");
@@ -157,22 +154,6 @@ describe("RemoveApp", () => {
 		expect(out).toContain("Removed successfully.");
 	});
 
-	test("hints about backup-less config files that were left in place", async () => {
-		stubRunRemove({
-			...SUCCESS_RESULT,
-			keptPaths: ["/x/.config/codev/codev.json", "/x/.claude/settings.json"],
-		});
-		const { frames } = render(<RemoveApp skipConfirm />);
-		await waitForFrame(frames, "Removed successfully.");
-		const out = flat(history(frames));
-		expect(out).toContain("Left 2 config files in place (no backup");
-		expect(out).toContain("Delete manually for a clean state:");
-		expect(out).toContain("- /x/.config/codev/codev.json");
-		expect(out).toContain("- /x/.claude/settings.json");
-		// The success message still shows alongside the hint.
-		expect(out).toContain("Removed successfully.");
-	});
-
 	test("failure surfaces 'Some steps failed' with the failed step details", async () => {
 		stubRunRemove(FAILED_RESULT);
 		const { frames } = render(<RemoveApp skipConfirm />);

tests/lib/configure.test.ts

@@ -1106,7 +1106,7 @@ describe("restoreTool", () => {
 		);
 	});
 
-	test("keeps the live CoDev config when no backup exists", async () => {
+	test("deletes the live CoDev config when no backup exists", async () => {
 		const dir = join(tempDir, ".claude");
 		const livePath = join(dir, "settings.json");
 		const backupPath = `${livePath}.backup`;
@@ -1117,10 +1117,9 @@ describe("restoreTool", () => {
 		const results = restoreTool("claude-code");
 		const settingsResult = results.find((r) => r.sourcePath === livePath);
 
-		expect(settingsResult?.status).toBe("kept-live");
-		// No backup to restore from, so the live file is left untouched.
-		expect(existsSync(livePath)).toBe(true);
-		expect(readFileSync(livePath, "utf-8")).toBe('{"marker":"codev-live"}');
+		expect(settingsResult?.status).toBe("deleted-live");
+		// No backup to restore from, so "no file" is the pre-CoDev state.
+		expect(existsSync(livePath)).toBe(false);
 		expect(existsSync(backupPath)).toBe(false);
 	});
 
@@ -1214,11 +1213,11 @@ describe("restoreTool", () => {
 		expect(existsSync(credBackup)).toBe(false);
 	});
 
-	test("Claude bundle: keeps live files that have no backup", async () => {
+	test("Claude bundle: deletes live files that have no backup", async () => {
 		const claudeDir = join(tempDir, ".claude");
 		mkdirSync(claudeDir, { recursive: true });
 
-		// Settings has a backup → restored. Others have only live files → kept.
+		// Settings has a backup → restored. Others have only live files → deleted.
 		const settingsLive = join(claudeDir, "settings.json");
 		writeFileSync(settingsLive, '{"env":{}}');
 		writeFileSync(`${settingsLive}.backup`, '{"marker":"orig"}');
@@ -1234,17 +1233,13 @@ describe("restoreTool", () => {
 
 		const byKind = new Map(results.map((r) => [r.sourcePath, r.status]));
 		expect(byKind.get(settingsLive)).toBe("restored");
-		expect(byKind.get(jsonLive)).toBe("kept-live");
-		expect(byKind.get(credLive)).toBe("kept-live");
+		expect(byKind.get(jsonLive)).toBe("deleted-live");
+		expect(byKind.get(credLive)).toBe("deleted-live");
 
-		// The two backup-less files are left in place untouched; only the one with
-		// a backup was restored (and its backup consumed).
-		expect(JSON.parse(readFileSync(jsonLive, "utf-8"))).toEqual({
-			hasCompletedOnboarding: true,
-		});
-		expect(JSON.parse(readFileSync(credLive, "utf-8"))).toEqual({
-			session: "post-install",
-		});
+		// All live files are now gone (no backup left to restore from for the two
+		// deleted-live cases, and the restored one's backup was consumed).
+		expect(existsSync(jsonLive)).toBe(false);
+		expect(existsSync(credLive)).toBe(false);
 		expect(JSON.parse(readFileSync(settingsLive, "utf-8"))).toEqual({
 			marker: "orig",
 		});