Skip to content

Commit 8764920

Browse files
committed
feat(tools): advertise one posix wire set; hide engine aliases
Advertise read/write/edit/delete/bash/grep/glob plus the control-plane. Registry engines stay posix-named. Hidden aliases dispatch; grants canonicalize both sides. Codex does not advertise apply_patch, shell, or update_plan. Fixes CL-8400
1 parent b167db3 commit 8764920

43 files changed

Lines changed: 744 additions & 409 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎docs/IMPLEMENTATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ Twenty packages under `src/agent/directors/<id>/` register in `DIRECTOR_REGISTRY
165165

166166
**Grok infer envelope.** `loadSessionChatPrompt` always appends `loadAgentContextExtensions` (`AGENTS.md`, capped at `MAX_AGENTS_MD_BYTES`) and advertises CORE+CATALOG full schemas via `advertisedToolNamesForSessionMode`. That assembly is family-agnostic — Grok does not substitute the trimmed director prompt (`buildSubAgentSystemPrompt` + `formatDirectorSystemPrompt`). Workers already use that trimmed path (no `AGENTS.md`, mounted-tool schemas only). Keep the infer envelope on Grok; do not strip `AGENTS.md` or core schemas. Measure in `src/agent/prompt-sizes.ts` (`assembleSkywalkerInferEnvelope` vs `assembleDirectorPrompt("skywalker", "grok")`).
167167

168-
**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.
168+
**One advertised posix set (CL-8400).** Registry engines stay posix-named (`read_file`, `write_file`, `edit_file`, `delete_file`, `run_shell`, `search_files`, `grep`). Advertise is a 1:1 projection onto wire names (`read`, `write`, `edit`, `delete`, `bash`, `glob`, `grep`) plus the unchanged control-plane. Incoming aliases (wire names, old posix ids, Codex `shell` → `run_shell`, `update_plan` → `manage_tasks`) canonicalize onto the engine id for dispatch and grants. `apply_patch` is neither advertised nor dispatched (not an alias of `edit`). `list_dir` stays mounted and unadvertised. Director `tools.allow` stays engine names. Codex does not dual-publish `shell`+`run_shell`. Hidden `shell` coerces Codex `command` (string or `["bash","-lc",script]` argv), `workdir`, and `timeout_ms` onto `run_shell`. Hidden `update_plan` maps `plan: [{step, status}]` onto `manage_tasks(action: "create")` (`pending`/`in_progress`/`completed` → `todo`/`doing`/`done`).
169169

170170
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. Attached skills (style + philosophy on directors that listed both; never intern or Skywalker primary) are injected into the worker system prompt at spawn from plugin skill dirs only (no project-local `.agents`/`.claude`/`.codex` fallback). Optional skills are listed in the identity header for awareness; workers mount `skill_search` + `use_skill` on every family, scoped to the union of `attachedSkills` and `optionalSkills`. `use_skill` refuses names already attached or already loaded this session and does not return the body again. Primary mounts `use_skill` for its own skill list (same in-session refuse; no attached set).

‎packages/prompt-variance/src/rows.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe("prompt-variance family rows", () => {
7474
expect(grokRow.residual).toContain("prefer the structured report");
7575
expect(grokRow.residual).toContain("re-open paths you already read");
7676
expect(grokRow.residual).toContain("done-definition is met");
77-
expect(grokRow.residual).toContain("never run_shell");
77+
expect(grokRow.residual).toContain("never bash");
7878
});
7979

8080
test("each ceremony line appears exactly once in the grok row (P2 invariant)", () => {

‎packages/prompt-variance/src/rows.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export const grokRow: PromptVarianceRow = {
6464
"- Once you can answer the dispatch brief, prefer the structured report over another speculative tool call.",
6565
"- If the next call would only re-open paths you already read, write the report instead.",
6666
"- When the dispatch brief's done-definition is met, write the report envelope instead of making one more search or micro-edit.",
67-
"- Route file and web work through the dedicated tools, never run_shell — mining showed grok reaching for shell first when a typed tool already covered the job.",
67+
"- Route file and web work through the dedicated tools, never bash — mining showed grok reaching for shell first when a typed tool already covered the job.",
6868
"- Never run git add, git commit, git stash, or any other state-changing git command unless the user asks.",
6969
"- Do not narrate a plan before acting on a small task; act, then report.",
7070
"- Verify with the test command once at the end, not after every edit.",

‎plugins/corbits-skills/skills/native-integration/SKILL.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ Do not delete Corbits-only skills (`plan`, `git-worktrees`, `idiot-proof`). They
1616

1717
Corbits tests use `bun:test` (`bun test`, `bun run test`), not GaaS `tap` (`import t from "tap"`). When the typescript skill shows tap examples, map them to bun:test (`import { expect, test } from "bun:test"`). Do not fork the typescript skill body.
1818

19-
GaaS opsh scripts are bash (`#!/usr/bin/env opsh`, `lib::import`) and use TAP via `prove` (`test-harness`). That harness is not Corbits `bun:test`. Write scripts with `write_file`/`edit_file`; agent commands use `run_shell`. Do not fork the GaaS opsh body.
19+
GaaS opsh scripts are bash (`#!/usr/bin/env opsh`, `lib::import`) and use TAP via `prove` (`test-harness`). That harness is not Corbits `bun:test`. Write scripts with `write`/`edit`; agent commands use `bash`. Do not fork the GaaS opsh body.
2020

