Skip to content

Commit 704fc35

Browse files
giodl73-repogiodl73-repo
authored
Doctor: expose session lock findings (#84366)
Merged via squash. Prepared head SHA: 93192bb Co-authored-by: giodl73-repo <[email protected]> Co-authored-by: giodl73-repo <[email protected]> Reviewed-by: @giodl73-repo
1 parent f1e38f2 commit 704fc35

12 files changed

Lines changed: 341 additions & 15 deletions

‎src/agents/main-session-restart-recovery.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ function cleanedLockForPath(lockPath: string): SessionLockInspection {
7373
ageMs: 1_000,
7474
stale: true,
7575
staleReasons: ["dead-pid"],
76+
removable: true,
7677
removed: true,
7778
};
7879
}

‎src/agents/session-write-lock.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export type SessionLockInspection = {
3636
ageMs: number | null;
3737
stale: boolean;
3838
staleReasons: string[];
39+
removable: boolean;
3940
removed: boolean;
4041
};
4142

@@ -858,13 +859,15 @@ export async function cleanStaleLockFiles(params: {
858859
reclaimLockWithoutStarttime: false,
859860
readOwnerProcessArgs: ownerProcessArgsReader,
860861
});
862+
const removable = await shouldRemoveLockDuringCleanup(lockPath, inspected, staleMs, nowMs);
861863
const lockInfo: SessionLockInspection = {
862864
lockPath,
863865
...inspected,
866+
removable,
864867
removed: false,
865868
};
866869

867-
if (removeStale && (await shouldRemoveLockDuringCleanup(lockPath, lockInfo, staleMs, nowMs))) {
870+
if (removeStale && removable) {
868871
await fs.rm(lockPath, { force: true });
869872
lockInfo.removed = true;
870873
cleaned.push(lockInfo);

‎src/commands/doctor-lint.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ describe("runDoctorLintCli", () => {
155155
try {
156156
const exitCode = await runDoctorLintCli(runtime, {
157157
json: true,
158-
onlyIds: ["core/doctor/session-locks"],
158+
onlyIds: ["core/doctor/not-a-check"],
159159
});
160160

161161
expect(exitCode).toBe(1);
@@ -167,7 +167,7 @@ describe("runDoctorLintCli", () => {
167167
{
168168
checkId: "core/doctor/lint-selection",
169169
severity: "error",
170-
path: "core/doctor/session-locks",
170+
path: "core/doctor/not-a-check",
171171
},
172172
],
173173
});

‎src/commands/doctor-session-locks.test.ts‎

Lines changed: 154 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,12 @@ vi.mock("../../packages/terminal-core/src/note.js", () => ({
1313
note,
1414
}));
1515

16-
import { noteSessionLockHealth } from "./doctor-session-locks.js";
16+
import {
17+
detectStaleSessionLocks,
18+
noteSessionLockHealth,
19+
sessionLockToHealthFinding,
20+
sessionLockToRepairEffect,
21+
} from "./doctor-session-locks.js";
1722

1823
async function expectPathMissing(targetPath: string): Promise<void> {
1924
try {
@@ -105,6 +110,154 @@ describe("noteSessionLockHealth", () => {
105110
await expect(fs.access(freshLock)).resolves.toBeUndefined();
106111
});
107112

113+
it("detects stale locks without removing them for structured lint", async () => {
114+
const sessionsDir = state.sessionsDir();
115+
await fs.mkdir(sessionsDir, { recursive: true });
116+
117+
const staleLock = path.join(sessionsDir, "stale.jsonl.lock");
118+
const freshLock = path.join(sessionsDir, "fresh.jsonl.lock");
119+
120+
await fs.writeFile(
121+
staleLock,
122+
JSON.stringify({ pid: -1, createdAt: new Date(Date.now() - 120_000).toISOString() }),
123+
"utf8",
124+
);
125+
await fs.writeFile(
126+
freshLock,
127+
JSON.stringify({ pid: process.pid, createdAt: new Date().toISOString() }),
128+
"utf8",
129+
);
130+
131+
const locks = await detectStaleSessionLocks({
132+
staleMs: 30_000,
133+
readOwnerProcessArgs: () => ["node", "/opt/openclaw/openclaw.mjs", "doctor"],
134+
});
135+
136+
expect(locks).toHaveLength(1);
137+
expect(locks[0]?.lockPath).toBe(staleLock);
138+
await expect(fs.access(staleLock)).resolves.toBeUndefined();
139+
await expect(fs.access(freshLock)).resolves.toBeUndefined();
140+
});
141+
142+
it("maps stale locks to structured findings and dry-run effects", async () => {
143+
const sessionsDir = state.sessionsDir();
144+
await fs.mkdir(sessionsDir, { recursive: true });
145+
const lockPath = path.join(sessionsDir, "stale.jsonl.lock");
146+
await fs.writeFile(
147+
lockPath,
148+
JSON.stringify({ pid: -1, createdAt: new Date(Date.now() - 120_000).toISOString() }),
149+
"utf8",
150+
);
151+
152+
const [lock] = await detectStaleSessionLocks({
153+
staleMs: 30_000,
154+
readOwnerProcessArgs: () => ["node", "/opt/openclaw/openclaw.mjs", "doctor"],
155+
});
156+
if (!lock) {
157+
throw new Error("expected stale session lock");
158+
}
159+
160+
expect(sessionLockToHealthFinding(lock)).toEqual(
161+
expect.objectContaining({
162+
checkId: "core/doctor/session-locks",
163+
severity: "warning",
164+
path: lockPath,
165+
}),
166+
);
167+
expect(sessionLockToRepairEffect(lock)).toEqual({
168+
kind: "state",
169+
action: "would-remove-stale-session-lock",
170+
target: lockPath,
171+
dryRunSafe: false,
172+
});
173+
});
174+
175+
it("preserves fresh malformed stale locks in dry-run repair effects", async () => {
176+
const sessionsDir = state.sessionsDir();
177+
await fs.mkdir(sessionsDir, { recursive: true });
178+
179+
const malformedLock = path.join(sessionsDir, "malformed.jsonl.lock");
180+
await fs.writeFile(malformedLock, "{}", "utf8");
181+
182+
const [lock] = await detectStaleSessionLocks({
183+
staleMs: 30_000,
184+
readOwnerProcessArgs: () => ["node", "/opt/openclaw/openclaw.mjs", "doctor"],
185+
});
186+
if (!lock) {
187+
throw new Error("expected stale session lock");
188+
}
189+
190+
expect(lock.staleReasons).toEqual(["missing-pid", "invalid-createdAt"]);
191+
expect(lock.removable).toBe(false);
192+
expect(sessionLockToHealthFinding(lock).fixHint).toContain("after the cleanup grace period");
193+
expect(sessionLockToRepairEffect(lock)).toEqual({
194+
kind: "state",
195+
action: "would-preserve-mtime-gated-stale-session-lock",
196+
target: malformedLock,
197+
dryRunSafe: false,
198+
});
199+
await expect(fs.access(malformedLock)).resolves.toBeUndefined();
200+
});
201+
202+
it("uses the supplied env to choose the structured lint state dir", async () => {
203+
const other = await createOpenClawTestState({
204+
layout: "state-only",
205+
prefix: "openclaw-doctor-locks-other-",
206+
applyEnv: false,
207+
});
208+
try {
209+
await fs.mkdir(other.sessionsDir(), { recursive: true });
210+
const lockPath = path.join(other.sessionsDir(), "other-stale.jsonl.lock");
211+
await fs.writeFile(
212+
lockPath,
213+
JSON.stringify({ pid: -1, createdAt: new Date(Date.now() - 120_000).toISOString() }),
214+
"utf8",
215+
);
216+
217+
const locks = await detectStaleSessionLocks({
218+
env: other.env,
219+
staleMs: 30_000,
220+
readOwnerProcessArgs: () => ["node", "/opt/openclaw/openclaw.mjs", "doctor"],
221+
});
222+
223+
expect(locks.map((lock) => lock.lockPath)).toEqual([lockPath]);
224+
} finally {
225+
await other.cleanup();
226+
}
227+
});
228+
229+
it("preserves report-only live OpenClaw locks in dry-run repair effects", async () => {
230+
const sessionsDir = state.sessionsDir();
231+
await fs.mkdir(sessionsDir, { recursive: true });
232+
233+
const reportOnlyLock = path.join(sessionsDir, "report-only.jsonl.lock");
234+
await fs.writeFile(
235+
reportOnlyLock,
236+
JSON.stringify({ pid: process.pid, createdAt: new Date(Date.now() - 45_000).toISOString() }),
237+
"utf8",
238+
);
239+
240+
const [lock] = await detectStaleSessionLocks({
241+
staleMs: 30_000,
242+
readOwnerProcessArgs: () => ["node", "/opt/openclaw/openclaw.mjs", "doctor"],
243+
});
244+
if (!lock) {
245+
throw new Error("expected stale session lock");
246+
}
247+
248+
expect(lock.staleReasons).toEqual(["too-old"]);
249+
expect(sessionLockToHealthFinding(lock).fixHint).toBe(
250+
"OpenClaw is preserving this live owned lock; inspect the owning process if it appears stuck.",
251+
);
252+
expect(sessionLockToRepairEffect(lock)).toEqual({
253+
kind: "state",
254+
action: "would-preserve-report-only-stale-session-lock",
255+
target: reportOnlyLock,
256+
dryRunSafe: false,
257+
});
258+
await expect(fs.access(reportOnlyLock)).resolves.toBeUndefined();
259+
});
260+
108261
it("uses configured stale threshold without removing live OpenClaw lock files", async () => {
109262
const sessionsDir = state.sessionsDir();
110263
await fs.mkdir(sessionsDir, { recursive: true });

‎src/commands/doctor-session-locks.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,19 @@ import {
99
type SessionWriteLockAcquireTimeoutConfig,
1010
} from "../agents/session-write-lock.js";
1111
import { resolveStateDir } from "../config/paths.js";
12+
import type { HealthFinding, HealthRepairEffect } from "../flows/health-checks.js";
1213
import { shortenHomePath } from "../utils.js";
1314

15+
const SESSION_LOCKS_CHECK_ID = "core/doctor/session-locks";
16+
const REPORT_ONLY_STALE_LOCK_REASONS = new Set(["too-old", "hold-exceeded"]);
17+
18+
function isReportOnlyStaleLock(lock: SessionLockInspection): boolean {
19+
return (
20+
lock.staleReasons.length > 0 &&
21+
lock.staleReasons.every((reason) => REPORT_ONLY_STALE_LOCK_REASONS.has(reason))
22+
);
23+
}
24+
1425
function formatAge(ageMs: number | null): string {
1526
if (ageMs === null) {
1627
return "unknown";
@@ -40,6 +51,57 @@ function formatLockLine(lock: SessionLockInspection): string {
4051
return `- ${shortenHomePath(lock.lockPath)} ${pidStatus} ${ageStatus} ${staleStatus}${removedStatus}`;
4152
}
4253

54+
export async function detectStaleSessionLocks(params?: {
55+
config?: SessionWriteLockAcquireTimeoutConfig;
56+
env?: NodeJS.ProcessEnv;
57+
staleMs?: number;
58+
readOwnerProcessArgs?: SessionLockOwnerProcessArgsReader;
59+
}): Promise<readonly SessionLockInspection[]> {
60+
const staleMs = params?.staleMs ?? resolveSessionWriteLockStaleMs(params?.config, params?.env);
61+
const env = params?.env ?? process.env;
62+
const sessionDirs = await resolveAgentSessionDirs(resolveStateDir(env));
63+
const staleLocks: SessionLockInspection[] = [];
64+
for (const sessionsDir of sessionDirs) {
65+
const result = await cleanStaleLockFiles({
66+
sessionsDir,
67+
staleMs,
68+
removeStale: false,
69+
readOwnerProcessArgs: params?.readOwnerProcessArgs,
70+
});
71+
staleLocks.push(...result.locks.filter((lock) => lock.stale));
72+
}
73+
return staleLocks.toSorted((a, b) => a.lockPath.localeCompare(b.lockPath));
74+
}
75+
76+
export function sessionLockToHealthFinding(lock: SessionLockInspection): HealthFinding {
77+
const fixHint = lock.removable
78+
? 'Run "openclaw doctor --fix" to remove this stale lock file automatically.'
79+
: isReportOnlyStaleLock(lock)
80+
? "OpenClaw is preserving this live owned lock; inspect the owning process if it appears stuck."
81+
: 'Run "openclaw doctor --fix" after the cleanup grace period if this stale lock remains.';
82+
return {
83+
checkId: SESSION_LOCKS_CHECK_ID,
84+
severity: "warning",
85+
message: `Stale session lock file: ${shortenHomePath(lock.lockPath)} (${lock.staleReasons.join(", ") || "unknown"})`,
86+
path: lock.lockPath,
87+
fixHint,
88+
};
89+
}
90+
91+
export function sessionLockToRepairEffect(lock: SessionLockInspection): HealthRepairEffect {
92+
const action = lock.removable
93+
? "would-remove-stale-session-lock"
94+
: isReportOnlyStaleLock(lock)
95+
? "would-preserve-report-only-stale-session-lock"
96+
: "would-preserve-mtime-gated-stale-session-lock";
97+
return {
98+
kind: "state",
99+
action,
100+
target: lock.lockPath,
101+
dryRunSafe: false,
102+
};
103+
}
104+
43105
/** Reports session write locks and removes stale locks when doctor repair is enabled. */
44106
export async function noteSessionLockHealth(params?: {
45107
shouldRepair?: boolean;

‎src/flows/doctor-core-checks.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -748,4 +748,28 @@ describe("CORE_HEALTH_CHECKS", () => {
748748
}),
749749
);
750750
});
751+
752+
it("registers stale session locks as a legacy-owned structured check", async () => {
753+
const check = getCheck(createCoreHealthChecks(createDeps()), "core/doctor/session-locks");
754+
755+
if (typeof check.repair !== "function") {
756+
throw new Error("expected session lock check repair");
757+
}
758+
await expect(
759+
check.repair(
760+
{
761+
mode: "fix",
762+
runtime,
763+
cfg: {},
764+
cwd: "/tmp/openclaw-test-workspace",
765+
},
766+
[],
767+
),
768+
).resolves.toEqual(
769+
expect.objectContaining({
770+
status: "skipped",
771+
reason: "legacy doctor session lock contribution owns cleanup",
772+
}),
773+
);
774+
});
751775
});

0 commit comments

Comments
 (0)