Skip to content

feat(canvas): APP-015 terminal agent integration - #114

Merged
AruNi-01 merged 7 commits into
mainfrom
aarynlu/app-015-canvas-terminal-agent-86ea
May 16, 2026
Merged

AruNi-01 merged 7 commits into
mainfrom
aarynlu/app-015-canvas-terminal-agent-86ea

Conversation

@AruNi-01

@AruNi-01 AruNi-01 commented May 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

Implements APP-015: Canvas Terminal Agent Integration end-to-end, letting terminal-resident code agents (Codex, Claude Code, Gemini CLI, custom CLIs) drive the live Atmos Canvas through atmos canvas + a bundled system skill, without configuring any new Atmos LLM API key.

Spec: specs/APP/APP-015_canvas-terminal-agent-integration/.

Backend

  • crates/infra — adds canvas_bridge_register, canvas_bridge_unregister, and canvas_agent_dispatch_result WsAction variants plus the canvas_agent_dispatch server notification (see crates/infra/src/websocket/message.rs).
  • crates/core-service — new CanvasAgentRelay (service/canvas_agent_relay.rs) owns the in-memory bridge registry + pending HTTP waiters; WsMessageService wires the new bridge actions and tears down state on disconnect.
  • crates/infra/src/utils/system_skill_sync.rs + skills/system-skills-manifest.json register the new system skill so installs pick it up.
  • apps/api — POST /api/canvas/agent/invoke (HTTP ingress) and GET /api/canvas/agent/status (apps/api/src/api/canvas/agent.rs); both gated by the existing local-token middleware. The invoke handler resolves the target tab, parks a oneshot waiter, dispatches over WS, and returns the browser's structured result (200 / 400 / 403 / 404 / 409 / 413 / 503 / 504).

Frontend (apps/web)

  • canvas-agent-bus.ts — translates dispatch envelopes into tldraw editor operations: status, get_state, create_note / _frame / _geo / _arrow / _draw, select, clear_selection, move, delete (gated by confirm: true), layout_row / _column / _grid (24×24, 256-id caps), update_shape (allow-listed patch keys), viewport. Surfaces stable BRIDGE_DISABLED / STALE_SHAPE_ID / VALIDATION_ARG / EDITOR_NOT_READY / UNSUPPORTED_COMMAND / INTERNAL_ERROR codes.
  • canvas-agent-presence.ts — virtual Agent presence store driving M20 Follow Agent.
  • use-canvas-agent-bridge.ts + CanvasAgentOverlay.tsx — registers the tab over WS, listens for dispatches, publishes results, and renders the bridge toggle, Agent activity chip, follow controls, and "Copy agent instructions" affordance (M19).
  • CanvasView.tsx — mounts the overlay inside <Tldraw> and exposes the bridge controls in the share panel.

CLI (apps/cli)

  • atmos canvas command tree (commands/canvas.rs): skill-dir / skill-path (offline, local-only), status, get-state, plus every mutation verb from TECH §10 with the spec'd flags (--client-id, --actor-id, --actor-name, --actor-color, --timeout-ms). Re-uses the existing local-runtime auth token resolution.

Skill

  • skills/atmos-canvas-agent/SKILL.md — prerequisites, full verb table, coordinate conventions, destructive-command policy, error-code reference, and the canonical copy-prompt template.

Related Issue

Implements specs/APP/APP-015_canvas-terminal-agent-integration/{PRD,TECH,TEST}.md.

Type of Change

  • New feature

Validation

  • cargo check -p infra -p core-service -p api -p atmos — clean (no errors).
  • cargo test -p core-service --lib canvas_agent_relay — 12 / 12 pass.
  • bun test src/components/canvas/__tests__/canvas-agent-bus.test.ts src/components/canvas/__tests__/canvas-agent-presence.test.ts — 17 / 17 pass.
  • cargo run -p atmos -- canvas --help — every spec'd verb listed.
  • bun lint — no new errors in canvas-agent files (pre-existing repo-wide errors in Workspace.tsx / FileBrowser.tsx / proxy.ts remain untouched).

Checklist

  • Spec docs (PRD/TECH/TEST) drive the implementation.
  • Added unit tests for both backend relay and frontend bus / presence.
  • Followed Atmos's WebSocket-first transport conventions: CLI uses authenticated HTTP ingress, browser keeps the live /ws channel, no new public REST surface.
  • No TODOs left in the new code.
Open in Web Open in Cursor 

Summary by cubic

Implements APP-015: lets terminal/CLI agents control the live Atmos Canvas via atmos canvas without new API keys. Adds an HTTP-to-WebSocket relay, a TLInstancePresence-based Follow Agent, and a lightweight overlay.

  • New Features

    • Backend (crates/core-service, crates/infra, apps/api): CanvasAgentRelay; WS actions canvas_bridge_register, canvas_bridge_unregister, canvas_agent_dispatch_result; WS event canvas_agent_dispatch; HTTP POST /api/canvas/agent/invoke, GET /api/canvas/agent/status (uses existing local auth).
    • Frontend (apps/web): command bus for Canvas edits; TLInstancePresence agent presence with startFollowingUser/zoomToUser; overlay shows live agent state and follow controls; bridge hook registers the tab and wires editor → presence.
    • CLI (apps/cli): atmos canvas subcommands (status, get-state, create/layout/update/delete verbs) with existing local auth.
    • Skill: adds atmos-canvas-agent to system skills with docs for verbs, persistence, and Follow Agent.
    • Specs: adds APP-016 Atmos Computer (Cloudflare Relay + DO) docs and registers them in specs/README.md.
  • Bug Fixes

    • Relay: dedupe registrations by client_id; reject duplicate request_ids; require the same tab to complete a dispatch; distinct CANVAS_CLIENT_NOT_FOUND error.
    • CLI: robust invoke response handling; UTF‑8‑safe non‑JSON preview; richer --help across root and subcommands; copy aligned with overlay and skill.
    • Bus/validation: get_state only for current page; delete requires confirm === true; update_shape validates numeric x/y; viewport with center_ids now zooms to the union bounds.
    • Presence/overlay: cache snapshot for useSyncExternalStore; use pageToViewport for correct badge position; theme‑friendly text color and a11y labels; default agent label aligned with UI copy; skip WS unregister if already disconnected.

Written for commit 11ed025. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Canvas terminal-agent integration enabling remote control via CLI and WebSocket bridge
    • New atmos canvas CLI commands for creating/editing canvas shapes and managing viewport
    • Added agent presence overlay showing active agents controlling the canvas with follow/jump functionality
    • Canvas agent bridge controls to enable/disable CLI command acceptance
    • Added atmos-canvas-agent system skill documentation
  • Documentation

    • Added Canvas agent skill guide with command reference and best practices
    • Added APP-016 (Atmos Computer) architecture specs with Relay/Durable Objects design

Review Change Stack

Adds CLI-driven Canvas control via the existing browser WebSocket,
without requiring a separate Atmos LLM API key.

Backend
- crates/infra: new `canvas_bridge_register / canvas_bridge_unregister /
  canvas_agent_dispatch_result` WsAction variants and
  `canvas_agent_dispatch` WsEvent.
- crates/core-service: `CanvasAgentRelay` in-memory bridge registry +
  pending-waiter map; `WsMessageService` routes bridge messages and
  cleans up on disconnect. Manifest + `ALL_SYSTEM_SKILL_NAMES` register
  the new `atmos-canvas-agent` system skill.
- apps/api: `POST /api/canvas/agent/invoke` HTTP ingress and
  `GET /api/canvas/agent/status`. Resolves target tab, dispatches over
  WS, parks the request, returns the browser's result. Guarded by the
  existing local-token auth middleware.

Frontend (apps/web)
- `canvas-agent-bus.ts`: command bus that translates dispatch envelopes
  into tldraw editor operations (create-note / -frame / -geo / -arrow /
  -draw, select, move, delete --confirm, layout-row/column/grid with
  24x24 cap, update-shape with allow-listed patch keys, viewport,
  get-state, status). Centralised structured error codes.
- `canvas-agent-presence.ts`: virtual Agent presence store driving the
  Follow Agent affordance.
- `use-canvas-agent-bridge.ts` + `CanvasAgentOverlay.tsx`: React hook
  that registers the tab, listens for dispatches, executes via the bus,
  publishes results, and renders the bridge toggle, copy-instructions
  button, agent activity chips, and follow controls.
- `CanvasView.tsx`: mounts the overlay inside <Tldraw> and exposes the
  bridge controls in the share panel.

CLI (apps/cli)
- `atmos canvas` command tree: `skill-dir` / `skill-path` (local-only),
  `status`, `get-state`, and all mutation verbs from TECH §10. Uses the
  existing local-runtime auth for the API call.

Skill
- `skills/atmos-canvas-agent/SKILL.md` documents prerequisites, verb
  table, coordinate conventions, destructive-command policy, error
  codes, and copy-prompt template.

Tests
- 12 unit tests for `CanvasAgentRelay` (cargo).
- 17 unit tests across `canvas-agent-bus` and `canvas-agent-presence`
  (bun).

Co-authored-by: AruNi_Lu <hello@0x3f4.run>
@vercel

vercel Bot commented May 15, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
atmos-landing Ready Ready Preview, Comment May 16, 2026 2:50am

@coderabbitai

coderabbitai Bot commented May 15, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR implements a complete bidirectional bridge allowing terminal-based agents to remotely dispatch tldraw canvas commands. It adds a shared in-memory relay state machine, HTTP/CLI ingress, WebSocket protocol extensions, browser-side command execution engine with presence tracking, and full UI integration.

Changes

Canvas Terminal-Agent Bridge System

