Skip to content

feat: scale-out phase A — instance ids, the schema, leases, the run protocol, a multi-server e2e harness - #666

Merged
jonwiggins merged 15 commits into
mainfrom
feat/scale-out-foundation
Oct 10, 2026
Merged

jonwiggins merged 15 commits into
mainfrom
feat/scale-out-foundation

Conversation

@jonwiggins

@jonwiggins jonwiggins commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

Phase A of docs/plans/scale-out.md (in this PR): the foundation for API pods that share one database and for runs that outlive the pod that started them. Nothing user-facing changes yet; the workers still run agents through the classic exec path. B (re-attachable runs), C1 (coordination) and C2 (sessions / glance / skills) build on this and touch no schema.

What landed

  • Instance identity. services/instance.ts: INSTANCE_ID = OPTIO_INSTANCE_ID (the chart sets it to the pod's name) + a suffix new at every boot, so a restarted pod is a new instance. Logged at boot; GET /api/health returns instanceId.
  • One migration, every schema change of the plan (1792010000_scale_out): the attachment columns exec_state, exec_pid, consumed_bytes, attached_by, attach_lease_until on tasks (through runColumns(), so repo_tasks and workflow_runs carry them), pr_review_runs and persistent_agent_turns; tables leases, ws_upgrade_tokens, inbound_webhook_deliveries, glance_state, installed_skill_files, ticket_sync_claims; a unique partial index pr_reviews_active_pr_url_key (older active duplicates cancelled first). migrate-safe.ts now leaves out any later-added run column when it remakes the views between historical migrations (one list instead of the recovery_required special case). Integration test proves columns, views, indexes and constraint behavior.
  • services/lease-service.ts: acquireLease (insert-or-update-where-expired, returning a row iff acquired), renewLease, releaseLease, leaseHolder, seizeLease (newest-wins), sweepLeases, and withLease(key, fn, opts) that renews on an interval, aborts fn's signal and rejects with LeaseLostError when the lease is taken away. Integration-tested with two contending instances.
  • The run protocol on ContainerRuntime (packages/container-runtime/src/run-protocol.ts, re-exported by utils/pod-env.ts): startRun, attachRun, deliverStdin, killRun. A start script ends with superviseAgent(runDir, agentLines), which writes agent.sh and supervisor.sh into the run home, primes stdin.ndjson from $OPTIO_RUN_STDIN, launches the supervisor with setsid … &, and prints __OPTIO_RUN_STARTED__:<pid>. The supervisor runs the agent with stdin fed from the file through a FIFO until __OPTIO_STDIN_EOF__, appends stdout to output.ndjson, and writes exit (143 on TERM). Attach is tail -c +N --pid=<pid> -f, with the exit marker on stderr so stdout is pure file bytes. Kubernetes and Docker share one exec-based implementation; Docker's non-TTY exec now demuxes stderr. Verified for real on ubuntu:24.04 (start → attach → deliver → EOF → exit 7; attach from an offset; kill TERM → 143 with no leftover processes; SIGKILLed supervisor → lost; zero exit marks the env warm); run-protocol.linux.test.ts does the same under vitest on Linux (skips on macOS).
  • Start-script builders: buildTaskStartScript (repo-pool-service.ts; the task setup is now one taskSetupLines shared with the unchanged buildTaskExecScript, which execTaskInRepoPod still uses) and buildPooledStartScript (pod-env.ts). No trap, no EPIPE watchdog, no run-home removal; the env-ready marker moves into the supervisor.
  • Multi-process fake runtime: FakeContainerRuntime({ dir }) / OPTIO_FAKE_RUNTIME_DIR keeps containers on disk; startRun spawns fake-agent.mjs detached, which plays every existing [[mock:*]] directive (plus [[mock:echo-stdin]]) into output.ndjson with a seq on every event, reads stdin.ndjson, waits for the EOF sentinel after its result like claude (exit 97 if none comes within OPTIO_FAKE_EOF_TIMEOUT_MS), and writes exit. Kill-style utility execs and destroy reach protocol runs. In-memory mode is unchanged for the existing tests.
  • test-utils/e2e/api-cluster.ts: startApiCluster({ size, fakeRuntimeDir?, env? }) → { servers, fakeRuntimeDir, live(), client(i), any(), kill(i, signal?), add(), stop() }. api-cluster.e2e.test.ts: two servers boot together on one DB, a Job made through A is read through B and its run completes, A is SIGKILLed and B keeps serving and running new work, a third server joins.

What B / C1 / C2 build on

  • Runtime: startRun(handle, { runId, runDir, script, initialStdin }) → { pid, output }; attachRun(handle, { runId, runDir, fromByte }) → { output: Readable, exit: Promise<RunExit>, close() } with RunExit = { kind: "exited", code } | { kind: "lost" } | { kind: "detached", reason }; deliverStdin(handle, { runId, runDir, line }); killRun(handle, { runId, runDir, signal: "TERM" | "KILL" | "INT" | "HUP" }) → boolean. runDir is runHome(runId).podDir.
  • Schema (Drizzle): execState, execPid, consumedBytes, attachedBy, attachLeaseUntil on tasks / workRuns / workflowRuns / prReviewRuns / persistentAgentTurns; leases, wsUpgradeTokens, inboundWebhookDeliveries, glanceState, installedSkillFiles, ticketSyncClaims.
  • STDIN_EOF_SENTINEL is what a worker appends where it called stdin.end().

Corrections to the plan, found while building

  • ticket_external_id is written by five paths besides the sync, so a unique index on tasks would reject work a person starts on purpose: the sync gets ticket_sync_claims instead.
  • webhook_deliveries already exists (the outbound webhooks' log): the inbound dedupe table is inbound_webhook_deliveries.
  • The protocol scripts live with the runtimes (the API imports the runtime package, not the other way round); pod-env.ts re-exports them.
  • RunExit has a third kind, detached, for an attach whose exec dropped without a marker.

Tiers

  • pnpm turbo test: shared 748, container-runtime 162 (+3 Linux-only skipped here), api 3021 — green.
  • pnpm --filter @optio/api test:integration: 50 files / 405 tests — green.
  • pnpm --filter @optio/api test:e2e: 23 files / 102 tests — green (incl. the cluster smoke).
  • pnpm format:check, pnpm turbo typecheck (13/13), helm lint — green.
  • No shared type changed; no regen needed.

Review fixes (15 items)

  1. TERM before the trap. The supervisor writes its pid file itself, after trap on_term … and after launching the agent and the feeder; the start script waits for that file (≤10 s, 50 ms polls) before __OPTIO_RUN_STARTED__. fake-agent.mjs writes its pid/identity file only after process.on("SIGTERM"), and the fake's startRun waits for it. CI's red test passes (3× locally).
  2. Pid reuse. The pid file is <pid> <starttime> (field 22 of /proc/$$/stat, read past the (comm) with sed 's/.*) //' then field 20). One shell helper, optio_run_pid (RUN_ALIVE_HELPER), shared by attach, kill and the pooled start guard, trusts a pid only when the live process's start time matches; a mismatch is "not running" (attach → LOST after what was written, kill → none, start guard → proceed). Unit-tested as text, exercised in the Linux vitest and the Docker check with a stale pid file.
  3. Zombies. tini added to images/base.Dockerfile; scripts/repo-init.sh and both pooled init scripts (workflow-pool-service.ts, persistent-agent-pool-service.ts, via KEEP_POD_ALIVE in pod-env.ts) end with exec tini -- sleep infinity, falling back to exec sleep infinity. The Docker check runs under tini as pid 1 and asserts no <defunct> after three runs, a TERM kill and a SIGKILL. The plan notes the images must be rebuilt (the fake is unaffected).
  4. FIFO. tail … | ( while …; done > "$d/stdin.pipe" ): only the loop holds the FIFO, so break closes the agent's stdin at once. Docker check: the run ends 999 ms after the sentinel (bounded by tail --pid's 1 s poll).
  5. Migration vs launchPrReview. The migration's CTE also cancels the queued/provisioning/running pr_review_runs of the reviews it cancels. findReviewByUrl (new, used by launchPrReview) orders active-state rows first, then newest updated_at; integration-tested with a cancelled duplicate both newer and older than the active one.
  6. exit after stdout. RunAttachment.exit settles only after the marker and the stdout end (writer's finish, reader's end, or close() which abandons the output); documented on the type; unit test with stdout still flowing after the marker.
  7. No stdin in argv. startRunPrelude(bytes) (exec 3<&0 0</dev/null; export OPTIO_RUN_STDIN_BYTES=<n>) + head -c "$OPTIO_RUN_STDIN_BYTES" <&3 > stdin.ndjson in the start script; the runtime writes exactly Buffer.byteLength bytes to the exec's stdin and closes it. deliverStdin is head -c <n> >> stdin.ndjson the same way. OPTIO_RUN_STDIN and OPTIO_STDIN_LINE are gone. Docker check and the Linux vitest: a 300 KB first message and a 200 KB delivered line (multi-byte UTF-8 in the vitest) arrive byte-exact, stdin.ndjson compared with cmp.
  8. Deliver judged by stdout. __OPTIO_STDIN_DELIVERED__ after the append; stderr only goes into the error message on failure (the runtime package has no logger; the API's debug log is phase B's worker).
  9. normalizeFromByte: Number.isFinite ? Math.max(0, Math.floor) : 0.
  10. No column list. viewSourceColumns() extracts the quoted source columns from each view statement ("col" AS "alias" → col), withoutColumns() rebuilds the select list without those absent from information_schema; LATER_RUN_VIEW_COLUMNS is gone. Unit-tested against the real RUN_VIEWS_SQL; the fresh-install path runs in every integration/e2e boot. No manual step remains, so no CLAUDE.md note.
  11. Run-home removal. buildTaskStartScript prunes the -wt worktree and runs git worktree prune right after the worktree setup. The plan (§1 "Where things live") and both builders' docs state that the attached worker removes the run directory when the run is terminal and consumed_bytes equals the file size (phase B). Backstop now: run-home-sweep-service.ts, called from the repo-cleanup worker's health cycle, lists /home/agent/optio/runs in every ready pod (with each output file's size) and removes directories of runs that are terminal and either non-protocol, drained, or terminal for over an hour; unknown directories are left alone.
  12. One tape module. fake-tape.mjs (+ fake-tape.d.mts) holds the markers, directive helpers, planCommandRun and planAgentRun; both fake.ts's exec session and fake-agent.mjs play the plan.
  13. ApiServerHandle.kill(signal) (group kill, SIGKILL after 15 s for other signals); stop() is kill("SIGTERM") with 10 s; api-cluster uses it — one killGroup.
  14. /api/health returns instanceId only when req.user is set (schema optional); route test covers both; the e2e asserts the public probe carries nothing identifying.
  15. INSTANCE_STARTED_AT dropped; instance.ts imported statically in index.ts.

Disagreed with nothing. Two notes: the fake runtime verifies a pid's identity through /proc on Linux and falls back to "pid alive" on macOS (no /proc), which is where the fake's own tests run; and item 7's prelude also points fd 0 at /dev/null, so no setup command can eat the stdin bytes before head does.

Verification after the fixes: Docker check on ubuntu:24.04 + tini (ALL PASS: pid 1 is tini; start/attach/deliver/EOF exit 7; offset replay; TERM → 143 with no leftovers; SIGKILL → lost; stale pid → kill none / attach LOST; 300 KB + 200 KB byte-exact; no zombies after every phase). Tiers: unit (shared 748, runtime 169 + 5 Linux-only skipped here, api 3027), integration 51 files / 409, e2e 23 files / 102, format, typecheck 13/13, helm lint — all green.

@jonwiggins
jonwiggins merged commit 69e5aac into main Oct 10, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant