feat(acp): source /resume picker from agent session/list#518
Merged
Conversation
When the live ACP agent advertises both `session/list` and `load_session`, the in-session `/resume` command sources its picker rows from the agent (via `session/list`) and resumes the selected session over ACP `session/load`, instead of the local transcript store. This is generic to any agent that advertises the capability. Agents that advertise `session/list` but not `load_session` fall back to the transcript-backed picker, since an agent-sourced resume carries no local transcript and depends entirely on server-side `session/load` replay -- without it the agent would silently start a blank session. `list_sessions()` drains cursor pagination under a page cap so a misbehaving agent cannot pin the picker in an unbounded loop. The standalone `nori resume` startup picker is unchanged. 🤖 Generated with [Nori](https://noriagentic.com) Co-Authored-By: Nori <contact@tilework.tech>
acp_resume_session_picker_params never used its AppEventSender argument -- each row's SelectionAction receives the sender when invoked, not at build time. Remove the dead parameter along with its call-site and test plumbing. 🤖 Generated with [Nori](https://noriagentic.com) Co-Authored-By: Nori <contact@tilework.tech>
This was referenced Jul 4, 2026
CSRessel
added a commit
that referenced
this pull request
Jul 6, 2026
…se and errors (#536) ## Summary 🤖 Generated with [Nori](https://noriagentic.com/) - **`session/resume` support**: `AcpBackend::resume_session` gains a third branch — when the agent advertises `sessionCapabilities.resume` but not `session/load` (the nori-cloud contract), reattach via `session/resume` with **no** silent `session/new` fallback (on a cloud agent that would claim a brand-new VM). Branch order: `load_session` (replay) > `session_resume` (live reattach) > client-side transcript replay. The shared MCP-registration block was deduped into one helper across the four branches. - **`/close` command**: capability-gated on `sessionCapabilities.close`; releases the live session over `session/close` and starts a fresh chat **only on success**. While the close is in flight, session-switching commands (`/new`, `/resume`, `/agent`, …) are blocked so the deferred `NewSession` can't clobber a conversation the user switched to; failures surface via `AppEvent::SessionCloseFailed` with the enhanced message and unblock the widget. - **Structured ACP error mapping**: `categorize_acp_error_chain` walks the anyhow chain for the structured `acp::Error`, maps by JSON-RPC code (`-32000` auth_required, `-32002` session-not-found, `-32010` unreachable (retryable), `-32012` not-resumable, `-32014` no-active-session, `-32015` already-active), code wins over substring heuristics, and surfaces `error.data.detail` (e.g. the deployment's exact login hint) — used by spawn/resume/close and prompt-failure paths. - **Picker fixes**: the /resume picker's agent-list gate is now `session_list && (load_session || session_resume)` — cloud sessions actually appear (PR #518's feature was gated off for the one agent that needed it). The cloud sentinel cwd `/` is hidden from row descriptions and search. - **Capabilities plumbing**: `AgentCapabilitiesView` gains serde-defaulted `session_resume`/`session_close`; mock-acp-agent gains env-gated resume/close handlers + failure switches for tests. This is the client half of `nori-cloud-cli-followup.md` (agent/router half landed in nori-sessions PR #1045). Capability-gated throughout — agents that don't advertise the new capabilities behave exactly as before. ## Decisions to review / known follow-ups - **`/close` = swap, not just release**: on success we start a new chat, which (for cloud) immediately claims a fresh VM — consistent with existing `/new` semantics, but "release and idle" isn't expressible today. Flagging the UX question. - `/close` shows in the slash popup for all agents (gated with an explanation on invoke); wiring it into the `disabled_builtin_commands` greying mechanism (like `/goal`) is a clean follow-up. - Prompt-path errors show `data.detail` as raw JSON via `{err:#}` rather than the curated treatment; cosmetic inconsistency. - The runtime `CloseSession` command arm is covered indirectly (connection tests below, widget tests above, harness close-failure test through `close_active_session`). ## Test Plan - [x] TDD: failing tests first (compile-RED for new API, runtime-RED for picker gate/resume branch), then green - [x] New coverage: structured categorization unit tests; connection resume/close round-trip + failure tests against the real mock-agent subprocess; harness resume-not-load / no-fallback / enhanced-close-failure / capabilities-projection tests; TUI picker gate (positive + negative), /close ordering + failure + gating tests; `/cl` slash-popup snapshot - [x] `cargo test` (full workspace) green; `cargo test -p tui-pty-e2e` green; `just fmt` + `just fix` clean - [x] Live TUI drive (tmux): `/resume` shows agent-sourced sessions under the cloud contract → select reattaches via `session/resume` → `/close` prints "Session closed (released)." and starts a fresh chat - [ ] Live loop against a real `nori-handroll acp --type cloud` + broker (needs Sessions auth; verified against the mock contract only) Share Nori with your team: https://www.npmjs.com/package/nori-skillsets --------- Co-authored-by: Nori <contact@tilework.tech>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
session/listandload_session, the in-session/resumecommand now sources its picker rows from the agent (via ACPsession/list) and resumes the selected session over ACPsession/load, instead of the local transcript store. This is generic to any agent advertising the capability — not Nori/Codex-specific.session/listbut notload_sessionfall back to the existing transcript-backed picker. An agent-sourced resume carries no local transcript and relies entirely on server-sidesession/loadreplay; withoutload_sessionthe agent would silently start a blank session.AcpConnection::list_sessions()drains cursor pagination internally under a page cap, so a misbehaving agent that keeps returningnext_cursorcannot pin the picker in an unbounded loop.nori resumestartup picker is unchanged (still transcript-backed).session/resume,session/close, andsession/deleteare stabilized in the ACP spec; this PR is deliberately scoped tosession/list+load_session-based resume and leaves those verbs as follow-ups.session/resumeis the natural next one: it reconnects to a listed session without replaying history (unlikesession/load), so adding it would widen thesession_list && load_sessiongate above to also cover agents that can reconnect but don't implementload_session(today those fall through to the transcript-backed picker).session/close(free an active session's resources) andsession/delete(remove fromsession/listhistory) are session-lifecycle operations, not/resumepicker extensions, and belong on their own surface. Cloud-mode gating is a separate follow-up.Test plan
cargo test -p nori-tui --lib resume— 23 passing (incl. newacp_resume_session_picker_snapshotinsta snapshot andacp_resume_session_picker_*unit tests)cargo test -p nori-acp --lib list_sessions—test_list_sessions_maps_agent_session_infopassingcargo build --bin nori && cargo build -p mock-acp-agentcargo test -p tui-pty-e2e --test acp_resume_session_list— drives the realnoribinary against the mock agent: lists the agent's two sessions and selects + resumes one (exercises theload_sessionpath end-to-end)cargo fmt+cargo clippy --all-features --tests -p nori-acp -p nori-tuicleandocs.mdupdated (acp, acp/connection, tui, mock-acp-agent)