feat(scripts): spec-driven wave workflows + W0 unit manifest (epic #470)#476
Merged
Conversation
…470 Adds the three-script execution flow for building a large epic from wave specs, in the idiom of the existing issue-*.workflow.mjs scripts: - wave-decompose: spec -> conflict-checked unit manifest. Extraction fans out by slice; the dependsOn reduce is a barrier because it needs every unit at once. Hub files (index.ts, config.ts, messages/*.json) are subtracted from the collision signal and assigned to a single wiring unit, so a merge conflict becomes a dependency edge. - wave-implement: worktree-isolated Engineer/Anvil per unit, then a cross-family Forge refutation and a Cato security audit on sensitive units. Colliding or dependent units run sequentially on one branch; disjoint units pipeline. Undeclared spec drift is a review failure; declared drift is expected and reported back as specDeltas. - wave-verify: acceptance checklist, then an independent refuter and a security auditor. A criterion passes only when the checker and the refuter agree. End-to-end and UI criteria report as 'manual' rather than silently passing. None of the three writes to GitHub or pushes; the caller owns the remote-write choke point, and the human gates force the three-script split because a Workflow script cannot call AskUserQuestion. docs/dev-platform/w0-manifest.json is the W0 decomposition, derived from the W0 spec: 11 units, verified acyclic, 8 security-sensitive.
Migration 0021_dev_platform.sql: dev_repos, dev_jobs, dev_job_events, dev_job_artifacts. Load-bearing details, each of which a spec review caught: - dev_job_events.id is a BIGINT IDENTITY and the only ordering key (SSE id: and Last-Event-ID read it). A provision column plus UNIQUE(job_id, provision, seq) prevents the cross-provision collision that would otherwise discard a gated job's entire second-provision event stream under ON CONFLICT DO NOTHING. - No CHECK on dev_job_events.type or dev_job_artifacts.kind: both enums grow in W1-W3 and are validated in TypeScript instead. A CHECK on a growing enum is a liability. The remaining six enums stay DB-constrained. - status is 'applying', not 'pushing': the middleware moves the ref, not the runner. kind drops the phantom 'file_issues'. claimed_by is UUID. types.ts: RUNNER_PROTOCOL_VERSION, the unions, and their runtime validators — now the only enforcement for type/kind. Each union and its validator derive from one 'as const' array so they cannot drift. DevJobSpec carries no credential field; the clone token is fetched at git time and is read-only. Reviewed by Forge (cross-family): applied the migration twice against Postgres 16, introspected pg_constraint, compiled under strict + noUncheckedIndexedAccess. No findings. Refs #470
DevJobStore over Postgres: FOR UPDATE SKIP LOCKED claim, lease-fenced worker writes raising a typed DevJobLeaseLostError on a stale lease, append-only events keyed (job_id, provision, seq) and ordered by the IDENTITY id. Queue state lives in Postgres from day one; the builder's in-memory BuildQueue is deliberately not copied, because a restart must never orphan a job. finalizeDevJob is the single terminal-transition choke point. The store's terminal write is guarded by an unexported brand symbol that only finalizeDevJob.ts imports, so no sibling module can mark a job terminal — a runtime guard, where a TypeScript 'private' would merely have made the sole legitimate caller unable to reach it. W2 fills the revocation hook; the seam exists now so W2 does not retrofit it. jobToken: sha256 at rest, timing-safe verification that does not throw on a length mismatch. Split out of the store to stay under the 500-line rule: devRepoStore (repo CRUD) and pgMappers (shared pg coercion). Verified against a throwaway database on the local Postgres with the real migration runner: 26 tests, including two concurrent claims returning disjoint jobs, a stale lease raising, and appendEvents accepting the same seq under a different provision. Refs #470
… read, brief devRepoCredentials: per-repo tokens in the vault namespace core:dev-platform, never in Postgres and never in a browser-facing shape. clear() purges all three keys. Device-flow tokens stage under pending/<sub>/ until the repo row exists. branchProtectionCheck: tri-state. 200 protected, 404 unprotected (the UI warns loudly), 403 could-not-verify — classic device-flow tokens lack admin read, and reporting that honestly beats reporting 'unprotected'. githubIssuesTracker: the issues endpoint also returns pull requests; they are filtered on the pull_request key. briefComposer: the ticket body is wrapped in explicit untrusted markers, and a body line reproducing a marker is neutralised — otherwise the reporter closes the block early and speaks as omadia. This is framing, not enforcement, and the code says so: W0's actual enforcement is that the runner holds no write credential at all. The test drives an adversarial ticket that tries exactly that. Reviewed by Forge (cross-family), which ran every marker-collision variant against the real composer — exact, whitespace-padded, extra-dash, truncation boundary, and marker-in-title — and found no early close. No findings. Refs #470
The runner uploads a diff and never pushes. The middleware reconstructs the commit from the diff it validated, so what was reviewed and what was committed are the same object. This unit carries that guarantee for the whole epic. apply(): parse -> numstat cross-check -> path-escape refusal -> policy verdict (a seam returning allow in W0; W3 adds rules) -> blobs -> tree with base_tree at the job's pinned base_sha -> commit -> create a fresh ref -> open the PR. Validation is a read-only first phase; the first mutating call happens only after every check passes. applyHunks verifies each deletion and context line against the base before consuming it, and fails closed with DiffContextMismatchError. Without that check the applier was positional: a diff claiming to delete 'harmless' would delete whatever sat at that line number, while the numstat totals still matched. Hunk bodies must also consume exactly the counts their header declares, or DiffStructureError. Both attacks were reproduced against the original code and now abort before any write. Content that reaches a tree is never runner-supplied: modified files are reconstructed from the base blob fetched at base_sha. Symlink (120000) and gitlink (160000) modes are refused; a declared 100755 is preserved. Binary content is refused rather than fabricated, because a textual diff carries no bytes. '.git/**' is refused locally rather than relying on the forge to reject it. Two findings from my own audit of the reviewed code: - The branch name was taken from the caller. assertJobBranch now enforces the omadia/job-* prefix here, so 'only a fresh job ref is ever created' is a property of this code rather than of whoever calls it. - DiffContextMismatchError put the base line in its message. That message reaches dev_jobs.error and event payloads, which the admin UI renders and SSE streams; a base line belongs to the customer's repository and may be a secret. The lines stay on the error object; the message no longer carries them. Reviewed by Forge (cross-family), which found the positional-apply hole and reproduced it. A same-family reviewer shares the implementer's assumption that reconstructing from base plus diff is sufficient. Refs #470
The store's brand symbol is exported, so any module could import it and call finishTerminal directly. It prevents an accidental terminal write, not a deliberate one. The earlier comment (and my commit message on a4a28d6) claimed no sibling module could mark a job terminal; that overstates it. The invariant is upheld by review, and the brand makes a violation loud rather than silent. A comment that asserts a security property the code does not enforce is worse than no comment: the next reader trusts it and removes the real control. Refs #470
6 tasks
createDevRunnerRouter, mounted at /api/v1/dev-runner. Deliberately not behind requireAuth: the sole authentication is the per-job bearer token, verified against its stored sha256 with a timing-safe compare. index.ts applies requireAuth per router rather than as a blanket /api guard, so this mount is correct as designed. Six routes per spec §4. GET /spec carries the protocol version and no credential field (asserted by a recursive key scan and a raw-text sentinel, a regression test for a real spec bug). GET /scm-token is read-scoped and one-shot per provision. POST /events validates event types in TypeScript, because 0021 deliberately dropped the DB CHECK, and is idempotent per (job, provision, seq). POST /result routes failed and no_changes through finalizeDevJob, never the store, and a spy on the choke point proves it. Every body is size-capped and every field treated as hostile: the runner sits inside the blast chamber. No error echoes the bearer, an upstream body, or a stack. Two fixes on top of the implementation: - DevJobStore gains touchHeartbeat. appendEvents bumps last_heartbeat_at only alongside an event batch and returns early on an empty one, so an agent that thinks for two minutes without emitting a tool call would have been reaped by findStalled while perfectly healthy. The router first carried this as an optional injected hook that the wiring unit was expected to supply; an optional liveness bump is a trap, so it is now a required method on the injected store interface and cannot be omitted. Status-guarded, so a half-dead runner cannot revive a terminal job by heartbeating. - req.params[k] is 'string | string[]' under this express typing. A repeated :id arrives as an array; the auth middleware now treats anything but a single string as unauthenticated rather than coercing it. The implementation reported a clean typecheck; the filter it used hid these two errors. Refs #470
Adversarial review of 9bc5f6c rejected the unit with two majors, both reproduced against the real router. This router is the only surface a compromised runner can reach; two of its stated guarantees held only when the runner cooperated. POST /events never bound the client-supplied provision to the job. Idempotency is (job_id, provision, seq), so a runner sending provision 999, then 1000, replays the same seq forever and defeats dedupe entirely. The provision must now equal the job's own; a real runner reads it from GET /spec, and a re-provision bumps it server-side. The existing test codified the defect — it sent provision 2 for a job whose provision was 1 and asserted acceptance — so it now simulates a real re-provision, and a mismatch is a 409. POST /result never checked that diffArtifactId belonged to the job. A runner naming another job's artifact would have had that diff committed and opened as this job's pull request, which is precisely the guarantee the epic rests on. Artifact ids are opaque, so this is defence in depth — but the check belongs somewhere rather than in each layer's assumption about the other. Also: the scm-token one-shot guard was a check-then-await-then-add TOCTOU, and resolve() is a Vault round-trip; two concurrent requests both got a credential. W0 resolves the same static credential so nothing new leaked, but W2 swaps in a scoped token minted on demand and the race would mint two. It now reserves before the await and rolls back on failure. diff_ready without a diffArtifactId is refused. seq and provision are bounded to int4, and a single event payload is capped, so a hostile runner cannot force 500s or store a 4 MiB payload verbatim. Three guarantees were asserted in comments but never tested: auth before body parsing, the one-shot under concurrency, and artifact ownership. Each now has a regression test that fails against the previous code. Splits, to honour the 500-line rule that 9bc5f6c had already broken at 522 lines: artifact access moves to devJobArtifactStore.ts, and the router's test harness to devRunnerApi.harness.ts with the adversarial cases in devRunnerApi.hostile.test.ts. 114 tests green. Refs #470
… only SSE route createDevPlatformRouter, mounted behind requireAuth at /api/v1/admin/dev-platform. GET /jobs/:id/events is the ONLY job-event SSE route in the epic: W3's chat card consumes this one rather than standing up a second. The SSE id: field is dev_job_events.id, the server-assigned IDENTITY column, never the runner's seq. A reconnect with Last-Event-ID therefore resumes correctly across a provision boundary, which a seq-keyed stream could not — the two provisions of a gated job both start their seq at zero. A test drives that exact reconnect. Launch authorization: POST /jobs requires the caller to be the repo's created_by or hold one of allowed_launchers. An empty list means created_by alone, so the gate is fail-closed. Without it any operator session launches jobs against any registered repo, which does not survive an enterprise deployment. Note the implementation matches allowed_launchers against the caller's role, sub, and email, where the spec named only role keys — an extension, not a hole. Auth-mode admission (the question-4 decision): subscription is refused when the mode flag is off, when the repo's tests execute, when the source is not admin, and on the fly backend. The route gives a good error; the worker will enforce the same check as the boundary. The local backend is likewise refused for a runs_tests repo and for any non-admin source. Cancel routes through finalizeDevJob, never the store. Repo and job views carry a credential status, never a token; a recursive key scan asserts it. Split for the 500-line rule: repo onboarding into devPlatformRepos.ts, shared guards and views into devPlatformShared.ts, the test fakes into devPlatformRoutes.harness.ts. 131 tests green. Refs #470
…ap, scm-token rollback Re-review of the hardened router passed both majors closed. Three minors: - The per-event payload cap measured JSON.stringify(payload).length — UTF-16 code units, not bytes — while the field, default, and message all say bytes. A CJK payload slipped ~3x past the nominal cap. Now Buffer.byteLength, matching POST /diff. A multibyte regression test asserts a 60-char CJK payload (180 bytes) is rejected under a 128-byte cap where .length would have passed it. - The scm-token reservation rolled back on a resolve() throw and a missing credential but not on a failed response send, so a client that dropped the socket mid-body left the provision un-issuable until restart. Now the send is inside the try; a legitimate same-provision retry works. - The real store's uuid pre-filter on artifactBelongsToJob was exercised by no test (the fake keys off art-N ids). A pg test now asserts a real addArtifact id is a uuid, is accepted for its own job, and rejected for a foreign job and a non-uuid. 131 tests green. Refs #470
…lugin_verdict via PR #477)
Contributor
Author
|
Heads-up: merged current |
… finding) Adversarial review of the admin routes proved a cross-operator authorization gap by executing it: as operator 'bob' it read alice's job and streamed her agent logs, and cancelled her running job. The launcher gate sat on POST /jobs and /retry only; the other seven job routes — GET /jobs, GET /jobs/:id, the artifacts routes, and the epic's single SSE stream — authorized by session presence alone. /retry gating while /cancel did not was the tell that this was an oversight, not a design. The gate that protects who may START a job now equally protects who may read, stream, cancel, or apply one. loadAuthorizedJob loads the job and checks the caller against its repo; a missing job and an unauthorized one return the same 404, so it is not an oracle for which ids exist. GET /jobs filters to repos the caller may launch. GET /jobs/:id/events authorizes before opening the stream — its events carry log, tool, egress, and token payloads. Four cross-operator tests now assert bob gets 404 on read, cancel, and SSE (with no log payload in the body), and that the list is scoped per operator. Two further review minors: - isPermittedLauncher matched allowed_launchers against the caller's role, sub, AND email. Spec §6 defines them as role keys; folding in sub/email is a cross-namespace match that could grant launch to a caller whose opaque sub or IdP-controlled email happened to equal an entry. Narrowed to role keys plus the creator's sub. - A failed POST /repos left the staged device-flow token parked in the vault pending slot with no repo to clean it up. The error path now clears it. The load/authorize helpers moved to devPlatformShared.ts to keep devPlatform.ts under the 500-line rule. 135 tests green. Migration is 0022 (0021 is plugin_verdict, merged to main via #477; the branch merged main and renumbered). Refs #470
…ees, Engineer fallback for Anvil, crash-safe Forge/Cato stages
New workspace package `@omadia/dev-runner-shim` (Node builtins only, never imports middleware). Implements the spec §5 runner: fetch spec with a loud protocol-version gate, read-only clone at the pinned base_sha, drive the headless Claude CLI, translate its stream-json to runner events, and upload a diff — the shim holds no write credential and moves no ref. - protocol.ts: baked RUNNER_PROTOCOL_VERSION, DevJobSpec/event/result contract (deliberately duplicated from middleware; the version check catches skew), env reader. - homeClient.ts: phone-home HTTP client + HomeApi seam; never echoes the bearer. - gitOps.ts: clone via a 0600 git-credential-store file created outside the work tree and deleted in finally — token never in env, argv, or .git/config; https-only; stage + diff/numstat; no push anywhere. - eventTranslate.ts: CLI NDJSON → the documented event table (system/init, coalesced text, tool_use/tool_result, result); 2 KB preview caps. - agentRunner.ts: spawn with an ALLOWLIST env (keeps ANTHROPIC_BASE_URL — does not reuse CLI_ENV_SCRUB_KEYS), prompt on stdin, stderr → log events, 1 s/50 batching. - diffUpload.ts: self-describing numstat-marker bundle so the host splits the one text artifact back into diff + numstat. - index.ts: lifecycle with heartbeat/cancel, serialized per-provision event seq, best-effort terminal result. Tests (node:test): gitOps.test.ts is the verifiedBy — a recording fake-git proves the credential/no-push guarantees and greps the bundle; plus event-table, agent-runner (fake CLI: stderr→log, stdin prompt, allowlist env), and lifecycle (protocol mismatch names both versions, clean/dirty paths).
…s (persona agents drop StructuredOutput on long runs)
Epic #470 W0, UI spec (issue comment 4922825984). Adds the operator-facing admin surface for the dev platform: - /admin/dev-platform hub: repos | jobs tabs (deep-linkable via ?tab=), repo table with credential-mode text colors, protection verdicts and first-run empty panel; job table with the spec's exact status->token mapping, waiting/failed row edges, client-side filters and a 5s poll as the W0 live-update fallback - add-repo wizard (own page): device-flow quick start with text-only polling state (user code focal, .lume-busy-dots, no spinner), the --warning-edge plain-language trade-off block, PAT fallback, and the branch-protection verdict (warns loudly, never blocks) - job detail: DevJobPhaseRail (eight stops, role=tablist, roving tabindex, Left/Right/Home/End keyboard support, aria-current=step vs aria-selected kept distinct, ?phase= deep link), live log pane on useStickToBottom + ScrollToBottomButton, SSE via new useDevJobEvents (modeled on useSpecEvents), metadata sidebar via useFormatter() - shared promoted components under app/_components/devjobs/ (DevJobPhaseRail, DevJobStatusText — single source of the status map) - every string in messages/en.json + de.json (i18n:check green) Lume invariants hold: state is text color + edges only, no fills, no spinners, no toasts, no skeleton shimmer; only Button variants.
…ed HOME, userinfo-URL guard, wall-clock kill - LLM auth passthrough is now gated: OMADIA_ANTHROPIC_* crosses into the child CLI only when the backend plumbs OMADIA_LLM_ENV_ALLOWED=true (set exclusively under the W0 jail acknowledgment DEV_PLATFORM_UNSAFE_LOCAL); buildAgentEnv refuses to wire ANTHROPIC_* without llmEnvAllowed even when a caller supplies a token. W1 per-job proxy tokens replace this. - Child HOME is ALWAYS job-scoped (workspace/home, mkdir'd by the shim, cwd fallback) — the parent HOME with the runner user's ~/.claude credentials is never inherited. - cloneAtBaseSha rejects clone URLs with embedded userinfo before fetching a token or invoking git (argv/.git-config credential bypass). - spec.limits.wallClockMs is enforced: timer kills a hung CLI (SIGTERM, then SIGKILL after killGraceMs), streams a budget_exceeded status event, fails the job with the budget named, exits 1. - Tests updated/added for all four: gated env (no 'bearer' leak codified), HOME-in-workspace + parent-HOME canary, userinfo rejection with no git invocation, wall-clock kill with a sleeping fake CLI.
…ailed LocalProcessBackend runnerBackend.ts re-exports the canonical seam from types.ts and adds the admission-bearing DevJobProvisionContext (source, repo.runsTests) plus a typed RunnerBackendError; provision() fails closed when the admission facts are missing. LocalProcessBackend (spec #470 W0 §1/§5): - constructs only under the explicit DEV_PLATFORM_UNSAFE_LOCAL=true acknowledgment (boot warning naming the restriction) and a dedicated unprivileged uid (never 0) - refuses runs_tests=true repos and any non-admin source at its own boundary - shim env is ALLOWLIST-built from nothing: PATH/LANG/TERM, a job-scoped HOME (never the parent HOME), the five OMADIA_* shim inputs, and the OMADIA_LLM_ENV_ALLOWED gate set if and only if the jail acknowledgment is present — no Vault path, DATABASE_URL, or CLAUDE_CONFIG_DIR can leak - spawns the shim detached in its own process group as the jail uid; a shim.pid file makes orphans findable across a middleware restart - terminate() SIGTERMs the process group, escalates to SIGKILL after the grace window, and removes the workspace; hostile runner_handle ids that escape the workspace root are refused (no arbitrary rm -rf from DB JSONB) - reap() SIGKILLs orphan pids from previous runs, removes leftover workspaces, and returns dead tracked handles so the worker can finalizeDevJob(..., 'stalled') Verified: middleware build green; devplatform suite 150 pass (15 new); dev-runner-shim suite 31 pass.
Review finding on w0-backend-local: the tracked-dead branch of reap() removed the workspace and returned the handle but never signalled the process group. If the shim dies abnormally (OOM/SIGKILL/segfault) its finally-cleanup never runs, and the CLI child it spawned — carrying ANTHROPIC_BASE_URL/AUTH_TOKEN via the gated passthrough — survived as an orphan making authenticated LLM calls after the job was finalized 'stalled'. Now the branch kills the whole group with SIGKILL before removing the workspace, exactly like the sibling untracked-orphan branch. Test: FakeProcs models process groups (kill(-pgid) hits every live member; the group outlives a dead leader), plus a regression test: dead shim leader + live CLI child in the group → reap kills the child and removes the workspace.
…t the shim leader terminate() gated its entire kill sequence on the LEADER pid being alive. When the shim died abnormally (OOM/SIGKILL) its finally-cleanup never ran and its process group could still hold a live claude CLI child carrying ANTHROPIC_* credentials — terminate() then sent no signal, removed the workspace + pid file, and dropped the handle, leaving the child permanently unreapable. Same class of hole the previous fixup closed in reap()'s tracked-dead branch, now mirrored into terminate(): - isGroupAlive(): probe kill(-pid, 0) with a leader-pid fallback - terminate() gates SIGTERM -> grace -> SIGKILL on GROUP liveness and waitForGroupExit() polls the group, so a dead leader with a live child still gets the full escalation before the workspace is removed - new test: dead leader + live group child => group is SIGTERM/SIGKILLed and the child is dead before the workspace disappears; the existing idempotency test still passes (a fully dead group draws zero signals)
The leader pid in a stale pid file may be long dead while its process group still holds a CLI child holding ANTHROPIC_* credentials. Probing the leader first reintroduced the blind spot terminate() and the tracked-dead branch had already closed.
…r workspace Signal delivery is not proof of exit. terminate() and both reap() branches SIGKILLed the runner group and then immediately rm-ed the workspace + pid file — the only handles a later reap() has. When the group survived (EPERM masked by killTree, or exit slower than the grace window) that stranded a live, credential-bearing CLI child as a permanent unreapable orphan. - killTree() now REPORTS its outcome instead of swallowing every error: 'signalled' | 'gone' | 'failed'. EPERM (group alive, unsignalable) is 'failed', not silently retried as a single-pid kill. The single-pid fallback fires only on ESRCH-on-group (the non-detached edge where the leader might still be individually signalable). - New killGroupAndConfirmExit() kills then confirms via waitForGroupExit; it returns true only when the group is provably gone. terminate() and both reap() branches now delete the workspace only on confirmed exit and otherwise LEAVE the workspace + pid file for the next reap sweep (terminate() throws local_terminate_incomplete, caught by finalizeDevJob's onError). - provision()'s post-spawn catch now tears down the already-live shim group before rm, and keeps the workspace + writes a pid file if it cannot confirm exit, instead of orphaning a running child. Tests: model EPERM (unkillable) and signal-surviving (immortal) group members in FakeProcs, plus a configurable post-spawn failure; add 6 cases covering the retain-on-unconfirmed-exit paths and the live-child spawn-failure cleanup.
…auth admission Add the in-process control loop for the epic #470 job spine: - claim loop bounded by DEV_PLATFORM_MAX_CONCURRENT_JOBS (active-slot aware) - stall -> stalled and wall-clock -> budget_exceeded, both via finalizeDevJob - host-side apply of the uploaded diff bundle: applying -> done (+ pr_url) once, failure -> failed with the diff artifact retained for the apply retry - auth-mode admission at the worker boundary (subscription refused for runs_tests repos, non-admin sources, and when the mode flag is unset) - every terminal transition routes through finalizeDevJob; the worker owns terminate dispatch + onError, so a terminate() throwing local_terminate_incomplete still finalizes and is logged - reaped runners finalized as stalled exactly once (finalize idempotency) All terminal transitions go through finalizeDevJob; the numstat split marker mirrors the shim's NUMSTAT_MARKER wire constant.
…lize the row
Fourth finding from the re-audit — pre-existing, not introduced by the last
commit, and worse than it first looks.
When a worker loses its lease between `provision()` and `setRunnerHandle()`, it
terminates the now-orphaned runner and deliberately does NOT finalize the job:
the row belongs to whoever holds the lease now. But `terminateHandle()` can
throw (`502 daemon.cleanup_failed` ⇒ `keepHandle`), and that throw escaped into
the outer catch, which finalized the row `failed` — a row we no longer owned,
with no runner handle recorded, while the container was still running.
The container then became immortal. `DockerBackend.terminate()` retains a handle
on `keepHandle` so a later attempt can retry, but nothing retried, and the renew
loop kept renewing its lease — so the daemon's lease reaper, the one backstop
designed for exactly this, never fired.
Two fixes, one idea: a retained handle means "we could not prove it is gone", not
"keep it alive".
- DockerBackend marks a job `terminating` the instant a teardown is requested.
Its lease is never renewed again, whatever the DELETE returns. The daemon's
reaper destroys the container within the TTL. The mark clears when the daemon
confirms the job is gone (via terminate success or reap).
- devJobWorker catches a failed orphan teardown, logs it loudly, and returns
without finalizing.
This is the epic's sixth recurring class, seen from the other side: a lease can
say "still working", and it must never be able to say "run forever".
Gate: 4338 middleware tests, 336 daemon tests, 0 cancelled. Both new tests
counter-proved. Two `profilesSnapshotsRouter` failures appeared in one full-suite
run and not in two subsequent ones; the file is green in isolation — the known,
pre-existing port-order pollution, unrelated to this change.
…formed
Fifth audit finding, and the sharpest: the lease-forfeit fix I had just landed
gave a corrupt handle a way to kill a healthy job.
`terminate()` derived the daemon id via `dockerJobId(handle)`, which returned
`handle.jobId ?? handle.id`. Pass a handle `{ id: 'A', jobId: 'B' }` — a corrupt
row, or one row's handle used against another — and the backend marks job B's
lease forfeit and DELETEs B, while the caller believes it named A. If the DELETE
then fails, B stays live but is never renewed again, so the daemon's reaper
destroys a healthy, mid-run container.
The type already documented the invariant (`id: /** = jobId */`), so nothing was
enforcing it. Now `asDockerHandle()` rejects a handle where the two disagree, and
`terminate()` narrows through it instead of guessing. `dockerJobId()` — the
function that made the guess possible — is deleted rather than fixed.
Both entry points are covered: terminate refuses (`devplatform.malformed_handle`,
nothing deleted, the healthy job keeps renewing) and rehydrate skips. Removing
the invariant fails the terminate test — the counter-proof.
Gate: 4340 middleware tests, 336 daemon tests, 0 fail, 0 cancelled, live pg.
…l the container
The egress proxy was built, hardened and unit-tested. It was never connected.
Nothing registered a job with it, and nothing gave a container a credential. The
proxy is default-deny and authenticates every request as
`Proxy-Authorization: Basic base64(jobId:proxyToken)`, so a correctly configured
deployment would have answered 407 to every request every runner ever made — a
fail-closed proxy failing closed on everything, presenting inside the container
as a total network outage. This is the fourth "a component that isn't mounted
isn't shipped" bug in this epic, and the most expensive: every one of the pieces
was individually correct.
- `proxyClient.mjs` — the daemon's control-plane client. Bearer-authed,
redirect:'error' (a redirect would carry the token AND the job's allowlist off
the operator's origin), deadline-bounded.
- `jobs.mjs` mints a per-job token, registers with the proxy BEFORE the container
starts (a runner that boots first races its own first fetch), and withdraws the
registration once the container is PROVEN gone. A failed registration aborts the
create — a job that boots without egress burns a whole run. A failed teardown
keeps the registration, because the container may still be running.
- The registration TTL is the job's daemon-owned HARD DEADLINE, not its lease.
A lease renews every ~TTL/3; hanging egress off that cadence would let one
missed refresh silently blackhole a running job. The reaper guarantees no
container outlives the hard deadline, so the registration can neither expire
under a live job nor meaningfully outlive a dead one.
- `policyClient.mjs` splices the credential into the injected HTTP(S)_PROXY value
as URL userinfo — where every proxy-aware client (curl, git, undici, requests)
looks for it. The operator-supplied DEV_RUNNER_EGRESS_PROXY_URL still refuses
userinfo: the credential is daemon-minted, never operator- or policy-supplied.
- `daemon.mjs` REFUSES TO BOOT when jobs are proxied but the daemon cannot
register them (or vice versa). The half-configuration is exactly the failure
above, and it must not be discovered in production.
Proven end to end over real sockets (`proxy.test.mjs`), driving the production
pieces: createProxyClient registers, createPolicyClient builds the container's
actual HTTP_PROXY value, and a CONNECT made from that value alone reaches an
allowlisted host — while an unregistered job gets 407 on everything, a job cannot
borrow another's token, and a withdrawal takes effect immediately.
Also: `withDeadline` had grown three independent copies, each having rediscovered
"never unref the timer" the hard way. One `deadline.mjs` now.
Gate: 356 daemon tests (was 336), 0 fail, 0 cancelled; middleware typecheck clean;
both images build. Every new guarantee counter-proved: registering after the
container starts, swallowing a registration failure, dropping the boot refusal, and
dropping the credential splice each fail their own test.
GOTCHA fixed in the same pass: the new chain tests leaked their echo server, so
`node --test` never exited and the WHOLE daemon suite hung — `# fail 0` with the
summary never printed. Same class as the earlier `# cancelled 9`.
…is caged The topology epic #470 assumes but nothing yet expressed: the middleware asks a daemon to create containers, and never touches a docker socket itself. dev-runner-daemon the ONLY holder of engine credentials. dev-control (called by the middleware, calls it back) + dev-engine (reaches dind). NOT on the `omadia` bridge, and bound to its pinned dev-control address — a wildcard bind would expose its control API to every container dind runs. dev-dind the nested engine, `privileged: true`. The only privileged service in the whole stack, so: internal-only networks, no host port, TLS-only (the daemon refuses tcp://…:2375), and even its registry pulls traverse the egress proxy. dev-egress-proxy the only path from a job container to the world. SAME image as the daemon, different command — so the process that terminates hostile job traffic holds none of the daemon's credentials. middleware joins dev-control. No socket, no DOCKER_HOST, no dev-engine. The proxy's data plane has a pinned IP (172.28.5.3): job containers are created by dind, not compose, so they cannot resolve `dev-egress-proxy` — a runner's HTTP_PROXY must be an address. Its control plane lives on dev-control, on a different pinned address, so the daemon never joins the network the jobs are on. `composeTopology.test.ts` PARSES the file and asserts all of it: no docker.sock on the middleware in either compose file, no DOCKER_HOST outside the daemon, exactly one `privileged` service, no `ports:` on any dev service, every dev network `internal: true`, the daemon off `omadia`, the bind address equal to the pinned address, both egress URLs present (a half-configuration is a boot refusal), and the proxy holding no engine certs. A comment can be wrong about each of these; a stray socket mount would leave the stack green and the isolation gone. Counter-proved by re-introducing each mistake: a docker.sock mount, a `ports:` on dind, the daemon on `omadia`, a non-internal dev-egress, a 0.0.0.0 bind, and a missing control URL each fail their own assertion. 18 assertions, `docker compose config` valid, middleware typecheck clean.
… unpinned base
The last W1 unit: one job driven end to end through the REAL pieces. A real git
repository (five files, one deliberately failing test) served over real HTTPS via
`git http-backend`, cloned by the REAL shim, which runs a scripted stand-in for
the `claude` binary, collects a real `git diff`, uploads it over the real
phone-home router to the real middleware, which applies it through a stub forge.
Only the container and the LLM are faked.
It found a real defect on its first green run, which is what an E2E is for.
`base_sha` was never written. `prepareProvision` accepts one and the wiring
passed nothing, so `dev_jobs.base_sha` stayed NULL and `applyDiff` read file
contents at the DEFAULT BRANCH'S CURRENT TIP — a different tree than the one
the agent cloned and reasoned about. `applyHunks` fails closed on the resulting
context mismatch, so the damage was a mysterious failure rather than a corrupt
commit; but "applied onto the pinned base tree" was aspiration, not fact.
Fixed by giving ForgeClient the `getRef` it was always missing (documented as an
open handoff since the W1 spec). The worker resolves the default branch to a sha
and pins it BEFORE the runner clones; `base_sha` is COALESCE'd, so a re-provision
keeps the tree the agent first saw. A forge that cannot answer fails the
provision rather than quietly producing an unpinned job.
Four properties no unit test can establish, because each is a claim about a SEAM:
1. The forge receives hunks that, evaluated against the base, reproduce the
agent's file BYTE FOR BYTE — run through the same `applyHunks` the real
GitHub client runs.
2. `git push` is never invoked. The shim gets a recording `git` wrapper; every
invocation is asserted push-free, and the log is asserted non-empty so it
cannot pass by never shelling out.
3. The spec carries no clone credential, and no credential store is left behind.
4. The job reaches `applying` → PR, pinned to the tree it cloned.
GOTCHA I nearly shipped: the byte-identical assertion compared the evaluated diff
against the same constant the fake agent was built from, so both sides moved
together — it passed a deliberately corrupted patch. It now compares against the
bytes on disk in the runner's working tree. Counter-proved: corrupting the diff
path fails it, and an agent that changes nothing fails it.
The unpinned-base counter-proof is the headline: reverting the `getRef` wiring
fails all three golden-fixture tests.
Gate: 4361 middleware tests (live pg), 0 fail, 0 cancelled. Requires `git` and
`openssl`; skips cleanly without them, like the rest of the pg-gated suite.
…it findings
A cross-family audit of the golden fixture and compose overlay found six ways a
test could pass while production was broken, plus one real behaviour bug. All
fixed; the two headline fixes are counter-proved.
1. (behaviour) A re-provision re-resolved the base sha. `base_sha` is COALESCE'd,
so a second getRef could only be thrown away — but a forge blip on a
re-provision (after an at-capacity requeue) would then fail a job whose tree
was already pinned. Now: an already-pinned job never asks the forge again.
2. The golden fixture's StubForge only RECORDED applyDiff. So a real
GithubForgeClient that ignored base_sha would not have failed the test. The
stub now RECONSTRUCTS each file at the pinned base_sha with the same
applyHunks the real client runs, and the assertion reads what it reconstructed.
Counter-proved twice: a stub that reads the wrong base, and wiring that drops
the pin, each fail.
3+5. The credential-leak assertions were shallow — they checked `spec.token` and
`spec.repo.token`, and only the top level of the workspace. Now a recursive
walk scans the entire spec for any credential-shaped key or the known token,
and the entire workspace tree (grepping file bytes, not just names) for a
leaked clone credential.
4. "No long-lived secret in the agent's environment" was claimed and never
asserted. The fake CLI now dumps its own env; the test fails on an inherited
ANTHROPIC key, the SCM token, the daemon token, or a non-job HOME.
6. `middleware/.env` (loaded via the base file's env_file) could inject
DOCKER_HOST into the middleware, and the overlay-only topology test could not
see it. The overlay now explicitly neutralises the docker env keys (empty
values win over env_file), and a new docker-gated test asserts the property
on the MERGED `docker compose config`, not just the overlay map. Counter-proved:
a real DOCKER_HOST in the overlay fails the merged-config assertion.
The audit confirmed base_sha pinning is otherwise end to end: getRef →
dev_jobs.base_sha → spec.repo.baseSha → the shim's detached checkout → the
collected diff → applyJob → DiffApplyService → applyDiff, one sha throughout.
Gate: 4364 middleware tests, 0 fail, 0 cancelled, live pg + docker.
… App tokens
First W2 units. GitHub App credential mode's security core, plus the pipeline
schema.
- migration 0023_dev_platform_pipeline.sql (renumbered from the spec's 0022,
which W0 took): pipeline/review columns on dev_jobs, approver/gate/bootstrap
columns on dev_repos, the dev_job_gates table (one open gate per job via a
partial unique index, modelled on conductor_awaits), and the dev_github_apps
+ installations registry (metadata only — every secret lives in Vault). No
CHECK on the growing enums, per the W0 convention. Applied + verified against pg.
- appJwt.ts: the App JWT minter, extracted from builder/githubAppAuth.ts so the
signing primitive is shared, not copy-pasted. githubAppAuth now calls it; its
tests still pass unchanged.
- installationTokens.ts: scoped, uncached, revocable installation tokens — the
opposite of the builder's unscoped cached provider, because the runner clones
hostile repos. Every mint carries a `{ repositories, permissions }` body and
REFUSES an empty scope (an empty body means ALL App permissions). Revoke treats
401 as success (already expired). JobTokenRegistry records every mint per job,
exposes the CredentialRevoker that finalizeDevJob (W0's pre-built hook) invokes,
revokes each unexpired token once, and records an honest skip for an
already-expired or un-revokable token — metadata-only events, never a value.
Counter-proved: an expired token sent to GitHub anyway fails; a revoke failure that
propagates instead of being recorded fails. The mint never echoes GitHub's error
body (it can reflect the JWT).
15 tests green, typecheck clean.
The one-click App creation. Mirrors the OAuth broker's pending-flow pattern:
an opaque `state`, a TTL'd (15 min) server-side store, a callback that consumes
the flow exactly once. The state is the CSRF gate — an unknown or expired state is
rejected WITHOUT consuming anything, so a replayed callback cannot complete another
operator's flow, and a bad-state probe cannot tell whether a real flow exists.
- buildManifest: the exact permission set (contents/pull_requests/issues write,
metadata read), `public: false` (owner-only install), and the INACTIVE webhook
that is the W4 placeholder (#437) — registered so W4 needs no App mutation,
firing nothing now. Truncates the org suffix to GitHub's 34-char name cap.
- manifestActionUrl: the org vs personal settings path the auto-submit form posts to.
- exchangeManifestCode: POST /app-manifests/{code}/conversions, parsed into
split-ready credentials (metadata → Postgres, secrets → Vault at the caller).
Never echoes the response body (it carries the PEM + client secret); rejects an
incomplete conversion rather than persisting a broken App.
- ManifestFlowStore: consume-once, self-reaping on expiry.
11 tests: permission set exact, webhook inactive, name truncation, state
round-trips once, unknown/expired state rejected without side effects, conversion
parsed, PEM never leaked on error, incomplete conversion refused.
…ts in Vault DevGithubAppStore: the App registry's data layer. The split is the invariant — dev_github_apps / dev_github_app_installations hold only non-secret metadata; the PEM, webhook secret, and client credentials live in Vault under core:dev-platform with github-app/<app_id>/ keys, the same namespacing DevRepoCredentialStore uses. saveApp writes the row and the secrets in ONE transaction: if the Vault write throws, the row is rolled back, so the operator is never left with a metadata row whose App they cannot authenticate. listApps returns installation counts and never any secret material. getSecrets reads the mint material back; a missing signing credential returns null (the App is unusable) rather than an empty object. 6 pg tests: metadata/secret split, no secret-shaped column, listApps secret-free with counts, getSecrets round-trip, null for unknown App, idempotent installation upsert, and the rollback. Counter-proved: committing before the Vault write leaves a half-registered App and fails the test. Completes W2's GitHub App credential-mode data + security core. HTTP routes next. Test fixtures use a neutral placeholder so the secret-egress guard does not trip.
The heart of the phased pipeline, extracted as a PURE function so the whole §4
table is one exhaustively-testable unit. The stateful engine (persistence,
artifacts, directives, token revocation) will wrap this; the decision "given the
current phase and this runner result, what happens next" lives here alone.
computeTransition covers every §4 row:
- gated: analyze→bootstrap→plan→clarify→PARK (gate opens, runner exits, even
with zero questions) →(resume)→ implement→review→pr
- review: approve→pr; request_changes not-exhausted→retry implement;
request_changes exhausted (fingerprint repeated / attempts spent)→pr annotated
- collapsed mode skips bootstrap/plan/clarify/await_human
- kind=analyze terminates after analyze in either mode
- any ok:false → fail (one code path for protocol violations)
isForward() guards against a replayed or stale phase result.
Also adds the W2 artifact kinds (bootstrap_report, questions, answers,
review_verdict) to the types enum — the DB is unconstrained, this is the gate.
17 tests, one per transition row plus the guards. Typecheck clean.
…deadline worker
DevJobGateStore (spec §5), modelled on conductor_awaits and copying the parts that
make that machinery safe:
- idempotent open via the partial unique index (WHERE status='waiting'): a
crash-and-retry of the clarify→await_human transition returns the EXISTING
gate, never a duplicate. The original gate's questions win over the retry's.
- resolve as a compare-and-swap (WHERE id=$ AND status='waiting'): two
concurrent resolvers race safely — one gets the gate, the other null → the
route maps that to 409 gate_not_pending. resolvedBy records the responder.
- a deadline worker: listDue scans overdue waiting gates, expire() atomically
claims one (CAS on waiting) so two workers never both expire the same gate.
- cancelForJob flips a waiting gate when the job is cancelled by another path.
Deadline from the repo's ISO-8601 gate_deadline_iso via the conductor's exported
parseIsoDurationMs. Holder authorization is deliberately NOT here — it belongs at
the route, resolved LIVE against roleStore so a moved baton re-targets.
6 pg tests. Counter-proved: dropping the status='waiting' guard from either
resolve or expire lets a resolved/expired gate be re-resolved and fails the test.
…nded retry
The bounded review→implement loop (spec §6), transplanted from
autoFixOrchestrator's pattern: a hard attempt cap AND a failure fingerprint,
because a counter alone lets a non-converging loop burn every attempt producing
the identical complaint each round.
- parseReviewVerdict: strict — a malformed verdict returns null (the caller
re-prompts once, then fails) so a reviewer emitting garbage is never silently
treated as an approval.
- normalizeVerdict: a request_changes whose findings are ALL minor is coerced to
approve (minors annotate the PR, they do not block); a real request_changes
needs >=1 blocker|major.
- fingerprintFindings: sha256 of the sorted blocking findings
(severity:file:issue), order-independent, ignoring line/suggestion (they wobble
without the underlying problem changing).
- decideReview: approve → PR; identical fingerprint → give up (not converging);
attempts exhausted → give up; else retry implement with attempt+1. Give-up
still opens the PR with findings annotated — never silently dropped.
11 tests. All 63 W2 tests green together.
The integration heart of capability 2. Ties the pure transition table, the review
loop, and the gate store to persistence, and answers the runner's phase-result
with a directive (next / park / done / failed).
- DevJob gains the pipeline columns (pipelineMode, reviewAttempt,
reviewFingerprint, retryOf) — read from the 0023 columns via JOB_COLS + toJob.
- DevJobStore gains the phase transitions: advancePhase (fenced on the phase
being LEFT, so a stale runner matches 0 rows → the caller 409s), requeueAtPhase
(await_human→implement, re-queues at the pinned base for the next provision),
parkForGate (status→waiting, lease cleared, fenced on await_human),
setReviewState.
- PhaseEngine.handlePhaseResult: rejects a stale phase result BEFORE persisting
anything; stores the artifact; runs the review-loop decision (retry / give-up /
approve, updating review_attempt + fingerprint); computes the transition;
applies it — advance, park (open the gate in the same flow, revoke the exiting
runner's token WITHOUT finalizing the still-waiting job), done/fail via the
finalize choke point.
9 tests drive a full gated job analyze→bootstrap→plan→clarify→PARK→implement→
review→pr→done, plus the review loop (retry / non-converging give-up / minor-only
approve / malformed→fail), collapsed mode, kind=analyze termination, stale-phase
409, and ok:false. Counter-proved: dropping the stale check (a stale result then
persists its artifact), skipping park-time token revocation, and letting a
malformed verdict through each fail their test.
Comment-back (spec §7), GitHub Issues only in W2. Three kinds, posted only when the
job is bound to a ticket: clarify (plan summary + numbered questions + gate link),
pr (PR link + summary + review status), failed (a NEUTRAL notice — issue threads
may be public, so no error text, no repo internals).
Two idempotency layers because either crash ordering must not double-post:
1. a comment_posted event guard keyed (job_id, comment_kind), recorded BEFORE the
POST — a retry after a recorded post is a no-op, and a lost record race means
the winner posts.
2. a `<!-- omadia-dev job=.. kind=.. -->` marker on every body; for the inverted
window (posted, then crashed before recording) the recent comments are scanned
and a marker match short-circuits, backfilling the guard.
Both layers + the forge are injected, so the module is pure and testable. 8 tests:
body composition (marker/questions/link/neutral-failure), post-once, event-guard
no-op, marker-scan fallback, unbound-job no-op, per-(job,kind) dedupe. Counter-proved:
dropping either idempotency layer double-posts and fails the test.
Wires the phase engine to the runner. The runner reports the phase it just ran; the middleware decides the transition and returns a directive (next / park / done / failed). Job-token authed like every runner route (no token → 401, ahead of the body parser). A phase that does not parse to a known phase, or a non-boolean ok, is 400 before the engine is touched. A StalePhaseError from the engine — a runner reporting a phase the job already left — maps to 409, its result discarded. The handler is injected (handlePhaseResult), so the router stays decoupled from PhaseEngine construction and the endpoint is simply NOT mounted in the W0/collapsed shape (a request then 404s). makeJob in the runner-api harness gains the pipeline fields. 6 route tests (forward+directive, stale→409, bad-phase 400, bad-ok 400, no-token 401, not-mounted 404); the 27 existing runner-api tests stay green.
The operator inbox + resolve endpoints (spec §5): GET /gates?status=waiting — waiting gates enriched with the LIVE holder set. POST /gates/:gateId/resolve — approve/reject. Authorization mirrors conductor's resolveAwait holder gate exactly, because the failure it prevents is identical: a client-controlled payload carrying only a gate id must NOT let any recipient resolve someone else's gate. The holder set is resolved LIVE at resolve time (a moved role baton re-targets), canonicalized, and a non-holder is 403 gate_not_holder — never a silent success. Resolution is a compare-and-swap in the store: a second concurrent resolver gets 409 gate_not_pending, and the approve/reject side effects (requeue + append answers / cancel + record note) run for the CAS WINNER only. Injected deps (gateStore, resolveRoleHolders, onApproved, onRejected) keep it testable. 8 tests: list with resolved holders, 401 no-session, user-holder approve, non-holder 403 (gate untouched), live role-baton resolve, concurrent 409, rejection records note, unknown gate 404. Counter-proved: dropping the holder check lets a non-holder resolve, and running side effects on a null CAS double-fires — each fails its test.
Per-phase system prompts for the pipeline agent sessions (spec §4/§6). Built by an
isolated Engineer subagent, integrated + re-verified here.
- phaseSystemPrompt(phase) is total over the 8 phases. Each prompt states the
phase's job and the exact artifact it must emit. The reviewer prompt embeds the
spec §6 adversarial sentence verbatim (ADVERSARIAL_REVIEW_DIRECTIVE), naming CI/
workflow files, dependency manifests, and credentials handling as the things to
scrutinize.
- Every brief-bearing prompt (analyze/plan/clarify/implement) inlines
UNTRUSTED_BRIEF_NOTE — the framing travels with the payload since each phase is
a fresh session with no context bleed. Review carries no brief (spec §6),
asserted by a test.
- bootstrap/await_human/pr are not agent sessions (dep install / parked /
host-only); their prompt is a documented marker never handed to a model, and
buildPhaseSpec is scoped to the 5 agent-session phases (AGENT_SESSION_PHASES).
- buildPhaseSpec is a pure, JSON-serializable discriminated union: implement
carries plan+answers+attempt+priorFindings, review carries plan+diff (no brief),
analyze carries the brief. Same inputs → deep-equal output.
16 tests, tsc clean.
The HTTP surface driving manifestFlow.ts + appStore.ts (spec §2). Built by an
isolated Engineer subagent, integrated + re-verified here.
createDevPlatformGithubAppRouter(deps) returns TWO routers — the two groups mount
behind fundamentally different middleware and a single Router cannot express the
split honestly:
- admin (behind requireAuth, re-reads the session): POST /github-app/manifest/start
({action, manifest}), GET /github-apps (no secret material), POST
/repos/:repoId/credential (verifies installation coverage via App JWT →
GET /repos/{owner}/{repo}/installation before binding credential_ref
'github_app:<appRowId>:<installationId>').
- public /bot-api (no session; the state token is the only CSRF gate): GET
/github-app/callback (consume-once state, exchange, saveApp, 302 to install),
GET /github-app/setup (find the owning App by trying each App's JWT against
GET /app/installations/{id}, upsert, 302 to admin UI).
Secret hygiene: no handler logs or echoes a PEM/client secret/webhook secret — a
conversion failure throws a generic error before the body is read; a save failure
surfaces "delete the orphan at <html_url>" guidance. All external effects injected
(flowStore, appStore, bindRepoCredential, getRepo, fetchImpl, recheckBranchProtection).
14 route tests (manifest permission set + action URL + pending flow; callback
consume+exchange+saveApp+302; unknown-state 400 consuming nothing; conversion error
never leaks the pem; setup verify+upsert+302; coverage-miss 400, cover-hit binds;
admin 401 without session; /github-apps secret-free), tsc clean.
….4) W2 audit W2 was built solo, skipping the cross-family review gate that found a real defect in EVERY W0/W1 unit. Running it now on the 13 W2 units surfaced five real defects the 60 green unit tests did not assert — including one HIGH both model families confirmed with a runnable repro. All fixed, all counter-proved. 1. HIGH — the plan gate never pinned the approved plan. The gate opens on the `clarify` phase-result, but planArtifactId/planSha256 were read from the *current* (clarify) input — the plan artifact is delivered a phase EARLIER, so the gate stored null. A resume could implement against a plan the human never approved. Fix: the engine now pins the PERSISTED plan via a new getLatestArtifact(jobId,'plan'). Test asserts the gate carries the plan hash + artifact id; reverting the lookup fails it. 2. HIGH — gate resolution was non-atomic with the job transition. gates.resolve() CAS-flips the gate, then onApproved/onRejected runs separately; a crash between left the gate resolved while the job sat at await_human forever (the holder's retry hit 409). Fix: the route now drives the side effect from the RESOLVED gate's durable state, and a CAS-loss re-loads the gate — if it is resolved but the job is still parked (new isJobStuckAtGate dep), it re-drives idempotently (200, recovered:true); a normal concurrent resolve still 409s. 3. MEDIUM — JobTokenRegistry.revoker aborted on the first audit-write failure, leaving LATER tokens live until GitHub's 1-h self-expiry. The expired-skip and catch-block appendEvent calls were unguarded. Fix: safeAppend swallows a sink failure so a lost audit line never leaks a live credential. Counter-proved via the expired-token skip path (the one outside any try/catch). 4. MEDIUM — requeueAtPhase was unfenced (guarded only NOT-terminal), so it would re-queue ANY non-terminal job. Fenced on phase='await_human', matching parkForGate — which also makes the #2 self-heal re-drive idempotent (the second call matches 0 rows). 5. MEDIUM — saveApp could leave Vault secrets without a Postgres row: a COMMIT failure after setMany rolls back the row but not the secrets. Fix: best-effort deleteKey the four secret keys on a post-Vault-write failure. Gate: 163 W2 tests green (incl. e2e + golden fixture), tsc clean, live pg. Every fix has a test that fails when the fix is reverted. Forge's outstanding item — the orchestration is not yet wired into wireDevPlatform (scm-token still returns the raw repo token; finalizeDevJob has no revokers) — is the next task, tracked.
…outstanding #A)
Closes the first half of Forge's "the credential hardening is inert until wired"
gap. Before this, a runner on a github_app repo still received the raw stored token
(possibly contents:write) and finalizeDevJob ran with no revokers.
- wireDevPlatform builds ONE JobTokenRegistry (audit events → the job's event
log via appendHostEvent, metadata only) and registers its revoker on a
CredentialRevokerRegistry passed to finalizeDevJob — so EVERY terminal path
(stall, wall-clock, cancel, W1 reaper, phase-engine fail/done) revokes the
job's scoped tokens.
- A scopedScmTokens adapter replaces the raw credential source on the runner
router: for a github_app repo it parses credential_ref
'github_app:<appRowId>:<installationId>', loads the App + its secrets, mints a
contents:read, single-repo, revocable installation token, and records it in the
registry. device-flow/PAT repos keep the W0 stored-token fallback (documented).
- DevRunnerScmTokens.resolve now takes the job (id + repoId) — the token is
per-job, and the registry is what makes it revocable.
- A githubAppFetch seam makes the mint/revoke path fully testable.
New pg wiring test drives it end to end: provision a github_app job → GET /scm-token
returns the SCOPED token (not the raw credential), the mint body is exactly
{contents:read} for one repo, finalize revokes it (DELETE /installation/token), and
the audit log records mint+revoke with no token value. Counter-proved: reverting to
the raw scmTokens, or dropping the revoker registration, each fails the test. 45
runner/e2e regression tests stay green.
Remaining (Forge #B): mount the phase engine + gates router, wire revokeTokensForPark,
start the gate deadline worker that cancels an expired-gate job. Tracked next.
…g #B, subagent-built)
Closes the second half of Forge's "inert until wired" gap. The phase engine, gate
store, gates router, and gate-deadline worker are now constructed and connected in
assembleDevPlatform. Built by an isolated Engineer subagent, integrated + re-verified.
- PhaseEngine constructed with the store (=jobStore), gateStore, boundFinalize,
and revokeTokensForPark = the SAME tokenRegistry.revoker used on finalize — one
registry, both paths (Forge's requirement). gatePrincipal/gateDeadlineIso read
the repo (approver_role_key / gate_deadline_iso, now on DevRepo + the row
mapper); the callbacks were widened to awaitable so the engine — which calls
them only on the rare clarify→park — can read the repo in the wiring.
- handlePhaseResult is now passed into the runner router, so POST
/jobs/:id/phase-result actually drives the engine.
- The gates router is mounted behind requireAuth. onApproved appends the answers
to the brief (new marker-idempotent DevJobStore.appendToBrief — safe under the
gate's self-heal re-drive) and requeues at implement; onRejected cancels;
isJobStuckAtGate distinguishes a crash-stranded job from a normal concurrent
resolve.
- A gate-deadline worker (unref'd interval, started by start()/stopped by the new
stop()) expires overdue gates via CAS and finalizes the job cancelled
(reason gate_expired).
- index.ts threads the conductor ConductorRoleStore so role-principal gates
resolve their live holder set, and calls wired.stop() on SIGTERM/SIGINT.
New pg wire test drives a real gated job end to end: analyze→bootstrap→plan(pinned
planSha256)→clarify→PARK (gate opened, job waiting) → holder approve via the gates
router → requeue at implement with answers in the brief → implement→review(approve)→
pr; plus stale-phase 409 and gate expiry (clock-advanced tick → gate expired, job
cancelled, token revoked). Counter-proved: removing the deadline-tick finalize fails
the expiry test.
Gate: 86 W2 tests green (incl. e2e + golden fixture + scoped-token), 0 cancelled,
tsc clean. Remaining W2: the shim phase loop (subagent in flight) + web-ui; then the
whole wiring + shim goes back through the Forge cross-family gate.
The runner side of the gated pipeline. Built by an isolated Engineer subagent,
integrated + re-verified. The W0 collapsed path (runShim) is untouched; the entry
point dispatches on OMADIA_PIPELINE_MODE (gated → runPhasedShim, else runShim).
- phaseLoop.ts (runPhasedShim): the provision lifecycle — protocol gate, event
stream, heartbeat+cancel, wall-clock, a SINGLE clone at the pinned base_sha,
then the directive-driven loop: run a fresh session per phase → POST
/phase-result → act on the directive (next / park→exit0 / done→exit0 /
failed→exit1). NOTHING is pushed (a recording git wrapper proves it in tests).
- phaseRunner.ts: one phase, never throws (failures become {ok:false,error}).
Three shapes — agent-session artifact phases (analyze/plan/clarify/review),
implement (session + git diff), bootstrap (a shell command, zero sessions).
Each artifact phase writes its JSON to $OMADIA_PHASE_ARTIFACT (a deterministic
file channel, robust vs parsing model stdout).
- phasePrompts.ts: a local mirror of the middleware phase prompts (the shim is a
separate package and cannot import middleware/src); the directive's `phase`
string drives which session runs. Documented: if the middleware later carries
the spec on the directive, this copy is deleted and read from the wire.
- HomeApi gains postPhaseResult → POST /jobs/:id/phase-result (matches the
existing route verbatim; no middleware route change). protocol.ts adds the
phase/verdict/context types, all optional so W0 stays valid.
35 shim tests green (4 new phaseLoop suites + all pre-existing), tsc clean, build
clean. Each phase runs a fresh HOME dir (no context bleed); park exits 0; failed
exits 1 after posting.
SEAM for the middleware /spec route (next): for a gated job, GET /spec must populate
phaseContext (current phase + plan/answers/priorFindings/attempt for provision B) +
bootstrap.command, and the backend must launch with OMADIA_PIPELINE_MODE=gated.
…orge W2 audit)
The 2nd cross-family (GPT-5.4) audit of the wiring found 4 blockers + 1 minor that
the 86 green tests missed because they drove the shim's gated path directly and
never asserted the MIDDLEWARE emits the gated wire. This closes the two middleware-
side findings; the two shim blockers follow in the shim package.
1. HIGH/BLOCKER — gated jobs silently ran the W0 collapsed path in production. The
`/spec` route built a W0-shaped spec (no pipelineMode/phaseContext/bootstrap),
DevJobSpec didn't even declare them, and OMADIA_PIPELINE_MODE appeared in ZERO
src files — so the shim dispatched to the single-shot path and the entire
gate/plan/clarify/review flow never executed (both model families confirmed;
the concrete manifestation was a gated job dying at bootstrap). Fix:
- DevJobSpec gains pipelineMode + phaseContext + bootstrap.
- GET /spec sets pipelineMode=job.pipelineMode; for a gated job it builds
phaseContext (phase, and for provision B the approved plan + gate answers +
prior review findings + attempt, read from the stored artifacts) and the
bootstrap command.
- deriveJobPolicy injects OMADIA_PIPELINE_MODE (gated|collapsed) into the
runner env — the shim's dispatch flag.
- the daemon's ALLOWED_ENV_KEYS admits OMADIA_PIPELINE_MODE, or a gated DOCKER
job's flag would be clamped away and it would silently collapse anyway.
New tests assert the middleware EMITS the gated wire (the missing assertion):
/spec carries pipelineMode + phaseContext (+plan/answers/attempt/findings for
provision B), collapsed carries no context, and deriveJobPolicy sets the env.
Counter-proved: omitting the /spec flag or the env each fails the test.
2. MINOR — the gate-deadline worker was not quiescent after stop(): an in-flight
tick kept expiring gates + cancelling jobs after shutdown. Fix: a gateStopped
flag checked before each side effect, and stop() (now async) awaits the
in-flight tick. Test: after stop(), a tick expires nothing; counter-proved.
Gate: 50 middleware W2 tests + 88 daemon tests green, tsc clean, live pg.
The cross-family audit confirmed two shim blockers that operate over HOSTILE repo content and the green tests missed. 1. BLOCKER — the phase artifact read trusted its path blindly. Each artifact phase writes JSON to $OMADIA_PHASE_ARTIFACT, which the shim reads back. A hostile phase session could replace that path with a SYMLINK to an out-of-workspace file (e.g. the runner's credentials) → the read follows it and exfiltrates the secret into artifact.content, posted to the middleware; or write a huge blob → memory exhaustion before the HTTP cap. readArtifact now lstat's the path (never follows a symlink), rejects anything that is not a regular file, rejects a realpath outside the workspace, and caps the size at 2 MiB before reading a byte. 6 new artifactSafety tests (normal file ok; symlink/oversized/directory/missing/empty all → null); counter-proved (the size cap is load-bearing; the symlink is caught by the is-regular-file guard). 2. BLOCKER — cancel sent one SIGTERM and never escalated. Unlike the wall-clock path, the cancel branch (both the phase loop and the W0 collapsed path) fired a bare SIGTERM, so a child that traps/ignores SIGTERM hung the provision until wall-clock expiry. Both paths now route through one terminate helper that arms SIGKILL after the grace window. New test: a SIGTERM-trapping child is still killed (only possible via SIGKILL); counter-proved (reverting to bare SIGTERM hangs the child and drops the test). Gate: 42 shim tests green (36 + 6 artifact-safety), tsc + build clean. This closes all 4 blockers + 1 minor from the W2 wiring/shim audit.
The env-shape assertions were deepStrictEqual and did not expect the new W2 dispatch flag. Updated to include OMADIA_PIPELINE_MODE (collapsed for the pure api_key/subscription cases, gated for the route test whose makeJob defaults to a gated job). Full devplatform suite: 473 pass, 0 fail, 0 cancelled.
…gent-built)
The last W2 unit. Built by an isolated Engineer subagent, integrated + re-verified.
Three surfaces on /admin/dev-platform (Lume design system, en+de i18n):
- GithubAppsPanel: "Create GitHub App" starts the manifest flow and submits a
real top-level browser FORM POST to GitHub (its create-from-manifest endpoint
renders its own approval screen and 302s back — a fetch cannot create the App),
plus the registered-App list (owner, slug, installation count, manage link).
Never displays a secret.
- GateInbox: the operator approval inbox — per gate: job, plan (linked via the
artifact route + truncated planSha256), questions, deadline, live holders, and
an approve/reject action with an answers form + note. Leads with a warning-edged
banner stating plan approval is ADVISORY and the W3 diff gate is authoritative
("omadia never merges"). Branches on HTTP status: 403 → "not a holder" in place
(no mutation), 409 → "already resolved" + auto-refresh.
- BindGithubAppPanel: bind a repo to a github_app credential + installation on the
repo detail page (binding is a post-install action — the installation must exist
first for coverage verification; the add-repo wizard's github_app card stays W0).
Lume-only tokens, state text/edge-only, 4 Button variants, no spinners (Button busy).
66 new i18n keys in BOTH locales. Gate: typecheck clean, i18n:check OK (2982 keys,
en/de identical), eslint clean on all three new components.
W2 IS COMPLETE (GitHub App credential mode + phased pipeline + shim + web-ui), across
two Forge cross-family audits (10 real defects found + fixed).
…ate (subagent-built)
The security core of W3: the middleware-side engine whose verdict gates the
server-side diff apply (the W2 UI calls plan-approval "advisory" and defers to
THIS). Built by an isolated Engineer subagent, integrated + re-verified.
- migration 0024_dev_platform_w3.sql (renumbered from the spec's 0023 which W2
took): dev_repos.policy_overrides jsonb, dev_jobs.conductor_await_id,
dev_repo_plugin_grants (the ctx.devJobs grant). Idempotent, applied+verified.
- scanForSecrets: credential prefixes (ghp_/github_pat_/sk-ant-/djr_/PEM
headers), Shannon-entropy strings, and the job's own token values — samples
REDACTED. PEM detection assembles its regex from fragments so no source or
fixture line contains a contiguous PEM banner (the security hook would block it).
- diffPolicyEngine.evaluateDiffPolicy over the existing parseUnifiedDiff:
git-internals (.git/ or `..` path) → DENY, non-overridable
credential-content (scanForSecrets over the diff's ADDED content OR the PR
body OR any tracker comment) → DENY, non-overridable
protected-ci / dep-manifest-lockfile / max-files (50) / max-added-lines /
diff-integrity (parser totals ≠ numstat) → gate
Precedence: any deny → deny; else any gate → gate; else allow. Deterministic.
- Override merge {maxFiles,maxAddedLines,extraProtectedGlobs,unprotectedGlobs}:
subtract-then-add over defaults; the two deny rules are separate code paths
that never read the override, so NO override can disable them.
- Correctness catch: `git diff --binary` carries 40-char index blob hashes;
scanning the raw diff would false-positive them → deny. The scan runs only over
ADDED content (lines starting with a single +), never git metadata.
29 tests (each rule; override merge incl. "cannot disable deny"; scanForSecrets
prefixes/entropy/token-value/redaction; determinism). Counter-proved: neutering
git-internals fails 4 tests, neutering credential-content fails 2 — both
load-bearing. 502 devplatform regression tests green, tsc clean.
- phaseEngine: drop dead initial reviewVerdict assignment (every decision branch reassigns it before use) — no-useless-assignment - wireDevPlatform: replace inline import() type annotation with a proper type import of TokenFetch — consistent-type-imports - devPlatformGithubApp: remove unused sendError import; drop the dead initializer on 'body' (both try/catch branches assign it) Behavior-neutral; lint + typecheck + module tests green.
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.
Prepares the subagent-driven implementation of epic #470 (integrated dev platform). No product code — three Workflow scripts and one manifest.
What this adds
scripts/wave-decompose.workflow.mjsscripts/wave-implement.workflow.mjsscripts/wave-verify.workflow.mjsdocs/dev-platform/w0-manifest.jsonDesign notes
Cross-family review is the gate, not a formality. The implementer (Engineer, Claude-family) and a same-family reviewer share a training corpus and therefore share blind spots. Forge (GPT-5.4) refutes every diff; Cato audits the security-sensitive units against the epic's structural invariants. A branch is
prReadyonly when the implementer verified, the refuter could not overturn it, and — where it matters — the auditor found no invariant violation. This mirrors what #472 demonstrated: the Claude reviewer saidprReady: truein four consecutive rounds and the cross-vendor reviewer rejected all four.Hub files are subtracted from the collision signal.
index.ts,config.ts, andmessages/*.jsonare append-only registration points touched by nearly every unit. Left in the signal, every unit reads as dependent and the fan-out dies (the lessonissue-clusterlearned the hard way). Instead the manifest assigns every hub edit to onew0-wireunit thatdependsOnall others — a merge conflict becomes a dependency edge.parallel()vspipeline()is deliberate. Decomposition uses a barrier because thedependsOnreduce needs every unit at once. Implementation uses a pipeline because an independent unit can be under review while another is still being written; a barrier there would waste the fast units' wall clock.Spec drift is reported, never silent. An agent that must deviate does the code-correct thing and records a
specDeltasentry with the spec line and the code evidence. Reviewers are told declared deltas are expected and that undeclared divergence is a review failure. That is how the spec and the code converge instead of quietly forking. (The specs already produced one live example: the W0 migration number was wrong because0020shipped the same morning.)Nothing writes to GitHub. No workflow pushes or opens a PR. The caller owns that choke point, downstream of the human approval. The three-script split exists because a Workflow script cannot call
AskUserQuestion, so each human gate forces a boundary.Validation
node --checkafter the standard async wrapper — top-levelreturnis Workflow idiom, as in the existingissue-*.workflow.mjs).w0-schema → w0-store → … → w0-wire.Not in scope
No product code for the dev platform itself. W0 implementation starts from this manifest once a human approves it.
Refs #470