Layer / File(s) Summary
In-memory relay state and dispatch lifecycle
crates/core-service/src/service/canvas_agent_relay.rs, crates/core-service/src/service/mod.rs, crates/core-service/src/lib.rs
Core relay infrastructure maintaining bridge client registry with TTL-based eviction, pending request tracking, target resolution logic, and dispatch outcome completion via oneshot channels. Includes comprehensive unit tests covering registration, target resolution, and pending dispatch handling.
WebSocket service relay binding and handlers
crates/core-service/src/service/ws_message.rs, crates/infra/src/websocket/message.rs, crates/infra/src/websocket/mod.rs
Integration of relay into WsMessageService with field storage, public accessor, and three action handlers (register, unregister, dispatch_result). Extends WebSocket protocol with new actions and event types for terminal-agent bridging.
HTTP API invoke and status endpoints
apps/api/src/api/canvas/agent.rs, apps/api/src/api/canvas/mod.rs
Axum HTTP handlers for /api/canvas/agent/invoke (POST) and /api/canvas/agent/status (GET) with payload validation, target resolution, relay dispatch coordination, and structured error responses. Includes helpers for consistent JSON error formatting.
Application state setup and relay dependency injection
apps/api/src/app_state.rs, apps/api/src/main.rs
Relay field additions to AppServices/AppState, relay instantiation at startup, and injection into both WsMessageService and app state during initialization.
CLI canvas command routing and HTTP invocation
apps/cli/src/commands/canvas.rs
Complete atmos canvas CLI implementation with command enum, 18 subcommand argument types, HTTP request building with URL/token resolution, and structured response parsing with graceful error formatting for non-JSON responses.
CLI module wiring and local state exports
apps/cli/src/commands/mod.rs, apps/cli/src/commands/local.rs, apps/cli/src/main.rs
Module declaration for canvas command, visibility changes to LocalRuntimeState/read_state_file for CLI cross-module access, and main command dispatch routing.
WebSocket message protocol for terminal-agent bridge
crates/infra/src/websocket/message.rs, apps/web/src/api/ws-api.ts, apps/web/src/hooks/use-websocket.ts
WsAction enum variants, WsEvent for dispatch, request/response payload structs for bridge register/unregister/dispatch_result. Includes client-side WebSocket API wrappers.
Browser-side canvas command execution engine
apps/web/src/components/canvas/canvas-agent-bus.ts
Client-side CanvasAgentBus executing 18 allow-listed canvas verbs against tldraw Editor: shape creation/mutation, selection, layout, viewport control, update_shape with patch validation, and comprehensive error handling with recoverable flags.
Canvas bus unit tests
apps/web/src/components/canvas/__tests__/canvas-agent-bus.test.ts
Comprehensive test suite validating command execution: status/get_state behavior, shape creation with aliasing, bridge acceptance gating, layout grid constraints, delete confirmation, update_shape patch validation, and error code handling.
Agent presence store with TTL eviction and editor binding
apps/web/src/components/canvas/canvas-agent-presence.ts
CanvasAgentPresenceStore managing in-memory agent metadata, TTL-based cleanup, tldraw editor integration for presence record persistence via TLInstancePresence, follow/jump controls, and snapshot subscription for UI rendering.
Agent presence store unit tests
apps/web/src/components/canvas/__tests__/canvas-agent-presence.test.ts
Tests validating presence metadata updates, user_id formatting, bounds/camera computation, editor persistence, follow/jump delegation, stale eviction, and cleanup on editor detach.
Bridge integration hook and WebSocket API wrappers
apps/web/src/components/canvas/use-canvas-agent-bridge.ts, apps/web/src/api/ws-api.ts
useCanvasAgentBridge hook coordinating bus/presence/localStorage/WebSocket, clientId generation, accepts_commands persistence, dispatch event listening, result recording and posting. Includes canvasAgentBridgeWsApi wrapper for WebSocket actions.
Canvas view, overlay, and bridge controls
apps/web/src/components/canvas/CanvasAgentOverlay.tsx, apps/web/src/components/canvas/CanvasView.tsx
CanvasAgentOverlay rendering agent presence badges with coordinate conversion and follow/jump buttons, CanvasAgentBridgeControls for toggling command acceptance and copying instructions, integration into CanvasView via hook and SharePanel.
Skill documentation and system skill registration
skills/atmos-canvas-agent/SKILL.md, skills/system-skills-manifest.json, crates/infra/src/utils/system_skill_sync.rs
Comprehensive SKILL.md defining atmos canvas command reference, prerequisites, error codes, coordinate conventions, and anti-patterns. Manifest entry and sync integration for bundled skill distribution.
APP-016 specification documents
specs/APP/APP-016_atmos-computer/, specs/README.md
BRAINSTORM, PRD, TECH, and TEST spec documents defining Atmos Computer architecture with Cloudflare Relay, including design decisions, protocols, integration expectations, and test scenarios.
Documentation improvements
apps/cli/src/commands/review.rs, apps/cli/src/commands/update.rs
Added Rustdoc comments to ReviewCommand enum variants and UpdateArgs.check field for improved CLI documentation.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • AruNi-01/atmos#77: Both PRs extend the shared WebSocket action typing (WsAction union) with new protocol actions, though for different domains (canvas agent bridge vs. workspace/kanban actions).

Suggested labels

codex

🐰✨
A bridge built with care,
Commands flow through the air,
Canvas and code unite,
Relay makes it right! 🎨

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aarynlu/app-015-canvas-terminal-agent-86ea

…tartFollowingUser

Previously the agent badge was a pure HTML overlay and 'follow' was an
ad-hoc centerOnPoint loop, which diverged from TECH.md §9 and from the
official tldraw user-following contract (https://tldraw.dev/sdk-features/
user-following). This commit makes Follow Agent honour the documented
API.

apps/web/src/components/canvas/canvas-agent-presence.ts
- Bind a live tldraw Editor via setEditor(editor).
- Every touch / recordResult writes a TLInstancePresence record into
  editor.store via InstancePresenceRecordType.create({ userId:
  'agent:<actor_id>', userName, color, currentPageId, camera,
  screenBounds, cursor, selectedShapeIds, lastActivityTimestamp,
  chatMessage, meta }).
- Synthesise camera + screenBounds so tldraw's
  getViewportPageBoundsForFollowing() frames the agent's last bounds
  (camera.z = 1, screenBounds = padded(last_bounds), camera = -origin).
  Without bounds yet, mirror the user's own viewport.
- setFollowedActor(id) now calls editor.startFollowingUser('agent:<id>')
  / stopFollowingUser(). jumpToActor(id) calls editor.zoomToUser(...).
- TTL eviction + clear() + setEditor(null) all clear the corresponding
  records from editor.store and cancel any active follow so we never
  strand the user on a ghost agent.
- Records live in the presence scope, so getSnapshot(editor.store).
  document — the payload Atmos persists server-side — is unaffected.

apps/web/src/components/canvas/CanvasAgentOverlay.tsx
- Read the followed user id via useValue(() => editor.getInstanceState
  ().followingUserId) so manual pan/zoom (tldraw stops following) is
  reflected in the UI immediately.
- Add a 'Jump to agent' (Crosshair) button alongside the follow toggle;
  follow toggle now drives editor.startFollowingUser via the presence
  store.

apps/web/src/components/canvas/use-canvas-agent-bridge.ts
- Pass editor into presence.setEditor when the editor mounts / unmounts.

skills/atmos-canvas-agent/SKILL.md
- New section on the persistence model: agent mutations flow through
  editor.store and are autosaved server-side via the existing APP-014
  pipeline. tldraw's persistenceKey IndexedDB sync is intentionally not
  used (Atmos persists per workspace, not per browser). Presence is
  ephemeral.
- New 'Follow Agent (M20)' section telling agents to pass --actor-id so
  the user gets a stable follow target across many commands.

Tests
- canvas-agent-presence.test.ts: 10 tests (was 4) covering
  TLInstancePresence write, follow / stop following / zoomToUser
  delegation, unknown actor rejection, TTL eviction with editor cleanup,
  and setEditor(null) teardown. All 33 web tests + 12 relay tests pass.

Co-authored-by: AruNi_Lu <hello@0x3f4.run>
@AruNi-01
AruNi-01 marked this pull request as ready for review May 15, 2026 13:56

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/core-service/src/service/ws_message.rs (1)

57-60: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Run rustfmt for this file before merge.

CI is failing cargo fmt --check on these ranges.

