Skip to content

Commit 4521a01

Browse files
steipetesteipete
committed
fix: validate restart continuations by session id
1 parent 147801f commit 4521a01

8 files changed

Lines changed: 83 additions & 100 deletions

File tree

‎src/agents/command/session-store.test.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ describe("updateSessionStoreAfterAgentRun", () => {
588588
});
589589
});
590590

591-
it("starts a fresh session instead of reusing a terminal entry", async () => {
591+
it("reuses a completed run entry while the session is still fresh", async () => {
592592
await withTempSessionStore(async ({ storePath }) => {
593593
const sessionKey = "agent:main:explicit:terminal-cli-session";
594594
const existingSessionId = "terminal-cli-session-old";
@@ -621,9 +621,11 @@ describe("updateSessionStoreAfterAgentRun", () => {
621621
sessionKey,
622622
});
623623

624-
expect(result.isNewSession).toBe(true);
625-
expect(result.sessionId).not.toBe(existingSessionId);
624+
expect(result.isNewSession).toBe(false);
625+
expect(result.sessionId).toBe(existingSessionId);
626626
expect(result.sessionEntry?.sessionId).toBe(existingSessionId);
627+
expect(result.sessionEntry?.status).toBe("done");
628+
expect(result.sessionEntry?.endedAt).toBe(now - 100);
627629
});
628630
});
629631

‎src/agents/command/session.ts‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
import { resolveChannelResetConfig, resolveSessionResetType } from "../../config/sessions/reset.js";
2020
import { resolveSessionKey } from "../../config/sessions/session-key.js";
2121
import { loadSessionStore } from "../../config/sessions/store-load.js";
22-
import { hasTerminalSessionLifecycle, type SessionEntry } from "../../config/sessions/types.js";
22+
import type { SessionEntry } from "../../config/sessions/types.js";
2323
import type { OpenClawConfig } from "../../config/types.openclaw.js";
2424
import {
2525
buildAgentMainSessionKey,
@@ -332,19 +332,18 @@ export function resolveSession(opts: {
332332
resetType,
333333
resetOverride: channelReset,
334334
});
335-
const fresh =
336-
sessionEntry && !hasTerminalSessionLifecycle(sessionEntry)
337-
? evaluateSessionFreshness({
338-
updatedAt: sessionEntry.updatedAt,
339-
...resolveSessionLifecycleTimestamps({
340-
entry: sessionEntry,
341-
agentId: opts.agentId,
342-
storePath,
343-
}),
344-
now,
345-
policy: resetPolicy,
346-
}).fresh
347-
: false;
335+
const fresh = sessionEntry
336+
? evaluateSessionFreshness({
337+
updatedAt: sessionEntry.updatedAt,
338+
...resolveSessionLifecycleTimestamps({
339+
entry: sessionEntry,
340+
agentId: opts.agentId,
341+
storePath,
342+
}),
343+
now,
344+
policy: resetPolicy,
345+
}).fresh
346+
: false;
348347
const sessionId =
349348
opts.sessionId?.trim() || (fresh ? sessionEntry?.sessionId : undefined) || crypto.randomUUID();
350349
const isNewSession = !fresh && !opts.sessionId;

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,7 +1555,7 @@ describe("initSessionState reset policy", () => {
15551555
expect(peekSystemEvents(existingSessionId)).toStrictEqual([]);
15561556
});
15571557

1558-
it("rotates terminal session entries without carrying lifecycle markers forward", async () => {
1558+
it("reuses completed run entries while the session is still fresh", async () => {
15591559
vi.setSystemTime(new Date(2026, 0, 18, 5, 30, 0));
15601560
const root = await makeCaseDir("openclaw-reset-terminal-entry-");
15611561
const storePath = path.join(root, "sessions.json");
@@ -1579,18 +1579,18 @@ describe("initSessionState reset policy", () => {
15791579
commandAuthorized: true,
15801580
});
15811581

1582-
expect(result.isNewSession).toBe(true);
1583-
expect(result.sessionId).not.toBe(existingSessionId);
1582+
expect(result.isNewSession).toBe(false);
1583+
expect(result.sessionId).toBe(existingSessionId);
15841584

15851585
const persisted = JSON.parse(await fs.readFile(storePath, "utf-8")) as Record<
15861586
string,
15871587
SessionEntry
15881588
>;
1589-
expect(persisted[sessionKey]?.sessionId).toBe(result.sessionId);
1590-
expect(persisted[sessionKey]?.status).toBeUndefined();
1591-
expect(persisted[sessionKey]?.startedAt).toBeUndefined();
1592-
expect(persisted[sessionKey]?.endedAt).toBeUndefined();
1593-
expect(persisted[sessionKey]?.runtimeMs).toBeUndefined();
1589+
expect(persisted[sessionKey]?.sessionId).toBe(existingSessionId);
1590+
expect(persisted[sessionKey]?.status).toBe("done");
1591+
expect(persisted[sessionKey]?.startedAt).toBe(Date.now() - 10_000);
1592+
expect(persisted[sessionKey]?.endedAt).toBe(Date.now() - 1_000);
1593+
expect(persisted[sessionKey]?.runtimeMs).toBe(9_000);
15941594
});
15951595

15961596
it("keeps the existing stale session for /reset soft", async () => {

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

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import { parseSessionThreadInfoFast } from "../../config/sessions/thread-info.js
2828
import {
2929
DEFAULT_RESET_TRIGGERS,
3030
type GroupKeyResolution,
31-
hasTerminalSessionLifecycle,
3231
type SessionEntry,
3332
type SessionScope,
3433
} from "../../config/sessions/types.js";
@@ -433,26 +432,23 @@ export async function initSessionState(params: {
433432
const canReuseExistingEntry =
434433
Boolean(entry?.sessionId) &&
435434
typeof entry?.updatedAt === "number" &&
436-
Number.isFinite(entry.updatedAt) &&
437-
!hasTerminalSessionLifecycle(entry);
435+
Number.isFinite(entry.updatedAt);
438436
const skipImplicitExpiry = hasProviderOwnedSession(entry) && resetPolicy.configured !== true;
439437
const lifecycleTimestamps = resolveSessionLifecycleTimestamps({
440438
entry,
441439
agentId,
442440
storePath,
443441
});
444442
const entryFreshness = entry
445-
? hasTerminalSessionLifecycle(entry)
446-
? undefined
447-
: skipImplicitExpiry
448-
? ({ fresh: true } satisfies SessionFreshness)
449-
: evaluateSessionFreshness({
450-
updatedAt: entry.updatedAt,
451-
sessionStartedAt: lifecycleTimestamps.sessionStartedAt,
452-
lastInteractionAt: lifecycleTimestamps.lastInteractionAt,
453-
now,
454-
policy: resetPolicy,
455-
})
443+
? skipImplicitExpiry
444+
? ({ fresh: true } satisfies SessionFreshness)
445+
: evaluateSessionFreshness({
446+
updatedAt: entry.updatedAt,
447+
sessionStartedAt: lifecycleTimestamps.sessionStartedAt,
448+
lastInteractionAt: lifecycleTimestamps.lastInteractionAt,
449+
now,
450+
policy: resetPolicy,
451+
})
456452
: undefined;
457453
const softResetAllowed =
458454
softReset.matched &&

‎src/config/sessions/types.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -402,16 +402,6 @@ export function isTerminalSessionStatus(
402402
return status === "done" || status === "failed" || status === "killed" || status === "timeout";
403403
}
404404

405-
export function hasTerminalSessionLifecycle(entry: SessionEntry | undefined): boolean {
406-
if (!entry) {
407-
return false;
408-
}
409-
return (
410-
isTerminalSessionStatus(entry.status) ||
411-
(typeof entry.endedAt === "number" && Number.isFinite(entry.endedAt))
412-
);
413-
}
414-
415405
function isSessionPluginTraceLine(line: string): boolean {
416406
const trimmed = line.trim();
417407
return trimmed.startsWith("🔎 ") || /(?:^|\s)(?:Debug|Trace):/.test(trimmed);

‎src/gateway/server-restart-sentinel.test.ts‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,7 @@ describe("scheduleRestartSentinelWake", () => {
619619
expect(mocks.requestHeartbeat).not.toHaveBeenCalled();
620620
});
621621

622-
it("does not dispatch agentTurn continuation into a terminal session entry", async () => {
622+
it("dispatches agentTurn continuation for a completed run entry", async () => {
623623
mocks.readRestartSentinel.mockResolvedValue({
624624
payload: {
625625
sessionKey: "agent:main:main",
@@ -652,44 +652,47 @@ describe("scheduleRestartSentinelWake", () => {
652652

653653
await scheduleRestartSentinelWake({ deps: {} as never });
654654

655-
expect(mocks.enqueueSessionDelivery).not.toHaveBeenCalled();
656-
expect(mocks.recordInboundSessionAndDispatchReply).not.toHaveBeenCalled();
657-
expect(mocks.enqueueSystemEvent).toHaveBeenCalledWith("restart message", {
658-
sessionKey: "agent:main:main",
659-
deliveryContext: {
660-
channel: "whatsapp",
661-
to: "+15550002",
662-
accountId: "acct-2",
663-
threadId: "thread-42",
664-
},
665-
});
666-
expect(mocks.logWarn).toHaveBeenCalledWith(
667-
"restart summary: continuation skipped: session is terminal",
668-
{
655+
expect(mocks.enqueueSessionDelivery).toHaveBeenCalledWith(
656+
expect.objectContaining({
657+
kind: "agentTurn",
669658
sessionKey: "agent:main:main",
670-
continuationKind: "agentTurn",
671-
status: "done",
672-
endedAt: expect.any(Number),
673-
},
659+
message: "continue after restart",
660+
messageId: "restart-sentinel:agent:main:main:agentTurn:123",
661+
expectedSessionId: "agent:main:main",
662+
route: {
663+
channel: "whatsapp",
664+
to: "+15550002",
665+
accountId: "acct-2",
666+
threadId: "thread-42",
667+
chatType: "direct",
668+
},
669+
}),
670+
);
671+
expect(mocks.recordInboundSessionAndDispatchReply).toHaveBeenCalledTimes(1);
672+
expectContinuationDispatchFields(
673+
{ routeSessionKey: "agent:main:main" },
674+
{ Body: "continue after restart" },
674675
);
676+
expect(mocks.enqueueSystemEvent).not.toHaveBeenCalled();
677+
expect(mocks.logWarn).not.toHaveBeenCalled();
675678
});
676679

677-
it("does not dispatch a queued agentTurn continuation after the session becomes terminal", async () => {
680+
it("does not dispatch a queued agentTurn continuation after the session key changes", async () => {
678681
const activeEntry: LoadedSessionEntry = {
679682
cfg: {},
680683
entry: {
681-
sessionId: "agent:main:main",
684+
sessionId: "old-session-id",
682685
updatedAt: Date.now(),
683686
},
684687
store: {},
685688
storePath: "/tmp/sessions.json",
686689
canonicalKey: "agent:main:main",
687690
legacyKey: undefined,
688691
};
689-
const terminalEntry: LoadedSessionEntry = {
692+
const replacementEntry: LoadedSessionEntry = {
690693
cfg: {},
691694
entry: {
692-
sessionId: "agent:main:main",
695+
sessionId: "new-session-id",
693696
updatedAt: Date.now(),
694697
status: "done",
695698
endedAt: Date.now() - 1_000,
@@ -715,7 +718,7 @@ describe("scheduleRestartSentinelWake", () => {
715718
},
716719
},
717720
} as Awaited<ReturnType<typeof mocks.readRestartSentinel>>);
718-
mocks.loadSessionEntry.mockReturnValueOnce(activeEntry).mockReturnValue(terminalEntry);
721+
mocks.loadSessionEntry.mockReturnValueOnce(activeEntry).mockReturnValue(replacementEntry);
719722

720723
await scheduleRestartSentinelWake({ deps: {} as never });
721724

@@ -736,18 +739,15 @@ describe("scheduleRestartSentinelWake", () => {
736739
reason: "wake",
737740
sessionKey: "agent:main:main",
738741
});
739-
expect(mocks.logWarn).toHaveBeenCalledWith(
740-
"restart continuation skipped: session is terminal",
741-
{
742-
sessionKey: "agent:main:main",
743-
queueId: "session-delivery-1",
744-
status: "done",
745-
endedAt: expect.any(Number),
746-
},
747-
);
742+
expect(mocks.logWarn).toHaveBeenCalledWith("restart continuation skipped: session changed", {
743+
sessionKey: "agent:main:main",
744+
queueId: "session-delivery-1",
745+
expectedSessionId: "old-session-id",
746+
actualSessionId: "new-session-id",
747+
});
748748
});
749749

750-
it("still delivers systemEvent continuations for terminal session entries", async () => {
750+
it("still delivers systemEvent continuations for completed run entries", async () => {
751751
mocks.readRestartSentinel.mockResolvedValue({
752752
payload: {
753753
sessionKey: "agent:main:main",
@@ -791,7 +791,7 @@ describe("scheduleRestartSentinelWake", () => {
791791
});
792792
expect(mocks.recordInboundSessionAndDispatchReply).not.toHaveBeenCalled();
793793
expect(mocks.logWarn).not.toHaveBeenCalledWith(
794-
"restart summary: continuation skipped: session is terminal",
794+
"restart continuation skipped: session changed",
795795
expect.anything(),
796796
);
797797
});

‎src/gateway/server-restart-sentinel.ts‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { dispatchAssembledChannelTurn } from "../channels/turn/kernel.js";
1010
import type { CliDeps } from "../cli/deps.types.js";
1111
import { resolveMainSessionKeyFromConfig } from "../config/sessions.js";
1212
import { parseSessionThreadInfo } from "../config/sessions/thread-info.js";
13-
import { hasTerminalSessionLifecycle } from "../config/sessions/types.js";
1413
import { formatErrorMessage } from "../infra/errors.js";
1514
import { requestHeartbeat } from "../infra/heartbeat-wake.js";
1615
import { ackDelivery, enqueueDelivery, failDelivery } from "../infra/outbound/delivery-queue.js";
@@ -263,12 +262,15 @@ async function deliverQueuedSessionDelivery(params: {
263262
return;
264263
}
265264

266-
if (hasTerminalSessionLifecycle(entry)) {
267-
log.warn("restart continuation skipped: session is terminal", {
265+
if (
266+
params.entry.expectedSessionId &&
267+
(!entry?.sessionId || entry.sessionId !== params.entry.expectedSessionId)
268+
) {
269+
log.warn("restart continuation skipped: session changed", {
268270
sessionKey: canonicalKey,
269271
queueId: params.entry.id,
270-
status: entry?.status ?? null,
271-
endedAt: entry?.endedAt ?? null,
272+
expectedSessionId: params.entry.expectedSessionId,
273+
actualSessionId: entry?.sessionId ?? null,
272274
});
273275
enqueueRestartSentinelWake(params.entry.message, canonicalKey, queuedDeliveryContext);
274276
return;
@@ -397,6 +399,7 @@ function buildQueuedRestartContinuation(params: {
397399
sessionKey: string;
398400
continuation: RestartSentinelContinuation;
399401
route?: SessionDeliveryRoute;
402+
expectedSessionId?: string | undefined;
400403
ts: number;
401404
deliveryContext?: {
402405
channel?: string;
@@ -425,6 +428,7 @@ function buildQueuedRestartContinuation(params: {
425428
sessionKey: params.sessionKey,
426429
message: params.continuation.message,
427430
messageId: idempotencyKey,
431+
...(params.expectedSessionId ? { expectedSessionId: params.expectedSessionId } : {}),
428432
maxRetries: RESTART_CONTINUATION_BUSY_MAX_ATTEMPTS,
429433
...(params.route ? { route: params.route } : {}),
430434
...(params.deliveryContext ? { deliveryContext: params.deliveryContext } : {}),
@@ -532,7 +536,6 @@ async function loadRestartSentinelStartupTask(params: {
532536
const { baseSessionKey, threadId: sessionThreadId } = parseSessionThreadInfo(sessionKey);
533537

534538
const { cfg, entry, canonicalKey } = loadSessionEntry(sessionKey);
535-
const terminalSessionEntry = hasTerminalSessionLifecycle(entry);
536539

537540
const sentinelContext = payload.deliveryContext;
538541
let sessionDeliveryContext = deliveryContextFromSession(entry);
@@ -591,9 +594,7 @@ async function loadRestartSentinelStartupTask(params: {
591594
}
592595
}
593596

594-
const terminalAgentTurnContinuation =
595-
payload.continuation?.kind === "agentTurn" && terminalSessionEntry;
596-
if (payload.continuation && !terminalAgentTurnContinuation) {
597+
if (payload.continuation) {
597598
continuationRoute = resolveRestartContinuationRoute({
598599
channel: channel ?? undefined,
599600
to: resolvedTo,
@@ -608,6 +609,7 @@ async function loadRestartSentinelStartupTask(params: {
608609
continuation: payload.continuation,
609610
ts: payload.ts,
610611
route: continuationRoute,
612+
expectedSessionId: entry?.sessionId,
611613
deliveryContext:
612614
resolvedTo && channel
613615
? {
@@ -619,13 +621,6 @@ async function loadRestartSentinelStartupTask(params: {
619621
: wakeDeliveryContext,
620622
}),
621623
);
622-
} else if (terminalAgentTurnContinuation) {
623-
log.warn(`${summary}: continuation skipped: session is terminal`, {
624-
sessionKey: canonicalKey,
625-
continuationKind: "agentTurn",
626-
status: entry?.status ?? null,
627-
endedAt: entry?.endedAt ?? null,
628-
});
629624
}
630625

631626
await removeRestartSentinelFile(sentinelPath);

‎src/infra/session-delivery-queue-storage.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export type QueuedSessionDeliveryPayload =
5353
sessionKey: string;
5454
message: string;
5555
messageId: string;
56+
expectedSessionId?: string;
5657
route?: SessionDeliveryRoute;
5758
deliveryContext?: SessionDeliveryContext;
5859
idempotencyKey?: string;

0 commit comments

Comments
 (0)