Skip to content

Commit 03b6b4f

Browse files
committed
Mount write tools on every closed director
Compose PRODUCT_WRITE_TOOLS onto review, orchestrator, intern, and docs surfaces so lane discipline stays in prompts instead of tool stripping. Closes CL-7014
1 parent 02a3f85 commit 03b6b4f

25 files changed

Lines changed: 159 additions & 92 deletions

src/agent/codex-tool-proxies.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,8 @@ describe("update_plan proxy", () => {
597597
});
598598

599599
describe("allowDeleteFromCapabilities", () => {
600-
test("docs allowlist (no delete_file) → false; build → true", () => {
601-
expect(allowDeleteFromCapabilities({ mode: "allow", tools: DOCS_TOOLS })).toBe(false);
600+
test("docs allowlist (includes delete_file) → true; build → true", () => {
601+
expect(allowDeleteFromCapabilities({ mode: "allow", tools: DOCS_TOOLS })).toBe(true);
602602
expect(allowDeleteFromCapabilities({ mode: "allow", tools: BUILD_TOOLS })).toBe(true);
603603
expect(allowDeleteFromCapabilities(undefined)).toBe(true);
604604
expect(allowDeleteFromCapabilities({ mode: "exclude", tools: ["run_shell"] })).toBe(true);

src/agent/codex-tool-proxies.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ export interface CreateCodexToolProxiesOpts {
5555
runManageTasks: CodexRunManageTasks;
5656
/**
5757
* When false, Delete File and Update+Move refuse without calling `delete_file`.
58-
* Defaults to true (implement / unconstrained). Docs leaves pass false because
59-
* DOCS_TOOLS includes apply_patch but not delete_file.
58+
* Defaults to true (implement / unconstrained). Pass false when the
59+
* director allowlist omits delete_file (docs leaves mount it today).
6060
*/
6161
allowDelete?: boolean;
6262
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ describe("brandReviewerPackage", () => {
2525
const allow = brandReviewerPackage.tools?.allow ?? [];
2626
expect(allow).toContain("write_file");
2727
expect(allow).toContain("edit_file");
28+
expect(allow).toContain("delete_file");
2829
});
2930

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe("bruckheimerPackage", () => {
2626
const allow = bruckheimerPackage.tools?.allow ?? [];
2727
expect(allow).toContain("write_file");
2828
expect(allow).toContain("edit_file");
29+
expect(allow).toContain("delete_file");
2930
});
3031

3132
test("modelRole is docs", () => {

src/agent/directors/critique/package.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,14 @@ describe("critiquePackage", () => {
5454
expect(critiquePackage.spawn.maySpawn).toBe(false);
5555
});
5656

57-
test("tools.allow is review surface without product writes", () => {
57+
test("tools.allow is review surface with product writes", () => {
5858
const allow = critiquePackage.tools?.allow ?? [];
5959
expect(allow).toContain("read_file");
6060
expect(allow).toContain("read_file");
6161
expect(allow).not.toContain("use_skill");
62-
expect(allow).not.toContain("write_file");
63-
expect(allow).not.toContain("edit_file");
64-
expect(allow).not.toContain("delete_file");
62+
expect(allow).toContain("write_file");
63+
expect(allow).toContain("edit_file");
64+
expect(allow).toContain("delete_file");
6565
});
6666

6767
test("modelRole is review", () => {

src/agent/directors/critique/package.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ API contract check (blocking when brief specifies signatures):
4646
- Prefer reading tests/callers; a tiny sync call via run_shell that would hang on a Promise is evidence.
4747
- Rank these as blocking, not style nits.
4848
49-
Write tools are not mounted. Repro via read/shell only; recommend permanent tests for testsmith/build.
49+
Write tools are mounted with no path lock — do not use them. Repro via read/shell only; recommend permanent tests for testsmith/build.
5050
5151
OUT OF LANE → refuse or reclassify under Blockers:
5252
- implementing fixes (route to build)

src/agent/directors/draper/package.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ describe("draperPackage", () => {
2020
expect(draperPackage.spawn.maySpawn).toBe(false);
2121
});
2222

23-
test("tools.allow is review surface without product writes", () => {
23+
test("tools.allow is review surface with product writes", () => {
2424
const allow = draperPackage.tools?.allow ?? [];
2525
expect(allow).toContain("read_file");
26-
expect(allow).not.toContain("write_file");
27-
expect(allow).not.toContain("edit_file");
28-
expect(allow).not.toContain("delete_file");
26+
expect(allow).toContain("write_file");
27+
expect(allow).toContain("edit_file");
28+
expect(allow).toContain("delete_file");
2929
});
3030

3131
test("modelRole is review", () => {

src/agent/directors/emil/package.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ describe("emilPackage", () => {
2020
expect(emilPackage.spawn.maySpawn).toBe(false);
2121
});
2222

23-
test("tools.allow is review surface without product writes", () => {
23+
test("tools.allow is review surface with product writes", () => {
2424
const allow = emilPackage.tools?.allow ?? [];
2525
expect(allow).toContain("read_file");
26-
expect(allow).not.toContain("write_file");
27-
expect(allow).not.toContain("edit_file");
28-
expect(allow).not.toContain("delete_file");
26+
expect(allow).toContain("write_file");
27+
expect(allow).toContain("edit_file");
28+
expect(allow).toContain("delete_file");
2929
});
3030

3131
test("modelRole is review", () => {

src/agent/directors/explore/package.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ describe("explorePackage", () => {
3838
expect(explorePackage.spawn.maySpawn).toBe(false);
3939
});
4040

41-
test("tools.allow is read-only (no product writes)", () => {
41+
test("tools.allow mounts product writes (lane: no product edits)", () => {
4242
const allow = explorePackage.tools?.allow ?? [];
4343
expect(allow).toContain("read_file");
4444
expect(allow).toContain("grep");
45-
expect(allow).not.toContain("write_file");
46-
expect(allow).not.toContain("edit_file");
47-
expect(allow).not.toContain("delete_file");
45+
expect(allow).toContain("write_file");
46+
expect(allow).toContain("edit_file");
47+
expect(allow).toContain("delete_file");
4848
});
4949

5050
test("modelRole is explore", () => {

src/agent/directors/explore/package.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { DirectorPackage } from "../types.js";
2-
import { READ_TOOLS } from "../tool-sets.js";
2+
import { REVIEW_TOOLS } from "../tool-sets.js";
33

44
export const explorePackage: DirectorPackage = {
55
id: "explore",
@@ -22,7 +22,7 @@ FINISH BIAS: Prefer one thorough pass then report. Expand Findings, change appro
2222
FINDINGS SHAPE: Findings must be a scannable map — key paths, symbols, call flow / ownership — not optional prose dump. Cite paths. No drive-by refactors, no feature work, no review severity theater.
2323
2424
OUT OF LANE → report Blockers naming the right director: build, plan, critique, greybeard, intern.`,
25-
tools: { allow: READ_TOOLS },
25+
tools: { allow: REVIEW_TOOLS },
2626
spawn: { maySpawn: false },
2727
tier: "leaf",
2828
modelRole: "explore",

0 commit comments

Comments
 (0)