Also applies to: 188-192, 1486-1491

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/core-service/src/service/ws_message.rs` around lines 57 - 60, The file
fails cargo fmt checks; run rustfmt (cargo fmt) on
crates/core-service/src/service/ws_message.rs and reformat the listed
enum/variant blocks such as UsageProviderFooterCarouselRequest,
UsageProviderManualSetupRequest, UsageProviderSwitchRequest,
WorkspaceArchiveRequest, WorkspaceConfirmTodosRequest, WorkspaceCreateRequest,
WorkspaceDeleteProgressNotification, WorkspaceDeleteRequest,
WorkspaceGitignoreSyncFailedNotification and the other misformatted variant
groups (the blocks around those symbols) so the commas/line breaks match rustfmt
style; commit the updated file so cargo fmt --check passes.
🧹 Nitpick comments (1)
apps/web/src/components/canvas/__tests__/canvas-agent-bus.test.ts (1)

32-39: ⚡ Quick win

Make the fake editor observable enough to cover viewport.

setCamera, zoomToBounds, and zoomToFit are no-ops, so this suite can't assert camera changes or catch regressions in the viewport path. Persist the camera state / last zoom request in the stub and add a command-level test for viewport.

Also applies to: 97-99, 118-241

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/canvas/__tests__/canvas-agent-bus.test.ts` around
lines 32 - 39, The fake editor used in the tests currently returns static values
from getCamera and getViewportPageBounds and leaves setCamera, zoomToBounds, and
zoomToFit as no-ops, so tests cannot observe camera/viewport changes; update the
stub to persist camera state and last zoom request (e.g., maintain an internal
camera object updated by setCamera and modified by zoomToBounds/zoomToFit) and
have getCamera/getViewportPageBounds reflect that state (and expose the last
zoom parameters) so you can add a command-level assertion for viewport changes;
modify the test stubs referenced around getCamera/getViewportPageBounds,
setCamera, zoomToBounds, zoomToFit and add a new test that triggers the viewport
command and asserts the updated camera/viewport state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/api/src/api/canvas/agent.rs`:
- Around line 172-181: ResolveTarget::NotFound is returning
CanvasAgentInvokeError with code "CANVAS_BRIDGE_OFFLINE", which conflates
missing client_id with a bridge offline error; update the error construction in
the ResolveTarget::NotFound branch (the call to error_resp with
CanvasAgentInvokeError::new) to use a distinct code such as
"CANVAS_CLIENT_NOT_FOUND" and adjust the message if needed so callers can
differentiate this case from an offline bridge.
- Around line 88-285: The invoke HTTP handler is doing too much orchestration
(validation aside) — move the dispatch/pending/timeout/WS-send logic into
core-service so the handler only extracts DTOs and calls a core API; create a
new core-service method (e.g., CanvasAgentRelay::dispatch_request or similar)
that accepts a DTO converted via From implementations (map
CanvasAgentInvokePayload -> core DispatchRequest), uses clamp_timeout,
begin_pending/cancel_pending and
manager.send_to/WsMessage::notification(WsEvent::CanvasAgentDispatch)
internally, waits for the response with tokio::time::timeout and returns a
unified result type (success data or error info) the handler can translate into
CanvasAgentInvokeResponse or CanvasAgentInvokeError; update invoke to perform
validation and target resolution only then call the new core method and convert
its return value to the HTTP response.
- Around line 102-119: Run rustfmt on this file to satisfy cargo fmt --check;
reformat the code containing the validation branches that call error_resp and
construct CanvasAgentInvokeError (the blocks referencing request_id, command,
error_resp, and CanvasAgentInvokeError) and also the other unformatted range
around the later use of these symbols (the code that includes request_id/command
validation further down). Ensure the file compiles after formatting and commit
the formatted changes.

In `@apps/api/src/api/canvas/mod.rs`:
- Around line 17-18: The new REST routes .route("/agent/invoke",
post(agent::invoke)) and .route("/agent/status", get(agent::status)) introduce
duplicate transport for interactive canvas dispatch; remove these route
registrations from the router (delete the lines registering agent::invoke and
agent::status) and keep the interactive dispatch logic on the existing WebSocket
channel/handlers; if agent::invoke or agent::status handlers become unused,
either remove or mark them as WebSocket-only (refactor their logic into the WS
handlers) and update any CLI or client code to call the WebSocket protocol
instead of these REST endpoints.

In `@apps/cli/src/commands/canvas.rs`:
- Around line 446-454: The CI is failing rustfmt checks: reformat the long error
string in CanvasCommand::body (the delete branch in fn body()) and the
ATMOS_API_TOKEN environment-variable chaining expressions so they match rustfmt
style; run cargo fmt --all (or rustfmt) to autoformat the repo and commit the
updated formatting changes ensuring the delete error string and the
ATMOS_API_TOKEN chain are split/wrapped as rustfmt produces (then push the
formatted commit).

In `@apps/web/src/components/canvas/canvas-agent-bus.ts`:
- Around line 573-583: The runViewport branch for center_ids currently only
validates ids then zooms to selection/viewport and calls zoomToFit, so the
requested center_ids are ignored; change runViewport to compute the page bounds
for the shapes identified by center_ids (retrieve shapes by id via the editor
API or map ids to shapes and union their page bounds), call
requireExistingShapes(editor, ids) as before, then pass those computed bounds
into editor.zoomToBounds(..., { targetZoom: optionalNumber(args.zoom) ??
undefined, animation: { duration: 200 } }) and remove the unconditional
editor.zoomToFit() call so the center_ids and targetZoom are honored.
- Around line 233-251: runGetState currently validates requestedPage exists but
still reads shapes/camera/viewport/selection from the current page, which can
return a mismatched payload; fix this by rejecting requests for non-current
pages until per-page read support is implemented: in runGetState, after
computing requestedPage and pageId, if requestedPage is provided and pageId !==
editor.getCurrentPageId(), throw a CanvasAgentError (similar to the existing
one) indicating non-current pages are not supported; this keeps the subsequent
uses of editor.getCurrentPageShapesSorted/getCurrentPageShapes,
editor.getCamera, editor.getViewportPageBounds and editor.getSelectedShapeIds
correct without silently returning data for the wrong page.
- Around line 550-559: The update_shape patch currently assigns Number(value)
for keys "x" and "y" without validating, allowing NaN/Infinity/invalid strings;
change the branch in canvas-agent-bus where UPDATE_SHAPE_ALLOWED_KEYS is checked
to first run the value through the existing finite-number validator used
elsewhere in this module (e.g., the validateFiniteNumber/isFiniteNumber helper)
and throw a CanvasAgentError("VALIDATION_ARG", ...) if it fails, otherwise
assign the validated finite number to (next as Record<string, unknown>)[key];
keep the same error type/message style and reuse the CanvasAgentError class for
consistency.

In `@apps/web/src/components/canvas/canvas-agent-presence.ts`:
- Around line 113-117: getSnapshot currently allocates and sorts a fresh array
on every call which breaks useSyncExternalStore (Object.is detects a new
reference and causes infinite re-renders); change getSnapshot to return a cached
snapshot (e.g., store a private cachedSnapshot and cachedSnapshotVersion) and
only recompute (allocate and sort a new array) when the underlying agent
collection changes, invalidating the cache inside every mutation method that
modifies this.agents (e.g., add/update/remove/clear methods), and add a unit
test that calls getSnapshot twice without mutations and asserts the returned
references are strictly equal to prevent regressions.

In `@apps/web/src/components/canvas/CanvasAgentOverlay.tsx`:
- Around line 220-223: Replace the hardcoded "text-emerald-500" tailwind class
used in the CanvasAgentOverlay component with a semantic token class (e.g., a
provided semantic text color like "text-success" or "text-accent-foreground") so
the active icon color adapts to light/dark themes; update the cn call where the
className is computed (the block that checks bridge.acceptsCommands) to use the
semantic token instead of "text-emerald-500" and ensure it falls back to the
existing "text-muted-foreground" when false.
- Around line 136-157: The two icon-only buttons for jump and follow in
CanvasAgentOverlay use only title attributes for identification; add explicit
aria-label attributes to both button elements so screen readers get stable names
(use aria-label="Jump to agent" for the button that triggers onJump and
aria-label={isFollowed ? "Stop following agent" : "Follow agent"} for the button
that triggers onToggleFollow), leaving existing title and icon components
(Crosshair, Eye, EyeOff) unchanged.

In `@apps/web/src/components/canvas/use-canvas-agent-bridge.ts`:
- Around line 124-127: The cleanup currently calls
canvasAgentBridgeWsApi.unregister(clientId) unconditionally which can trigger
wsRequest's auto-connect during teardown; change the cleanup to only call
unregister when the websocket is already open/connected (e.g., check a
connection flag or call a provided predicate such as
canvasAgentBridgeWsApi.isConnected(clientId) or
canvasAgentBridgeWsApi.wsRequest?.isConnected()/isOpen() before calling
unregister), or first disable/close the wsRequest (e.g.,
wsRequest.disableAutoConnect() or wsRequest.close()) and then call
canvasAgentBridgeWsApi.unregister(clientId) so that unregister cannot cause a
reconnect during unmount.

In `@crates/core-service/src/service/canvas_agent_relay.rs`:
- Around line 73-79: The file fails rustfmt checks; run rustfmt (e.g., cargo
fmt) and reformat this file so the enum variant doc comment alignment and
surrounding code match rustfmt expectations—specifically adjust formatting
around the enum variants Offline, Ambiguous, NotFound (and the other affected
ranges around lines shown) in canvas_agent_relay.rs so CI passes; ensure you run
cargo fmt or rustfmt on the file and commit the formatted changes.
- Around line 109-110: The register logic currently only removes entries
matching both conn_id and client_id, allowing the same client_id to exist on
multiple connections which causes resolve_target(Some(client_id)) to pick a
stale entry; update the deduplication in the register function to remove any
existing entries with the same client_id (regardless of conn_id) by changing the
bridge.retain predicate to drop entries where entry.client_id == client_id, and
apply the same fix to the other registration/cleanup block referenced around the
172-185 region so both code paths consistently deduplicate by client_id; keep
resolve_target behavior unchanged.
- Around line 220-227: The begin_pending function currently overwrites an
existing oneshot sender for the same request_id, which silently drops the
original caller; change begin_pending to detect duplicates and reject them
instead of replacing: check the pending HashMap (e.g. with pending.contains_key
or pending.entry(request_id).or_insert) and if a key exists return an error
rather than inserting the new sender; update the function signature from pub fn
begin_pending(&self, request_id: impl Into<String>) ->
oneshot::Receiver<CanvasAgentDispatchOutcome> to return a
Result<oneshot::Receiver<CanvasAgentDispatchOutcome>, DuplicateRequestError> (or
an existing error type), and only insert the sender (pending.insert) when no
existing entry is present so duplicates are explicitly rejected.

In `@crates/core-service/src/service/ws_message.rs`:
- Around line 656-658: The CanvasAgentDispatchResult path currently matches only
on request_id, allowing a different connection to complete another tab's
request; update the parsing and handler call so the parsed request includes
conn_id and pass that conn_id into handle_canvas_agent_dispatch_result (i.e.,
propagate conn_id from parse_request(request.data)? into the handler), then
change handle_canvas_agent_dispatch_result to validate that the conn_id matches
the original WS connection before marking the request complete; apply the same
change to the other dispatch-result handling site (the second occurrence that
completes relay responses) so relay completion enforces conn_id equality when
resolving by request_id.

---

Outside diff comments:
In `@crates/core-service/src/service/ws_message.rs`:
- Around line 57-60: The file fails cargo fmt checks; run rustfmt (cargo fmt) on
crates/core-service/src/service/ws_message.rs and reformat the listed
enum/variant blocks such as UsageProviderFooterCarouselRequest,
UsageProviderManualSetupRequest, UsageProviderSwitchRequest,
WorkspaceArchiveRequest, WorkspaceConfirmTodosRequest, WorkspaceCreateRequest,
WorkspaceDeleteProgressNotification, WorkspaceDeleteRequest,
WorkspaceGitignoreSyncFailedNotification and the other misformatted variant
groups (the blocks around those symbols) so the commas/line breaks match rustfmt
style; commit the updated file so cargo fmt --check passes.

---

Nitpick comments:
In `@apps/web/src/components/canvas/__tests__/canvas-agent-bus.test.ts`:
- Around line 32-39: The fake editor used in the tests currently returns static
values from getCamera and getViewportPageBounds and leaves setCamera,
zoomToBounds, and zoomToFit as no-ops, so tests cannot observe camera/viewport
changes; update the stub to persist camera state and last zoom request (e.g.,
maintain an internal camera object updated by setCamera and modified by
zoomToBounds/zoomToFit) and have getCamera/getViewportPageBounds reflect that
state (and expose the last zoom parameters) so you can add a command-level
assertion for viewport changes; modify the test stubs referenced around
getCamera/getViewportPageBounds, setCamera, zoomToBounds, zoomToFit and add a
new test that triggers the viewport command and asserts the updated
camera/viewport state.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 49b9bb56-6866-4e7b-850b-c825dae472b6

📥 Commits

Reviewing files that changed from the base of the PR and between 9067b27 and 028c9de.

📒 Files selected for processing (26)
  • apps/api/src/api/canvas/agent.rs
  • apps/api/src/api/canvas/mod.rs
  • apps/api/src/app_state.rs
  • apps/api/src/main.rs
  • apps/cli/src/commands/canvas.rs
  • apps/cli/src/commands/local.rs
  • apps/cli/src/commands/mod.rs
  • apps/cli/src/main.rs
  • apps/web/src/api/ws-api.ts
  • apps/web/src/components/canvas/CanvasAgentOverlay.tsx
  • apps/web/src/components/canvas/CanvasView.tsx
  • apps/web/src/components/canvas/__tests__/canvas-agent-bus.test.ts
  • apps/web/src/components/canvas/__tests__/canvas-agent-presence.test.ts
  • apps/web/src/components/canvas/canvas-agent-bus.ts
  • apps/web/src/components/canvas/canvas-agent-presence.ts
  • apps/web/src/components/canvas/use-canvas-agent-bridge.ts
  • apps/web/src/hooks/use-websocket.ts
  • crates/core-service/src/lib.rs
  • crates/core-service/src/service/canvas_agent_relay.rs
  • crates/core-service/src/service/mod.rs
  • crates/core-service/src/service/ws_message.rs
  • crates/infra/src/utils/system_skill_sync.rs
  • crates/infra/src/websocket/message.rs
  • crates/infra/src/websocket/mod.rs
  • skills/atmos-canvas-agent/SKILL.md
  • skills/system-skills-manifest.json

Comment on lines +88 to +285
pub async fn invoke(
State(state): State<AppState>,
Json(payload): Json<CanvasAgentInvokePayload>,
) -> (StatusCode, Json<CanvasAgentInvokeResponse>) {
let CanvasAgentInvokePayload {
request_id,
command,
args,
client_id,
timeout_ms,
actor,
} = payload;

if request_id.trim().is_empty() {
return error_resp(
"",
StatusCode::BAD_REQUEST,
CanvasAgentInvokeError::new(
"VALIDATION_ARG",
"request_id must not be empty",
false,
),
);
}

if command.trim().is_empty() {
return error_resp(
&request_id,
StatusCode::BAD_REQUEST,
CanvasAgentInvokeError::new(
"VALIDATION_ARG",
"command must not be empty",
false,
),
);
}

if serde_json::to_vec(&args)
.map(|v| v.len() > MAX_PAYLOAD_BYTES)
.unwrap_or(false)
{
return error_resp(
&request_id,
StatusCode::PAYLOAD_TOO_LARGE,
CanvasAgentInvokeError::new(
"VALIDATION_ARG",
format!("args exceeds {} bytes", MAX_PAYLOAD_BYTES),
false,
),
);
}

let relay = state.canvas_agent_relay.clone();
let target = relay.resolve_target(client_id.as_deref());

let (conn_id, resolved_client_id) = match target {
ResolveTarget::Single { conn_id, client_id } => (conn_id, client_id),
ResolveTarget::Offline => {
return error_resp(
&request_id,
StatusCode::SERVICE_UNAVAILABLE,
CanvasAgentInvokeError::new(
"CANVAS_BRIDGE_OFFLINE",
"No Canvas tab is currently registered. Open Atmos Canvas in the browser.",
true,
),
);
}
ResolveTarget::Ambiguous { clients } => {
let ids: Vec<String> = clients.iter().map(|c| c.client_id.clone()).collect();
return error_resp_with_extra(
&request_id,
StatusCode::CONFLICT,
CanvasAgentInvokeError::new(
"CANVAS_CLIENT_AMBIGUOUS",
format!(
"Multiple Canvas tabs registered ({}). Re-run with --client-id <id>.",
ids.join(", ")
),
true,
),
Some(json!({ "candidates": clients })),
);
}
ResolveTarget::NotFound => {
return error_resp(
&request_id,
StatusCode::NOT_FOUND,
CanvasAgentInvokeError::new(
"CANVAS_BRIDGE_OFFLINE",
"No registered Canvas tab matches the supplied client_id.",
true,
),
);
}
ResolveTarget::NotAccepting { client_id } => {
return error_resp(
&request_id,
StatusCode::FORBIDDEN,
CanvasAgentInvokeError::new(
"BRIDGE_DISABLED",
format!(
"Tab {} has not enabled 'Allow terminal/CLI control'.",
client_id
),
true,
),
);
}
};

let timeout = core_service::CanvasAgentRelay::clamp_timeout(timeout_ms);
let rx = relay.begin_pending(&request_id);

let dispatch_payload = json!({
"request_id": request_id,
"client_id": resolved_client_id,
"command": command,
"args": args,
"actor": actor,
"deadline_ms": timeout.as_millis() as u64,
});

let manager = state.ws_service.manager();
let message = WsMessage::notification(WsEvent::CanvasAgentDispatch, dispatch_payload);
if let Err(err) = manager.send_to(&conn_id, &message).await {
relay.cancel_pending(&request_id);
tracing::warn!(
"canvas_agent: failed to deliver dispatch to conn {}: {}",
conn_id,
err
);
return error_resp(
&request_id,
StatusCode::SERVICE_UNAVAILABLE,
CanvasAgentInvokeError::new(
"CANVAS_BRIDGE_OFFLINE",
"Canvas tab disconnected before the command could be delivered.",
true,
),
);
}

match tokio::time::timeout(timeout, rx).await {
Ok(Ok(outcome)) => {
if outcome.success {
(
StatusCode::OK,
Json(CanvasAgentInvokeResponse {
ok: true,
request_id,
data: Some(outcome.data),
error: None,
}),
)
} else {
error_resp(
&request_id,
StatusCode::BAD_REQUEST,
CanvasAgentInvokeError::new(
outcome.error_code.as_deref().unwrap_or("UNKNOWN"),
outcome
.error_message
.unwrap_or_else(|| "Browser reported an unspecified failure".into()),
outcome.recoverable.unwrap_or(true),
),
)
}
}
Ok(Err(_)) => {
relay.cancel_pending(&request_id);
error_resp(
&request_id,
StatusCode::SERVICE_UNAVAILABLE,
CanvasAgentInvokeError::new(
"RELAY_TIMEOUT",
"Browser connection dropped before responding.",
true,
),
)
}
Err(_) => {
relay.cancel_pending(&request_id);
error_resp(
&request_id,
StatusCode::GATEWAY_TIMEOUT,
CanvasAgentInvokeError::new(
"RELAY_TIMEOUT",
format!(
"Browser did not answer within {:?}",
Duration::from_millis(timeout.as_millis() as u64)
),
true,
),
)
}
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Move dispatch orchestration out of the HTTP handler into core-service.

This handler now owns validation, target resolution, pending lifecycle, WS dispatch, and timeout mapping. It should delegate this workflow to core-service and keep API-layer DTO extraction/translation only.

As per coding guidelines, "Handlers should be thin — extract data from requests and call core-service" and "Implement From traits to convert between DTOs and Core Service types".

🧰 Tools
🪛 GitHub Actions: CI - Backend (Rust) / 3_Format Check.txt

[error] 102-107: rustfmt formatting difference in CanvasAgentInvokeError::new call (multiline -> single line).


[error] 114-119: rustfmt formatting difference in CanvasAgentInvokeError::new call (multiline -> single line).

🪛 GitHub Actions: CI - Backend (Rust) / Format Check

[error] 102-102: cargo fmt --check failed for this file (rustfmt would reformat CanvasAgentInvokeError::new(...) call). Run 'cargo fmt --all'.


[error] 114-114: cargo fmt --check failed for this file (rustfmt would reformat CanvasAgentInvokeError::new(...) call). Run 'cargo fmt --all'.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/api/canvas/agent.rs` around lines 88 - 285, The invoke HTTP
handler is doing too much orchestration (validation aside) — move the
dispatch/pending/timeout/WS-send logic into core-service so the handler only
extracts DTOs and calls a core API; create a new core-service method (e.g.,
CanvasAgentRelay::dispatch_request or similar) that accepts a DTO converted via
From implementations (map CanvasAgentInvokePayload -> core DispatchRequest),
uses clamp_timeout, begin_pending/cancel_pending and
manager.send_to/WsMessage::notification(WsEvent::CanvasAgentDispatch)
internally, waits for the response with tokio::time::timeout and returns a
unified result type (success data or error info) the handler can translate into
CanvasAgentInvokeResponse or CanvasAgentInvokeError; update invoke to perform
validation and target resolution only then call the new core method and convert
its return value to the HTTP response.

