fix(agent-adapters): robustness fixes to pi + claude ACP adapters#134
Closed
NathanFlurry wants to merge 1 commit into
Closed
Conversation
Five verified fixes to the Pi and Claude ACP agent adapters (found via a
multi-agent review of all adapters), each with a unit test:
- claude: the tool-permission handler no longer auto-resolves on a 2s timer
(fail-open: the guest's tool ran without host consent, and a real reject
arriving after 2s was discarded). The host's requestPermission handler is now
authoritative; an unanswered request fails via the bounded ACP method timeout.
- claude: partial tool-input (input_json_delta) is now attributed by the
streaming content-block index, not Map insertion order — a text/thinking block
before a tool_use no longer mis-attributes or drops live rawInput.
- claude: a dead query reader now marks the session closed in consume()'s
finally, so the next prompt() fails fast instead of hanging to the ACP timeout.
- pi: the live session subscription is stored and torn down on session replace
and on conn.closed (no SDK close hook exists); editSnapshots is cleared per
turn + on cancel so it can't grow unbounded.
- pi + claude: a failed session/update write is logged to stderr (the
onAgentStderr channel) instead of being silently swallowed, while keeping the
emit chain alive.
Tests: registry/agent/{claude,pi}/tests/adapter.test.mjs (claude 6, pi 4; pi
test infra added). No auto-resolve-on-timeout remains in any adapter.
Member
Author
|
Stack for rivet-dev/secure-exec
Get stack: |
|
🚅 Deployed to the secure-exec-pr-134 environment in rivet-frontend
🚅 Deployed to the secure-exec-pr-134 environment in secure-exec
|
Member
Author
|
Superseded by #136 (clean re-extraction onto current main). |
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
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.
Five verified fixes to the Pi and Claude ACP agent adapters (found via a
multi-agent review of all adapters), each with a unit test:
(fail-open: the guest's tool ran without host consent, and a real reject
arriving after 2s was discarded). The host's requestPermission handler is now
authoritative; an unanswered request fails via the bounded ACP method timeout.
streaming content-block index, not Map insertion order — a text/thinking block
before a tool_use no longer mis-attributes or drops live rawInput.
finally, so the next prompt() fails fast instead of hanging to the ACP timeout.
and on conn.closed (no SDK close hook exists); editSnapshots is cleared per
turn + on cancel so it can't grow unbounded.
onAgentStderr channel) instead of being silently swallowed, while keeping the
emit chain alive.
Tests: registry/agent/{claude,pi}/tests/adapter.test.mjs (claude 6, pi 4; pi
test infra added). No auto-resolve-on-timeout remains in any adapter.