Files
waggle-os/docs/ux-refactor/p1b-residuals.md
Oleg Maslov 0c3e2ead3b
Some checks failed
Installer Smoke / installer-smoke (push) Has been cancelled
moving
2026-09-02 10:10:29 +02:00

9.1 KiB
Raw Permalink Blame History

P1b Auth Gate — Review Residuals (2026-06-11)

Source: adversarial review workflow over fa3797c..HEAD (5 lenses, 23 agents; 17 confirmed → ALL fixed in 54ec629; 0 refuted; 26 LOWs triaged below). Plan-review record: p1b-plan-review-record.md.

LOW findings fixed alongside (in 54ec629)

  • getJobStatus 404 console-spam from GroupDetail's 1.5s poll → 404s no longer logged
  • ComplianceDashboard art19/art26 sibling derefs unguarded → ?. guards
  • useChat catch updater returned a clone (forfeited setState bail-out) → returns prev
  • ServiceProvider retry-timer unmount race → mounted ref
  • Watchdog TimeoutError message read "Request to connect() timed out" → carries baseUrl
  • reconnect() silent no-op after success memo → forceReconnect()
  • launchTool/killTool envelope mapping dead → fetchRaw (with the HIGH manageHooks fix)

LOW residuals — ledgered, not fixed (P7 / by-design)

  • ~30 dead if (!res.ok) blocks remain in adapter.ts (installSkill, installPack, testSkill, getStarterPacks, getCapabilityPacks, getMarketplacePacks, runAgent, spawnAgent, cron family, wiki/compliance exporters, …) — unreachable post-chokepoint, behavior unchanged (AdapterHttpError is field-compatible incl. runAgent's C23 {status,body} consumer). Removing them is a large mechanical diff; P7 dead-code commit.
  • fetchRaw has one residual throw path: a 401 whose token refresh fails (loud by design) propagates out of fetchRaw. Every envelope consumer has a catch fallback (verified at review). Acceptable: the alternative (swallowing refresh failures on the raw path) hides the true cause.
  • Adapter-gate re-arm successes don't dispatch waggle:connect-settled — after ServiceProvider's 3 retries exhaust, a late sidecar recovery propagates to errored surfaces via the next request's re-arm + focus/online events, not the bus. Matches the plan's design; if desktop telemetry shows stuck-errored-until-focus sessions, dispatch the bus event from the adapter's re-arm success (needs a window-reference guard in the adapter).
  • Retry-loop connecting toggles re-run the 8 gated mount-loads per cycle (~3 doomed loads over ~13s while the sidecar is down; error-panel→skeleton flicker). Bounded by the retry cap; cosmetic under a down sidecar.
  • HomeCockpit Retry click during a retry-in-flight window can render a stuck skeleton until the next settle (its cancelled ref interplay with the now-toggling connecting). Self-heals on settle; P7 with the dock-era residuals.
  • ServiceProvider can broadcast connected:true from an epoch-stale connect after setServerUrl mid-flight (adapter state stays correct; provider state diverges until next reconnect). Zero production setServerUrl callers today; the Settings server-URL flow should call forceReconnect() when built.
  • "Zero network at module import" pin is partially vacuous (spy installed in beforeEach, after module eval). Constructor-time is genuinely pinned; import-time is structurally enforced by boot-connect.ts being the only module-scope caller. A vi.resetModules-based import-time pin is possible if this ever regresses.
  • subscribeHarvestProgress silently degrades under 401 (import works, no progress UI) — part of the §1.4 SSE follow-up.
  • connectWebSocket dead code (builds ?token=null pre-token) — P7 removal candidate (also §1.4's candidate auth pattern).
  • Stale settings-tier-filter.ts:8-9 comment + orphaned onboarding-tier-filter.ts — P7 cleanup.
  • CapabilityRequestCard.tsx:55 any-403-as-tier (pre-existing) — P7.

Live-smoke scope notes

Acceptance checks 13 (boot 401s / restart recovery / cold-sidecar recovery) are fetch-layer only — the five EventSource SSE channels are 401-dead in default config pre-existing since D1 (see §1.4 of the plan + decision-log note; founder ruling pending). Chat streaming (POST /api/chat) IS covered — it authenticates via headers.

Live-smoke RESULTS (2026-06-11, vite:8080 + branch sidecar:3501 via SIDECAR_TARGET; browser base = localhost:8080)

  • Check 1 — boot burst fully gated, ZERO fetch-layer 401s. Warm-boot network log: /health/api/auth/session-token FIRST, then 20+ API requests all 200 (workspaces, tier, permissions, briefing, identity, memory search/stats, per-workspace contexts). Pre-P1b this was a token-less 401 burst.
  • Check 2 — stale-token recovery without reload. Live sequence captured: GET /api/memory/stats → 401 (stale token) → GET /api/auth/session-token → 200 (silent single-flight refresh) → retried GET /api/memory/stats → 200. Same origin, no page reload, no user action.
  • No crashes under server failure. Sidecar killed mid-session: surfaces showed errors (vite proxy 500s), useAgentStatus logged + kept backing off, NO error-boundary hits, NO undefined-deref crashes (the HomeCockpit/ComplianceDashboard crash classes held). Healthy-path LoginBriefing rendered full real data (brag line, highlights, summaries).
  • §1.4 SSE defect live-confirmed: /api/waggle/stream + /api/notifications/stream EventSource 401s in console — exactly the ledgered pre-existing class.
  • Dev-env note (not a P1b defect): with TWO sidecars running (branch:3501-via-proxy + long-running pre-refactor:3333), a kill-window health probe triggers the pre-existing FR#10 auto-discovery fallback to DEFAULT_SERVER (3333) and persists it — the smoke's second restart attempt hopped servers this way (then 404'd on post-refactor routes the old sidecar lacks). In production DEFAULT_SERVER == the configured URL, so the fallback is inert and restart recovery is purely the 401-refresh leg (demonstrated). Recipe: re-set localStorage['waggle:server-url'] after any kill-window.
  • Check 3 (cold-sidecar boot → errored surfaces → recovery on connect-settled/focus) is pinned by unit tests (p1b-authgate-surfaces.test.tsx recovery cases ×4 + ServiceProvider backoff); the dual-sidecar fallback magnet makes a clean live repro impractical in this dev env.

P1b-SSE follow-up (founder-ratified 2026-06-11) — review residuals

Adversarial review (3 lenses, 8 agents): 5 confirmed (3 HIGH — all fixed: fan-out subscriptions, named-event listeners for audit/signal, handshake filter), 0 refuted, 17 LOWs. Ledgered LOWs:

  • Flapping server = constant 1s retry (backoff resets in onopen; an open-then-drop loop never escalates) + each round fires a token refresh; two streams on /api/notifications/stream could brush the 100/min default rate limit in a pathological crash-loop. Acceptable for loopback; revisit if telemetry shows 429s.
  • No event replay on any channel — events emitted during a backoff window are dropped (no id:/Last-Event-ID anywhere). Notifications could close the gap for free by refetching history on reopen — P7 candidate.
  • /api/harvest/progress is the only SSE route with no heartbeat and an unguarded write in its listener — P7 server-side polish.
  • setServerUrl doesn't tear down live streams (old-server events keep arriving until natural error; self-heals on next reopen). Zero production setServerUrl callers; fold into the future Settings server-URL flow.
  • Socket budget: realistic ceiling 4 concurrent SSE sockets vs Chromium's 6/host HTTP/1.1 cap (2 fetch slots remain). Multiplexing events/waggle onto the notifications socket's named-event channel would collapse 3 sockets to 1 — P7.
  • Auth 401s bypass the rate limiter + non-constant-time token compare — pre-existing properties of the header path inherited by the query transport; 256-bit CSPRNG token makes both moot on loopback.
  • ?token= (empty) reports MISSING_TOKEN instead of INVALID_TOKEN — cosmetic.
  • Stale _connected-gate comments in room-parallel-agents.spec.ts mock + backend-map docs — P7 doc sweep.

Security lens verdict: query-token transport sound — GET-only, allowlist-only, header-absent-only; no leakage sink beyond the existing /ws posture (no server URL logging; EventSource URLs don't enter history/cache/referrer).

P1b-SSE live-smoke RESULTS (2026-06-11)

  • Server auth contract: GET /api/notifications/stream?token=<valid> → 200 (stream holds); without token → 401. Verified via curl against the branch sidecar.
  • Browser streams LIVE: both always-mounted streams (/api/waggle/stream, /api/notifications/stream) connect 200 with the token attached — the exact requests that were 401 in the P1b smoke the day before.
  • Reconnect loop proven live: sidecar killed mid-session → both streams errored → the loop reopened them with a FRESHLY REFRESHED token on capped backoff (console shows retries carrying a new token, not the stale one). After re-pointing at the proxy (dev-env step), streams reconnected 200 with the restarted process's rotated token — error→refresh→reopen full cycle observed.
  • Dev-env artifact (same as the P1b smoke, ledgered): during the kill window the health-probe fallback hops baseUrl to DEFAULT_SERVER (3333 = the long-running PRE-refactor sidecar without the new middleware), so backoff retries 401 against the wrong server until re-point/reload. Production has one server; the fallback is inert there.