From 486231f6be4c1bf0ab3c3220a5f256dce28cefdb Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 19 Sep 2026 08:40:30 -0500 Subject: [PATCH] fix(runner): clean up retired read-only harness backups Co-Authored-By: Paperclip --- .../native-harness-backup-cleanup.test.ts | 34 +++++++++++++++++++ .../native-harness-backup-cleanup.ts | 18 ++++++++++ .../native-runtime/native-session-executor.ts | 9 ++--- 3 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 server/src/services/native-runtime/native-harness-backup-cleanup.test.ts create mode 100644 server/src/services/native-runtime/native-harness-backup-cleanup.ts diff --git a/server/src/services/native-runtime/native-harness-backup-cleanup.test.ts b/server/src/services/native-runtime/native-harness-backup-cleanup.test.ts new file mode 100644 index 00000000000..1c969758ac7 --- /dev/null +++ b/server/src/services/native-runtime/native-harness-backup-cleanup.test.ts @@ -0,0 +1,34 @@ +import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { expect, it } from "vitest"; +import { removeNativeHarnessBackup } from "./native-harness-backup-cleanup.js"; + +it("removes retired backups containing immutable skills without modifying live files or symlink targets", () => { + const base = mkdtempSync(join(tmpdir(), "paperclip-backup-cleanup-")); + const retired = join(base, "previous"); + const nested = join(retired, "claude", "skills", "first-task"); + const live = join(base, "current"); + mkdirSync(nested, { recursive: true }); + mkdirSync(live); + writeFileSync(join(nested, "SKILL.md"), "retired instructions", { mode: 0o444 }); + writeFileSync(join(live, "SKILL.md"), "live instructions", { mode: 0o444 }); + symlinkSync(live, join(retired, "outside")); + chmodSync(nested, 0o555); + chmodSync(join(retired, "claude", "skills"), 0o555); + chmodSync(retired, 0o555); + chmodSync(live, 0o555); + try { + removeNativeHarnessBackup(retired); + expect(existsSync(retired)).toBe(false); + expect(readFileSync(join(live, "SKILL.md"), "utf8")).toBe("live instructions"); + expect(lstatSync(live).mode & 0o777).toBe(0o555); + expect(lstatSync(join(live, "SKILL.md")).mode & 0o777).toBe(0o444); + expect(() => removeNativeHarnessBackup(retired)).not.toThrow(); + } finally { + for (const directory of [retired, join(retired, "claude", "skills"), nested, live]) { + if (existsSync(directory)) chmodSync(directory, 0o700); + } + rmSync(base, { recursive: true, force: true }); + } +}); diff --git a/server/src/services/native-runtime/native-harness-backup-cleanup.ts b/server/src/services/native-runtime/native-harness-backup-cleanup.ts new file mode 100644 index 00000000000..d52e50ba5c4 --- /dev/null +++ b/server/src/services/native-runtime/native-harness-backup-cleanup.ts @@ -0,0 +1,18 @@ +import { chmodSync, lstatSync, readdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; + +/** Remove a retired controller-owned backup, never a live harness directory. */ +export function removeNativeHarnessBackup(root: string): void { + // Archives preserve immutable skill directory modes. Deleting their children + // requires write permission on those directories, even with rm's force flag. + // Only change directories in the backup being discarded. lstat deliberately + // avoids following provider-created symlinks into live or unrelated trees. + const prepareDirectory = (path: string): void => { + const metadata = lstatSync(path, { throwIfNoEntry: false }); + if (!metadata?.isDirectory()) return; + chmodSync(path, (metadata.mode & 0o777) | 0o700); + for (const name of readdirSync(path)) prepareDirectory(join(path, name)); + }; + prepareDirectory(root); + rmSync(root, { recursive: true, force: true }); +} diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 93489cab1b4..813d33839f2 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -153,6 +153,7 @@ import { type NativeRunTrace, } from "./native-run-trace.js"; import { createNativeHarnessBackupStamp } from "./native-harness-backup-stamp.js"; +import { removeNativeHarnessBackup } from "./native-harness-backup-cleanup.js"; import { registerLiveRunnerGoalController } from "../runner-goal-control-broker.js"; import { applyRunnerGoalPrpEvent } from "../runner-goals.js"; import { readProcessStartedAt } from "../hot-restart.js"; @@ -11359,7 +11360,7 @@ async function createRunnerdBackendWithinSessionClaim( const currentRoot = resolve(backupRoot, "current"); const previousRoot = resolve(backupRoot, "previous"); - rmSync(previousRoot, { recursive: true, force: true }); + removeNativeHarnessBackup(previousRoot); let movedCurrent = false; if (existsSync(currentRoot)) { renameSync(currentRoot, previousRoot); @@ -11379,7 +11380,7 @@ async function createRunnerdBackendWithinSessionClaim( } } catch (error) { if (existsSync(currentRoot)) { - rmSync(currentRoot, { recursive: true, force: true }); + removeNativeHarnessBackup(currentRoot); } if ( movedCurrent && @@ -11390,9 +11391,9 @@ async function createRunnerdBackendWithinSessionClaim( } throw error; } - rmSync(previousRoot, { recursive: true, force: true }); + removeNativeHarnessBackup(previousRoot); } finally { - rmSync(pendingRoot, { recursive: true, force: true }); + removeNativeHarnessBackup(pendingRoot); } }, {