Add bounded ACP form elicitation to conversation chat - #2206
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: raphaeltm/simple-agent-manager/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
|
Parent review of exact7702452d29d6538d94bff311cc955285a848b454: staging HOLD pending the following corrections.
Reviewed the Worker validation/encryption/deadline/receipt delta, Go schema validation/raw-frame guard, prompt/generation ownership, capability gating, and UI. Their core direction is sound; the findings above remain open. The #2200 permission assertions are preserved. No ready transition or merge is approved. Staging is reserved for C1 after these corrections and parent review of the resulting exact head. |
|
Reviewed7702452d..39d0f48: form validation/HMAC extraction and committed-answer/count helpers preserve the relevant behavior; no new blocker found in that refactor. Inspected committed mobile owner/actions and desktop actions screenshots: readable layout and unobstructed action buttons at the captured positions. Staging remains HOLD: AcpFormCard is unchanged, so the prior empty-string answer, stale detail callback, retained receipt cleanup, and forms deployment-path findings remain open. Durable screenshot availability is now addressed. Await revised UI/tests and CI; do not treat this refactor review as staging authorization. |
|
Parent review PASS at exact418d5f2ea708d525ca3c645119911141402c9436. Reviewed the delta from39d0f48a: valid empty strings/enum selections now preserved; aborted detail responses ignored; terminal/revoked state discards private values/receipt; delayed hash completion fenced by active interaction/access identity. Regression tests exercise the races. Reviewed deployment flag propagation through both sync passes, checked-in false default, and new mobile/desktop empty-answer screenshots. Prior parent blockers addressed. Staging is authorized for THIS exact head once current CI is green, subject to a fresh contention check. No additional parent confirmation required unless the code head changes. Root independently verified zero active staging nodes, no active staging deployment, ACP_INTERACTIONS_ENABLED=false, and forms binding absent. Owner must snapshot both override states, temporarily enable staging global+forms flags, deploy exact head, read back effective bindings/runtime version, validate the real account and fixture flows, then clean resources and restore both override presence and effective disabled values. Preserve precise session/interaction/receipt evidence. No production mutation, ready transition, or merge authorized by this approval. |
|
Parent delta review PASS exactb66937b1850df32e5850ddcdbb8a86cc7d8e1745. Compared against418d5f2e: capability-test expectation includes formBridge; Codex extracted-code evaluation removed and replaced with pinned source/helper/constants/fixture drift checks; Claude builder still executes. No production behavior change. Codex fingerprints remain static evidence, distinct from runtime execution. Exact-head staging approval reinstated once latest-head CI (including Sonar) is green, subject to immediate contention check and the existing temporary staging flag, live roundtrip, cleanup and verified restoration conditions from comment5917571805. No additional confirmation needed unless head changes. No production mutation, ready transition or merge approved. |
|
Parent delta review PASS8098badd8927b4855f0580c6f9361c33d45e6aa2: compared tob66937b1, only verifier stdout wording changed to explicitly distinguish executable Claude evidence from static Codex fingerprints. Exact-head staging approval now applies to8098badd after full latest-head CI including Sonar passes, under all existing contention, temporary staging flags, live verification, rollback and cleanup conditions. No additional confirmation required then. Freeze the candidate; routine evidence can be recorded in this PR body/comments without restarting CI. No production/ready/merge approval. |
|
Parent delta review PASS exact53b170f706136b26814286c621506ec9d175519d. Compared with8098badd: removed child_process/npm PATH discovery; verifier requires explicit absolute existing adapter directories. Version/source/fixture checks retained. Only test tooling and its documentation changed. Exact-head staging authorization applies to53b170f7 once all latest-head CI including Sonar passes and an immediate contention check is clear. All existing temporary staging flag, live proof, restoration and cleanup conditions remain; no additional parent confirmation needed then. No production/ready/merge authorization. |
|
Parent delta review PASS exact7a6f56f25847932fea8e8a592522aac2212a765e. Only inherited Worker tests changed: permission answers now select the offered allow option, preserving duplicate/race/outbox assertions. The purge test seeds legacy encrypted_answer/IV after a valid answer and confirms delivery before purge; it still asserts all detail/answer/decision ciphertext and IV columns are cleared while summaries survive. No production-validation change. Staging approval applies to7a6f56f2 once ALL latest-head CI including full Worker suite and Sonar passes, plus immediate contention check. Previous temporary staging flags, live proof, rollback and cleanup conditions remain. No further parent confirmation needed then. No production/ready/merge approval. |
|
|
Final C1 evidence is now in the PR body at frozen head Rollback workflow 36778422816 passed configuration, Cloudflare deploy, and smoke on that exact SHA. Fresh Worker settings read GitHub records #2206 merged at 2026-09-30T21:43:58Z, before this evidence update. I made no code commit or merge. C1 activation remains separate from #2204. |
|
Cleanup addendum at frozen head The PR body now distinguishes the live Instant decline callback from deterministic wrapper/prompt cancellation tests; no separate live cancel result is claimed. Live VM acceptance and the default-mode no-emission limit remain recorded with full IDs and safe receipts. |
|
Production verified: merge 989bf7b deployed successfully in run https://github.com/raphaeltm/simple-agent-manager/actions/runs/36783707657 after main CI36781294922 passed. Read-only Cloudflare settings confirm VM_AGENT_REQUIRED_VERSION matches that merge and both ACP_INTERACTIONS_ENABLED and ACP_INTERACTION_FORMS_ENABLED are false. Production API /health and app returned HTTP200. This ships dormant forms support; it does not activate interactions or resolve the separate legacy-runtime preservation, URL elicitation, auth diagnostics, or Sol6.1 provider-entitlement work. |