Comment thread apps/api/src/api/canvas/agent.rs
Comment thread apps/api/src/api/canvas/agent.rs
Comment on lines +17 to +18
.route("/agent/invoke", post(agent::invoke))
.route("/agent/status", get(agent::status))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avoid adding REST transport for interactive canvas dispatch.

This introduces duplicate transport (/agent/invoke, /agent/status) for a workflow already integrated into WebSocket actions/events. Please keep this capability on the WebSocket path and have CLI use that channel.

As per coding guidelines, "Use WebSocket-first by default for chat, session state, streaming updates, and interactive app behavior instead of creating new REST APIs" and "Avoid duplicate transports—do not build new REST paths for capabilities that should use WebSocket; extend the existing WebSocket protocol instead".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/api/canvas/mod.rs` around lines 17 - 18, The new REST routes
.route("/agent/invoke", post(agent::invoke)) and .route("/agent/status",
get(agent::status)) introduce duplicate transport for interactive canvas
dispatch; remove these route registrations from the router (delete the lines
registering agent::invoke and agent::status) and keep the interactive dispatch
logic on the existing WebSocket channel/handlers; if agent::invoke or
agent::status handlers become unused, either remove or mark them as
WebSocket-only (refactor their logic into the WS handlers) and update any CLI or
client code to call the WebSocket protocol instead of these REST endpoints.

Comment on lines +446 to +454
fn body(&self) -> Result<Value, String> {
if !self.confirm {
return Err(
"delete is destructive — re-run with --confirm to acknowledge.".to_string()
);
}
Ok(json!({ "ids": split_ids(&self.ids), "confirm": true }))
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix cargo fmt --check failures blocking CI.

The pipeline reports rustfmt would reformat the delete error string at lines 446-449 and the ATMOS_API_TOKEN chain at 694-697. Run cargo fmt --all and commit the result; the most likely rewrites are:

♻️ Likely rustfmt result
     fn body(&self) -> Result<Value, String> {
         if !self.confirm {
-            return Err(
-                "delete is destructive — re-run with --confirm to acknowledge.".to_string()
-            );
+            return Err("delete is destructive — re-run with --confirm to acknowledge.".to_string());
         }
         Ok(json!({ "ids": split_ids(&self.ids), "confirm": true }))
     }
 fn resolve_token(global: &GlobalArgs) -> Option<String> {
     if let Some(token) = &global.api_token {
         if !token.is_empty() {
             return Some(token.clone());
         }
     }
-    std::env::var("ATMOS_API_TOKEN").ok().filter(|v| !v.is_empty())
+    std::env::var("ATMOS_API_TOKEN")
+        .ok()
+        .filter(|v| !v.is_empty())
 }

Also applies to: 691-698

🧰 Tools
🪛 GitHub Actions: CI - Backend (Rust) / 3_Format Check.txt

[error] 446-449: rustfmt formatting difference in string .to_string() call (trailing comma placement).

🪛 GitHub Actions: CI - Backend (Rust) / Format Check

[error] 446-446: cargo fmt --check failed for this file (rustfmt would reformat a to_string() call in an error message). Run 'cargo fmt --all'.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/src/commands/canvas.rs` around lines 446 - 454, The CI is failing
rustfmt checks: reformat the long error string in CanvasCommand::body (the
delete branch in fn body()) and the ATMOS_API_TOKEN environment-variable
chaining expressions so they match rustfmt style; run cargo fmt --all (or
rustfmt) to autoformat the repo and commit the updated formatting changes
ensuring the delete error string and the ATMOS_API_TOKEN chain are split/wrapped
as rustfmt produces (then push the formatted commit).

