Skip to content

feat(discord): support ACP form elicitation - #1533

Closed
gandazgul wants to merge 8 commits into
openabdev:mainfrom
gandazgul:discord-acp-form-elicitation
Closed

gandazgul wants to merge 8 commits into
openabdev:mainfrom
gandazgul:discord-acp-form-elicitation

Conversation

@gandazgul

@gandazgul gandazgul commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Discord ACP form elicitation

Summary

  • Add official ACP v1 elicitation/create form support for Discord-backed sessions.
  • Advertise clientCapabilities.elicitation.form only when a Discord form presenter is available.
  • Add a platform-neutral elicitation coordinator, Discord progressive form presentation, text-only fallback commands, lifecycle invalidation, and bounded request handling.
  • Add deterministic subprocess and behavior tests for the ACP wire contract, authorization, overload, lifecycle, and Discord presentation repair paths.
  • Update the Discord guide, canary guide, domain language, README, and plan/work-record artifacts.

Validation

Automated checks reported by the saved work record and prior execution:

  • cargo check --workspace — passed.
  • cargo clippy --workspace -- -D warnings — passed.
  • cargo test --workspace — passed.
  • cargo clippy --workspace --features unified -- -D warnings — passed.
  • cargo test --features acp — passed.
  • cargo build --features unified — passed.
  • cargo fmt --check — did not pass cleanly. The saved work record says repository-wide formatting diffs outside the feature path remained.
  • cargo check --target x86_64-pc-windows-gnu — not run successfully because the Windows target standard library was not installed.

No live Discord manual canary was run in this preparation step.

Review notes

  • Prepared with RunWield.
  • Branch: gandazgul:discord-acp-form-elicitation
  • Base: openabdev:main
  • Head commit: ccc334e1993980b1c88097809c158699a78af6e9
  • Diff size: 49 files changed, 9228 insertions, 1898 deletions.

Review Contract

Goal

This PR lets OpenAB act as an ACP Client that can receive official ACP v1 form elicitation requests from a downstream Agent Runtime during Discord turns. It must show bounded, progressive forms to the verified human users who contributed to the active turn, return exactly one ACP result, and leave unsupported adapters unchanged.

Non-goals

  • No URL elicitation support.
  • No Slack, gateway, or other non-Discord form UI.
  • No migration of the legacy session/request_input extension in openab-agent.
  • No nested JSON Schema or array support beyond string-array multi-select enums.
  • No secret-collection flow. The Discord form warns users not to enter credentials or other secrets.

Accepted Residual Risks

  • Discord cleanup after a terminal Submit, Decline, Cancel, expiry, or teardown is best effort. A stale visible message can remain if Discord edit/delete fails, but server-side nonce, revision, message, generation, and coordinator checks must prevent a second ACP response or state change.
  • Live Discord manual evidence is deferred to canary validation because this preparation used the deterministic test harness and no Discord token.
  • cargo fmt --check is not clean in the saved validation record because repository-wide formatting diffs remained outside the feature path. Reviewers should decide whether to require a separate formatting cleanup before merge.
  • Windows cross-check evidence is absent because the Windows target standard library was not installed in the execution environment.

Acceptance Criteria

  • Discord-backed ACP sessions advertise exactly clientCapabilities.elicitation.form; unsupported sessions do not advertise clientCapabilities.elicitation and never advertise URL elicitation.
  • Agent-to-client JSON-RPC requests are classified by method plus ID before outbound response correlation. Numeric and string request IDs are echoed unchanged, including same-number collisions with active OpenAB requests.
  • Valid flat ACP form schemas can be answered through Discord progressive rich controls or through text-only fallback commands. Defaults, validation, review, edit, Submit, Decline, and Cancel all work.
  • Agent-controlled form work is bounded to one pending form per connection, 64 KiB elicitation request envelope, 50 fields, 100 choices per field, and a 1 MiB inbound reader frame.
  • Only verified human senders from the exact active dispatch batch can answer. Unauthorized, wrong-channel, wrong-message, stale-generation, stale-revision, duplicate, and post-terminal inputs are harmless.
  • Pool cancel, cancel-all, reset, prompt deadline, prompt completion, process EOF, writer failure, stale replacement, idle/hung eviction, and shutdown invalidate the exact connection generation before old Discord inputs can write to a child process or successor session.
  • Long form content, long choices, Unicode, defaults, validation errors, and review values are reachable through bounded Discord pages without silent truncation or attachment-only substitution.
  • Tests cover R1-15 complete pagination, R1-16 full text-only recovery and stale-input protection, and R1-26 deterministic wire, dispatch, pool lifecycle, overload, liveness, and exact-boundary behavior through production paths with fake external services only.
  • The Discord guide, canary guide, and domain-language glossary describe the delivered behavior and do not claim URL elicitation or legacy session/request_input compatibility.

Follow-ups

  • Run a live Discord canary with a disposable bot and an elicitation-capable Agent Runtime before release.
  • Install the Windows target and run cargo check --target x86_64-pc-windows-gnu if cross-platform evidence is required before merge.
  • Decide whether repository-wide formatting drift should be cleaned in this PR or a separate maintenance PR.
  • Consider Slack or gateway form elicitation in a separate design if users need it.

https://discord.com/channels/1491295327620169908/1491365158868619404/1549133818089316454

