From 2ec9a70dfd61fce5c245b57acc08b7c825688f27 Mon Sep 17 00:00:00 2001 From: archipelago Date: Thu, 30 Jul 2026 21:39:13 -0400 Subject: [PATCH] =?UTF-8?q?docs(09-01):=20arena-proxy=20tracer=20complete?= =?UTF-8?q?=20=E2=80=94=20SUMMARY=20+=20deferred-items?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .planning/ROADMAP.md | 2 +- .../09-01-SUMMARY.md | 202 ++++++++++++++++++ .../deferred-items.md | 43 ++++ 3 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 .planning/phases/09-botfights-platform-upgrade/09-01-SUMMARY.md create mode 100644 .planning/phases/09-botfights-platform-upgrade/deferred-items.md diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index f9bfefb5..22478457 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -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) diff --git a/.planning/phases/09-botfights-platform-upgrade/09-01-SUMMARY.md b/.planning/phases/09-botfights-platform-upgrade/09-01-SUMMARY.md new file mode 100644 index 00000000..3aeee552 --- /dev/null +++ b/.planning/phases/09-botfights-platform-upgrade/09-01-SUMMARY.md @@ -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 diff --git a/.planning/phases/09-botfights-platform-upgrade/deferred-items.md b/.planning/phases/09-botfights-platform-upgrade/deferred-items.md new file mode 100644 index 00000000..74997e4c --- /dev/null +++ b/.planning/phases/09-botfights-platform-upgrade/deferred-items.md @@ -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`.