Skip to content

U7 — Client surface (victor/ui/, victor/integrations/, vscode-victor, web/)

200 modules / 74,090 LOC (Python) + 23,226 LOC TS / 2,211 churn commits (12m). Reviewer: parallel unit review, 2026-09-03.

Overview

Five live frontends (REPL, Textual TUI, Chainlit, FastAPI/WS/SSE, MCP) over one VictorClient seam — the TUI/Chainlit path uses it cleanly (stream() + shared map_event), but the CLI REPL, /completions, MCP server, and several routes bypass it. The streaming renderer is genuinely co-designed (incremental HEAD/TAIL, throttling, bounded buffers); the weak points are the API auth seam (3 call sites across 19 route files), an unshaped API chat contract (no session, history dropped), and dead parallel command systems. Churn concentrates in commands/chat.py (112 commits, 3,873 LOC) which is both the hotspot and the biggest bypass.

Findings

ID Sev Location Finding Why it matters Fix sketch Effort
F1 P1 victor/integrations/api/fastapi_server.py:865-882 /ws chat branch runs full agent turns (tools included) without checking ws.state.authenticated; the auth message sets the flag but nothing enforces it Unauthenticated WebSocket client gets full agent execution even when API keys are configured Gate the chat branch on ws.state.authenticated; add a regression test S
F2 P1 victor/integrations/api/routes/terminal_routes.py:97-146 /terminal/execute executes arbitrary shell with zero auth; danger blocklist is client-bypassable; approval is caller-attested (if is_dangerous and request.require_approval) Remote code execution on the host from any client that can reach the port Route through _verify_api_key; server-side deny-by-default approval; never trust require_approval=false from the wire S
F3 P1 victor/integrations/api/routes/* (only chat_routes.py:54,90,131 call _verify_api_key) Auth is opt-in per handler: 1 of 19 route files authenticates; git, config, agents, terminal, WS are open even with keys configured The FEP-0020 auth seam exists but only protects /chat* Convert _verify_api_key to a FastAPI dependency on the root router + WS accept handshake; delete per-handler copies M
F4 P2 tests/unit/framework/test_architectural_boundaries.py:44-49 vs integrations/protocol/adapters.py:63, mcp/server.py:28, 56 victor.agent.* imports in ui / 20 in integrations Guard fixture covers only victor/ui, victor/commands, victor/observability/dashboard; integrations unguarded and AgentOrchestrator imported directly; the documented rule is enforced only for the orchestrator symbol The client-layer boundary is documented but half-enforced; 56 UI-site violations today Add integrations dirs to the fixture; extend the AST check to victor.agent. imports with an explicit allowlist or route through VictorClient M
F5 P2 chat_routes.py:62,143; fastapi_server.py:871-882, 725-734 API accepts an OpenAI-style messages array but only messages[-1].content is used; ChatRequest has no session_id; the server holds ONE cached VictorClient for the process Two concurrent API/vscode clients silently share a single conversation; client-supplied history dropped — the wire contract lies Add session_id + per-session client pool (VictorClient already has create_session/resume_session); honor or document last-message-only M
F6 P2 victor/ui/commands/chat.py:3473-3489, 2222-2256 vs victor/framework/client.py:789-840 REPL turns bypass VictorClient (agent.chat / stream_response(agent)); session resume re-implemented inline, and unlike VictorClient.resume_session it does not hydrate the ContextService buffer The flagship client is not a client of the seam; two divergent resume implementations Route REPL turns through client.stream(); replace inline resume with await client.resume_session(...) M
F7 P2 victor/integrations/api/event_bridge.py:389,583 (dead handlers at 496-497, 525-526) Both the DeliveryEngine event queue and per-client sender queues are unbounded asyncio.Queue(); the QueueFull handlers can never fire One slow SSE/WS consumer grows memory without bound on the observability path asyncio.Queue(maxsize=N) + drop-oldest policy; disconnect on persistent overflow S
F8 P2 victor/ui/slash/init.py:71 → commands/bayesian.py → framework.rl.monitoring CLI startup: import victor.ui.cli costs ~1.73s (measured, -X importtime); eager slash-package import drags the entire framework + RL stack for victor --help Every user pays 1.7s before any command runs; the purpose-built fix (chat_lazy.py) was abandoned Defer from victor.ui.slash import commands to first slash dispatch; import heavy deps inside execute() S
F9 P2 victor/ui/commands/chat.py:3199 → 2654-2708 → 2610-2639 bottom_toolbar lambda runs per prompt_toolkit render (per keystroke) and each call copies the full message list and sums all content chars O(history) work on every keystroke; typing latency degrades as the session grows — render-bound surface doing data movement per frame Cache the runtime segment per turn; recompute only O(1) counters per keystroke S
F10 P2 chat_routes.py:198-199 /completions reaches orchestrator.provider_manager.current_provider directly, bypassing VictorClient and service resolution Client layer reaching infrastructure internals; misses attribution/correlation wrappers Expose complete(prompt, ...) on VictorClient (or a CompletionService) S
F11 P3 victor/ui/commands/chat_lazy.py (205 LOC), chat_refactored.py (283), optimization.py (1,262, click-based), victor/commands/ (3 modules, 1,071) Four dead/unwired surfaces: two abandoned chat refactors, an unreferenced optimization command, and a "unified command interface" no consumer adopted 2.8k LOC of parallel implementations mislead maintenance; test_chat_refactored_compliance.py guards a module that isn't shipped Delete all four (and the compliance test), or wire optimization in if intended S
F12 P3 victor/integrations/mcp/protocol.py:158; mcp/server.py:613-621 MCP protocol version hard-pinned to 2024-11-05, no negotiation; server API takes a raw AgentOrchestrator as its execution dependency Can't negotiate with 2025-spec clients; the ctor shape forces the layering violation of F4 Negotiate version in initialize; accept VictorClient/chat callable instead of orchestrator M
F13 P3 vscode-victor/src/victorClient.ts (2,385 LOC) vs fastapi_server.py models; client.py:559-609 vscode hand-duplicates every pydantic wire model in TS with no OpenAPI codegen; the wire carries legacy _RenderChunk renderer metadata rather than the typed event stream TUI/Chainlit consume Wire contract shaped by the Rich CLI renderer's needs; every model change needs 2-file manual sync; web/server re-maps again Generate TS types from the FastAPI schema; move API SSE to the typed stream() event set M
F14 P3 victor/ui/commands/chat.py:714-1522 (~800-line chat()), 3,873 LOC, churn 112; commands/benchmark.py 3,624 LOC God-function CLI entry and benchmark domain logic in the UI layer; top churn×size risk Every flag addition touches the same 800-line function; benchmark engine untestable from the client layer Split chat() into flag-group parsers + surface runners; move benchmark engine behind a framework/vertical service L

Co-design opportunities

  1. One event wire contract: extend the typed RenderAction set (already shared by TUI+Chainlit via map_event) to API SSE/WS and MCP; retire _RenderChunk metadata dicts from anything crossing a process boundary.
  2. Session-aware client seam: session_id on VictorClient.chat/stream + bounded client pool — vscode, web/server, and /hitl converge on resume_session instead of three ad-hoc hydrations.
  3. Auth as infrastructure, not convention: router-level dependency + WS accept-time auth co-designed with the FEP-0020 attribution join so client_id flows to every endpoint.
  4. Status segment shared by both renderers: _status_widget mirrors _build_cli_runtime_segment (live_renderer.py:99-103) — extract one per-turn-cached provider.
  5. Startup budget as a gate: CI check that import victor.ui.cli stays under an importtime threshold — the client's roofline is process startup and nothing measures it.
  6. Contract-first vscode: emit OpenAPI at build time and generate victorClient.ts models; drift becomes mechanical.

Keep intact

  • LiveDisplayRenderer incremental HEAD/TAIL render with content-keyed cache, 50k/10k buffer caps, nested pause depth counting, 10fps tool-progress throttle (live_renderer.py:63-65, 133-174, 653-678) — correct render-bound co-design.
  • EventBridge/DeliveryEngine loop-rebind hygiene (stop() dropping loop-bound queues/tasks, start() backfilling — event_bridge.py:416-476) — load-bearing for test isolation; any queue-bounding change must preserve it.
  • _resolve_stream_factory duck-typing (handler.py:26-44) — the compatibility seam letting oneshot pass the client while the REPL passes the agent; remove only together with F6.
  • The slash registry/protocol pair (fan_in=24 each) — protocol-driven, alias/category-aware, stable; the eager-import issue is in slash/__init__.py, not the registry design.
  • FastAPI secure CORS defaults and lifespan-managed bridge shutdown (fastapi_server.py:515-537, 571-602).

Quick metrics

  • Auth coverage: _verify_api_key called in 3 places, all in chat_routes.py; 0 in the other 18 route files; WS chat branch never reads ws.state.authenticated.
  • Startup: python -X importtime -c "import victor.ui.cli" = 1.73s cumulative; 731ms under victor.ui.commands.benchmark alone (via slash/init.py:71).
  • Layering: 56 victor.agent.* import sites in victor/ui (16 files), 20 in victor/integrations (9 files); guard scans 3 directories, misses integrations entirely.
  • Dead code: 2,821 LOC (chat_lazy 205 + chat_refactored 283 + optimization 1,262 + victor/commands 1,071) referenced only by one unit test.
  • Queue bounds: 0 bounded queues in event_bridge.py (2 unbounded; 2 unreachable QueueFull handlers).
  • Wire contract: ChatRequest has 1 field; 0 session fields; 1 cached VictorClient per process.