Skip to content

Commit 0b042a8

Browse files
committed
Proxy Codex apply_patch onto Corbits file tools
Why: Codex-trained models call apply_patch from pinned instructions, but Corbits only advertised write_file/edit_file/delete_file. That dialect mismatch hurts Codex evals versus Codex-native harnesses. What changed: Codex-only apply_patch proxy (parse envelope, forward through posix write/edit/delete), shared product-mutation ownership, primary deny unchanged, IMPLEMENT/DOCS allowlists, docs leaves refuse Delete/Move via allowDelete. Bridge text left for the next stacked change. Test plan: bun test on codex-apply-patch, codex-tool-proxies, codex-tool-mount, product-mutation-tools, tool-sets; bun run typecheck.
1 parent c2e72ce commit 0b042a8

26 files changed

Lines changed: 1703 additions & 40 deletions

docs/IMPLEMENTATION.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ Sixteen packages under `src/agent/directors/<id>/` register in `DIRECTOR_REGISTR
159159
3. Nested spawn: packages with `spawn.allowlist` forward that list into nested `task` (`spawnAllowlist` on nestedDispatch). Off-list `agent` is refused. `task(agent=skywalker)` is refused (primary is not a spawned worker). Primary omits the list so plugin profiles stay reachable.
160160
4. `directorProfiles()` is the spawn catalog (`default-agents.ts`) — closed set minus skywalker; plugin agent profiles still load and can override by id.
161161
5. Primary chat role is Skywalker: `buildChatRole()``createSkywalkerSystemPrompt()`. Product mutation tools are stripped from the primary toolset and from CORE/CATALOG ads (`PRIMARY_DENIED_PRODUCT_TOOLS`) — never-implement is structural for path tools. Residual: `run_shell` stays on primary; MCP tools loaded later are not re-stripped by that deny list; optional `writePaths` (when a profile sets it) only gate path-keyed product tools.
162+
163+
**Codex `apply_patch` proxy.** When the active provider is Codex (`isCodexProviderName`), `createAgentToolset` and `runSubAgent` mount an `apply_patch` stringTool from `createCodexToolProxies` that parses the Codex envelope and forwards each op through the posix `ToolRunner` (`write_file` / `delete_file` / `read_file`) so permission plugins still apply. Primary still denies `apply_patch` via `PRIMARY_DENIED_PRODUCT_TOOLS`; implement and docs leaf allowlists (`IMPLEMENT_TOOLS` / `DOCS_TOOLS`) include it so Codex workers keep the proxy after the capability filter. `CORE_TOOL_NAMES` does not list it.
162164
6. Shipped directors omit `writePaths`. The optional field is still enforced in the permission gate via ALS identity (`identity-context.ts` + `write-path-policy.ts`) when a plugin/custom profile sets it.
163165
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.
164166

Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,306 @@
1+
import { describe, expect, test } from "bun:test";
2+
import {
3+
CodexApplyPatchError,
4+
applyUpdateHunks,
5+
extractAffectedPaths,
6+
parseCodexApplyPatch,
7+
} from "./codex-apply-patch.js";
8+
9+
describe("parseCodexApplyPatch", () => {
10+
test("parses Add File with Codex trailing newlines", () => {
11+
const patch = parseCodexApplyPatch(`*** Begin Patch
12+
*** Add File: hello.txt
13+
+Hello world
14+
+second line
15+
*** End Patch
16+
`);
17+
expect(patch.ops).toEqual([
18+
{
19+
type: "add",
20+
path: "hello.txt",
21+
content: "Hello world\nsecond line\n",
22+
},
23+
]);
24+
});
25+
26+
test("Add File with no + lines yields empty content", () => {
27+
const patch = parseCodexApplyPatch(`*** Begin Patch
28+
*** Add File: empty.txt
29+
*** End Patch
30+
`);
31+
expect(patch.ops).toEqual([{ type: "add", path: "empty.txt", content: "" }]);
32+
});
33+
34+
test("parses Delete File", () => {
35+
const patch = parseCodexApplyPatch(`*** Begin Patch
36+
*** Delete File: obsolete.txt
37+
*** End Patch
38+
`);
39+
expect(patch.ops).toEqual([{ type: "delete", path: "obsolete.txt" }]);
40+
});
41+
42+
test("parses Update File with Move to", () => {
43+
const patch = parseCodexApplyPatch(`*** Begin Patch
44+
*** Update File: src/app.py
45+
*** Move to: src/main.py
46+
@@ def greet():
47+
-print("Hi")
48+
+print("Hello, world!")
49+
*** End Patch
50+
`);
51+
expect(patch.ops).toHaveLength(1);
52+
const op = patch.ops[0]!;
53+
expect(op.type).toBe("update");
54+
if (op.type !== "update") throw new Error("unreachable");
55+
expect(op.path).toBe("src/app.py");
56+
expect(op.moveTo).toBe("src/main.py");
57+
expect(op.hunks).toHaveLength(1);
58+
expect(op.hunks[0]!.header).toBe("def greet():");
59+
expect(op.hunks[0]!.lines).toEqual([
60+
{ kind: "-", text: 'print("Hi")' },
61+
{ kind: "+", text: 'print("Hello, world!")' },
62+
]);
63+
});
64+
65+
test("parses stacked multi-@@ context anchors as header-only then change hunk", () => {
66+
const patch = parseCodexApplyPatch(`*** Begin Patch
67+
*** Update File: src/app.py
68+
@@ class BaseClass
69+
@@ def method():
70+
-old_line
71+
+new_line
72+
*** End Patch
73+
`);
74+
const op = patch.ops[0]!;
75+
expect(op.type).toBe("update");
76+
if (op.type !== "update") throw new Error("unreachable");
77+
expect(op.hunks).toHaveLength(2);
78+
expect(op.hunks[0]).toEqual({ header: "class BaseClass", lines: [] });
79+
expect(op.hunks[1]!.header).toBe(" def method():");
80+
expect(op.hunks[1]!.lines).toEqual([
81+
{ kind: "-", text: "old_line" },
82+
{ kind: "+", text: "new_line" },
83+
]);
84+
});
85+
86+
test("rejects bare empty @@ without lines", () => {
87+
expect(() =>
88+
parseCodexApplyPatch(`*** Begin Patch
89+
*** Update File: src/app.py
90+
@@
91+
*** End Patch
92+
`),
93+
).toThrow(/empty hunk/);
94+
});
95+
96+
test("rejects malformed envelope (missing Begin)", () => {
97+
expect(() =>
98+
parseCodexApplyPatch(`*** Add File: a.txt
99+
+hi
100+
*** End Patch
101+
`),
102+
).toThrow(CodexApplyPatchError);
103+
expect(() =>
104+
parseCodexApplyPatch(`*** Add File: a.txt
105+
+hi
106+
*** End Patch
107+
`),
108+
).toThrow(/Begin Patch/);
109+
});
110+
111+
test("rejects malformed envelope (missing End)", () => {
112+
expect(() =>
113+
parseCodexApplyPatch(`*** Begin Patch
114+
*** Add File: a.txt
115+
+hi
116+
`),
117+
).toThrow(/End Patch/);
118+
});
119+
120+
test("rejects absolute paths", () => {
121+
expect(() =>
122+
parseCodexApplyPatch(`*** Begin Patch
123+
*** Add File: /etc/passwd
124+
+x
125+
*** End Patch
126+
`),
127+
).toThrow(/relative/);
128+
129+
expect(() =>
130+
parseCodexApplyPatch(`*** Begin Patch
131+
*** Delete File: /tmp/x
132+
*** End Patch
133+
`),
134+
).toThrow(/absolute/);
135+
136+
expect(() =>
137+
parseCodexApplyPatch(`*** Begin Patch
138+
*** Update File: C:\\Windows\\system32\\x
139+
@@
140+
-a
141+
+b
142+
*** End Patch
143+
`),
144+
).toThrow(/absolute/);
145+
});
146+
});
147+
148+
describe("extractAffectedPaths", () => {
149+
test("multi-file path extraction", () => {
150+
const patch = parseCodexApplyPatch(`*** Begin Patch
151+
*** Add File: hello.txt
152+
+Hello
153+
*** Update File: src/app.py
154+
@@
155+
-old
156+
+new
157+
*** Delete File: obsolete.txt
158+
*** End Patch
159+
`);
160+
expect(extractAffectedPaths(patch)).toEqual([
161+
"hello.txt",
162+
"src/app.py",
163+
"obsolete.txt",
164+
]);
165+
});
166+
167+
test("move path extraction includes source and destination", () => {
168+
const patch = parseCodexApplyPatch(`*** Begin Patch
169+
*** Update File: src/app.py
170+
*** Move to: src/main.py
171+
@@
172+
-a
173+
+b
174+
*** End Patch
175+
`);
176+
expect(extractAffectedPaths(patch)).toEqual(["src/app.py", "src/main.py"]);
177+
});
178+
});
179+
180+
describe("applyUpdateHunks", () => {
181+
test("applies a simple replacement hunk", () => {
182+
const original = `def greet():
183+
print("Hi")
184+
print("bye")
185+
`;
186+
const patch = parseCodexApplyPatch(`*** Begin Patch
187+
*** Update File: src/app.py
188+
@@ def greet():
189+
-print("Hi")
190+
+print("Hello, world!")
191+
*** End Patch
192+
`);
193+
const op = patch.ops[0]!;
194+
expect(op.type).toBe("update");
195+
if (op.type !== "update") throw new Error("unreachable");
196+
const updated = applyUpdateHunks(original, op.hunks);
197+
expect(updated).toBe(`def greet():
198+
print("Hello, world!")
199+
print("bye")
200+
`);
201+
});
202+
203+
test("applies stacked multi-@@ anchors then replacement", () => {
204+
const original = `class BaseClass
205+
def method():
206+
old_line
207+
keep
208+
`;
209+
const patch = parseCodexApplyPatch(`*** Begin Patch
210+
*** Update File: src/app.py
211+
@@ class BaseClass
212+
@@ def method():
213+
- old_line
214+
+ new_line
215+
*** End Patch
216+
`);
217+
const op = patch.ops[0]!;
218+
expect(op.type).toBe("update");
219+
if (op.type !== "update") throw new Error("unreachable");
220+
expect(applyUpdateHunks(original, op.hunks)).toBe(`class BaseClass
221+
def method():
222+
new_line
223+
keep
224+
`);
225+
});
226+
227+
test("applies context-aware multi-line hunk", () => {
228+
const original = `line1
229+
line2
230+
target
231+
line4
232+
`;
233+
const hunks = [
234+
{
235+
lines: [
236+
{ kind: " " as const, text: "line2" },
237+
{ kind: "-" as const, text: "target" },
238+
{ kind: "+" as const, text: "replaced" },
239+
{ kind: " " as const, text: "line4" },
240+
],
241+
},
242+
];
243+
expect(applyUpdateHunks(original, hunks)).toBe(`line1
244+
line2
245+
replaced
246+
line4
247+
`);
248+
});
249+
250+
test("fuzzy match: rstrip then trim after exact fail", () => {
251+
const original = `foo
252+
bar
253+
baz
254+
`;
255+
const updated = applyUpdateHunks(original, [
256+
{
257+
lines: [
258+
{ kind: "-", text: "bar" },
259+
{ kind: "+", text: "qux" },
260+
],
261+
},
262+
]);
263+
expect(updated).toBe(`foo
264+
qux
265+
baz
266+
`);
267+
268+
const padded = applyUpdateHunks(` foo \nbar\n`, [
269+
{
270+
lines: [
271+
{ kind: "-", text: "foo" },
272+
{ kind: "+", text: "FOO" },
273+
],
274+
},
275+
]);
276+
expect(padded).toBe(`FOO
277+
bar
278+
`);
279+
});
280+
281+
test("NormalizeToLf: non-empty update result ends with newline", () => {
282+
const updated = applyUpdateHunks("a\nb", [
283+
{
284+
lines: [
285+
{ kind: "-", text: "b" },
286+
{ kind: "+", text: "c" },
287+
],
288+
},
289+
]);
290+
expect(updated).toBe("a\nc\n");
291+
expect(updated.endsWith("\n")).toBe(true);
292+
});
293+
294+
test("throws when context cannot be found", () => {
295+
expect(() =>
296+
applyUpdateHunks("a\nb\n", [
297+
{
298+
lines: [
299+
{ kind: "-", text: "missing" },
300+
{ kind: "+", text: "x" },
301+
],
302+
},
303+
]),
304+
).toThrow(/failed to find expected lines/);
305+
});
306+
});

0 commit comments

Comments
 (0)