Comment thread apps/web/src/components/canvas/use-canvas-agent-bridge.ts
Comment thread crates/core-service/src/service/canvas_agent_relay.rs
Comment thread crates/core-service/src/service/canvas_agent_relay.rs Outdated
Comment thread crates/core-service/src/service/canvas_agent_relay.rs Outdated
Comment thread crates/core-service/src/service/ws_message.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 issues found across 26 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/cli/src/commands/canvas.rs">

<violation number="1" location="apps/cli/src/commands/canvas.rs:634">
P2: `invoke` unconditionally parses JSON, which masks real server/proxy failures when error responses are non-JSON.</violation>
</file>

<file name="crates/core-service/src/service/canvas_agent_relay.rs">

<violation number="1" location="crates/core-service/src/service/canvas_agent_relay.rs:110">
P1: `client_id` is not uniquely deduplicated on register, so targeted dispatch by `client_id` can resolve to the wrong/stale bridge entry.</violation>
</file>

<file name="apps/web/src/components/canvas/CanvasAgentOverlay.tsx">

<violation number="1" location="apps/web/src/components/canvas/CanvasAgentOverlay.tsx:106">
P2: Agent badges use screen-space coordinates as container-space positions, which can render them at incorrect offsets. Convert page coordinates to container-relative coordinates before applying `left/top`.</violation>
</file>

<file name="apps/web/src/components/canvas/canvas-agent-bus.ts">

<violation number="1" location="apps/web/src/components/canvas/canvas-agent-bus.ts:245">
P2: `get_state` validates `page_id` but still serializes shapes from the current page, so responses can be internally inconsistent when `page_id` is not the active page.</violation>

<violation number="2" location="apps/web/src/components/canvas/canvas-agent-bus.ts:424">
P1: `delete` should require `confirm === true`; the current truthy check allows non-boolean values to perform destructive deletes.</violation>

<violation number="3" location="apps/web/src/components/canvas/canvas-agent-bus.ts:559">
P2: `update_shape` should validate `x`/`y` as finite numbers; direct `Number(value)` can pass `NaN` into shape updates.</violation>

<violation number="4" location="apps/web/src/components/canvas/canvas-agent-bus.ts:578">
P1: `viewport` with `center_ids` ignores the requested ids and then overrides the zoom with `zoomToFit`, so it does not reliably center on the target shapes.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread crates/core-service/src/service/canvas_agent_relay.rs Outdated
Comment thread apps/web/src/components/canvas/canvas-agent-bus.ts Outdated
Comment thread apps/web/src/components/canvas/canvas-agent-bus.ts Outdated
Comment thread apps/cli/src/commands/canvas.rs Outdated
Comment thread apps/web/src/components/canvas/CanvasAgentOverlay.tsx Outdated
Comment thread apps/web/src/components/canvas/canvas-agent-bus.ts
Comment thread apps/web/src/components/canvas/canvas-agent-bus.ts Outdated
Backend (core-service / api):
- Relay `register` now dedupes by `client_id` globally (not by
  conn_id+client_id), so reconnects from the same browser tab no longer leave
  a stale entry that `resolve_target(Some(client_id))` could route to.
- `begin_pending` rejects duplicate `request_id`s instead of silently
  overwriting the existing oneshot sender (returns `DuplicateRequestError`);
  the HTTP handler surfaces this as 409 VALIDATION_ARG.
- `complete_dispatch` now requires `conn_id` to match the connection that
  originally received the dispatch — prevents a different tab from completing
  another tab's request (returns `CompleteDispatchResult::ConnMismatch`,
  which the WS handler converts to a ServiceError).
- `ResolveTarget::NotFound` returns the distinct `CANVAS_CLIENT_NOT_FOUND`
  code (was conflated with `CANVAS_BRIDGE_OFFLINE`).

CLI:
- `invoke` no longer assumes the response is JSON: on parse failure it
  surfaces the raw HTTP status + body preview so users can diagnose proxy /
  auth / HTML-error pages.

Frontend (apps/web/src/components/canvas):
- `CanvasAgentBus.runGetState` rejects requests for non-current pages
  (VALIDATION_ARG); the downstream `getCurrentPage*` reads always operate on
  the active page, so any other answer was internally inconsistent. The
  status/get_state early-returns are now wrapped by the same try/catch as the
  mutating commands.
- `runDelete` requires `confirm === true` (was a truthy check, which would
  accept strings/numbers/objects).
- `runUpdateShape` validates x/y via `requireNumber` (was raw `Number(value)`
  which silently produced NaN/Infinity for invalid input).
- `runViewport` with `center_ids` now computes the union of the requested
  shapes' page bounds and calls `zoomToBounds` with that — the previous
  implementation used `getSelectionPageBounds()` (ignored the ids) and
  immediately overrode the result with `zoomToFit()`.
- `CanvasAgentPresenceStore.getSnapshot` is now cached and invalidated on
  every emit; required for `useSyncExternalStore` (which compares with
  Object.is and would loop forever otherwise).
- `CanvasAgentOverlay` uses `pageToViewport` instead of `pageToScreen` so the
  agent badge `left/top` lands in the overlay container's local frame (not
  document space, which over-shifted by the editor's screenBounds offset).
- Replace hardcoded `text-emerald-500` with the semantic `text-foreground`
  token (theme-aware); add `aria-label`s to the icon-only Jump / Follow
  buttons for screen readers.
