Skip to content

fix(ci): boot protocol E2E on the peer-stamped custom server with preserved open bootstrap (#11535) - #11549

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
jonlwheat2-gif:fix/11535-protocol-e2e-peer-stamp-v3.8.51
Aug 25, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
jonlwheat2-gif:fix/11535-protocol-e2e-peer-stamp-v3.8.51

Conversation

@jonlwheat2-gif

Copy link
Copy Markdown
Contributor

Summary

Freeze-aware retarget of #11535's fix to the active development branch: release/v3.8.50 is frozen, so per the parallel-cycle model this lands on release/v3.8.51. Cherry-picked from the validated fork branch (fix/11535-protocol-e2e-peer-stamped-server, commit 3abf113) onto .51 tip (ae7a843) as 73fb0f6 — applied cleanly, no drift in any touched file.

Fixes the deterministic Protocol Clients E2E failure (#10049 / #11535): GET /api/mcp/audit answered 403 LOCAL_ONLY from loopback because the harness booted a server flavour that never writes the trusted peer stamp. Two further green-shallow 401 traps surfaced while swapping boot targets; all three root causes are fixed here without relaxing any assertion and without touching the shared Playwright webServer runner.

Full investigation (root causes, failed attempts, line-level before/after, red/green evidence): #11535 (comment)

Changes

1. scripts/dev/run-protocol-clients-tests.mjs

  • Line 77: spawn target "scripts/dev/run-next-playwright.mjs" → "scripts/dev/run-next.mjs" — the real custom Node server stamps the TCP peer IP (peer-stamp.mjs:48–68, wired at run-next.mjs:163), so LOCAL_ONLY locality resolves instead of failing closed (management.ts:228).
  • Lines 60–65 (new): HOST: process.env.HOST || "127.0.0.1". Under the programmatic next() entry, middleware nextUrl.hostname mirrors the bind address (run-next.mjs:83, default 0.0.0.0), and apiAuth.isLoopbackRequest() reads nextUrl.hostname first (apiAuth.ts:89–137) — an unpinned boot makes every request look remote and the anonymous open-bootstrap allow never fires.

2. scripts/dev/run-next.mjs

  • New block lines 64–81, after the bootstrap env merge (lines 58–62), gated on the test-only OMNIROUTE_E2E_BOOTSTRAP_MODE === "open": clears INITIAL_PASSWORD / OMNIROUTE_E2E_PASSWORD / OMNIROUTE_API_KEY to empty strings, not deletes.
    • After the merge because bootstrap-env.mjs:180–182 filters empty strings — an injected "" cannot survive it.
    • Empty string because Next's env loader re-reads repo .env during prepare() after this point; a deleted var gets restored and instrumentation-node.ts:392 bcrypt-persists it as a real login at startup (observed live). An existing empty var is falsy to every consumer and wins over dotenv's no-override load.
    • Production boots unchanged: no production flow sets this env var.

3. New regression guard: tests/unit/protocol-e2e-server-stamping-11535.test.ts
Four source-contract tests (repo pattern per dev-script-heap-limit.test.ts): stamped-server spawn target, env-contract incl. HOST pin, peer stamp wired into the custom listener, open-mode clear present + empty-string-not-delete + positioned after the merge.

4. changelog.d/fixes/11535-protocol-e2e-peer-stamped-server.md — fragment per convention.

Validation on this branch (.51 tip, 73fb0f6)

  • Unit guard: ✔ pass 4 / fail 0
  • Live boot (isolated DATA_DIR, default port, INITIAL_PASSWORD=CHANGEME forced into env to simulate the CI .env leak):
    • /api/monitoring/health → 200
    • GET /api/mcp/audit?limit=50&tool=omniroute_get_health → 200 {"entries":[],"total":0,"limit":50,"offset":0} — the audit block in protocol-clients.test.ts:138–143 is genuinely exercised
  • Pre-fix control (same machine, .50 harness flavour): audit 403 LOCAL_ONLY, byte-identical to the CI failure.

Issue requirements (#11535)

Requirement Status
Boot the suite's server with peer stamping ✅ dedicated entry-point path; Playwright runner untouched (blocking test-e2e)
Preserve "open" bootstrap bypassing bootstrap-env.mjs:176 filtering ✅ proven at HTTP level (200 with entries JSON)
Do not relax the assertion ✅ protocol-clients.test.ts:137 untouched

Follow-up (intentionally not in this surgical change): once merged, drop continue-on-error on test-protocols-e2e (ci.yml:1327, annotated :1319–1326).

…served open bootstrap (diegosouzapw#11535)

- run-protocol-clients-tests.mjs spawns scripts/dev/run-next.mjs dev (real
  custom server, trusted PEER_IP_HEADER stamp) instead of the bare next CLI
  via run-next-playwright.mjs, fixing the deterministic 403 LOCAL_ONLY on
  GET /api/mcp/audit from loopback; pins HOST=127.0.0.1 because under the
  programmatic next() entry middleware nextUrl.hostname mirrors the bind
  address and apiAuth.isLoopbackRequest reads it first
- run-next.mjs honors OMNIROUTE_E2E_BOOTSTRAP_MODE=open by clearing
  INITIAL_PASSWORD/OMNIROUTE_E2E_PASSWORD/OMNIROUTE_API_KEY to empty strings
  AFTER the bootstrap env merge — empty string, not delete, so Next's dotenv
  re-read of repo .env during prepare() cannot restore a leaked credential
  that instrumentation would bcrypt-persist (401 green-shallow)
- regression guard: tests/unit/protocol-e2e-server-stamping-11535.test.ts
  (4 source-contract tests, red before / green after)
- changelog fragment: changelog.d/fixes/11535-protocol-e2e-peer-stamped-server.md

Live validation on release/v3.8.50 @ 7790b0d:
before: health 200, audit 403 LOCAL_ONLY | after: audit 200 with entries JSON,
settings PATCH 200, agent card 200. Playwright webServer runner untouched.
@diegosouzapw
diegosouzapw merged commit 700819a into diegosouzapw:release/v3.8.51 Aug 25, 2026
10 of 16 checks passed
diegosouzapw pushed a commit to jonlwheat2-gif/OmniRoute that referenced this pull request Aug 25, 2026
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…served open bootstrap (diegosouzapw#11535) (diegosouzapw#11549)

Validated in a combined 3-PR batch worktree off release/v3.8.51 tip.
- Focused test: protocol-e2e-server-stamping-11535.test.ts — part of batch's 165/165 node:test run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity, check:docs-counts-sync — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for the meticulous root-causing here — three distinct issues (server flavor, HOST pin, open-bootstrap env leak) traced to exact line numbers with live-boot before/after evidence.
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.

2 participants