Skip to content

Commit d4e57af

Browse files
committed
Phase 2 start: worker scaffold, exit-code contract, provider-derived env allowlist
The worker is high-criticality (trust boundary, credentials, money), so it is built in small verified slices. This first slice is the pure, security/money-critical core that needs no GitHub App or Redis to test. exit-code.mjs -- INT-RUNNER-EXIT-CODE-PROTOCOL from the worker's side. The container exit code becomes BullMQ's throw-vs-return, which IS CONST-RETRY-INFRA-ONLY: only exit 1 (infra) retries; 0 (agent ran, incl. "can't fix") and 2 (budget/policy) are determinate outcomes that return. An unknown code is retried-then-visible, never silently accepted as done. env-allowlist.mjs -- the container env is a CLOSED set, never a pass-through (no-broad-env-into-container is a BLOCKER: ANTHROPIC_OAUTH_TOKEN silently outranks ANTHROPIC_API_KEY, so one stray host var would redirect which credential every job spends). The provider key variable is DERIVED from pi's own table via findEnvKeys(provider, process.env), verified against the pinned pi-ai@0.80.7 tarball -- it returns the provider's key var names PRESENT in the worker env, in precedence order (OAuth before API key), and undefined when the provider is unconfigured, which becomes a pre-spend refusal. Deriving it means any of pi's ~30 providers works with no code change and the list cannot drift when pi adds one. A hand-copied table would be the reinvention no-reimplementing-pi forbids. Tests: exit-code runs everywhere (no deps); env-allowlist imports pi-ai so it skips below the 22.19 floor and CI sets PI_DISPATCH_REQUIRE_WORKER_TESTS=1 to make a skip a hard failure. Verified with pi-ai actually present: 5/5 pass, and the allowlist forwards only the provider key + declared vars, never a stray AWS_SECRET_ACCESS_KEY. CI paths and require flag extended to cover worker/. git-show materialisation of .pi/ (with the symlink/traversal guards sec-review flagged) and the BullMQ processor follow, informed by research in flight.
1 parent a55770f commit d4e57af

8 files changed

Lines changed: 2831 additions & 933 deletions

File tree

.github/workflows/pi-upgrade-check.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ on:
1818
push:
1919
paths:
2020
- "image/**"
21+
- "worker/**"
2122
- "guardrails/**"
2223
- "package.json"
2324
- ".github/workflows/pi-upgrade-check.yml"
2425
pull_request:
2526
paths:
2627
- "image/**"
28+
- "worker/**"
2729
- "guardrails/**"
2830
- "package.json"
2931
- ".github/workflows/pi-upgrade-check.yml"
@@ -78,9 +80,10 @@ jobs:
7880
# PI_DISPATCH_REQUIRE_LOADER_TESTS=1 turns a skip into a hard failure. A skipped assertion is
7981
# an UNVERIFIED assertion, and "skipped = pass" is precisely the reasoning that lets a
8082
# guardrail-less agent ship green.
81-
- name: Guardrails present, hostile AGENTS.md absent, exit codes correct
83+
- name: Contract tests -- guardrails, -nc, exit codes, env allowlist (all required)
8284
env:
8385
PI_DISPATCH_REQUIRE_LOADER_TESTS: "1"
86+
PI_DISPATCH_REQUIRE_WORKER_TESTS: "1"
8487
run: npm test
8588

8689
image:

package-lock.json

Lines changed: 2608 additions & 931 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,6 @@
1515
"panel"
1616
],
1717
"scripts": {
18-
"test": "node --test \"image/runner/test/*.test.mjs\""
18+
"test": "node --test \"image/runner/test/*.test.mjs\" \"worker/test/*.test.mjs\""
1919
}
2020
}

worker/package.json

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"name": "@pi-dispatch/worker",
3+
"version": "0.1.0",
4+
"private": true,
5+
"type": "module",
6+
"description": "BullMQ worker: drains the queue, mints scoped tokens, runs one container per job.",
7+
"license": "MIT",
8+
"main": "src/index.mjs",
9+
"engines": {
10+
"node": ">=22.19.0"
11+
},
12+
"scripts": {
13+
"test": "node --test \"test/*.test.mjs\""
14+
},
15+
"dependencies": {
16+
"@earendil-works/pi-ai": "0.80.7",
17+
"@octokit/auth-app": "8.2.0",
18+
"bullmq": "5.80.4"
19+
}
20+
}

