Skip to content

Commit 5913441

Browse files
committed
refactor(subagent): drop dead malformed branch from capability preflight
1 parent 99e4d75 commit 5913441

3 files changed

Lines changed: 69 additions & 76 deletions

File tree

‎src/agent/profiles.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,11 @@ const registry: AgentProfile[] = [...defaultPlugin.agents];
7474
// Load diagnostics: additive over loadAgentProfiles. `revision` stamps the
7575
// profile snapshot a dispatch was verified against (agent-fleet records it on
7676
// the session); `malformed` names local files that failed to load and why.
77-
// A malformed file never blocks the load — it fails closed only when a
78-
// dispatch's requires_tools preflight must verify against it (CL-9476).
77+
// A malformed file never blocks the load — it is skipped and named here so
78+
// callers can surface it. requires_tools preflight verifies against the
79+
// resolved dispatch capabilities only, never against this list (CL-9476 keeps
80+
// no malformed_profile preflight branch: agent-fleet never plumbed a failing
81+
// source through dispatch, so the branch was dead and has been removed).
7982
export interface MalformedAgentProfile {
8083
path: string;
8184
reason: string;

‎src/subagent/capability-preflight.test.ts‎

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe("preflightCapabilities", () => {
7474
expect(result.unavailable.tool).toBe("run_shell");
7575
});
7676

77-
test("known tool with an absent binary rejects missing_binary", () => {
77+
test("narrowed knownEngines reject missing_binary (test-only seam: production always passes the full catalog)", () => {
7878
const result = preflight(["run_shell"], ALLOW_SHELL, []);
7979
expect(result.ok).toBe(false);
8080
if (result.ok) throw new Error("expected rejection");
@@ -95,23 +95,6 @@ describe("preflightCapabilities", () => {
9595
expect(result.unavailable.suggestion).toBe("run_shell");
9696
});
9797

98-
test("malformed profile source fails closed before per-tool checks", () => {
99-
const result = preflightCapabilities({
100-
required: ["read_file"],
101-
resolvedFilter: ALLOW_SHELL,
102-
knownEngines: DEFAULT_KNOWN_ENGINES,
103-
agentLabel: "test-worker",
104-
profileSource: {
105-
malformed: true,
106-
path: "/agents/broken.json",
107-
reason: "unexpected token at line 3",
108-
},
109-
});
110-
expect(result.ok).toBe(false);
111-
if (result.ok) throw new Error("expected rejection");
112-
expect(result.unavailable.code).toBe("malformed_profile");
113-
});
114-
11598
test("reroute alternatives collapse aliases (shell and run_shell agree)", () => {
11699
expect(rerouteAlternatives("shell")).toEqual(
117100
rerouteAlternatives("run_shell"),
@@ -149,14 +132,6 @@ describe("formatCapabilityUnavailable", () => {
149132
code: "stale_snapshot",
150133
unavailable: { code: "stale_snapshot", tool: "run_shell" },
151134
},
152-
{
153-
code: "malformed_profile",
154-
unavailable: {
155-
code: "malformed_profile",
156-
tool: "/agents/broken.json",
157-
suggestion: "unexpected token at line 3",
158-
},
159-
},
160135
{
161136
code: "unknown_tool",
162137
unavailable: {
@@ -181,9 +156,7 @@ describe("formatCapabilityUnavailable", () => {
181156
expect(sentences.length).toBeGreaterThanOrEqual(2);
182157
const action = sentences[sentences.length - 1] ?? "";
183158
expect(
184-
/re-dispatch|drop the requirement|install the backing runtime|fix the profile|correct requires_tools/i.test(
185-
action,
186-
),
159+
/re-dispatch|drop the requirement|correct requires_tools/i.test(action),
187160
).toBe(true);
188161
});
189162
}
@@ -211,6 +184,28 @@ describe("formatCapabilityUnavailable", () => {
211184
expect(message).toContain(alternatives[0] ?? "");
212185
});
213186

187+
test("missing_tool detail renders a tier fact between fact and action", () => {
188+
const message = formatCapabilityUnavailable(
189+
{
190+
code: "missing_tool",
191+
tool: "spawn_agent",
192+
alternatives: ["skywalker"],
193+
detail: 'Tier 3 leaf directors cannot mount fleet verb "spawn_agent"',
194+
},
195+
"test-worker",
196+
);
197+
expect(message.startsWith("Error:")).toBe(true);
198+
expect(message).toContain(
199+
'Tier 3 leaf directors cannot mount fleet verb "spawn_agent".',
200+
);
201+
expect(message.indexOf("Tier 3 leaf")).toBeGreaterThan(
202+
message.indexOf("allowlist omits it"),
203+
);
204+
expect(message.indexOf("Tier 3 leaf")).toBeLessThan(
205+
message.indexOf("Re-dispatch"),
206+
);
207+
});
208+
214209
test("unknown_tool message carries the did-you-mean hint", () => {
215210
const message = formatCapabilityUnavailable(
216211
{ code: "unknown_tool", tool: "run_shel", suggestion: "run_shell" },

‎src/subagent/capability-preflight.ts‎

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,14 @@
55
* mounted on the worker or the dispatch is rejected before any session,
66
* telemetry, or worktree exists. Names are canonical engine ids on both sides
77
* (wire/hidden aliases collapse via canonicalToolName), so `shell` and
8-
* `run_shell` are the same requirement. Fail-closed throughout: unknown names,
9-
* unverifiable profiles, and absent binaries all reject.
8+
* `run_shell` are the same requirement. Fail-closed throughout: unknown names
9+
* and allowlist/denylist misses all reject.
10+
*
11+
* `missing_binary` never fires in production: dispatch always passes the full
12+
* catalog as `knownEngines`, so every catalogued engine verifies. Narrowed
13+
* `knownEngines` sets are a test-only seam for simulating an incomplete
14+
* runtime — the branch exists so tests can prove the fail-closed shape, not
15+
* because production probes binaries per engine (most engines are in-process).
1016
*
1117
* The `stale_snapshot` code is never emitted by preflightCapabilities — it is
1218
* the mount-time echo in run.ts (a tool stamped at dispatch is missing from
@@ -25,39 +31,37 @@ export type CapabilityUnavailableCode =
2531
| "permission_static"
2632
| "missing_binary"
2733
| "stale_snapshot"
28-
| "malformed_profile"
2934
| "unknown_tool";
3035

31-
export interface CapabilityProfileSource {
32-
/** Directory or file the dispatched profile was loaded from, for messages. */
33-
path?: string;
34-
/** True when the profile file failed to parse/validate — unverifiable. */
35-
malformed?: boolean;
36-
/** Loader's reason for the malformed flag, for messages. */
37-
reason?: string;
38-
}
39-
4036
export interface PreflightCapabilitiesInput {
4137
/** Raw requires_tools entries (aliases welcome — canonicalized here). */
4238
required: readonly string[];
4339
/** Resolved dispatch filter; undefined means full mount (everything passes). */
4440
resolvedFilter?: CapabilityFilter | undefined;
45-
/** Canonical engine ids available in this runtime (binary-present). */
41+
/**
42+
* Canonical engine ids verifiable in this dispatch. Production always
43+
* passes the full catalog (DEFAULT_KNOWN_ENGINES); narrowed sets are a
44+
* test-only seam for simulating an incomplete runtime.
45+
*/
4646
knownEngines: readonly string[];
4747
/** Worker label for messages (director id or profile id). */
4848
agentLabel: string;
49-
/** Profile provenance; a malformed source fails closed. */
50-
profileSource?: CapabilityProfileSource;
5149
}
5250

5351
export interface CapabilityUnavailable {
5452
code: CapabilityUnavailableCode;
55-
/** Canonical tool id (or the raw entry for unknown/malformed). */
53+
/** Canonical tool id (or the raw entry for unknown). */
5654
tool: string;
5755
/** Nearest known name, for the unknown_tool typo guard. */
5856
suggestion?: string;
5957
/** Spawnable directors that mount the tool, for reroute hints. */
6058
alternatives?: readonly string[];
59+
/**
60+
* Extra fact sentence for missing_tool, naming a mount restriction the
61+
* allowlist framing cannot see — e.g. a tier gate that withholds fleet
62+
* verbs from leaves. Rendered between the fact and the action sentence.
63+
*/
64+
detail?: string;
6165
}
6266

6367
export type CapabilityPreflightResult =
@@ -98,14 +102,23 @@ export const KNOWN_CAPABILITY_ENGINES: readonly string[] = [
98102
"submit_result",
99103
];
100104

101-
/** Production `knownEngines`: every known engine is binary-present. */
105+
/**
106+
* Dispatch-time `knownEngines`: the full catalog. Dispatch always passes this,
107+
* so every catalogued engine verifies and `missing_binary` never fires in
108+
* production. Narrowed `knownEngines` sets are a test-only seam for
109+
* simulating an incomplete runtime — production probes no binaries per engine
110+
* (most engines are in-process).
111+
*/
102112
export const DEFAULT_KNOWN_ENGINES: readonly string[] =
103113
KNOWN_CAPABILITY_ENGINES;
104114

105115
/**
106-
* Engines mounted outside the capability filter (run.ts mounts manage_tasks
116+
* Engines mounted outside the capability filter (run.ts appends manage_tasks
107117
* after filtering), so a requires_tools entry for them passes preflight even
108-
* when the dispatch filter is a narrow allowlist.
118+
* when the dispatch filter is a narrow allowlist. The update_plan alias
119+
* canonicalizes here, so it rides the same exemption — and the mount-time
120+
* echo in run.ts runs after ALL appends, so the stamped entry always matches
121+
* the live mount.
109122
*/
110123
const POST_FILTER_MOUNTED_ENGINES: readonly string[] = ["manage_tasks"];
111124

@@ -181,19 +194,7 @@ export function rerouteAlternatives(canonical: string): readonly string[] {
181194
export function preflightCapabilities(
182195
input: PreflightCapabilitiesInput,
183196
): CapabilityPreflightResult {
184-
const { resolvedFilter, knownEngines, profileSource } = input;
185-
if (profileSource?.malformed === true) {
186-
return {
187-
ok: false,
188-
unavailable: {
189-
code: "malformed_profile",
190-
tool: profileSource.path ?? "(profile source)",
191-
...(profileSource.reason !== undefined
192-
? { suggestion: profileSource.reason }
193-
: {}),
194-
},
195-
};
196-
}
197+
const { resolvedFilter, knownEngines } = input;
197198
const known = new Set(knownEngines.map((name) => canonicalToolName(name)));
198199
const allow =
199200
resolvedFilter?.mode === "allow"
@@ -293,36 +294,30 @@ export function formatCapabilityUnavailable(
293294
? ` Re-dispatch to one of (${unavailable.alternatives.join(", ")}) or drop the requirement.`
294295
: ` No spawnable director mounts "${unavailable.tool}" — drop the requirement or add a profile that mounts it.`;
295296
switch (unavailable.code) {
296-
case "missing_tool":
297+
case "missing_tool": {
298+
const detail =
299+
unavailable.detail !== undefined ? ` ${unavailable.detail}.` : "";
297300
return (
298301
`Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but its capability allowlist omits it.` +
302+
detail +
299303
alternatives
300304
);
305+
}
301306
case "permission_static":
302307
return (
303308
`Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but its capability denylist blocks it.` +
304309
alternatives
305310
);
306311
case "missing_binary":
307312
return (
308-
`Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but its runtime engine is unavailable in this environment. ` +
309-
`Re-dispatch without requires_tools=["${unavailable.tool}"] or install the backing runtime first.`
313+
`Error: worker "${agentLabel}" requires tool "${unavailable.tool}" but this dispatch cannot verify its runtime engine (not in the dispatch's known-engine set). ` +
314+
`Re-dispatch without requires_tools=["${unavailable.tool}"] or drop the requirement.`
310315
);
311316
case "stale_snapshot":
312317
return (
313318
`Error: stale_snapshot setup_error (non-continuable) — worker "${agentLabel}" was dispatched requiring "${unavailable.tool}" but the live mount no longer provides it. ` +
314319
`Re-dispatch the worker deliberately with a fresh brief instead of retrying in place.`
315320
);
316-
case "malformed_profile": {
317-
const reason =
318-
unavailable.suggestion !== undefined
319-
? ` (${unavailable.suggestion})`
320-
: "";
321-
return (
322-
`Error: worker "${agentLabel}" cannot verify requires_tools — profile source "${unavailable.tool}" is malformed${reason}, so capabilities fail closed. ` +
323-
`Fix the profile file and re-dispatch deliberately.`
324-
);
325-
}
326321
case "unknown_tool": {
327322
const hint =
328323
unavailable.suggestion !== undefined

0 commit comments

Comments
 (0)