- Plan: discord-acp-form-elicitation
- Branch: worktree/discord-acp-form-elicitation-e627a028
- Files: .gitignore, docs/plans/discord-acp-form-elicitation.md
- Plan: discord-acp-form-elicitation
- Description: ### Approved repair expansion — 2026-09-11
- Branch: worktree/discord-acp-form-elicitation-e627a028
- Files: README.md, crates/openab-core/src/acp/connection.rs, crates/openab-core/src/acp/elicitation.rs, crates/openab-core/src/acp/mod.rs, crates/openab-core/src/acp/pool.rs and 14 more
- Plan: discord-acp-form-elicitation
- Description: OpenAB initializes each Agent Runtime with `clientCapabilities: {}`. Its downstream ACP reader has one special Agent-to-Client request path: it automatically answers `session/request_permission`. Other ID-bearing messages are treated as possible responses before their `method` is considered. An Agent Runtime therefore cannot use the official ACP v1 `elicitation/create` method to pause a turn and ask Discord users for structured information. A reverse request can also collide with an OpenAB request ID and remove the wrong pending response.
- Branch: worktree/discord-acp-form-elicitation-e627a028
- Files: .wld/settings.json, crates/openab-core/src/acp/agentcore.rs, crates/openab-core/src/acp/connection.rs, crates/openab-core/src/acp/elicitation.rs, crates/openab-core/src/acp/pool.rs and 31 more
- Plan: discord-acp-form-elicitation
- Description: OpenAB initializes each Agent Runtime with `clientCapabilities: {}`. Its downstream ACP reader has one special Agent-to-Client request path: it automatically answers `session/request_permission`. Other ID-bearing messages are treated as possible responses before their `method` is considered. An Agent Runtime therefore cannot use the official ACP v1 `elicitation/create` method to pause a turn and ask Discord users for structured information. A reverse request can also collide with an OpenAB request ID and remove the wrong pending response.
- Branch: worktree/discord-acp-form-elicitation-e627a028
- Files: docs/plans/discord-acp-form-elicitation.md, docs/work-records/2026-09-14-discord-acp-form-elicitation.md
RunWield-Publication-Attempt: e627a028
RunWield-Publication-Plan-Path: docs/plans/discord-acp-form-elicitation.md
@openab-app openab-app Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Sep 14, 2026
@chaodu-obk

This comment has been minimized.

@chaodu-obk

chaodu-obk Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Important

CHANGES REQUESTED ⚠️ - The core elicitation design is sound, but the diff is heavily inflated by unrelated repository-wide reformatting that violates the PR plan's own scope exclusion, and the review surfaced 1 critical and 25 important findings across correctness, performance, safety, security hardening, API design, and docs.

What This PR Does

Adds official ACP v1 elicitation/create form support for Discord-backed sessions: a downstream Agent Runtime can pause an active Discord turn, present a bounded progressive form (selects, buttons, modals, plus text-only !form fallback commands), collect exactly one Accept/Decline/Cancel result from the verified human users of the active turn, and receive it as a single ACP response. Non-goals are explicit: no URL elicitation, no non-Discord form UI, no migration of the legacy session/request_input extension.

How It Works

A new platform-neutral ElicitationCoordinator owns schema parsing, active-turn authority, request leases, and atomic terminal resolution. AcpConnection advertises clientCapabilities.elicitation.form only when a Discord presenter is available, classifies agent-to-client JSON-RPC requests by method plus ID before outbound response correlation, and enforces bounds (64 KiB request envelope, 50 fields, 100 choices per field, 1 MiB reader frame, one pending form per connection). The session pool invalidates elicitation generations on cancel/reset/eviction/shutdown/EOF/Drop. Discord rendering uses nonce-based custom_id values with authorization checks on every interaction path.

Findings

