Skip to content

Commit efa87d9

Browse files
Mount write tools on every closed director (#634)
* 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 * Fix prettier formatting in director tool-sets
1 parent 50eeac4 commit efa87d9

24 files changed

Lines changed: 152 additions & 90 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
@@ -73,6 +73,7 @@ describe("brandReviewerPackage", () => {
7373
const allow = brandReviewerPackage.tools?.allow ?? [];
7474
expect(allow).toContain("write_file");
7575
expect(allow).toContain("edit_file");
76+
expect(allow).toContain("delete_file");
7677
});
7778

7879
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
@@ -84,14 +84,14 @@ describe("critiquePackage", () => {
8484
expect(critiquePackage.spawn.maySpawn).toBe(false);
8585
});
8686

87-
test("tools.allow is review surface without product writes", () => {
87+
test("tools.allow is review surface with product writes", () => {
8888
const allow = critiquePackage.tools?.allow ?? [];
8989
expect(allow).toContain("read_file");
9090
expect(allow).toContain("read_file");
9191
expect(allow).not.toContain("use_skill");
92-
expect(allow).not.toContain("write_file");
93-
expect(allow).not.toContain("edit_file");
94-
expect(allow).not.toContain("delete_file");
92+
expect(allow).toContain("write_file");
93+
expect(allow).toContain("edit_file");
94+
expect(allow).toContain("delete_file");
9595
});
9696

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

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ describe("draperPackage", () => {
7676
expect(draperPackage.spawn.maySpawn).toBe(false);
7777
});
7878

79-
test("tools.allow is review surface without product writes", () => {
79+
test("tools.allow is review surface with product writes", () => {
8080
const allow = draperPackage.tools?.allow ?? [];
8181
expect(allow).toContain("read_file");
8282
expect(allow).not.toContain("use_skill");
83-
expect(allow).not.toContain("write_file");
84-
expect(allow).not.toContain("edit_file");
85-
expect(allow).not.toContain("delete_file");
83+
expect(allow).toContain("write_file");
84+
expect(allow).toContain("edit_file");
85+
expect(allow).toContain("delete_file");
8686
});
8787

8888
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
@@ -73,13 +73,13 @@ describe("emilPackage", () => {
7373
expect(emilPackage.spawn.maySpawn).toBe(false);
7474
});
7575

76-
test("tools.allow is review surface without product writes", () => {
76+
test("tools.allow is review surface with product writes", () => {
7777
const allow = emilPackage.tools?.allow ?? [];
7878
expect(allow).toContain("read_file");
7979
expect(allow).not.toContain("use_skill");
80-
expect(allow).not.toContain("write_file");
81-
expect(allow).not.toContain("edit_file");
82-
expect(allow).not.toContain("delete_file");
80+
expect(allow).toContain("write_file");
81+
expect(allow).toContain("edit_file");
82+
expect(allow).toContain("delete_file");
8383
});
8484

8585
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
@@ -82,13 +82,13 @@ describe("explorePackage", () => {
8282
expect(explorePackage.spawn.maySpawn).toBe(false);
8383
});
8484

85-
test("tools.allow is read-only (no product writes)", () => {
85+
test("tools.allow mounts product writes (lane: no product edits)", () => {
8686
const allow = explorePackage.tools?.allow ?? [];
8787
expect(allow).toContain("read_file");
8888
expect(allow).toContain("grep");
89-
expect(allow).not.toContain("write_file");
90-
expect(allow).not.toContain("edit_file");
91-
expect(allow).not.toContain("delete_file");
89+
expect(allow).toContain("write_file");
90+
expect(allow).toContain("edit_file");
91+
expect(allow).toContain("delete_file");
9292
});
9393

9494
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
/**
55
* Explorer leaf (CL-7020).
@@ -33,7 +33,7 @@ FINDINGS SHAPE: Findings must be a scannable map — key paths, symbols, call fl
3333
FINISH BIAS: Prefer one thorough pass then report. Expand Findings, change approach, or write the final report — do not keep re-reading the same paths.
3434
3535
OUT OF LANE: product writes, drive-by fixes, shipping features, review severity theater, orchestration, spawning specialists, fleet discovery, becoming Builder/Critic/orchestrator as primary.`,
36-
tools: { allow: READ_TOOLS },
36+
tools: { allow: REVIEW_TOOLS },
3737
spawn: { maySpawn: false },
3838
tier: "leaf",
3939
modelRole: "explore",

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ describe("gaasbotPackage", () => {
6767
expect(gaasbotPackage.spawn.maySpawn).toBe(false);
6868
});
6969

70-
test("denies product write tools (advice only)", () => {
70+
test("mounts product write tools (lane discipline in prompts)", () => {
7171
const allow = gaasbotPackage.tools?.allow ?? [];
7272
expect(allow).toContain("read_file");
73-
expect(allow).not.toContain("write_file");
74-
expect(allow).not.toContain("edit_file");
75-
expect(allow).not.toContain("delete_file");
73+
expect(allow).toContain("write_file");
74+
expect(allow).toContain("edit_file");
75+
expect(allow).toContain("delete_file");
7676
});
7777

7878
test("modelRole is plan", () => {

0 commit comments

Comments
 (0)