Skip to content

Commit 8d0e5c7

Browse files
Rate-limit concurrent mutating lane-overlap warnings (#891)
* 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. * Rate-limit mutating cwd overlap warnings and skip read-only peers Shared-cwd concurrent-lane-overlap noise was one record per peer with no filter for explore/plan/review/test directors. Emit at most one conflict when a cwd wave first has two live mutating writers, suppress declared read-only peers, and clear the wave flag once no live writer remains. No age TTL or map sweep. * Cover queued writers in concurrent-lane-overlap tests
1 parent 802c4ac commit 8d0e5c7

3 files changed

Lines changed: 330 additions & 14 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 mutating dispatch that lands on the same cwd as a live mutating peer (`pending_init`/`running`, and not a declared read-only `modelRole` of `explore`/`plan`/`review`/`test`) records at most one `concurrent-lane-overlap` entry per cwd wave in `intervention-log.ts` (class `conflict`). The wave flag clears when no live mutating writer remains for that cwd. 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: 253 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 = [
@@ -706,10 +745,224 @@ describe("spawn_agent same-cwd concurrency", () => {
706745
expect(log).toContain("/repo");
707746
expect(log).toContain("build one");
708747
expect(log).toContain("build two");
748+
expect(
749+
log
750+
.trim()
751+
.split("\n")
752+
.filter((line) => line.includes("concurrent-lane-overlap")),
753+
).toHaveLength(1);
709754

710755
defined(gates[0]).resolve({ report: "one done" });
711756
defined(gates[1]).resolve({ report: "two done" });
712757
});
758+
759+
test("three concurrent mutating shared-cwd lanes log one concurrent-lane-overlap", async () => {
760+
const dir = await mkdtemp(join(tmpdir(), "fleet-overlap-wave-"));
761+
const gates = [
762+
deferred<RunSubAgentResult>(),
763+
deferred<RunSubAgentResult>(),
764+
deferred<RunSubAgentResult>(),
765+
];
766+
let callIndex = 0;
767+
const deps = makeDeps(async () => defined(gates[callIndex++]).promise, {
768+
cwd: "/repo",
769+
});
770+
deps.getWorkdirBase = () => dir;
771+
const spawn = createSpawnAgentTool(deps);
772+
773+
for (const label of ["one", "two", "three"] as const) {
774+
await callTool(spawn, {
775+
description: `build ${label}`,
776+
prompt: `implement thing ${label}`,
777+
intent: "implement",
778+
success_criteria: [`thing ${label} ships`],
779+
});
780+
}
781+
782+
const path = join(dir, INTERVENTION_FILE);
783+
let log = "";
784+
for (let i = 0; i < 50; i++) {
785+
try {
786+
log = await readFile(path, "utf8");
787+
if (log.includes("concurrent-lane-overlap")) break;
788+
} catch {
789+
// append is fire-and-forget
790+
}
791+
await new Promise((resolve) => setTimeout(resolve, 20));
792+
}
793+
expect(
794+
log
795+
.trim()
796+
.split("\n")
797+
.filter((line) => line.includes("concurrent-lane-overlap")),
798+
).toHaveLength(1);
799+
800+
for (const gate of gates) defined(gate).resolve({ report: "done" });
801+
});
802+
803+
test("shared-cwd explore then implement does not log concurrent-lane-overlap", async () => {
804+
const dir = await mkdtemp(join(tmpdir(), "fleet-overlap-readonly-"));
805+
const gates = [
806+
deferred<RunSubAgentResult>(),
807+
deferred<RunSubAgentResult>(),
808+
];
809+
let callIndex = 0;
810+
const deps = makeDeps(async () => defined(gates[callIndex++]).promise, {
811+
cwd: "/repo",
812+
});
813+
deps.getWorkdirBase = () => dir;
814+
const spawn = createSpawnAgentTool(deps);
815+
816+
await callTool(spawn, {
817+
description: "look around",
818+
prompt: "map the tree",
819+
intent: "explore",
820+
});
821+
await callTool(spawn, {
822+
description: "build one",
823+
prompt: "implement thing one",
824+
intent: "implement",
825+
success_criteria: ["thing one ships"],
826+
});
827+
await new Promise((resolve) => setTimeout(resolve, 50));
828+
829+
await expect(
830+
readFile(join(dir, INTERVENTION_FILE), "utf8"),
831+
).rejects.toThrow();
832+
833+
defined(gates[0]).resolve({ report: "mapped" });
834+
defined(gates[1]).resolve({ report: "one done" });
835+
});
836+
837+
test("a later mutating wave can warn again after the prior wave settles", async () => {
838+
const dir = await mkdtemp(join(tmpdir(), "fleet-overlap-reset-"));
839+
const gates = [
840+
deferred<RunSubAgentResult>(),
841+
deferred<RunSubAgentResult>(),
842+
deferred<RunSubAgentResult>(),
843+
deferred<RunSubAgentResult>(),
844+
];
845+
let callIndex = 0;
846+
const deps = makeDeps(async () => defined(gates[callIndex++]).promise, {
847+
cwd: "/repo",
848+
});
849+
deps.getWorkdirBase = () => dir;
850+
const spawn = createSpawnAgentTool(deps);
851+
852+
await callTool(spawn, {
853+
description: "wave1 a",
854+
prompt: "implement a",
855+
intent: "implement",
856+
success_criteria: ["a ships"],
857+
});
858+
await callTool(spawn, {
859+
description: "wave1 b",
860+
prompt: "implement b",
861+
intent: "implement",
862+
success_criteria: ["b ships"],
863+
});
864+
865+
const path = join(dir, INTERVENTION_FILE);
866+
let log = "";
867+
for (let i = 0; i < 50; i++) {
868+
try {
869+
log = await readFile(path, "utf8");
870+
if (log.includes("concurrent-lane-overlap")) break;
871+
} catch {
872+
// append is fire-and-forget
873+
}
874+
await new Promise((resolve) => setTimeout(resolve, 20));
875+
}
876+
expect(
877+
log
878+
.trim()
879+
.split("\n")
880+
.filter((line) => line.includes("concurrent-lane-overlap")),
881+
).toHaveLength(1);
882+
883+
defined(gates[0]).resolve({ report: "a done" });
884+
defined(gates[1]).resolve({ report: "b done" });
885+
await new Promise((resolve) => setTimeout(resolve, 30));
886+
887+
await callTool(spawn, {
888+
description: "wave2 a",
889+
prompt: "implement c",
890+
intent: "implement",
891+
success_criteria: ["c ships"],
892+
});
893+
await callTool(spawn, {
894+
description: "wave2 b",
895+
prompt: "implement d",
896+
intent: "implement",
897+
success_criteria: ["d ships"],
898+
});
899+
900+
for (let i = 0; i < 50; i++) {
901+
try {
902+
log = await readFile(path, "utf8");
903+
if (
904+
log
905+
.trim()
906+
.split("\n")
907+
.filter((line) => line.includes("concurrent-lane-overlap"))
908+
.length >= 2
909+
) {
910+
break;
911+
}
912+
} catch {
913+
// append is fire-and-forget
914+
}
915+
await new Promise((resolve) => setTimeout(resolve, 20));
916+
}
917+
expect(
918+
log
919+
.trim()
920+
.split("\n")
921+
.filter((line) => line.includes("concurrent-lane-overlap")),
922+
).toHaveLength(2);
923+
expect(log).toContain("wave2");
924+
925+
defined(gates[2]).resolve({ report: "c done" });
926+
defined(gates[3]).resolve({ report: "d done" });
927+
});
928+
929+
test("a queued mutating peer does not log concurrent-lane-overlap", async () => {
930+
const dir = await mkdtemp(join(tmpdir(), "fleet-overlap-queued-"));
931+
const gates = [
932+
deferred<RunSubAgentResult>(),
933+
deferred<RunSubAgentResult>(),
934+
];
935+
let callIndex = 0;
936+
const deps = makeDeps(async () => defined(gates[callIndex++]).promise, {
937+
cwd: "/repo",
938+
});
939+
deps.getWorkdirBase = () => dir;
940+
deps.admission = createAdmissionQueue({ capacity: 1 });
941+
const spawn = createSpawnAgentTool(deps);
942+
943+
const first = await callTool(spawn, {
944+
description: "holder",
945+
prompt: "implement holder",
946+
intent: "implement",
947+
success_criteria: ["holder ships"],
948+
});
949+
const queued = await callTool(spawn, {
950+
description: "queued writer",
951+
prompt: "implement queued",
952+
intent: "implement",
953+
success_criteria: ["queued ships"],
954+
});
955+
expect(first.status).toBe("running");
956+
expect(queued.status).toBe("queued");
957+
await new Promise((resolve) => setTimeout(resolve, 50));
958+
959+
await expect(
960+
readFile(join(dir, INTERVENTION_FILE), "utf8"),
961+
).rejects.toThrow();
962+
963+
defined(gates[0]).resolve({ report: "holder done" });
964+
defined(gates[1]).resolve({ report: "queued done" });
965+
});
713966
});
714967

715968
describe("wait mailbox session tombstone and pin", () => {

0 commit comments

Comments
 (0)