Skip to content

dashboards: one lock around every dashboard file read-modify-write, a half-failed duplicate rolls its copied sessions back, and the routes reach sibling apps through an injected boundary - #158

Draft
kai-openswarm wants to merge 6 commits into
openswarm-ai:devfrom
kai-openswarm:c10/dashboards

Conversation

@kai-openswarm

Copy link
Copy Markdown

Draft, stacked on #147 (the extra commits in the diff disappear once #147 merges). Backend hardening in backend/apps/dashboards; no product behaviour changes. Independent of the other C10 PRs.

Change Why
One lock per dashboard file p_dashboard_lifecycle_lock (an RLock) wraps save/load/delete/update and the duplicate's final write. Layout saves, renames, thumbnail writes, delete and duplicate arrive concurrently from the renderer (and from the threads FastAPI runs sync routes on); two writers interleaving on the same file lost one of them.
A half-failed duplicate rolls back duplicate copies the source dashboard's sessions first and writes the new dashboard file last; if that write fails, p_rollback_duplicated_sessions deletes the copies (and purges memory + the session file for one that refuses to delete), then re-raises. The copies were left as orphans owned by a dashboard that does not exist.
Injected runtime boundary dashboard_runtime.py: the routes' calls into the agents app (live sessions, session store, delete/duplicate/purge), the analytics client and the aux-naming primitives go through three small Protocols with default adapters (SessionAuthority, DashboardTelemetry, AuxNaming) instead of function-local imports. Behaviour is identical (the adapters look the siblings up dynamically, so existing test seams keep working); it is what makes the routes testable in isolation — the tests patch the ports and assert the sibling apps are never touched.

Tests: test_dashboard_runtime_port.py (default adapters and fakes conform to the protocols; create / delete / duplicate / generate-name / pruning consult only the injected ports; both rollback paths; a guard that no function-local sibling import can return), test_dashboards_lifecycle_characterization.py (telemetry emission and failure swallowing, owned-session removal on delete, session copy/remap on duplicate) and test_dashboards_naming_characterization.py (draft cards always survive pruning, pruning never touches disk, naming falls back to the first four prompt words on aux failure, a streamed label is cleaned and persisted).

Proof: full backend suite on 3.14 with this tree — 2974 passed / 15 skipped; upstream's linter identical to baseline.

… 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.
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.
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.
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.
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.
… duplicate that fails half-way rolls its copied sessions back, and the routes reach sibling apps through an injected boundary

Three changes to backend/apps/dashboards, no product behaviour changes:

- p_dashboard_lifecycle_lock (an RLock) wraps save/load/delete/update and the
  duplicate's final write. Layout saves, renames, thumbnail writes, delete and
  duplicate arrive concurrently from the renderer (and from threads FastAPI runs
  sync routes on); two writers interleaving on the same file lost one of them.
- duplicate copies the source dashboard's sessions first and writes the new
  dashboard file last; if that write fails, the copied sessions were left as
  orphans owned by a dashboard that does not exist. p_rollback_duplicated_sessions
  deletes them (and purges memory + the session file for one that refuses to
  delete), then re-raises.
- dashboard_runtime.py: the routes' calls into the agents app (live sessions,
  session store, delete/duplicate/purge), the analytics client and the aux-naming
  primitives go through three small Protocols with default adapters
  (SessionAuthority, DashboardTelemetry, AuxNaming) instead of function-local
  imports. Behaviour is identical (the adapters look the siblings up dynamically,
  so existing test seams keep working); it is what makes the routes testable in
  isolation — the tests below patch the ports and assert the sibling apps are
  never touched.

Tests: the runtime-port contract (default adapters and fakes conform; create /
delete / duplicate / generate-name / pruning consult only the injected ports;
the two rollback paths; no function-local sibling imports can return), and the
lifecycle + naming characterization suites (telemetry emission and failure
swallowing, owned-session removal on delete, session copy/remap on duplicate,
orphan-card pruning never touches disk, naming fallbacks). Full backend suite on
3.14: 2974 passed / 15 skipped.
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.

1 participant