Problem and behavior
Pinned Claude and Codex ACP adapters emit
elicitation/createform questions, but SAM's permission bridge did not handle them. WithACP_INTERACTION_FORMS_ENABLED=trueon a conversation session, the client now advertiseselicitation.form, validates a bounded flat schema, persists the request encrypted in Cloudflare, and displays an owner-only chat card. A valid creator answer is validated at the Worker and VM boundaries, saved with idempotency, and returned once to the live callback; decline, cancel, expiry, task-mode requests, URL mode, and unsupported schemas cancel explicitly.The form flag remains off by default and independent from permission rollout. URL elicitation remains disabled. The coordinating parent agent (SAM task
01M3SG06CFJYF7F6HVJXHTFTN1) reviewed the exact candidate and staging evidence under Raphaël’s authorization to ship. GitHub records this PR merged at 2026-09-30T21:43:58Z on the frozen head; C1 remains dormant/default-false. Activation and the legacy-runtime hold belong to the separate #2204 decision.Evidence
CLAUDE_ACP_PACKAGE_DIRandCODEX_ACP_PACKAGE_DIRpaths, avoiding command lookup through mutablePATH. Go tests exercise the pinned acp-go-sdk v0.13.5 JSON-RPC path, raw-frame rejection before lossy decode, prompt ownership, deadline, cancellation, and duplicate receipt.docs/notes/acp-c1-screenshots/and linked below. Raphaël's permission clearance assertions fromfca210a90are preserved with a positioning fix.Validation
-racepassed; previous full ACP/server Go package run passed.418d5f2ea, CI found two stale API capability expectations (the newformBridgefield) and SonarCloud flagged dynamic fixture-verifier execution. A second scan at8098badd8found the verifier’snpm root -gsubprocess command lookup through mutablePATH. All three issues are fixed; focused API tests 54/54 and the pinned verifier against installed packages pass, and security re-review passed. At53b170f7, SonarCloud, aggregate Test, Playwright and all other checks passed, but the full Worker suite exposed four stale permission decision fixtures inacp-interaction-store.test.ts(legacyacceptedwhere the current contract requiresselected_option). The current test-only delta corrects those inputs and preserves legacy encrypted-answer purge coverage; the focused Worker file passes 8/8. Exact-head CI run 36768485895 passed all jobs including full Durable Object Workers; SonarCloud PR quality gate is OK. CodeRabbit evidence is recorded separately below.Staging proof and rollback
The coordinating parent agent approved exact head
7a6f56f25847932fea8e8a592522aac2212a765eafter CI. A fresh contention check found no active staging deployment or node. Neither staging GitHub Environment ACP override existed; the staging Worker hadACP_INTERACTIONS_ENABLED=falseand no forms binding. Temporary staging-only overrides set both true. Exact-head enable deployment 36771134033 and built-in smoke passed; effective Worker bindings read true/true.cfe72088-946b-444e-956b-b652bfd2f5028a46e581-d0df-4bfc-a7ed-0248887a0935FORM_CALLBACK_RECEIVED. The first harness waited for the wrong terminal predicate and did not capture a structuraldelivery_confirmedreceipt. The post-submit card capture and callback marker are separate observations.255b8c1a-8ca3-48f9-b189-ad76517f03ba86e84a3c-2bcf-45b7-b21e-e46ee993fc1fstate=delivery_confirmed,deliveryState=confirmed; assistant message containedFORM_CALLBACK_DECLINED. The decline path did not accept an empty object.bdbfe51f-64e6-4278-8cec-2e93b84eb867e0c20294-3838-4f45-9f78-9867695fa00cstate=delivery_confirmed,deliveryState=confirmed; assistant message containedFORM_CALLBACK_RECEIVED.VM timing is separated from the form wait: task
01M3T21XEE9HV73MYXGP6KXM9Jwas queued at 21:03:15.918Z and delegated after provisioning at 21:08:59.148Z (5m43s);workspace_readywas observed at 21:10:20Z,in_progress/runningat 21:13:49Z, and the form was observed pending at 21:15:45Z. The harness did not capture a distinctagent.readyframe or exact prompt-start timestamp, so those stages are not inferred from the longer timeout. The earlier VM probe, session3539074c-2aee-48fa-a45d-112cbbf9e0a6, had no assistant/tool output before its 10-minute window ended; it supplies no form result.The Instant default-mode probe, session
36cab77f-f7a0-47c3-982c-85bb68fdbe42, emitted no form, and the model response reportedAskUserQuestionunavailable in that run. This is no observed emission, not proof that SAM failed to advertise the capability or that the tool was unavailable to every default-mode session. The successful external-account proofs above used plan mode; deterministic pinned-wrapper fixtures are separate evidence.Cleanup read-back: all five listed test sessions returned
stopped; all five temporary profile DELETEs returned HTTP 200 (01M3T0AM19P5H6H2M7CFWRWYA2,01M3T0W8RX5ZT04HC6MANW3BGG,01M3T16WCVB6AYPWQ45ASRGPA6,01M3T19XFHY3ZBHKTKH11GTF7F,01M3T21RHF31VYKPC89GV7TGAW). Test-owned node deletion calls succeeded; the retained first Instant node01M3T0AR6TV1HTCCQF01NWFWMHreadsdeleted, and a fresh staging D1 query returned zero active nodes. The retained default Instant and first VM workspaces readdeleted; other test workspace rows were removed. The two temporary staging GitHub ACP overrides were removed back to their prior absent state.The Instant accept harness's stop request returned HTTP 500. Its explicit fallback deleted test node
01M3T0WCN6Y1Y29VRMND4V7V5Qwith HTTP 200. A fresh read-back found sessioncfe72088-946b-444e-956b-b652bfd2f502stopped, no row for workspace01M3T0WCTP3PMEX50FJMV2M2WN, and backing task01M3T0WC760R3AMR9N9WGPN9SZcancelledwithworkspace_id=null; there is no live orphan from that fallback. All five temporary profile GETs now return 404. The live decline path above proves a declined callback; wrapper/prompt cancellation and stale-answer fencing were verified by focused Go/Worker tests, without claiming a separate live external-account cancel run.Same-head rollback deployment 36778422816 completed successfully: configuration, Cloudflare deploy, and built-in smoke jobs all passed on SHA
7a6f56f25847932fea8e8a592522aac2212a765e. Fresh Cloudflare Worker settings readACP_INTERACTIONS_ENABLED=falseandACP_INTERACTION_FORMS_ENABLED=false; staging GitHub Environment has neither override. No production variable was changed by this staging exercise. The reusable deploy workflow forwards the independent forms flag in both sync passes; checked-in Wrangler defaults false. Merge and any later production deployment are separate from this staging read-back.C2 and D handoff
C2 URL elicitation needs its own capability flag and completion lifecycle; C1 advertises form only and cancels URL requests. Reuse interaction identity, creator authorization, runtime generation, deadline, no-wake delivery and encrypted Cloudflare authority, but define a URL-specific decision schema. D diagnostics may use safe IDs, state, generation and receipt reason codes; never log or event raw schemas, answers, wrapper metadata, or tokens. The task file records the contract in detail.
UI Screenshot Evidence
Surface: Conversation form card
Additional desktop empty answer, mobile empty answer, desktop receipt, mobile receipt, desktop noncreator, and mobile noncreator views are included in the PR.
Agent Preflight (Required)
Classification
External References
Official documentation and pinned adapter sources were inspected before implementation: ACP protocol, Claude ACP package, and Codex ACP package. The installed package manifests were checked for versions 0.81.2 and 1.13.1, and the pinned Go SDK v0.13.5 wire decode was exercised.
Codebase Impact Analysis
packages/shareddefines the bounded schema and cross-language corpus;packages/vm-agent/internal/acpadvertises and handles the callback;apps/apivalidates, encrypts, stores, and delivers;apps/webpresents owner-only chat forms;apps/wwwdocuments behavior. The existing permission bridge and task-mode cancellation were checked as regression boundaries.Documentation & Specs
Updated
apps/www/src/content/docs/docs/guides/chat-features.md,apps/www/src/content/docs/docs/reference/configuration.md,.claude/skills/env-reference/SKILL.md, and the C1 task file. The docs describe conversation-only, flag-gated forms and URL unsupported behavior.Constitution & Risk Check
Checked Principle XI configurable limits with the constitution specialist. Bounds and form deadlines come from existing or new environment-backed configuration. Security review covered creator authorization, encrypted form payloads, HMAC-protected answer receipts, no-wake delivery, generation fencing, cancellation, and sensitive-data exclusion from logs/events/transcripts/cache. URL and task forms remain disabled by explicit decisions.
Specialist Review Evidence
All six local specialists returned final re-reviews. Their earlier blocking findings were fixed; the Go/Cloudflare reviewer’s remaining low-priority old-waiter observation cannot attach an answer to the next prompt and was not a release blocker. The coordinating parent agent separately reviewed the exact head and later deltas.
/root/security_review)PATHlookup); no remaining security blocker./root/go_cloudflare_review)/root/ui_review)/root/constitution_review)/root/docs_env_review)/root/task_validation)01M3SG06CFJYF7F6HVJXHTFTN1)7a6f56f2, parent-requested UI/deployment fixes and later test-only deltas; authorized gated staging and independently confirmed rollback. This is agent review under Raphaël’s authorization, not a human code review.CodeRabbit Review Evidence
The trusted
workflow_dispatch36757343075 requested a review at 2026-09-30T18:16:02Z and completed successfully. CodeRabbit replied to that dispatched command at 18:16:19Z withAction not completed: Review rate limited. Its earlier 18:15:29ZReview skipped: Auto reviews are disabledstatus was the automatic-review path, not the dispatched request's outcome. No CodeRabbit review arrived after more than three hours; GitHub's PR reviews remain empty. The best-effort request/wait requirement is satisfied. Neither comment is a review, and no retrigger or waiver is needed.