Skip to content

Commit e48dd56

Browse files
openperfopenperf
authored
Merge f63c221 into 6cb82ea 
2 parents 6cb82ea + f63c221 commit e48dd56

6 files changed

Lines changed: 442 additions & 119 deletions

‎src/agents/subagent-registry-run-manager.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import type { OpenClawConfig } from "../config/types.openclaw.js";
88
import { callGateway } from "../gateway/call.js";
99
import { createSubsystemLogger } from "../logging/subsystem.js";
1010
import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
11-
import { createRunningTaskRun } from "../tasks/detached-task-runtime.js";
11+
import {
12+
createRunningTaskRun,
13+
setDetachedTaskDeliveryStatusByRunId,
14+
} from "../tasks/detached-task-runtime.js";
1215
import { normalizeDeliveryContext } from "../utils/delivery-context.shared.js";
1316
import type { DeliveryContext } from "../utils/delivery-context.types.js";
1417
import { buildAgentRunTerminalOutcomeFromWaitResult } from "./agent-run-terminal-outcome.js";
@@ -795,6 +798,23 @@ export function createSubagentRunManager(params: {
795798
if (updated > 0) {
796799
params.persist();
797800
for (const entry of entriesByChildSessionKey.values()) {
801+
// Mark delivery not_applicable immediately; task-row status is left to
802+
// maintenance to avoid writing "failed" before a concurrent lifecycle
803+
// COMPLETE can write "succeeded" (terminal-to-terminal guard blocks it).
804+
try {
805+
setDetachedTaskDeliveryStatusByRunId({
806+
runId: entry.runId,
807+
runtime: "subagent",
808+
sessionKey: entry.childSessionKey,
809+
deliveryStatus: "not_applicable",
810+
});
811+
} catch (err) {
812+
log.warn("failed to update killed subagent background task delivery state", {
813+
err,
814+
runId: entry.runId,
815+
childSessionKey: entry.childSessionKey,
816+
});
817+
}
798818
const emitEndedHook = () =>
799819
emitSubagentEndedHookOnce({
800820
entry,

‎src/agents/subagent-registry.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ const mocks = vi.hoisted(() => ({
152152
runSubagentEnded: vi.fn(async () => {}),
153153
resolveAgentTimeoutMs: vi.fn(() => 1_000),
154154
scheduleOrphanRecovery: vi.fn(),
155+
failTaskRunByRunId: vi.fn(),
156+
completeTaskRunByRunId: vi.fn(),
157+
setDetachedTaskDeliveryStatusByRunId: vi.fn(),
155158
}));
156159

157160
vi.mock("../gateway/call.js", () => ({
@@ -222,6 +225,13 @@ vi.mock("./subagent-orphan-recovery.js", () => ({
222225
scheduleOrphanRecovery: mocks.scheduleOrphanRecovery,
223226
}));
224227

228+
vi.mock("../tasks/detached-task-runtime.js", () => ({
229+
createRunningTaskRun: vi.fn(),
230+
failTaskRunByRunId: mocks.failTaskRunByRunId,
231+
completeTaskRunByRunId: mocks.completeTaskRunByRunId,
232+
setDetachedTaskDeliveryStatusByRunId: mocks.setDetachedTaskDeliveryStatusByRunId,
233+
}));
234+
225235
describe("subagent registry seam flow", () => {
226236
let mod: typeof import("./subagent-registry.js");
227237

@@ -3560,6 +3570,51 @@ describe("subagent registry seam flow", () => {
35603570
});
35613571
});
35623572

3573+
it("marks delivery not_applicable when a run is killed", () => {
3574+
// Regression: the kill path persisted subagent run terminal state but
3575+
// never finalized the mirrored task row, leaving it stuck in running so
3576+
// cancel and maintenance could not clear it (#90444).
3577+
// The fix: mark delivery not_applicable immediately (killed runs skip the
3578+
// announce flow) and defer task-row status finalization to maintenance,
3579+
// which avoids the kill-vs-complete race where a premature "failed" write
3580+
// blocks a concurrent lifecycle completion from writing "succeeded".
3581+
mod.registerSubagentRun({
3582+
runId: "run-task-kill-finalize",
3583+
childSessionKey: "agent:main:subagent:task-kill",
3584+
requesterSessionKey: "agent:main:main",
3585+
requesterDisplayKey: "main",
3586+
task: "task finalized on kill",
3587+
cleanup: "keep",
3588+
});
3589+
3590+
mocks.failTaskRunByRunId.mockClear();
3591+
mocks.setDetachedTaskDeliveryStatusByRunId.mockClear();
3592+
3593+
const updated = mod.markSubagentRunTerminated({
3594+
runId: "run-task-kill-finalize",
3595+
reason: "killed",
3596+
});
3597+
3598+
expect(updated).toBe(1);
3599+
expect(mocks.failTaskRunByRunId).not.toHaveBeenCalled();
3600+
expect(mocks.setDetachedTaskDeliveryStatusByRunId).toHaveBeenCalledOnce();
3601+
expectRecordFields(
3602+
getMockCallArg(
3603+
mocks.setDetachedTaskDeliveryStatusByRunId,
3604+
0,
3605+
0,
3606+
"setDetachedTaskDeliveryStatusByRunId call",
3607+
),
3608+
{
3609+
runId: "run-task-kill-finalize",
3610+
runtime: "subagent",
3611+
sessionKey: "agent:main:subagent:task-kill",
3612+
deliveryStatus: "not_applicable",
3613+
},
3614+
"setDetachedTaskDeliveryStatusByRunId params",
3615+
);
3616+
});
3617+
35633618
it("announces readable failure when an interrupted run is finalized", async () => {
35643619
mod.addSubagentRunForTests({
35653620
runId: "run-interrupted",

‎src/tasks/task-registry.maintenance.issue-60299.test.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,15 @@ function createTaskRegistryMaintenanceHarness(params: {
6666
cronStore?: CronStoreFile;
6767
cronRunLogEntries?: Record<string, CronRunLogEntry[]>;
6868
runtimeAuthoritative?: boolean;
69+
terminalSubagentRunEndedAt?: Record<string, number>;
6970
}) {
7071
const sessionStore = params.sessionStore ?? {};
7172
const acpEntry = params.acpEntry;
7273
const activeCronJobIds = new Set(params.activeCronJobIds ?? []);
7374
const activeRunIds = new Set(params.activeRunIds ?? []);
7475
const activeAcpSessionKeys = new Set(params.activeAcpSessionKeys ?? []);
7576
const cronRunLogEntries = params.cronRunLogEntries ?? {};
77+
const terminalSubagentRunEndedAt = params.terminalSubagentRunEndedAt ?? {};
7678
const currentTasks = new Map(params.tasks.map((task) => [task.taskId, { ...task }]));
7779

7880
const runtime: TaskRegistryMaintenanceRuntime = {
@@ -181,6 +183,7 @@ function createTaskRegistryMaintenanceHarness(params: {
181183
resolveCronJobsStorePath: () => "/tmp/openclaw-test-cron/jobs.json",
182184
loadCronJobsStoreSync: () => params.cronStore ?? { version: 1, jobs: [] },
183185
readCronRunLogEntriesSync: ({ jobId }) => (jobId ? (cronRunLogEntries[jobId] ?? []) : []),
186+
getSubagentRunEndedAt: (runId: string) => terminalSubagentRunEndedAt[runId],
184187
};
185188

186189
setTaskRegistryMaintenanceRuntimeForTests(runtime);
@@ -861,3 +864,153 @@ describe("task-registry maintenance issue #60299", () => {
861864
expect(hookNow).toBeGreaterThanOrEqual(beforeMaintenance);
862865
});
863866
});
867+
868+
describe("task-registry maintenance issue #90444", () => {
869+
it("marks a running subagent task lost when its in-memory run is terminal", async () => {
870+
// Regression: the kill path defers task-row finalization to maintenance to
871+
// avoid the kill-vs-complete race. Maintenance must detect terminal
872+
// in-memory subagent runs and clear their stuck running task rows.
873+
const runId = "run-killed-zombie-90444";
874+
const task = makeStaleTask({
875+
runtime: "subagent",
876+
runId,
877+
childSessionKey: "agent:main:subagent:zombie-90444",
878+
});
879+
880+
// Session store still has an entry (kill happened before session cleanup).
881+
const { currentTasks } = createTaskRegistryMaintenanceHarness({
882+
tasks: [task],
883+
sessionStore: {
884+
"agent:main:subagent:zombie-90444": {
885+
sessionId: "sess-zombie-90444",
886+
updatedAt: Date.now(),
887+
},
888+
},
889+
// The in-memory run is terminal (endedAt set).
890+
terminalSubagentRunEndedAt: { [runId]: Date.now() - 5000 },
891+
runtimeAuthoritative: true,
892+
});
893+
894+
expectMaintenanceCounts(await runTaskRegistryMaintenance(), { reconciled: 1 });
895+
expectTaskStatus(currentTasks, task.taskId, "lost");
896+
});
897+
898+
it("keeps a running subagent task live when its in-memory run has not ended", async () => {
899+
const runId = "run-active-subagent-90444";
900+
const task = makeStaleTask({
901+
runtime: "subagent",
902+
runId,
903+
childSessionKey: "agent:main:subagent:active-90444",
904+
});
905+
906+
const { currentTasks } = createTaskRegistryMaintenanceHarness({
907+
tasks: [task],
908+
sessionStore: {
909+
"agent:main:subagent:active-90444": {
910+
sessionId: "sess-active-90444",
911+
updatedAt: Date.now(),
912+
},
913+
},
914+
// No endedAt in the terminal map → run is still live.
915+
terminalSubagentRunEndedAt: {},
916+
runtimeAuthoritative: true,
917+
});
918+
919+
expectMaintenanceCounts(await runTaskRegistryMaintenance(), { reconciled: 0 });
920+
expectTaskStatus(currentTasks, task.taskId, "running");
921+
});
922+
923+
it("marks a killed subagent task lost in non-authoritative (CLI maintenance) context", async () => {
924+
const runId = "run-nonauth-zombie-90444";
925+
const task = makeStaleTask({
926+
runtime: "subagent",
927+
runId,
928+
childSessionKey: "agent:main:subagent:nonauth-90444",
929+
});
930+
931+
// CLI maintenance reads endedAt from the SQLite-backed snapshot rather than
932+
// the process-local in-memory map, so it can finalize kills the gateway
933+
// persisted to SQLite even when isRuntimeAuthoritative() is false.
934+
const { currentTasks } = createTaskRegistryMaintenanceHarness({
935+
tasks: [task],
936+
sessionStore: {
937+
"agent:main:subagent:nonauth-90444": {
938+
sessionId: "sess-nonauth-90444",
939+
updatedAt: Date.now(),
940+
},
941+
},
942+
terminalSubagentRunEndedAt: { [runId]: Date.now() - 5000 },
943+
runtimeAuthoritative: false,
944+
});
945+
946+
expectMaintenanceCounts(await runTaskRegistryMaintenance(), { reconciled: 1 });
947+
expectTaskStatus(currentTasks, task.taskId, "lost");
948+
});
949+
950+
it("marks a freshly killed subagent task lost before the lost-grace window expires", async () => {
951+
// Regression for the timing gap ClawSweeper caught: the terminal-run check
952+
// must fire in shouldMarkLost before hasLostGraceExpired so a task killed
953+
// seconds ago is finalized on the next sweep, not after 5+ minutes.
954+
const now = Date.now();
955+
const runId = "run-fresh-killed-90444";
956+
const task = makeStaleTask({
957+
runtime: "subagent",
958+
runId,
959+
childSessionKey: "agent:main:subagent:fresh-90444",
960+
// Fresh timestamps: task was created and killed 30 s ago, well within
961+
// the 5-minute TASK_RECONCILE_GRACE_MS window.
962+
createdAt: now - 30_000,
963+
startedAt: now - 30_000,
964+
lastEventAt: now - 30_000,
965+
});
966+
967+
const { currentTasks } = createTaskRegistryMaintenanceHarness({
968+
tasks: [task],
969+
sessionStore: {
970+
"agent:main:subagent:fresh-90444": {
971+
sessionId: "sess-fresh-90444",
972+
updatedAt: now,
973+
},
974+
},
975+
terminalSubagentRunEndedAt: { [runId]: now - 5_000 },
976+
runtimeAuthoritative: true,
977+
});
978+
979+
expectMaintenanceCounts(await runTaskRegistryMaintenance(), { reconciled: 1 });
980+
expectTaskStatus(currentTasks, task.taskId, "lost");
981+
});
982+
983+
it("marks a same-run CLI peer task lost when the parent subagent run is terminal", async () => {
984+
// Regression for #90444: the issue reports both the parent runtime='subagent'
985+
// row and the child runtime='cli' row for the same run staying stuck. The
986+
// terminal-run fast path must cover both runtimes.
987+
const runId = "run-cli-peer-90444";
988+
const subagentTask = makeStaleTask({
989+
runtime: "subagent",
990+
runId,
991+
childSessionKey: "agent:main:subagent:peer-90444",
992+
});
993+
const cliPeerTask = makeStaleTask({
994+
runtime: "cli",
995+
sourceId: runId,
996+
childSessionKey: "agent:main:cli:peer-90444",
997+
});
998+
999+
const { currentTasks } = createTaskRegistryMaintenanceHarness({
1000+
tasks: [subagentTask, cliPeerTask],
1001+
sessionStore: {
1002+
"agent:main:subagent:peer-90444": {
1003+
sessionId: "sess-sub-peer-90444",
1004+
updatedAt: Date.now(),
1005+
},
1006+
"agent:main:cli:peer-90444": { sessionId: "sess-cli-peer-90444", updatedAt: Date.now() },
1007+
},
1008+
terminalSubagentRunEndedAt: { [runId]: Date.now() - 5000 },
1009+
runtimeAuthoritative: true,
1010+
});
1011+
1012+
expectMaintenanceCounts(await runTaskRegistryMaintenance(), { reconciled: 2 });
1013+
expectTaskStatus(currentTasks, subagentTask.taskId, "lost");
1014+
expectTaskStatus(currentTasks, cliPeerTask.taskId, "lost");
1015+
});
1016+
});

0 commit comments

Comments
 (0)