- `useCanvasAgentBridge`: unmount-only `unregister` now skips the WS call
  when the socket is already disconnected (otherwise it would trigger
  wsRequest's auto-reconnect during teardown).

Tests: 15 backend (`canvas_agent_relay`) and 38 frontend
(`canvas-agent-bus` / `canvas-agent-presence`) passing, including new
regression coverage for each fix.

Co-authored-by: AruNi_Lu <hello@0x3f4.run>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
apps/web/src/components/canvas/CanvasAgentOverlay.tsx (1)

127-127: ⚡ Quick win

Specify semantic ring color for theme adaptation.

The ring-2 class without an explicit color defaults to a hardcoded palette value (typically ring-blue-500) that won't adapt across light/dark themes.

🎨 Suggested fix
       className={cn(
         "pointer-events-auto absolute flex items-center gap-1 rounded-full px-2 py-0.5",
         "border border-border bg-background/95 shadow-md backdrop-blur",
-        isFollowed && "ring-2 ring-offset-1 ring-offset-background",
+        isFollowed && "ring-2 ring-foreground/30 ring-offset-1 ring-offset-background",
       )}

As per coding guidelines, "ALWAYS use semantic CSS variables (bg-background, text-muted-foreground, border-border) instead of hardcoded colors for theme adaptation".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/canvas/CanvasAgentOverlay.tsx` at line 127, The ring
utility lacks a semantic color and should use the project's semantic CSS
variable; update the conditional class string in CanvasAgentOverlay where
`isFollowed && "ring-2 ring-offset-1 ring-offset-background"` is used to include
a semantic ring color class (for example `ring-ring` or your theme's equivalent
like `ring-primary`) so it becomes `isFollowed && "ring-2 ring-ring
ring-offset-1 ring-offset-background"` to ensure the ring adapts to light/dark
themes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@apps/web/src/components/canvas/CanvasAgentOverlay.tsx`:
- Line 127: The ring utility lacks a semantic color and should use the project's
semantic CSS variable; update the conditional class string in CanvasAgentOverlay
where `isFollowed && "ring-2 ring-offset-1 ring-offset-background"` is used to
include a semantic ring color class (for example `ring-ring` or your theme's
equivalent like `ring-primary`) so it becomes `isFollowed && "ring-2 ring-ring
ring-offset-1 ring-offset-background"` to ensure the ring adapts to light/dark
themes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3090f56c-61be-43c5-a1c4-542b2fd9ff62

📥 Commits

Reviewing files that changed from the base of the PR and between 028c9de and fb7aa58.

📒 Files selected for processing (11)
  • apps/api/src/api/canvas/agent.rs
  • apps/cli/src/commands/canvas.rs
  • apps/web/src/components/canvas/CanvasAgentOverlay.tsx
  • apps/web/src/components/canvas/__tests__/canvas-agent-bus.test.ts
  • apps/web/src/components/canvas/__tests__/canvas-agent-presence.test.ts
  • apps/web/src/components/canvas/canvas-agent-bus.ts
  • apps/web/src/components/canvas/canvas-agent-presence.ts
  • apps/web/src/components/canvas/use-canvas-agent-bridge.ts
  • crates/core-service/src/lib.rs
  • crates/core-service/src/service/canvas_agent_relay.rs
  • crates/core-service/src/service/ws_message.rs
✅ Files skipped from review due to trivial changes (1)
  • crates/core-service/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (6)
  • apps/web/src/components/canvas/tests/canvas-agent-presence.test.ts
  • apps/api/src/api/canvas/agent.rs
  • apps/web/src/components/canvas/canvas-agent-presence.ts
  • crates/core-service/src/service/ws_message.rs
  • apps/cli/src/commands/canvas.rs
  • apps/web/src/components/canvas/canvas-agent-bus.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 11 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/cli/src/commands/canvas.rs">

<violation number="1" location="apps/cli/src/commands/canvas.rs:650">
P2: Avoid slicing the response preview string at a fixed byte offset; this can panic on non-ASCII UTF-8 content.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Re-trigger cubic

Comment thread apps/cli/src/commands/canvas.rs Outdated
AruNi-01 added 3 commits May 16, 2026 00:52
Document control plane, relay envelope, rollout phases, and tests;
register in specs index. Paseo reference corrected to Worker+DO.
Truncate non-JSON invoke errors by char count; add clap docs for root and
review/local/update subcommands. Align canvas agent copy in help, overlay
tooltip, and atmos-canvas-agent skill.
Use Agent label and update presence docs; keep TLInstancePresence behavior.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md (1)

1-267: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Rewrite the entire spec in English.

This TECH document is written entirely in Chinese, violating the coding guideline that requires all spec files to be written in English (titles, headings, and body text). Only inline quotes from Chinese source material are acceptable.

As per coding guidelines: "Write all spec files in English (titles, headings, body). Inline Chinese quotes from source material are acceptable when needed." and "TECH.md should document HOW: architecture, data, APIs, and rollout. Audience is engineers implementing the change."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md` around lines 1 -
267, The TECH.md for APP-016 is written in Chinese and must be converted to
English: rewrite the entire document (titles, headings, body, tables, lists, and
comments) into clear English while preserving all technical content, structure
(sections like "1. Architecture Overview", "3. Control Plane", "4. Relay data
plane: Envelope spec", "5. Durable Object: ServerHub", etc.), code blocks,
variable names (e.g., server_id, server_secret, client_session_id,
last_relay_seq), API shapes (/v1/pair_codes, /v1/servers/register), and
deployment/rollout plans; keep original semantics and examples intact, ensure
the target audience is engineers (implementable architecture, data shapes, and
APIs), and only retain any original Chinese text as brief inline quotes when
absolutely necessary.
specs/APP/APP-016_multi-server-relay-cloudflare-do/BRAINSTORM.md (1)

1-61: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Rewrite this spec file entirely in English.

All spec files must have titles, headings, and body text in English per the coding guidelines. Only inline quotes from Chinese source material are acceptable. This BRAINSTORM.md is written entirely in Chinese and requires complete translation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_multi-server-relay-cloudflare-do/BRAINSTORM.md` around
lines 1 - 61, Translate the entire BRAINSTORM.md content into clear, idiomatic
English while preserving the original structure, headings, section numbers
(e.g., "1. 我们要解决什么", "2. 参考产品形态", "3. 方案轴心:Relay + 出站 WebSocket", "4.
与现有仓库能力的关系", "5. 开放问题") and the spec identifier APP-016; keep all links, tables,
and technical symbols (e.g., "Cloudflare Durable Objects", "RelayDurableObject",
"D1", "boot_data.json", "WS") intact, and ensure any original Chinese inline
quotations remain quoted rather than translated. Ensure terminology that maps to
product concepts (Server, Relay, CLI, Web/Desktop UI, Replay, control plane/data
plane) is consistently translated and that non-goal/assumption bullets (like
APP-012 exclusion) retain their meaning.
♻️ Duplicate comments (2)
apps/cli/src/commands/canvas.rs (2)

446-454: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix rustfmt formatting to unblock CI.

The CI pipeline is failing cargo fmt --check on this error string. The multi-line .to_string() call needs reformatting.

🔧 Required rustfmt fix
     fn body(&self) -> Result<Value, String> {
         if !self.confirm {
-            return Err(
-                "delete is destructive — re-run with --confirm to acknowledge.".to_string()
-            );
+            return Err("delete is destructive — re-run with --confirm to acknowledge.".to_string());
         }
         Ok(json!({ "ids": split_ids(&self.ids), "confirm": true }))
     }

Run cargo fmt --all to fix this and commit the result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/src/commands/canvas.rs` around lines 446 - 454, The multi-line
string in the method body(&self) -> Result<Value, String) is misformatted for
rustfmt; reflow the Err return so the string and .to_string() are on one line
(or otherwise conform to rustfmt style) inside the body method, then run cargo
fmt --all and commit the formatted file; specifically update the Err("delete is
destructive — re-run with --confirm to acknowledge.".to_string()) expression in
the body function to match rustfmt expectations.

715-722: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix rustfmt formatting to unblock CI.

The CI pipeline is failing cargo fmt --check on the ATMOS_API_TOKEN environment variable chain at line 721. The chained method calls need to be split across lines per rustfmt style.

🔧 Required rustfmt fix
 fn resolve_token(global: &GlobalArgs) -> Option<String> {
     if let Some(token) = &global.api_token {
         if !token.is_empty() {
             return Some(token.clone());
         }
     }
-    std::env::var("ATMOS_API_TOKEN").ok().filter(|v| !v.is_empty())
+    std::env::var("ATMOS_API_TOKEN")
+        .ok()
+        .filter(|v| !v.is_empty())
 }

Run cargo fmt --all to fix this and commit the result.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/cli/src/commands/canvas.rs` around lines 715 - 722, The function
resolve_token has a rustfmt failure due to a chained call on
std::env::var("ATMOS_API_TOKEN").ok().filter(|v| !v.is_empty()); update the
formatting so the chained method calls are split across lines (e.g., put .ok()
and .filter(...) each on their own line) to satisfy rustfmt for the
resolve_token function and the ATMOS_API_TOKEN environment lookup, then run
cargo fmt --all and commit the formatted changes.
🧹 Nitpick comments (1)
specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md (1)

18-33: 💤 Low value

Add language identifier to fenced code block.

The ASCII diagram at lines 18-33 is in a fenced code block without a language identifier. While ASCII art doesn't need syntax highlighting, specifying a language (or using text) improves tool compatibility.

📝 Suggested fix
-```
+```text
 ┌─────────────┐     出站 WSS      ┌──────────────────┐
 │ Atmos Server│ ───────────────► │ DO(server_id)    │
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md` around lines 18 -
33, The fenced ASCII diagram in TECH.md is missing a language identifier; update
the opening fence for the diagram (the triple backticks that enclose the ASCII
art beginning around the block with "┌─────────────┐     出站 WSS") to include a
language tag such as text (e.g., change ``` to ```text) so tools correctly treat
it as plain text and improve compatibility while leaving the diagram content
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/README.md`:
- Line 78: The spec entry labeled APP-016 currently uses a Chinese title "多
Server 与 Cloudflare Relay(DO)"; update that title to English for consistency
(e.g., "Multi-Server and Cloudflare Relay (DO)") so all spec files follow the
guideline to be written in English—edit the APP-016 line in specs/README.md
replacing the Chinese title with the chosen English title (ensure the identifier
APP-016 and referenced docs `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md`
remain unchanged).
- Line 78: Insert a new table row for APP-015 (Canvas Terminal Agent
Integration) into the "Current Specs" table in README.md between the existing
APP-014 and APP-016 rows; the row should list APP-015 as the key, the title
"Canvas Terminal Agent Integration", and the completed spec files
`BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` so the table reflects the
published and active spec.

---

Outside diff comments:
In `@specs/APP/APP-016_multi-server-relay-cloudflare-do/BRAINSTORM.md`:
- Around line 1-61: Translate the entire BRAINSTORM.md content into clear,
idiomatic English while preserving the original structure, headings, section
numbers (e.g., "1. 我们要解决什么", "2. 参考产品形态", "3. 方案轴心:Relay + 出站 WebSocket", "4.
与现有仓库能力的关系", "5. 开放问题") and the spec identifier APP-016; keep all links, tables,
and technical symbols (e.g., "Cloudflare Durable Objects", "RelayDurableObject",
"D1", "boot_data.json", "WS") intact, and ensure any original Chinese inline
quotations remain quoted rather than translated. Ensure terminology that maps to
product concepts (Server, Relay, CLI, Web/Desktop UI, Replay, control plane/data
plane) is consistently translated and that non-goal/assumption bullets (like
APP-012 exclusion) retain their meaning.

In `@specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md`:
- Around line 1-267: The TECH.md for APP-016 is written in Chinese and must be
converted to English: rewrite the entire document (titles, headings, body,
tables, lists, and comments) into clear English while preserving all technical
content, structure (sections like "1. Architecture Overview", "3. Control
Plane", "4. Relay data plane: Envelope spec", "5. Durable Object: ServerHub",
etc.), code blocks, variable names (e.g., server_id, server_secret,
client_session_id, last_relay_seq), API shapes (/v1/pair_codes,
/v1/servers/register), and deployment/rollout plans; keep original semantics and
examples intact, ensure the target audience is engineers (implementable
architecture, data shapes, and APIs), and only retain any original Chinese text
as brief inline quotes when absolutely necessary.

---

Duplicate comments:
In `@apps/cli/src/commands/canvas.rs`:
- Around line 446-454: The multi-line string in the method body(&self) ->
Result<Value, String) is misformatted for rustfmt; reflow the Err return so the
string and .to_string() are on one line (or otherwise conform to rustfmt style)
inside the body method, then run cargo fmt --all and commit the formatted file;
specifically update the Err("delete is destructive — re-run with --confirm to
acknowledge.".to_string()) expression in the body function to match rustfmt
expectations.
- Around line 715-722: The function resolve_token has a rustfmt failure due to a
chained call on std::env::var("ATMOS_API_TOKEN").ok().filter(|v| !v.is_empty());
update the formatting so the chained method calls are split across lines (e.g.,
put .ok() and .filter(...) each on their own line) to satisfy rustfmt for the
resolve_token function and the ATMOS_API_TOKEN environment lookup, then run
cargo fmt --all and commit the formatted changes.

---

Nitpick comments:
In `@specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md`:
- Around line 18-33: The fenced ASCII diagram in TECH.md is missing a language
identifier; update the opening fence for the diagram (the triple backticks that
enclose the ASCII art beginning around the block with "┌─────────────┐     出站
WSS") to include a language tag such as text (e.g., change ``` to ```text) so
tools correctly treat it as plain text and improve compatibility while leaving
the diagram content unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e079f0a5-2591-47db-b28a-ff2b852b02f0

📥 Commits

Reviewing files that changed from the base of the PR and between fb7aa58 and e516ae2.

📒 Files selected for processing (13)
  • apps/cli/src/commands/canvas.rs
  • apps/cli/src/commands/local.rs
  • apps/cli/src/commands/review.rs
  • apps/cli/src/commands/update.rs
  • apps/cli/src/main.rs
  • apps/web/src/components/canvas/CanvasAgentOverlay.tsx
  • apps/web/src/components/canvas/canvas-agent-presence.ts
  • skills/atmos-canvas-agent/SKILL.md
  • specs/APP/APP-016_multi-server-relay-cloudflare-do/BRAINSTORM.md
  • specs/APP/APP-016_multi-server-relay-cloudflare-do/PRD.md
  • specs/APP/APP-016_multi-server-relay-cloudflare-do/TECH.md
  • specs/APP/APP-016_multi-server-relay-cloudflare-do/TEST.md
  • specs/README.md
✅ Files skipped from review due to trivial changes (5)
  • specs/APP/APP-016_multi-server-relay-cloudflare-do/PRD.md
  • apps/cli/src/commands/update.rs
  • specs/APP/APP-016_multi-server-relay-cloudflare-do/TEST.md
  • apps/cli/src/commands/review.rs
  • skills/atmos-canvas-agent/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/cli/src/main.rs
  • apps/web/src/components/canvas/CanvasAgentOverlay.tsx
  • apps/web/src/components/canvas/canvas-agent-presence.ts

Comment thread specs/README.md Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 13 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/cli/src/commands/review.rs">

<violation number="1" location="apps/cli/src/commands/review.rs:258">
P3: The new help text is incorrect: this command sets an agent run status, not a review session status.</violation>
</file>

<file name="specs/README.md">

<violation number="1" location="specs/README.md:78">
P2: This row replacement drops APP-015 from the Current Specs table; APP-016 should be added without removing APP-015.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic

Comment thread specs/README.md Outdated
| **APP-013** | Project-Level Review Session | `BRAINSTORM.md` |
| **APP-014** | Canvas | `PRD.md` |
| **APP-015** | Canvas Terminal Agent Integration | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |
| **APP-016** | 多 Server 与 Cloudflare Relay(DO) | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This row replacement drops APP-015 from the Current Specs table; APP-016 should be added without removing APP-015.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At specs/README.md, line 78:

<comment>This row replacement drops APP-015 from the Current Specs table; APP-016 should be added without removing APP-015.</comment>

<file context>
@@ -74,7 +75,7 @@ All four files are always present. Missing content stays as a **template placeho
 | **APP-013** | Project-Level Review Session | `BRAINSTORM.md` |
 | **APP-014** | Canvas | `PRD.md` |
-| **APP-015** | Canvas Terminal Agent Integration | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |
+| **APP-016** | 多 Server 与 Cloudflare Relay(DO) | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |
 
 ### Landing
</file context>
Suggested change
| **APP-016** | 多 Server 与 Cloudflare Relay(DO) | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |
| **APP-015** | Canvas Terminal Agent Integration | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |
| **APP-016** | Multi-Server and Cloudflare Relay (DO) | `BRAINSTORM.md`, `PRD.md`, `TECH.md`, `TEST.md` |

Tip: Review your code locally with the cubic CLI to iterate faster.

SummarizeRun(SummarizeRunArgs),
/// Finalize a review agent run.
FinalizeRun(FinalizeRunArgs),
/// Set the status of a review session.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The new help text is incorrect: this command sets an agent run status, not a review session status.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/cli/src/commands/review.rs, line 258:

<comment>The new help text is incorrect: this command sets an agent run status, not a review session status.</comment>

<file context>
@@ -235,16 +235,27 @@ fn read_optional_body_input(
     SummarizeRun(SummarizeRunArgs),
+    /// Finalize a review agent run.
     FinalizeRun(FinalizeRunArgs),
+    /// Set the status of a review session.
     SetStatus(SetStatusArgs),
 }
</file context>
Suggested change
/// Set the status of a review session.
/// Set the status of a review agent run.

Move specs to specs/APP/APP-016_atmos-computer/ and align PRD/TECH/TEST/brainstorm
with the Atmos Computer product name plus API-anchor CLI guidance.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
specs/APP/APP-016_atmos-computer/TECH.md (2)

265-293: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add an explicit risks section in TECH.md.

PRD has risks, but TECH is also required to list implementation/operational risks in this file.

Suggested patch
+## 13. Risks
+
+| Risk | Impact | Mitigation |
+|------|--------|------------|
+| Relay hub hotspot on popular `server_id` | Increased latency / dropped frames | Per-hub rate limiting, backpressure, shard strategy |
+| Replay ring growth or mis-sizing | Memory/storage pressure | Hard caps, TTL, eviction policy, alerting |
+| Token revocation lag across edge | Unauthorized short-lived access window | Short token TTL + push revoke + connection revalidation |
+| Client/server protocol drift | Session failures after deploy | Version negotiation and reject-incompatible behavior |

As per coding guidelines, "specs/**/{PRD,TECH,TEST}.md: PRD.md must state both scope AND non-scope. TECH.md must list risks. TEST.md must list acceptance criteria."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_atmos-computer/TECH.md` around lines 265 - 293, Add an
explicit "风险" section to TECH.md (e.g., after "## 11. 安全清单(摘要)" or before "##
12. 开放实现项") that enumerates implementation and operational risks and
mitigations: include items covering TLS full‑chain failures, token
short‑termization and instantaneous revocation edge cases, rate‑limit
misconfigurations per user_id/server_id/IP, Relay availability/loopback
degradation, observability gaps (missing metrics like connection count/frame
rate/drop/replay hit), logging/privacy (body redact/sampling risks), and open
design decisions (body encoding choice, Control Plane vs Relay key sharing) with
suggested owners and mitigation steps; ensure each risk is concise, has
potential impact and a proposed mitigation or owner listed.

1-293: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Translate the TECH spec to English.

The current Chinese content conflicts with the spec language policy for published docs.

As per coding guidelines, "Write all spec files in English (titles, headings, body). Inline Chinese quotes from source material are acceptable when needed."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_atmos-computer/TECH.md` around lines 1 - 293, The TECH.md
file is written in Chinese and must be fully translated into English while
preserving all technical identifiers and code/examples (e.g., server_id,
server_secret, apps/api, Relay Hub, Durable Object, ServerHub, boot_data.json,
~/.atmos paths, envelope fields like
v/stream/kind/from/to/request_id/relay_seq/body, endpoints such as
/v1/pair_codes and /v1/servers/register, and section references like
§3.2/§4/§8.2); replace the title "APP-016:Atmos Computer" and all headings and
body text with idiomatic English, keep inline Chinese quotes only where
necessary, leave code blocks/json and diagrams unchanged except for surrounding
explanatory text translation, and ensure references to PRD.md and other spec
filenames remain intact.
specs/APP/APP-016_atmos-computer/PRD.md (1)

1-100: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

PRD must be written in English.

This document is currently Chinese-first; please translate title, headings, and body to English for compliance.

As per coding guidelines, "Write all spec files in English (titles, headings, body). Inline Chinese quotes from source material are acceptable when needed."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_atmos-computer/PRD.md` around lines 1 - 100, The PRD is
written in Chinese but must be English-first; translate the entire document
(title "PRD · APP-016:Atmos Computer", all section headings like "1. 背景与动机", "2.
目标用户与核心场景", user stories, requirements, scope, success metrics, risks, and doc
relations) into clear English while preserving any necessary inline Chinese
quotes, names (e.g., "Atmos Computer", "Atmos Server", "server_id"), links to
TECH.md/TEST.md, and the original structure; ensure terminology (M1/M2 IDs,
bullets, tables) remains intact and update any in-text cross-references to other
specs so they remain accurate in English.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/APP/APP-016_atmos-computer/BRAINSTORM.md`:
- Around line 1-61: The spec BRAINSTORM.md is written in Chinese and must be
translated to English before merge; convert all titles, headings, and body text
into fluent English while preserving technical terms and links (e.g., "Atmos
Computer", "Atmos Server", "Relay + DO", "PRD.md", "TECH.md", "APP-016") and
keep inline Chinese quotes only if necessary, ensure lists, tables, and section
semantics remain identical, and update any referenced English names or labels
(like product comparisons and section headings) so the file conforms to the
repository rule "Write all spec files in English."

In `@specs/APP/APP-016_atmos-computer/TECH.md`:
- Around line 21-36: The fenced topology diagram block in the TECH.md file lacks
a language identifier which breaks markdown lint/rendering; update the fenced
code block delimiter for that ASCII topology (the multi-line diagram starting
with "┌─────────────┐     出站 WSS      ┌──────────────────┐") to include a
language tag such as "text" (i.e., change ``` to ```text) so the block is
properly recognized by linters and renderers.

---

Outside diff comments:
In `@specs/APP/APP-016_atmos-computer/PRD.md`:
- Around line 1-100: The PRD is written in Chinese but must be English-first;
translate the entire document (title "PRD · APP-016:Atmos Computer", all section
headings like "1. 背景与动机", "2. 目标用户与核心场景", user stories, requirements, scope,
success metrics, risks, and doc relations) into clear English while preserving
any necessary inline Chinese quotes, names (e.g., "Atmos Computer", "Atmos
Server", "server_id"), links to TECH.md/TEST.md, and the original structure;
ensure terminology (M1/M2 IDs, bullets, tables) remains intact and update any
in-text cross-references to other specs so they remain accurate in English.

In `@specs/APP/APP-016_atmos-computer/TECH.md`:
- Around line 265-293: Add an explicit "风险" section to TECH.md (e.g., after "##
11. 安全清单(摘要)" or before "## 12. 开放实现项") that enumerates implementation and
operational risks and mitigations: include items covering TLS full‑chain
failures, token short‑termization and instantaneous revocation edge cases,
rate‑limit misconfigurations per user_id/server_id/IP, Relay
availability/loopback degradation, observability gaps (missing metrics like
connection count/frame rate/drop/replay hit), logging/privacy (body
redact/sampling risks), and open design decisions (body encoding choice, Control
Plane vs Relay key sharing) with suggested owners and mitigation steps; ensure
each risk is concise, has potential impact and a proposed mitigation or owner
listed.
- Around line 1-293: The TECH.md file is written in Chinese and must be fully
translated into English while preserving all technical identifiers and
code/examples (e.g., server_id, server_secret, apps/api, Relay Hub, Durable
Object, ServerHub, boot_data.json, ~/.atmos paths, envelope fields like
v/stream/kind/from/to/request_id/relay_seq/body, endpoints such as
/v1/pair_codes and /v1/servers/register, and section references like
§3.2/§4/§8.2); replace the title "APP-016:Atmos Computer" and all headings and
body text with idiomatic English, keep inline Chinese quotes only where
necessary, leave code blocks/json and diagrams unchanged except for surrounding
explanatory text translation, and ensure references to PRD.md and other spec
filenames remain intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 53d25572-db63-4343-95be-e4c0fb9b4829

📥 Commits

Reviewing files that changed from the base of the PR and between e516ae2 and 11ed025.

📒 Files selected for processing (5)
  • specs/APP/APP-016_atmos-computer/BRAINSTORM.md
  • specs/APP/APP-016_atmos-computer/PRD.md
  • specs/APP/APP-016_atmos-computer/TECH.md
  • specs/APP/APP-016_atmos-computer/TEST.md
  • specs/README.md
✅ Files skipped from review due to trivial changes (2)
  • specs/APP/APP-016_atmos-computer/TEST.md
  • specs/README.md

Comment on lines +1 to +61
# 头脑风暴 · APP-016:Atmos Computer

> 探索阶段:问题空间、方案取舍、开放问题。**命名**:用户向功能名 **Atmos Computer**;**Atmos Server** = 某 Computer 上的 `apps/api`。**Relay + DO** 见 `TECH.md`。定稿内容以 `PRD.md` / `TECH.md` 为准。

## 1. 我们要解决什么

### 1.1 现状痛点

- **开发期**:Web(Next)与 API(Axum)双进程、双端口;CLI 与浏览器要对齐「同一台 **Computer** 上的 API」,依赖环境变量或本机 `boot_data.json` 等发现机制,心智负担仍在。
- **产品愿景**:用户有多台 **Atmos Computer**——本地笔记本、**云端 VPS/虚拟机**、办公室工作站——希望 **Web / Desktop UI 选定一台 Computer**,在该环境中开发;**CLI** 与 UI **共用当前所选 Computer**;常在一台 **Computer** 的终端里跑命令时,与 UI 所见为 **同一台 Computer**。
- **网络约束**:不希望把「多 **Computer** / 多入口」与「入站 NAT 穿透 / 自建隧道」强绑定;倾向 **Server 仅出站**、云端提供 **Relay**,由 Relay 做路由与(可选)回放。

### 1.2 非目标(本 spec 刻意不包含)

- **不包含 [APP-012](../APP-012_remote-access/TECH.md) 的 remote-access 能力**:remote-access 解决的是「如何把本机 API 暴露到公网/LAN」这一层;本 spec 的 **Relay 是另一条独立路径**(出站 + 云端路由)。两者可并存,但 **设计决策互不依赖**。
- **不把 OpenCode 式「单二进制内嵌 serve+web」当作目标**:Atmos 保持 `api` / `web` / `desktop` / `cli` 分仓发布;Relay 只解决 **跨进程、跨机器的控制面与数据面连接**。

## 2. 参考产品形态(类比,非实现约束)

| 产品/概念 | 启发点 |
|-----------|--------|
| [Factory Droid Computers](https://docs.factory.ai/cli/features/droid-computers)(**第三方品牌**) | 对方产品里「可寻址的一台计算环境」的**产品形态**可参考;**本 spec 用户向名称:Atmos Computer**;不把 Atmos 叫作 Droid,以免与 Factory 的 Agent 品牌混淆。 |
| [Paseo](https://github.com/getpaseo/paseo) | 官方 `packages/relay` 用 **Wrangler + Worker + Durable Object**(`RelayDurableObject`)做 WS 中继、按 `serverId` 路由;**未见 D1 绑定**(DO 侧可用 **Durable Objects SQLite** 等能力)。README 亦指向社区 **[paseo-relay](https://github.com/zenghongtu/paseo-relay)(Go 自建)**,与 Cloudflare 无关。 |

**借鉴结论**:把 **Atmos Server**(`apps/api` 进程)定义为 **单台 Atmos Computer 内计算与状态的唯一宿主**;Relay 只做 **连接与路由(+ 可选事件回放)**;Web/Desktop 是 **多 Computer 客户端**。

## 3. 方案轴心:Relay + 出站 WebSocket

### 3.1 为什么用「出站 WS」而不是「入站 API」

- Server 部署在用户机、内网、随机端口时,**入站公网地址不稳定**。
- **Cloudflare Durable Objects(DO)** 适合维护「每 `server_id` 一个 hub」的长连接与会话状态;Worker + D1 适合 **控制面**(账号、**Computer** 列表、配对码、令牌签发)。
- 用户明确要求使用 **Cloudflare DO 提供 replay**(事件缓冲与断线重放语义),与「仅转发」可分期实现。

### 3.2 备选方案(记录取舍)

| 方案 | 优点 | 缺点 | 结论 |
|------|------|------|------|
| A. 每台 Server 公网 IP + TLS | 简单直连 | 端口、证书、防火墙、动态 IP | 不作为主路径 |
| B. 自建 STUN/TURN + P2P | 低云端成本 | 实现与排障成本高 | 不采纳 |
| C. 仅 Tailscale/WireGuard | 安全 | 强制用户网络栈 | 可作为 **可选增强**,非本 spec 核心 |
| **D. CF Worker + DO Relay,Server 出站** | 无入站、易扩展多台 **Computer**、与 DO replay 契合 | 依赖 CF、需设计协议与配额 | **主路径** |

## 4. 与现有仓库能力的关系

- **现有 WS 协议**(终端、Canvas bridge、业务消息):理想情况下 **body 不在 Relay 解析**,仅路由 **信封** → 降低耦合,保留未来 E2EE 空间。
- **`boot_data.json`(APP-016 实施时可调整)**:描述 **某一 Computer 上 Server 进程在本机的 loopback 监听**(端口、本地 token),当 **Web/Desktop/CLI 的当前上下文 = 该 Computer** 时,可作为 **该上下文的** API 基址发现来源;**不是**「凡 CLI 必读本机」的全局规则。
- **CLI `review` 与 API 锚点**:当前仓库里 `atmos review` 通过 **本机 SQLite**(`~/.atmos/db/atmos.db`)直连 `ReviewService`,与浏览器连 **远端/另一套 API** 时会产生 **静默分叉**;本 spec 与 PRD **M1-7** 要求改为 **仅经 `apps/api`**,与「**Computer** 上 **Server** 为计算与状态宿主」叙事一致。
- **Desktop 侧车 API**:仍是「本机一台 **Computer**」的一种形态;连 Relay 时与远程 **Computer** **同一套出站客户端逻辑**。

## 5. 开放问题(需在 PRD/TECH 迭代中收敛)

1. **控制面域名与多环境**:`staging` / `prod` Relay 是否分 Worker,还是单 Worker + 环境头?
2. **身份模型**:M1 是否仅「单用户拥有多台 **Computer**」,还是预留组织/工作区共享?
3. **Replay 语义**:与现有 WS「请求-响应」混用时,**幂等键**(`request_id` / `client_seq`)的规范。
4. **合规与审计**:Relay 是否持久化消息内容,还是仅元数据 + 短期环形缓冲?
5. **CLI 在用户笔记本上经 Relay 调 Canvas**:与「Canvas 仅连当前激活 **Computer**」的 UX 是否要在 M1 就做,还是 M2?

---

*本文件随讨论更新;实现以前以 PRD/TECH 锁定范围为准。*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Convert this spec file to English before merge.

The file is currently written in Chinese, which violates repository spec-language requirements for published specs.

As per coding guidelines, "Write all spec files in English (titles, headings, body). Inline Chinese quotes from source material are acceptable when needed."

🧰 Tools
🪛 LanguageTool

[uncategorized] ~11-~11: 您的意思是“由"于"”吗?
Context: ...Server 仅出站*、云端提供 Relay,由 Relay 做路由与(可选)回放。 ### 1.2 非目标(本 spec 刻意不包含) - **...

(YU7_YU8)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/APP/APP-016_atmos-computer/BRAINSTORM.md` around lines 1 - 61, The spec
BRAINSTORM.md is written in Chinese and must be translated to English before
merge; convert all titles, headings, and body text into fluent English while
preserving technical terms and links (e.g., "Atmos Computer", "Atmos Server",
"Relay + DO", "PRD.md", "TECH.md", "APP-016") and keep inline Chinese quotes
only if necessary, ensure lists, tables, and section semantics remain identical,
and update any referenced English names or labels (like product comparisons and
section headings) so the file conforms to the repository rule "Write all spec
files in English."

Comment on lines +21 to +36
```
┌─────────────┐ 出站 WSS ┌──────────────────┐
│ Atmos Server│ ───────────────► │ DO(server_id) │
│ (任意网络) │ ◄─────────────── │ Relay Hub │
└─────────────┘ 下行帧 └────────▲───────────┘
│
WSS(客户端路径) │
┌─────────────┐ │
│ Web/Desktop │ ─────────────────────────┘
└─────────────┘
│
▼
┌──────────────────┐
│ Worker (路由) │ TLS 终止、鉴权、将 client 绑定到对应 DO
└──────────────────┘
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced topology block.

This improves markdown lint compliance and renderer behavior.

Suggested patch
-```
+```text
 ┌─────────────┐     出站 WSS      ┌──────────────────┐
 │ Atmos Server│ ───────────────► │ DO(server_id)    │
 │ (任意网络)  │ ◄─────────────── │ Relay Hub        │
 └─────────────┘     下行帧      └────────▲───────────┘
                                          │
               WSS(客户端路径)           │
 ┌─────────────┐                          │
 │ Web/Desktop │ ─────────────────────────┘
 └─────────────┘
         │
         ▼
 ┌──────────────────┐
 │ Worker (路由)     │  TLS 终止、鉴权、将 client 绑定到对应 DO
 └──────────────────┘
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 21-21: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @specs/APP/APP-016_atmos-computer/TECH.md around lines 21 - 36, The fenced
topology diagram block in the TECH.md file lacks a language identifier which
breaks markdown lint/rendering; update the fenced code block delimiter for that
ASCII topology (the multi-line diagram starting with "┌─────────────┐ 出站 WSS
┌──────────────────┐") to include a language tag such as "text" (i.e., change
totext) so the block is properly recognized by linters and renderers.


</details>

<!-- fingerprinting:phantom:triton:hawk -->

<!-- This is an auto-generated comment by CodeRabbit -->

This branch was successfully deployed

1 active deployment
Preview — 11ed025e Deployed May 16, 2026 by vercel[bot]
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