Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions APPLICATION_SPEC.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
PR https://github.com/tilework-tech/nori-sessions/pull/830 and PR https://github.com/tilework-tech/nori-cli/pull/486 together add support for the `nori cloud` command, a cli-local command that allows a user to connect to and spin up a nori sessions (~/code/nori/nori-sessions/**) session.

We now want to improve the behavior of nori sessions out so that the product experience is more seamless.

These changes will require a PR on the nori-sessions repo AND the nori-cli repo.

1. When the user runs `nori cloud`, instead of automatically starting a new session, it should show a list of previous sessions and have an option to select a new one. If the user selects one of the previous sessions it should just resume that one on the remote machine.

2. Many of the nori-cli options are exclusively for client side behavior. That includes:
- most of the things under /settings
- many of the slash commands
- many of the other options like the worktree stuff and the skillset switching stuff.
These should all be default disabled when in cloud mode.

3. The user should be able to restart conversations that were originally started on slack or discord through the cli (nori cloud).
57 changes: 57 additions & 0 deletions CURRENT-PROGRESS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Current Progress

## Status: All CLI-side items complete

### Completed: Cloud Session Selection (Spec Item 1)

**What was done:**
- Added `CloudSessionSummary` type and `BrokerError::ListFailed`/`ResumeFailed` variants to `acp/src/broker/mod.rs`
- Added `BrokerClient::list_sessions()` (GET /api/sessions) and `BrokerClient::resume_session()` (POST /api/sessions/{id}/resume) methods
- Created `cli/src/cloud.rs` with session selection UI: `format_cloud_session_list()`, `parse_session_choice()`, `prompt_session_selection()`
- Modified `cli/src/main.rs` cloud handler to list sessions, show picker, and route to resume or acquire
- Added 9 integration tests for broker methods and 8 unit tests for CLI selection logic
- Updated broker, CLI, and ACP docs