2121
## Tool mapping
2222

@@ -32,14 +32,14 @@ When a GaaS skill names a Claude/GaaS tool, use the Corbits equivalent. Do not c
3232
| `@critic` / `@critique` | `spawn_agent(agent="critic")` |
3333
| `@intern` | `spawn_agent(agent="intern")` |
3434
| `@explorer` | `spawn_agent(agent="explorer")` |
35-
| Read / Write / Edit | `read_file` / `write_file` / `edit_file` |
36-
| Glob / Grep | `search_files` / `grep` |
37-
| Bash | `run_shell` |
35+
| Read / Write / Edit | `read` / `write` / `edit` |
36+
| Glob / Grep | `glob` / `grep` |
37+
| Bash | `bash` |
3838
| WebFetch / WebSearch | `web_fetch` / `web_search` |
3939

4040
`intent="general"` is not a Corbits spawn. Use a closed director id.
4141

42-
GaaS ast-grep invokes `sg` as a CLI. Corbits extras: run `sg` via `run_shell`. Do not fork the GaaS ast-grep body.
42+
GaaS ast-grep invokes `sg` as a CLI. Corbits extras: run `sg` via `bash`. Do not fork the GaaS ast-grep body.
4343

4444
Slash names that differ from GaaS skill ids: `/review` is GaaS `code-review`; `/create-issue` is GaaS `linear-create`. Keep those Corbits names.
4545

@@ -67,7 +67,7 @@ GaaS linear-issue-workflow inlines `git worktree add` and marks In Progress afte
6767

6868
GaaS `style` refuses to operate outside a git repo. Corbits does not: a folder without `.git` is a valid working directory (scratch, unpacked tarball, new project). Git-using skills (`implement`, `review`, `git-rebase`, `pull-request-review`) still no-op or ask when they need a repo. Do not invent a git repo to satisfy those skills.
6969

70-
When GaaS git-rebase writes `/tmp` editor scripts, Corbits still plans on the primary and intern executes sequenced git via `run_shell`; intern may use inline `GIT_SEQUENCE_EDITOR` instead of write_file editor scripts. Do not fork the GaaS git-rebase body.
70+
When GaaS git-rebase writes `/tmp` editor scripts, Corbits still plans on the primary and intern executes sequenced git via `bash`; intern may use inline `GIT_SEQUENCE_EDITOR` instead of write editor scripts. Do not fork the GaaS git-rebase body.
7171

