Skip to content

Commit 9f32467

Browse files
committed
Remove static writePaths lock; detect concurrent lane overlap at spawn
Deletes DirectorPackage.writePaths / AgentProfile.writePaths and the gate.ts enforcement that read them (write-path-policy.ts and its test are now dead and removed). No shipped director ever set a non-empty writePaths, so this was already unenforced-in-practice authz machinery that read as active. Replaces it with a non-blocking conflict detector in the task tool: each running dispatch is tracked by call id and resolved cwd; a new dispatch that shares a still-running lane's cwd is recorded (not blocked) as a concurrent-lane-overlap intervention. A completed lane is removed before the next dispatch starts, so sequential work against the same cwd never triggers it. Judgement calls: - task() has no field for a caller to declare which paths a dispatch will touch, so cwd is the only intended-scope signal honestly available at spawn. Worktree-isolated lanes get distinct cwds by construction and can never collide here; this only fires in the shared-cwd fallback, which is the one case where two lanes can really overwrite each other. - Warn/record, not block: cwd equality does not prove two lanes touch the same files, only that they could, so refusing the spawn would invent precision the signal does not have. - Recorded in intervention-log.ts (new "conflict" class) rather than a separate mechanism, alongside the existing parent-side refusal/outcome records it already keeps for this task tool instance.
1 parent d1594a2 commit 9f32467

17 files changed

Lines changed: 49 additions & 358 deletions

‎src/agent/directors/brand-reviewer/package.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ describe("brandReviewerPackage", () => {
2121
expect(brandReviewerPackage.spawn.maySpawn).toBe(false);
2222
});
2323

24-
test("tools.allow includes write tools; writePaths is omitted", () => {
24+
test("tools.allow includes write tools", () => {
2525
const allow = brandReviewerPackage.tools?.allow ?? [];
2626
expect(allow).toContain("write_file");
2727
expect(allow).toContain("edit_file");
28-
expect(brandReviewerPackage.writePaths).toBeUndefined();
2928
});
3029

3130
test("systemPrompt mentions DESIGN.md", () => {

‎src/agent/directors/bruckheimer/package.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ describe("bruckheimerPackage", () => {
2222
expect(bruckheimerPackage.spawn.maySpawn).toBe(false);
2323
});
2424

25-
test("tools.allow includes write tools; writePaths is omitted", () => {
25+
test("tools.allow includes write tools", () => {
2626
const allow = bruckheimerPackage.tools?.allow ?? [];
2727
expect(allow).toContain("write_file");
2828
expect(allow).toContain("edit_file");
29-
expect(bruckheimerPackage.writePaths).toBeUndefined();
3029
});
3130

3231
test("report requires envelope sections", () => {

‎src/agent/directors/registry.test.ts‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ describe("director registry", () => {
107107
expect(grey.maxTurns).toBe(DIRECTOR_REGISTRY.greybeard.nudge?.maxTurns);
108108

109109
const shakespeare = packageToProfile(DIRECTOR_REGISTRY.shakespeare);
110-
expect(shakespeare.writePaths).toBeUndefined();
111110
expect(shakespeare.capabilities?.mode).toBe("allow");
112111
expect(shakespeare.capabilities?.tools).toContain("write_file");
113112
});
@@ -147,13 +146,6 @@ describe("director registry", () => {
147146
}
148147
});
149148

150-
test("no shipped director in DIRECTOR_IDS has a non-empty writePaths", () => {
151-
for (const id of DIRECTOR_IDS) {
152-
const paths = DIRECTOR_REGISTRY[id].writePaths;
153-
expect(paths === undefined || paths.length === 0).toBe(true);
154-
}
155-
});
156-
157149
test("build mounts product writes; intern is shell-only; other leaves do not spawn", () => {
158150
expect(DIRECTOR_REGISTRY.build.tools?.allow).toEqual(
159151
expect.arrayContaining(["write_file", "edit_file", "delete_file", "apply_patch"]),
@@ -181,15 +173,6 @@ describe("director registry", () => {
181173
expect(s.spawn.allowlist).toHaveLength(15);
182174
});
183175

184-
test("writePaths guards: a director with non-empty writePaths never allows run_shell", () => {
185-
for (const id of DIRECTOR_IDS) {
186-
const pkg = DIRECTOR_REGISTRY[id];
187-
if (!pkg.writePaths || pkg.writePaths.length === 0) continue;
188-
const allow = pkg.tools?.allow ?? [];
189-
expect(allow).not.toContain("run_shell");
190-
}
191-
});
192-
193176
test("every director profile declares matching agent id in system prompt", () => {
194177
for (const id of DIRECTOR_IDS) {
195178
const profile = packageToProfile(DIRECTOR_REGISTRY[id]);

‎src/agent/directors/registry.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,6 @@ export function packageToProfile(pkg: DirectorPackage): AgentProfile {
127127
orchestrator: pkg.spawn.maySpawn,
128128
...(pkg.nudge?.maxTurns !== undefined ? { maxTurns: pkg.nudge.maxTurns } : {}),
129129
...(capabilities !== undefined ? { capabilities } : {}),
130-
...(pkg.writePaths !== undefined && pkg.writePaths.length > 0
131-
? { writePaths: [...pkg.writePaths] }
132-
: {}),
133130
};
134131
}
135132

‎src/agent/directors/shakespeare/package.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,10 @@ describe("shakespearePackage", () => {
3232
expect(shakespearePackage.spawn.maySpawn).toBe(false);
3333
});
3434

35-
test("tools.allow includes write tools; writePaths is omitted", () => {
35+
test("tools.allow includes write tools", () => {
3636
const allow = shakespearePackage.tools?.allow ?? [];
3737
expect(allow).toContain("write_file");
3838
expect(allow).toContain("edit_file");
39-
expect(shakespearePackage.writePaths).toBeUndefined();
4039
});
4140

4241
test("report.requiredSections includes Summary, Findings, Blockers, Paths", () => {

‎src/agent/directors/tool-sets.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,8 @@ export const BUILD_TOOLS = [
3434

3535
/**
3636
* Docs leaves: read/search/lsp/web + file writes — no run_shell, no delete_file.
37-
* Envelope policy, not a writePaths lock: docs leaves omit shell so they cannot
38-
* mutate via the terminal. Optional package writePaths, when a profile sets it,
39-
* is still enforced by the permission gate on path-keyed write tools.
37+
* Envelope policy only: docs leaves omit shell so they cannot mutate via the
38+
* terminal. There is no separate path-level lock on top of the tool envelope.
4039
*
4140
* Composed from READ_TOOLS minus run_shell so it tracks the read surface
4241
* automatically; only the write tools are added explicitly. `apply_patch` is

‎src/agent/directors/types.ts‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,6 @@ export interface DirectorPackage {
6969
/** Optional skills the worker may load dynamically (ordered). */
7070
readonly optionalSkills?: readonly string[];
7171
readonly tools?: ToolEnvelope;
72-
/**
73-
* Authz write-path allowlist for write_file/edit_file/delete_file.
74-
* Enforced by the permission gate (not prompt policy). A bare filename (no
75-
* slash) matches only at the workspace root; a glob matches the resolved
76-
* workspace-relative path; anything outside the worker cwd is denied. yolo
77-
* mode bypasses this gate. Omitted = no path lock (tool allow/deny alone
78-
* decides whether writes exist).
79-
*/
80-
readonly writePaths?: readonly string[];
8172
readonly spawn: SpawnRights;
8273
readonly nudge?: NudgePolicy;
8374
readonly report: ReportContract;

‎src/agent/profile-types.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,6 @@ export interface AgentProfile {
5555
inference?: InferenceSpec;
5656
// Optional tool restriction. Controls which tools the sub-agent can call.
5757
capabilities?: CapabilityFilter;
58-
/**
59-
* Authz write-path allowlist for write_file/edit_file/delete_file (director
60-
* packages). Enforced by the permission gate, not prompt policy.
61-
*/
62-
writePaths?: readonly string[];
6358
// Appended to the sub-agent's base system prompt to specialize its behavior.
6459
systemPromptRole?: string;
6560
// Relative path to a markdown file whose content is loaded as systemPromptRole

‎src/permission/gate.test.ts‎

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -163,73 +163,6 @@ describe("grant coverage rebinds relative paths to the request process cwd", ()
163163
});
164164
});
165165

166-
describe("director writePaths authz on evaluate", () => {
167-
const cwd = mkdtempSync(join(tmpdir(), "gate-writepaths-"));
168-
169-
test("denies write_file outside allowlist under ALS identity", async () => {
170-
const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js");
171-
const gate = createPermissionGate({
172-
approvals: [{ tool: "write_file", pattern: "*" }],
173-
interactive: false,
174-
skipPermissions: false,
175-
cwd,
176-
});
177-
const verdict = await runWithSubAgentIdentity(
178-
{ description: "shakespeare", cwd, writePaths: ["PRODUCT.md"] },
179-
() =>
180-
gate.evaluate({
181-
id: "w1",
182-
name: "write_file",
183-
arguments: { path: "src/hack.ts", content: "nope" },
184-
}),
185-
);
186-
expect(verdict.allowed).toBe(false);
187-
if (!verdict.allowed) {
188-
expect(verdict.reason).toMatch(/authz allowlist/i);
189-
}
190-
});
191-
192-
test("allows write_file matching bare basename allowlist", async () => {
193-
const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js");
194-
const gate = createPermissionGate({
195-
approvals: [{ tool: "write_file", pattern: "*" }],
196-
interactive: false,
197-
skipPermissions: false,
198-
cwd,
199-
});
200-
const verdict = await runWithSubAgentIdentity(
201-
{ description: "shakespeare", cwd, writePaths: ["PRODUCT.md"] },
202-
() =>
203-
gate.evaluate({
204-
id: "w2",
205-
name: "write_file",
206-
arguments: { path: "PRODUCT.md", content: "ok" },
207-
}),
208-
);
209-
expect(verdict.allowed).toBe(true);
210-
});
211-
212-
test("yolo (skipPermissions) bypasses writePaths", async () => {
213-
const { runWithSubAgentIdentity } = await import("../subagent/identity-context.js");
214-
const gate = createPermissionGate({
215-
approvals: [],
216-
interactive: false,
217-
skipPermissions: true,
218-
cwd,
219-
});
220-
const verdict = await runWithSubAgentIdentity(
221-
{ description: "shakespeare", cwd, writePaths: ["PRODUCT.md"] },
222-
() =>
223-
gate.evaluate({
224-
id: "w3",
225-
name: "write_file",
226-
arguments: { path: "src/hack.ts", content: "yolo" },
227-
}),
228-
);
229-
expect(verdict.allowed).toBe(true);
230-
});
231-
});
232-
233166
// CL-5638: an Always-allow grant minted for `git worktree *` must cover a later
234167
// worktree command whose destination is a sibling directory the operator has
235168
// already implicitly approved under that pattern, without a second prompt.

‎src/permission/gate.ts‎

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,7 @@ import { splitChainedCommand, isShellCommentOnly, stripCommentLines } from "./co
2626
import { createPathRestriction } from "./path-restriction.js";
2727
import { createWorktreeRootsProvider, type RootsProvider } from "./worktree-roots.js";
2828
import { getSubAgentIdentity } from "../subagent/identity-context.js";
29-
import { matchesWritePathAllowlist, writePathDeniedReason } from "./write-path-policy.js";
30-
import {
31-
isProductMutationTool,
32-
productMutationPaths,
33-
PRODUCT_MUTATION_TOOLS,
34-
} from "../agent/product-mutation-tools.js";
29+
import { PRODUCT_MUTATION_TOOLS } from "../agent/product-mutation-tools.js";
3530

3631
import {
3732
createMcpToolPermissionRegistry,
@@ -432,31 +427,6 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
432427
const subAgentIdentity = getSubAgentIdentity();
433428
const effectiveCwd = subAgentIdentity?.cwd ?? resolvedCwd;
434429

435-
// Director write-path authz (not prompt policy). Leaves with writePaths only
436-
// mutate matching subjects. auto mode still enforces; yolo already returned.
437-
if (
438-
subAgentIdentity?.writePaths !== undefined &&
439-
subAgentIdentity.writePaths.length > 0 &&
440-
isProductMutationTool(call.name)
441-
) {
442-
const paths = productMutationPaths(call.name, call.arguments);
443-
// Fail-closed: no extractable subject is the same as an empty path deny.
444-
if (paths.length === 0) {
445-
return {
446-
allowed: false,
447-
reason: writePathDeniedReason("", subAgentIdentity.writePaths),
448-
};
449-
}
450-
for (const path of paths) {
451-
if (!matchesWritePathAllowlist(path, subAgentIdentity.writePaths, effectiveCwd)) {
452-
return {
453-
allowed: false,
454-
reason: writePathDeniedReason(path, subAgentIdentity.writePaths),
455-
};
456-
}
457-
}
458-
}
459-
460430
const isRestrictedHere = bindRestrictedToProcessCwd(isRestricted, effectiveCwd);
461431
// A call targeting a restricted path (outside the workspace, or a write
462432
// under the session state root) drops from allow to ask, so it never auto-allows on

0 commit comments

Comments
 (0)