worker/src/env-allowlist.mjs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { findEnvKeys } from "@earendil-works/pi-ai/compat";
2+
3+
/**
4+
* Build the EXACT environment a job container receives. Never a pass-through.
5+
*
6+
* `no-broad-env-into-container` is a BLOCKER, and for good reason: `ANTHROPIC_OAUTH_TOKEN`
7+
* silently outranks `ANTHROPIC_API_KEY`, so one stray host variable would redirect which
8+
* credential every job spends, with no error and no log line. So we forward a closed set.
9+
*
10+
* The provider key variable is DERIVED from pi's own table, not hardcoded. `findEnvKeys(provider,
11+
* env)` returns the provider's key variable names that are actually PRESENT in `env`, in
12+
* precedence order (OAuth before API key). Deriving it means:
13+
* - any of pi's ~30 providers works with no code change here;
14+
* - the list cannot drift when pi adds a provider (a hand-copied table would);
15+
* - `undefined` return === "this provider is not configured on this host" === refuse the job
16+
* BEFORE spending, rather than launch a container that will fail auth on the first call.
17+
*
18+
* `getApiKeyEnvVars` (the full candidate list) is intentionally NOT exported by pi; `findEnvKeys`
19+
* against our own process.env is the right tool anyway, because we only ever forward keys we have.
20+
*/
21+
export function providerKeyVars(provider, hostEnv) {
22+
return findEnvKeys(provider, hostEnv);
23+
}
24+
25+
/**
26+
* Assemble the container env. `hostEnv` is the worker's process.env; `job` carries the resolved
27+
* config and the per-job scoped token (GitHub-backed jobs only).
28+
*
29+
* Throws if the provider is not configured -- a deterministic misconfiguration the caller maps to
30+
* a pre-spend refusal, never a launched-then-failed container.
31+
*/
32+
export function buildContainerEnv({ provider, model, maxTurns, jobId, githubToken, hostEnv }) {
33+
const keyVars = providerKeyVars(provider, hostEnv);
34+
if (!keyVars || keyVars.length === 0) {
35+
const error = new Error(`provider ${provider} has no configured credential in the worker environment`);
36+
error.piDispatchConfig = true;
37+
throw error;
38+
}
39+
40+
const env = {
41+
PI_PROVIDER: provider,
42+
PI_MODEL: model,
43+
PI_MAX_TURNS: String(maxTurns),
44+
PI_JOB_ID: jobId,
45+
// Baked into the image, but harmless to restate; kept here so the container contract is
46+
// visible in one place. INT-CONTAINER-RUNTIME-CONTRACT.
47+
PLAYWRIGHT_BROWSERS_PATH: "/ms-playwright",
48+
PLAYWRIGHT_MCP_BROWSER: "chromium",
49+
PLAYWRIGHT_MCP_SANDBOX: "false",
50+
};
51+
52+
// The provider credential(s), by their real names, copied from the host by EXACT name. Passing
53+
// the value under pi's expected variable name is what lets pi's own auth resolution find it.
54+
for (const name of keyVars) {
55+
env[name] = hostEnv[name];
56+
}
57+
58+
// GitHub-backed jobs only. Local-folder jobs have no token (CONST-TOKEN-SCOPED-PER-JOB is
59+
// scoped to GitHub jobs). Absent token => absent variable, never an empty one.
60+
if (githubToken) env.GITHUB_TOKEN = githubToken;
61+
62+
return env;
63+
}

worker/src/exit-code.mjs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* INT-RUNNER-EXIT-CODE-PROTOCOL, worker side.
3+
*
4+
* The container's exit code is the ONLY channel telling "the agent ran and concluded something"
5+
* from "the container died". The worker turns that into BullMQ's throw-vs-return, which IS
6+
* CONST-RETRY-INFRA-ONLY: a thrown processor is retried, a returned one is not.
7+
*/
8+
export const EXIT_COMPLETED = 0; // agent ran, INCLUDING "I cannot fix this" -- a determinate success
9+
export const EXIT_INFRA = 1; // container died, network, provider 5xx/429 -- the only retryable class
10+
export const EXIT_POLICY = 2; // budget/turn cap/config -- a determinate refusal, never retried
11+
12+
/**
13+
* Decide whether the processor should RETURN (BullMQ records success, no retry) or THROW (BullMQ
14+
* retries per `attempts`). Returns `{ retry }`; the caller returns on false and throws on true.
15+
*
16+
* Only exit 1 is retryable. 0 and 2 are both determinate outcomes -- the agent's verdict (or our
17+
* own budget refusal) is the product, not a failure to paper over by paying for it again. An
18+
* unknown code is treated as infra: a runner that exits with something we do not recognise is a
19+
* runner we cannot reason about, and retrying-then-alerting beats silently accepting it as done.
20+
*/
21+
export function decideRetry(exitCode) {
22+
switch (exitCode) {
23+
case EXIT_COMPLETED:
24+
return { retry: false, outcome: "completed" };
25+
case EXIT_POLICY:
26+
return { retry: false, outcome: "policy" };
27+
case EXIT_INFRA:
28+
return { retry: true, outcome: "infra" };
29+
default:
30+
return { retry: true, outcome: `unknown-exit-${exitCode}` };
31+
}
32+
}

