Skip to content

Commit 23f73b3

Browse files
joshavantjoshavant
authored
Fix session reset files and ACPX orphan reaping (#82459)
* fix gateway reset transcript rotation * fix acpx orphan adapter reaping * add changelog for pr 82459 * fix ci session metadata normalization * preserve canonical session ids in store reads * fix session metadata id normalization
1 parent 2fa853d commit 23f73b3

12 files changed

Lines changed: 345 additions & 69 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ Docs: https://docs.openclaw.ai
5959
- Providers/embeddings: reject malformed successful OpenAI-compatible, Google Gemini, and Amazon Bedrock embedding responses instead of silently returning empty or coerced vectors.
6060
- Providers/catalogs: reject malformed successful LM Studio, GitHub Copilot, DeepInfra, Vercel AI Gateway, and Kilocode model-list responses with provider-owned errors instead of raw parser/type failures or silent fallback catalogs.
6161
- Providers/polling: reject array, null, or scalar successful operation status responses with provider-owned malformed JSON errors instead of waiting until timeout.
62+
- ACPX/Codex: reap plugin-local Codex ACP adapter orphans on startup after wrapper crashes while keeping direct adapter commands out of launch-lease injection. Fixes #82364. (#82459) Thanks @joshavant.
6263
- Telegram: send presentation-only payloads by rendering fallback text and inline buttons instead of treating them as empty. Fixes #82404. (#82449) Thanks @joshavant.
6364
- Trajectory export: skip and report malformed session/runtime JSONL rows in `manifest.json` instead of letting wrong-shaped session rows crash support bundle export.
6465
- Voice calls: persist rejected inbound-call replay keys so duplicate carrier webhook retries stay ignored after a Gateway restart.

‎extensions/acpx/src/process-reaper.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
import path from "node:path";
12
import { describe, expect, it, vi } from "vitest";
23
import { OPENCLAW_ACPX_LEASE_ID_ARG, OPENCLAW_GATEWAY_INSTANCE_ID_ARG } from "./process-lease.js";
34
import {
45
cleanupOpenClawOwnedAcpxProcessTree,
6+
isOpenClawLeaseAwareAcpxProcessCommand,
57
isOpenClawOwnedAcpxProcessCommand,
68
reapStaleOpenClawOwnedAcpxOrphans,
79
type AcpxProcessInfo,
@@ -13,6 +15,12 @@ const CODEX_WRAPPER_COMMAND_WITH_LEASE = `${CODEX_WRAPPER_COMMAND} ${OPENCLAW_AC
1315
const CLAUDE_WRAPPER_COMMAND = `node ${WRAPPER_ROOT}/claude-agent-acp-wrapper.mjs`;
1416
const PLUGIN_DEPS_CODEX_COMMAND =
1517
"node /tmp/openclaw/plugin-runtime-deps/node_modules/@zed-industries/codex-acp/bin/codex-acp.js";
18+
const LOCAL_NODE_MODULES_CODEX_COMMAND = `node ${path.resolve(
19+
"node_modules/@zed-industries/codex-acp/bin/codex-acp.js",
20+
)}`;
21+
const LOCAL_NODE_MODULES_CODEX_PLATFORM_COMMAND = path.resolve(
22+
"node_modules/@zed-industries/codex-acp-linux-x64/bin/codex-acp",
23+
);
1624

1725
function cleanupDeps(processes: AcpxProcessInfo[]) {
1826
const killed: Array<{ pid: number; signal: NodeJS.Signals }> = [];
@@ -64,13 +72,39 @@ describe("process reaper", () => {
6472
).toBe(false);
6573
});
6674

75+
it("only treats generated wrappers as launch-lease aware", () => {
76+
expect(
77+
isOpenClawLeaseAwareAcpxProcessCommand({
78+
command: CODEX_WRAPPER_COMMAND,
79+
wrapperRoot: WRAPPER_ROOT,
80+
}),
81+
).toBe(true);
82+
expect(
83+
isOpenClawLeaseAwareAcpxProcessCommand({ command: LOCAL_NODE_MODULES_CODEX_COMMAND }),
84+
).toBe(false);
85+
expect(isOpenClawLeaseAwareAcpxProcessCommand({ command: PLUGIN_DEPS_CODEX_COMMAND })).toBe(
86+
false,
87+
);
88+
});
89+
6790
it("recognizes OpenClaw plugin-runtime-deps ACP adapter children", () => {
6891
expect(isOpenClawOwnedAcpxProcessCommand({ command: PLUGIN_DEPS_CODEX_COMMAND })).toBe(true);
6992
expect(isOpenClawOwnedAcpxProcessCommand({ command: "npx @zed-industries/codex-acp" })).toBe(
7093
false,
7194
);
7295
});
7396

97+
it("recognizes plugin-local ACP adapter package paths without trusting arbitrary installs", () => {
98+
expect(isOpenClawOwnedAcpxProcessCommand({ command: LOCAL_NODE_MODULES_CODEX_COMMAND })).toBe(
99+
true,
100+
);
101+
expect(
102+
isOpenClawOwnedAcpxProcessCommand({
103+
command: "node /tmp/other-project/node_modules/@zed-industries/codex-acp/bin/codex-acp.js",
104+
}),
105+
).toBe(false);
106+
});
107+
74108
it("kills an owned recorded process tree children first", async () => {
75109
const { deps, killed } = cleanupDeps([
76110
{ pid: 100, ppid: 1, command: CODEX_WRAPPER_COMMAND },
@@ -260,6 +294,28 @@ describe("process reaper", () => {
260294
).toEqual([402, 401, 400, 404, 403, 405]);
261295
});
262296

297+
it("reaps plugin-local Codex ACP adapter orphans when the generated wrapper is already gone", async () => {
298+
const { deps, killed } = cleanupDeps([
299+
{ pid: 500, ppid: 1, command: LOCAL_NODE_MODULES_CODEX_COMMAND },
300+
{ pid: 501, ppid: 500, command: LOCAL_NODE_MODULES_CODEX_PLATFORM_COMMAND },
301+
]);
302+
303+
const result = await reapStaleOpenClawOwnedAcpxOrphans({
304+
wrapperRoot: WRAPPER_ROOT,
305+
deps,
306+
});
307+
308+
expect(result.skippedReason).toBeUndefined();
309+
expect(result.inspectedPids).toEqual([500, 501]);
310+
expect(
311+
collectMatching(
312+
killed,
313+
(entry) => entry.signal === "SIGTERM",
314+
(entry) => entry.pid,
315+
),
316+
).toEqual([501, 500]);
317+
});
318+
263319
it("keeps startup scans quiet when process listing is unavailable", async () => {
264320
const result = await reapStaleOpenClawOwnedAcpxOrphans({
265321
wrapperRoot: WRAPPER_ROOT,

‎extensions/acpx/src/process-reaper.ts‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,27 @@
11
import { execFile } from "node:child_process";
2+
import { createRequire } from "node:module";
3+
import path from "node:path";
24
import { promisify } from "node:util";
35
import { OPENCLAW_ACPX_LEASE_ID_ARG, OPENCLAW_GATEWAY_INSTANCE_ID_ARG } from "./process-lease.js";
46

57
const execFileAsync = promisify(execFile);
8+
const requireFromHere = createRequire(import.meta.url);
69
const GENERATED_WRAPPER_BASENAMES = new Set([
710
"codex-acp-wrapper.mjs",
811
"claude-agent-acp-wrapper.mjs",
912
]);
1013
const OPENCLAW_PLUGIN_DEPS_MARKER = "/plugin-runtime-deps/";
14+
const OWNED_ACP_PACKAGE_NAMES = [
15+
"@zed-industries/codex-acp",
16+
"@zed-industries/codex-acp-darwin-arm64",
17+
"@zed-industries/codex-acp-darwin-x64",
18+
"@zed-industries/codex-acp-linux-arm64",
19+
"@zed-industries/codex-acp-linux-x64",
20+
"@zed-industries/codex-acp-win32-arm64",
21+
"@zed-industries/codex-acp-win32-x64",
22+
"@agentclientprotocol/claude-agent-acp",
23+
"acpx",
24+
];
1125
const ACP_PACKAGE_MARKERS = [
1226
"/@zed-industries/codex-acp/",
1327
"/@agentclientprotocol/claude-agent-acp/",
@@ -42,6 +56,22 @@ function normalizePathLike(value: string): string {
4256
return value.replaceAll("\\", "/");
4357
}
4458

59+
function resolvePackageRoot(packageName: string): string | undefined {
60+
try {
61+
return normalizePathLike(path.dirname(requireFromHere.resolve(`${packageName}/package.json`)));
62+
} catch {
63+
return undefined;
64+
}
65+
}
66+
67+
const OWNED_ACP_PACKAGE_ROOTS = OWNED_ACP_PACKAGE_NAMES.map(resolvePackageRoot).filter(
68+
(root): root is string => Boolean(root),
69+
);
70+
71+
function commandBelongsToResolvedAcpPackage(command: string): boolean {
72+
return OWNED_ACP_PACKAGE_ROOTS.some((root) => command.includes(`${root}/`));
73+
}
74+
4575
function commandMentionsGeneratedWrapper(command: string): boolean {
4676
return Array.from(GENERATED_WRAPPER_BASENAMES).some((basename) => command.includes(basename));
4777
}
@@ -57,6 +87,21 @@ function commandWrapperBelongsToRoot(command: string, wrapperRoot: string | unde
5787
);
5888
}
5989

90+
export function isOpenClawLeaseAwareAcpxProcessCommand(params: {
91+
command: string | undefined;
92+
wrapperRoot?: string;
93+
}): boolean {
94+
const command = params.command?.trim();
95+
if (!command) {
96+
return false;
97+
}
98+
const normalized = normalizePathLike(command);
99+
return (
100+
commandMentionsGeneratedWrapper(normalized) &&
101+
commandWrapperBelongsToRoot(normalized, params.wrapperRoot)
102+
);
103+
}
104+
60105
function commandsReferToSameRootCommand(liveCommand: string, storedCommand: string | undefined) {
61106
if (!storedCommand?.trim()) {
62107
return true;
@@ -147,8 +192,16 @@ export function isOpenClawOwnedAcpxProcessCommand(params: {
147192
return false;
148193
}
149194
const normalized = normalizePathLike(command);
150-
if (commandMentionsGeneratedWrapper(normalized)) {
151-
return commandWrapperBelongsToRoot(normalized, params.wrapperRoot);
195+
if (
196+
isOpenClawLeaseAwareAcpxProcessCommand({
197+
command: normalized,
198+
wrapperRoot: params.wrapperRoot,
199+
})
200+
) {
201+
return true;
202+
}
203+
if (commandBelongsToResolvedAcpPackage(normalized)) {
204+
return true;
152205
}
153206
if (!normalized.includes(OPENCLAW_PLUGIN_DEPS_MARKER)) {
154207
return false;

‎extensions/acpx/src/runtime.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ const DOCUMENTED_OPENCLAW_BRIDGE_COMMAND =
1616
const CODEX_ACP_COMMAND = "npx @zed-industries/codex-acp@0.13.0";
1717
const CODEX_ACP_WRAPPER_COMMAND = `node "/tmp/openclaw/acpx/codex-acp-wrapper.mjs"`;
1818
const CODEX_ACP_WRAPPER_COMMAND_WITH_LEASE = `${CODEX_ACP_WRAPPER_COMMAND} ${OPENCLAW_ACPX_LEASE_ID_ARG} lease-close ${OPENCLAW_GATEWAY_INSTANCE_ID_ARG} gateway-test`;
19+
const LOCAL_NODE_MODULES_CODEX_COMMAND = `node "${path.resolve(
20+
"node_modules/@zed-industries/codex-acp/bin/codex-acp.js",
21+
)}"`;
1922

