docs(09-01): arena-proxy tracer complete — SUMMARY + deferred-items

Implements 09-01-PLAN.md (BOT-03 reverse-proxy tracer) end-to-end in the
botfight repo (pushed to origin main): server/src/middleware/arena-proxy.ts,
its 9-test end-to-end suite, app.ts mount, fights.ts SSE fix, docker-compose.yml
env-var docs. Records the T-09-02 threat-model correction (plain HTTP is a
supported upstream scheme, not https-only, per user decision) and a
pre-existing migrate.ts/schema.ts drift found out-of-scope during
verification.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
archipelago 2026-07-30 21:39:13 -04:00
parent 6a4043ce3d
commit 2ec9a70dfd
3 changed files with 246 additions and 1 deletions

View File

@ -220,7 +220,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8
Plans:
- [ ] 09-01-PLAN.md — Arena reverse-proxy tracer: node instances become thin clients of one shared arena (BOT-03)
- [x] 09-01-PLAN.md — Arena reverse-proxy tracer: node instances become thin clients of one shared arena (BOT-03)
- [ ] 09-02-PLAN.md — Finish native nostr signer login: JWT-only GET /api/auth/me, bare-pubkey path retired (BOT-01)
- [ ] 09-03-PLAN.md — One self-contained AI bot-setup prompt served at /api/docs/prompt (BOT-02)
- [ ] 09-04-PLAN.md — Canonical public arena on VPS2 + DNS/TLS via nginx-proxy-manager (BOT-03)

View File