worker/test/env-allowlist.test.mjs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import assert from "node:assert/strict";
2+
import { test } from "node:test";
3+
4+
// env-allowlist imports @earendil-works/pi-ai (for findEnvKeys). That needs node >=22.19.0 and
5+
// installed deps, so it skips on a below-floor dev box and runs in CI, where
6+
// PI_DISPATCH_REQUIRE_WORKER_TESTS=1 turns a skip into a hard failure. A skipped security test is
7+
// an unverified one -- the same discipline as the runner's loader tests.
8+
let mod;
9+
let importError;
10+
try {
11+
mod = await import("../src/env-allowlist.mjs");
12+
} catch (error) {
13+
importError = error;
14+
}
15+
if (!mod && process.env.PI_DISPATCH_REQUIRE_WORKER_TESTS === "1") {
16+
throw new Error(`env-allowlist tests are REQUIRED here but pi-ai could not import.\n${importError}`);
17+
}
18+
const skip = mod ? false : `pi-ai not installed (node ${process.version} < 22.19.0); CI runs these`;
19+
const { buildContainerEnv, providerKeyVars } = mod ?? {};
20+
21+
const HOST = {
22+
ANTHROPIC_API_KEY: "sk-ant-real",
23+
OPENAI_API_KEY: "sk-openai-real",
24+
// The stray host variable no-broad-env-into-container exists to defend against:
25+
AWS_SECRET_ACCESS_KEY: "must-not-leak",
26+
HOME: "/root",
27+
PATH: "/usr/bin",
28+
};
29+
30+
test("derives the provider key var from the host env, in precedence order", { skip }, () => {
31+
assert.deepEqual(providerKeyVars("anthropic", HOST), ["ANTHROPIC_API_KEY"]);
32+
assert.deepEqual(providerKeyVars("openai", HOST), ["OPENAI_API_KEY"]);
33+
// OAuth outranks API key -- the array order is the precedence.
34+
assert.deepEqual(
35+
providerKeyVars("anthropic", { ...HOST, ANTHROPIC_OAUTH_TOKEN: "oauth" }),
36+
["ANTHROPIC_OAUTH_TOKEN", "ANTHROPIC_API_KEY"],
37+
);
38+
});
39+
40+
test("an unconfigured provider yields undefined (=> refuse before spend)", { skip }, () => {
41+
assert.equal(providerKeyVars("google", HOST), undefined);
42+
});
43+
44+
test("the container env is a CLOSED set: only the provider key, never the whole host", { skip }, () => {
45+
const env = buildContainerEnv({
46+
provider: "anthropic",
47+
model: "claude-x",
48+
maxTurns: 20,
49+
jobId: "abc",
50+
githubToken: "ghs_scoped",
51+
hostEnv: HOST,
52+
});
53+
assert.equal(env.ANTHROPIC_API_KEY, "sk-ant-real");
54+
assert.equal(env.GITHUB_TOKEN, "ghs_scoped");
55+
assert.equal(env.PI_PROVIDER, "anthropic");
56+
// The stray host secrets are NOT forwarded.
57+
assert.equal(env.AWS_SECRET_ACCESS_KEY, undefined);
58+
assert.equal(env.HOME, undefined);
59+
assert.equal(env.OPENAI_API_KEY, undefined); // wrong provider's key not forwarded either
60+
});
61+
62+
test("a local-folder job (no token) gets NO GITHUB_TOKEN var at all -- not an empty one", { skip }, () => {
63+
const env = buildContainerEnv({
64+
provider: "anthropic",
65+
model: "m",
66+
maxTurns: 5,
67+
jobId: "j",
68+
githubToken: undefined,
69+
hostEnv: HOST,
70+
});
71+
assert.ok(!("GITHUB_TOKEN" in env), "absent token must mean absent variable");
72+
});
73+
74+
test("an unconfigured provider throws a config-tagged error (=> pre-spend refusal)", { skip }, () => {
75+
assert.throws(
76+
() => buildContainerEnv({ provider: "google", model: "m", maxTurns: 5, jobId: "j", hostEnv: HOST }),
77+
(e) => e.piDispatchConfig === true,
78+
);
79+
});

worker/test/exit-code.test.mjs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import assert from "node:assert/strict";
2+
import { test } from "node:test";
3+
import { decideRetry, EXIT_COMPLETED, EXIT_INFRA, EXIT_POLICY } from "../src/exit-code.mjs";
4+
5+
// INT-RUNNER-EXIT-CODE-PROTOCOL / CONST-RETRY-INFRA-ONLY. The whole point: only infra retries.
6+
7+
test("exit 0 (agent ran, incl. 'can't fix') is success, NOT retried", () => {
8+
assert.deepEqual(decideRetry(EXIT_COMPLETED), { retry: false, outcome: "completed" });
9+
});
10+
11+
test("exit 2 (budget/policy) is determinate, NOT retried -- paying twice for a refusal is the bug", () => {
12+
assert.equal(decideRetry(EXIT_POLICY).retry, false);
13+
});
14+
15+
test("exit 1 (infra) is the ONLY retryable class", () => {
16+
assert.equal(decideRetry(EXIT_INFRA).retry, true);
17+
});
18+
19+
test("an unknown exit code is retried-then-visible, not silently accepted as done", () => {
20+
// A runner we can't reason about must not be recorded as a clean success.
21+
const d = decideRetry(137); // SIGKILL, e.g. OOM
22+
assert.equal(d.retry, true);
23+
assert.match(d.outcome, /unknown-exit-137/);
24+
});

0 commit comments

Comments
 (0)