2023
function makeRuntime(
2124
baseStore: TestSessionStore,
@@ -904,6 +907,50 @@ describe("AcpxRuntime fresh reset wrapper", () => {
904907
expect(savedRecords[0]?.openclawLeaseId).toBe(lease?.leaseId);
905908
});
906909

910+
it("does not create launch leases for direct plugin-local ACP adapter commands", async () => {
911+
const launchCommands: string[] = [];
912+
const baseStore: TestSessionStore = {
913+
load: vi.fn(async () => undefined),
914+
save: vi.fn(async () => {}),
915+
};
916+
const leaseStore = makeLeaseStore();
917+
const { runtime, delegate, wrappedStore } = makeRuntime(baseStore, {
918+
openclawGatewayInstanceId: "gateway-test",
919+
openclawProcessLeaseStore: leaseStore.store,
920+
openclawWrapperRoot: "/tmp/openclaw/acpx",
921+
agentRegistry: {
922+
resolve: (agentName: string) =>
923+
agentName === "codex" ? LOCAL_NODE_MODULES_CODEX_COMMAND : agentName,
924+
list: () => ["codex"],
925+
},
926+
});
927+
vi.spyOn(delegate, "ensureSession").mockImplementation(async (input) => {
928+
const command = (
929+
runtime as unknown as { scopedAgentRegistry: { resolve(agent: string): string } }
930+
).scopedAgentRegistry.resolve("codex");
931+
launchCommands.push(command);
932+
await wrappedStore.save({
933+
name: input.sessionKey,
934+
agentCommand: command,
935+
pid: 777,
936+
});
937+
return {
938+
sessionKey: input.sessionKey,
939+
backend: "acpx",
940+
runtimeSessionName: input.sessionKey,
941+
};
942+
});
943+
944+
await runtime.ensureSession({
945+
sessionKey: "agent:codex:acp:binding:test",
946+
agent: "codex",
947+
mode: "persistent",
948+
});
949+
950+
expect(leaseStore.store.save).not.toHaveBeenCalled();
951+
expect(launchCommands).toEqual([LOCAL_NODE_MODULES_CODEX_COMMAND]);
952+
});
953+
907954
it("keeps reusable persistent ACP launch commands stable across ensures", async () => {
908955
const baseStore: TestSessionStore = {
909956
load: vi.fn(async () => ({

‎extensions/acpx/src/runtime.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
} from "./process-lease.js";
2828
import {
2929
cleanupOpenClawOwnedAcpxProcessTree,
30-
isOpenClawOwnedAcpxProcessCommand,
30+
isOpenClawLeaseAwareAcpxProcessCommand,
3131
type AcpxProcessCleanupDeps,
3232
} from "./process-reaper.js";
3333

@@ -785,7 +785,7 @@ export class AcpxRuntime implements AcpRuntime {
785785
!this.wrapperRoot ||
786786
!this.gatewayInstanceId ||
787787
!this.processLeaseStore ||
788-
!isOpenClawOwnedAcpxProcessCommand({
788+
!isOpenClawLeaseAwareAcpxProcessCommand({
789789
command: params.command,
790790
wrapperRoot: this.wrapperRoot,
791791
})

‎src/auto-reply/reply/session-updates.ts‎

Lines changed: 2 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import crypto from "node:crypto";
2-
import fs from "node:fs";
32
import path from "node:path";
43
import { resolveSessionAgentId } from "../../agents/agent-scope.js";
54
import { canExecRequestNode } from "../../agents/exec-defaults.js";
@@ -13,8 +12,10 @@ import { ensureSkillsWatcher } from "../../agents/skills/refresh.js";
1312
import { hydrateResolvedSkills } from "../../agents/skills/snapshot-hydration.js";
1413
import { stableStringify } from "../../agents/stable-stringify.js";
1514
import {
15+
canonicalizeAbsoluteSessionFilePath,
1616
resolveSessionFilePath,
1717
resolveSessionFilePathOptions,
18+
rewriteSessionFileForNewSessionId,
1819
type SessionEntry,
1920
updateSessionStore,
2021
} from "../../config/sessions.js";
@@ -470,53 +471,3 @@ function resolveCompactionSessionFile(params: {
470471
pathOpts,
471472
);
472473
}
473-
474-
function canonicalizeAbsoluteSessionFilePath(filePath: string): string {
475-
const resolved = path.resolve(filePath);
476-
const missingSegments: string[] = [];
477-
let cursor = resolved;
478-
while (true) {
479-
try {
480-
return path.join(fs.realpathSync(cursor), ...missingSegments.toReversed());
481-
} catch {
482-
const parent = path.dirname(cursor);
483-
if (parent === cursor) {
484-
return resolved;
485-
}
486-
missingSegments.push(path.basename(cursor));
487-
cursor = parent;
488-
}
489-
}
490-
}
491-
492-
function rewriteSessionFileForNewSessionId(params: {
493-
sessionFile?: string;
494-
previousSessionId: string;
495-
nextSessionId: string;
496-
}): string | undefined {
497-
const trimmed = normalizeOptionalString(params.sessionFile);
498-
if (!trimmed) {
499-
return undefined;
500-
}
501-
const base = path.basename(trimmed);
502-
if (!base.endsWith(".jsonl")) {
503-
return undefined;
504-
}
505-
const withoutExt = base.slice(0, -".jsonl".length);
506-
if (withoutExt === params.previousSessionId) {
507-
return path.join(path.dirname(trimmed), `${params.nextSessionId}.jsonl`);
508-
}
509-
if (withoutExt.startsWith(`${params.previousSessionId}-topic-`)) {
510-
return path.join(
511-
path.dirname(trimmed),
512-
`${params.nextSessionId}${base.slice(params.previousSessionId.length)}`,
513-
);
514-
}
515-
const forkMatch = withoutExt.match(
516-
/^(\d{4}-\d{2}-\d{2}T[\w-]+(?:Z|[+-]\d{2}(?:-\d{2})?)?)_(.+)$/,
517-
);
518-
if (forkMatch?.[2] === params.previousSessionId) {
519-
return path.join(path.dirname(trimmed), `${forkMatch[1]}_${params.nextSessionId}.jsonl`);
520-
}
521-
return undefined;
522-
}

‎src/config/sessions.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ export * from "./sessions/store.js";
1212
export * from "./sessions/types.js";
1313
export * from "./sessions/transcript.js";
1414
export * from "./sessions/session-file.js";
15+
export * from "./sessions/session-file-rotation.js";
1516
export * from "./sessions/delivery-info.js";
1617
export * from "./sessions/disk-budget.js";
1718
export * from "./sessions/targets.js";

0 commit comments

Comments
 (0)