From 377ea1990984557429bf7f1f549a25be246a1834 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:05:46 -0700 Subject: [PATCH 1/5] feat(agent-runtime): staging hosted-runtime worker env + walk skips hosted steps until it's live MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - wrangler.toml: add [env.staging] — a separate worker (commonly-agent-runtime-staging) with its own COMMONLY_API_URL (api-staging.commonly.me) and its own admin token held in staging Secret Manager. A named Cloudflare env inherits neither bindings nor vars, so the AGENT durable-object binding and vars are repeated; the top-level migration applies to every env. - values-staging.yaml: set backend.env.hostedRuntimeUrl to the staging worker URL, which renders the hosted-runtime ExternalSecret (pulling the commonly-staging- prefixed admin token) so the backend stops returning 503 on hosted provision. - first-user-walk: a 503 hosted_runtime_unconfigured on provision now SKIPS the hosted-install / hosted-reply steps instead of failing them, so the staging walk stays incomplete (not red) until the worker is deployed. Adds a SkipStep result kind + a test for the skip path. The worker deploy and token sync are operator/CI-side (the Cloudflare account is operator-private): wrangler deploy --env staging, with the admin token piped from Secret Manager into wrangler secret put so it is never typed by hand. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015AKgpNr8tFMMj8aFvFPUdA --- k8s/helm/commonly/values-staging.yaml | 7 +++++++ scripts/first-user-walk.mjs | 30 ++++++++++++++++++++++++++- scripts/first-user-walk.test.mjs | 29 +++++++++++++++++++++++++- workers/agent-runtime/wrangler.toml | 18 ++++++++++++++++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/k8s/helm/commonly/values-staging.yaml b/k8s/helm/commonly/values-staging.yaml index 0df2880a8e..70c113cdfd 100644 --- a/k8s/helm/commonly/values-staging.yaml +++ b/k8s/helm/commonly/values-staging.yaml @@ -204,6 +204,13 @@ backend: # and accepting a staging consent would have sent a staging authorization code to prod. # Caught on the staging Atlassian walk 2026-10-23. backendUrl: "https://api-staging.commonly.me" + # ADR-023 W2 hosted runtime, staging worker (added 2026-11-01). Setting this + # renders the hosted-runtime ExternalSecret (templates/secrets/hosted-runtime.yaml), + # which pulls commonly-staging-hosted-runtime-admin-token from staging SM, so the + # backend stops returning 503 hosted_runtime_unconfigured on provision. The worker + # is the separate `commonly-agent-runtime-staging` Cloudflare env (wrangler.toml); + # until it is deployed the first-user-walk skips the hosted steps rather than failing. + hostedRuntimeUrl: "https://commonly-agent-runtime-staging.commonlyai.workers.dev" # Staging backend runs on the SPOT pool at a fraction of prod's requests, so it fits # the nodes we already have (spot-pool sits at single-digit % CPU) without growing a # pool. Prod backend stays on dev-pool because it holds session state a spot diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs index 14e43a08e3..2be9b1ce2a 100644 --- a/scripts/first-user-walk.mjs +++ b/scripts/first-user-walk.mjs @@ -59,6 +59,12 @@ const NOT_COVERED = 'Bring your own connect step is not implemented yet'; const expect = (cond, msg) => { if (!cond) throw new Error(msg); }; +// A step can decline to run rather than fail. runStep records a SkipStep as +// result 'skipped' and does not rethrow, so the run continues and stays +// incomplete rather than going red. Used when the hosted runtime worker is not +// configured on this instance (staging before its worker is deployed). +class SkipStep extends Error { constructor(message) { super(message); this.skip = true; } } + /** * One walk run. Every dependency is injectable so the production path can be * exercised under test without the network or the clock. @@ -143,6 +149,12 @@ export const runWalk = async ({ log(`PASS ${name} (${now() - started} ms)`); return value; } catch (caught) { + if (caught instanceof SkipStep) { + const reason = redact(caught.message); + results.push({ name, result: 'skipped', ms: now() - started, reason }); + log(`SKIP ${name} (${now() - started} ms): ${reason}`); + return undefined; + } const reason = redact(caught.message); results.push({ name, result: 'fail', ms: now() - started, reason }); error(`FAIL ${name} (${now() - started} ms): ${reason}`); @@ -150,6 +162,14 @@ export const runWalk = async ({ } }; + // Record a downstream step as skipped without running it — used when an + // earlier skipped step means this one cannot run (hosted-reply needs the + // provision that hosted-install skipped). + const skipStep = (name, reason) => { + results.push({ name, result: 'skipped', ms: 0, reason }); + log(`SKIP ${name}: ${reason}`); + }; + // Per-run manifest: ids only, outside the repo, for SRE's admin purge of what // the walk's own token cannot remove (Scout, the hosted agent's User). It holds // no password, token or email. It is named by the run stamp, not the username. @@ -324,6 +344,7 @@ export const runWalk = async ({ return created; }); + let hostedSkipped = false; await runStep('hosted-install', async () => { manifest.agentName = agentName; writeManifest(); @@ -333,10 +354,17 @@ export const runWalk = async ({ }); expect(r.status < 400, `install ${r.status}: ${JSON.stringify(r.data)}`); const p = await api('POST', '/api/hosted/provision', { token, body: { agentName } }); + // No hosted worker on this instance (staging before its worker is deployed): + // the backend returns 503 hosted_runtime_unconfigured. That is a missing + // worker, not a product defect, so the hosted steps are SKIPPED rather + // than failed — once the staging worker is live the 503 becomes a 200 and + // they run for real. + if (p.status === 503) { hostedSkipped = true; throw new SkipStep('no staging worker (hosted_runtime_unconfigured)'); } expect(p.status === 200 && p.data?.provisioned, `provision ${p.status}: ${JSON.stringify(p.data)}`); }); - await runStep('hosted-reply', async () => { + if (hostedSkipped) skipStep('hosted-reply', 'no staging worker (hosted_runtime_unconfigured)'); + else await runStep('hosted-reply', async () => { const m = await api('POST', `/api/messages/${pod._id}`, { token, body: { content: `@${agentName} walk check: reply with one short sentence containing KINGFISHER.` }, diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs index 931835ff20..827f121bdb 100644 --- a/scripts/first-user-walk.test.mjs +++ b/scripts/first-user-walk.test.mjs @@ -20,8 +20,11 @@ const SHARED_EMAIL = 'walk-shared@commonly.test'; // password the password the fake account accepts // echoRegister a refused register echoes the request body back, as a bad error would // seeded the account already exists (the shared staging sign-in account) +// hostedProvision status of POST /api/hosted/provision (503 = no worker on +// this instance, so the hosted steps skip rather than fail) const fakeBackend = ({ preview = 200, create = 'ok', password = PASSWORD, echoRegister = false, seeded = false, + hostedProvision = 200, } = {}) => { const state = { accountExists: seeded, pods: [], lastAgent: null, calls: [], loginEmails: [] }; const reply = (status, data) => ({ status, json: async () => data }); @@ -69,7 +72,11 @@ const fakeBackend = ({ } } if (method === 'POST' && p === '/api/registry/install') return reply(200, {}); - if (method === 'POST' && p === '/api/hosted/provision') return reply(200, { provisioned: true }); + if (method === 'POST' && p === '/api/hosted/provision') { + return hostedProvision === 200 + ? reply(200, { provisioned: true }) + : reply(hostedProvision, { error: 'hosted_runtime_unconfigured' }); + } if (method === 'POST' && p === '/api/hosted/deprovision') return reply(404, {}); if (method === 'DELETE' && p.startsWith('/api/registry/agents/')) return reply(404, {}); if (method === 'POST' && /^\/api\/messages\//.test(p)) { @@ -135,6 +142,26 @@ test('a clean run writes the machine-readable signal: every covered step passes, assert.equal(state.pods.length, 0); }); +test('no hosted worker (provision 503): the hosted steps are skipped, not failed, and overall stays incomplete', async () => { + const dirsFor = dirs(); + const { fetchImpl, state } = fakeBackend({ hostedProvision: 503 }); + const { exitCode, result } = await run(fetchImpl, dirsFor); + + // Incomplete (byo not-covered), never fail: a missing worker is not a defect. + assert.equal(exitCode, 2); + assert.equal(result.overall, 'incomplete'); + + const byName = Object.fromEntries(result.steps.map((s) => [s.name, s.result])); + assert.equal(byName['hosted-install'], 'skipped'); + assert.equal(byName['hosted-reply'], 'skipped'); + const skipReason = result.steps.find((s) => s.name === 'hosted-install').reason; + assert.match(skipReason, /no staging worker/); + // No fail anywhere; the other covered steps still pass. + assert.ok(result.steps.every((s) => s.result !== 'fail')); + // Cleanup still runs and the account is removed. + assert.equal(state.accountExists, false); +}); + test('a forced step failure writes overall fail, names the step, and exits 1', async () => { const dirsFor = dirs(); const { fetchImpl } = fakeBackend({ preview: 500 }); diff --git a/workers/agent-runtime/wrangler.toml b/workers/agent-runtime/wrangler.toml index d4fb657030..ea953b43fc 100644 --- a/workers/agent-runtime/wrangler.toml +++ b/workers/agent-runtime/wrangler.toml @@ -20,3 +20,21 @@ new_sqlite_classes = ["AgentRuntimeDO"] COMMONLY_API_URL = "https://api.commonly.me" # Provider decision 2026-08-29 (Sam): DeepSeek. Key is a secret (DEEPSEEK_API_KEY). MODEL_PROVIDER = "deepseek" + +# Staging env (added 2026-11-01): a separate worker with its own URL and its +# own admin token (staging SM key commonly-staging-hosted-runtime-admin-token), +# serving the staging Commonly instance at api-staging.commonly.me. A named +# Cloudflare env does not inherit bindings or vars, so they are repeated here; the +# top-level [[migrations]] applies to every env. Deploy is operator/CI-side (see +# the header) — `wrangler deploy --env staging` plus a `wrangler secret put` that +# pipes the token from staging SM, so the token is never typed by hand. +[env.staging] +name = "commonly-agent-runtime-staging" + +[[env.staging.durable_objects.bindings]] +name = "AGENT" +class_name = "AgentRuntimeDO" + +[env.staging.vars] +COMMONLY_API_URL = "https://api-staging.commonly.me" +MODEL_PROVIDER = "deepseek" From 80f2e927ed6b8222fa4b45771cec97cd0e72d2e0 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:16:52 -0700 Subject: [PATCH 2/5] fix(agent-runtime): walk skips hosted steps on both not-ready codes (502 + 503) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review catch: once values-staging sets hostedRuntimeUrl the backend is CONFIGURED, so an undeployed worker returns 502 hosted_runtime_unreachable, not 503 — the 503-only skip would have gone red in the exact interim window it was meant to keep green (the backend maps every worker fetch/non-ok to 502 and discards the upstream status; only the !isConfigured path returns 503). Key the skip on the backend error CODE (hosted_runtime_unconfigured | hosted_runtime_ unreachable) so a genuine 500 (token_mint_failed, unexpected error) still fails. Tests now cover 503-skip, 502-skip, and 500-still-fails; the values comment that claimed the URL "stops 503-ing" is corrected to the 502 reality. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015AKgpNr8tFMMj8aFvFPUdA --- k8s/helm/commonly/values-staging.yaml | 10 +++-- scripts/first-user-walk.mjs | 24 +++++++---- scripts/first-user-walk.test.mjs | 60 ++++++++++++++++++--------- 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/k8s/helm/commonly/values-staging.yaml b/k8s/helm/commonly/values-staging.yaml index 70c113cdfd..2e47ad00e8 100644 --- a/k8s/helm/commonly/values-staging.yaml +++ b/k8s/helm/commonly/values-staging.yaml @@ -206,10 +206,12 @@ backend: backendUrl: "https://api-staging.commonly.me" # ADR-023 W2 hosted runtime, staging worker (added 2026-11-01). Setting this # renders the hosted-runtime ExternalSecret (templates/secrets/hosted-runtime.yaml), - # which pulls commonly-staging-hosted-runtime-admin-token from staging SM, so the - # backend stops returning 503 hosted_runtime_unconfigured on provision. The worker - # is the separate `commonly-agent-runtime-staging` Cloudflare env (wrangler.toml); - # until it is deployed the first-user-walk skips the hosted steps rather than failing. + # which pulls commonly-staging-hosted-runtime-admin-token from staging SM. With the + # URL and token both set the backend is CONFIGURED, so provision no longer returns + # 503 unconfigured; until the worker itself is deployed it returns 502 + # hosted_runtime_unreachable instead. The worker is the separate + # `commonly-agent-runtime-staging` Cloudflare env (wrangler.toml); until it is live + # the first-user-walk treats both not-ready codes as a skip, not a failure. hostedRuntimeUrl: "https://commonly-agent-runtime-staging.commonlyai.workers.dev" # Staging backend runs on the SPOT pool at a fraction of prod's requests, so it fits # the nodes we already have (spot-pool sits at single-digit % CPU) without growing a diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs index 2be9b1ce2a..67a9b7ddc4 100644 --- a/scripts/first-user-walk.mjs +++ b/scripts/first-user-walk.mjs @@ -62,7 +62,8 @@ const expect = (cond, msg) => { if (!cond) throw new Error(msg); }; // A step can decline to run rather than fail. runStep records a SkipStep as // result 'skipped' and does not rethrow, so the run continues and stays // incomplete rather than going red. Used when the hosted runtime worker is not -// configured on this instance (staging before its worker is deployed). +// ready on this instance — staging before its worker is deployed, where +// provision returns 503 hosted_runtime_unconfigured or 502 hosted_runtime_unreachable. class SkipStep extends Error { constructor(message) { super(message); this.skip = true; } } /** @@ -354,16 +355,23 @@ export const runWalk = async ({ }); expect(r.status < 400, `install ${r.status}: ${JSON.stringify(r.data)}`); const p = await api('POST', '/api/hosted/provision', { token, body: { agentName } }); - // No hosted worker on this instance (staging before its worker is deployed): - // the backend returns 503 hosted_runtime_unconfigured. That is a missing - // worker, not a product defect, so the hosted steps are SKIPPED rather - // than failed — once the staging worker is live the 503 becomes a 200 and - // they run for real. - if (p.status === 503) { hostedSkipped = true; throw new SkipStep('no staging worker (hosted_runtime_unconfigured)'); } + // The hosted runtime worker is not ready on this instance (staging before + // its worker is deployed). Provision then comes back not-ready in one of two + // shapes: 503 hosted_runtime_unconfigured (URL or admin token unset) or 502 + // hosted_runtime_unreachable (configured, but the worker is not deployed yet — + // the backend maps every worker fetch/non-ok to 502, see hostedRuntimeService). + // Either is a missing worker, not a product defect, so the hosted steps are + // SKIPPED, not failed. Keyed on the error CODE, so a genuine 500 + // (token_mint_failed, unexpected error) still fails. Once the worker is live + // provision returns 200 and the steps run for real. This walk is advisory — + // staging-smoke, not it, gates the deploy. + const notReady = p.status >= 400 + && (p.data?.code === 'hosted_runtime_unconfigured' || p.data?.code === 'hosted_runtime_unreachable'); + if (notReady) { hostedSkipped = true; throw new SkipStep(`no staging worker (${p.data.code})`); } expect(p.status === 200 && p.data?.provisioned, `provision ${p.status}: ${JSON.stringify(p.data)}`); }); - if (hostedSkipped) skipStep('hosted-reply', 'no staging worker (hosted_runtime_unconfigured)'); + if (hostedSkipped) skipStep('hosted-reply', 'no staging worker (hosted runtime not ready)'); else await runStep('hosted-reply', async () => { const m = await api('POST', `/api/messages/${pod._id}`, { token, diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs index 827f121bdb..62e586dbd8 100644 --- a/scripts/first-user-walk.test.mjs +++ b/scripts/first-user-walk.test.mjs @@ -20,8 +20,9 @@ const SHARED_EMAIL = 'walk-shared@commonly.test'; // password the password the fake account accepts // echoRegister a refused register echoes the request body back, as a bad error would // seeded the account already exists (the shared staging sign-in account) -// hostedProvision status of POST /api/hosted/provision (503 = no worker on -// this instance, so the hosted steps skip rather than fail) +// hostedProvision status of POST /api/hosted/provision. 503 (unconfigured) and +// 502 (unreachable) are "no worker yet" -> the hosted steps skip; +// any other 5xx is a real error with no not-ready code -> they fail. const fakeBackend = ({ preview = 200, create = 'ok', password = PASSWORD, echoRegister = false, seeded = false, hostedProvision = 200, @@ -73,9 +74,13 @@ const fakeBackend = ({ } if (method === 'POST' && p === '/api/registry/install') return reply(200, {}); if (method === 'POST' && p === '/api/hosted/provision') { - return hostedProvision === 200 - ? reply(200, { provisioned: true }) - : reply(hostedProvision, { error: 'hosted_runtime_unconfigured' }); + // The backend's real shapes: 200 provisioned; 503 hosted_runtime_unconfigured + // (URL/token unset); 502 hosted_runtime_unreachable (worker not deployed). Any + // other 5xx is a genuine error with NO not-ready code, which must still fail. + if (hostedProvision === 200) return reply(200, { provisioned: true }); + if (hostedProvision === 503) return reply(503, { code: 'hosted_runtime_unconfigured', message: 'not ready' }); + if (hostedProvision === 502) return reply(502, { code: 'hosted_runtime_unreachable', message: 'not ready' }); + return reply(hostedProvision, { message: 'Hosted runtime request failed' }); } if (method === 'POST' && p === '/api/hosted/deprovision') return reply(404, {}); if (method === 'DELETE' && p.startsWith('/api/registry/agents/')) return reply(404, {}); @@ -142,24 +147,39 @@ test('a clean run writes the machine-readable signal: every covered step passes, assert.equal(state.pods.length, 0); }); -test('no hosted worker (provision 503): the hosted steps are skipped, not failed, and overall stays incomplete', async () => { +// Both not-ready shapes skip: 503 (URL/token unset) and 502 (configured but the +// worker is not deployed — the state values-staging.yaml actually creates before +// the operator runs `wrangler deploy --env staging`). Neither may go red. +for (const status of [503, 502]) { + test(`no hosted worker (provision ${status}): the hosted steps are skipped, not failed, and overall stays incomplete`, async () => { + const dirsFor = dirs(); + const { fetchImpl, state } = fakeBackend({ hostedProvision: status }); + const { exitCode, result } = await run(fetchImpl, dirsFor); + + // Incomplete (byo not-covered), never fail: a not-ready worker is not a defect. + assert.equal(exitCode, 2); + assert.equal(result.overall, 'incomplete'); + + const byName = Object.fromEntries(result.steps.map((s) => [s.name, s.result])); + assert.equal(byName['hosted-install'], 'skipped'); + assert.equal(byName['hosted-reply'], 'skipped'); + const skipReason = result.steps.find((s) => s.name === 'hosted-install').reason; + assert.match(skipReason, /no staging worker/); + // No fail anywhere; the other covered steps still pass. + assert.ok(result.steps.every((s) => s.result !== 'fail')); + // Cleanup still runs and the account is removed. + assert.equal(state.accountExists, false); + }); +} + +test('a real provision error (500, no not-ready code) still FAILS — the skip never masks a defect', async () => { const dirsFor = dirs(); - const { fetchImpl, state } = fakeBackend({ hostedProvision: 503 }); + const { fetchImpl } = fakeBackend({ hostedProvision: 500 }); const { exitCode, result } = await run(fetchImpl, dirsFor); - // Incomplete (byo not-covered), never fail: a missing worker is not a defect. - assert.equal(exitCode, 2); - assert.equal(result.overall, 'incomplete'); - - const byName = Object.fromEntries(result.steps.map((s) => [s.name, s.result])); - assert.equal(byName['hosted-install'], 'skipped'); - assert.equal(byName['hosted-reply'], 'skipped'); - const skipReason = result.steps.find((s) => s.name === 'hosted-install').reason; - assert.match(skipReason, /no staging worker/); - // No fail anywhere; the other covered steps still pass. - assert.ok(result.steps.every((s) => s.result !== 'fail')); - // Cleanup still runs and the account is removed. - assert.equal(state.accountExists, false); + assert.equal(exitCode, 1); + assert.equal(result.overall, 'fail'); + assert.equal(result.steps.find((s) => s.name === 'hosted-install').result, 'fail'); }); test('a forced step failure writes overall fail, names the step, and exits 1', async () => { From 7962117dd2c556946d981a1d62ee1a46818d081d Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:24:56 -0700 Subject: [PATCH 3/5] fix(agent-runtime): scope hosted-step skip to staging, keep cleanup clean, document both worker secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addressing the CodeRabbit review on #3177: - Skip scoped to the STAGING run (mode 'signin', the shared account on api-staging). On prod (signin-prod) and signup a not-ready provision is a genuine outage and FAILS — never hidden as a skip. (CodeRabbit: "this branch also runs against production" / "without hiding genuine worker outages".) - cleanup: a not-ready deprovision (503 unconfigured / 502 unreachable, by code) on a staging run is no longer recorded as a survivor — provisioning was skipped, so nothing leaked — which previously flipped the whole run to fail. The test's fake deprovision now returns the not-ready code so this path is actually exercised. - wrangler.toml: document that a named Cloudflare env inherits no secrets, so the staging worker needs BOTH set per-env: RUNTIME_ADMIN_TOKEN (gates provision) and DEEPSEEK_API_KEY (MODEL_PROVIDER=deepseek; a turn throws without it). - Tests: staging 503/502 skip + cleanup-clean, PROD 502 FAILS, 500 FAILS. 45/45. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015AKgpNr8tFMMj8aFvFPUdA --- scripts/first-user-walk-cleanup.mjs | 13 +++++-- scripts/first-user-walk.mjs | 28 ++++++++------ scripts/first-user-walk.test.mjs | 58 +++++++++++++++++++++-------- workers/agent-runtime/wrangler.toml | 18 +++++---- 4 files changed, 80 insertions(+), 37 deletions(-) diff --git a/scripts/first-user-walk-cleanup.mjs b/scripts/first-user-walk-cleanup.mjs index 1b5e25ef67..e9d6d38d56 100644 --- a/scripts/first-user-walk-cleanup.mjs +++ b/scripts/first-user-walk-cleanup.mjs @@ -58,7 +58,7 @@ export const recoverRunPods = async ({ api, token, userId, podName, podIds }) => // `deleteAccount: false` is the shared staging sign-in account: the run removes // only the pods it recorded, never pods that account owns from elsewhere, and // never the account itself. -export const cleanupRun = async ({ api, token, password, email, userId, agentName, podIds = [], onBotIds = () => {}, deleteAccount = true }) => { +export const cleanupRun = async ({ api, token, password, email, userId, agentName, podIds = [], onBotIds = () => {}, deleteAccount = true, hostedOptional = false }) => { const survivors = []; const botIds = new Set(); @@ -69,9 +69,16 @@ export const cleanupRun = async ({ api, token, password, email, userId, agentNam }; // Stop the runtime before the seat goes: uninstalling the seat does not stop - // the hosted worker. 404 means there was no hosted install to stop. + // the hosted worker. 404 means there was no hosted install to stop. On a staging + // run where provisioning was skipped (hostedOptional), the backend answers + // deprovision with the same not-ready code (503 unconfigured / 502 unreachable): + // nothing was provisioned, so that is not a leaked runtime and must not become a + // survivor that reddens the run. On prod the agent WAS provisioned, so a failed + // deprovision stays a survivor. const stop = await api('POST', '/api/hosted/deprovision', { token, body: { agentName } }); - if (stop.status >= 300 && stop.status !== 404) survivors.push(`hosted:${agentName} (deprovision ${stop.status})`); + const stopNotReady = hostedOptional + && (stop.data?.code === 'hosted_runtime_unconfigured' || stop.data?.code === 'hosted_runtime_unreachable'); + if (stop.status >= 300 && stop.status !== 404 && !stopNotReady) survivors.push(`hosted:${agentName} (deprovision ${stop.status})`); let owned = []; if (deleteAccount) { diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs index 67a9b7ddc4..6aad5827e5 100644 --- a/scripts/first-user-walk.mjs +++ b/scripts/first-user-walk.mjs @@ -219,6 +219,10 @@ export const runWalk = async ({ agentName, podIds: manifest.podIds, deleteAccount: walkMode.deleteAccount, + // Staging run only: a not-ready deprovision (503 unconfigured / 502 unreachable) + // is not a leaked agent — provisioning was skipped, nothing was created — so it + // must not become a survivor and redden the run. On prod it stays a survivor. + hostedOptional: walkMode.mode === 'signin', // Persist each pod's bot ids before that pod is deleted, so a throw // later in the loop still leaves the ids SRE needs. onBotIds: (podId, bots) => { @@ -345,6 +349,13 @@ export const runWalk = async ({ return created; }); + // The not-ready skip is scoped to the STAGING run (mode 'signin', the shared + // account on api-staging), which is the only place the worker may legitimately + // be undeployed during rollout. On prod (signin-prod) and on signup the worker + // is expected to be live, so a not-ready provision there is a genuine outage and + // must FAIL — never be hidden as a skip. This walk is advisory; staging-smoke, + // not it, gates the deploy. + const hostedOptional = walkMode.mode === 'signin'; let hostedSkipped = false; await runStep('hosted-install', async () => { manifest.agentName = agentName; @@ -355,19 +366,14 @@ export const runWalk = async ({ }); expect(r.status < 400, `install ${r.status}: ${JSON.stringify(r.data)}`); const p = await api('POST', '/api/hosted/provision', { token, body: { agentName } }); - // The hosted runtime worker is not ready on this instance (staging before - // its worker is deployed). Provision then comes back not-ready in one of two - // shapes: 503 hosted_runtime_unconfigured (URL or admin token unset) or 502 - // hosted_runtime_unreachable (configured, but the worker is not deployed yet — - // the backend maps every worker fetch/non-ok to 502, see hostedRuntimeService). - // Either is a missing worker, not a product defect, so the hosted steps are - // SKIPPED, not failed. Keyed on the error CODE, so a genuine 500 - // (token_mint_failed, unexpected error) still fails. Once the worker is live - // provision returns 200 and the steps run for real. This walk is advisory — - // staging-smoke, not it, gates the deploy. + // Not-ready comes back in two shapes: 503 hosted_runtime_unconfigured (URL or + // admin token unset) or 502 hosted_runtime_unreachable (configured, but the + // worker is not deployed yet — the backend maps every worker fetch/non-ok to + // 502, see hostedRuntimeService). Keyed on the error CODE so a 503/502 from a + // proxy without that code, or a genuine 500 (token_mint_failed), still fails. const notReady = p.status >= 400 && (p.data?.code === 'hosted_runtime_unconfigured' || p.data?.code === 'hosted_runtime_unreachable'); - if (notReady) { hostedSkipped = true; throw new SkipStep(`no staging worker (${p.data.code})`); } + if (hostedOptional && notReady) { hostedSkipped = true; throw new SkipStep(`no staging worker (${p.data.code})`); } expect(p.status === 200 && p.data?.provisioned, `provision ${p.status}: ${JSON.stringify(p.data)}`); }); diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs index 62e586dbd8..fa3aa729c0 100644 --- a/scripts/first-user-walk.test.mjs +++ b/scripts/first-user-walk.test.mjs @@ -20,9 +20,10 @@ const SHARED_EMAIL = 'walk-shared@commonly.test'; // password the password the fake account accepts // echoRegister a refused register echoes the request body back, as a bad error would // seeded the account already exists (the shared staging sign-in account) -// hostedProvision status of POST /api/hosted/provision. 503 (unconfigured) and -// 502 (unreachable) are "no worker yet" -> the hosted steps skip; -// any other 5xx is a real error with no not-ready code -> they fail. +// hostedProvision status of POST /api/hosted/provision (and the matching +// deprovision). 503 (unconfigured) and 502 (unreachable) are +// "no worker yet"; on a STAGING run the hosted steps skip, off +// staging they fail. Any other 5xx has no not-ready code -> fail. const fakeBackend = ({ preview = 200, create = 'ok', password = PASSWORD, echoRegister = false, seeded = false, hostedProvision = 200, @@ -82,7 +83,14 @@ const fakeBackend = ({ if (hostedProvision === 502) return reply(502, { code: 'hosted_runtime_unreachable', message: 'not ready' }); return reply(hostedProvision, { message: 'Hosted runtime request failed' }); } - if (method === 'POST' && p === '/api/hosted/deprovision') return reply(404, {}); + if (method === 'POST' && p === '/api/hosted/deprovision') { + // When provision was not-ready, deprovision answers with the same not-ready + // code (the backend is unconfigured/unreachable for both) — cleanup must not + // treat that as a survivor on a staging run. + if (hostedProvision === 503) return reply(503, { code: 'hosted_runtime_unconfigured' }); + if (hostedProvision === 502) return reply(502, { code: 'hosted_runtime_unreachable' }); + return reply(404, {}); + } if (method === 'DELETE' && p.startsWith('/api/registry/agents/')) return reply(404, {}); if (method === 'POST' && /^\/api\/messages\//.test(p)) { state.lastAgent = (body.content.match(/@(\S+) walk check/) || [])[1] || null; @@ -147,31 +155,49 @@ test('a clean run writes the machine-readable signal: every covered step passes, assert.equal(state.pods.length, 0); }); -// Both not-ready shapes skip: 503 (URL/token unset) and 502 (configured but the -// worker is not deployed — the state values-staging.yaml actually creates before -// the operator runs `wrangler deploy --env staging`). Neither may go red. +// STAGING run only: both not-ready shapes skip — 503 (URL/token unset) and 502 +// (configured but the worker is not deployed, the state values-staging.yaml creates +// before the operator runs `wrangler deploy --env staging`). The run must stay +// incomplete, and the matching not-ready deprovision must NOT redden cleanup. for (const status of [503, 502]) { - test(`no hosted worker (provision ${status}): the hosted steps are skipped, not failed, and overall stays incomplete`, async () => { + test(`staging: no hosted worker (provision ${status}) skips the hosted steps, keeps cleanup clean, overall incomplete`, async () => { const dirsFor = dirs(); - const { fetchImpl, state } = fakeBackend({ hostedProvision: status }); - const { exitCode, result } = await run(fetchImpl, dirsFor); + const { fetchImpl, state } = fakeBackend({ seeded: true, hostedProvision: status }); + const { exitCode, result } = await run(fetchImpl, dirsFor, { + base: 'https://api-staging.commonly.me', + env: { COMMONLY_WALK_EMAIL: SHARED_EMAIL }, + }); - // Incomplete (byo not-covered), never fail: a not-ready worker is not a defect. + assert.equal(result.mode, 'signin'); assert.equal(exitCode, 2); assert.equal(result.overall, 'incomplete'); const byName = Object.fromEntries(result.steps.map((s) => [s.name, s.result])); assert.equal(byName['hosted-install'], 'skipped'); assert.equal(byName['hosted-reply'], 'skipped'); - const skipReason = result.steps.find((s) => s.name === 'hosted-install').reason; - assert.match(skipReason, /no staging worker/); - // No fail anywhere; the other covered steps still pass. + assert.match(result.steps.find((s) => s.name === 'hosted-install').reason, /no staging worker/); assert.ok(result.steps.every((s) => s.result !== 'fail')); - // Cleanup still runs and the account is removed. - assert.equal(state.accountExists, false); + // The not-ready deprovision did NOT become a survivor; cleanup stays clean, + // and the shared staging account is preserved. + assert.equal(result.cleanup.ok, true); + assert.equal(state.accountExists, true); }); } +test('PROD: a not-ready hosted provision (502) FAILS — the staging skip is never applied off staging', async () => { + const dirsFor = dirs(); + const { fetchImpl } = fakeBackend({ seeded: true, hostedProvision: 502 }); + const { exitCode, result } = await run(fetchImpl, dirsFor, { + base: 'https://api.commonly.me', + env: { COMMONLY_WALK_EMAIL: 'walk-test@commonly.test' }, + }); + + assert.equal(result.mode, 'signin-prod'); + assert.equal(exitCode, 1); + assert.equal(result.overall, 'fail'); + assert.equal(result.steps.find((s) => s.name === 'hosted-install').result, 'fail'); +}); + test('a real provision error (500, no not-ready code) still FAILS — the skip never masks a defect', async () => { const dirsFor = dirs(); const { fetchImpl } = fakeBackend({ hostedProvision: 500 }); diff --git a/workers/agent-runtime/wrangler.toml b/workers/agent-runtime/wrangler.toml index ea953b43fc..cda417ce5c 100644 --- a/workers/agent-runtime/wrangler.toml +++ b/workers/agent-runtime/wrangler.toml @@ -21,13 +21,17 @@ COMMONLY_API_URL = "https://api.commonly.me" # Provider decision 2026-08-29 (Sam): DeepSeek. Key is a secret (DEEPSEEK_API_KEY). MODEL_PROVIDER = "deepseek" -# Staging env (added 2026-11-01): a separate worker with its own URL and its -# own admin token (staging SM key commonly-staging-hosted-runtime-admin-token), -# serving the staging Commonly instance at api-staging.commonly.me. A named -# Cloudflare env does not inherit bindings or vars, so they are repeated here; the -# top-level [[migrations]] applies to every env. Deploy is operator/CI-side (see -# the header) — `wrangler deploy --env staging` plus a `wrangler secret put` that -# pipes the token from staging SM, so the token is never typed by hand. +# Staging env (added 2026-11-01): a separate worker serving the staging Commonly +# instance at api-staging.commonly.me. A named Cloudflare env inherits neither +# bindings, vars NOR secrets, so the AGENT binding and vars are repeated here and +# BOTH secrets must be set per-env; the top-level [[migrations]] applies to every env. +# Deploy is operator/CI-side (see the header): `wrangler deploy --env staging`, then +# two `wrangler secret put ... --env staging` piped from staging Secret Manager so +# nothing is typed by hand: +# RUNTIME_ADMIN_TOKEN <- commonly-staging-hosted-runtime-admin-token (gates provision) +# DEEPSEEK_API_KEY <- commonly-deepseek-api-key (MODEL_PROVIDER=deepseek; a turn +# throws "DEEPSEEK_API_KEY unset" without it, so hosted-reply +# would fail even once provision succeeds) [env.staging] name = "commonly-agent-runtime-staging" From 4099a18e694288c207c59ef7cad7e6a27440fe7b Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:29:10 -0700 Subject: [PATCH 4/5] docs(agent-runtime): staging DeepSeek key must be a separate capped key, not prod's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fresh-keys rule, same as the admin token: a staging leak must not spend on prod's DeepSeek key. The worker calls DeepSeek directly (no base-URL env in the worker), so pointing it at staging LiteLLM's capped virtual key isn't an option from the Cloudflare edge. So the staging worker's DEEPSEEK_API_KEY is a SEPARATE spend-capped staging key, stored as commonly-staging-deepseek-api-key and set per-env operator-side — never prod's commonly-deepseek-api-key. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015AKgpNr8tFMMj8aFvFPUdA --- workers/agent-runtime/wrangler.toml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/workers/agent-runtime/wrangler.toml b/workers/agent-runtime/wrangler.toml index cda417ce5c..ea4e56e718 100644 --- a/workers/agent-runtime/wrangler.toml +++ b/workers/agent-runtime/wrangler.toml @@ -29,9 +29,13 @@ MODEL_PROVIDER = "deepseek" # two `wrangler secret put ... --env staging` piped from staging Secret Manager so # nothing is typed by hand: # RUNTIME_ADMIN_TOKEN <- commonly-staging-hosted-runtime-admin-token (gates provision) -# DEEPSEEK_API_KEY <- commonly-deepseek-api-key (MODEL_PROVIDER=deepseek; a turn -# throws "DEEPSEEK_API_KEY unset" without it, so hosted-reply -# would fail even once provision succeeds) +# DEEPSEEK_API_KEY <- commonly-staging-deepseek-api-key — a SEPARATE, spend-capped +# staging DeepSeek key, NOT prod's key (same fresh-keys rule as +# the admin token: a staging leak must not spend on prod's key). +# MODEL_PROVIDER=deepseek, so a turn throws "DEEPSEEK_API_KEY +# unset" without it and hosted-reply fails even once provision +# succeeds. The worker calls DeepSeek directly (no base-URL env), +# so staging LiteLLM's virtual key is not an option here. [env.staging] name = "commonly-agent-runtime-staging" From 6c020170867621728a057d1d51acaf7a5658b04b Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Sun, 1 Nov 2026 00:42:27 -0700 Subject: [PATCH 5/5] fix(agent-runtime): cleanup exempts a not-ready deprovision only when provisioning did NOT succeed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit catch: on a staging run where provision SUCCEEDED (200), a failed deprovision (502 unreachable) leaks the worker + runtime token — the backend revokes the token only after a successful stop — but the exemption keyed on staging alone reported it clean. cleanupRun now keys the exemption on hostedProvisioned (false only when provision was skipped or failed, so nothing was created to leak); a failed stop after a successful provision is always a survivor, staging or not. Default hostedProvisioned=true keeps the conservative behaviour for any caller that omits it. Added a provision-200 + deprovision-502 survivor test. 46/46. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_015AKgpNr8tFMMj8aFvFPUdA --- scripts/first-user-walk-cleanup.mjs | 18 ++++++++------- scripts/first-user-walk.mjs | 16 +++++++++---- scripts/first-user-walk.test.mjs | 35 +++++++++++++++++++++++------ 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/scripts/first-user-walk-cleanup.mjs b/scripts/first-user-walk-cleanup.mjs index e9d6d38d56..7d04b4d27f 100644 --- a/scripts/first-user-walk-cleanup.mjs +++ b/scripts/first-user-walk-cleanup.mjs @@ -58,7 +58,7 @@ export const recoverRunPods = async ({ api, token, userId, podName, podIds }) => // `deleteAccount: false` is the shared staging sign-in account: the run removes // only the pods it recorded, never pods that account owns from elsewhere, and // never the account itself. -export const cleanupRun = async ({ api, token, password, email, userId, agentName, podIds = [], onBotIds = () => {}, deleteAccount = true, hostedOptional = false }) => { +export const cleanupRun = async ({ api, token, password, email, userId, agentName, podIds = [], onBotIds = () => {}, deleteAccount = true, hostedProvisioned = true }) => { const survivors = []; const botIds = new Set(); @@ -69,14 +69,16 @@ export const cleanupRun = async ({ api, token, password, email, userId, agentNam }; // Stop the runtime before the seat goes: uninstalling the seat does not stop - // the hosted worker. 404 means there was no hosted install to stop. On a staging - // run where provisioning was skipped (hostedOptional), the backend answers - // deprovision with the same not-ready code (503 unconfigured / 502 unreachable): - // nothing was provisioned, so that is not a leaked runtime and must not become a - // survivor that reddens the run. On prod the agent WAS provisioned, so a failed - // deprovision stays a survivor. + // the hosted worker. 404 means there was no hosted install to stop. A not-ready + // deprovision (503 unconfigured / 502 unreachable by code) is exempt from + // survivor-tracking ONLY when provisioning did NOT succeed (hostedProvisioned + // false — skipped or failed, so nothing was created to leak). When provision + // succeeded, the backend revokes the runtime token only after a SUCCESSFUL stop, + // so a failed deprovision leaves the worker + token live — a real survivor, + // staging or not. Default hostedProvisioned=true keeps the conservative behaviour + // for any caller that does not pass the provisioning outcome. const stop = await api('POST', '/api/hosted/deprovision', { token, body: { agentName } }); - const stopNotReady = hostedOptional + const stopNotReady = !hostedProvisioned && (stop.data?.code === 'hosted_runtime_unconfigured' || stop.data?.code === 'hosted_runtime_unreachable'); if (stop.status >= 300 && stop.status !== 404 && !stopNotReady) survivors.push(`hosted:${agentName} (deprovision ${stop.status})`); diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs index 6aad5827e5..84076564d4 100644 --- a/scripts/first-user-walk.mjs +++ b/scripts/first-user-walk.mjs @@ -105,6 +105,10 @@ export const runWalk = async ({ // Null until chooseWalkMode accepts the run, so a refused run is never reported as signup. let walkMode = null; let cleanupRan = false; + // True only once provision returned 200: a worker + runtime token were actually + // created, so cleanup must treat a FAILED deprovision as a survivor even on staging. + // False when provision was skipped or failed — nothing was created to leak. + let hostedProvisioned = false; // Signup returns no user id, so the manifest carries the exact username instead: // SRE finds the account by that name, not by a prefix. Never the email. const manifest = { mode: null, userId: null, username: null, podIds: [], unresolvedPodNames: [], agentName: null, botIds: [], survivors: [], status: 'running' }; @@ -219,10 +223,11 @@ export const runWalk = async ({ agentName, podIds: manifest.podIds, deleteAccount: walkMode.deleteAccount, - // Staging run only: a not-ready deprovision (503 unconfigured / 502 unreachable) - // is not a leaked agent — provisioning was skipped, nothing was created — so it - // must not become a survivor and redden the run. On prod it stays a survivor. - hostedOptional: walkMode.mode === 'signin', + // A not-ready deprovision (503/502 by code) is exempt from survivor-tracking + // ONLY when provisioning did not succeed — nothing was created to leak. If + // provision returned 200, a failed deprovision IS a leak (the backend revokes + // the token only after a successful stop), so it stays a survivor, staging or not. + hostedProvisioned, // Persist each pod's bot ids before that pod is deleted, so a throw // later in the loop still leaves the ids SRE needs. onBotIds: (podId, bots) => { @@ -375,6 +380,9 @@ export const runWalk = async ({ && (p.data?.code === 'hosted_runtime_unconfigured' || p.data?.code === 'hosted_runtime_unreachable'); if (hostedOptional && notReady) { hostedSkipped = true; throw new SkipStep(`no staging worker (${p.data.code})`); } expect(p.status === 200 && p.data?.provisioned, `provision ${p.status}: ${JSON.stringify(p.data)}`); + // Provision succeeded: a worker + token now exist, so a failed deprovision in + // cleanup is a real leak (survivor), staging or not. + hostedProvisioned = true; }); if (hostedSkipped) skipStep('hosted-reply', 'no staging worker (hosted runtime not ready)'); diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs index fa3aa729c0..a60366163b 100644 --- a/scripts/first-user-walk.test.mjs +++ b/scripts/first-user-walk.test.mjs @@ -26,7 +26,7 @@ const SHARED_EMAIL = 'walk-shared@commonly.test'; // staging they fail. Any other 5xx has no not-ready code -> fail. const fakeBackend = ({ preview = 200, create = 'ok', password = PASSWORD, echoRegister = false, seeded = false, - hostedProvision = 200, + hostedProvision = 200, hostedDeprovision = undefined, } = {}) => { const state = { accountExists: seeded, pods: [], lastAgent: null, calls: [], loginEmails: [] }; const reply = (status, data) => ({ status, json: async () => data }); @@ -84,12 +84,14 @@ const fakeBackend = ({ return reply(hostedProvision, { message: 'Hosted runtime request failed' }); } if (method === 'POST' && p === '/api/hosted/deprovision') { - // When provision was not-ready, deprovision answers with the same not-ready - // code (the backend is unconfigured/unreachable for both) — cleanup must not - // treat that as a survivor on a staging run. - if (hostedProvision === 503) return reply(503, { code: 'hosted_runtime_unconfigured' }); - if (hostedProvision === 502) return reply(502, { code: 'hosted_runtime_unreachable' }); - return reply(404, {}); + // Default: mirror provision (unconfigured/unreachable backend answers both the + // same). hostedDeprovision overrides it to model a SUCCESSFUL provision whose + // deprovision then fails (worker + token leak), which must be a survivor. + const d = hostedDeprovision ?? (hostedProvision === 200 ? 404 : hostedProvision); + if (d === 404) return reply(404, {}); + if (d === 503) return reply(503, { code: 'hosted_runtime_unconfigured' }); + if (d === 502) return reply(502, { code: 'hosted_runtime_unreachable' }); + return reply(d, {}); } if (method === 'DELETE' && p.startsWith('/api/registry/agents/')) return reply(404, {}); if (method === 'POST' && /^\/api\/messages\//.test(p)) { @@ -184,6 +186,25 @@ for (const status of [503, 502]) { }); } +test('staging: provision SUCCEEDS but deprovision 502 is a SURVIVOR — a live worker/token is never reported clean', async () => { + const dirsFor = dirs(); + const { fetchImpl } = fakeBackend({ seeded: true, hostedProvision: 200, hostedDeprovision: 502 }); + const { exitCode, result } = await run(fetchImpl, dirsFor, { + base: 'https://api-staging.commonly.me', + env: { COMMONLY_WALK_EMAIL: SHARED_EMAIL }, + }); + + assert.equal(result.mode, 'signin'); + // Provision succeeded, so the hosted steps ran and passed; but the failed stop + // leaks the worker + token, so cleanup is NOT clean and the run fails — the + // staging exemption must never cover a failed stop after a successful provision. + assert.equal(result.steps.find((s) => s.name === 'hosted-install').result, 'pass'); + assert.equal(result.cleanup.ok, false); + assert.ok(result.cleanup.survivors.some((s) => /deprovision 502/.test(s))); + assert.equal(exitCode, 1); + assert.equal(result.overall, 'fail'); +}); + test('PROD: a not-ready hosted provision (502) FAILS — the staging skip is never applied off staging', async () => { const dirsFor = dirs(); const { fetchImpl } = fakeBackend({ seeded: true, hostedProvision: 502 });