Skip to content

Commit 370e6b6

Browse files
BrettNyeclaude
andauthored
feat(orchestrator): forward per-item capability selection in DispatchExecutor.fire (#13a) (#142)
`DispatchWork.capabilities` and `.addCapabilities` have been in the core contract since at least 0.4.0 — `dispatch.d.ts` is byte-identical between 0.4.0 and 0.5.0 — but `fire()` forwarded eleven fields and neither of these, so the selection was unreachable from a plan. The consequence is not cosmetic. Without a per-item override, changing a worker's workspace substrate requires re-registering the subagent, mutating a registration shared by every plan that names it, so two runs against different workspace snapshots cannot coexist. Sharper: `resolveLatest` resolves per DISPATCH, not per run, so a re-registration mid-run can change worker identity between two items of the same run — and nothing afterwards reports it, because each manifest is internally consistent and the bundle still verifies intact. Follows the shape-guard posture its two neighbouring carriers already use: a non-array value is IGNORED rather than thrown, so one malformed field cannot fail an item. Spread conditionally for a reason specific to this field — a present-but-undefined `capabilities` reads as "replace the bound set with nothing" rather than "not specified", which would be a silent capability strip. TWO THINGS THE ISSUE ASKED FOR THAT TURNED OUT TO BE UNNECESSARY, both verified rather than assumed: 1. "Accept the field in validateRun" is a no-op. `validateRun` never references `inputs` at all (grep count 0); `WorkItem.inputs` is `Record<string, unknown>` and nothing constrains its keys. 2. Shape validation does not strip it. `tick.ts:180` runs `shape.inputSchema.safeParse(fireItem.inputs)` but checks only `.success` and discards `parsed.data`, so the unstripped `fireItem.inputs` is what reaches the executor. Had it used `parsed.data`, a plain `z.object` would have silently dropped the new key and the forward would have been a no-op for every shape-validated item. The audit trail needed no change: `DispatchExecutorManifest.capabilities` already records the resolved {name, contentHash} set per dispatch, sealed via `canonEntry`. Verified the omit-when-absent test BITES: making the spread unconditional makes it fail. It passes before the fix too, so without that check it would have been indistinguishable from a vacuous test. 818 orchestrator tests pass (103 files), `pnpm -r lint` and `pnpm -r typecheck` clean. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3a807e5 commit 370e6b6

2 files changed

Lines changed: 97 additions & 0 deletions

File tree

packages/pangolin-orchestrator/src/executors/dispatch.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,22 @@ export class DispatchExecutor implements Executor {
7373
const pipelineRef =
7474
typeof rawPipeline === 'string' && rawPipeline.length > 0 ? rawPipeline : undefined;
7575

76+
// Per-item capability selection. `capabilities` REPLACES the subagent's bound
77+
// set, `addCapabilities` augments it — the core contract has carried both since
78+
// 0.4.0; only this forward was missing, which made the selection unreachable
79+
// from a plan and forced a subagent re-registration to change workspace
80+
// substrate. Same shape-guard posture as the two carriers above: a non-array is
81+
// IGNORED, never thrown, so one malformed field cannot fail an item.
82+
//
83+
// Spread conditionally: a present-but-undefined `capabilities` would read as
84+
// "replace the bound set with nothing" rather than "not specified".
85+
const capabilities = Array.isArray(item.inputs.capabilities)
86+
? (item.inputs.capabilities as DispatchWork['capabilities'])
87+
: undefined;
88+
const addCapabilities = Array.isArray(item.inputs.addCapabilities)
89+
? (item.inputs.addCapabilities as DispatchWork['addCapabilities'])
90+
: undefined;
91+
7692
// Pre-fire model resolution (authorization side): resolve the subagent's
7793
// latest def blob and read its pinned model; fall back to the executor's
7894
// configured defaultModel. Best-effort — any failure here yields just the
@@ -96,6 +112,8 @@ export class DispatchExecutor implements Executor {
96112
workerImage: this.opts.workerImage,
97113
secrets: this.opts.secrets,
98114
...(requestedModel !== undefined ? { model: requestedModel } : {}),
115+
...(capabilities !== undefined ? { capabilities } : {}),
116+
...(addCapabilities !== undefined ? { addCapabilities } : {}),
99117
...(inputRefs && Object.keys(inputRefs).length ? { inputRefs } : {}),
100118
...(pipelineRef !== undefined ? { pipelineRef } : {}),
101119
...(ctx?.runId ? { trace: { traceId: ctx.runId, runId: ctx.runId, itemId: item.id } } : {}),

packages/pangolin-orchestrator/test/executors/dispatch.test.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,4 +1895,83 @@ describe('DispatchExecutor', () => {
18951895
finishedAt: new Date(1),
18961896
});
18971897
});
1898+
1899+
/**
1900+
* KNOWN-ISSUES 13a. `DispatchWork.capabilities` / `.addCapabilities` have been in
1901+
* the core contract since at least 0.4.0, but `fire()` never forwarded them, so
1902+
* the selection was unreachable from a plan. Without it, changing a worker's
1903+
* workspace substrate means re-registering the subagent — mutating a
1904+
* registration shared by every plan that names it, so two runs against different
1905+
* workspace snapshots cannot coexist.
1906+
*/
1907+
function capabilityHarness() {
1908+
const { compute, resolveExit } = makeDeferredCompute();
1909+
const storage = makeMemoryStorage();
1910+
storage.seed('s', 'subagent', 'ns', 'sha256:s', { name: 's' });
1911+
// A bare-string capability resolves through resolveLatest, so it must exist —
1912+
// otherwise the client throws "capability not found" and the assertion below
1913+
// would be measuring the wrong failure.
1914+
storage.seed('ws-snapshot-1', 'capability', 'ns', 'sha256:ws1', { name: 'ws-snapshot-1' });
1915+
storage.seed('extra-tool', 'capability', 'ns', 'sha256:xt', { name: 'extra-tool' });
1916+
const client = new PangolinClient({
1917+
namespace: 'ns',
1918+
compute: { default: compute },
1919+
credentials: { default: makeCredentials() },
1920+
storage,
1921+
targets: { prod: { compute: 'default', credentials: 'default' } },
1922+
});
1923+
const captured = captureDispatchFire(client);
1924+
const executor = new DispatchExecutor({ client, target: 'prod', workerImage: 'img' });
1925+
const settle = () =>
1926+
resolveExit({
1927+
exitCode: 0,
1928+
stdout: '',
1929+
stderr: '',
1930+
startedAt: new Date(0),
1931+
finishedAt: new Date(1),
1932+
});
1933+
return { executor, captured, settle };
1934+
}
1935+
1936+
it('forwards inputs.capabilities to the dispatched work', async () => {
1937+
const { executor, captured, settle } = capabilityHarness();
1938+
await executor.fire(
1939+
{ ...baseItem, inputs: { subagent: 's', capabilities: ['ws-snapshot-1'] } },
1940+
{ runId: 'r1', actor: 'human:x' },
1941+
);
1942+
expect(captured.work?.capabilities).toEqual(['ws-snapshot-1']);
1943+
settle();
1944+
});
1945+
1946+
it('forwards inputs.addCapabilities to the dispatched work', async () => {
1947+
const { executor, captured, settle } = capabilityHarness();
1948+
await executor.fire(
1949+
{ ...baseItem, inputs: { subagent: 's', addCapabilities: ['extra-tool'] } },
1950+
{ runId: 'r1', actor: 'human:x' },
1951+
);
1952+
expect(captured.work?.addCapabilities).toEqual(['extra-tool']);
1953+
settle();
1954+
});
1955+
1956+
it('omits both capability fields entirely when the item declares neither', async () => {
1957+
const { executor, captured, settle } = capabilityHarness();
1958+
await executor.fire(baseItem, { runId: 'r1', actor: 'human:x' });
1959+
// Absent, not `undefined` — the conditional-spread posture its neighbours use.
1960+
// A present-but-undefined key would replace the subagent's bound set with nothing.
1961+
expect('capabilities' in captured.work!).toBe(false);
1962+
expect('addCapabilities' in captured.work!).toBe(false);
1963+
settle();
1964+
});
1965+
1966+
it('ignores a non-array capabilities value rather than throwing', async () => {
1967+
const { executor, captured, settle } = capabilityHarness();
1968+
// Shape guard, not a trust guard — matches how inputRefs and pipeline treat
1969+
// malformed carriers: ignored, never thrown, so one bad field cannot fail an item.
1970+
await executor.fire(
1971+
{ ...baseItem, inputs: { subagent: 's', capabilities: 'not-an-array' } },
1972+
{ runId: 'r1', actor: 'human:x' },
1973+
);
1974+
expect('capabilities' in captured.work!).toBe(false);
1975+
settle();
1976+
});
18981977
});

0 commit comments

Comments
 (0)