7272
## Tracker-agnostic issues
7373

‎plugins/corbits-skills/skills/native-runtime/SKILL.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ disable-model-invocation: true
55
description: Compact Corbits worker runtime invariants for baked prompts.
66
---
77

8-
Use Corbits tool names: `read_file`, `write_file`, `edit_file`, `delete_file`,
9-
`grep`, `search_files`, `run_shell`, `web_search`, `web_fetch`,
8+
Use Corbits tool names: `read`, `write`, `edit`, `delete`,
9+
`grep`, `glob`, `bash`, `web_search`, `web_fetch`,
1010
`manage_tasks`, and `ask_director` for worker questions.
1111

1212
Use file tools for file reads, edits, writes, and deletions. Never use shell
1313
redirects, heredocs, `echo`, `cat`, stream editors, or remove commands as
14-
substitutes for file tools. Use bounded `grep` and `search_files` instead of
14+
substitutes for file tools. Use bounded `grep` and `glob` instead of
1515
unbounded recursive shell searches. Use web tools for URLs; never use curl or
1616
wget.
1717

‎scripts/eval-capability.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -381,8 +381,8 @@ describe("buildEvalDiagnostics", () => {
381381
const diagnostics = await buildEvalDiagnostics(
382382
sampleConfig({ providerName: "openai" }),
383383
);
384-
expect(diagnostics.advertisedTools).toContain("read_file");
385-
expect(diagnostics.advertisedTools).toContain("run_shell");
384+
expect(diagnostics.advertisedTools).toContain("read");
385+
expect(diagnostics.advertisedTools).toContain("bash");
386386
expect(diagnostics.advertisedTools).not.toContain("ask_operator");
387387
expect(diagnostics.reasoningEffort).toBeNull();
388388
});
@@ -394,7 +394,7 @@ describe("buildEvalDiagnostics", () => {
394394
sampleConfig({ providerName }),
395395
);
396396
expect(diagnostics).not.toHaveProperty("codexInstructionsHash");
397-
expect(diagnostics.advertisedTools).toContain("read_file");
397+
expect(diagnostics.advertisedTools).toContain("read");
398398
},
399399
);
400400

‎src/agent/canonical-tool-name.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,20 @@
1+
import { engineToolName } from "./tool-aliases.js";
2+
13
const DEFAULT_PREFIX = "default.";
24

35
// Muse Spark emits `default.<name>` and duplicated `<name>.<name>`. Dispatch
46
// already strips those onto catalog keys; classify, grants, and the execution
57
// cache must use the same name so an alias cannot force a second ask/deny.
8+
// Wire names (read/bash/…) and hidden aliases (shell/update_plan) collapse onto
9+
// the registry engine id so a grant stored as run_shell covers bash.
610
export function canonicalToolName(requested: string): string {
711
let name = requested;
812
if (name.startsWith(DEFAULT_PREFIX)) {
913
const stripped = name.slice(DEFAULT_PREFIX.length);
1014
if (stripped.length > 0) name = stripped;
1115
}
12-
return undoubledName(name) ?? name;
16+
name = undoubledName(name) ?? name;
17+
return engineToolName(name);
1318
}
1419

