[Aspirant200715]

Part of #26 (O10). Covers the worker VM, the hosting wiring around it, and the edge cases around crashes and redeploys.

What this adds

• deploy/worker-vm/: compose file, runbook, env template, preflight.sh, create-db-role.sh + db-role.sql (a role that can touch only jobs, pr_requests, plans, tasks)
• Worker start-up: one process per WORKER_ID (advisory lock), optional REQUIRE_ALL_AGENT_KEYS, sweep of abandoned workspaces, warning when named after the hostname
• API: GET /healthz, compose healthcheck; update.sh waits for it
• Caddy: api.mergit.site serves browser routes and /healthz; /v0/internal/* only from WORKER_VM_IP; the rest is 404. One shared route list for app. and api.
• cp_postgres published on ${CP_DB_BIND:-127.0.0.1}:5433

What I verified

• 557 control-plane tests pass (incl. new: health, startup, restricted-role privileges, caddy routes); ruff and pyright strict clean
• Worker image run from the VM compose file against real Postgres as the restricted role: duplicate id refused, missing key refused, stale workspace swept, graceful stop, stuck jobs recovered
• Grants file mutation-tested in both directions; Caddyfile validated with real Caddy and every route probed against a stub API

Not verified

• A second real VM, Postgres over a private network, the hosted honest run (Y14 step 1)
• gVisor, a real model call
• Merging changes production on the next deploy: api.mergit.site returns 404 outside the allow-list and cp_postgres/api are recreated

Follow-up

• Remove the worker service from compose.controlplane.yaml after cutover (see deploy/worker-vm/README.md)
• Update O10: host the API and the worker VM #26's text: hosting moved from Render to Hetzner

[OfficialAbhinavSingh]

grants line up with every query the worker makes and the role script reruns cleanly. one real problem: the stock postgres image lets any role in over tcp and the postgres password is committed, so once CP_DB_BIND is set the worker VM could log in as superuser and skip mergit_worker. pushed a pg_hba.conf so only mergit_worker gets in from outside the compose network, with a test.

follow-ups for later: the advisory lock isn't retaken if cp_postgres restarts, and a db that's down at start gives a raw traceback.

[Viscous106]

Good direction on the lockdown. The Caddy routes, the create-db-role.sh input handling and /healthz returning only {"ok": …} all look solid. I read the diff only and didn't run it, so please treat the first two items as things to confirm.

Should fix

1. Lock release after a power loss or reboot (startup.py:247-268). The advisory lock only frees when Postgres notices the dead session. A SIGKILL frees it at once, but a VM power loss or network partition sends no FIN, so the lock can stay held until TCP keepalive fires (about 2h by default). The restarted worker then logs "already running", exits 2, and Docker loops it. test_a_killed_worker… uses pg_terminate_backend, which doesn't model this. Please set keepalives_idle, keepalives_interval and keepalives_count on the lock connection, or tcp_keepalives_* on the server. A dropped lock connection also loses the lock silently, so it's worth handling.
2. Superuser protection relies on samenet (pg_hba.conf:1017). postgres with the hardcoded mergit password is allowed from samenet. The comment says Docker preserves the source IP on published ports. With docker-proxy, userland-proxy or iptables:false, the source is the bridge gateway, which is inside samenet, so the worker VM would get superuser. Please use the explicit compose subnet, or a non-guessable password. The tests are static text checks and can't catch this.
3. /healthz is public (health.py:69-76) and runs a DB query per request with no rate limit. A flood can exhaust the API pool. Cache the result for about 2s, or probe the DB only from the compose healthcheck.
4. README order. Step 3 (create-db-role.sh) runs before step 4 (up -d), but the script needs cp_postgres running and the tables created. On a fresh server it can leave a half-configured role.

Worth fixing

• The DB link is cleartext by default (sslmode=prefer, plain host). Use hostssl and verify-full, or say plainly that cleartext is accepted.
• ports: ["8000:8000"] on api is unchanged but still publishes the unproxied API, bypassing the Caddy lockdown. Bind it to 127.0.0.1 or drop it.
• update.sh:1055 makes a good deploy exit non-zero if the API isn't reachable through Caddy, even for web or app deploys.
• UPDATE is granted on every column of jobs and pr_requests. A compromised worker can rewrite any pr_url, so the README's "least privilege" wording overstates it.
• The tests run DROP ROLE mergit_worker (cluster-global) and terminate any backend matching pg_try_advisory_lock. That's unsafe against a real Postgres and flaky in parallel runs.

Nits: os.chmod in _remove_workspace follows symlinks. preflight.sh keeps quote characters Compose strips. psycopg.connect has no connect_timeout. Nothing asserts the CP_DB_BIND default of 127.0.0.1.

[OfficialAbhinavSingh]

pushed ddd1c04 to this branch, addressing the should-fix items from the last review that were still open:

• pg_hba.conf + compose.controlplane.yaml: pinned the compose network to an explicit subnet (172.30.9.0/24) instead of samenet, which some Docker network settings can widen to include the worker VM.
• api's 8000:8000 bound to 127.0.0.1 instead of every interface, for a run without compose.server.yaml.
• README: create-db-role.sh now runs after the stack is up, matching what the script itself says it needs.
• connect_timeout=10 on the three places a worker/api reconnects to Postgres, so an unreachable database fails fast instead of hanging.

two new tests pin the subnet and the loopback binding. full suite green (328 passed), ruff and pyright clean.

still open from that review, not attempted here: the worker-VM-power-loss case (TCP keepalives so Postgres notices a dead session faster than its ~2h default) and /healthz caching/rate-limiting.

bigger thing found doing this: this branch is now stale against main's Caddyfile in a real, non-mechanical way. main has since merged mergit.site and app.mergit.site into one combined block serving the live app on both hostnames, with the @api matcher written inline. this branch still has the old split (mergit.site as a separate static site, a shared (api_routes) snippet imported by app.mergit.site and api.mergit.site). there's a matching conflict in app.py's router imports (main has added redeem, routes_account, routes_pr, routes_reputation, routes_routing since) and in test_caddy_routes.py.

i don't know whether that unification on main was deliberate for mergit.site, so i'm not guessing at it here. this needs a real rebase against current main before merging, not just a fast-forward.

[Viscous106]

Re-checked the current diff (25 files) against my earlier review. Three of the findings are fixed, which is good, but #38 conflicts with main, has no CI checks, and several findings remain. I read the diff only and ran nothing.

Fixed, thanks

• pg_hba.conf now uses an exact CIDR with the pinned subnet instead of samenet.
• The README order is right (stack up, then the role script).
• The API publishes on 127.0.0.1:8000, with a test.
• connect_timeout=10 is in WorkerIdLock.acquire, pr_store.py and jobs.py.

Still open

1. Lock after a power loss or reboot (startup.py). There are no TCP keepalives anywhere. A VM power loss or network partition leaves a half-open backend holding the advisory lock for about 2 hours until the OS timeout, and the restarted worker exits 2 and loops. Set keepalives_idle, keepalives_interval and keepalives_count on the lock conninfo, or tcp_keepalives_* on the server.
2. /healthz is public, runs a pool SELECT 1 per request with no cache or rate limit, and logs a full traceback with exc_info=True. A flood can exhaust the API pool. Cache the result for about 2s, or probe the DB only from the compose healthcheck.
3. Cleartext DB link: sslmode=prefer in create-db-role.sh and worker.env.example, with no hostssl.
4. update.sh: wait_healthy returns 1 under set -e if api.mergit.site is unreachable, so a good deploy exits non-zero.
5. Tests run DROP ROLE mergit_worker (test_worker_role.py), which is cluster-wide. Against a dev or prod cluster it destroys the real role.
6. CONNECT isn't revoked from PUBLIC in db-role.sql, and there are no default privileges, so tables created later aren't covered by REVOKE ALL ON ALL TABLES.

New

• The networks.default.ipam pin in compose.controlplane.yaml: the merged compose files share one default network, so up may fail on a server whose existing network has a different subnet. The README doesn't say the network must be recreated. I couldn't confirm this.

Process

• It conflicts with main (GitHub shows CONFLICTING), so it needs a rebase.
• #42 is stacked on an older snapshot of this PR and lacks several of the fixes above. Please coordinate: land this first, then rebase #42 to its delta only.

Not verified: anything at runtime, the cause of the conflict (inferred), and Docker's source-IP behaviour.