From ac9d0ceab897fbf61cbb9a5bbd90894949b8bf2a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 29 Jul 2026 07:47:37 -0400 Subject: [PATCH] refactor: simplify TOOLS.md doctor migration recovery (#115857) * refactor: prototype simpler tools migration recovery Not landable until shipped interrupted-claim compatibility is decided. * fix: diagnose waived beta migration artifacts --- .../doctor-tools-md-migration.test.ts | 145 ++----- src/commands/doctor-tools-md-migration.ts | 399 +++++------------- src/infra/directory-durability.ts | 14 - 3 files changed, 147 insertions(+), 411 deletions(-) diff --git a/src/commands/doctor-tools-md-migration.test.ts b/src/commands/doctor-tools-md-migration.test.ts index 7f6a1af34056..e38bce2e0ec4 100644 --- a/src/commands/doctor-tools-md-migration.test.ts +++ b/src/commands/doctor-tools-md-migration.test.ts @@ -304,116 +304,38 @@ describe("TOOLS.md migration", () => { }, ); - it.runIf(process.platform !== "win32")( - "refuses symlinked AGENTS.md files and interrupted claims", - async () => { - const linkedFixture = await createFixture(); - const linkedTarget = path.join(linkedFixture.root, "outside-agents.md"); - await fs.writeFile(linkedTarget, "Private external instructions.\n"); - await fs.writeFile(linkedFixture.toolsPath, "Local tool notes.\n"); - await fs.symlink(linkedTarget, linkedFixture.agentsPath); + it.runIf(process.platform !== "win32")("refuses symlinked AGENTS.md files", async () => { + const linkedFixture = await createFixture(); + const linkedTarget = path.join(linkedFixture.root, "outside-agents.md"); + await fs.writeFile(linkedTarget, "Private external instructions.\n"); + await fs.writeFile(linkedFixture.toolsPath, "Local tool notes.\n"); + await fs.symlink(linkedTarget, linkedFixture.agentsPath); - const linkedResult = await maybeMigrateToolsMd({ - cfg: linkedFixture.cfg, - shouldRepair: true, - env: linkedFixture.env, - }); - - expect(linkedResult.changes).toEqual([]); - expect(linkedResult.warnings).toEqual([ - expect.stringContaining("AGENTS.md must be an unlinked regular file"), - ]); - await expect(fs.readFile(linkedFixture.toolsPath, "utf8")).resolves.toBe( - "Local tool notes.\n", - ); - await expect(fs.readFile(linkedTarget, "utf8")).resolves.toBe( - "Private external instructions.\n", - ); - - const claimFixture = await createFixture(); - const claimTarget = path.join(claimFixture.root, "outside-claim.md"); - const claimPath = `${claimFixture.agentsPath}.doctor-backup-999999-${Date.now() - 60_000}`; - await fs.writeFile(claimTarget, "Untrusted claim content.\n"); - await fs.writeFile(claimFixture.toolsPath, "Local tool notes.\n"); - await fs.symlink(claimTarget, claimPath); - - const claimResult = await maybeMigrateToolsMd({ - cfg: claimFixture.cfg, - shouldRepair: true, - env: claimFixture.env, - }); - - expect(claimResult.changes).toEqual([]); - expect(claimResult.warnings).toEqual([ - expect.stringContaining("AGENTS.md migration claim must be an unlinked regular file"), - ]); - await expect(fs.readFile(claimFixture.toolsPath, "utf8")).resolves.toBe( - "Local tool notes.\n", - ); - await expect(fs.readFile(claimTarget, "utf8")).resolves.toBe("Untrusted claim content.\n"); - await expectMissing(claimFixture.agentsPath); - }, - ); - - it("keeps live claims created from old source files fresh", async () => { - const ownerPid = process.ppid; - const oldTimestamp = new Date("2000-01-01T00:00:00.000Z"); - - const toolsFixture = await createFixture(); - await fs.writeFile(toolsFixture.toolsPath, "old tool notes\n"); - await fs.utimes(toolsFixture.toolsPath, oldTimestamp, oldTimestamp); - const toolsClaimPath = `${toolsFixture.toolsPath}.doctor-importing-${ownerPid}-${Date.now()}-claim`; - await fs.rename(toolsFixture.toolsPath, toolsClaimPath); - - const toolsResult = await maybeMigrateToolsMd({ - cfg: toolsFixture.cfg, + const linkedResult = await maybeMigrateToolsMd({ + cfg: linkedFixture.cfg, shouldRepair: true, - env: toolsFixture.env, + env: linkedFixture.env, }); - expect(toolsResult.warnings).toEqual([ - expect.stringContaining(`TOOLS.md migration claim is held by running process ${ownerPid}`), + expect(linkedResult.changes).toEqual([]); + expect(linkedResult.warnings).toEqual([ + expect.stringContaining("AGENTS.md must be an unlinked regular file"), ]); - await expect(fs.stat(toolsClaimPath)).resolves.toMatchObject({ - mtimeMs: oldTimestamp.getTime(), - }); - - const agentsFixture = await createFixture(); - await fs.writeFile(agentsFixture.toolsPath, "old tool notes\n"); - await fs.writeFile(agentsFixture.agentsPath, "# Agent\n"); - await fs.utimes(agentsFixture.agentsPath, oldTimestamp, oldTimestamp); - const agentsClaimPath = `${agentsFixture.agentsPath}.doctor-backup-${ownerPid}-${Date.now()}`; - await fs.rename(agentsFixture.agentsPath, agentsClaimPath); - - const agentsResult = await maybeMigrateToolsMd({ - cfg: agentsFixture.cfg, - shouldRepair: true, - env: agentsFixture.env, - }); - - expect(agentsResult.warnings).toEqual([ - expect.stringContaining(`AGENTS.md migration claim is held by running process ${ownerPid}`), - ]); - await expect(fs.readFile(agentsFixture.toolsPath, "utf8")).resolves.toBe("old tool notes\n"); - await expect(fs.stat(agentsClaimPath)).resolves.toMatchObject({ - mtimeMs: oldTimestamp.getTime(), - }); + await expect(fs.readFile(linkedFixture.toolsPath, "utf8")).resolves.toBe("Local tool notes.\n"); + await expect(fs.readFile(linkedTarget, "utf8")).resolves.toBe( + "Private external instructions.\n", + ); }); - it("recovers an AGENTS.md publish interrupted after the hard link", async () => { + it("reruns after an interrupted AGENTS.md temp write and converges", async () => { const fixture = await createFixture(); const agents = "# Agent\n\n## Tools\n\nExisting notes.\n"; const tools = "### Cameras\n\n- kitchen → wide angle\n"; const merged = `${agents}\n### Local notes (migrated from TOOLS.md)\n\n${tools}`; - const interruptedAt = Date.now() - 60_000; - const toolsClaimPath = `${fixture.toolsPath}.doctor-importing-999999-${interruptedAt}-claim`; - const agentsClaimPath = `${fixture.agentsPath}.doctor-backup-999999-${interruptedAt}`; - const tempPath = `${fixture.agentsPath}.doctor-writing-999999-${interruptedAt}`; - await fs.writeFile(toolsClaimPath, tools); - await fs.writeFile(agentsClaimPath, agents); - await fs.writeFile(tempPath, merged); - await fs.link(tempPath, fixture.agentsPath); - await expect(fs.stat(fixture.agentsPath)).resolves.toMatchObject({ nlink: 2 }); + const tempPath = `${fixture.agentsPath}.doctor-writing-999999-interrupted`; + await fs.writeFile(fixture.toolsPath, tools); + await fs.writeFile(fixture.agentsPath, agents); + await fs.writeFile(tempPath, "partial merged content"); const result = await maybeMigrateToolsMd({ cfg: fixture.cfg, @@ -424,8 +346,31 @@ describe("TOOLS.md migration", () => { expect(result.warnings).toEqual([]); expect(result.changes).toHaveLength(1); await expect(fs.readFile(fixture.agentsPath, "utf8")).resolves.toBe(merged); - await expect(fs.stat(fixture.agentsPath)).resolves.toMatchObject({ nlink: 1 }); await expect(fs.readdir(fixture.workspace)).resolves.toEqual(["AGENTS.md"]); + await expect(readOnlyArchive(fixture.stateDir)).resolves.toEqual(Buffer.from(tools)); + }); + + it("finishes cleanup without duplicating already-merged content", async () => { + const fixture = await createFixture(); + const agents = "# Agent\n\n## Tools\n\nExisting notes.\n"; + const tools = "### Cameras\n\n- kitchen → wide angle\n"; + const merged = `${agents}\n### Local notes (migrated from TOOLS.md)\n\n${tools}`; + await fs.writeFile(fixture.agentsPath, merged); + await fs.writeFile(fixture.toolsPath, tools); + + const result = await maybeMigrateToolsMd({ + cfg: fixture.cfg, + shouldRepair: true, + env: fixture.env, + }); + + expect(result.warnings).toEqual([]); + await expect(fs.readFile(fixture.agentsPath, "utf8")).resolves.toBe(merged); + expect( + (await fs.readFile(fixture.agentsPath, "utf8")).match(/migrated from TOOLS\.md/gu), + ).toHaveLength(1); + await expectMissing(fixture.toolsPath); + await expect(readOnlyArchive(fixture.stateDir)).resolves.toEqual(Buffer.from(tools)); }); it("appends a Tools section when AGENTS.md has no Tools heading", async () => { diff --git a/src/commands/doctor-tools-md-migration.ts b/src/commands/doctor-tools-md-migration.ts index 364faabf34c7..09d384144328 100644 --- a/src/commands/doctor-tools-md-migration.ts +++ b/src/commands/doctor-tools-md-migration.ts @@ -9,11 +9,7 @@ import { formatCliCommand } from "../cli/command-format.js"; import { resolveStateDir } from "../config/paths.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { HealthFinding } from "../flows/health-checks.js"; -import { - publishFileNoClobber, - publishFileNoClobberSync, - syncDirectoryIfSupported, -} from "../infra/directory-durability.js"; +import { publishFileNoClobber, syncDirectoryIfSupported } from "../infra/directory-durability.js"; import { formatErrorMessage as errorMessage } from "../infra/errors.js"; import { shortenHomePath } from "../utils.js"; import { @@ -25,8 +21,6 @@ import { rewriteLegacyAgentsToolsGuidance as rewriteLegacyToolsGuidance } from " const TOOLS_MD_MIGRATION_CHECK_ID = "core/doctor/tools-md-migration"; const MIGRATED_SUBSECTION_HEADING = "### Local notes (migrated from TOOLS.md)"; -const TOOLS_CLAIM_INFIX = ".doctor-importing-"; -const ACTIVE_CLAIM_MAX_AGE_MS = 10 * 60 * 1000; const NO_CLOBBER_PUBLICATION = { strategy: "link-or-copy", durability: "degrade", @@ -41,11 +35,7 @@ type ToolsMdSource = { path: string; content: string; sha256: string; -}; - -type MigrationClaimIdentity = { - ownerPid: number; - createdAtMs: number; + stat: syncFs.Stats; }; type MigrationFileSnapshot = { @@ -97,117 +87,33 @@ async function readMigrationFileSnapshot(params: { } } -function parseMigrationClaimIdentity( - claimName: string, - prefix: string, -): MigrationClaimIdentity | undefined { - const [ownerPidText, createdAtMsText] = claimName.slice(prefix.length).split("-"); - const ownerPid = Number(ownerPidText); - const createdAtMs = Number(createdAtMsText); - if (!Number.isSafeInteger(ownerPid) || !Number.isSafeInteger(createdAtMs) || createdAtMs <= 0) { +async function readToolsMd(workspaceDir: string): Promise { + const entries = await fs.readdir(workspaceDir).catch(() => [] as string[]); + const betaArtifacts = entries.filter( + (entry) => + entry.startsWith(`${DEFAULT_TOOLS_FILENAME}.doctor-importing-`) || + entry.startsWith(`${DEFAULT_AGENTS_FILENAME}.doctor-backup-`), + ); + if (betaArtifacts.length > 0) { + throw new Error( + `Interrupted v2026.7.2-beta.5 migration artifact(s) left untouched: ${betaArtifacts.join(", ")}. Restore the desired file manually before rerunning doctor.`, + ); + } + const toolsPath = path.join(workspaceDir, DEFAULT_TOOLS_FILENAME); + const snapshot = await readMigrationFileSnapshot({ + filePath: toolsPath, + label: "TOOLS.md", + allowMissing: true, + }); + if (!snapshot.stat) { return undefined; } - return { ownerPid, createdAtMs }; -} - -async function readToolsMd( - workspaceDir: string, - options?: { recoverClaims?: boolean }, -): Promise { - const toolsPath = path.join(workspaceDir, DEFAULT_TOOLS_FILENAME); - let stat; - try { - stat = await fs.lstat(toolsPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") { - const entries = await fs.readdir(workspaceDir).catch(() => [] as string[]); - const claims = entries.filter((entry) => - entry.startsWith(`${DEFAULT_TOOLS_FILENAME}${TOOLS_CLAIM_INFIX}`), - ); - if (claims.length === 0) { - return undefined; - } - if (claims.length > 1) { - throw new Error("multiple interrupted TOOLS.md migration claims require manual recovery", { - cause: error, - }); - } - const claimPath = path.join(workspaceDir, claims[0]!); - const claimIdentity = parseMigrationClaimIdentity( - claims[0]!, - `${DEFAULT_TOOLS_FILENAME}${TOOLS_CLAIM_INFIX}`, - ); - if ( - claimIdentity && - claimIdentity.ownerPid !== process.pid && - Date.now() - claimIdentity.createdAtMs < ACTIVE_CLAIM_MAX_AGE_MS && - isProcessAlive(claimIdentity.ownerPid) - ) { - throw new Error( - `TOOLS.md migration claim is held by running process ${claimIdentity.ownerPid}`, - { cause: error }, - ); - } - if (!options?.recoverClaims) { - throw new Error("an interrupted TOOLS.md migration claim requires doctor --fix", { - cause: error, - }); - } - await restoreClaimNoClobber(claimPath, toolsPath); - stat = await fs.lstat(toolsPath); - } else { - throw error; - } - } - if (!stat.isFile()) { - throw new Error("TOOLS.md must be a regular file"); - } - if (stat.nlink > 1) { - if (!options?.recoverClaims) { - throw new Error("an interrupted TOOLS.md migration restoration requires doctor --fix"); - } - const entries = await fs.readdir(workspaceDir); - const claims = entries.filter((entry) => - entry.startsWith(`${DEFAULT_TOOLS_FILENAME}${TOOLS_CLAIM_INFIX}`), - ); - if (claims.length === 1) { - const claimPath = path.join(workspaceDir, claims[0]!); - const claimStat = await fs.lstat(claimPath); - if (claimStat.dev === stat.dev && claimStat.ino === stat.ino && stat.nlink === 2) { - await fs.rm(claimPath); - stat = await fs.lstat(toolsPath); - } - } - if (stat.nlink > 1) { - throw new Error("TOOLS.md has multiple hard links; refusing automatic removal"); - } - } - const noFollow = syncFs.constants.O_NOFOLLOW ?? 0; - const handle = await fs.open(toolsPath, syncFs.constants.O_RDONLY | noFollow); - let content: string; - try { - const openedStat = await handle.stat(); - if (!openedStat.isFile() || openedStat.nlink !== stat.nlink) { - throw new Error("TOOLS.md changed while opening it for migration"); - } - content = await handle.readFile("utf8"); - const currentStat = await fs.lstat(toolsPath); - if (currentStat.dev !== openedStat.dev || currentStat.ino !== openedStat.ino) { - throw new Error("TOOLS.md changed while opening it for migration"); - } - } finally { - await handle.close(); - } - return { path: toolsPath, content, sha256: sha256(content) }; -} - -function isProcessAlive(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch (error) { - return (error as NodeJS.ErrnoException).code === "EPERM"; - } + return { + path: toolsPath, + content: snapshot.content, + sha256: sha256(snapshot.content), + stat: snapshot.stat, + }; } function migratedBlock(content: string): string { @@ -328,156 +234,80 @@ async function writeAgentsAtomically(params: { const stat = snapshot.stat; const mode = stat?.mode ?? 0o600; const tempPath = `${params.agentsPath}.doctor-writing-${process.pid}-${Date.now()}`; - const handle = await fs.open(tempPath, "wx", mode); try { - await handle.writeFile(params.content, "utf8"); - await handle.sync(); - } finally { - await handle.close(); - } - const backupPath = `${params.agentsPath}.doctor-backup-${process.pid}-${Date.now()}`; - let claimed = false; - try { - if (stat) { - const currentStat = await fs.lstat(params.agentsPath); - if ( - currentStat.dev !== stat.dev || - currentStat.ino !== stat.ino || - (await readMigrationFileSnapshot({ filePath: params.agentsPath, label: "AGENTS.md" })) - .content !== params.expected - ) { - throw new Error("AGENTS.md changed during TOOLS.md migration"); - } - syncFs.renameSync(params.agentsPath, backupPath); - claimed = true; - publishFileNoClobberSync(tempPath, params.agentsPath); - syncFs.unlinkSync(tempPath); - if ( - (await readMigrationFileSnapshot({ filePath: backupPath, label: "AGENTS.md backup" })) - .content !== params.expected - ) { - syncFs.renameSync(backupPath, params.agentsPath); - claimed = false; - throw new Error("AGENTS.md changed during TOOLS.md migration"); - } - } else { - publishFileNoClobberSync(tempPath, params.agentsPath); - syncFs.unlinkSync(tempPath); + const handle = await fs.open(tempPath, "wx", mode); + try { + await handle.writeFile(params.content, "utf8"); + await handle.sync(); + } finally { + await handle.close(); } + // Doctor is a single-operator flow. This final snapshot catches edits before + // commit without retaining the retired cross-process claim protocol. + const current = await readMigrationFileSnapshot({ + filePath: params.agentsPath, + label: "AGENTS.md", + allowMissing: true, + }); + if ( + current.content !== params.expected || + current.stat?.dev !== stat?.dev || + current.stat?.ino !== stat?.ino + ) { + throw new Error("AGENTS.md changed during TOOLS.md migration"); + } + await fs.rename(tempPath, params.agentsPath); await syncDirectoryIfSupported(path.dirname(params.agentsPath)); - if (stat) { - await fs.rm(backupPath); - claimed = false; - await syncDirectoryIfSupported(path.dirname(params.agentsPath)); - } } catch (error) { await fs.rm(tempPath, { force: true }); - if (claimed) { - try { - await fs.lstat(params.agentsPath); - } catch (pathError) { - if ((pathError as NodeJS.ErrnoException).code === "ENOENT") { - await restoreClaimNoClobber(backupPath, params.agentsPath); - } - } - } throw error; } } -async function recoverInterruptedAgentsClaim(params: { - agentsPath: string; - toolsContent: string; - shouldMerge: boolean; -}): Promise { - const { agentsPath } = params; - await recoverInterruptedAgentsPublish(agentsPath); - const entries = await fs.readdir(path.dirname(agentsPath)).catch(() => [] as string[]); - const prefix = `${path.basename(agentsPath)}.doctor-backup-`; - const claims = entries.filter((entry) => entry.startsWith(prefix)); - if (claims.length === 0) { - return; - } - if (claims.length > 1) { - throw new Error("multiple interrupted AGENTS.md migration claims require manual recovery"); - } - const claimPath = path.join(path.dirname(agentsPath), claims[0]!); - const claimSnapshot = await readMigrationFileSnapshot({ - filePath: claimPath, - label: "AGENTS.md migration claim", - }); - const claimIdentity = parseMigrationClaimIdentity(claims[0]!, prefix); - if ( - claimIdentity && - claimIdentity.ownerPid !== process.pid && - Date.now() - claimIdentity.createdAtMs < ACTIVE_CLAIM_MAX_AGE_MS && - isProcessAlive(claimIdentity.ownerPid) - ) { - throw new Error( - `AGENTS.md migration claim is held by running process ${claimIdentity.ownerPid}`, - ); - } - try { - const agentsSnapshot = await readMigrationFileSnapshot({ - filePath: agentsPath, - label: "AGENTS.md", - }); - const claimedContent = claimSnapshot.content; - const expected = params.shouldMerge - ? mergeToolsMdIntoAgentsMd(claimedContent, params.toolsContent) - : rewriteLegacyAgentsToolsGuidance(claimedContent); - if (agentsSnapshot.content === expected || agentsSnapshot.content === claimedContent) { - await fs.rm(claimPath); - return; - } - throw new Error(`interrupted AGENTS.md claim is preserved at ${claimPath}`); - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") { - throw error; - } - } - await publishFileNoClobber(claimPath, agentsPath, NO_CLOBBER_PUBLICATION); - await fs.rm(claimPath); -} - -async function recoverInterruptedAgentsPublish(agentsPath: string): Promise { +async function recoverInterruptedAgentsWrite(agentsPath: string): Promise { const dir = path.dirname(agentsPath); const prefix = `${path.basename(agentsPath)}.doctor-writing-`; - let agentsStat: syncFs.Stats; - try { - agentsStat = syncFs.lstatSync(agentsPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") { - return; - } - throw error; - } - const linkedTemps = syncFs.readdirSync(dir).filter((entry) => { + const entries = await fs.readdir(dir).catch(() => [] as string[]); + let removed = false; + for (const entry of entries) { if (!entry.startsWith(prefix)) { - return false; + continue; } - const tempStat = syncFs.lstatSync(path.join(dir, entry)); - return tempStat.isFile() && tempStat.dev === agentsStat.dev && tempStat.ino === agentsStat.ino; - }); - if (!agentsStat.isFile() || agentsStat.nlink !== 2 || linkedTemps.length !== 1) { - return; + const tempPath = path.join(dir, entry); + const stat = await fs.lstat(tempPath); + if (!stat.isFile() || stat.nlink !== 1) { + throw new Error(`interrupted AGENTS.md write must be an unlinked regular file: ${tempPath}`); + } + await fs.rm(tempPath); + removed = true; + } + if (removed) { + await syncDirectoryIfSupported(dir); } - // The active TOOLS.md claim excludes concurrent doctor writers here; this - // same-inode link can only be the completed half of an interrupted publish. - syncFs.unlinkSync(path.join(dir, linkedTemps[0]!)); - await syncDirectoryIfSupported(dir); } -async function restoreClaimNoClobber(claimPath: string, destinationPath: string): Promise { - try { - await publishFileNoClobber(claimPath, destinationPath, NO_CLOBBER_PUBLICATION); - await fs.rm(claimPath); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "EEXIST") { - throw new Error(`migration claim is preserved at ${claimPath}`, { cause: error }); - } - throw error; +async function removeToolsSource(source: ToolsMdSource, workspaceDir: string): Promise { + const current = await readMigrationFileSnapshot({ + filePath: source.path, + label: "TOOLS.md", + }); + if (sha256(current.content) !== source.sha256) { + throw new Error("TOOLS.md changed during migration"); } + // The original bytes are durable in both the archive and merged AGENTS.md; + // the single-operator migration deliberately has no concurrent-writer claim. + const currentStat = syncFs.lstatSync(source.path); + if ( + !current.stat || + currentStat.dev !== current.stat.dev || + currentStat.ino !== current.stat.ino || + currentStat.dev !== source.stat.dev || + currentStat.ino !== source.stat.ino + ) { + throw new Error("TOOLS.md changed during migration"); + } + syncFs.unlinkSync(source.path); + await syncDirectoryIfSupported(workspaceDir); } function archivePathForSource( @@ -611,9 +441,7 @@ export async function maybeMigrateToolsMd(params: { const warnings: string[] = []; for (const target of resolveToolsMdMigrationWorkspaceTargets(params.cfg)) { try { - const source = await readToolsMd(target.workspaceDir, { - recoverClaims: params.shouldRepair, - }); + const source = await readToolsMd(target.workspaceDir); if (!source) { continue; } @@ -627,53 +455,30 @@ export async function maybeMigrateToolsMd(params: { const shouldMerge = shouldMergeToolsMd(source.content); await archiveSource({ agentId: target.primaryAgentId, source, env }); - const claimPath = `${source.path}${TOOLS_CLAIM_INFIX}${process.pid}-${Date.now()}-${source.sha256.slice(0, 12)}`; - await fs.rename(source.path, claimPath); - try { - if (sha256(await fs.readFile(claimPath, "utf8")) !== source.sha256) { - throw new Error("TOOLS.md changed before the migration claim was acquired"); - } - const agentsPath = path.join(target.workspaceDir, DEFAULT_AGENTS_FILENAME); - await recoverInterruptedAgentsClaim({ - agentsPath, - toolsContent: source.content, - shouldMerge, - }); - const agentsContent = ( - await readMigrationFileSnapshot({ - filePath: agentsPath, - label: "AGENTS.md", - allowMissing: true, - }) - ).content; - const merged = shouldMerge - ? mergeToolsMdIntoAgentsMd(agentsContent, source.content) - : rewriteLegacyAgentsToolsGuidance(agentsContent); - if (merged !== agentsContent) { - await writeAgentsAtomically({ agentsPath, expected: agentsContent, content: merged }); - } - if (sha256(await fs.readFile(claimPath, "utf8")) !== source.sha256) { - throw new Error("TOOLS.md changed while the migration claim was held"); - } + const agentsPath = path.join(target.workspaceDir, DEFAULT_AGENTS_FILENAME); + await recoverInterruptedAgentsWrite(agentsPath); + const agentsContent = ( + await readMigrationFileSnapshot({ + filePath: agentsPath, + label: "AGENTS.md", + allowMissing: true, + }) + ).content; + const merged = shouldMerge + ? mergeToolsMdIntoAgentsMd(agentsContent, source.content) + : rewriteLegacyAgentsToolsGuidance(agentsContent); + if (merged !== agentsContent) { + await writeAgentsAtomically({ agentsPath, expected: agentsContent, content: merged }); if ( - merged !== agentsContent && (await readMigrationFileSnapshot({ filePath: agentsPath, label: "AGENTS.md" })) .content !== merged ) { throw new Error("AGENTS.md changed after TOOLS.md migration was written"); } - await fs.rm(claimPath); - await syncDirectoryIfSupported(target.workspaceDir); - } catch (error) { - try { - await restoreClaimNoClobber(claimPath, source.path); - } catch (restoreError) { - throw new Error(`TOOLS.md migration claim is preserved at ${claimPath}`, { - cause: restoreError, - }); - } - throw error; } + // Fence an earlier AGENTS rename before the durable source unlink, including reruns. + await syncDirectoryIfSupported(target.workspaceDir); + await removeToolsSource(source, target.workspaceDir); changes.push( shouldMerge ? `Merged ${shortenHomePath(source.path)} into AGENTS.md and archived the original.` diff --git a/src/infra/directory-durability.ts b/src/infra/directory-durability.ts index c2e1d9105636..c9efc4d1da42 100644 --- a/src/infra/directory-durability.ts +++ b/src/infra/directory-durability.ts @@ -1,4 +1,3 @@ -import { constants, copyFileSync, linkSync } from "node:fs"; import fs from "node:fs/promises"; import { publishFileExclusive, @@ -26,7 +25,6 @@ export { } from "@openclaw/fs-safe/durability"; type DirectoryDurabilityOutcome = DirectorySyncOutcome | { status: "not-needed" }; -const HARD_LINK_UNSUPPORTED_CODES = new Set(["EPERM", "ENOTSUP", "EOPNOTSUPP", "EXDEV"]); function isUnsupportedDirectorySyncError(error: unknown): boolean { const code = (error as NodeJS.ErrnoException).code; @@ -141,18 +139,6 @@ export async function publishFileNoClobber( return { ...published, durability: degraded ? "degraded" : "durable" }; } -/** Synchronous no-clobber publication for critical sections that cannot yield. */ -export function publishFileNoClobberSync(sourcePath: string, targetPath: string): void { - try { - linkSync(sourcePath, targetPath); - } catch (error) { - if (!HARD_LINK_UNSUPPORTED_CODES.has((error as NodeJS.ErrnoException).code ?? "")) { - throw error; - } - copyFileSync(sourcePath, targetPath, constants.COPYFILE_EXCL); - } -} - /** Compatibility adapter for former best-effort call sites. */ export async function syncDirectoryIfSupported( directoryPath: string,