From 4a9ba1d056879e09c483ad83eda4553a16912eaf Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:33:31 -0700 Subject: [PATCH 1/7] renderer: a read that asks for fresh data, or any read after a mutation, no longer answers from the 1s GET dedupe cache The fetch interceptor dedupes identical GETs through a 1s response cache. Two cases must never be served from it: a caller that said cache: 'no-store' / 'reload' (it wants the network), and any GET after a mutation that may have changed what it reads. Neither was honoured, so Settings > Memory could add a fact and immediately refetch the list into the pre-save copy cached by its own mount fetch, showing "Nothing saved yet" for a fact the store already held until the panel was reopened. Now no-store/reload bypasses both the cache and an in-flight join, and every successful mutation clears the GET cache (it is a burst dedupe, not a store; the cost is at most one extra round trip per URL). Policy lives in a pure module with a node test. Co-Authored-By: Claude Opus 5 --- frontend/src/shared/config.ts | 17 ++++++++++++++--- frontend/src/shared/getCachePolicy.test.ts | 17 +++++++++++++++++ frontend/src/shared/getCachePolicy.ts | 15 +++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 frontend/src/shared/getCachePolicy.test.ts create mode 100644 frontend/src/shared/getCachePolicy.ts diff --git a/frontend/src/shared/config.ts b/frontend/src/shared/config.ts index c0aec765f..bec5aad44 100644 --- a/frontend/src/shared/config.ts +++ b/frontend/src/shared/config.ts @@ -1,4 +1,5 @@ import { noteBackendFailure, noteBackendSuccess, noteRequestStalled, setBackendProber } from '@/shared/backendConnection'; +import { bypassesGetCache, mutationClearsGetCache } from '@/shared/getCachePolicy'; const _w = window as any; // Prefer the preload-injected port; if it's missing (preload raced the backend port being picked), re-query the live value before falling back to 8324. The bare 8324 guess is wrong on any machine where the backend landed on a fallback port (e.g. 8324 was held by a leftover backend); see the self-heal below. @@ -141,6 +142,11 @@ function _installAuthFetchInterceptor() { try { const resp = await withStallWatch(() => originalFetch(input, finalInit)); noteBackendSuccess(); + // A refresh right after a save must see the save: a list GET cached moments before the + // mutation (a panel's mount fetch, a poll) would otherwise answer the refresh with the + // pre-save list, and the UI sits stale until something else refetches (Settings > Memory + // showed "Nothing saved yet" for a fact the store already held). + if (mutationClearsGetCache(method)) _cachedFetches.clear(); return resp; } catch (err) { noteBackendFailure(); @@ -151,15 +157,20 @@ function _installAuthFetchInterceptor() { const cacheKey = `GET ${url}`; + // A caller that asked for no-store/reload gets the network; the dedupe cache is for bursts of + // identical default reads (twenty cards mounting), not for a read that wants fresh truth. + const wantsFresh = bypassesGetCache(finalInit?.cache ?? (input instanceof Request ? input.cache : undefined)); const cached = _cachedFetches.get(cacheKey); - if (cached && cached.expiresAt > Date.now()) { + if (cached && cached.expiresAt > Date.now() && !wantsFresh) { return cached.resp.clone(); - } else if (cached) { + } else if (cached && (wantsFresh || cached.expiresAt <= Date.now())) { _cachedFetches.delete(cacheKey); } + // Same for an identical GET already in flight: joining one that started before a mutation + // would hand a fresh-wanting caller the pre-mutation answer. const inflight = _inflightFetches.get(cacheKey); - if (inflight) { + if (inflight && !wantsFresh) { const resp = await inflight; return resp.clone(); } diff --git a/frontend/src/shared/getCachePolicy.test.ts b/frontend/src/shared/getCachePolicy.test.ts new file mode 100644 index 000000000..84e4bfae0 --- /dev/null +++ b/frontend/src/shared/getCachePolicy.test.ts @@ -0,0 +1,17 @@ +// Run: node --test frontend/src/shared/getCachePolicy.test.ts +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { bypassesGetCache, mutationClearsGetCache } from './getCachePolicy.ts'; + +test('no-store and reload bypass the dedupe cache; the defaults do not', () => { + assert.equal(bypassesGetCache('no-store'), true); + assert.equal(bypassesGetCache('reload'), true); + assert.equal(bypassesGetCache('default'), false); + assert.equal(bypassesGetCache('force-cache'), false); + assert.equal(bypassesGetCache(undefined), false); +}); + +test('every mutation clears the GET cache; reads never do', () => { + for (const m of ['POST', 'post', 'PATCH', 'PUT', 'DELETE']) assert.equal(mutationClearsGetCache(m), true, m); + for (const m of ['GET', 'get', 'HEAD', 'OPTIONS']) assert.equal(mutationClearsGetCache(m), false, m); +}); diff --git a/frontend/src/shared/getCachePolicy.ts b/frontend/src/shared/getCachePolicy.ts new file mode 100644 index 000000000..086165c75 --- /dev/null +++ b/frontend/src/shared/getCachePolicy.ts @@ -0,0 +1,15 @@ +// The renderer dedupes identical GETs to our API through a 1s response cache (config.ts). Two +// things must never be served from it: a request that asked for fresh data, and any GET after a +// mutation that could have changed what it reads. Pure so it can be tested without a window. + +/** A caller that says no-store / reload wants the network, not the 1s dedupe cache. */ +export function bypassesGetCache(cache: RequestCache | undefined): boolean { + return cache === 'no-store' || cache === 'reload'; +} + +/** After a successful mutation the whole GET cache is stale-by-assumption; it is a burst dedupe, + * not a store, so dropping it costs at most one extra round trip per URL. */ +export function mutationClearsGetCache(method: string): boolean { + const m = method.toUpperCase(); + return m !== 'GET' && m !== 'HEAD' && m !== 'OPTIONS'; +} From 2f141ccee9d57e2c35de37ef355d34e0005d1b02 Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:32:02 -0700 Subject: [PATCH 2/7] renderer: shared/config and backendConnection are import-safe without a window, so reducer tests run under node:test Both modules touched window at import time (port/host derivation, the fetch interceptor install, the debug handle), so any node:test file that imports a reducer importing API_BASE died with 'window is not defined' before its first assertion; fetchSessionsStrip.test.ts has been red that way since the resilience work landed, unnoticed because nothing runs these tests in CI. In a renderer (window present) nothing changes: same port/host, same interceptor, same handle. Without one the module answers with the defaults and installs nothing. --- frontend/src/shared/backendConnection.ts | 5 ++++- frontend/src/shared/config.ts | 13 +++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/frontend/src/shared/backendConnection.ts b/frontend/src/shared/backendConnection.ts index e93237e10..c410bfc04 100644 --- a/frontend/src/shared/backendConnection.ts +++ b/frontend/src/shared/backendConnection.ts @@ -80,4 +80,7 @@ export function noteRequestStalled(): void { } // Harness/debug handle: lets a live session (CDP, support) read the signal without a store import. -(window as unknown as { __OSW_CONN?: object }).__OSW_CONN = { backendReachable, onBackendReachability }; +// Guarded: reducers that import this module also run under node:test, where there is no window. +if (typeof window !== 'undefined') { + (window as unknown as { __OSW_CONN?: object }).__OSW_CONN = { backendReachable, onBackendReachability }; +} diff --git a/frontend/src/shared/config.ts b/frontend/src/shared/config.ts index c0aec765f..f92bbe614 100644 --- a/frontend/src/shared/config.ts +++ b/frontend/src/shared/config.ts @@ -1,6 +1,9 @@ import { noteBackendFailure, noteBackendSuccess, noteRequestStalled, setBackendProber } from '@/shared/backendConnection'; -const _w = window as any; +// Import-safe outside a renderer: reducers that import API_BASE also run under node:test, where there +// is no window. The module then answers with the defaults and installs nothing. +const hasWindow = typeof window !== 'undefined'; +const _w = (hasWindow ? window : {}) as any; // Prefer the preload-injected port; if it's missing (preload raced the backend port being picked), re-query the live value before falling back to 8324. The bare 8324 guess is wrong on any machine where the backend landed on a fallback port (e.g. 8324 was held by a leftover backend); see the self-heal below. const port = _w.__OPENSWARM_PORT__ || @@ -8,7 +11,7 @@ const port = ? _w.openswarm.getBackendPortLive() : 0) || 8324; -const host = window.location.hostname || 'localhost'; +const host = (hasWindow && window.location.hostname) || 'localhost'; export const API_BASE = `http://${host}:${port}/api`; export const WS_BASE = `ws://${host}:${port}`; @@ -217,5 +220,7 @@ function _installAuthFetchInterceptor() { }; } -_installAuthFetchInterceptor(); -ensureAuthToken(); +if (hasWindow) { + _installAuthFetchInterceptor(); + ensureAuthToken(); +} From 71475b66cd157cc33dee5c3801616cf17e182442 Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:32:02 -0700 Subject: [PATCH 3/7] ci: run the backend, frontend and edge test suites on every pull request Nothing ran any of them in CI: the 235-file backend pytest suite, the 22 renderer node:test files and the edge suite were run by hand, one file at a time, so a regression only surfaced when someone happened to run the right one. Three small workflows, hosted ubuntu, path-filtered, read-only token: - backend-tests: pytest on Python 3.13 from the locked requirements, plus a completion assertion (junit testcase count == collect-only count) so a test process that dies mid-run can never read as green - frontend-tests: tsc --noEmit + node:test via tsx over src/**/*.test.ts(x), through frontend/scripts/run-tests.mjs (the runner the tests already name) - edge-tests: pytest for openswarm-edge All three are green on the current tree: 2951 backend tests, 143 frontend tests across 22 files, 14 edge tests. --- .github/workflows/backend-tests.yml | 65 ++++++++++++++++++++++++++++ .github/workflows/edge-tests.yml | 44 +++++++++++++++++++ .github/workflows/frontend-tests.yml | 43 ++++++++++++++++++ frontend/scripts/run-tests.mjs | 29 +++++++++++++ 4 files changed, 181 insertions(+) create mode 100644 .github/workflows/backend-tests.yml create mode 100644 .github/workflows/edge-tests.yml create mode 100644 .github/workflows/frontend-tests.yml create mode 100644 frontend/scripts/run-tests.mjs diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml new file mode 100644 index 000000000..adb758a23 --- /dev/null +++ b/.github/workflows/backend-tests.yml @@ -0,0 +1,65 @@ +name: backend-tests + +# The backend pytest suite, run on every pull request and on pushes to the mainline branches. Until +# now nothing ran it in CI, so a regression only surfaced when someone ran it by hand. +on: + pull_request: + paths: + - 'backend/**' + - '.github/workflows/backend-tests.yml' + push: + branches: [main, dev] + paths: + - 'backend/**' + - '.github/workflows/backend-tests.yml' + workflow_dispatch: + +# Runs checked-out project code on pull_request: the token stays read-only. +permissions: + contents: read + +concurrency: + group: backend-tests-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + pytest: + name: pytest (ubuntu) + runs-on: ubuntu-latest + timeout-minutes: 25 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.13' + cache: pip + cache-dependency-path: | + backend/requirements.lock + backend/requirements-dev.txt + - name: Install backend deps (locked runtime + dev) + run: | + python -m pip install --require-hashes --only-binary=:all: -r backend/requirements.lock + python -m pip install -r backend/requirements-dev.txt + # Two numbers that must agree. A test process that dies mid-run can exit 0 with no summary + # (a hard-exit shutdown path did exactly that once and silently skipped ~42% of the suite), so + # the job also asserts that every collected test was actually run: junit testcase count == + # collect-only count. Green then means green, not "green as far as it got". + - name: Count the suite + run: | + python -m pytest backend/tests --ignore=backend/tests/formal --collect-only -q -p no:cacheprovider \ + | tail -1 | tee collected.txt + - name: Run the backend suite + run: | + python -m pytest backend/tests --ignore=backend/tests/formal -q -p no:cacheprovider \ + --junitxml "${RUNNER_TEMP}/pytest.xml" + - name: Every collected test ran + run: | + python - "${RUNNER_TEMP}/pytest.xml" collected.txt <<'PY' + import re, sys, xml.etree.ElementTree as ET + ran = sum(1 for _ in ET.parse(sys.argv[1]).getroot().iter('testcase')) + m = re.search(r'(\d+) tests? collected', open(sys.argv[2]).read()) + collected = int(m.group(1)) if m else -1 + print(f'collected={collected} ran={ran}') + if collected < 1 or ran != collected: + sys.exit(f'FAIL: {ran} of {collected} collected tests reached the report; the run was truncated') + PY diff --git a/.github/workflows/edge-tests.yml b/.github/workflows/edge-tests.yml new file mode 100644 index 000000000..82ef10d9d --- /dev/null +++ b/.github/workflows/edge-tests.yml @@ -0,0 +1,44 @@ +name: edge-tests + +# The openswarm-edge pytest suite, on every pull request that touches it and on pushes to the +# mainline branches. +on: + pull_request: + paths: + - 'openswarm-edge/**' + - '.github/workflows/edge-tests.yml' + push: + branches: [main, dev] + paths: + - 'openswarm-edge/**' + - '.github/workflows/edge-tests.yml' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: edge-tests-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + pytest: + name: pytest (openswarm-edge) + runs-on: ubuntu-latest + timeout-minutes: 10 + defaults: + run: + working-directory: openswarm-edge + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: '3.13' + cache: pip + cache-dependency-path: openswarm-edge/requirements.txt + - name: Install edge deps + run: | + python -m pip install -r requirements.txt + python -m pip install pytest pytest-asyncio + - name: Run the edge suite + run: python -m pytest tests -q -p no:cacheprovider diff --git a/.github/workflows/frontend-tests.yml b/.github/workflows/frontend-tests.yml new file mode 100644 index 000000000..00578bb8f --- /dev/null +++ b/.github/workflows/frontend-tests.yml @@ -0,0 +1,43 @@ +name: frontend-tests + +# Typecheck plus the renderer's node:test suite, on every pull request and on pushes to the mainline +# branches. Until now neither ran in CI; the tests were run by hand, one file at a time. +on: + pull_request: + paths: + - 'frontend/**' + - '.github/workflows/frontend-tests.yml' + push: + branches: [main, dev] + paths: + - 'frontend/**' + - '.github/workflows/frontend-tests.yml' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: frontend-tests-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + typecheck-and-tests: + name: tsc + node:test + runs-on: ubuntu-latest + timeout-minutes: 15 + defaults: + run: + working-directory: frontend + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '20.18.1' + cache: npm + cache-dependency-path: frontend/package-lock.json + - run: npm ci + - name: Typecheck + run: npx tsc --noEmit -p tsconfig.json + - name: Unit tests (node:test via tsx) + run: node scripts/run-tests.mjs diff --git a/frontend/scripts/run-tests.mjs b/frontend/scripts/run-tests.mjs new file mode 100644 index 000000000..8fad6fe2d --- /dev/null +++ b/frontend/scripts/run-tests.mjs @@ -0,0 +1,29 @@ +#!/usr/bin/env node +// Runs every renderer unit test (src/**/*.test.ts, *.test.tsx) under node:test, with tsx doing the +// TypeScript. One command for CI and for a dev machine: `node scripts/run-tests.mjs`, optionally +// followed by file paths to run a subset. Exits non-zero if any test fails or nothing was found. +import { spawnSync } from 'node:child_process'; +import { readdirSync, statSync } from 'node:fs'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = join(dirname(fileURLToPath(import.meta.url)), '..'); +const testFile = (p) => /\.test\.tsx?$/.test(p); + +function walk(dir, out) { + for (const name of readdirSync(dir)) { + if (name === 'node_modules' || name === 'dist') continue; + const p = join(dir, name); + if (statSync(p).isDirectory()) walk(p, out); + else if (testFile(p)) out.push(p); + } + return out; +} + +const files = process.argv.length > 2 ? process.argv.slice(2) : walk(join(root, 'src'), []).sort(); +if (files.length === 0) { + console.error('run-tests: no test files found under src/'); + process.exit(1); +} +const result = spawnSync(process.execPath, ['--import', 'tsx', '--test', ...files], { cwd: root, stdio: 'inherit' }); +process.exit(result.status ?? 1); From 941dfb73be0b97225654b868bd755e601ec17c51 Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Mon, 17 Aug 2026 19:07:12 -0700 Subject: [PATCH 4/7] ci: cap each backend test at 300s so a stall fails by name Two of five hosted runs of the backend suite stalled at 99% until the job cap with no summary and no junit: one test blocked forever on a bare ws.receive_json() (fixed on its own in a separate change). A CI lane should never depend on every test being unable to hang, so add pytest-timeout to the dev requirements and run the suite with --timeout=300. On Linux the default signal method fails just the offending test and the run continues, so the report and the "every collected test ran" assertion stay meaningful. --- .github/workflows/backend-tests.yml | 6 ++++++ backend/requirements-dev.txt | 3 +++ 2 files changed, 9 insertions(+) diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index adb758a23..39972626f 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -48,11 +48,17 @@ jobs: run: | python -m pytest backend/tests --ignore=backend/tests/formal --collect-only -q -p no:cacheprovider \ | tail -1 | tee collected.txt + # --timeout: a test that blocks forever (a bare ws.receive_json() waiting for an event that never + # comes, say) otherwise stalls the run at 99% until timeout-minutes with no summary and no junit. + # With the cap it fails by name, the rest of the suite runs, and the assertion below still holds. - name: Run the backend suite run: | python -m pytest backend/tests --ignore=backend/tests/formal -q -p no:cacheprovider \ + --timeout=300 \ --junitxml "${RUNNER_TEMP}/pytest.xml" - name: Every collected test ran + # Runs after a red suite too, so a failure report also says whether the run was complete. + if: ${{ !cancelled() }} run: | python - "${RUNNER_TEMP}/pytest.xml" collected.txt <<'PY' import re, sys, xml.etree.ElementTree as ET diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt index fe1535065..710b74dc3 100644 --- a/backend/requirements-dev.txt +++ b/backend/requirements-dev.txt @@ -9,6 +9,9 @@ pytest==8.3.4 pytest-asyncio==0.25.2 +# Per-test wall-clock cap for CI (see .github/workflows/backend-tests.yml): a test that +# blocks forever fails by name instead of stalling the whole run until the job cap. +pytest-timeout==2.4.0 # Used by linter/lint.py (the vulture dead-code check). watchfiles, also # needed by lint.py, already comes in transitively via uvicorn[standard]. From 5ced7185c0cf4bc644d17b486b82d004d08fc7a3 Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Mon, 17 Aug 2026 19:05:42 -0700 Subject: [PATCH 5/7] tests: the WS end-to-end turn test cannot hang the suite test_ws_endpoint_streams_a_full_turn_end_to_end read the socket with a bare ws.receive_json() in a 40-iteration loop and broke only on the assistant reply. When the loop ends early for any reason (fewer than 40 events, no reply), the next receive blocks forever and the whole pytest run stalls at 99% until the job cap. On hosted runners it does exactly that intermittently, on Linux and Windows alike: the turn path's configure_provider_env decides whether 9Router needs reviving from provider evidence earlier tests may leave behind, and that revival spawns/installs the router behind a module-level asyncio.Lock; the background turn-label aux call reaches the same machinery. Neither is part of this test's contract ("SDK and WS auth mocked, everything else real"). Pin both out with monkeypatch, bound every receive at 5s (a regression now fails this test instead of hanging the runner), and wait for the turn's completed status before asserting on session.messages so the assertion cannot race the loop's tail. --- backend/tests/test_ws_integration.py | 48 ++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/backend/tests/test_ws_integration.py b/backend/tests/test_ws_integration.py index 741cc223f..33f86210e 100644 --- a/backend/tests/test_ws_integration.py +++ b/backend/tests/test_ws_integration.py @@ -6,6 +6,8 @@ The SDK and WS auth are mocked; everything else is the real running stack.""" import asyncio +import queue +import threading import pytest @@ -17,6 +19,29 @@ import backend.main as main_mod from backend.apps.agents.agent_manager import agent_manager from backend.apps.agents.core.models import AgentSession +import backend.apps.agents.manager.run.RunOptions as run_options_mod + + +def p_receive_json(ws, timeout: float = 5.0): + """ws.receive_json() blocks forever when the event never comes; a regression in the loop then + hangs the whole pytest run instead of failing this test. Bound every receive.""" + out = queue.Queue(maxsize=1) + + def p_recv(): + try: + out.put((True, ws.receive_json())) + except BaseException as exc: + out.put((False, exc)) + + thread = threading.Thread(target=p_recv, daemon=True) + thread.start() + try: + ok, value = out.get(timeout=timeout) + except queue.Empty as exc: + raise AssertionError(f"timed out waiting for websocket event after {timeout}s") from exc + if ok: + return value + raise value def p_assistant(): @@ -33,6 +58,20 @@ def p_result(): def test_ws_endpoint_streams_a_full_turn_end_to_end(monkeypatch): monkeypatch.setattr(main_mod, "p_ws_auth_ok", lambda ws: True, raising=True) + # The contract of this test is "SDK and WS auth mocked, everything else real", but two things on + # the turn path reach outside the process and must not decide the outcome: configure_provider_env + # can wander into 9Router revival (spawn/npm install, serialized on a module-level lock) whenever + # earlier tests left provider evidence behind, and the background turn-label aux call does the + # same. Pin both out; the persistent-client path is pinned off suite-wide in conftest. + async def p_noop_provider_env(*args, **kwargs): + return None + + async def p_noop_turn_label(*args, **kwargs): + return None + + monkeypatch.setattr(run_options_mod, "configure_provider_env", p_noop_provider_env, raising=True) + monkeypatch.setattr(agent_manager, "generate_turn_label", p_noop_turn_label, raising=True) + async def fake_query(*args, **kwargs): yield p_assistant() yield p_result() @@ -49,10 +88,15 @@ async def fake_query(*args, **kwargs): ws.send_json({"event": "agent:send_message", "data": {"prompt": "hi"}}) seen = [] for _ in range(40): - ev = ws.receive_json() + ev = p_receive_json(ws) seen.append(ev.get("event")) - if ev.get("event") == "agent:message" and "hello from the loop" in str(ev.get("data", {})): + if ( + ev.get("event") == "agent:status" + and ev.get("data", {}).get("status") == "completed" + ): break + else: + raise AssertionError(f"did not receive completed status; saw events={seen}") # the real loop's assistant reply made it all the way back over the WS assert "agent:message" in seen assert any(m.role == "assistant" and "hello from the loop" in str(m.content) From ec41b84aa265f0a2fecdbc8c943a7649c0e13446 Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:34:54 -0700 Subject: [PATCH 6/7] renderer: shared/safeMode is import-safe without a window Same class as the config/backendConnection change: safeMode.ts read `window` at import, and dashboardLayoutSlice imports it, so any reducer test that imports the slice died under node:test before it ran. Guard the read; in a renderer nothing changes. --- frontend/src/shared/safeMode.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frontend/src/shared/safeMode.ts b/frontend/src/shared/safeMode.ts index c8c3f6e93..432fb7691 100644 --- a/frontend/src/shared/safeMode.ts +++ b/frontend/src/shared/safeMode.ts @@ -11,7 +11,10 @@ export interface SafeModeInfo { let cached: SafeModeInfo = { safeMode: false, dirtyCount: 0, fingerprint: null }; -const api = (window as unknown as { openswarm?: { getSafeMode?: () => Promise } }).openswarm; +// Import-safe outside a renderer: the layout slice imports this, and its reducer tests run under node:test. +const api = typeof window === 'undefined' + ? undefined + : (window as unknown as { openswarm?: { getSafeMode?: () => Promise } }).openswarm; if (api?.getSafeMode) { void api.getSafeMode().then((info) => { if (info && typeof info.safeMode === 'boolean') cached = info; From da32587d7ca5e846227e740003bc001dfe06287c Mon Sep 17 00:00:00 2001 From: Kai <300677314+kai-openswarm@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:34:05 -0700 Subject: [PATCH 7/7] renderer: token bootstrap is single-flight only while in flight, the dev-token route never re-enters the interceptor, and a transport failure consults the live port at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small changes to shared/config.ts, all in the auth/fetch layer every request goes through: - ensureAuthToken() shares its promise only while a refresh is IN FLIGHT; once settled the slot clears and the cache is the source of truth, and a non-empty cache short-circuits. The resolved-forever memo it replaces either poisoned retries (an empty result) or shadowed a later forced refreshAuthToken() failure — the WebSocket 4401 path — with the stale token. The boot race (ENG-207) stays covered: an empty resolve is never memoized because the slot clears on settle. - refreshAuthToken() treats a non-OK dev-token response like a thrown transport error and clears the cache; a forced refresh that gets a 500 no longer returns the token it was asked to doubt. - The interceptor lets /api/dev/token through as raw transport (no bearer, no dedupe) and awaits it, so a rejection lands in the catch below. Without the bypass, refreshAuthToken's own fetch re-enters the interceptor before _authTokenPromise is assigned (an async fn suspends only at its first await), ensureAuthToken starts another refresh, and the renderer recurses synchronously until RangeError whenever there is no preload bridge — Electron always has one, a plain browser tab on run.sh does not. - A GET transport failure consults the live backend port on the first failed attempt (one-shot, reloads only if the port differs) instead of retrying against a dead port for seconds first, and a TypeError reaching the outer catch does the same. Tests (node:test, one module instance over a dispatching transport stub): the import-time acquisition is recursion-free (stack depth pinned) and one request; concurrent callers share one request; a forced refresh failure or a 500 is never shadowed by a stale token; a failed refresh is not poisoned; bearer injection and passthrough; the 1s GET dedupe; mutations never deduped; the heal hook is consulted at once and the bounded retry recovers; a rejected token transport heals in the catch path with no hidden retry and no bearer; a preload bridge wins over the HTTP route. Against the previous config.ts the file's own import-time bootstrap overflows the stack. --- frontend/src/shared/config.test.ts | 233 +++++++++++++++++++++++++++++ frontend/src/shared/config.ts | 29 +++- 2 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 frontend/src/shared/config.test.ts diff --git a/frontend/src/shared/config.test.ts b/frontend/src/shared/config.test.ts new file mode 100644 index 000000000..2b0f7f0af --- /dev/null +++ b/frontend/src/shared/config.test.ts @@ -0,0 +1,233 @@ +// Characterization of the global fetch interceptor + token acquisition in shared/config.ts. The module installs the interceptor and preloads the token at import, so it is loaded ONCE over a dispatching transport stub (what the interceptor captures as its raw transport) and each case steers the stub and the token state instead of reloading; `window` is the global object, as in a renderer. +import assert from 'node:assert/strict'; +import { before, mock, test } from 'node:test'; + +const TOKEN_URL = 'http://localhost:8324/api/dev/token'; +const apiUrl = (name: string) => `http://localhost:8324/api/test/${name}`; + +type Impl = (input: RequestInfo | URL, init?: RequestInit) => Promise; + +function jsonResponse(body: unknown): Response { + return new Response(JSON.stringify(body), { status: 200, headers: { 'Content-Type': 'application/json' } }); +} + +const g = globalThis as any; +let tokenCallDepth = -1; +let current: Impl = async (input) => { + if (String(input) === TOKEN_URL) { + tokenCallDepth = (new Error().stack ?? '').split('\n').length; + return jsonResponse({ token: 'tok_1' }); + } + return jsonResponse({}); +}; +const underlying = mock.fn((input, init) => current(input, init)); +const callsTo = (url: string) => underlying.mock.calls.filter((c) => String(c.arguments[0]) === url); +const tokenCallsSince = (mark: number) => callsTo(TOKEN_URL).filter((c) => underlying.mock.calls.indexOf(c) >= mark); +const mark = () => underlying.mock.calls.length; +const patchedFetch = () => g.window.fetch as typeof fetch; + +let cfg: typeof import('./config'); +before(async () => { + Error.stackTraceLimit = 500; + g.window = g; + g.location = { hostname: 'localhost', reload: () => {} }; + g.__OPENSWARM_PORT__ = 8324; + delete g.openswarm; + g.fetch = underlying; + cfg = await import('./config'); +}); + +// Empty the cache without a bridge in the way: a failing forced refresh clears it (that behaviour is itself under test below). +async function emptyTokenCache() { + const previous = current; + current = async (input) => { + if (String(input) === TOKEN_URL) throw new TypeError('reset'); + return previous(input); + }; + await cfg.refreshAuthToken(); + assert.equal(cfg.getAuthToken(), ''); + current = previous; +} + +test('token acquisition is recursion-free: shallow stack, one request, no bearer on the token request', async () => { + // Pre-fix mechanism: refreshAuthToken's fetch re-enters the interceptor BEFORE _authTokenPromise is assigned (an async fn suspends only at its first await), so ensureAuthToken starts another refresh, which re-enters again — unbounded SYNCHRONOUS recursion until RangeError, which the interceptor's catch then rescues via the raw transport; the token still resolves, so the sharp pin is the stack depth when the transport is actually called. + const tok = await Promise.race([ + cfg.ensureAuthToken(), + new Promise((_, rej) => setTimeout(() => rej(new Error('token acquisition deadlocked')), 2000)), + ]); + assert.equal(tok, 'tok_1'); + const calls = callsTo(TOKEN_URL); + assert.equal(calls.length, 1); // the import-time preload; ensureAuthToken shared it + assert.ok(tokenCallDepth > 0); + assert.ok(tokenCallDepth < 50, `token transport called ${tokenCallDepth} frames deep`); // pre-fix: hundreds of interceptor frames + assert.equal(new Headers(calls[0].arguments[1]?.headers).has('Authorization'), false); +}); + +test('single-flight: concurrent callers share ONE token request', async () => { + await emptyTokenCache(); + current = async () => jsonResponse({ token: 'tok_1' }); + const m = mark(); + const [a, b] = await Promise.all([cfg.ensureAuthToken(), cfg.ensureAuthToken()]); + assert.equal(a, 'tok_1'); + assert.equal(b, 'tok_1'); + assert.equal(tokenCallsSince(m).length, 1); + assert.equal(cfg.getAuthToken(), 'tok_1'); +}); + +test('a forced refresh failure is never shadowed by a stale token: the next ensureAuthToken returns the NEW token', async () => { + // The 4401 path (WebSocketManager) calls refreshAuthToken() directly; if that forced refresh fails, ensureAuthToken must re-acquire — a still-resolved earlier _authTokenPromise returning stale tok_1 is the defect (single-flight must be in-flight-only, not resolved-forever). + let phase: 'tok1' | 'fail' | 'tok2' = 'tok1'; + current = async (input) => { + if (String(input) === TOKEN_URL) { + if (phase === 'fail') throw new TypeError('backend restarting'); + return jsonResponse({ token: phase === 'tok1' ? 'tok_1' : 'tok_2' }); + } + return jsonResponse({}); + }; + await emptyTokenCache(); + assert.equal(await cfg.ensureAuthToken(), 'tok_1'); + phase = 'fail'; + assert.equal(await cfg.refreshAuthToken(), ''); // forced refresh fails and clears the cache + phase = 'tok2'; + assert.equal(await cfg.ensureAuthToken(), 'tok_2'); // never stale tok_1 + assert.equal(cfg.getAuthToken(), 'tok_2'); +}); + +test('a non-OK token response also clears the cache: forced 500 refresh yields "", then tok_2, never stale tok_1', async () => { + // Only THROWN transport errors hit refreshAuthToken's catch; an HTTP 500 lands in the `r.ok` branch, which must clear the cache too — otherwise the forced refresh returns stale tok_1 and the cache short-circuit keeps serving it. + let phase: 'tok1' | 'http500' | 'tok2' = 'tok1'; + current = async (input) => { + if (String(input) === TOKEN_URL) { + if (phase === 'http500') return new Response('backend error', { status: 500 }); + return jsonResponse({ token: phase === 'tok1' ? 'tok_1' : 'tok_2' }); + } + return jsonResponse({}); + }; + await emptyTokenCache(); + assert.equal(await cfg.ensureAuthToken(), 'tok_1'); + phase = 'http500'; + assert.equal(await cfg.refreshAuthToken(), ''); + assert.equal(cfg.getAuthToken(), ''); + phase = 'tok2'; + assert.equal(await cfg.ensureAuthToken(), 'tok_2'); // never stale tok_1 + assert.equal(cfg.getAuthToken(), 'tok_2'); +}); + +test('a failed refresh is not poisoned: the next ensureAuthToken retries and succeeds', async () => { + let up = false; + current = async (input) => { + if (String(input) === TOKEN_URL) { + if (!up) throw new TypeError('backend not up yet'); + return jsonResponse({ token: 'tok_2' }); + } + return jsonResponse({}); + }; + await emptyTokenCache(); + assert.equal(await cfg.ensureAuthToken(), ''); + up = true; + assert.equal(await cfg.ensureAuthToken(), 'tok_2'); + assert.equal(cfg.getAuthToken(), 'tok_2'); +}); + +test('local API requests get the bearer once the token is resolved; foreign origins pass through untouched', async () => { + current = async (input) => (String(input) === TOKEN_URL ? jsonResponse({ token: 'tok_1' }) : jsonResponse({ ok: true })); + await emptyTokenCache(); + await cfg.ensureAuthToken(); + const url = apiUrl('bearer'); + await patchedFetch()(url, { method: 'POST', body: '{}' }); + assert.equal(new Headers((callsTo(url)[0].arguments[1] as RequestInit).headers).get('Authorization'), 'Bearer tok_1'); + await patchedFetch()('https://example.com/x'); + assert.equal(callsTo('https://example.com/x')[0].arguments[1], undefined); +}); + +test('a caller-set Authorization header is never overwritten', async () => { + current = async (input) => (String(input) === TOKEN_URL ? jsonResponse({ token: 'tok_1' }) : jsonResponse({})); + const url = apiUrl('caller-auth'); + await patchedFetch()(url, { method: 'POST', headers: { Authorization: 'Bearer mine' } }); + assert.equal(new Headers((callsTo(url)[0].arguments[1] as RequestInit).headers).get('Authorization'), 'Bearer mine'); +}); + +test('two GETs inside the window share one transport call; both responses are independently readable', async () => { + current = async (input) => (String(input) === TOKEN_URL ? jsonResponse({ token: 'tok_1' }) : jsonResponse({ n: 1 })); + await cfg.ensureAuthToken(); + const url = apiUrl('dedupe'); + const [r1, r2] = await Promise.all([patchedFetch()(url), patchedFetch()(url)]); + assert.deepEqual(await r1.json(), { n: 1 }); + assert.deepEqual(await r2.json(), { n: 1 }); + assert.equal(callsTo(url).length, 1); +}); + +test('mutations are never deduped: two POSTs are two transport calls', async () => { + current = async (input) => (String(input) === TOKEN_URL ? jsonResponse({ token: 'tok_1' }) : jsonResponse({})); + await cfg.ensureAuthToken(); + const url = apiUrl('mutations'); + await patchedFetch()(url, { method: 'POST', body: '{}' }); + await patchedFetch()(url, { method: 'POST', body: '{}' }); + assert.equal(callsTo(url).length, 2); +}); + +test('a local API transport failure consults the live port (self-heal hook) at once and the bounded GET retry recovers', async () => { + const url = apiUrl('recover'); + let apiAttempts = 0; + current = async (input) => { + if (String(input) === TOKEN_URL) return jsonResponse({ token: 'tok_1' }); + if (String(input) === url) { + apiAttempts += 1; + if (apiAttempts === 1) throw new TypeError('connection refused'); + return jsonResponse({ recovered: true }); + } + return jsonResponse({}); + }; + const getBackendPortLive = mock.fn(() => 8324); // same port → no reload, but the heal hook must be consulted + g.openswarm = { getBackendPortLive, getAuthToken: async () => 'tok_pre' }; + try { + await cfg.ensureAuthToken(); + const resp = await patchedFetch()(url); + assert.deepEqual(await resp.json(), { recovered: true }); + assert.ok(getBackendPortLive.mock.calls.length >= 1); + assert.equal(apiAttempts, 2); + } finally { + delete g.openswarm; + } +}); + +test('a rejected token transport attempt heals in the catch path: port consulted, honest empty token, the next ensure retries, still no bearer', async () => { + // Discriminates `return await originalFetch(...)` from `return originalFetch(...)` on the token bypass: without the await the rejection escapes patchedFetch AFTER it has returned, the catch (port self-heal) never runs, so the live-port hook is never consulted; the failure stays honest (no silent raw retry) and an empty resolve is never memoized, so the next ensureAuthToken() acquires the token. + let tokenAttempts = 0; + current = async (input) => { + if (String(input) === TOKEN_URL) { + tokenAttempts += 1; + if (tokenAttempts === 1) throw new TypeError('connection refused'); + return jsonResponse({ token: 'tok_heal' }); + } + return jsonResponse({}); + }; + const getBackendPortLive = mock.fn(() => 8324); // same port → consulted, but no reload + g.openswarm = { getBackendPortLive }; // no getAuthToken on the bridge: the HTTP dev-token route stays in play while the heal hook exists + try { + const m = mark(); + assert.equal(await cfg.refreshAuthToken(), ''); // the failing attempt: healed, honest empty + assert.equal(getBackendPortLive.mock.calls.length, 1); + assert.equal(tokenCallsSince(m).length, 1); // no hidden raw retry + assert.equal(await cfg.ensureAuthToken(), 'tok_heal'); // the next ensure acquires it + const calls = tokenCallsSince(m); + assert.equal(calls.length, 2); + for (const call of calls) assert.equal(new Headers(call.arguments[1]?.headers).has('Authorization'), false); + assert.equal(cfg.getAuthToken(), 'tok_heal'); + } finally { + delete g.openswarm; + } +}); + +test('preload bridge (Electron path) wins over the dev-token route and its failure is caught', async () => { + await emptyTokenCache(); + current = async () => jsonResponse({ token: 'never' }); + g.openswarm = { getAuthToken: async () => { throw new Error('bridge broken'); } }; + try { + const m = mark(); + assert.equal(await cfg.ensureAuthToken(), ''); + assert.equal(tokenCallsSince(m).length, 0); // never falls through to the HTTP route when a bridge exists + } finally { + delete g.openswarm; + } +}); diff --git a/frontend/src/shared/config.ts b/frontend/src/shared/config.ts index c6f380745..263ef6662 100644 --- a/frontend/src/shared/config.ts +++ b/frontend/src/shared/config.ts @@ -44,6 +44,9 @@ export async function refreshAuthToken(): Promise { if (r.ok) { const data = await r.json(); _authTokenCache = typeof data?.token === 'string' ? data.token : ''; + } else { + // A refresh is a statement that the current token is suspect (the 4401 path): a non-OK response must clear the cache like a thrown transport error does, or the stale token survives the refresh. + _authTokenCache = ''; } } catch { _authTokenCache = ''; @@ -51,14 +54,19 @@ export async function refreshAuthToken(): Promise { return _authTokenCache; } -/** Resolve auth token once; concurrent callers share the same promise. */ +/** + * Single-flight token acquisition. The shared promise is held only while a refresh is IN FLIGHT — once + * settled the slot clears and the cache is the source of truth. A resolved-forever slot either poisons + * retries (an empty result) or shadows a later forced refreshAuthToken() failure (the 4401 path) with a + * stale token; in-flight-only does neither, and a non-empty cache short-circuits so callers never start + * redundant refreshes. Also covers the boot race (ENG-207): an EMPTY resolve is never memoized because + * the slot clears on settle. + */ export function ensureAuthToken(): Promise { if (_authTokenPromise) return _authTokenPromise; - _authTokenPromise = refreshAuthToken().then((tok) => { - // A boot race can resolve EMPTY (backend hadn't written the token file yet); memoizing that - // left the renderer auth-dead until a manual reload (ENG-207). Empty = not an answer; retry. - if (!tok) _authTokenPromise = null; - return tok; + if (_authTokenCache) return Promise.resolve(_authTokenCache); + _authTokenPromise = refreshAuthToken().finally(() => { + _authTokenPromise = null; }); return _authTokenPromise; } @@ -122,6 +130,9 @@ function _installAuthFetchInterceptor() { const isOurApi = url.startsWith(API_BASE) || url.startsWith(`http://${host}:${port}/`); if (!isOurApi) return originalFetch(input, init); + // The dev-token bootstrap must never re-enter the auth path: refreshAuthToken's fetch arrives here BEFORE _authTokenPromise is assigned (an async fn suspends only at its first await), so calling ensureAuthToken from this frame would start another refresh and recurse synchronously until RangeError. Raw transport, no bearer, no dedupe/cache; `await` so a rejection lands in the catch below (port self-heal, then the honest rethrow) instead of escaping the try. + if (url.endsWith('/api/dev/token')) return await originalFetch(input, init); + const existingHeaders = new Headers(init?.headers ?? (input instanceof Request ? input.headers : undefined)); const callerSetAuth = existingHeaders.has('Authorization') || existingHeaders.has('authorization'); @@ -196,6 +207,8 @@ function _installAuthFetchInterceptor() { lastErr = err; // A caller-driven abort is a real answer, never something to retry through. if (finalInit?.signal?.aborted) throw err; + // A transport failure may mean the renderer is pinned to a stale port: consult the live port NOW (one-shot, reloads only if it differs) instead of retrying against a dead port for seconds first. + _maybeHealBackendPort(); if (attempt < GET_RETRY_DELAYS_MS.length) { await new Promise((r) => setTimeout(r, GET_RETRY_DELAYS_MS[attempt])); continue; @@ -224,7 +237,9 @@ function _installAuthFetchInterceptor() { _inflightFetches.delete(cacheKey); } } catch (err) { - // Interceptor plumbing must never turn a workable request into a failure; fall through raw. + // A network failure reaching our backend may mean we're on a stale port; the heal is one-shot and a no-op on the same port. + if (err instanceof TypeError) _maybeHealBackendPort(); + // Interceptor plumbing must never turn a workable request into a failure; fall through raw. Real transport/abort/timeout answers stay honest (the retry budget above already ran for GETs). if (err instanceof TypeError || (err as Error)?.name === 'AbortError' || (err as Error)?.name === 'TimeoutError') throw err; return originalFetch(input, init); }