1520
function undoubledName(requested: string): string | undefined {

‎src/agent/codex-tool-mount.test.ts‎

Lines changed: 33 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,6 @@
11
/**
2-
* Mount coverage for Codex tool proxies (apply_patch, shell, update_plan):
3-
* primary strip of apply_patch, allowlists, and build-shaped capability filter
4-
* retention.
5-
*
6-
* runSubAgent has no standalone toolset-factory export to import directly (the
7-
* mount is inline in runSubAgent's tool-assembly), so the subagent mount path
8-
* is covered here via the same allowDeleteFromCapabilities /
9-
* allowShellFromCapabilities calls runSubAgent makes against a leaf
10-
* capability filter, feeding createCodexToolProxies exactly as run.ts does.
2+
* Mount coverage for Codex: no advertised apply_patch/shell/update_plan,
3+
* engines stay posix-named, hidden aliases dispatch without dual-publish.
114
*/
125
import { mkdtempSync } from "node:fs";
136
import { tmpdir } from "node:os";
@@ -21,7 +14,7 @@ import {
2114
createCodexToolProxies,
2215
} from "./codex-tool-proxies.js";
2316
import { BUILD_TOOLS, DOCS_TOOLS } from "./directors/tool-sets.js";
24-
import { CORE_TOOL_NAMES } from "./tool-search.js";
17+
import { advertisedTools, CORE_TOOL_NAMES } from "./tool-search.js";
2518

2619
afterEach(() => {
2720
spyOn(posixModule, "createPosixTools").mockRestore();
@@ -55,7 +48,7 @@ describe("Codex tool proxy mount", () => {
5548
await toolset.dispose();
5649
});
5750

58-
test("Codex createAgentToolset strips apply_patch on primary; keeps shell/update_plan", async () => {
51+
test("Codex createAgentToolset does not advertise apply_patch/shell/update_plan", async () => {
5952
// Unstubbed createPosixTools: write_file / edit_file / delete_file come from
6053
// the real posix + delete-file plugin mount. An empty stub would hide them
6154
// and make the DIY-remains assertion meaningless.
@@ -74,19 +67,24 @@ describe("Codex tool proxy mount", () => {
7467
});
7568
const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name);
7669
expect(names).not.toContain("apply_patch");
77-
// Primary DIY product writes remain mounted.
7870
expect(names).toContain("write_file");
7971
expect(names).toContain("edit_file");
8072
expect(names).toContain("delete_file");
81-
// shell / update_plan are not product-mutation tools (same classification
82-
// as run_shell / manage_tasks), so the primary apply_patch strip does not
83-
// remove them — they stay mounted on primary, mirroring run_shell.
84-
expect(names).toContain("shell");
85-
expect(names).toContain("update_plan");
73+
expect(names).toContain("run_shell");
74+
expect(names).not.toContain("shell");
75+
expect(names).not.toContain("update_plan");
76+
const advertised = advertisedTools(
77+
toolset.dynamicRunner.currentDefinitions(),
78+
).map((d) => d.name);
79+
expect(advertised).toContain("bash");
80+
expect(advertised).not.toContain("run_shell");
81+
expect(advertised).not.toContain("shell");
82+
expect(advertised).not.toContain("apply_patch");
83+
expect(advertised).not.toContain("update_plan");
8684
await toolset.dispose();
8785
});
8886

89-
test("update_plan dispatches through the real mount without hitting posixTools", async () => {
87+
test("update_plan hidden-dispatches through manage_tasks without a proxy mount", async () => {
9088
// Unstubbed createPosixTools (real temp dir): update_plan used to call
9189
// runTool("manage_tasks", ...), which forwards onto posixTools.run and
9290
// fails with "unknown tool: manage_tasks" — posixTools has no
@@ -120,13 +118,13 @@ describe("Codex tool proxy mount", () => {
120118
await toolset.dispose();
121119
});
122120

123-
test("BUILD_TOOLS and DOCS_TOOLS include apply_patch; CORE_TOOL_NAMES does not", () => {
124-
expect(BUILD_TOOLS).toContain("apply_patch");
125-
expect(DOCS_TOOLS).toContain("apply_patch");
121+
test("BUILD_TOOLS and DOCS_TOOLS omit apply_patch; CORE_TOOL_NAMES does not list it", () => {
122+
expect(BUILD_TOOLS).not.toContain("apply_patch");
123+
expect(DOCS_TOOLS).not.toContain("apply_patch");
126124
expect(CORE_TOOL_NAMES).not.toContain("apply_patch");
127125
});
128126

129-
test("capability include-filter keeps proxies for build-shaped allowlists", () => {
127+
test("capability include-filter no longer keeps Codex proxy names", () => {
130128
const proxies = createCodexToolProxies({
131129
isCodex: true,
132130
runTool: async () => ({ content: "ok" }),
@@ -141,46 +139,31 @@ describe("Codex tool proxy mount", () => {
141139

142140
const allow = new Set<string>(BUILD_TOOLS);
143141
const kept = proxies.filter((t) => allow.has(t.definition.name));
144-
expect(kept.map((t) => t.definition.name)).toEqual([
145-
"apply_patch",
146-
"shell",
147-
"update_plan",
148-
]);
142+
expect(kept).toEqual([]);
149143

150144
const docsAllow = new Set<string>(DOCS_TOOLS);
151145
const docsKept = proxies.filter((t) => docsAllow.has(t.definition.name));
152-
expect(docsKept.map((t) => t.definition.name)).toEqual([
153-
"apply_patch",
154-
"update_plan",
155-
]);
146+
expect(docsKept).toEqual([]);
156147
});
157148

158-
test("runSubAgent-shaped mount: docs capability filter denies shell, keeps update_plan", () => {
159-
// Mirrors run.ts: allowDelete / allowShell are derived from the leaf
160-
// capability filter before createCodexToolProxies runs. update_plan is
161-
// never gated by it (manage_tasks is unconditionally mounted for every
162-
// sub-agent), so it stays regardless of the allowlist shape.
163-
const docsCapabilities = { mode: "allow" as const, tools: DOCS_TOOLS };
149+
test("runSubAgent-shaped allowlists do not keep Codex proxy names", () => {
150+
const docsAllow = new Set<string>(DOCS_TOOLS);
164151
const proxies = createCodexToolProxies({
165152
isCodex: true,
166153
runTool: async () => ({ content: "ok" }),
167154
readRawFile: async () => ({ content: "ok" }),
168155
runManageTasks: async () => ({ content: "ok" }),
169-
allowDelete: allowDeleteFromCapabilities(docsCapabilities),
170-
allowShell: allowShellFromCapabilities(docsCapabilities),
156+
allowDelete: allowDeleteFromCapabilities({
157+
mode: "allow",
158+
tools: DOCS_TOOLS,
159+
}),
160+
allowShell: allowShellFromCapabilities({
161+
mode: "allow",
162+
tools: DOCS_TOOLS,
163+
}),
171164
});
172-
expect(proxies.map((t) => t.definition.name)).toEqual([
173-
"apply_patch",
174-
"shell",
175-
"update_plan",
176-
]);
177-
178-
const docsAllow = new Set<string>(DOCS_TOOLS);
179165
const docsKept = proxies.filter((t) => docsAllow.has(t.definition.name));
180-
expect(docsKept.map((t) => t.definition.name)).toEqual([
181-
"apply_patch",
182-
"update_plan",
183-
]);
166+
expect(docsKept).toEqual([]);
184167
});
185168

186169
test("non-Codex runSubAgent-shaped mount produces no proxies at all", () => {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,10 @@ describe("bruckheimerPackage", () => {
4242

4343
test("systemPrompt uses Corbits file tools (not Read/Write/Bash)", () => {
4444
const p = bruckheimerPackage.systemPrompt;
45-
expect(p).toMatch(/read_file/);
46-
expect(p).toMatch(/write_file/);
47-
expect(p).toMatch(/edit_file/);
48-
expect(p).toMatch(/search_files/);
45+
expect(p).toMatch(/`read`/);
46+
expect(p).toMatch(/`write`/);
47+
expect(p).toMatch(/`edit`/);
48+
expect(p).toMatch(/`glob`/);
4949
expect(p).not.toMatch(/\bAskUserQuestion\b/);
5050
expect(p).not.toMatch(/Use Read and Write/);
5151
expect(p).not.toMatch(/Use Bash sparingly/);

0 commit comments

Comments
 (0)