# Severity Finding Location
1 🔴 ~38% of the diff is feature-unrelated rustfmt churn, including 6 gateway files (+2527/-1032) the PR plan explicitly excludes; cargo fmt --check still fails repo-wide, e.g. crates/openab-gateway/src/adapters/*
2 🟡 ACP reader consumes frames one byte per awaited read; a permitted 1 MiB frame can cost ~1M async polls on a hot path crates/openab-core/src/acp/connection.rs:274-298
3 🟡 Oversized-frame remainder is not drained before close; safe today only via process isolation, becomes a corruption bug if sockets are ever reused crates/openab-core/src/acp/connection.rs reader loop
4 🟡 completed_messages retains every terminal form message ID forever; no TTL, capacity, or eviction — unbounded memory growth in a long-lived bot process crates/openab-core/src/discord.rs:94-105,384-394
5 🟡 Presenter-capability mismatch invariant is implicit and undocumented; on trigger, every message in the thread errors until /reset with no recovery hint crates/openab-core/src/acp/pool.rs:437-440
6 🟡 JsonRpcId::Number(i64): negative JSON-RPC IDs are silently degraded to notifications and never answered crates/openab-core/src/acp/protocol.rs
7 🟡 Drop spawns expire_generation asynchronously; cleanup ordering races with process kill (benign today, fragile) crates/openab-core/src/acp/connection.rs:1133
8 🟡 Component terminal actions (submit/decline/cancel) pass None message_id to coordinator.resolve, skipping the coordinator's independent message-reference check; text replies pass it correctly crates/openab-core/src/discord.rs:3821-3853
9 🟡 Agent-controlled select labels/descriptions and modal titles/labels bypass neutralize_agent_text crates/openab-core/src/discord.rs:646-679,3629-3641
10 🟡 neutralize_agent_text only defangs scheme:// URLs; bare domains remain auto-linkable crates/openab-core/src/discord.rs:897-916
11 🟡 session_prompt widened with three presentation-layer identity params; every non-Discord adapter threads dead empty sets through the call graph crates/openab-core/src/acp/connection.rs:710, dispatch.rs:154
12 🟡 Event-path sends now use a second serenity Http instance with its own ratelimiter instead of the shared ctx.http; raises 429 probability under bursts src/main.rs:897
13 🟡 Unauthorized or stale text replies targeting the form message are silently swallowed (return true): they never reach normal dispatch and produce no notice, causing "my message disappeared" confusion in shared threads crates/openab-core/src/discord.rs handle_text_reply
14 🟡 When the ACP stdin write fails, the Discord form status is still projected as Submitted/Declined instead of Expired — the UI reports success while the agent never received the answer crates/openab-core/src/acp/connection.rs present branch
15 🟡 Agent-supplied regex pattern is recompiled on every validate_value call and has no explicit length cap beyond the 64 KiB envelope crates/openab-core/src/acp/elicitation.rs FormField::validate_value
16 🟡 Contributor tooling artifacts committed: .wld/settings.json, RunWield .gitignore block, new docs/plans/ and docs/work-records/ directories with no repo convention repo root, docs/
17 🟡 Magic number 1700 duplicated at 5 sites while PAGE_BUDGET const exists but is not reused crates/openab-core/src/discord.rs:434,584,740,3681,4974
18 🟡 handle_elicitation_component is a 333-line function; the UpdateMessage boilerplate suffix is repeated ~17 times crates/openab-core/src/discord.rs:3560-3892
19 🟡 parse_elicitation_custom_id returns an opaque 4-tuple; call sites must memorize positional meaning crates/openab-core/src/discord.rs:995-1008
20 🟡 The answerability/authorization predicate is copy-pasted 6 times across 3 handlers (lock, check, drop, await, re-lock, re-check) crates/openab-core/src/discord.rs handlers
21 🟡 Canary procedure omits !form review, !form skip, and !form value from the text-fallback verification steps docs/canary-tests.md:178
22 🟡 !form value error path leaks serde_json parser internals to end users; docs give no examples of a valid JSON value crates/openab-core/src/discord.rs:867, docs/discord.md:194
23 🟡 New docs/domain-language.md glossary is not linked from README or any docs index docs/domain-language.md
24 🟡 README wording scopes elicitation as Discord-only on the platform-neutral ACP protocol line README.md:69
25 🟡 !form commands parsed via strip_prefix with no required separator; !formnext is accepted crates/openab-core/src/discord.rs:735
26 🟡 The Modify button always jumps to field 0 with no field picker, while help text advertises !form edit N crates/openab-core/src/discord.rs:601
27 🟢 Layered fail-closed authorization: batch-derived human-only authority, coordinator re-checks nonce/channel/user/reply-target, no fail-open path found -
28 🟢 Resource bounds map to a real threat model and Discord hard limits; deadline correctly reuses the parent prompt deadline instead of adding a new knob -
29 🟢 Request classification fixes a real ID-collision correctness bug; numeric/string/null IDs echoed unchanged; unsupported methods get -32601 -
30 🟢 No form values or user inputs are logged; secrets warning is consistent in docs and in-app; values hidden after terminal resolution -
31 🟢 Deterministic subprocess test coverage: accept/cancel/decline, string IDs, overload, EOF, deadline, reset, idle eviction, capability mismatch crates/openab-core/tests/acp_form_elicitation.rs
32 🟢 Clean coordinator/presenter split; FormPresenter is a minimal two-method capability hook defaulting to None on unsupported adapters crates/openab-core/src/adapter.rs:367
33 🟢 docs/domain-language.md cleanly distinguishes form elicitation from permission prompts and Discord modals docs/domain-language.md
Finding Details

🔴 F1: Unrelated repository-wide reformatting violates the PR's own scope

docs/plans/discord-acp-form-elicitation.md states that crates/openab-gateway/ is deliberately excluded, yet the diff touches 6 gateway files with roughly 2,527 insertions and 1,032 deletions of pure rustfmt line rewrapping (whitespace-stripped content differs by less than 0.1%). Reviewers had to verify by hand that no logic changes hide inside these hunks. This inflates a "Discord-only, additive" PR to 49 files, weakens semantic review, creates merge-conflict risk, and leaves cargo fmt --check failing per the PR's own validation record.
Requested change: restore all reformatting-only files to their main state (for example git checkout origin/main -- <files>), keep only lines the feature requires, and move mechanical formatting to a separate maintenance PR that makes cargo fmt --check pass.

🟡 F2: Byte-by-byte reader on a hot path

The reader awaits a one-byte read per byte instead of buffered chunk scanning. BufReader avoids per-byte syscalls but still pays one poll per byte, so a single permitted 1 MiB frame can require about one million polls.
Requested change: use a bounded buffered chunk scanner that searches for the newline within read chunks while still enforcing MAX_ACP_FRAME_BYTES.

🟡 F3: Oversized frame remainder not drained

On an over-limit frame the reader closes without consuming the remaining bytes. Process isolation guarantees a fresh socket today, so there is no current exploit, but any future socket reuse (connection pooling) turns this into cross-connection data corruption.
Requested change: drain or explicitly document the invariant at the close site.

🟡 F4: Unbounded completed_messages

The set that intercepts text replies to already-terminal forms only ever grows; there is no prune path, TTL, or capacity bound, and no churn test. In a multi-day bot process this is monotonic memory growth.
Requested change: bounded TTL/LRU cache (or drop the set and rely on the coordinator's authoritative nonce/generation checks) plus a churn test.

🟡 F5: Presenter-capability mismatch is a hard, hint-free failure

get_or_create records form capability at session creation and hard-errors on any later mismatch. Fail-closed is correct, but the invariant is implicit; a future caller passing None for a discord: key bricks the thread until /reset, and neither the error string nor the user-facing message mentions recovery. The redacted thread key also lands in a warn-level log line.
Requested change: document the invariant on get_or_create/ensure_session and add a recovery hint or auto-reset on mismatch.

🟡 F6: Negative JSON-RPC IDs silently dropped

JsonRpcId::Number(i64) with as_u64() returning None for negatives means {"id": -1} requests are treated as notifications and never answered. Low practical risk, but a silent spec deviation.
Requested change: handle negative IDs explicitly or widen the type.

🟡 F7: Drop-time cleanup ordering

Drop revokes the generation synchronously but spawns expire_generation on the runtime while kill_process_group() proceeds. The race is benign because revocation rejects new starts, but the ordering is fragile.
Requested change: await the expiry before the kill if cleanup ordering matters, or document why it does not.

🟡 F8: Coordinator cannot independently verify component terminal actions

Component submit/decline/cancel call registry.complete(nonce, user, channel, None, outcome); the None skips the coordinator's message-reference check. The Discord layer does verify comp.message.id first, but the source of truth should re-verify.
Requested change: pass Some(&state.message_id) from component terminal handlers.

🟡 F9: Inconsistent sanitization of agent-controlled component text

Select option labels/descriptions and modal titles/input labels (including pre-populated defaults) reach Discord UI with only truncation. Discord does not render markdown there, so no known exploit, but the path is inconsistent with the main content sanitizer.
Requested change: run neutralize_agent_text or a component-specific sanitizer over all agent-controlled UI text.

🟡 F10: Bare domains not defanged

The URL neutralizer matches only scheme:// prefixes; evil.example style bare domains remain auto-linkable in Discord.
Requested change: extend the neutralizer to defang bare domains.

🟡 F11: Core prompt API widened with presentation concerns

session_prompt(content_blocks, turn_channel, trigger_message, authorized_user_ids) and the mirrored DispatchTarget::stream_prompt force every non-Discord adapter to thread an always-empty HashSet.
Requested change: accept a single optional elicitation-context struct so the elicitation-only nature is explicit and the API stays narrow.

🟡 F12: Second ratelimiter on the event path

shared_discord_state builds Http::new(&bot_token) and pre-seeds the event handler adapter, so event-path sends no longer share the serenity client's ctx.http ratelimiter. Two independent ratelimiters for one token raise 429 probability under bursts; this is a behavior change on a hot path.
Requested change: construct the shared adapter from the client's Http after client build, or list this as an accepted residual risk in the PR body.

🟡 F13: Unauthorized text replies are silently consumed

When a reply targets the form message but the author is unauthorized or the form is already terminal, handle_text_reply returns true, so the message is consumed: it neither reaches normal dispatch nor produces any notice. Component interactions correctly get an ephemeral denial; the text path just eats the message. This is fail-closed applied to the wrong target — it locks chat availability rather than only the form state, and will surface as hard-to-diagnose "the bot ignored me" reports in shared threads.
Requested change: for unauthorized/stale text replies, either return false so the message flows to normal dispatch, or send a brief notice — only return true when the input was actually applied as form data.

🟡 F14: Write failure still reports Submitted

The present branch computes the Discord status from the outcome (Accept -> Submitted, Decline -> Declined) before attempting the stdin write. If write_json_line fails, the generation is expired but expire_form is still called with the success status: the UI says Submitted while the agent never received the content. Users may assume success or re-enter data.
Requested change: downgrade the projected status to Expired (or an explicit delivery-failed state) when the write fails.

🟡 F15: Agent-controlled regex recompiled per validation

Rust's regex crate is linear-time, so classic ReDoS is largely mitigated, but the agent-supplied pattern is recompiled by Regex::new on every validation (modal, text, and submit paths) with no explicit pattern-length cap beyond the whole-request 64 KiB envelope. Compile cost is the exposure.
Requested change: compile once at schema parse time and store the compiled regex (rejecting invalid patterns up front), and hard-cap pattern length (for example 256-1024 chars).

🟡 F16: Tooling artifacts

.wld/settings.json, a RunWield block in .gitignore, and new docs/plans/ plus docs/work-records/ directories are contributor-workflow artifacts with no existing repo convention.
Requested change: drop them from the PR, or get maintainer sign-off on adopting the convention (and index the docs if kept).

🟡 F17-F20: Readability/maintainability extractions (no behavior change)

  • F17: extract one module-level page-budget constant and use it at all five sites.
  • F18: extract an update-response helper and split terminal vs navigation actions out of the 333-line handler.
  • F19: return a named struct from parse_elicitation_custom_id.
  • F20: extract a single state_is_answerable(...) predicate used by all three handlers; also consider a shared pending-lookup helper for the coordinator's six methods that repeat lock/lookup/nonce-compare boilerplate.

🟡 F21-F26: Docs/UX corrections

  • F21: add the three missing !form commands to the canary procedure so all documented commands are exercised.
  • F22: map JSON parse failures to an actionable user message with an example; keep parser internals out of user-facing text.
  • F23: link docs/domain-language.md from the README or a docs index.
  • F24: reword the README bullet so form elicitation reads as an ACP protocol feature with Discord as the current presenter.
  • F25: require a separator after !form so !formnext is rejected.
  • F26: either give Modify a field picker or route it through the documented !form edit N flow.
Baseline Check
  • PR opened: 2026-09-14
  • Base: main (declared and default), merge-base 843bb729380a2718ba4c5c009f311131f5ee5058
  • Head SHA reviewed: ccc334e1993980b1c88097809c158699a78af6e9
  • Diff: 49 files, +9228 / -1898
  • Main already has: auto-answered session/request_permission only; no form elicitation
  • Net-new value: the full ACP v1 form elicitation subsystem (acp/elicitation.rs, connection classification, pool invalidation, dispatch authority, Discord rendering, tests). Roughly 38% of the diff is reformatting-only churn with no net-new value (F1).
What's Good (🟢)
  • The authorization chain is layered and fail-closed at every hop; bot-only and cron-only turns get an immediate Cancel instead of an open form; custom_id carries only nonce and action with all authorization server-side.
  • Bounds are justified by a real threat model (semi-trusted external agent process) and by Discord hard limits, not speculative hardening; the deadline deliberately reuses the parent prompt deadline instead of adding a second timeout knob; per-connection single pending lease answers overload with -32000 busy.
  • The JSON-RPC request-vs-response classification fix removes a genuine wrong-pending-removal bug and is well tested, including string IDs and same-number collisions.
  • Elicitation state (states map) is removed on completion; prompt-injection hardening (allowed_mentions, suppress_embeds, mention/URL neutralization) is applied on the main content path; non-finite numbers are rejected; the terminal oneshot can be claimed exactly once.
  • Generation revocation covers pool cancel, reset, hung eviction, EOF, Drop, prompt_done, and abandon_request before stale inputs can reach a child process or successor session.
  • Test strategy is deterministic (real subprocess fake runtime, bounded waits, no network/token dependencies) and covers lifecycle, overload, and boundary cases.
Accepted Residual Risks (acknowledged, not blocking)
  • Stale Discord controls can remain visible if the terminal edit/delete fails; the coordinator still rejects any second ACP write (documented in the PR plan).
  • Form values are visible in-channel before Submit; users must heed the secrets warning.
  • The per-connection revoked set is bounded by connection lifetime; the only shared-registry growth concern is F4.

Addressing External Reviewer Feedback

openab-app[bot]

This PR is missing a Discord Discussion URL in the body. This PR will be automatically closed in 24 hours if the link is not added.

Not yet addressed - contributor action required. The PR body still lacks a Discord Discussion URL and the closing-soon label is applied. Please add the link promptly or the PR will auto-close regardless of review outcome.

Three Reasons We Might Not Need This PR (Devil's Advocate)

  1. No demonstrated demand yet - we found no evidence that any currently supported Agent Runtime emits ACP v1 elicitation/create form requests today. This builds a 1,800+ line subsystem ahead of a validated consumer; it may sit untriggered for months while still costing maintenance.
  2. A text-only v1 might be enough - the PR already implements a complete text-fallback path. Progressive rich-component rendering (pagination, selects, modals, review/edit) is the majority of the Discord-side complexity for what may be the last 20% of value.
  3. Permanent trust-boundary and coupling expansion - this opens a reverse agent-to-user request path whose safety depends on the nonce/generation/authorization chain staying correct forever, and couples rendering tightly to Discord component constraints (25-option selects, modal layout, custom_id budget) that Discord can change.

If these can be answered convincingly (a concrete runtime that needs forms now, rich UI as a hard requirement, and acceptance of the boundary cost), the case for the PR becomes much stronger.

Suggested merge gate: F1 (split out the reformatting), F13, and F14 are must-fix; the remaining important findings should be fixed or explicitly accepted with tracking before merge.

@chaodu-obk

chaodu-obk Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

@gandazgul Could you identify the concrete coding CLI(s) or ACP Agent Runtime(s) that currently emit official ACP v1 elicitation/create form requests, including the supported version or branch?

This feature requires support on both sides: OpenAB (the ACP client) can present the form and return one result, but the downstream ACP agent must actually initiate the elicitation/create request. Without a consumer that sends it, this path would ship untriggered.

To help us assess readiness, please share:

  1. Which coding CLI / Agent Runtime already supports emitting elicitation/create, with version or branch.
  2. Whether that support is released, unreleased/experimental, or targeted at future compatibility only.
  3. Any manual interoperability evidence between OpenAB and that agent (a live run, not only the deterministic test harness).
  4. A reproducible command or minimal scenario a reviewer can follow to trigger a form end to end.

Thanks!

@openab-app openab-app Bot removed the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Sep 14, 2026
@gandazgul

Copy link
Copy Markdown
Contributor Author

Here is a coding agent that implements this currently:

Peri agent-v3.14.5 was published on 2026-09-12. The tag
resolves to bb089eecfba78b1b7d7cdd79c675ab4e417dc378.

The release source contains a production question-to-ACP path:


  1. AskUserTool

    registers AskUserQuestion, converts questions to InteractionContext::Questions, and waits for its interaction
    broker.

  2. host/prompt.rs

    constructs AcpTransportBroker during prompt execution.

  3. AcpTransportBroker::handle_questions

    calls send_request("elicitation/create", params). Its builder uses ACP v1 form/schema types and a session scope.
    Its response parser handles accept, decline, and cancel.
  4. The release README
    documents peri acp --cwd /path/to/project.

This is evidence of an emitter in released source, not proof that its release binary interoperates with OpenAB.


I intend to implement this with RunWield, I will release the ACP ability soon, some parts of it are already in the code just need to finish the full support.


I could run this experiment if you want?

  1. Install Peri from the pinned release and configure a model through its documented setup. Use a disposable workspace and Discord bot.
  2. Configure the OpenAB PR build to launch peri acp --cwd /absolute/path/to/disposable-project. This command is an agent stdio entry point, not a standalone form trigger.
  3. From an authorized Discord user, request: "Use AskUserQuestion to ask whether I want conservative or balanced refactoring. Wait for my answer, then repeat the selected option. Do not modify files."
  4. Confirm the agent actually calls the tool. Record the initialize capability, reverse form request, selected result, and completed original prompt. A plain-text question does not count.
  5. Repeat with Decline and Cancel. Record both binary versions, source SHAs, configuration with secrets removed, and visible Discord evidence.

Or I can release RunWield's support or both let me know. Thank you

@gandazgul

Copy link
Copy Markdown
Contributor Author

Major Coding Agents elicitation support research findings:

Kimi: first-party, released support

This is the strongest match for a vendor-owned CLI requirement.

  • Moonshot's official ACP reference explicitly
    lists elicitation/create as implemented: ask-user questions use forms when the client advertises elicitation.form.
    RPC failures fall back to session/request_permission.
  • Kimi Code CLI 0.43.0 was
    released on 2026-09-14. Its tag resolves to ffa94fae854dedf594919acbea280d98cbe8e14e.
  • At that release,
    AcpInteractionBridge.handleQuestion

    sends AskUserQuestion through createElicitation when form support is available.
  • The release's
    acp-client.ts

    maps that call directly to methods.client.elicitation.create.
    question.ts

    builds a form and maps accepted answers back to the tool.
  • Implementation PR #2583, merged 2026-08-04, explains the actual
    need: the permission fallback reduces multiple questions to the first and multi-select to single-select. Forms retain
    the full question set.
  • PR #3434, merged 2026-09-01, removed the legacy adapter.
    The release README
    documents kimi acp as the normal entry point.

Do not confuse the new Kimi Code CLI (MoonshotAI/kimi-code) with the older Python Kimi CLI
(MoonshotAI/kimi-cli). The old repository directs users to its successor.
Also, Kimi's release source says the synthetic free-text Other option remains unsupported; not every possible question
interaction is covered.

Claude: released ACP adapter around the official SDK

  • Claude Agent ACP v0.77.0, released
    2026-09-14, resolves to dfe823b9581979cd22db40272d4469cc7e42b77e.

  • acp-agent.ts

    intercepts the SDK's AskUserQuestion, checks clientCapabilities.elicitation.form, sends
    methods.client.elicitation.create, and supplies the answers back as tool input. It disables AskUserQuestion when
    the client lacks form support.

  • elicitation.ts

    builds form requests for single/multiple questions and translates responses.
  • PR #756, merged 2026-06-09, introduced
    experimental elicitation support.
    The changelog
    includes it in 0.43.0. Use the current inspected release as evidence, not an assumption that the old experimental wire
    shape matches today's protocol.

The release package
is @agentclientprotocol/claude-agent-acp, uses @anthropic-ai/claude-agent-sdk 0.3.270, and names Zed Industries as
its author.

This is concrete Claude-backed runtime support, not proof that Anthropic's standalone claude command natively emits ACP.

Codex: released ACP adapter around Codex App Server

  • Codex ACP v1.11.0, released 2026-09-09,
    resolves to 51d6247ac7448485bfcf534b813196fafc26df59.

  • CodexElicitationHandler.ts

    forwards supported MCP forms and Codex request_user_input as methods.client.elicitation.create, then converts the
    returned content into Codex answers.
  • PR #268 added MCP forwarding on
    2026-07-09; PR #291 added request_user_input bridging
    on 2026-07-13. The latter explicitly says it handles events without enabling experimental Codex flags itself.

The release README
describes the stdio adapter and its bundled Codex dependency.
The package manifest
specifies @openai/codex ^0.154.0; this is a version range, not proof of a specific installed binary.

  • Caution: issue #344 reports immediate cancellation when
    autoResolutionMs is omitted. The inspected release still distinguishes null from omitted in its timeout path. Do not
    promise that every user-input scenario works without a live check.

The old zed-industries/codex-acp repository is not the current implementation.
A maintainer comment on old issue #294
explicitly directs development to agentclientprotocol/codex-acp.

Other requested vendors

Vendor Evidence found Conclusion for this merge requirement
DeepSeek The first-party DSH ACP package reference explicitly excludes elicitation from its automation-only surface. Not a current consumer in that server. No near-term commitment established.
Qwen PR #4405 is open and implements MCP elicitation with terminal UI. Public code/issue searches did not establish ACP form forwarding. MCP work is not sufficient ACP evidence, nor a release commitment.
GLM / Z.ai Official docs show GLM models used through Claude Code. Search also found a third-party ZCode adapter, but its support was not verified here. A model provider is not an ACP runtime. No first-party ACP emitter or dated plan established.
Antigravity Checked the official feature overview and targeted public searches. No direct form-emitter implementation or announcement established. Unconfirmed; not proof of absence.

@gandazgul

Copy link
Copy Markdown
Contributor Author

Review fixes pushed:

  • F1 fixed and separately committed/pushed as f0693c2 on fork:discord-acp-form-elicitation; removed formatting-only files and hunks. Main
    checkout preserved.
  • F2–F3 fixed: bounded buffered ACP reader and documented no-reuse pipe invariant. F5/F7 fixed with reset guidance and cleanup-ordering
    documentation. F6 disproved with a negative-ID subprocess regression test; no protocol change needed.
  • F4/F8–F10/F13 fixed: bounded completed-form cache, stale-reply protection and notices, independent component message verification, and
    presentation text/domain sanitization. Editable defaults and enum values remain unchanged.
  • F11/F12/F14/F15 fixed: optional elicitation context, shared Serenity HTTP ratelimiter, Expired status on failed delivery, and compiled
    bounded regex patterns.
  • F16 fixed: removed workflow artifacts from PR diff and preserved backup copies. F17–F20 fixed with shared constants/helpers, named
    custom-ID fields, terminal-action extraction, and shared authorization checks.
  • F21–F26 fixed: complete canary commands, JSON examples and actionable errors, glossary link, protocol-neutral README wording, command
    separator validation, and Modify instructions for !form edit N.
  • Verification passed: workspace check, workspace Clippy with denied warnings, unified-feature Clippy, workspace tests, and diff whitespace
    checks. Tests: 1499 passed, 10 ignored; +21, none removed. Reset/idle-eviction/EOF tests retain lifecycle coverage but now permit Expired
    on failed delivery; sanitizer assertion updated for defanged domains.
  • Verification limitations: fmt check fails, also reproduced on base checkout; Windows check fails because target is missing.

@chaodu-obk

chaodu-obk Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Thanks for the detailed evidence and the review-fix round, @gandazgul.

The team broadly endorses the direction. Standard ACP elicitation/create support is a reasonable capability for OpenAB to offer, and we agree that without it, agents that rely on it degrade in ways users do not see (silent auto-answer, dropped questions, or a disabled ask-user tool depending on the agent).

Before we finalize how this adapter should be written, we want to do some investigation of our own. We plan to write a small test that stands up a minimal ACP client, advertises clientCapabilities.elicitation.form, and drives each of the coding CLIs we actually support (Kimi, Codex, Claude, and others) through scenarios that should trigger a user question. The goal is to observe, empirically, which of them really call the standard ACP elicitation/create method versus using a vendor-specific path or a permission fallback, and to capture the exact request shapes they send.

We will produce a summary of that behavior. With that data we will have a much clearer, evidence-based picture of what the adapter needs to handle and how to shape it, rather than relying on source-level inference alone.

No action needed from you right now. We will follow up here with the findings. Thanks again for the thorough work and the fixes already pushed.

A completed form retained its coordinator slot until expire_form finished
editing the Discord message. The ACP result had already been delivered,
so an Agent could immediately ask its next question while the previous
form still occupied the slot. OpenAB rejected that request with
"elicitation already pending". In the live RunWield canary, selecting
Other submitted the choice but prevented the free-text follow-up from
opening, leaving the interview without an answer.

Release the completed form's nonce-scoped slot before publishing its ACP
response. Keep Discord message cleanup best effort after delivery, and
keep failed delivery marked Expired. Remove the now-redundant attempt to
expire a slot that has already been released. Unanswered concurrent forms
still receive the existing busy response.

Add a subprocess regression that holds Discord cleanup open while the
Agent submits Other, immediately requests text, receives the typed answer,
and completes the same prompt. It fails before this fix and passes after.
Also cover simultaneous coordinator/presenter completion for Cancel and
Decline across repeated follow-up turns in the same Session.

Validation:
- cargo test: 18 passed.
- cargo test -p openab-core --test acp_form_elicitation: 16 passed.
- cargo clippy -- -D warnings: passed.
- cargo build --features unified: passed.
- Updated regression file passes rustfmt; repository-wide formatting
  drift remains unchanged and is already documented in the PR.

Refs openabdev#1533
@gandazgul

gandazgul commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor Author

An agent can request a form, receive its answer, and immediately request a follow-up form based on that answer.

Live Discord testing exposed a sequencing bug:

OpenAB sent the first answer before releasing the completed form’s pending slot. While Discord was still updating the first message, the follow-up request was rejected with “elicitation already pending,” even though the first form had already been answered.

The fix releases the completed form’s slot before sending its response. Discord message cleanup happens afterward, so it cannot block the next question.

A regression test deliberately delays Discord cleanup and verifies that a follow-up form opens and returns its answer successfully.


New fix: Replies after form elicitation were appended to an earlier progress message, placing the answer above the form. OpenAB now sends the completed reply below the form as a new message, while preserving tool status updates in the earlier message. Applies to any ACP agent using forms.

A turn keeps one streaming progress message, but elicitation presents its
form as a separate later message. When the agent resumes, editing its
reply into the original progress message places the answer above the form.

Track whether the current ACP turn admitted a form. Keep subsequent
narration out of the earlier placeholder, finalize its tool status, and
send the completed reply as a new message below the form. Reset this state
for the next prompt so ordinary turns retain their streaming behavior.
This applies to any ACP agent using form elicitation.

Add subprocess regression coverage for submit, cancel, and decline, plus a
following turn without a form. The test fails with the old adapter and
passes with this change. Document the delivery behavior and canary checks.

Validation: 17 form integration tests and 38 adapter unit tests passed;
cargo test (18 tests), cargo clippy -- -D warnings, and
cargo build --features unified passed. Repository-wide cargo fmt --check
still reports existing formatting drift outside this change.
@chaodu-agent

Copy link
Copy Markdown
Collaborator

Thanks for the substantial work here, and for running the live Claude canary — that closed the one question that was blocking this.

We support the direction. Before we take on a change of this size, though, we want to be able to measure who actually benefits. The premise of the PR is that downstream agents will honor clientCapabilities.elicitation.form by sending elicitation/create. We spent today verifying that on the wire with a mock ACP client that advertises elicitation: { form: {}, url: {} } and logs every agent→client request. Results:

Agent Emits elicitation/create? Evidence
Claude (claude-agent-acp, claude 2.1.273) Yes — one mode: "form" request with toolCallId; we answered accept and the model read it back. Full loop closes. live wire, independently reproduced
Kiro CLI (kiro-cli acp, OpenAB's default) No — agentCapabilities does not advertise elicitation; forced prompts end the turn with zero reverse requests. live wire
Codex (codex-acp 1.12.0, gpt-6-astra) Not observed — the adapter does implement the bridge (shouldUseAcpElicitation → buildElicitationRequest, gated on clientSupportsFormElicitation), but across three forcing prompts the model answered in prose and never produced a form. Note also that codex's own openai/form mode explicitly returns false and is not bridged. source read + 3 live runs
Gemini, OpenCode, MiMo, agy-acp, pi-acp No emission path in current source static
Kimi Appears to have one in source static, not run
Copilot, Cursor, Grok, Devin Unknown —

So today: one confirmed producer (Claude), one adapter that can but whose model didn't (Codex), and the default agent that cannot (Kiro). That is a thin base for a change of this footprint. Roughly 4.5k lines of production code, most of it a Discord-only presentation layer, currently serving one backend.

We'd like to proceed in a way that captures the value without committing to the whole surface on one data point:

  1. Split out the protocol fix and merge it now. The reader currently classifies ID-bearing frames as responses before checking method, so an agent→client request can collide with an OpenAB request ID and remove the wrong pending response. That is a correctness bug independent of elicitation, and it's small. Please open it as its own PR — we'll take it quickly.

  2. For the elicitation feature itself, please add live-wire evidence beyond Claude, ideally Codex and Kimi, showing an actual elicitation/create on the wire when the capability is advertised. If Codex can be made to produce one (a different model tier or a natural clarifying-question task), that materially changes the ROI. If only Claude produces it, we'd want to scope and document this explicitly as Claude Code support rather than general ACP elicitation.

  3. Docs must match the evidence. README/discord.md should name the agents verified to trigger it and state that Kiro does not, rather than describing it as generic ACP support — otherwise users on the default agent will file "form never appears" issues.

  4. One presentation finding from the real payload: Claude pairs every select with a question_<n>_custom free-text field marked _meta._askUserQuestionCustomAnswer. The PR renders that as a standalone string field rather than the select's "Other" box. Not a correctness issue, but worth handling since it appears on every Claude form.

The probe we used is a small stdio JSON-RPC client; happy to share it so the same runs can be reproduced against other adapters.

@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. label Sep 19, 2026
@github-actions

Copy link
Copy Markdown

Caution

This PR has been waiting on the author for more than 2 days (labeled pending-contributor since 2026-09-16).
It will be automatically closed in 24 hours if there is no update.

@gandazgul — You must add a new comment on this PR to remove the closing-soon label and keep it open. Pushing commits alone is not sufficient. Feel free to reopen a new PR later if it gets closed and you want to pick it back up.

@github-actions

Copy link
Copy Markdown

🔒 Auto-closing: this PR has had the closing-soon label for more than 1 days without activity from the author.

If you'd like to continue working on this, please submit a new PR and link to this one if necessary.

@github-actions github-actions Bot closed this Sep 20, 2026
@gandazgul

Copy link
Copy Markdown
Contributor Author

new PR #1540 I cant produce this evidence unfortunately, RunWield will find a different path. I'll continue using OpenAB anyways if I find other feature gaps I'll bring them up in Discord first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 24 hours. pending-contributor response-requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants