fix(dev-platform): wire the runner image into the middleware's job policy + delete jobs - #529
Open
Weegy wants to merge 31 commits into
Open
fix(dev-platform): wire the runner image into the middleware's job policy + delete jobs#529Weegy wants to merge 31 commits into
Weegy wants to merge 31 commits into
Conversation
…licy + let operators delete terminal jobs The middleware's DockerBackend job-policy endpoint (GET /internal/job-policy/:jobId, fetched by the daemon at provision time) has been unreachable from the day the epic shipped: docker-compose.dev-platform.yaml never set DEV_RUNNER_DEFAULT_IMAGE (or DEV_RUNNER_IMAGE) on the middleware container, only on the daemon (as DEV_RUNNER_IMAGES). Without a resolved runner image, wireDevPlatform's jobPolicyConfig stays undefined and the endpoint 503s forever. Every real DockerBackend job dies instantly at the implement phase (the first phase that actually provisions a container -- analyze/bootstrap/plan/clarify/gate run without one) with the daemon reporting a generic 502 'the middleware could not supply the job policy' and zero tokens spent. Fix: wire DEV_RUNNER_DEFAULT_IMAGE onto the middleware from the same DEV_RUNNER_IMAGE source var the daemon's allowlist already uses, and make index.ts's runner-image resolution consistent between the Fly and Docker backends (both now share one DEV_RUNNER_IMAGE ?? DEV_RUNNER_DEFAULT_IMAGE fallback, previously only the Fly path had it). Added two composeTopology tests asserting the middleware carries a runner image and that it agrees with the daemon's allowlist, so this exact gap cannot silently reopen. Also: there was no way to remove a finished job from the operator's list short of the daily retention sweep -- add DELETE /jobs/:id (terminal jobs only, 409 for an active one) with matching UI actions on the job list and job detail page. Full gate green: middleware build/lint/typecheck/test (4799/4803, 0 fail, 4 skipped pg-only locally), web-ui lint/typecheck/vitest (0 errors, 306/306), i18n:check OK (3136 keys).
…daemon A second, distinct cause of the same generic 'the middleware could not supply the job policy' 502: DEV_RUNNER_REQUIRE_DIGEST was only ever mentioned in a comment, never wired into the dev-runner-daemon service's environment block. env.DEV_RUNNER_REQUIRE_DIGEST was therefore always undefined inside the container regardless of .env, and parseRequireDigest() defaults undefined to true -- so every locally-built runner image (a floating tag, no digest, no registry to have pinned one from) was refused by assertPolicyImage at the allowlist/digest check, mapped by the daemon's HTTP layer to the exact same 502 the runner-image gap produced. Reproduced live: after the runner-image fix (this same PR) the job-policy endpoint returned 200 with a valid policy, yet a fresh real job (issue #445) still failed identically -- the daemon logs showed DEV_RUNNER_REQUIRE_DIGEST=true despite .env setting it to 0. New composeTopology test asserts the key is forwarded at all, independent of its value -- presence, not correctness, is what a comment cannot provide.
… just the policy client The third gate in the same rejection chain, and the first one that is a real code bug rather than an unwired env var. With the runner image wired into the job policy and DEV_RUNNER_REQUIRE_DIGEST actually forwarded into the daemon container (the two preceding commits), a fresh real job stopped 502-ing and instead failed instantly at implement with a NEW, more specific error: devplatform.spec_rejected: daemon rejected the job (spec rejected (image_not_digest_pinned): the job image is a floating tag, not a digest reference) buildContainerCreateOptions in clamp.mjs re-checks digest-pinning after assertPolicyImage already passed -- but hardcoded the requirement ON instead of reading the operator's posture. Its own comment described itself as "the last line if that knob is ever turned off", which is not defence-in-depth: two enforcement points reading one posture is defence-in-depth, one of them ignoring it makes the posture a no-op. DEV_RUNNER_REQUIRE_DIGEST=0 -- the escape hatch docker-compose.dev-platform.yaml documents for exactly this case -- could therefore never work, and the local-dev shape the epic specifies (an image docker load'ed straight into the nested dind engine, no registry, so nothing to have pinned a digest from) was structurally unprovisionable. The posture is now passed into the clamp explicitly and defaults to true, so it fails closed if a caller forgets to thread it; createDockerEngine resolves it with the SAME parseRequireDigest the policy client uses, so there is one decision and two enforcement points. What no posture relaxes: a digest that IS present must still be a real content address (image_bad_digest is unconditional -- a knob about whether a digest is required never tolerates a malformed one), and the rest of the clamp (non-root, read-only rootfs, CapDrop ALL, no-new-privileges, resource bounds, the single workspace bind) is untouched. Relaxing that gate alone would have failed a fourth and fifth way, so both are fixed here too. jobs.mjs carried a branch commented "Unreachable: buildContainerCreateOptions already rejected a tag-only image" -- admitting a floating tag makes it reachable, throwing the identical error one line later. And imageDigest is a REQUIRED daemon<->middleware wire field (z.string().min(1)), so an empty one would have failed the middleware's response parse with an opaque protocol error AFTER the container was already running. Both are resolved by reading the content address back from the engine for a floating tag: the RepoDigest for the pulled repository, else the ref's own digest, else the local image Id -- which is the only content address a docker load'ed image has, and is exactly the precedence warmImages already used. That duplicated block is now the shared resolveImageDigest helper. The prod path is deliberately byte-identical: a digest-pinned ref resolves from the ref itself with no engine round-trip, and with the posture unset a floating tag is still refused before any docker resource is created. Tests: 5 clamp cases pinning the posture (fails closed when omitted, explicit true refuses, explicit false admits, relaxing it relaxes nothing else, malformed digests still rejected) and 3 engine cases covering the full provision path (prod default refuses and creates nothing, a loaded floating tag provisions and records its Id, a pinned ref ignores what the engine reports). The fake dockerode now models a load'ed image -- no RepoDigests, Id only. 392 daemon tests green.
…eal provision moment
Gate 4 in the same rejection chain the previous two commits closed. The daemon's
own ALLOWED_ENV_KEYS (policyClient.mjs) already special-cases OMADIA_JOB_TOKEN
as 'policy-supplied' -- the middleware legitimately mints it -- but nothing ever
put it there. deriveJobPolicy.ts's env is deliberately secret-free by design (a
regression-tested invariant), and DockerBackend.provision() deliberately posts
only {protocol, jobId, leaseTtlSec} to the daemon (spec S3: 'a caller never
dictates policy'), so the token could never ride either of those.
The docker backend's real provision moment is the daemon's OWN later fetch of
GET /internal/job-policy/:jobId -- unlike Local/Fly backends, which spawn their
container synchronously right after minting a token, the docker backend's
container is born whenever the daemon independently decides to ask. So this
reissues a fresh token right there (devJobStore.reissueRunnerToken: mint +
replace runner_token_hash, same one-time-plaintext contract every backend
already promises) and folds it into the response's env, next to
deriveJobPolicy's own fields, verified live: a real POST /v1/jobs provision
against the previously-failing job returned 201, but the spawned runner
immediately exited 1 with 'missing required env OMADIA_JOB_TOKEN' -- this
closes that gate.
createJob's original token (used by every backend's schema, unconditionally)
is simply never handed to a docker-backed container -- reissuing here doesn't
reuse it, it replaces its now-provably-unused hash.
New tests: devJobStore.reissueRunnerToken (mint/replace/invalidate), an E2E
test proving the reissued token authenticates the phone-home surface while the
original no longer does, and updated the existing job-policy env fixture
(OMADIA_JOB_TOKEN is now the one deliberate exception to deriveJobPolicy's
no-secret invariant, not a violation of it).
Full gate green: build/lint (0 errors)/typecheck/test (4800/4804, 0 fail, 4
pg-only skip locally/run in CI; one unrelated pre-existing flaky test in
profilesHealthRoutes.test.ts confirmed passing in isolation, not caused by
this change).
Gate 5. After the runner-image/digest/token gates were fixed, a real
provision succeeded (HTTP 201, container created) but the runner immediately
exited: "fatal: fetch failed". Live-reproduced the exact cause by running the
runner image on the jobs own per-job network: "getaddrinfo ENOTFOUND
middleware".
DEV_RUNNER_NO_PROXY: middleware,localhost,127.0.0.1 told the runner to bypass
the proxy and connect to the middleware directly, with the comment stating
that as fact ("The runner reaches the middleware directly, not through the
proxy"). It cannot: job containers are created by dind on their own per-job
network, which has no route to dev-control -- the network "middleware"
actually lives on. Only dev-egress-proxy is dual-homed onto both dev-egress
(job-reachable) and dev-control (middleware-reachable). Verified live:
dev-egress-proxy resolves "middleware" fine (dns.lookup -> 172.28.4.4); the
runners own network does not.
The proxy already has no reason to be bypassed here: egressPolicy.mjs allowInternal
branch matches any request whose host+port equals OMADIA_INTERNAL_API_URL
(this proxys own env, http://middleware:8080) and allows it regardless of
path -- not scoped to the LLM-proxy route alone, as the surrounding comments
imply. Removed "middleware" from the daemons DEV_RUNNER_NO_PROXY (kept
localhost/127.0.0.1, which legitimately never leave the container).
New composeTopology test asserts "middleware" is never in this list.
Full gate green: lint 0 errors; ran the full devplatform suite + composeTopology
in isolation (24/24, 7/7 pass) -- this change is compose-config + one test
file, no source logic touched.
…PROXY
Gate 6. After fixing DEV_RUNNER_NO_PROXY (previous commit), a fresh provision
still failed with "fatal: fetch failed" -- routing "middleware" through the
proxy in the env vars did nothing, because the shim never uses the proxy in
the first place.
Root cause: unlike curl and git, Node's global fetch (undici) does NOT read
HTTP_PROXY/HTTPS_PROXY/NO_PROXY by default -- that behaviour is opt-in, gated
behind NODE_USE_ENV_PROXY (undici's EnvHttpProxyAgent). The comment this
injectDaemonOwnedEnv function carried asserted the opposite ("Standard http
clients (curl, git, undici, python-requests) derive that header from the
proxy URL's userinfo"), which is the misconception that let this ship: the
shim's homeClient.ts is deliberately "Node's global fetch only -- no
dependency", so every phone-home call (spec fetch, events, diff upload,
result) silently ignored HTTP_PROXY and tried the middleware direct.
Verified empirically inside the deployed middleware container, isolating the
exact mechanism before touching code:
- HTTP_PROXY set, NODE_USE_ENV_PROXY unset: a fetch to an unreachable host
-> getaddrinfo ENOTFOUND (proxy ignored entirely)
- HTTP_PROXY set, NODE_USE_ENV_PROXY=1: the same fetch call
-> ECONNREFUSED <proxy address> (proxy honoured, as expected)
- process.env.NODE_USE_ENV_PROXY set IN-PROCESS after Node starts: no
effect -- undici reads it once at dispatcher construction, so it MUST be
a real container env var, not something the shim could set for itself.
Fix: inject NODE_USE_ENV_PROXY=1 alongside HTTP_PROXY/HTTPS_PROXY, only when
a proxy is actually configured -- same conditional the proxy vars already
use. No new dependency (the flag activates undici's own bundled
EnvHttpProxyAgent, already inside Node 22); the "no dependency" shim design
is unchanged.
New test: NODE_USE_ENV_PROXY is present exactly when a proxy is configured,
absent otherwise (extended the existing symmetric proxy-vars test).
Daemon suite green: 393/393 (this worktree previously had no node_modules
installed for the sidecar -- a separate package from middleware/ -- ran npm
ci there first; the 5 apparent failures before that were ERR_MODULE_NOT_FOUND
for dockerode, not a real regression).
…ess env too Gate 7. After gate 6 (Node's fetch honouring HTTP_PROXY), a fresh provision got past phone-home entirely -- it authenticated, ran, and reached the actual clone step -- then failed: "git clone failed (128): fatal: unable to access 'https://github.com/byte5ai/omadia.git/': Could not resolve host: github.com". Same root cause as gate 6, different subprocess. runGit() in gitOps.ts builds a deliberately hermetic env for the git child process -- an explicit allowlist (PATH, HOME, GIT_TERMINAL_PROMPT, GIT_CONFIG_NOSYSTEM, GIT_CONFIG_GLOBAL, LANG), NOT the parent env, so no ambient secret rides along and no global credential helper interferes. Good security design -- but it silently dropped HTTP_PROXY/HTTPS_PROXY/NO_PROXY along with everything else. The job's isolated network has no route to github.com except through the daemon's egress proxy (the exact same constraint gate 5/6 already established for the middleware itself), so without these, git falls back to a direct DNS lookup that always fails. Fix: forward HTTP_PROXY/HTTPS_PROXY/NO_PROXY (both cases -- curl, git's HTTPS transport, historically trusts lowercase by default; the daemon injects both, see policyClient.mjs) from the shim's own env onto this allowlist, conditionally (only when actually set, so no proxy configured means no proxy-shaped keys at all -- matches the existing symmetric pattern in policyClient.mjs's own proxy injection). These are deployment topology, not a secret -- same category PATH/HOME already sit in on this exact allowlist. New tests: proxy vars ARE forwarded when set (all four spellings), and are completely ABSENT (not empty-string) when no proxy is configured -- extends the existing "does not forward arbitrary parent env" hermetic-environment test, doesn't touch it. Verified live end-to-end against the real deployed stack, using the actual job/worker lifecycle (not a direct daemon call): reset a real job to queued, watched the middleware's own claim loop provision it, and the runner reached git clone before this fix landed -- confirming gates 1-6 are genuinely closed, not just individually reproduced in isolation. Shim suite green: 48/48 (dev-runner-shim's own tests, node_modules already present in this worktree). Full middleware gate green: build/lint (0 errors)/typecheck clean (this package is part of the middleware workspace build).
Gate 8 (partial -- the CONNECT tunnel itself is still under investigation,
see follow-up). After gate 7 (git honouring HTTP_PROXY), a fresh provision
reached git clone and failed differently: "Could not resolve host:
github.com" -- from INSIDE the egress proxy container itself, not the job.
Root cause, verified live: dev-egress-proxy's `networks:` list names only
dev-egress and dev-control, and BOTH are `internal: true` in this overlay --
correctly, neither may reach outside. But dev-egress-proxy's entire purpose
is being the one path a job container has to the real internet, and it had
no route out either. `docker exec omadia-dev-dev-egress-proxy-1 node -e
"require('node:dns').lookup('github.com',...)"` returned EAI_AGAIN before
this fix, a real IP after.
Fix: a THIRD, dedicated network (dev-egress-external, plain bridge, no
subnet pin -- the proxy is its only member) gives the proxy real internet
route without sharing `omadia` with middleware/web-ui, which would make it
reachable from (and able to reach) the app services laterally -- exactly
what the separate egress plane exists to prevent.
Also added logger.warn diagnostics to three previously-silent catch/error
paths in proxy.mjs (dataServer's clientError handler, the catch around
handleConnect, and the upstream socket's error handler) -- these were
completely silent (destroySocket with no logging) which made an in-flight
CONNECT-tunnel failure impossible to diagnose from container logs alone.
Kept regardless of the CONNECT-tunnel root cause (still being isolated,
see the next commit on this branch): better failure visibility here is
worth keeping on its own merits, and cost nothing to verify -- the daemon
suite is still 393/393 green with these in place.
New composeTopology tests: the proxy joins at least one non-internal
network, and that network is not `omadia`.
Full gate green: middleware build/lint (0 errors)/typecheck/test
(4803/4807, 0 fail, 4 pg-only skip locally/run in CI); daemon suite
393/393; composeTopology 26/26 in isolation.
…proxy's challenge
Gate 8, the actual CONNECT bug the previous commit deferred. After gate 7 (git
honouring HTTP_PROXY) and the proxy getting a real internet route, every fresh
provision still died at the clone:
git clone failed (128): fatal: unable to access
'https://github.com/byte5ai/omadia.git/': Proxy CONNECT aborted
Two earlier rounds could not find it because the evidence pointed the wrong way:
`handleConnect`, `clientError` and the upstream-error handler were all
instrumented and NONE of them ever logged, while a job's plain-HTTP phone-home
through the same 172.28.5.3:3128 path worked perfectly. That looked like "the
CONNECT never reaches the proxy". It does.
Root cause, reproduced race-free and read straight out of git's own curl trace
(GIT_CURL_VERBOSE=1, real runner image, on a standalone bridge network inside
dind, with a registration this session PUT on the proxy's control plane):
Connected to 172.28.5.3 port 3128
Establish HTTP proxy tunnel to github.com:443
<= HTTP/1.1 407 Proxy Authentication Required
<= Proxy-Authenticate: Basic realm="omadia-dev-egress"
== Proxy auth using Basic with user '...'
=> Send header: Proxy-Authorization: Basic <redacted> <-- 168 bytes
== Proxy CONNECT aborted
Proxy auth over CONNECT is a CHALLENGE-RESPONSE ON ONE CONNECTION. libcurl --
so `git`, whose `http.proxyAuthMethod` defaults to `anyauth` -- sends an
unauthenticated CONNECT first, reads the 407 + `Proxy-Authenticate`, then
re-sends the CONNECT with `Proxy-Authorization` on that same socket.
The proxy answered the 407 correctly and then `clientSocket.end()`d, FIN'ing the
socket without ever saying it was closing: no `Connection: close`, no
`Content-Length`. The client had no way to know, so it wrote its authenticated
retry into a connection we had already closed, read EOF, and reported "Proxy
CONNECT aborted". Basic proxy auth was therefore impossible for any libcurl
client, and it always failed on the FIRST request of every job.
Why the instrumentation lied: the 407 is emitted from the deny branch that
predates all three log points, so nothing warned; and the reason the phone-home
path worked is that node's `EnvHttpProxyAgent` (gate 6) sends
`Proxy-Authorization` PREEMPTIVELY and so never triggers a 407 at all. The same
is true of a hand-rolled CONNECT probe -- which is exactly why a manual probe
against this proxy succeeds while git fails. The asymmetry was the bug's
fingerprint, not evidence against the proxy.
Fix, in `writeConnectStatus`: every non-2xx CONNECT reply now carries
`Connection: close` (plus the legacy `Proxy-Connection: close` spelling libcurl
also honours) and `Content-Length: 0`. Put in the writer rather than at the 407
call site because EVERY non-2xx reply on this path is terminal -- node detaches
its HTTP parser from the socket at the `connect` event and hands it to us raw,
so there is no second request to serve on it -- and no call site should be able
to forget. A 2xx is deliberately untouched: that reply IS the tunnel.
Not taken: setting `http.proxyAuthMethod=basic` in the shim's git invocation so
curl sends credentials preemptively. It would work, but it fixes one client of
a proxy that is broken for all of them; the defect is the unannounced close.
Nothing about the allow/deny decision, the allowlist, the rebinding classifier
or the pinning changes -- this only adds response headers on paths that were
already refusing and already closing. The isolation invariants are untouched.
Live verification (local docker-compose overlay, omadia-dev):
- the reproduction above, re-run after the rebuild: 407 -> a NEW connection ->
`HTTP/1.1 200 Connection Established` -> `git clone exit code: 0`, real tree
at 4a838b7, 27 entries. Before the fix, byte-identical command, 3/3 failures.
- the REAL pipeline, driven by the middleware's own claim worker (job
4ab8b724, provisions 11/12/13): `phase implement start` ->
`status agent_started` -> `status agent_done`, with `dev_jobs.error` NULL.
`phaseLoop` step 4 clones at the pinned base sha and the agent runs with cwd
= that clone, so `agent_started` is only reachable through a clone that
succeeded. Every provision before this one died at the clone.
- the jobs now stop at `agent_done` with 0 tokens / $0 -- the coding CLI has no
Anthropic credential in Vault yet. Known, separate, out of scope here.
Two regression tests over REAL sockets in `proxy.test.mjs`: the 407 announces the
close, actually closes, and the authenticated retry on a fresh connection
establishes a tunnel that carries bytes (and the 200 does NOT carry the close);
plus the same announcement on the non-auth 403 paths, since they share the trap.
Both fail against the pre-fix writer (verified by reverting it) and pass after.
Sidecar suite green: 395/395 (393 before, +2 new). eslint on both changed files
reports exactly the same 35 `no-undef` problems as the pristine HEAD versions --
a pre-existing gap where the middleware config supplies no node globals for
`.mjs` sidecars, unrelated to this change; `tsc --noEmit` likewise reports only
the pre-existing `src/reaper.mjs` errors in a file this commit never touches.
…omments said existed Gate 9 -- the last one. After gate 8 (proxy 407 fix), a real job cleared clone and the CLI actually started, but exited instantly with 0 tokens/$0, "implement session exited with code 1", ~27ms after starting -- too fast for even a rejected network round-trip. Root cause: agentRunner.ts's buildAgentEnv only forwards ANTHROPIC_BASE_URL/ ANTHROPIC_AUTH_TOKEN into the CLI child process when ShimEnv.llmEnvAllowed is true. That flag is set ONLY by the legacy LocalProcessBackend (OMADIA_LLM_ENV_ALLOWED=true, the W0 "jail acknowledgment"). Both phaseRunner.ts (gated) and index.ts (collapsed) read the LLM auth pair from OMADIA_ANTHROPIC_BASE_URL / OMADIA_ANTHROPIC_AUTH_TOKEN -- vars ONLY the LocalProcessBackend ever sets. For the real docker path, deriveJobPolicy.ts sets plain ANTHROPIC_BASE_URL (no OMADIA_ prefix) and no auth-token var at all matching either name. Every one of these three files' own comments already say the quiet part: "W1's per-job, short-lived LLM-proxy tokens replace this passthrough entirely" -- the replacement was documented as existing and never actually written. The CLI ran with no ANTHROPIC_BASE_URL and no ANTHROPIC_AUTH_TOKEN at all, so it never even reached the middleware's LLM proxy -- unrelated to the earlier "no key in Vault" finding, which was also real but wasn't the (whole) story. Fix: in both phaseRunner.ts and index.ts, prefer the W1 inputs when present -- plain ANTHROPIC_BASE_URL (process.env) as the proxy target, and ShimEnv.jobToken (already required, already the exact bearer llmProxy.ts's resolveJobByToken resolves a calling job from -- "the CLI never sends a jobId -- only Authorization: Bearer <djr_...>") as the auth token. Presence of a W1 base URL stands in for the W0 jail acknowledgment (llmEnvAllowed = env.llmEnvAllowed || Boolean(w1BaseUrl)) rather than requiring it -- a short-lived, per-job token is a different threat model from W0's long-lived middleware secret, which stays gated exactly as before when there is no W1 base URL. Falls back to the legacy OMADIA_ANTHROPIC_* pair unchanged for a genuine W0 LocalProcessBackend run. New tests: index.test.ts asserts the W1 path forwards ANTHROPIC_BASE_URL + ShimEnv.jobToken with no jail acknowledgment; phaseLoop.test.ts extends the fake CLI fixture to record what it received and asserts the same across all three phase sessions of a gated run. Both existing W0-gate tests (withholds/forwards under OMADIA_ANTHROPIC_*) pass unchanged -- they never set plain ANTHROPIC_BASE_URL, so the new W1 branch never activates for them. Shim suite green: 50/50 (48 baseline + 2 new). Full middleware gate green: build/lint (0 errors)/typecheck. Deployment note: the operator had an Anthropic key configured for the chat/orchestrator feature (@omadia/orchestrator / provider:anthropic/api_key) but none yet under the dev-platform's own namespace (core:dev-platform / llm/anthropic/api_key) -- a deliberately separate namespace for budget/ security isolation between the two features, not a bug. Copied one over for local live verification of this fix.
…hild env too Gate 10. After the previous commit (W1 LLM-auth passthrough), a job cleared clone and started the CLI for real -- but then hung indefinitely with zero log output and zero tokens, for as long as it was left running (135s+ in one observed run, no wall-clock timeout hit yet). "agent_started" fired; "agent_done" never did. Same root cause family as the git fix and gate 6, one more subprocess. The `claude` CLI is a SEPARATE process from this shim -- `agentRunner.ts` spawns it with an explicit, from-scratch allowlisted env (`buildAgentEnv`), so it does NOT inherit the shim's own process.env, only what that allowlist hands it. The allowlist wired ANTHROPIC_BASE_URL/ANTHROPIC_AUTH_TOKEN (the previous commit's fix) but never HTTP_PROXY/HTTPS_PROXY/NO_PROXY or NODE_USE_ENV_PROXY. The CLI is itself Node/undici-based, so without NODE_USE_ENV_PROXY it silently ignores HTTP_PROXY exactly like this shim's own fetch calls did before gate 6 -- except this time there is no error at all: the CLI just tries a direct connection to an unreachable host and sits there. No stderr line ever gets translated because the CLI's own network stack is still trying, not failing. Fix: inside the same `if (opts.llmEnvAllowed === true)` block that wires ANTHROPIC_BASE_URL/ANTHROPIC_AUTH_TOKEN, also forward HTTP_PROXY/HTTPS_PROXY/ NO_PROXY (both cases) and NODE_USE_ENV_PROXY=1 when a proxy is configured -- scoped to the same LLM-routing gate as the auth pair, since that is exactly when the CLI needs to reach the proxy at all. New tests in agentRunner.test.ts (buildAgentEnv is unit-testable directly, no subprocess needed): proxy vars forwarded under the gate, withheld without it (mirrors the existing auth-pair test), and absent entirely (not empty-string) when no proxy is configured. Shim suite green: 52/52 (50 baseline + 2 new). Full middleware gate green: build/lint (0 errors)/typecheck.
….json()
Gate 11. After gates 6/10 (proxy env forwarding), a job reached the CLI and
the CLI reached the proxy -- but every real request 400'd:
"devplatform.invalid_body -- request body must name a model", even though
the client sent a well-formed Messages-API body naming a model.
llmProxy.ts's handleMessages only trusts a Buffer: it canonicalises the raw
bytes itself (own comment: "validate one representation, forward another"
is the exact class of bug this avoids) via its own route-level
express.raw({ type: () => true }), mounted when mountDevPlatform() wires the
runner router. That router mounts late in index.ts's boot sequence, well
after the *global* app.use(express.json({limit:'10mb'})) which runs earlier,
unscoped by path -- so it runs first, for every request including this one.
body-parser's read() bails via onFinished.isFinished(req) once the stream
is already drained and never touches req.body again (confirmed in
node_modules/body-parser/lib/read.js). So express.json() parses the body
into an object, and the route's own express.raw() -- reached second -- silently
no-ops, leaving req.body as that object. handleMessages's
`Buffer.isBuffer(req.body) ? req.body : Buffer.alloc(0)` then discards it as
an empty buffer, which parses to {} -- no model field, hence the exact error
observed.
The existing llmProxy.test.ts suite never caught this: its fixture mounts
the runner router directly on a bare express() app, with no global
express.json() ahead of it, so the ordering bug this specific mount produces
in the real app was structurally unreachable in the test harness.
Fix: mirror the GitHub-webhook router's existing raw-body-before-json
pattern (see the log line right above the json() mount: "raw-body, before
express.json"). The runner router can't move earlier -- mountDevPlatform
needs graphPool/vault/store set up first -- so instead the global json()
mount is wrapped to skip /api/v1/dev-runner/llm/*, leaving that path's body
untouched for the route's own raw() parser to read exactly as designed.
New test in llmProxy.test.ts reproduces the real mount order (global
express.json() suffixed with the same path-exclusion gate, then the runner
router) and proves the request now lands; a second variant confirms
unrelated routes still parse JSON normally. Manually confirmed against the
old (buggy) mount order that this test fails with the exact same 400 before
the fix, and passes after.
Full middleware gate green: build/lint (0 errors)/typecheck, 566/566
devplatform tests (test/devplatform/*.test.ts).
…og pane
Gate 12. Two operator-facing follow-ups from the first real job run (fcafa3ce):
1. DEV_JOB_DEFAULT_BUDGET_USD default was $5 -- too tight for an Opus-driven
implement phase in a codebase this size; the verification job hit it
mid-task. Raised the config default to $100 (still overridable per-job or
per-repo via the existing budget_cost_usd column and repo settings UI).
2. The implement-phase log pane rendered every tool call as a flat
`$ Name {...raw JSON...}` line -- the start and result events for one call
arrive as two independent, unpaired dev_job_events rows
({name, inputPreview} then {ok, name, outputPreview}), and eventToLine()
just dumped each one verbatim.
New `_lib/toolCallLog.ts`: pairs a call's start/result events by walking
back to the nearest still-pending entry with the same tool name (safe for
the single-threaded CLI agent loop this feeds from), then formats a
one-line headline + tool-shaped detail per tool: file path for
Read/Write, an inline unified diff for Edit (new `_lib/lineDiff.ts`, a
small self-contained LCS line-diff -- no new dependency for something
this size), command+output for Bash, description+subagent for
Agent/Task, pattern for Grep/Glob. Unknown tool names fall back to the
raw input/output text, so nothing silently disappears.
New `ToolCallCard.tsx`: collapsed by default, status glyph (text/edge
only, no spinners, per this project's Lume rules) + headline; expands to
the formatted detail. `JobLogPane`/`page.tsx` swap the old flat
`LogLine[]` for `LogItem[]` (`foldDevJobEvent` replaces `eventToLine`).
New tests: lineDiff.test.ts (8 cases incl. pure add/remove and the
oversized-input fallback), toolCallLog.test.ts (18 cases: FIFO pairing
across repeated same-name calls, orphan results, malformed JSON input, and
one summarizer case per tool kind). All new i18n strings added to both
en.json/de.json per this project's hard i18n rule.
Verified: tsc --noEmit clean, eslint clean, `npm run i18n:check` clean (no
new untranslated-key warnings), `npm run build` succeeds, 36/36 dev-platform
vitest files green, 326/331 full web-ui suite green (5 pre-existing
toolTemplates.test.ts failures, unrelated file, fails identically in
isolation on unmodified main).
…o captured start
Found live while verifying the previous commit against a real running job: two
Edit cards rendered "Edit ? +0 -0" instead of their real diff. Root cause:
useDevJobEvents' native EventSource reconnected once mid-run (visible as
"reconnecting" in the connection line), and the resumed stream's replay
boundary meant the browser saw a tool-call's *result* event without ever
having seen its *start* event -- foldDevJobEvent's orphan-result fallback
correctly renders something rather than crashing, but the entry it produces
has `inputPreview: undefined`.
summarizeToolCall was reaching summarizeEdit anyway, which parses that
undefined input as `{}` (empty object, same shape as a real edit with no
args), so it silently computed a plausible-looking "file_path '?', 0 lines
added, 0 removed" -- indistinguishable from a genuine no-op edit rather than
"this call's real input was never captured."
Fix: treat `inputPreview === undefined` (start never seen) as distinct from
"the tool genuinely had an input JSON object" and route it through the same
raw/output-only fallback already used for unknown tool names, before any
per-tool summarizer runs. New regression test locks this in.
Confirmed live against the job that surfaced it (fcafa3ce, requeued a third
time under the new $100 budget): tsc --noEmit clean, eslint clean, 18/18
toolCallLog tests green.
Root cause of ticket #445's implement-phase agent refusing to proceed ("the plan artifact is empty... zero operator gate answers"): the agent was behaving correctly. The phase rail showed ANALYZE/BOOTSTRAP/PLAN/CLARIFY/GATE as checkmarked, but that's a purely positional render (DevJobPhaseRail.tsx's computePhaseStops marks every stop before the CURRENT phase as done, regardless of whether it ran) -- this job was created directly at phase='implement' and never actually went through planning at all. Traced to devJobStore.createJob's INSERT: `input.phase ?? 'implement'`. None of the five job-creation call sites (admin REST route, chat tool, plugin API, retry route, webhook trigger) ever pass an explicit `phase`, so every real job in this codebase's history has silently skipped straight to implement (or, for gated webhook triggers, parked at an empty `await_human` gate with no plan behind it). This is inconsistent with the rest of the system's own design: transitions.ts's test suite title is literally "collapsed mode skips THE GATE" and that test still begins `analyze -> implement` -- both pipeline modes are designed to start at analyze. The dev-runner-shim already defaults to it independently (phaseLoop.ts: `ctx?.phase ?? 'analyze'`, with real, non-stub prompt-building for every phase in phasePrompts.ts) and one test's own mock harness (devJobOrchestratorTool.test.ts) already modeled `phase ?? 'analyze'` -- while devPlatformPipeline.wire.pg.test.ts had a comment admitting the gap outright: "a gated pipeline starts at analyze (createJob defaults to implement)". The store was the one place never fixed to match. Fix: default to 'analyze' in devJobStore.createJob. Only `kind: 'analyze'` jobs terminate right after that phase (transitions.ts, unaffected); an explicit `phase` override still wins (e.g. the gated-webhook trigger parking straight at 'await_human'). Test fallout: devJobStore.pg.test.ts's default-value assertion updated (implement -> analyze); its unrelated requeueAtPhase test was coupled to the old default via a hardcoded `advancePhase(job.id, 'implement', ...)` -- switched to `job.phase` so it derives the real starting phase instead of assuming one. devPlatformPipeline.wire.pg.test.ts's now-stale comment corrected. Verified against the live omadia-dev Postgres (DATABASE_URL override): devJobStore.pg.test.ts 23/23, full test/devplatform/*.pg.test.ts 81/81. Two files (devPlatformPipeline.wire.pg.test.ts, goldenFixture.e2e.test.ts) fail when the ENTIRE test/devplatform/*.test.ts glob runs in one process but pass 100% in isolation -- confirmed pre-existing test-pollution unrelated to this change (documented in prior session notes), not introduced here. tsc --noEmit clean, eslint clean, `npm run build` succeeds.
…mplement
Marcel's ask, prompted by the very first job to actually run analyze (now
that job creation defaults there, previous commit): the ANALYZE tab showed
"No artifact for this phase yet" with zero visibility into what the agent
was doing during a real, running phase -- no peace of mind that anything
was happening.
analyze/bootstrap/plan/clarify run a real `claude -p` session or the
bootstrap command (phaseLoop.ts) and emit the exact same tool/log event
shapes implement does. The log pane was implement-only for no structural
reason -- toolCallLog.ts just never tracked which phase an event belonged
to, so there was no way to filter one flat stream per phase-tab.
`toolCallLog.ts`: replaced the bare `LogItem[]` accumulator with a
`LogState { items, phase }` that tracks a running phase cursor, updated on
every `phase` event (`{phase, state:'start'}` -- always the first event of
a provision) and stamped onto every subsequent tool/log item as it's
created. A tool call's result keeps the phase its OWN start happened in,
not whatever the cursor has moved to by the time the result arrives (new
test: phase moves on mid-call, the paired item still reports its start's
phase). `INITIAL_LOG_STATE.phase` defaults to 'analyze', matching
devJobStore.createJob's own new default.
`page.tsx`: `foldDevJobEvent` now folds into `LogState`; the phase tab body
renders `JobLogPane` for every phase except `pr` (which keeps its dedicated
PR-link view), filtering `logState.items` down to the currently VIEWED
phase via the existing `phaseToUi()` mapping. The now-fully-superseded
`noArtifact` empty-state (JobLogPane's own "no log yet" message covers it)
is removed from both message catalogs.
7 new tests for the phase-cursor behavior (starts at analyze, phase events
update the cursor without emitting an item, items are stamped per-phase,
result-keeps-start's-phase, an event with no phase field leaves the cursor
unchanged); existing pairing tests updated for the LogState shape.
Verified: tsc --noEmit clean, eslint clean (pre-existing unrelated warning
in DeviceFlowPanel.tsx, untouched by this change), i18n:check clean,
`npm run build` succeeds, 42/42 dev-platform vitest files green, 332/337
full suite green (same 5 pre-existing toolTemplates.test.ts failures as
every prior commit this session, unrelated file).
…iling
Root cause of "Error while bootstrapping?!" (job a3479368, byte5ai/omadia):
phaseRunner.ts's runBootstrap() hard-failed with "no bootstrap command
provisioned for this repo" whenever spec.bootstrap was absent -- which it
always is for a repo with no dev_repos.bootstrap_command configured (omadia
has none set). This made the entire bootstrap phase a guaranteed dead end
for any repo without an explicit override, now that jobs actually reach it
(previous commit fixed the phase-default bug that made this visible for the
first time).
Traced the full wiring first: `GET /jobs/:id/spec` (devRunnerApi.ts:404,
what the runner itself fetches) ALREADY correctly threads
repo.bootstrapCommand into spec.bootstrap when set -- that path was fine.
The gap is specifically "repo has no bootstrap_command AND the shim gives up
immediately" -- exactly the "null = auto-detect at runtime" case the type's
own doc comment (types.ts:325) names but nothing implemented.
(Note: an earlier version of this fix touched deriveJobPolicy.ts /
devRunnerJobPolicyRoute.ts, a completely different daemon-facing policy path
nothing consumes for bootstrap purposes -- reverted once the real /spec
route was confirmed already correct, to keep this change minimal.)
Fix, entirely shim-side (the middleware has no filesystem to inspect before
the repo is cloned -- only the runner, once the workspace exists, can look):
- New bootstrapDetect.ts: a small pure function mapping the cloned repo
ROOT's directory listing to a install command (npm ci / yarn / pnpm /
pip / pipenv / cargo / go, lockfile-over-manifest priority), or null.
Root-only by design -- a monorepo with per-workspace manifests and no
root manifest (omadia's own layout: middleware/package.json +
web-ui/package.json, nothing at root) won't match anything here, and
that's intentional rather than a guessed multi-directory heuristic.
- phaseRunner.ts's runBootstrap(): explicit spec.bootstrap.command still
wins; absent that, auto-detect from repoDir; absent BOTH, report ok:true
with a `{command:null, skipped:true}` artifact instead of failing the
whole pipeline -- not every repo needs a distinct install step, and an
undetectable one is not itself an error.
For omadia specifically: auto-detect finds nothing (no root manifest), so
bootstrap now gracefully skips rather than hard-failing -- unblocking the
pipeline. It does NOT install omadia's actual deps (a genuine per-workspace
`cd middleware && npm ci && cd ../web-ui && npm ci`, which the analyze
phase's own agent already derives and can run itself during implement).
Configuring an explicit bootstrap_command for this repo is a separate,
optional follow-up if pre-installing before implement starts is wanted --
deliberately not done here to keep this fix to the platform-level gap only.
New tests: bootstrapDetect.test.ts (13 cases covering every manager,
lockfile-priority, empty/unrecognized dirs, subdirectory-manifests are
correctly NOT detected). phaseLoop.test.ts +2 end-to-end cases (auto-detect
actually executes with repoDir as cwd; nothing detectable skips
gracefully). Full shim suite 67/67 green. tsc --noEmit clean, eslint clean,
`npm run build` succeeds.
…ilures
Root cause, found by a background agent that won the AutoRemove log-capture
race (docker events + docker logs armed before triggering a fresh
reproduction): job a3479368's bootstrap-phase crash was never a shim/daemon
bug. phaseRunner.ts correctly reported {ok:false, error:'no bootstrap
command provisioned for this repo'} -- byte5ai/omadia genuinely has no
bootstrap_command configured, a legitimate config gap, not an infra defect.
The actual defect: PhaseEngine.finalize's adapter in wireDevPlatform.ts
passed the failure reason as FinalizeContext.reason only:
finalize: (jobId, status, reason) =>
boundFinalize(jobId, status, reason !== undefined ? { reason } : undefined)
FinalizeContext has two distinct fields -- `reason` (status event payload
only) and `error` (dev_jobs.error column, finalizeDevJob.ts:135) -- and the
adapter's own comment even named `reason` landing "in the status event
payload" without noticing it therefore never reaches the job row. Every
event trail carried the real reason; dev_jobs.error was always NULL for
every gated-pipeline phase failure, this bootstrap crash included -- which
is exactly why this investigation kept hitting an empty error column no
matter how the failure was triggered.
Fix: pass `error: reason` alongside `reason` in the same adapter call, so
FinalizeContext gets both.
The background agent also found a second, structural bug in the same area
(finalizeDevJob.ts awaits container teardown via the daemon BEFORE the
runner's phase-result HTTP response is sent, so the runner is always killed
by external SIGTERM before it can react to its own directive) -- NOT fixed
here. It's a design question (should the finalize choke point used by every
terminal path -- stall, wall-clock, cancel, reaper -- guarantee the runner
gets to react first, or is racing it against teardown acceptable?) rather
than a small, obviously-safe change, and is being tracked separately.
New regression test in devPlatformPipeline.wire.pg.test.ts, against the real
wired platform (not mocked): posts an ok:false gated phase-result and
asserts dev_jobs.error carries the real reason, not just the event payload.
Confirmed it fails against the pre-fix adapter (actual: null) and passes
with the fix.
Verified against the live omadia-dev Postgres: devPlatformPipeline.wire.pg
.test.ts 5/5, full test/devplatform/*.pg.test.ts 82/82, full
test/devplatform/*.test.ts 566/566. tsc --noEmit clean, eslint clean,
`npm run build` succeeds.
…age.json Found live against byte5ai/omadia's real repo root, immediately after the previous two fixes finally let bootstrap run cleanly enough to expose it: `bootstrap exited with code 254`. dev_jobs.error (now populated, per the prior commit) named the real reason -- the previous commit's detectBootstrapCommand matched on package-lock.json ALONE and returned `npm ci`, but omadia's actual root has that lockfile (an 87-byte empty-packages stub, left over from before the repo moved to per-workspace- directory manifests -- middleware/package.json, web-ui/package.json, nothing at root) with NO matching package.json. `npm ci` fundamentally requires both files; running it against a lockfile alone fails outright. Fix: gate all npm-family lockfile checks (package-lock.json, npm-shrinkwrap.json, yarn.lock, pnpm-lock.yaml) behind package.json being present first -- a lockfile alone is not installable evidence, only a manifest+lockfile pair is. Non-npm ecosystem checks (requirements.txt, Pipfile, Cargo.toml, go.mod) are unaffected -- each file already fully signals its own ecosystem with no separate-manifest ambiguity. For omadia specifically: with this fix, detectBootstrapCommand now correctly returns null at its root (no package.json there at all), so bootstrap gracefully skips exactly as designed -- the two prior commits' graceful-skip path was already correct, this one just stops the detector from producing a bad command in the first place. New tests: two regression cases (lockfile without package.json for npm/yarn/pnpm all return null). Existing phaseLoop.test.ts end-to-end auto-detect test updated to seed BOTH package.json and package-lock.json (its original intent -- a real detectable npm project), since it was unknowingly exercising the exact buggy case this fix closes. Full shim suite 69/69 green (was 67, +2 new cases). tsc --noEmit clean, eslint clean, `npm run build` succeeds. Live-verified against the actual failure: rebuilding+reloading the runner image under the correct tag (`omadia-dev-runner:latest` -- the daemon's actual configured DEV_RUNNER_DEFAULT_IMAGE, NOT the ghcr.io/byte5ai/-prefixed default the compose file's own comment implies; earlier rebuilds this session went to the wrong tag and were silently never used) reproduced exit 254 exactly as described before this fix.
…tab switch
Marcel's ask: the GATE step showed only "No log output yet" with no way to
act — approving a plan required navigating to a separate Freigaben
(Approvals) tab, losing the job's own context. Then: show the plan content
itself inline too, not just a link to open it elsewhere.
`GateInbox.tsx`'s `GateCard` already had all the resolve logic (questions,
note, approve/reject, holder/conflict/error states) -- reused rather than
duplicated:
- Exported it with a new `compact` prop (drops the job-id header and the
outer bordered card; the job-detail page already shows both).
- Added inline plan-content fetching: a new `getArtifactText()` API
helper (`GET /artifacts/:id` returns text/plain, the existing `req()`
JSON wrapper doesn't fit) plus a small state machine
(loading/ready/error/none) rendered as a scrollable pre block. The
"Plan ansehen" link stays as a secondary affordance (new tab, full
view) alongside the inline preview.
`page.tsx`: fetches the job's own waiting gate the same way
`DevJobChatCard.tsx` already does for the chat surface
(`listWaitingGates()` + `findGateForJob()`), and renders `<GateCard compact
/>` on the GATE phase stop instead of the (always-empty, since nothing runs
during the human wait) log pane. Falls back to the log pane if the gate
hasn't loaded yet. On resolve, refetches the job so phase/status flip
correctly and the gate view naturally clears.
Both fetch effects derive their "reset" value (`gate`/`planText` = null/none
when the precondition isn't met) instead of calling setState synchronously
in the effect's early-return branch, avoiding
react-hooks/set-state-in-effect entirely for the gate fetch; one remaining
warning on the plan-fetch's "kick off loading" line matches an existing,
already-accepted pattern elsewhere in this codebase (DeviceFlowPanel.tsx) --
not introduced fresh here, not a new class of issue.
New i18n keys (planLoading/planLoadError) in both en.json/de.json per this
project's hard i18n rule.
Verified: tsc --noEmit clean, eslint clean except the one pre-existing-
pattern warning noted above, `npm run i18n:check` clean (no new
untranslated-key warnings), `npm run build` succeeds. Live-verified against
a real waiting gate (job a3479368, ticket #445): the plan's full JSON
artifact renders inline on the job's own GATE stop with holder/deadline info
and working Ablehnen/Plan-freigeben buttons, no tab switch.
… JSON Marcel's ask, immediately after the plan finally rendered inline: it showed the raw fetched text verbatim -- a single JSON blob whose string fields carry escaped \n sequences (JSON's own escaping for a real newline inside a string literal), so multi-paragraph fields like `approach` rendered as one dense wall of text with literal backslash-n characters instead of actual line breaks. `filesToTouch` printed as a JSON array literal instead of a list. New PrettyArtifact.tsx + prettyArtifact.ts: JSON.parse the artifact text (this is the fix for the \n problem -- parsing turns the escape sequence into a real newline byte, which `white-space: pre-wrap` then wraps correctly) and render each top-level field by its own JS type -- a string as a wrapped paragraph, a string array as a bullet list, other values as a small indented JSON block. Fully generic per field rather than a per-kind template, since artifact shape varies by kind (plan/analysis/ bootstrap_report/...) and isn't rigidly typed client-side. Falls back to the raw text verbatim when the content isn't parseable JSON or isn't a plain object at the top level -- never worse than the previous behavior. Wired into GateInbox.tsx's inline plan view (the previous commit's work) in place of the raw <pre> block. New tests (prettyArtifact.test.ts, 7 cases): the load-bearing one confirms JSON.parse actually turns `\n` escapes into real newline characters; others cover malformed JSON, top-level arrays/primitives/null (all correctly fall back), and that nested arrays/objects pass through as-is for the component to render. Verified: tsc --noEmit clean, eslint clean (same one pre-existing-pattern warning as the prior commit, unchanged), 49/49 dev-platform vitest files green, i18n:check clean, `npm run build` succeeds.
… agent_done
Found by a background agent investigating why job a3479368's implement
phase failed with the generic `error='implement session exited with code
1'`: the stored dev_job_events for that failure showed the CLI go silent
for 14 minutes, then emit a `result` line that got translated to a
completely normal-looking `status:{state:'agent_done', usage:{...}}` --
and the underlying claude CLI process STILL exited non-zero right after.
Nothing in the event trail explained why; the failure was only visible via
the exit code, with zero context attached.
Root cause: eventTranslate.ts's handleResult() unconditionally mapped every
`result` NDJSON line to `agent_done`, never inspecting the CLI's own
`subtype` ('success' | 'error_max_turns' | 'error_during_execution' | ...)
or `is_error` field. A `result` line is not automatically success -- if the
CLI's own result was itself an error subtype, the event stream displayed it
identically to a clean success, masking exactly the information that would
explain a later non-zero exit.
Fix: handleResult() now checks `is_error`/`subtype` and emits a distinct
`status:{state:'agent_error', subtype, errorText, usage}` when the CLI's own
result indicates an error, instead of blindly reporting agent_done. Purely
additive -- confirmed nothing in the shim's own control flow or the
middleware gates on the exact status `state` string (grepped both; zero
hits), so this can't regress anything, only add detail. Doesn't touch the
already-correct exit-code-based fail gate in phaseRunner.ts -- this closes
an observability gap, not a functional bug in the pass/fail decision itself.
New tests (5): explicit subtype:'success' still reports agent_done (not
falsely flagged); a non-success subtype reports agent_error with
subtype+errorText+usage; is_error:true alone (no subtype) also triggers it;
errorText is omitted rather than a placeholder when the CLI's result has no
text payload. Full shim suite 73/73 green (was 69, +4 new -- one of the 5
listed IS the pre-existing baseline test, confirmed unchanged). tsc
--noEmit clean, eslint clean, `npm run build` succeeds.
This doesn't explain the CLI's own exit-1 behavior (that's inside the
`claude` binary, out of this codebase) -- it makes the next occurrence
self-diagnosing straight from dev_job_events instead of requiring another
expensive live reproduction to even see what happened.
Weegy
enabled auto-merge (squash)
July 28, 2026 13:34
…gress proxy
Found live: `docker logs` on the egress-proxy container showed an uncaught
exception mid-run --
Error: read ECONNRESET
at TCP.onStreamRead (node:internal/stream_base_commons:216:20)
Emitted 'error' event on Socket instance at: ...
-- followed by the container's own restart. In the same window the daemon
logged `PUT .../jobs/<id> failed: fetch failed` for a DIFFERENT job's
control-plane call, and the job under investigation (a3479368, ticket #445
implement phase) reported npm install "stalled, node_modules not growing" --
all fallout from the SAME event: the proxy's control plane was unreachable
while it crashed and restarted.
Root cause: `dataServer.on('connect', (req, socket, head) => ...)` hands
`handleConnect` a raw net.Socket with NO 'error' listener attached.
handleConnect only adds one (`clientSocket.on('error', teardown)`) on its
success path -- well after the allowlist decision AND the `await
resolve(host)` call. A client that resets the connection (ECONNRESET) at
ANY point during that async window fires an unhandled 'error' event; Node's
default for a listener-less EventEmitter 'error' event is to throw, which
took down the ENTIRE proxy process -- and with it, egress for every OTHER
concurrent job, not just the one whose client reset.
Fix: attach a defensive `socket.on('error', ...)` listener unconditionally,
as the very first thing in the 'connect' handler, before handleConnect (or
any DNS/allowlist work) even starts. handleConnect's own later listener
still gets added once the tunnel exists -- multiple listeners on the same
event coexist harmlessly in Node, and `destroySocket` is already idempotent.
New test reproduces the exact crash deterministically: opens a CONNECT
against an allowlisted host whose DNS resolution is held open indefinitely
(the existing `resolveHangs` test seam), waits for the socket to land in
that vulnerable pre-tunnel window, then calls `resetAndDestroy()` to send a
real TCP RST (not just a clean FIN) -- reproducing ECONNRESET server-side --
then proves the SAME process is still alive by making a completely separate,
successful request afterward (a crashed process cannot execute that
assertion at all). Confirmed: reverting just the fix reproduces the exact
live crash signature verbatim (`Error: read ECONNRESET ... at
TCP.onStreamRead`) and fails the test; restoring the fix passes it.
Full daemon suite 396/396 green (was 395, +1 new). Pre-existing eslint
no-undef errors for Node globals (setTimeout/Buffer/URL/process/console)
throughout this whole .mjs package are unrelated -- confirmed identical on
the unmodified file via git stash before touching anything, a package-level
eslint-env gap, not introduced or worsened here.
Separately: registry.npmjs.org (or any package registry) is not in this
deployment's DEV_EGRESS_BASE_ALLOWLIST at all -- that env var is unset, so
`npm install`/`npm ci` inside a job container has no route through the
proxy regardless of this crash. Worth an explicit operator allowlist entry
as a follow-up; not changed here since it's a deployment config choice, not
a code defect.
…ring' into fix/dev-platform-runner-image-wiring
Separate contributing cause to the same "install is stalled" symptom the
egress-proxy crash fix (previous commit) also explains: DEV_EGRESS_BASE_
ALLOWLIST is unset in this compose file, and config.ts's own default for it
is empty. That's correct behavior for a repo needing no package install at
all, but wrong for this one -- an npm-workspaces repo where bootstrap now
auto-detects `npm ci`/`npm install` (bootstrapDetect.ts) when no explicit
bootstrap_command is set, and where an implement-phase agent may
legitimately run its own install too. Neither has any route to
registry.npmjs.org without this, and the egress proxy's default-deny means
every attempt fails -- but npm's own connection-retry resilience makes that
failure look like an indefinite hang rather than a fast, clear rejection,
which is exactly what the screenshot showed ("install is stalled,
node_modules not growing", agent had to manually detect and kill it).
Sets DEV_EGRESS_BASE_ALLOWLIST=registry.npmjs.org (operator-overridable via
the same env var, matching every other tunable in this file). Verified the
merged compose config parses correctly and the running middleware picks it
up after a restart.
Weegy
disabled auto-merge
July 28, 2026 13:39
Found live while verifying the two previous commits (egress-proxy crash fix
+ npm registry allowlist): a real `npm ci` bootstrap ran for a genuine 70
real seconds against the now-fixed proxy (no more hang -- those fixes
worked) but then failed with exit code 1, and there was NO way to see why.
`bootstrap_report` only ever recorded `{command, exitCode, timedOut,
durationMs}` -- npm's own error output was gone.
Root cause: `runCommand()` spawned the child with `stdio:
['ignore','pipe','pipe']` but never attached a listener to either pipe.
Piped-but-unread streams are simply discarded by Node -- not buffered
anywhere, not inherited to the container's own stdout/stderr either (so
`docker logs` structurally cannot see them, confirmed by a background agent
investigating a separate crash earlier in this same chain). A bootstrap
failure has been completely opaque since this code existed: exit code only,
zero diagnostic content, for every possible cause.
Fix: read both streams, bounded to a trailing 4KB (the tail matters most --
npm/pip/etc. print their actual error at the END of their output, and
unbounded capture risks a memory/artifact-size blowup on a verbose or
runaway command), and include it as `outputTail` in the bootstrap_report
artifact. Bounded while the command is STILL RUNNING too, not just at
completion, so a chatty long-lived process can't accumulate unboundedly in
memory first.
New tests: stdout AND stderr both land in the report on a real failure;
output well past the cap is truncated to a bounded tail while the
diagnostic end of it survives. Existing bootstrap tests use substring regex
matching on the report JSON (not exact deep-equality), so the new field is
additive and didn't require changing any of them.
Full shim suite 75/75 green (was 73, +2 new). tsc --noEmit clean, eslint
clean, `npm run build` succeeds.
…m analyze POST /jobs/:id/retry?resumeFromPhase=true clones the job starting at the source job's own dev_jobs.phase instead of always restarting at 'analyze'. This is safe by construction: dev_jobs.phase is exactly the value the dev-runner-shim reads to decide where to begin (protocol.ts's ProvisionSpec.phase doc), so handing a new job the old job's last-attempted phase reproduces a starting point the runner already knows how to execute, no new runner-side code path needed. Artifacts from phases that already succeeded are copied onto the new job's own row (getLatestArtifact reads by job id), so a later phase (e.g. plan reading the analysis artifact) still finds what it needs. Motivation: every prior retry during this epic's live npm-bug investigation re-ran the full analyze phase (~$1.72 in LLM tokens each time) just to get back to testing the free, no-LLM bootstrap step. Default behavior (no query param) is unchanged.
…ncurrent same-host lookups The proxy's resolve() called dns.lookup() fresh on EVERY CONNECT, with no caching or in-flight de-dup. dns.lookup() runs on Node's libuv threadpool (default 4 workers, never tuned in this image). npm's own registry traffic fires up to `maxsockets` (default 15) concurrent CONNECTs to the SAME hostname (registry.npmjs.org) for package tarballs, each independently re-resolving a host that another concurrent fetch just resolved milliseconds earlier. This unifies three previously-inexplicable live symptoms (epic #470, 2026-07-28) under one root cause: - default npm concurrency: deterministic crash ~70s into npm ci with npm's own 'Exit handler never called!' bug (npm/cli#9751 — a race triggered by near-simultaneous registry-fetch timeouts) - --maxsockets 1000: same contention pushed past the 5s resolve deadline -> literal EAI_AGAIN - --maxsockets 3 (lowered): less contention, only delayed the same crash (70s -> 251s) Fix: a 30s TTL cache + in-flight de-dup, keyed by hostname, shared across jobs. N concurrent CONNECTs to the same host now share ONE underlying lookup. Failed lookups are never cached. The rebinding defence (resolve-once/connect-to-what-you-checked) is unaffected — every CONNECT still classifies+pins its own resolved addresses, only the lookup itself is reused.
…their egress allowlist
Root cause of npm's 'Exit handler never called!' bug (confirmed live,
2026-07-28): npm's own HTTP client (@npmcli/agent) resolves its target
hostname LOCALLY before/alongside going through the CONNECT proxy.
Confirmed with a direct in-container test: dns.lookup('registry.npmjs.org')
fails in 4ms with EAI_AGAIN — Docker's embedded resolver (127.0.0.11) has
no upstream route for external names inside the job's isolated per-job
network (spec section 6's DNS-exfil defence: no direct internet DNS by
design). Hit for every concurrent package fetch, this triggers npm's own
confirmed ExitHandler re-entrancy race (npm/cli#9751). This is NOT specific
to npm ci, npm's version, or maxsockets — ANY tool doing local resolution
of an allowlisted host hits the same wall (confirmed: npm install -g of a
single package fails identically).
An earlier attempt to fix this by caching DNS resolution INSIDE the egress
proxy (commit 3be2800) had zero effect, because the proxy's own resolve()
was never the bottleneck — it only ever saw 3 successful calls for the
entire failing install, confirmed via temporary diagnostic logging. The
failures were happening entirely inside the job container's own network
namespace, invisible to the proxy.
Fix: the daemon already knows a job's exact egress allowlist before the
container starts (it registers it with the proxy's control plane). It now
ALSO pre-resolves each allowlisted hostname with its own (real, unsandboxed)
DNS and passes the result as Docker's ExtraHosts, giving the job container
static host:ip entries. Any tool's local resolution of an allowlisted host
now succeeds immediately via /etc/hosts, with zero new egress capability —
every entry names a host the job could already reach through the CONNECT
proxy; this only makes LOCAL resolution of that SAME host succeed too.
A host that fails to resolve is skipped, not fatal — the CONNECT tunnel
path (the proxy's own independent resolution) still works for it regardless.
clamp.mjs's buildContainerCreateOptions gains an extraHosts parameter,
applied as HostConfig.ExtraHosts and added to the clamp's own 'exactly
these keys' allowlist test — distinct from the still-forbidden Dns field
(a general resolver override would be a real allowlist bypass; this is
not, since it only pre-answers hosts already permitted).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two fixes to the dev-platform, found while running the very first real job through the freshly-deployed local stack (epic #470).
1. Every real job died instantly at
implement(the root cause)The daemon's
GET /internal/job-policy/:jobIdcallback — fetched at provision time to get the job's image/env/egress policy — has 503'd for the life of the epic:docker-compose.dev-platform.yamlnever set a runner-image env var on the middleware container, only on the daemon (asDEV_RUNNER_IMAGES). Without a resolved image,wireDevPlatform'sjobPolicyConfignever builds.analyze/bootstrap/plan/clarify/gateall run without ever provisioning a container, so they succeeded —implementis the first phase that callsDockerBackend.provision(), and that's exactly where every job failed, with zero tokens spent and a generic daemon-side 502 (the middleware could not supply the job policy).Fix: wire
DEV_RUNNER_DEFAULT_IMAGEonto the middleware from the sameDEV_RUNNER_IMAGEsource var the daemon's allowlist already reads, and makeindex.ts's runner-image resolution consistent between the Fly and Docker backends (previously only the Fly path fell back through both env var names). Two newcomposeTopologytests assert the middleware carries a runner image and that it names the same one as the daemon's allowlist, so this exact gap can't silently reopen.2. No way to delete a job
There was no way to remove a finished job from the operator's list short of the daily retention sweep. Added
DELETE /jobs/:id(terminal jobs only — 409 refuses an active job so a live backend handle is never orphaned) plus matching delete actions on the job list and job-detail page.Verification
GET /internal/job-policy/:jobIdon the actual failed job returned503 devplatform.daemon_not_configuredagainst the deployedomadia-devstack.npm run i18n:checkOK (3136 keys).devJobStore.deleteJob(terminal/active/missing),DELETE /jobs/:idroute (204/409/404),composeTopologyrunner-image wiring.Test plan
omadia-devstack from this branch and re-run the exact job that failed (issue Add iMessages-Channel #410 style) — confirm it now reachesimplementand actually provisions a container.failed/donejob from the Jobs list and from a job's detail page; confirm 409 if attempted on a running job.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.