Skip to content

fix(acp): preserve typed form answers across clients - #16074

Merged
maria-rcks merged 13 commits into
pingdotgg:t3code/local-acp-provider-supportfrom
maria-rcks:t3code/acp-typed-elicitation
Oct 5, 2026
Merged

maria-rcks merged 13 commits into
pingdotgg:t3code/local-acp-provider-supportfrom
maria-rcks:t3code/acp-typed-elicitation

Conversation

@maria-rcks

@maria-rcks maria-rcks commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

ACP forms lost titled choices and multiselect fields, then returned strings for boolean and numeric answers. Decode native form schemas, preserve opaque choice values, and send validated typed responses. Pattern- and format-constrained forms decline before opening; the client does not advertise validation it cannot safely perform. Integer answers must be safe JavaScript integers. Optional answers can be skipped, and required arrays can submit an empty selection when permitted, across web and mobile clients.

Base chain: #16021 → #16074 → #16075. gh stack link was attempted; GitHub rejects native stacks containing fork PRs. No harness-specific hooks or new test files.

Verified: 296 focused existing adapter/client/contract tests, 113 runtime/extension tests, scoped lint (0 errors), server/contracts/web/mobile typechecks on Blacksmith. Real client submission returned {"target":" workspace ","scopes":["read","write"],"approved":true,"count":2}; compact web submission omitted an optional choice and returned {"scopes":[]}. Stopping a fresh pending form returned native ACP {"action":"cancel"} and interrupted the turn. Native mobile interaction remains unverified. Form evidence uses the existing ACP mock agent through the real protocol and client, since dsh does not emit structured questions.

Before: native titled choice is missing

After: native titled choices are available

Optional choice can advance without an answer in compact web

Zero-selection array can submit in compact web

Review fixes: 28 native-wire form cases and 122 existing web/mobile tests passed on Blacksmith, plus scoped lint (0 errors) and server/web/mobile typechecks. Fresh runtime checks declined the reported unsafe integer, catastrophic regex pattern, and email-format form. Web/mobile default to zero selections when the maximum is zero and the minimum is omitted.

Native zero-maximum form without a minimum can submit an empty array

Filtered native interaction results

current-head verification on 6d086c1d5f: Parent service fixes are integrated; this PR's own typed-form or tool-metadata behavior is unchanged. Both independent reviewers approved all three exact stack heads. Blacksmith passed 51 focused catalog/adapter/websocket tests, scoped lint, and server typecheck on integrated head 2d718dba8b. The real websocket uninstall preserved a referenced registry provider. Fresh dsh Codex and OpenCode Go read/edit/readback runs completed, and official OpenCode completed a file-tool conversation. Sanitized current-head runtime proof. The final one-line change makes the internally used catalog error guard private. Blacksmith passed the server export check, changed-file lint (0 warnings/errors), and 37 catalog tests on 761a16874e; the preceding behavior checks above passed on 2d718dba8b. Desktop shell and native mobile interaction remain unverified.

Model: gpt-6.1-sol. Harness: Codex through T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 5, 2026
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts Outdated
Comment thread apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx
@macroscopeapp

macroscopeapp Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR adds substantial typed ACP form handling and changes how requiredness, selection bounds, optional questions, and empty arrays behave across server, web, and mobile clients. It also changes product defaults, so the runtime and UX impact should receive human review.

You can add or adjust custom eligibility rules. Learn more.

@maria-rcks

Copy link
Copy Markdown
Collaborator Author

Note

Written by gpt-6.1-sol on behalf of Maria

this pr implements the requested generic acp form support. the coordinated server, contract, web, and mobile changes are intentional, with no harness-specific hooks.

the four correctness findings on 6df093b were fixed in 604f93d: pattern/format-constrained forms decline before opening, unsafe integers decline, and zero-maximum selection defaults agree across clients. all four threads have replies and are resolved. current ci passes; 28 focused native-wire form cases and 122 client tests, scoped lint, and server/web/mobile typechecks also passed on blacksmith. fresh native runtime results and client screenshots are in the pr body. native mobile interaction remains unverified.

the approvability objection to feature scope remains for maintainer review.

@maria-rcks

Copy link
Copy Markdown
Collaborator Author

Note

Written by gpt-6.1-sol on behalf of Maria

the scope objection was checked against current trusted upstream policy: maria-rcks is explicitly exempt from contribution triage. the coordinated server/contracts/client changes fix one native ACP form answer pipeline. all four bot correctness threads were fixed and resolved.

final independent review found an additional decimal-rounding edge, fixed in f6b857a: 1.0000000000000001 and nonzero underflow now decline rather than silently returning a different integer. exact decimal/exponent integers remain supported. 34 existing native-wire cases and the fresh browser-to-provider reproduction passed; both reviewers approved this head. native mobile interaction is still unverified because device access is disabled. filtered runtime proof. no review bot was invoked and no review or CI settings were changed.

@maria-rcks
maria-rcks merged commit 99f738c into pingdotgg:t3code/local-acp-provider-support Oct 5, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant