Skip to content

Commit 319bef7

Browse files
wangmiao0668000666wangmiao0668000666
committed
fix(tasks): thread task scope into lease lifecycle hooks (PR #95481 P1/P2)
1 parent dcd9129 commit 319bef7

5 files changed

Lines changed: 132 additions & 9 deletions

File tree

‎docs/concepts/task-completion.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ The completion-time resolver already has higher-precedence sources:
1919
2. The active main session bucket (most useful, but can be retargeted)
2020
3. The task's stored `delivery` config (can be partial or stale)
2121

22-
When all three lose the outbound target, the resolver falls back to the route lease keyed by `run_id`. The lease is keyed by `run_id` rather than `task_id` because a single task may produce multiple runs over time, and each run owns its own delivery origin.
22+
When all three lose the outbound target, the resolver falls back to the route lease. The lease is keyed by the `(run_id, runtime, scope_kind, owner_key, child_session_key)` tuple — the same scope facts the task registry uses to disambiguate shared-runId task records. The lease is keyed by that 5-tuple rather than `task_id` because a single task may produce multiple runs over time, and each run owns its own delivery origin within its scope.
2323

2424
## Lifecycle
2525

‎src/tasks/task-executor.test.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import {
4242
setTaskRegistryControlRuntimeForTests,
4343
} from "./task-registry.js";
4444
import type { TaskRecord } from "./task-registry.types.js";
45+
import { getActiveTaskRouteLease } from "./task-route-lease.js";
4546

4647
const ORIGINAL_ENV = captureEnv(["OPENCLAW_STATE_DIR"]);
4748

@@ -820,4 +821,89 @@ describe("task-executor", () => {
820821
expect(getTaskById(victim.taskId)?.status).toBe("running");
821822
});
822823
});
824+
825+
it("scopes the auto-acquired task-route lease to each task's runtime/session scope (PR #95481 P1 review)", async () => {
826+
await withTaskExecutorStateDir(async () => {
827+
const acpTask = createRunningTaskRun({
828+
runtime: "acp",
829+
ownerKey: "agent:victim:main",
830+
scopeKind: "session",
831+
childSessionKey: "agent:victim:acp:child",
832+
runId: "run-shared-executor-lease",
833+
task: "ACP task with its own requester origin",
834+
requesterOrigin: { channel: "telegram", to: "victim", accountId: "default" },
835+
deliveryStatus: "pending",
836+
});
837+
const cliTask = createRunningTaskRun({
838+
runtime: "cli",
839+
ownerKey: "agent:attacker:main",
840+
scopeKind: "session",
841+
childSessionKey: "agent:attacker:main",
842+
runId: "run-shared-executor-lease",
843+
task: "CLI task sharing the same raw runId",
844+
requesterOrigin: { channel: "discord", to: "attacker", accountId: "default" },
845+
deliveryStatus: "pending",
846+
});
847+
848+
// Each task's auto-acquired lease must be keyed by its own scope,
849+
// not the default scope, so the two do not overwrite each other.
850+
const acpLease = getActiveTaskRouteLease("run-shared-executor-lease", {
851+
scope: {
852+
runtime: "acp",
853+
scopeKind: "session",
854+
ownerKey: "agent:victim:main",
855+
childSessionKey: "agent:victim:acp:child",
856+
},
857+
});
858+
const cliLease = getActiveTaskRouteLease("run-shared-executor-lease", {
859+
scope: {
860+
runtime: "cli",
861+
scopeKind: "session",
862+
ownerKey: "agent:attacker:main",
863+
childSessionKey: "agent:attacker:main",
864+
},
865+
});
866+
expect(acpLease).toBeDefined();
867+
expect(cliLease).toBeDefined();
868+
expect(acpLease?.requesterOrigin?.channel).toBe("telegram");
869+
expect(cliLease?.requesterOrigin?.channel).toBe("discord");
870+
871+
// The default-scope lookup must not see either of the scoped rows,
872+
// proving both auto-acquires used the task's real scope.
873+
expect(getActiveTaskRouteLease("run-shared-executor-lease")).toBeUndefined();
874+
875+
// Settling the ACP task's delivery status must retire only the
876+
// ACP-scoped lease, leaving the CLI-scoped lease intact.
877+
setDetachedTaskDeliveryStatusByRunId({
878+
runId: "run-shared-executor-lease",
879+
runtime: "acp",
880+
sessionKey: "agent:victim:acp:child",
881+
deliveryStatus: "delivered",
882+
});
883+
884+
expect(
885+
getActiveTaskRouteLease("run-shared-executor-lease", {
886+
scope: {
887+
runtime: "acp",
888+
scopeKind: "session",
889+
ownerKey: "agent:victim:main",
890+
childSessionKey: "agent:victim:acp:child",
891+
},
892+
}),
893+
).toBeUndefined();
894+
expect(
895+
getActiveTaskRouteLease("run-shared-executor-lease", {
896+
scope: {
897+
runtime: "cli",
898+
scopeKind: "session",
899+
ownerKey: "agent:attacker:main",
900+
childSessionKey: "agent:attacker:main",
901+
},
902+
}),
903+
).toBeDefined();
904+
905+
expect(acpTask.runId).toBe("run-shared-executor-lease");
906+
expect(cliTask.runId).toBe("run-shared-executor-lease");
907+
});
908+
});
823909
});

