prototype: nested ACP form elicitation, replay, and a Cursor transport - #1
Draft
lnittman wants to merge 12 commits into
Draft
prototype: nested ACP form elicitation, replay, and a Cursor transport#1lnittman wants to merge 12 commits into
lnittman wants to merge 12 commits into
Conversation
…ansport Exploratory work behind the design discussion on aaif-goose#11346. Not proposed for upstream: that issue has not reached Ready, and this branch deliberately spans three separable concerns, only the first of which is in its scope. 1. Nested ACP form relay. A form request from a managed ACP provider reaches the outer client and its typed response returns to the originating provider, under both the legacy loop and the state machine. The request is persisted before the response is accepted so an answer cannot sort ahead of its own question, and liveness check, persistence, and waiter consumption are one operation so two clients cannot both persist an answer while only one wins the channel. 2. Session-load replay. A non-goal of aaif-goose#11346 and explicitly deferred by aaif-goose#9797. Replays an unanswered question on load and distinguishes a live waiter from a dead one so a client can continue honestly. 3. Cursor ACP transport. A non-goal of aaif-goose#11346, and against the direction settled in aaif-goose#8391. Kept only because it is what surfaced the cursor/ask_question schema details reported there.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
lnittman
marked this pull request as draft
August 20, 2026 02:10
The comment claimed liveness, persistence, and waiter consumption were one operation. The lock only serializes concurrent submitters; for a provider-owned request the waiter is still not reserved across the append, so a stream dropping in between can leave an answered transcript row the nested agent never received. Say so, and name the claim shape that would close it.
Answering a nested form checked that the waiter was live, appended the response to the conversation, and only then delivered it. A stream dropping between the check and the send left an accepted answer recorded in history that the originating agent never received — and replay reads that response id as proof the question was answered, so it is not asked again. Claiming takes the waiter out of the provider's pending set, so nothing can cancel or consume it while the response is written. A claim is refused when the receiver has already closed, which is what stops a response being recorded against a request nothing is left to receive. If the append fails the waiter is released unanswered; a stream that drops while a claim is held releases it too, rather than leaving it hanging. This replaces the liveness probe on the response path — claiming subsumes it — and follows the shape ActionRequiredManager already uses for MCP-originated elicitations.
The parity tests drive a stub provider whose elicitation methods stood in for a real one. Claiming now gates the response path, so the stub has to reserve its waiter the same way — otherwise both concurrent submitters are refused and the test proves the opposite of what it asserts.
Only advertise ACP form elicitation to nested agents when the outer host explicitly negotiated it. This lets nested agents use their prose fallback instead of receiving a misleading cancellation from form-incapable clients.
Carry the nested tool call ID and ACP metadata through the persisted action-required record and back out to the outer client. This preserves forward-compatible relay context while retaining the Goose elicitation ID for response correlation.
Handle deterministic session-description requests before consulting the selected Cursor transport. This keeps title generation local even when the conversation transport is pinned to ACP or unavailable.
Prepare the provider before either agent loop resolves context or other transport-sensitive behavior. This prevents Cursor first turns from being configured with direct-provider semantics and then executed on ACP.
Propagate provider session preparation failures from both agent loops instead of logging a handoff that never occurs. A failed saved-session resume now stops before any provider stream can run.
The elicitation relay now merges incoming metadata directly, leaving the no-steer wrapper without production callers. Remove the dead wrapper while retaining its serialization assertion against the underlying helper.
Preserving the nested request's `toolCallId` and `_meta` through the relay added two fields to `ActionRequiredData::Elicitation`, which breaks every exhaustive pattern over it. The CLI's extractor was one, so the workspace stopped building even though `-p goose --lib` stayed green. The CLI renders the prompt itself and has no use for relay correlation fields, so it ignores them explicitly rather than threading them through.
Honouring the outer client's form capability gave the provider factory a `ProviderHostCapabilities` parameter and `AcpProviderConfig` a `supports_form_elicitation` field. Both are the right shape, and both ripple into every fixture that constructs them. Seven call sites in `crates/goose/tests/` still used the old arity, so the workspace did not build even though `-p goose --lib` stayed green. The integration fixtures now match, and the host stub advertises form support because that is what it stands in for.
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.
Important
This is an exploratory branch in Luke's fork. It is not a pull request
against
aaif-goose/gooseand is not proposed for upstream.aaif-goose/goose#11346 has
not reached Ready on the Goose Issues board, and Goose's contribution guide
asks that implementation wait for Ready. Whatever ships will be re-derived
against the design that issue settles on.
What this is
A working prototype used to find the real constraints in nested ACP form
elicitation, so the design discussion on aaif-goose#11346 could be grounded in something
that actually runs. This branch is the evidence, not a proposed patch; the
findings it produced are being written up for that issue rather than argued
through this diff.
Scope honesty
The branch deliberately spans three separable concerns. Only the first is in
aaif-goose#11346's scope; the other two are that issue's stated non-goals and would each
need their own issue.
ACP provider reaches the outer client and its typed response returns to the
originating provider, under both the legacy loop and
GOOSE_STATE_MACHINE=1.feat: implement acp method for elicitation and elicitation improvement aaif-goose/goose#9797. Replays an unanswered question on session load and distinguishes a live
waiter from a dead one so a client can continue honestly rather than pretend
to reattach a dead responder.
direction settled in feat: Cursor ACP Provider aaif-goose/goose#8391, where per-vendor ACP wrappers were declined in
favour of a general configuration-driven ACP provider. It is kept here only
because it is what surfaced the
cursor/ask_questionschema details reportedon that issue.
Known issues
Since the first push, the relay claims its waiter before persisting a response,
outer form capability reaches the nested agent's initialization, the nested
toolCallIdand_metasurvive forwarding, and the Cursor transport isselected before any capability-sensitive work with session titles kept out of
the user's real conversation and resume failure failing closed.
What is still open, stated plainly:
persist-versus-deliver invariant. A stream dropped during the append still
drains the claimed set, and the receiver can close between the claim and the
send, so a persisted-undelivered response remains reachable. Closing it needs
an owned, run-scoped claim and a durable delivery state, not more booleans.
written, so an unconvertible value persists an answer that is never delivered.
secondary stream's guard can cancel a question created by a different run.
leaves the outer form on screen until the user acts.
terminal ones like invalid params or method-not-found.
client can answer a replayed form successfully without the agent continuing.
prepare_sessionand the elicitation methods still sit on the genericProvidertrait. A relay/interaction manager owned by the agent core would bethe better home.
ACP.
ProviderMetadatadeclares the defaultautoand thecomposer-*listthe CLI accepts, but Cursor's ACP exposes different values —
default[],grok-4.6[effort=high,fast=true]. Setting the declared default is rejectedwith
Invalid model value: auto, the ACP session fails, and the providerfalls back to the direct CLI path, which is prose-only. The transport must
take its model vocabulary and default from the ACP session rather than from
the CLI constants. This is the clearest evidence so far that a per-vendor
wrapper reintroduces exactly the coupling feat: Cursor ACP Provider aaif-goose/goose#8391 declined.
option.
extract_model_info_from_config_optionskeepsoption.valueanddrops
option.name, andfetch_supported_modelsreturnsVec<String>, so aclient can only display the opaque id. This predates the branch, but the
Cursor transport is what makes it visible.
Verification
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warningscargo build -p goose-cli --bin goosecargo test -p goose --lib: elicitation 12,acp::provider84, cursor 19 —115 passed, 0 failed, including an in-process duplex-transport test that
drives a real nested ACP agent rather than injecting the internal update
Seven failures elsewhere in
cargo test -p goose(gcpauth, JWT, catalog_util,an insta snapshot, and
turn_context_is_persisted) reproduce unchanged onupstream/mainand are not from this branch.Refs aaif-goose#11346