@ -0,0 +1,202 @@
---
phase: 09-botfights-platform-upgrade
plan: 01
subsystem: api
tags: [hono, reverse-proxy, sse, nodejs-fetch, arena-federation]
requires: []
provides:
- "ARENA_UPSTREAM_URL reverse-proxy middleware (server/src/middleware/arena-proxy.ts) mounted on /api/* in server/src/app.ts"
- "SSE-through-nginx opt-out header (X-Accel-Buffering: no) on GET /api/fights/:id/stream"
- "Documented env-var contract (ARENA_UPSTREAM_URL, TRUSTED_PROXY) in docker-compose.yml"
affects: ["09-04 canonical VPS2 arena deploy", "09-06 apps/botfights/manifest.yml ARENA_UPSTREAM_URL default"]
tech-stack:
added: []
patterns:
- "Reverse-proxy middleware reads process.env on every call (not module scope) so it can be toggled by tests/config without a restart"
- "Response(upstreamRes.body, ...) passthrough — never buffer with .text()/.json() before returning, or SSE breaks"
- "Hop-by-hop header set (HOP_BY_HOP) shared between request-forward and response-copy paths"
key-files:
created:
- server/src/middleware/arena-proxy.ts
- server/src/middleware/arena-proxy.test.ts
modified:
- server/src/app.ts
- server/src/routes/fights.ts
- docker-compose.yml
key-decisions:
- "Arena upstream accepts both http:// and https:// (no scheme assumption in code) — the default VPS2 arena is plain HTTP per user decision; TLS is an env-only upgrade path later, never hardcoded."
- "SSE stream path (/api/fights/:id/stream) is exempt from the 30s AbortSignal.timeout; every other proxied request gets one."
- "/api/health is bypassed (answered locally) even in proxy mode so the manifest health check never depends on arena reachability."
patterns-established:
- "arena-proxy.ts's HOP_BY_HOP header set + LOCAL_BYPASS_PATHS constant is the pattern for any future local-vs-proxied route split in this app."
requirements-completed: [BOT-03]
coverage:
- id: D1
description: "POST /api/bots against a node instance in proxy mode is served by the canonical arena, and the bot is visible via that node's GET /api/bots (cross-node visibility)"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#registers a bot upstream and reads it back through the proxy"
status: pass
human_judgment: false
- id: D2
description: "Standalone mode (ARENA_UPSTREAM_URL unset) falls through to local routers unchanged"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#falls through to local routers when ARENA_UPSTREAM_URL is unset"
status: pass
human_judgment: false
- id: D3
description: "GET /api/health answers locally even when the arena is unreachable"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#answers /api/health locally even in proxy mode"
status: pass
human_judgment: false
- id: D4
description: "Method, query string, and JSON body forward unchanged; inbound Host header is dropped; stale content-encoding/content-length are stripped from the response"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#forwards method, query string and JSON body unchanged"
status: pass
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#does not forward the inbound Host header"
status: pass
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#strips response content-encoding and content-length"
status: pass
human_judgment: false
- id: D5
description: "A proxied SSE fight stream delivers events incrementally (not buffered until the upstream stream closes)"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#streams SSE incrementally through the proxy"
status: pass
human_judgment: false
- id: D6
description: "Originating client IP reaches the canonical arena via x-forwarded-for/x-real-ip, verified over a real loopback socket"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#forwards the client address in x-forwarded-for"
status: pass
human_judgment: false
- id: D7
description: "An unreachable arena degrades to 502 {error} instead of a hang or a 500 stack trace"
requirement: "BOT-03"
verification:
- kind: integration
ref: "server/src/middleware/arena-proxy.test.ts#answers 502 when the arena is unreachable"
status: pass
human_judgment: false
- id: D8
description: "Live real-world cross-node fight visibility (a bot registered on one node instance visibly fights on another) against the deployed VPS2 arena"
verification: []
human_judgment: true
rationale: "Requires the canonical VPS2 arena to actually be deployed and reachable (plan 09-04) and apps/botfights/manifest.yml to carry ARENA_UPSTREAM_URL (plan 09-06) — this plan only proves the proxy middleware in isolation against a real second HTTP process, not the full deployed topology. Out of this plan's scope per its objective."
duration: 40min
completed: 2026-07-31
status: complete
---
# Phase 9 Plan 01: Arena-Proxy Reverse-Proxy Middleware Summary
**Hono middleware that forwards every `/api/*` request (REST + SSE) to `ARENA_UPSTREAM_URL` via native `fetch`/stream-passthrough when set, proven end-to-end against a real second HTTP server — zero new dependencies.**
## Performance
- **Duration:** ~40 min
- **Started:** 2026-07-31T01:12:00Z (approx.)
- **Completed:** 2026-07-31T01:36:32Z
- **Tasks:** 2/2 completed
- **Files modified:** 5 (2 created, 3 modified)
## Accomplishments
- `server/src/middleware/arena-proxy.ts` — the BOT-03 reverse-proxy: no-ops to `next()` when `ARENA_UPSTREAM_URL` is unset (today's behavior, byte-for-byte); bypasses `/api/health` locally even in proxy mode; forwards method/query/body/headers with a shared `HOP_BY_HOP` drop-list; strips stale `content-encoding`/`content-length` from the response; passes the upstream `ReadableStream` straight through (never buffers, which is what makes SSE work); forwards the caller's IP via `x-forwarded-for`/`x-real-ip` when a real socket address is available; times out non-stream requests at 30s (SSE exempt); returns a clean `502 {error}` JSON body when the arena is unreachable.
- Mounted in `server/src/app.ts` via `app.use('/api/*', arenaProxy)`, positioned before every `app.route('/api/...')` registration.
- `server/src/routes/fights.ts`'s SSE stream handler now sets `X-Accel-Buffering: no` so an nginx-fronted canonical arena (nginx-proxy-manager) doesn't buffer live fight events.
- `docker-compose.yml` documents `ARENA_UPSTREAM_URL` and `TRUSTED_PROXY` (commented, no active value — the canonical arena's own compose file is a later plan).
- 9 end-to-end tests in `server/src/middleware/arena-proxy.test.ts`, all driven against a REAL second Hono process started with `@hono/node-server`'s `serve({ port: 0 })` (not mocks): cross-node bot registration + readback, standalone fallthrough, health bypass, method/query/body forwarding, Host-header drop, content-encoding/length stripping, incremental SSE delivery (elapsed-time assertion), x-forwarded-for over a real loopback socket, and 502 on an unreachable arena.
## Task Commits
Each task was committed atomically in `/home/archipelago/Projects/botfight` (pushed to `origin main`):
1. **Task 1: End-to-end REST forwarding (tracer)**`143ca80` (feat) — `arena-proxy.ts` + 6 REST-focused tests + `app.ts` mount. TDD: tests written first, confirmed failing (module didn't exist — `Failed to load url ./arena-proxy.js`), then implemented to green.
2. **Task 2: SSE, client-IP forwarding, upstream-down**`0511b97` (feat) — x-forwarded-for/x-real-ip, 30s timeout (SSE exempt), 502 handling, `fights.ts`'s `X-Accel-Buffering` header, `docker-compose.yml` docs, + 3 more tests (SSE, XFF, 502). TDD: the 502 test was written first and confirmed failing (`expected 502 to be 500`) against the Task 1 implementation, then implemented to green.
3. **Test hardening (post-hoc improvement)**`a95cada` (test) — strengthened the x-forwarded-for test to drive the proxying app over a real loopback socket (`@hono/node-server`) instead of Hono's in-process `app.request()` harness, which has no real `socket.remoteAddress` to exercise. Not a plan task, but needed to make D6's coverage claim genuinely proven rather than a presence-or-absence tautology.
**Plan metadata:** this SUMMARY + STATE/ROADMAP updates, committed in `archy` via `git push gitea-ai main`.
_Note: both feature tasks used a TDD red→green cycle; no separate refactor commit was needed._
## Files Created/Modified
- `server/src/middleware/arena-proxy.ts` — the reverse-proxy middleware (created)
- `server/src/middleware/arena-proxy.test.ts` — 9 end-to-end tests against a real upstream HTTP server (created)
- `server/src/app.ts` — mounts `arenaProxy` on `/api/*` before the route registrations
- `server/src/routes/fights.ts``X-Accel-Buffering: no` on the SSE stream handler
- `docker-compose.yml` — documents `ARENA_UPSTREAM_URL`/`TRUSTED_PROXY` (commented)
## Decisions Made
- **Plain HTTP is an explicitly supported upstream scheme, not just HTTPS.** The plan's threat register (T-09-02) as originally written assumed `ARENA_UPSTREAM_URL` is always an `https://` origin. Per the user's direct instruction during execution, this is superseded: the default VPS2 arena (`http://146.59.87.168:9100`) is plain HTTP and the proxy must accept both `http://` and `https://` upstreams with no scheme assumption anywhere in the code — and there is none; `buildTargetUrl` and `fetch` are scheme-agnostic by construction. See "Deviations from Plan" below.
- **Timeout scoping is by path pattern, not by streaming-detection at runtime.** `SSE_STREAM_PATH = /^\/api\/fights\/[^/]+\/stream$/` is checked before the fetch, matching the plan's explicit path guidance rather than trying to detect a streaming response after the fact (which would be too late to skip an already-attached `AbortSignal`).
## Deviations from Plan
### Auto-fixed Issues
**1. [Rule 2 - missing critical functionality / threat-model correction] T-09-02 mitigation text superseded — HTTP is a supported upstream scheme, not just HTTPS**
- **Found during:** pre-execution review of the plan's threat model (T-09-02) against the phase's CONTEXT.md "Arena-as-relay" decision and the user's direct instruction for this run.
- **Issue:** The plan's threat register states `ARENA_UPSTREAM_URL` "is an `https://` origin." The actual architecture decision (CONTEXT.md, and the default arena address `http://146.59.87.168:9100` used throughout 09-CONTEXT.md/09-RESEARCH.md/09-PATTERNS.md) is plain HTTP for the default arena, with TLS as an operator-chosen upgrade later — not a hardcoded assumption.
- **Fix:** No code change was needed — the implementation was already scheme-agnostic (`new URL(path + search, upstream)` and `fetch()` work identically for `http://` and `https://`; nothing in `arena-proxy.ts` checks or assumes a scheme). This is a **documentation correction to the threat register**, recorded here since the plan's committed threat model text is now stale.
- **Updated mitigation (superseding T-09-02 as written):** "User-accepted plain HTTP for the default arena; TLS upgrade is env-only later (an operator can point `ARENA_UPSTREAM_URL` at an `https://` origin with zero code change). Credentials crossing node → arena over plain HTTP are visible to any on-path observer between the node and VPS2 — accepted for the alpha/beta default arena per user decision; nodes on an untrusted network path to VPS2 should either use a VPN/Tailscale hop or wait for the TLS-upgraded arena."
- **Files affected:** none (documentation-only; no source change required).
- **Not blocking:** per explicit instruction, this deviation is recorded and not treated as a blocker.
**Total deviations:** 1 (threat-model documentation correction, no code change; already scheme-agnostic by construction).
**Impact on plan:** None on delivered code — the implementation matches the corrected, user-stated architecture. Threat register text should be updated in a future pass over `09-01-PLAN.md` or carried forward into `09-04`/`09-06`'s threat models.
## Issues Encountered
- **Fresh local dev environment needed setup before the pre-existing full server test suite (`pnpm vitest run --project server`) could run at all:** `node_modules` was missing (ran `pnpm install`); `better-sqlite3`'s native binding wasn't built because pnpm's newer build-approval gate silently skipped postinstall scripts (`pnpm approve-builds --all`); the local dev SQLite DB had never been migrated. These are one-time local environment bootstrap steps, not code changes, and none were committed (the `pnpm-lock.yaml`/`pnpm-workspace.yaml` churn from a locally-different pnpm version was explicitly reverted with `git checkout --` before committing anything, to avoid polluting the shared repo with unrelated lockfile noise).
- **Pre-existing schema drift, unrelated to this plan, blocks ~13-20 tests in the full server suite:** `server/src/db/migrate.ts`'s hand-written `CREATE TABLE IF NOT EXISTS` DDL is stale versus `server/src/db/schema.ts` (missing at least the `sats_won` column added for payments features), causing `SqliteError: no such column` 500s in `auth.test.ts`, `auth-audit.test.ts`, `auth-edge.test.ts`, and `tournaments.test.ts` on any freshly-migrated local DB. Verified this is **not caused by arena-proxy**`ARENA_UPSTREAM_URL` is unset in the test environment, so `arenaProxy` is a pure `next()` no-op, and the failures occur deep inside `db.select(...)` calls that never touch the new middleware or its mount point. A small number of additional failures (`answers.test.ts`, `lifecycle.test.ts`, `bot-auth.test.ts`'s constant-time-comparison test) are timing-budget assertions that flake under this machine's parallel-worker CPU contention — also pre-existing and unrelated. Full detail logged to `.planning/phases/09-botfights-platform-upgrade/deferred-items.md` per the Scope Boundary rule (out-of-scope for this plan's files). **This plan's own required commands are unaffected and green:** `pnpm vitest run server/src/middleware/arena-proxy.test.ts` (9/9), `pnpm vitest run server/src/middleware/arena-proxy.test.ts server/src/middleware/rate-limit.test.ts` (17/17), `pnpm exec tsc --noEmit -p server/tsconfig.json` (clean), and `eslint` on all four touched files (clean).
## User Setup Required
None — no external service configuration required. (VPS2 canonical-arena deployment and the manifest's `ARENA_UPSTREAM_URL` default are later plans, 09-04 and 09-06.)
## Next Phase Readiness
- `arena-proxy.ts` is ready to be exercised against the real canonical VPS2 arena once it's deployed (plan 09-04) — no further code change expected on the proxy side; `docker-compose.yml`'s documented `ARENA_UPSTREAM_URL`/`TRUSTED_PROXY` vars are the contract the manifest work (09-06) should follow.
- Deferred item worth carrying forward: the `migrate.ts`/`schema.ts` drift (see `deferred-items.md`) will keep breaking fresh local dev environments and CI-from-scratch until a future plan replaces the hand-written migration script with real `drizzle-kit generate`/`migrate` output.
- The plan's threat register text for T-09-02 should be corrected in a future edit pass to match the "plain HTTP accepted for the default arena" decision recorded above, so it doesn't read as contradicting what was actually built.
---
*Phase: 09-botfights-platform-upgrade*
*Completed: 2026-07-31*
## Self-Check: PASSED
- FOUND: `/home/archipelago/Projects/botfight/server/src/middleware/arena-proxy.ts`
- FOUND: `/home/archipelago/Projects/botfight/server/src/middleware/arena-proxy.test.ts`
- FOUND: `/home/archipelago/Projects/archy/.planning/phases/09-botfights-platform-upgrade/09-01-SUMMARY.md`
- FOUND: `/home/archipelago/Projects/archy/.planning/phases/09-botfights-platform-upgrade/deferred-items.md`
- FOUND commit `143ca80` (Task 1) in `botfight` git history
- FOUND commit `0511b97` (Task 2) in `botfight` git history
- FOUND commit `a95cada` (test hardening) in `botfight` git history
- `origin/main` HEAD matches local HEAD (`a95cada`) — push confirmed landed

View File

@ -0,0 +1,43 @@
# Deferred Items — Phase 9 Plan 01 (arena-proxy tracer)
Out-of-scope discoveries found while executing 09-01-PLAN.md. Not fixed (per
Scope Boundary rule — these are pre-existing, unrelated to the files this plan
touches).
## 1. `server/src/db/migrate.ts` is stale vs `server/src/db/schema.ts` (blocks a clean local `pnpm vitest run --project server`)
- **Found during:** Task 1/2 verification (`pnpm vitest run --project server`).
- **Symptom:** `SqliteError: no such column: "sats_won"` (and similar) on a freshly
migrated local dev DB (`server/data/botfights.db`, gitignored). Causes ~14-20
pre-existing test failures in `auth.test.ts`, `auth-audit.test.ts`,
`auth-edge.test.ts`, `tournaments.test.ts` — all going through the real
`db/index.ts` singleton against a table missing columns that `schema.ts`
declares (e.g. `satsWon` / `sats_won` in the `bots` table).
- **Root cause:** `server/src/db/migrate.ts` hand-writes `CREATE TABLE IF NOT
EXISTS` DDL that has drifted out of sync with `schema.ts` (payments/wallet
columns were added to the Drizzle schema without updating the raw migration
script). `drizzle-kit push` also fails against the existing db for the same
reason (introspection step queries a column drizzle-kit itself expects to
exist).
- **Verified unrelated to this plan's changes:** the failing requests never
touch `arena-proxy.ts``ARENA_UPSTREAM_URL` is unset in the test env, so
`arenaProxy` is a pure `next()` no-op; the SQL error occurs deep in
`db.select(...)` inside `routes/auth.ts`/`routes/tournaments.ts`, completely
independent of the proxy mount added in `app.ts`. Confirmed by reproducing
the identical error on the fresh migrated DB before making any arena-proxy
changes were involved in that code path.
- **Recommendation:** A future plan should either update `migrate.ts`'s DDL to
match `schema.ts` column-for-column, or replace it with real drizzle-kit
generated migrations (`drizzle-kit generate` + `drizzle-kit migrate`) so the
two never diverge again.
## 2. Timing-sensitive perf tests are flaky under parallel CPU load
- **Found during:** same full-suite run.
- **Symptom:** `answers.test.ts` ("completes 1000 checks in under 50ms"),
`lifecycle.test.ts` ("10,000 automated fight simulations", "throughput
>500 fights/second"), `bot-auth.test.ts` ("constant-time comparison...
variance is minimal") intermittently fail when Vitest's parallel workers
contend for CPU on this machine. Pre-existing, unrelated to arena-proxy.
- **Recommendation:** Not this plan's concern; note if a future hardening
pass wants to raise these budgets or mark them `--sequential`.