‎src/tasks/task-executor.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,12 @@ export function createRunningTaskRun(params: RunningTaskRunCreateParams): TaskRe
135135
runId: task.runId,
136136
taskId: task.taskId,
137137
requesterOrigin: params.requesterOrigin,
138+
scope: {
139+
runtime: task.runtime,
140+
scopeKind: task.scopeKind,
141+
ownerKey: task.ownerKey,
142+
childSessionKey: task.childSessionKey,
143+
},
138144
});
139145
}
140146
return ensureSingleTaskFlow({
@@ -232,10 +238,24 @@ export function setDetachedTaskDeliveryStatusByRunId(params: {
232238
const updated = setTaskRunDeliveryStatusByRunId(params);
233239
// Settle the task-route lease on terminal delivery so it can be GC'd
234240
// instead of waiting for TTL expiry. Idempotent — re-runs on already-
235-
// settled leases are no-ops.
241+
// settled leases are no-ops. Settle every task row that matched the
242+
// runId scope, because the lease key now mirrors the task-registry
243+
// scope tuple and each task row owns its own lease.
236244
const retirement = mapDeliveryStatusToLeaseRetirement(params.deliveryStatus);
237245
if (retirement) {
238-
settleTaskRouteLease(params.runId, retirement);
246+
for (const task of updated) {
247+
if (!task.runId) {
248+
continue;
249+
}
250+
settleTaskRouteLease(task.runId, retirement, {
251+
scope: {
252+
runtime: task.runtime,
253+
scopeKind: task.scopeKind,
254+
ownerKey: task.ownerKey,
255+
childSessionKey: task.childSessionKey,
256+
},
257+
});
258+
}
239259
}
240260
return updated;
241261
}

‎src/tasks/task-registry.store.sqlite.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,8 +292,7 @@ function deleteTaskRowsWithDeliveryState(db: DatabaseSync, taskId: string): void
292292
// task. Delete those leases atomically with task_runs and
293293
// task_delivery_state so settled/expired/orphaned rows do not
294294
// accumulate in the shared state DB. Lease module owns the lease
295-
// SQL; we own the transaction. See PR #95352 ClawSweeper review
296-
// (P2 retention, confidence 0.9).
295+
// SQL; we own the transaction.
297296
deleteTaskRouteLeasesByTaskIdInDb(db, taskId);
298297
}
299298

‎src/tasks/task-registry.store.test.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,13 +1138,31 @@ describe("task-registry store runtime", () => {
11381138
notifyPolicy: "done_only",
11391139
});
11401140
expect(task).not.toBeNull();
1141-
// Auto-acquire wrote a lease row keyed by runId. This proves the
1142-
// pre-delete precondition.
1143-
expect(getActiveTaskRouteLease("run-cascade-cron")).toBeDefined();
1141+
// Auto-acquire wrote a lease row keyed by (runId, runtime, scopeKind,
1142+
// ownerKey, childSessionKey). This proves the pre-delete precondition.
1143+
expect(
1144+
getActiveTaskRouteLease("run-cascade-cron", {
1145+
scope: {
1146+
runtime: "cron",
1147+
scopeKind: "session",
1148+
ownerKey: "agent:main:main",
1149+
childSessionKey: "",
1150+
},
1151+
}),
1152+
).toBeDefined();
11441153

11451154
// The composite store delete also drops the lease row atomically.
11461155
expect(deleteTaskRecordById(task!.taskId)).toBe(true);
1147-
expect(getActiveTaskRouteLease("run-cascade-cron")).toBeUndefined();
1156+
expect(
1157+
getActiveTaskRouteLease("run-cascade-cron", {
1158+
scope: {
1159+
runtime: "cron",
1160+
scopeKind: "session",
1161+
ownerKey: "agent:main:main",
1162+
childSessionKey: "",
1163+
},
1164+
}),
1165+
).toBeUndefined();
11481166
},
11491167
);
11501168
});

0 commit comments

Comments
 (0)