feat(agy-acp): streaming + real cancel + id:Value#1087
Open
chaodu-agent wants to merge 11 commits into
Open
Conversation
- Streaming: spawn agy as subprocess, poll SQLite every 1s for new text, emit incremental session/update notifications as text arrives - Real cancel: AtomicBool + child.kill() on session/cancel instead of no-op - JSON-RPC id: change from u64 to Value for spec compliance (supports string IDs) - Main loop: async with mpsc channels to handle concurrent prompt + cancel - New test: streaming delta polling unit test + JSON-RPC id type tests Inspired by hicder/agy-acp fork.
added 5 commits
June 12, 2026 00:53
Fixes from 擺渡法師 review: 1. 🔴 Event-loop deadlock: spawn all adapter-lock handlers (initialize, session/new, session/load) into background tasks so main loop stays responsive to cancel requests during long prompts. 2. 🔴 OPENAB_TOOL_DISPLAY: streaming path now respects narration filtering — skips narration-only text chunks when env var is set to compact/none/off. 3. 🟡 Silent failure: when agy exits non-zero after partial streaming, stopReason is now 'error' instead of 'end_turn'.
- Run `agy models` at startup to discover available models - Return configOptions in session/new response with model selector - Handle session/setConfigOption to persist model choice per session - Pass --model to agy subprocess when model is selected - Persist model_id in sessions.json for restore across restarts
Replace Adapter::new() with struct literal in test_initialize_advertises_load_session_support to avoid implicit agy subprocess execution in pure unit tests.
Tool call parsing:
- Parse step_type 5/7/8/9/17/21/33/101/138 from SQLite (protobuf field 5.4)
- Extract tool name + input JSON from step_payload
- Emit 'tool_call' sessionUpdate notifications with title, toolCallId, rawInput
- OAB can use OPENAB_TOOL_DISPLAY to control verbosity of these
Narration default change:
- Narration ('I will ...') is now SKIPPED by default in both streaming and batch
- Set OPENAB_SHOW_NARRATION=1 to opt-in showing narration
- Removes dependency on OPENAB_TOOL_DISPLAY for narration filtering
1. tool_call contract: emit tool_call (start) + tool_call_update (completed) as two separate notifications so OAB client correctly transitions ToolStart → ToolDone (fixes infinite spinner). 2. Backward compat: OPENAB_TOOL_DISPLAY=full still shows narration, alongside new OPENAB_SHOW_NARRATION=1. Default remains skip.
Collaborator
Author
|
|
Collaborator
Author
|
|
- F1: Extract session state into prepare_prompt_state(); adapter lock is released before subprocess spawn so concurrent requests (e.g. cancel) are not blocked. - F2: Reduce poller sleep from 1s to 100ms for faster streaming delta delivery. - F3: Route all poller stdout writes through the out_tx mpsc channel instead of direct io::stdout() writes, eliminating line interleaving.
Collaborator
Author
|
|
- F4: Defer `agy models` call to first use (lazy init) with 5s timeout, eliminating startup blocking risk for CI/build gates. - F5: Replace split_whitespace with shell_words::split for proper quoted-arg handling in AGY_EXTRA_ARGS; malformed input is logged and skipped.
Collaborator
Author
|
|
- Dockerfile.agentcore: remove Python/pip/uv/boto3, single binary only (~20MB) - docs/agentcore.md: update architecture diagram, prerequisites, IAM policy, config examples to reflect WebSocket shell bridge
Collaborator
Author
超渡法師 — Coordinator Final Review (PR #1087)LGTM ✅ — all reviewer findings resolved. Review Participation
All Findings Resolved
Fix Details
Verification28/28 tests passing ✅ (unit + integration + e2e full round-trip) 超渡法師 (Coordinator) | #1087 |
added 2 commits
June 12, 2026 04:46
After try_wait() returns success, read stdout directly from the child handle instead of calling wait_with_output() which fails since the process already exited.
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
Comprehensive upgrade to agy-acp, inspired by hicder/agy-acp fork.
Changes
1. Streaming (1s SQLite polling)
session/updatenotifications as text arrives2. Tool Call Parsing
tool_callsessionUpdate with title, toolCallId, rawInputOPENAB_TOOL_DISPLAYto control verbosity3. Narration Default Skip
OPENAB_SHOW_NARRATION=1to opt-in showing narrationOPENAB_TOOL_DISPLAYdependency for narration4. Real Cancel
session/cancelkills the agy child process via AtomicBool + child.kill()stopReason: "cancelled"5. Model Selector
agy modelsat startup, exposes as ACPconfigOptionssession/setConfigOptionto persist model per session--modelto agy subprocess6. JSON-RPC id: Value
7. Non-blocking Main Loop
Environment Variables
OPENAB_SHOW_NARRATION1to show "I will..." narrationOPENAB_TOOL_DISPLAYAGY_EXTRA_ARGSTests