feat(session-port): route claude/codex through the direct-CLI SessionAgentTask#609
Open
BlackKeyZ wants to merge 28 commits into
Open
feat(session-port): route claude/codex through the direct-CLI SessionAgentTask#609BlackKeyZ wants to merge 28 commits into
BlackKeyZ wants to merge 28 commits into
Conversation
…shed) Squashed port of the clean-slate session model onto the origin/main baseline: claude/codex backends run through SessionAgentTask (IAgentTask adapter over SessionBackend), with event-pump, resume/self-heal, permission recovery, usage/mode/model handling, and codex tool-output + plan streaming. Standalone test files are kept out of tree by intent. Rolled-up commits: - chore(session-port): land aionui-session + aionui-process on origin baseline (phase 0) - feat(session-port): SessionAgentTask adapter (SessionBackend → IAgentTask) [phase 1a] - feat(session-port): add AgentInstance::Session variant + enum-level arms [phase 1b-1] - feat(session-port): route claude/codex through SessionAgentTask in the factory [phase 1b-2] - session-port: close build_session_instance enrichment gaps (resume/mode/mcp/preset/env/sandbox/catalog) - session-port: wire event-pump gaps the ACP path had (persist anchor/config, permission, usage) - session-port: port clean-slate's session-mapping tests + cover pump persistence - session-port: align Start/Finish frames with the ACP path (live A/B verified) - session-port: fix usage payload shape + suppress opening config frame (A/B) - session-port: render AskUserQuestion as its own options, not a generic allow/deny card - session-port: recover pending permissions via get_confirmations (was empty → turn hang) - session-port: stop mode/model switch from lighting a spurious timer bar - session-port: forward codex Plan to the frontend (was dropped) - session-port: fix mode/model switch command_ack error + suppress cancel error noise - session-port: stream codex tool output (ToolOutputDelta) live to the frontend - session-port: terminate turn errors as the rich AgentStreamEvent::Error, not a plain Tips - session-port: normalize mode alias on resume (was raw → wrong/rejected mode) - session(claude): disable AskUserQuestion until frontend has a multi-question renderer - fix(db): bump conversations.updated_at on message persist so recent activity floats to top - session-port: suppress non-blocking Workflow launch result via SegmentBreak - session-port: reap child CLI on task drop + self-heal dead resume anchor - fix(session): preserve tool name across a call's follow-up frames
…t catalog Three related first-turn robustness fixes for fresh sessions: - codex: stop binding the persisted model at thread/start. A stale frontend picker default (e.g. gpt-5.5) not present on the local codex poisoned the whole thread, failing every turn with UNKNOWN_UPSTREAM_ERROR. Now thread/start carries no model; after handshake, spawn a reconcile task that learns the real catalog from model/list, drops the desire if invalid, and applies a valid one via SetModel — a faithful port of the ACP clear_invalid_desired_model + separate set_model RPC path. - claude: validate config.mode against CLAUDE_PERMISSION_MODE_IDS before passing it as --permission-mode. An unknown mode makes claude spawn exit 1 (hard crash); we now warn and fall back to "default". - Cargo.lock: sync workspace version bump and aion-config dependency.
Port the ACP reconcile mechanisms (clear_invalid_desired_* + validated
apply) to the direct-CLI SessionBackend path:
- codex mode reconcile: apply config.mode via a validated SetMode after
collaborationMode/list fills the catalog (codex has no thread/start mode
param, so mode was never applied at open before). Sequenced after the
model reconcile since SetMode needs a settled current_model. Drop
(WARN) if not in catalog; empty catalog = permissive.
- claude effort validation: validate SetConfigOption{effort} against the
current model's reasoning_efforts before sending; reject unsupported
levels instead of poisoning current_effort. Empty catalog = permissive.
- runtime invalid mode/model switch: session_agent set_config_option now
validates mode/model against the advertised catalog before dispatch and
returns BadRequest (reject + report) rather than silently dropping.
- effort persistence: persist the chosen effort into config_selections
(claude emits no ConfigChanged for effort) and re-apply it after open,
so it survives respawn/resume.
The direct-CLI session-port path (claude/codex via SessionAgentTask) let a
clean "blank reply" turn — the model reached a normal terminal without
emitting any user-visible output — finish silently, leaving the user with an
empty bubble. The legacy ACP path already surfaces a diagnostic tip for this
(ELECTRON-1JG) via is_empty_turn + prompt_outcome_from_stop_reason; the
session-port pump did not.
Mirror that behavior off the already-normalized TurnOutcome (no stderr peek,
no watchdog, no timeout): track a per-turn saw_visible_output flag in the
event pump, and on a clean TurnResult (is_error:false, not cancelled, not a
suppressed workflow-launch) with no visible output, emit AgentStreamEvent::Tips
with a code mapped 1:1 to the ACP codes — EndTurn -> ACP_EMPTY_TURN (info);
MaxTokens/MaxTurns/Refused -> ACP_EMPTY_TURN_{MAX_TOKENS,MAX_TURN_REQUESTS,
REFUSAL} (warning); other truncation kinds and Failed -> generic warning;
Cancelled -> no tip. The Tips is emitted immediately BEFORE the Finish because
the relay breaks the turn on Finish, so a tip after it would be dropped.
Cargo.lock: sync aionui-process/aionui-session to the workspace 0.1.43 version.
claude/codex resolve their selectable model/mode/slash-command catalog
asynchronously (~6s after session open), unlike ACP which carries it in
the NewSessionResponse body. The direct-CLI shell filled the capability
cache on arrival but never pushed it upward, so the top-right model
picker stayed empty until a manual reload.
Add a CatalogUpdated session event emitted when the catalog resolves
(claude control_request{initialize} response; codex model/list +
collaborationMode/list). SessionAgentTask projects it to an
acp_config_option frame so the frontend re-fetches config-options and
fills the picker. Marked Ephemeral (re-discovered on open, not history).
…lent to legacy ACP Codex (Plan B): backend translates the colon profile ids (:read-only/:workspace/:danger-full-access) to the legacy bare tokens (read-only/auto/full-access) on the way out and back on the way in, so the frontend receives byte-identical values and needs zero change; custom profiles keep their colon id. Fresh sessions seed current_mode=auto. Claude: claude_permission_modes() now reproduces the legacy bridge buildAvailableModes verbatim (default, acceptEdits, plan, dontAsk, bypassPermissions). auto is omitted because the bridge gates it on supportsAutoMode, which the direct CLI never reports; the validation whitelist still accepts auto (superset) so a resumed session carrying it does not crash the spawn. bypassPermissions is always advertised.
…ct terminal The direct-CLI bridge collapsed every SessionEvent::Detached to a plain Finish, so a CLI that died mid-reply rendered as a normal empty completion instead of the legacy ACP path's UserAgentDisconnected error card. Track terminal_result_seen in the event pump (mirroring the reducer's Running flag) and route Detached through aionui_session::crash_outcome so a genuine mid-turn crash (signal / non-zero / unknown exit, no prior result) surfaces as AgentStreamEvent::Error via the same AcpError::Disconnected classifier legacy used, with the allowlisted redacted_summary on the message. Absorbed teardowns (Detached after a terminal result) and clean exit-0 blank turns still end with a plain Finish. Adds three unit tests covering crash, post-terminal teardown, and clean exit-0.
Forbid inferring any agent CLI's wire protocol / message shapes / field semantics / timing / defaults from names, mental models, or training knowledge. Every such claim must cite an approved source by path (captured samples, the ACP library, or an official adapter / machine-generated schema). A claim with no citation is a guess and violates the rule.
The session-port direct-CLI path (SessionAgentTask) served the model/mode picker purely off the backend's live capabilities(), which is empty until the initialize round-trip lands (~seconds on resume). So reopening a history conversation showed a blank model list until the async catalog arrived — the ACP path avoids this via preload_metadata_catalogs reading the persisted agent_metadata handshake. Mirror that on the session path: parse the persisted handshake into a CatalogPreload (reusing the ACP extract_models_from_value/extract_modes_from_value multi-shape parser) and serve it as a per-axis fallback in get_model / get_config_options / mode when live capabilities are still empty. Live catalog overwrites the preload the instant it is present (fill-when-empty, live-overwrites), matching the ACP semantics — so a stale persisted catalog never masks a fresh engine's model list. The write side (spawn_catalog_writeback) was already wired; this connects the missing read side.
…icker effective_catalog gated the current model/mode fallback on the LIST being empty: pre-init (live capabilities not yet discovered) it served the persisted-handshake preload's current, which is frozen at the prior session's catalog write-back and does not reflect a mid-turn model switch. The backend, however, seeds caps.current_model/current_mode from the resolved snapshot at spawn (the persisted current_model_id column) — already the user's last interactive selection, and the model claude actually runs. Decouple the per-axis fallback: the LIST still prefers live-then-preload, but the CURRENT prefers caps (snapshot-seeded) then preload. This removes the pre-init window where the picker briefly showed a stale model after the user switched models last session, aligning it with the ACP path (which seeds current_model into the session at construction). Adds caps_current_wins_over_stale_preload_while_list_is_empty covering the "current seeded, list still empty" case the old list-gated logic could not distinguish.
The claude reader's unbounded read (post-first-frame) relied solely on stdout EOF (Ok(0)) or a read error to terminate a turn. On Windows the claude stdout write handle can be inherited by a surviving grandchild (a detached MCP/tool descendant); killing the claude leaf then leaves the pipe's write end open, so stdout.read() never returns 0 — the reader parks forever, Detached never fires, and the UI wedges at pending with no error. (macOS fds are close-on-exec, so EOF is prompt and the reader terminates normally.) Race the unbounded read against the process's exit watch (io.wait_for_exit(), a cancel-safe watch::Receiver over the direct child's child.wait(), orthogonal to the stdout pipe). When the exit leg wins, bounded-drain any buffered tail (EOF may never come, so we cannot wait for Ok(0)) via the same process_batch + panic net, then break to the existing terminal path reusing the captured exit status (no re-await of wait_for_exit). Scope: claude_conn only; codex/acp connections are persistent ACP whose stdout does not EOF between turns and must not adopt this. The startup handshake-budget read, the hung/panicked (exit:None) paths, and flush_tail/R1a are unchanged. biased select prefers the read leg so a turn that emits then exits still delivers its result frame before terminating. Adds process_exit_without_eof_surfaces_terminal_detached (mutation-proven: a bare read hangs it) — the mirror of the long-silent-turn-not-killed test, pinning terminate-on-real-exit vs never-on-mere-silence.
The session-port direct-CLI path (SessionAgentTask) discovered per-model reasoning-effort levels in its backend (Capabilities.current_effort + ModelInfo.reasoning_efforts) but never forwarded them to the frontend as a config option, so the top-right effort/thinking picker silently disappeared for claude/codex on session-port — a parity regression vs the legacy ACP path. Surface the effort levels as a `reasoning_effort` config option (category `thought_level`) in both the REST getter (get_config_options) and the streaming CatalogUpdated push, resolving levels for the current model (union fallback when no current model is set). Because claude emits no ConfigChanged/echo for effort, track the picker highlight through a new effort_override cell and let set_config_option fall through to an observed re-read matched by category.
claude advertises its slash-command list in the same async `initialize` response that carries the model/mode catalog — long after the frontend's mount-time REST read of the command list returns empty. The legacy ACP path recovers by emitting a live `AvailableCommands` frame from its `AvailableCommandsUpdate` handler, but the session-port pump destructured `CatalogUpdated.slash_commands` as `_` and dropped it, so the `/` menu stayed empty for claude/codex until a manual refetch — the same late-catalog parity regression class as the model/mode/effort pickers. Emit `AgentStreamEvent::AvailableCommands` alongside the config-option frame on `CatalogUpdated`, guarded so an empty list never sends a spurious frame that would clobber a REST-loaded menu.
…update
Setting reasoning effort for a codex conversation 400'd with "command not
supported by this backend: set_config_option": codex_conn's dispatch rejected
every Command::SetConfigOption unconditionally, on the premise (stale comment)
that codex has no standalone effort wire and effort can only ride SetMode's
collaborationMode. The generated schema contradicts that — ThreadSettingsUpdate-
Params exposes a first-class `effort` field ("Override the reasoning effort for
subsequent turns" -> ReasoningEffort), and codex_conn already speaks that exact
wire for SetModel/SetMode.
Route the effort option ids (effort/reasoning_effort/thought_level) through
thread/settings/update{threadId, effort}, mirroring SetModel: register the rpc
id so a rejection surfaces as a Notice, pass the value verbatim (it is one of the
model's advertised supportedReasoningEfforts; codex validates on the response).
Any other generic config option still rejects. This makes codex symmetric with
claude so the effort picker's set path succeeds.
verified: samples/codex-cli/0.137.0/schema-full/ClientRequest.json
(ThreadSettingsUpdateParams.effort, ReasoningEffort enum)
Also fixes a pre-existing newer-clippy single_match lint in a claude_conn test
(unrelated to the change but in the same crate, blocking its clippy gate).
…models
Inject a synthetic "ultracode" reasoning-effort level into an xhigh-capable
model's advertised efforts (mirroring the claude CLI's own effort-picker entry
"ultracode: xhigh + dynamic workflow orchestration"), so it flows through both
config-option picker builds and passes effort_is_supported like a real level.
Unlike real levels it does NOT ride the effortLevel field: on dispatch it emits
the dedicated boolean apply_flag_settings{settings:{ultracode:true}}, which the
CLI auto-forces to xhigh. Sending effortLevel:"ultracode" would be rejected by
our own effort_is_supported gate since ultracode is absent from the model's
supportedEffortLevels catalog.
Gated to xhigh-capable models only, matching the CLI gate (ultracode requires an
xhigh-capable model + dynamic workflows). codex has a separate effort parse and
never gains the level, so this is claude-scoped by construction.
Wire LIVE-PROBED against claude-cli 2.1.206 (candidate a returns
control_response{success}; get_settings.applied reads back {effort:"xhigh",
ultracode:true}) — see protocols/samples/claude-cli/2.1.206/ultracode_wire.result.md.
Mechanical rustfmt-only reflow (tuple + block-expression wrapping) on lines introduced by the recent effort/slash/codex-effort commits; no logic change. Restores a clean cargo fmt --all --check for the push gate.
…gement #588) into feat/session-port
…only first-turn write, dropped Notice, codex confirmation recovery) Every silent capability loss at the session-port seam (backend supports X but the Session path drops it) is a bug. Three found and fixed; all reachable only via claude/codex (the sole backends routed to AgentInstance::Session). #7 codex read-only first-turn write (SECURITY). codex_sandbox_for_mode returned None for read-only, so thread/start seeded workspace-write and the FIRST turn could write; the post-open SetMode permission profile applies only to the NEXT turn. Now seed sandbox:"read-only" at thread/start so turn 1 is locked (SandboxMode verified codex-cli/0.137.0/schema-full/ClientRequest.json). sandbox and permissions ride separate wire calls, so the two axes compose. claude is unaffected: its mode is a spawn-time --permission-mode arg (fail-CLOSED), not a delayed reconcile. #2 backend Notice dropped at seam. Both backends emit SessionEvent::Notice specifically so a rejected mode/model/effort set or codex out-of-turn warning is visible; translate_event's catch-all re-dropped it. Now maps Notice → Tips (the one advisory frame the origin frontend renders), without touching turn state. #1 codex confirmation recovery. codex had no pending_permission_requests() override, so a tool/file/elicitation approval raised before subscribe or lost on reload could never be rebuilt and the turn hung. Added a transient pending_tool_approvals registry (insert on */requestApproval + elicitation, remove on serverRequest/resolved + AnswerPermission) and the override, keyed by the surfaced request_id so live+recovered de-dup. Titles carry the approval class only, never the command body (TIO-13). mod.rs trait doc corrected: ACP SessionBackend keeps such a registry but does not override (latent, unreachable). Tests: codex_sandbox_maps_full_access_and_read_only_modes, thread_start_sandbox_is_data_driven_from_config (read-only wire assert), notice_surfaces_as_tips, codex_pending_tool_approval_lists_open_then_clears_on_answer. aionui-ai-agent + aionui-session green, clippy -D clean, fmt clean.
…ude_str!
The aionui-session lib tests embed captured claude CLI transcripts via
include_str!("../../tests/fixtures/*.ndjson") (claude.rs, claude_conn.rs), but
the tests/fixtures/ directory was never tracked on this branch — it landed on
the phase-0 baseline commit that feat/session-port does not descend from. Local
builds passed because the files exist on disk; CI failed to compile aionui-session
(lib test) because the three referenced fixtures were absent from the checkout:
couldn't read tests/fixtures/claude_2.1.169_single_tool_turn.ndjson
couldn't read tests/fixtures/claude_2.1.176_workflow_multiagent_3parallel_1fail.ndjson
couldn't read tests/fixtures/claude_2.1.185_cancel_before_output_result.ndjson
Track all 11 fixtures (byte-identical to the phase-0 baseline), so the crate's
tests compile from a clean checkout. Captured wire samples only; no code change.
…ession-port #608 canonicalizes the Codex ACP full-access mode to `agent-full-access` (migration 021 rewrites builtin `agent_metadata.yolo_id`, and `normalize_requested_mode` resolves yolo aliases to it). The direct-CLI session path in this branch consumes that resolved mode downstream, so teach its three codex mode mappings to recognize the new canonical value (legacy `full-access` / `:danger-full-access` / `yoloNoSandbox` kept for pre-021 persisted data): - session_agent `codex_sandbox_for_mode` → `danger-full-access` - session_agent `codex_approval_for_mode` → `never` - codex_conn `codex_perm::normalize_to_profile_id` → `:danger-full-access` Without this a full-access codex conversation started via the direct-CLI path would silently degrade to workspace-write + on-request approval.
`SessionAgentTask::build` spawns the event pump at construction, so it starts polling the backend's `events()` stream immediately. `ScriptBackend` returned an already-ready `stream::iter`, so on a multi_thread runtime the pump could project — and drop — frames before the test's post-`new()` `subscribe()` ran (the broadcast channel keeps no pre-subscribe buffer). This flaked `catalog_updated_projects_available_commands_frame` in CI (single-event script = smallest race window) while passing locally. Fix is test-only: `GatedScriptBackend` parks its stream on a `Notify` until released, and `drain_script` subscribes BEFORE releasing the gate, so no frame can predate the subscribe. Mirrors production, where the backend event stream is subscribed at open but stays silent until the CLI emits (well after the frontend's WS subscribe). Confirmed deterministic: injecting a 200ms pre-subscribe delay (which fails the old structure) keeps all 17 pump_tests green.
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
Introduces the direct-CLI session model and routes claude and codex through it via
SessionAgentTask, talking to each CLI over its native wire instead of a third-party ACP shim. opencode/gemini/hermes remain on theAgentInstance::Acppath (unchanged).The core is the new
aionui-sessioncrate: an actor-mailboxSessionBackend/BackendConnectiontrait pair with per-backend connections (claude_conn,codex_conn, plus an ACPSessionBackend), and a neutralSessionEvent/Commandvocabulary thatSessionAgentTask(inaionui-ai-agent) translates to/from the originAgentStreamEventsurface.Capabilities
Native transports
--print stream-jsoncontrol protocol (spawn-time--permission-mode, in-bandset_permission_mode/set_model, control-request initialize for the model/slash catalog).thread/start,thread/settings/update,permissionProfiles/list, reverse-RPC approvals/elicitations).Session lifecycle
CatalogUpdatedso the model/mode pickers fill as discovery lands.Runtime controls
thread/settings/update), includingultracodeas an effort level for xhigh-capable models.Permission & approval model
Advisory & diagnostics
Notice→Tipsframes.Test plan
cargo nextest run --workspace— 7168 passed, 0 failed, 18 skipped (~475s).cargo clippy -p aionui-ai-agent -p aionui-session -- -D warnings— clean.cargo fmt --all -- --check— clean.Notes