**Design decisions:**
- Pre-TUI session picker (stdin/stderr) — TUI needs WebSocket URL before it can launch
- Graceful degradation on 404 (broker doesn't support listing yet) — falls back to new session
- Non-interactive terminals skip picker and use old behavior (auto-acquire)
- No new crate dependencies

### Completed: Cloud Mode Feature Gating (Spec Item 2)

**What was done:**
- Added `SlashCommand::available_in_cloud_mode()` method with exhaustive match (no wildcard) classifying each command
- 11 client-only commands disabled in cloud mode: `/settings`, `/init`, `/browse`, `/diff`, `/mention`, `/memory`, `/mcp`, `/browser`, `/switch-skillset`, `/resume`, `/resume-viewonly`
- Created `chatwidget/cloud_mode.rs` with `apply_cloud_mode_restrictions()` that uses existing `CommandAvailability`/`disabled_builtins` infrastructure
- Added `is_cloud_session: bool` field to `ChatWidget`, set from `cloud_connection.is_some()` at construction
- Belt-and-suspenders dispatch guard in `dispatch_command()` blocks cloud-disabled commands even via direct entry
- Auto-worktree setup skipped entirely when `cloud_connection.is_some()` in `lib.rs`
- Deferred skillset-per-session spawn disabled in cloud mode in `App::run()`
- Added 2 unit tests for command classification
- Updated TUI docs

**Design decisions:**
- Reuses existing `CommandAvailability` infrastructure rather than a new mechanism
- Exhaustive match forces classification of new commands at compile time
- Cloud-enabled commands: `/agent`, `/model`, `/config`, `/approvals`, `/goal`, `/new`, `/compact`, `/undo`, `/status`, `/first-prompt`, `/quit`, `/exit`, `/login`, `/logout`, `/fork`
- Cloud restrictions re-applied after `SessionCapabilitiesChanged` events to ensure local-only commands stay disabled even if the backend sends new capabilities

### Completed: Slack/Discord Session Resumption (Spec Item 3)

**What was done:**
- Naturally supported by item 1 — the session picker displays all sessions from the broker, with a `source` field (cli, slack, discord, etc.) shown next to each entry
- `CloudSessionSummary.source` field differentiates session origins
- No additional CLI code needed; the `format_cloud_session_list()` function already renders `(source)` per session

**Design decisions:**
- No CLI-side filtering by source — all sessions are shown regardless of origin
- The broker is responsible for returning sessions from all sources

### Broker-side work needed (nori-sessions repo — separate PR)
- `GET /api/sessions` endpoint returning `Vec<CloudSessionSummary>` with sessions from all sources (cli, slack, discord)
- `POST /api/sessions/{id}/resume` endpoint returning `SessionInfo { session_id, ws_url }`

## Status: All CLI-side items complete

All three spec items are implemented on the CLI side. The broker-side work (nori-sessions repo) is tracked separately.
80 changes: 80 additions & 0 deletions RESEARCH-NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Research Notes

## Cloud Session Architecture

### Current State
- `nori cloud` authenticates with broker, acquires a NEW session every time, connects via WebSocket, launches TUI
- `BrokerClient` has only `acquire_session()` and `release_session()` — no list/resume
- Cloud sessions skip local transcript recording (broker records server-side)
- No feature gating in cloud mode — all TUI features available identically

### Key Files
- `nori-rs/acp/src/broker/mod.rs` — BrokerClient, CloudConnectionInfo, SessionInfo, BrokerError
- `nori-rs/cli/src/main.rs:522-608` — Cloud subcommand handler
- `nori-rs/tui/src/nori/resume_session_picker.rs` — Local resume picker (reference for patterns)
- `nori-rs/acp/src/broker/tests.rs` — Broker client tests (integration tests with tiny_http mock servers)

### API Flow
1. CLI resolves broker URL (flag > config.toml > interactive prompt)
2. CLI authenticates (OAuth browser flow → JWT persisted to cloud-auth.json)
3. CLI acquires session (POST /api/sessions/acquire → SessionInfo { session_id, ws_url })
4. CLI launches TUI with CloudConnectionInfo { ws_url, auth_token }
5. On exit, CLI releases session (POST /api/sessions/{id}/release, best-effort 5s timeout)

### Design Decision: Pre-TUI Session Picker
The session selection must happen BEFORE the TUI launches because the TUI needs a WebSocket URL.
Using simple stdin interaction (eprintln + read_line) — no new crate deps needed.
The local `/resume` picker uses in-TUI SelectionView, but that requires a running TUI.

### New Broker API Endpoints Assumed
- `GET /api/sessions` → Vec<CloudSessionSummary> (list user's sessions)
- `POST /api/sessions/{id}/resume` → SessionInfo { session_id, ws_url } (resume existing session)

### New Types Needed
- `CloudSessionSummary` — session_id, source, created_at, last_active_at, first_message_preview, status
- `BrokerError::ListFailed` — for list endpoint failures
- `BrokerError::ResumeFailed` — for resume endpoint failures

## Cloud Mode Feature Gating (Item 2)

### Existing Infrastructure
The codebase already has a `CommandAvailability` / `disabled_builtins` system:
- `nori_protocol::CommandAvailability { enabled: bool, reason: Option<String> }` — per-command gate with user-facing reason
- `ChatWidget::builtin_command_availability` (HashMap) — stores availability state
- `ChatWidget::ensure_builtin_command_enabled(cmd)` — checks map at dispatch time, shows error if disabled
- `BottomPane::set_builtin_command_disabled(cmd, reason)` — propagates to `ChatComposer` → `CommandPopup` for greyed-out rendering
- `CommandPopup::disabled_builtins` — prevents selection and dims disabled commands in the popup

### Cloud Connection Propagation
- `CloudConnectionInfo` flows: CLI → `App::run` → `ChatWidgetInit` → `spawn_agent()`
- `App.cloud_connection: Option<CloudConnectionInfo>` stored on App struct
- `ChatWidget` does NOT currently store `cloud_connection` — it's consumed at construction time for agent spawning only
- Need to add `is_cloud_session: bool` field to `ChatWidget` for runtime checks

### Slash Commands to Disable in Cloud Mode
Client-side only (need local filesystem/process/config access):
- `/settings` — local CLI config (theme, hotkeys, layout)
- `/init` — creates local AGENTS.md file
- `/browse` — opens local file manager
- `/diff` — runs local git diff
- `/mention` — references local files
- `/memory` — shows local instruction files
- `/mcp` — manages local MCP server connections
- `/browser` — launches local Chrome via CDP
- `/switch-skillset` — switches local skillsets

Keep enabled in cloud mode (interact with backend or are UI-only):
- `/agent`, `/model`, `/config`, `/approvals` — remote session config
- `/goal`, `/compact`, `/status`, `/first-prompt` — session state
- `/new` — start new session
- `/undo`, `/fork` — conversation operations (sent to backend)
- `/quit`, `/exit` — exit TUI
- `/login`, `/logout` — authentication

Disabled in cloud mode (added during implementation):
- `/resume`, `/resume-viewonly` — these pick from local transcripts which don't exist for cloud sessions

### Additional Gating Needed
1. **Auto-worktree setup** (`lib.rs:217-277`): skip when `cloud_connection.is_some()`
2. **Deferred spawn for skillset_per_session** (`App::run` lines 328-334): force `needs_deferred_spawn = false` in cloud mode
3. **Worktree warning** in App: suppress in cloud mode
2 changes: 1 addition & 1 deletion nori-rs/acp/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Key files:

- `registry.rs` - Agent configuration and npm package detection
- `connection/` - SACP v11-based agent communication (local subprocess and remote WebSocket)
- `broker/` - Cloud session broker client: OAuth auth, JWT management, session acquisition/release, and `CloudConnectionInfo` (see `@/nori-rs/acp/src/broker/docs.md`)
- `broker/` - Cloud session broker client: OAuth auth, JWT management, session listing/acquisition/resume/release, and `CloudConnectionInfo` (see `@/nori-rs/acp/src/broker/docs.md`)
- `translator.rs` - User input to ACP `ContentBlock` conversion and related parsing helpers
- `backend/mod.rs` - Implements `ConversationClient` trait from codex-core and emits normalized ACP session events
- `backend/thread_goal.rs` - Owns per-session `/goal` state, prompt goal-context formatting, transcript rehydration, and usage checkpoint updates
Expand Down
20 changes: 16 additions & 4 deletions nori-rs/acp/src/broker/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Path: @/nori-rs/acp/src/broker
### Overview

- Implements the client-side integration with the nori-sessions broker for cloud VM sessions
- Manages OAuth browser-based authentication, JWT credential persistence, session acquisition, and session release
- Manages OAuth browser-based authentication, JWT credential persistence, and the full session lifecycle: list, acquire, resume, and release
- Defines `CloudConnectionInfo`, the value type whose presence signals cloud mode throughout the codebase

### How it fits into the larger codebase
Expand All @@ -14,7 +14,13 @@ Path: @/nori-rs/acp/src/broker
nori cloud (CLI) nori-sessions broker (external)
| |
v |
BrokerClient ----HTTP POST /api/sessions/acquire---->|
BrokerClient ----GET /api/sessions------------>| (list existing sessions)
| |
v |
|--- user picks session or new --------->|
| |
+----POST /api/sessions/acquire--------->| (new session)
+----POST /api/sessions/{id}/resume----->| (resume existing)
| |
v v
CloudConnectionInfo { ws_url, auth_token } SessionInfo { session_id, ws_url }
Expand All @@ -36,7 +42,8 @@ BrokerClient ----HTTP POST /api/sessions/{id}/release---->|
(best-effort, 5s timeout, called from CLI after run_main)
```

- The CLI (`@/nori-rs/cli/src/main.rs`) is the sole caller of `BrokerClient` -- it authenticates, acquires a session, constructs the `CloudConnectionInfo` that flows downstream, and releases the session after the TUI exits
- The CLI (`@/nori-rs/cli/src/main.rs`) is the sole caller of `BrokerClient` -- it authenticates, lists sessions, acquires or resumes a session, constructs the `CloudConnectionInfo` that flows downstream, and releases the session after the TUI exits
- The session selection UI (`@/nori-rs/cli/src/cloud.rs`) formats listed sessions and parses user input before the TUI launches; this is a pre-TUI step because the TUI needs a WebSocket URL to start
- `CloudConnectionInfo` is threaded through the TUI layer (`Cli` -> `App` -> `ChatWidgetInit` -> `spawn_agent()` or `spawn_acp_agent_resume()`) without modification; the TUI does not interact with the broker directly
- The ACP backend (`@/nori-rs/acp/src/backend/spawn_and_relay.rs`) branches on `config.cloud_connection` in `spawn()`: when present, it calls `SacpConnection::connect_remote()` instead of `SacpConnection::spawn()`. `resume_session()` does not have a cloud path -- cloud sessions are not recorded locally, so they never appear in the resume pickers
- The WebSocket transport adapter (`@/nori-rs/acp/src/connection/ws_transport.rs`) is the component that actually opens the WebSocket connection using the `ws_url` and `auth_token` from `CloudConnectionInfo`
Expand All @@ -45,10 +52,13 @@ BrokerClient ----HTTP POST /api/sessions/{id}/release---->|
### Core Implementation

- `CloudConnectionInfo` is a plain struct with `ws_url: String` and `auth_token: String`. It is the branch condition throughout the system -- `Option<CloudConnectionInfo>` being `Some` means cloud mode, `None` means local subprocess mode
- `CloudSessionSummary` is the listing type returned by `list_sessions()`, carrying `session_id`, `source` (e.g. "cli", "slack", "discord"), `created_at`, `last_active_at`, optional `first_message_preview`, and `status`. The `source` field enables the CLI session picker to show where each session originated
- `BrokerClient::new()` loads persisted credentials from `{nori_home}/cloud-auth.json` and filters them to match the current `broker_url`. If the stored credentials are for a different broker, they are ignored
- `BrokerClient::has_valid_token()` checks whether the stored JWT is present and not expired, using `is_token_expired()` which decodes the base64url JWT payload and compares the `exp` claim against the current system time
- `BrokerClient::authenticate()` runs an OAuth browser flow: it binds a local HTTP server on an ephemeral port, prints the broker's `/api/auth/cli?redirect_uri=...` URL to stderr, opens that URL in the default browser, waits up to 2 minutes for a callback with a `?token=` query parameter, and persists the credentials via `save_credentials()`. Printing the URL before opening it keeps `nori cloud` usable from SSH or other headless shells. The 2-minute timeout prevents the CLI from hanging indefinitely if the user abandons the browser flow; on timeout, `server.unblock()` is called to shut down the callback listener thread cleanly
- `BrokerClient::list_sessions()` GETs `{broker_url}/api/sessions` with a Bearer token, returning `Vec<CloudSessionSummary>`. This enables the CLI to show the user their existing sessions before deciding whether to create a new one or resume. HTTP 401 maps to `BrokerError::TokenExpired`; non-success responses map to `BrokerError::ListFailed`
- `BrokerClient::acquire_session()` POSTs to `{broker_url}/api/sessions/acquire` with a Bearer token and a JSON body `{"source": "cli"}`, returning a `SessionInfo` containing `session_id` and `ws_url`. The `source` field identifies this as a CLI client so the broker uses the correct claim identity format. HTTP 401 responses map to `BrokerError::TokenExpired`
- `BrokerClient::resume_session()` POSTs to `{broker_url}/api/sessions/{session_id}/resume` with a Bearer token, returning a `SessionInfo` with the reconnection `ws_url`. This allows resuming sessions originally started from any source (CLI, Slack, Discord). HTTP 401 maps to `BrokerError::TokenExpired`; non-success responses map to `BrokerError::ResumeFailed`
- `BrokerClient::release_session()` POSTs to `{broker_url}/api/sessions/{session_id}/release` with a Bearer token to explicitly release a cloud session. Called by the CLI as a best-effort cleanup after the TUI exits (wrapped in a 5-second timeout). HTTP 401 maps to `TokenExpired`; non-success responses map to `BrokerError::ReleaseFailed`
- `CloudCredentials` is the serialized form persisted to `cloud-auth.json`; it pairs `broker_url` with `auth_token` so credentials for different brokers do not collide

Expand All @@ -57,7 +67,9 @@ BrokerClient ----HTTP POST /api/sessions/{id}/release---->|
- The authentication callback server runs in a separate `std::thread` (not a tokio task) because `tiny_http::Server` is synchronous. The server is wrapped in `Arc` so both the callback thread and the timeout handler can call `unblock()` to shut it down. Communication with the async caller uses a `tokio::sync::oneshot` channel, wrapped in a `tokio::time::timeout` for the 2-minute deadline
- JWT expiry checking (`is_token_expired()`) is deliberately lenient: any token that cannot be decoded as a three-part base64url JWT with a valid `exp` claim is treated as expired. This avoids storing invalid tokens
- The `auth_token()` accessor on `BrokerClient` is used by the CLI to extract the token for constructing `CloudConnectionInfo` after session acquisition -- the token flows to the WebSocket connection as a Bearer auth header
- `BrokerClient` now covers the full session lifecycle: authenticate -> acquire -> release. Release is the terminal step, called from the CLI layer (not the backend) since the broker client and session ID are scoped there
- `BrokerClient` covers the full session lifecycle: authenticate -> list -> acquire/resume -> release. The CLI drives this lifecycle; the TUI and backend never call the broker directly
- All broker API methods follow the same error pattern: check for token presence, check local JWT expiry, make the HTTP request, map 401 to `TokenExpired`, and map other non-success status codes to the method-specific `BrokerError` variant (`ListFailed`, `AcquireFailed`, `ResumeFailed`, `ReleaseFailed`)
- The CLI gracefully degrades when `list_sessions()` fails: a 404 (broker does not support listing) returns an empty list, other errors log a warning and fall back to creating a new session. Non-interactive terminals skip session listing entirely and go directly to `acquire_session()`
- Cloud mode in `AcpBackend::spawn()` skips agent config lookup (`get_agent_config`) since the remote agent is already running on the cloud VM. Error messages for cloud connection failures use simple messages instead of the enhanced error categorization used for local subprocess failures. Cloud sessions also skip local transcript recording (`transcript_recorder` is left `None`) because the broker records transcripts server-side; as a result they are not persisted locally and do not surface in the local resume pickers
- The CLI uses its local cwd for cloud sessions -- the broker's SACP proxy (in the nori-sessions repo) manages the sprite-side working directory independently via `AcpTunnelManager`. The `cwd` sent in the `session/new` RPC is discarded by the broker. This means client-side file handlers, transcript recording, and TUI display all reflect the user's local directory, not the sprite's workspace path

Expand Down
Loading
Loading