Skip to content

Commit 68a55f3

Browse files
committed
Gate lane-overlap warnings on live session-store status
Cancel can stamp finishedAt before the run promise settles and reaches finally, so an activeLanes map entry alone was enough to emit concurrent-lane-overlap against a worker that was already terminal. Overlap checks now trust pending_init/running from the session store, prune terminal or absent map entries opportunistically, and keep the finally delete.
1 parent 440b9cb commit 68a55f3

3 files changed

Lines changed: 53 additions & 5 deletions

File tree

docs/IMPLEMENTATION.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ Sixteen packages under `src/agent/directors/<id>/` register in `DIRECTOR_REGISTR
167167

168168
**Codex tool proxies.** When the active provider is Codex (`isCodexProviderName`), `createAgentToolset` and `runSubAgent` mount `apply_patch`, `shell`, and `update_plan` stringTools from `createCodexToolProxies`, all forwarding through the same posix `ToolRunner` seam (`runTool`) so permission plugins still apply. `apply_patch` parses the Codex envelope and forwards each op (`write_file` / `delete_file` / `read_file`). `shell` — the native Codex name is `shell`, not `exec_command` — normalizes Codex's `command` (string or `["bash","-lc",script]`-style argv array), `workdir`, and `timeout_ms` onto `run_shell`'s `{command, cwd?, timeout?}` and is gated by `allowShellFromCapabilities` (mirrors `allowDeleteFromCapabilities` against `run_shell`). `update_plan` maps Codex's `plan: [{step, status}]` onto `manage_tasks(action: "create")`; `pending`/`in_progress`/`completed` map to `todo`/`doing`/`done` — `manage_tasks`'s `cancelled` status has no Codex equivalent and is never produced by this proxy. Primary strips `apply_patch` after mount (Corbits DIY stays on `write_file` / `edit_file` / `delete_file`); `shell` and `update_plan` stay on primary (same classification as `run_shell` / `manage_tasks`). Build and docs worker allowlists (`BUILD_TOOLS` / `DOCS_TOOLS`) include `apply_patch` so Codex workers keep the proxy after the capability filter. `CORE_TOOL_NAMES` does not list it.
169169

170-
6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `agent-fleet.ts` tracks each running dispatch by cwd; a new dispatch that lands on the same cwd as a still-running lane records a `concurrent-lane-overlap` entry in `intervention-log.ts` (class `conflict`). This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files.
170+
6. There is no static write-path declaration on packages or profiles (CL-6952 removed it — no shipped director ever set one). Instead, `agent-fleet.ts` tracks each running dispatch by cwd; a new dispatch that lands on the same cwd as a lane whose session-store lifecycle is still `pending_init` or `running` records a `concurrent-lane-overlap` entry in `intervention-log.ts` (class `conflict`). Terminal-but-unsettled lanes (for example cancelled with `finishedAt` set while the run promise has not reached `finally`) are pruned from the map and do not warn. This is advisory only — it never blocks the spawn, since cwd overlap does not prove the two lanes touch the same files.
171171
7. Spawn effort: pin > package `modelRole` default (`defaultEffortForDirector`; intern=low; plan/review/orchestrator=high; implement/explore/docs/test=medium) > orchestrator/worker binary > parent inheritance. Optional skills are listed in the identity header for awareness; workers do not mount `use_skill` (guidance is baked into package system prompts). Primary mounts `use_skill` for its own skill list.
172172

173173
Intent defaults: `intent=implement` → director `builder`; `explore``explorer`; `plan``counsel`; `review``critic`; general → error. Spawn: skywalker full fleet; greybeard intern/explorer/critic only; all other directors mount no fleet tools. Live `<env>` injects cwd, platform, arch, runtime, date, and git status on every chat and worker prompt.

src/subagent/agent-fleet.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,45 @@ describe("spawn_agent same-cwd concurrency", () => {
664664
defined(gates[1]).resolve({ report: "two done" });
665665
});
666666

667+
test("a terminal but unsettled shared-cwd lane does not conflict with a later spawn", async () => {
668+
const dir = await mkdtemp(join(tmpdir(), "fleet-overlap-terminal-"));
669+
const gates = [
670+
deferred<RunSubAgentResult>(),
671+
deferred<RunSubAgentResult>(),
672+
];
673+
let callIndex = 0;
674+
const deps = makeDeps(async () => defined(gates[callIndex++]).promise, {
675+
cwd: "/repo",
676+
});
677+
deps.getWorkdirBase = () => dir;
678+
const spawn = createSpawnAgentTool(deps);
679+
680+
const first = await callTool(spawn, {
681+
description: "build one",
682+
prompt: "implement thing one",
683+
intent: "implement",
684+
success_criteria: ["thing one ships"],
685+
});
686+
const firstId = first.agent_id as string;
687+
expect(deps.sessions.cancel(firstId)).toBe(true);
688+
expect(deps.sessions.get(firstId)?.finishedAt).toBeNumber();
689+
690+
await callTool(spawn, {
691+
description: "build two",
692+
prompt: "implement thing two",
693+
intent: "implement",
694+
success_criteria: ["thing two ships"],
695+
});
696+
await new Promise((resolve) => setTimeout(resolve, 50));
697+
698+
await expect(
699+
readFile(join(dir, INTERVENTION_FILE), "utf8"),
700+
).rejects.toThrow();
701+
702+
defined(gates[0]).resolve({ report: "one cancelled" });
703+
defined(gates[1]).resolve({ report: "two done" });
704+
});
705+
667706
test("two concurrent shared-cwd spawn_agent lanes log concurrent-lane-overlap", async () => {
668707
const dir = await mkdtemp(join(tmpdir(), "fleet-overlap-"));
669708
const gates = [

src/subagent/agent-fleet.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -868,10 +868,10 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
868868
// will run in — worktree-isolated lanes always get a fresh, disjoint path
869869
// here, so this can only ever fire in the shared-cwd fallback, which is
870870
// exactly where two lanes really can stomp each other's writes.
871-
// Keyed by call.id so a completed lane (removed when the worker settles)
872-
// is never mistaken for one still running: sequential dispatches to the
873-
// same cwd are always clean. Tracking lasts the worker lifetime, not the
874-
// immediate spawn_agent return.
871+
// Keyed by call.id for finally cleanup. The session store is authoritative
872+
// for liveness: cancel (and other terminals) can stamp finishedAt before the
873+
// run promise settles and reaches finally, so a map entry alone is not proof
874+
// the lane is still working.
875875
const activeLanes = new Map<string, { description: string; cwd: string }>();
876876
let conflictLog: InterventionSink | null = null;
877877
const recordConflict = (event: Parameters<InterventionSink>[0]): void => {
@@ -1244,6 +1244,15 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
12441244
// files, only that they could.
12451245
const laneCwd = worktreeCwd ?? deps.cwd;
12461246
for (const [otherId, other] of activeLanes) {
1247+
const otherSession = deps.sessions.get(otherId);
1248+
if (
1249+
otherSession === undefined ||
1250+
(otherSession.lifecycle.state !== "pending_init" &&
1251+
otherSession.lifecycle.state !== "running")
1252+
) {
1253+
activeLanes.delete(otherId);
1254+
continue;
1255+
}
12471256
if (other.cwd !== laneCwd) continue;
12481257
recordConflict({
12491258
id: "concurrent-lane-overlap",

0 commit comments

Comments
 (0)