feat(acp): bridge runtime permission requests - #2201
simple-agent-manager[bot] wants to merge 13 commits into
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
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 |
|
Coordinator review of d7439c9: changes required before integrated staging/readiness.
Additional integration concern to investigate in the same follow-up: permissionDeadline examines the inbound SDK callback context, but pinned acp-go-sdk v0.13.5 derives inbound request contexts from its independent background connection context. That context does not inherit the outgoing prompt deadline. Prove with the real ACP fixture that prompt deadline/cancel closes the matching permission promptly while the connection remains alive; bind it to the correct prompt attempt if needed, without allowing an old cancel to affect a new attempt. This is source/SDK review with an executed SDK reproduction, not a rerun of the full repository suite. CI passing is acknowledged. Draft/no-merge/global-disabled constraints remain; coordinator-owned integrated permission-reversed staging is still required after fixes. |
|
Coordinator source re-review of 96df904: The two blocking findings and the SDK prompt-lifetime concern are addressed in the reviewed diff:
No additional blocking source finding in this pass. I inspected the regression implementations; I did not rerun the suites locally. Final-head CI is currently running. This is not readiness/merge approval: final CI, integrated permission-reversed VM/Instant staging, and remaining review gates are outstanding. Keep draft and global enable false. |
|
Coordinator follow-up fixes are pushed to the existing branch at Review findings addressed:
Validation at handoff:
|
|
|
Final-head validation for coordinator review follow-up
PR remains draft. No staging mutation, deployment, readiness change, production flag change, force push, or merge was performed. Integrated staging remains explicitly delegated to the parent. |



Summary
ACP agents now route permission requests through the durable Cloudflare interaction authority instead of broadcasting raw requests or choosing the first option. The runtime creates bounded encrypted detail, waits for an exact generation-bound answer, and settles cancellation, expiry, process loss, connection replacement, or Stop without a browser-to-runtime response path.
This coordinator follow-up closes three runtime races:
ctx.container.getTcpPort(...).fetch(...)after persisted and actual-running checks. It never calls the pinned Containers 0.3.7containerFetch()helper, which can auto-start compute.404within its existing bounded budget, covering the ordering where cancellation settles before a slow create commits.The real pinned
acp-go-sdk@v0.13.5fixture proves its inbound permission context lacks the outgoing prompt deadline, then proves attempt-owned deadline/cancel settlement while the same connection remains usable. Global enablement remainsfalse; forms and URL elicitation are not advertised.Validation
pnpm lintpnpm typecheckpnpm buildgo test ./...go vet ./...go test -race ./internal/acp ./internal/serverexpired; explicit cancel settledwrapper_cancelled;Initializeand a later prompt succeeded; stale attempt cancellation could not claim the later waiterpnpm testexercised 11,079 API tests; three unrelated load-sensitive failures occurred in two files, and both files then passed all 76 tests in isolationStaging Verification (REQUIRED for all code changes — merge-blocking)
Staging Verification Evidence
Raphaël explicitly instructed this delegated follow-up not to deploy or mutate shared staging. Staging remains required before readiness or merge and is owned by the coordinator.
UI Compliance Checklist (Required for UI changes)
N/A: no files under
apps/weborpackages/acp-clientchanged; UI PR #2200 was untouched.UI Screenshot Evidence
N/A: no UI surface changed.
End-to-End Verification (Required for multi-component changes)
Data Flow Trace
acpInteractionscontract for VM and Instant runtimes.RequestPermissionregisters the attempt/generation waiter before starting bounded durable create. Only a definitive create rejection cancels immediately; ambiguous acknowledgement keeps waiting for a potentially committed answer.Untested Gaps
The deployed Worker↔VM/Instant roundtrip has not run because this delegated task forbids shared staging mutation. Coordinator staging should run the deterministic
permission-reversedfixture, duplicate/conflicting answers, Stop/process loss, and stopped/sleeping no-wake checks.Post-Mortem (Required for bug fix PRs)
The original no-wake guard trusted persisted lifecycle state but then called an SDK helper whose internal health check could start a stopped container. The permission path also executed create synchronously and assumed the inbound SDK callback inherited the outgoing Prompt context. The fix uses a direct non-starting port primitive, explicit prompt-attempt ownership, and ambiguity-aware create/settle ordering tests.
Specialist Review Evidence (Required for agent-authored PRs)
404ordering gap;7e6030174retries within the bounded settle context and adds the exact race regressionResidual risk: a deliberately noncompliant injected Go
RoundTrippercan retain its own goroutine if it ignores both request context and client timeout forever. The production HTTP client is bounded and context-aware. Direct-port behavior is unit-tested against the documented Cloudflare primitive; deployed confirmation remains with coordinator staging.CodeRabbit Review Evidence (Required for agent-authored PRs)
CodeRabbit Notes
The current check reports review skipped by label configuration. The coordinator owns readiness and any later review gate.
Exceptions (If any)
Agent Preflight (Required)
Classification
External References
github.com/coder/acp-go-sdk@v0.13.5source and executable fixture@cloudflare/containers@0.3.7source plus the direct Durable Object container port contractCodebase Impact Analysis
packages/vm-agent: prompt-attempt ownership, bounded create/wait, ambiguous acknowledgement, settle ordering, receipts, and real SDK fixtureapps/api: direct-port Instant no-wake forwarding and capability/answer race coveragespecs/001-mvp/contracts/api.md: exact attempt ownership, ambiguity, and direct-port delivery contractDocumentation & Specs
specs/001-mvp/contracts/api.mdtasks/active/2026-09-30-acp-runtime-permission-bridge.mdConstitution & Risk Check
The follow-up reuses the centralized configurable deadlines, response bounds, and retry schedule. No rollout flag changed. Identity remains fenced by workspace, agent session, execution runtime, connection generation, interaction ID, and exact option ID.