feat(web): add ACP permission cards to project chat - #2200
simple-agent-manager[bot] wants to merge 8 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 |
Playwright UI evidenceAll captures use the production project-chat components with mocked production HTTP boundaries. The fixture stresses long content, special characters, anchored and unanchored requests, 30+ lifecycle rows, creator/noncreator access, detail failure, and revoked access. The implementation agent and UI specialist reviewed every capture for header clearance, wrapping, clipping, horizontal overflow, readability, and responsive behavior; no blocking visual issues remain.
Evidence commit: |
|
Parent review of 1abff4b: changes requested before integration.
Security positives: browser answers use the Cloudflare route; exact option IDs are preserved and validated; secure detail is fetched outside persistent query state; noncreators receive generic cards. These do not resolve the issues above. CI is green at the reviewed head. This review was source tracing plus inspection of posted Playwright captures, not a fresh test execution. Staging, integrated runtime validation, and final parent re-review remain outstanding. Keep the PR draft and unmerged. |
Parent review fixes — ready for draft-only handoffFixed all findings from the parent review at
Updated visual evidence
Evidence commit: Validation
The PR remains draft and unmerged. I did not deploy or mutate staging, mark the PR ready, trigger final CodeRabbit review, touch runtime task |
|
Coordinator re-review of e687505:
This pass reviewed source, tests, and posted images; I did not rerun the suites locally. Full Test CI is still in progress. Final head review and integrated staging remain required; keep draft/unmerged. |
|
Final review-fix handoff
The PR remains open and draft. No staging deployment or mutation, production flag change, ready-for-review transition, merge, force push, API/VM change, or runtime-task branch change was performed. |
|
Final CI is green on |
|
Coordinator review of e13a166: remaining mobile clearance finding resolved. I personally inspected all five updated Playwright captures at evidence commit b153245 (320px/375px saved status and retry state, plus desktop). Status text and retry controls now clear the floating jump control; exact option labels remain intact and desktop spacing is preserved. The source change is scoped to permission content clearance and the compact visible retry label, with the exact option retained in its accessible name. No additional blocking source or visual finding in this pass. Full final-head Test CI is still running. Integrated VM/Instant permission roundtrip, staging evidence, and final release gates remain required. This is source/screenshot review, not a local rerun or merge approval. Keep draft until those gates are complete. |
Final ACP clearance evidenceExact head: The implementation at
Parent review: I personally inspected all six captures. Status text, uncertain copy, and retry controls remain readable and hit-testable beside jump-to-latest; option actions retain their prior rectangle and center-point checks. No horizontal overflow, clipping, or excessive retry-label wrapping was found. Validation on final working tree:
UI rubric: visual hierarchy 5/5; interaction clarity 5/5; mobile usability 4/5; accessibility 5/5; system consistency 5/5. The PR remains draft and unmerged. No staging deployment/mutation, ready transition, production flag change, API/VM change, or force push was performed. |
|
|
Final exact-head verification is complete for
The PR remains draft. No merge, readiness change, deployment, staging mutation, or production flag change was performed. |





















Summary
Project chat now reads durable ACP permission snapshots from Cloudflare and renders permission cards beside their originating tool call, with an unanchored live-tail fallback. Session creators can fetch transient no-store detail and submit an exact option ID through the existing Cloudflare answer route; other viewers see generic waiting state only.
The UI distinguishes pending, answered, delivery-confirmed, delivery-unconfirmed, expired, cancelled, and interrupted states. It also handles refresh/reconnect, deadline expiry, revoked access, two-tab conflict recovery, and lost-receipt retry with the original idempotency key. The orphan callback-based
PermissionDialogwas removed frompackages/acp-client.Contract reconciliation from current
main:GET /api/projects/:projectId/sessions/:sessionId/interactionsGET /api/projects/:projectId/sessions/:sessionId/interactions/:interactionIdPOST /api/projects/:projectId/sessions/:sessionId/interactions/:interactionId/answerRecord<string, unknown>on main. The browser accepts only exactAcpInteractionOptionSchemaoptions and never infers malformed choices.optionIdis the selected choice authority. The browser sends SHA-256 of that exact ID asanswerHash; runtime task01M3RF53QVR8ZWK446ZSZAFWB6must preserve that contract.Validation
pnpm lintpnpm typecheckpnpm test.claude/rules/47-control-loop-io-budget.md)Scoped validation:
pnpm --filter @simple-agent-manager/web test -- project-message-view.test.tsx project-chat.test.tsx useAcpPermissionInteractions.test.tsx acp-permission-card.test.tsx— 148 passedpnpm --filter @simple-agent-manager/web test -- acp-permission-card.test.tsx— 24 passed after the status/retry clearance follow-uppnpm --filter @simple-agent-manager/web exec playwright test tests/playwright/acp-permission-chat-audit.spec.ts --project='iPhone SE (375x667)'— 12 passedpnpm --filter @simple-agent-manager/web typecheck— passedpnpm --filter @simple-agent-manager/web lint— passed with 3 pre-existing warningspnpm --filter @simple-agent-manager/web build— passedpnpm --filter @simple-agent-manager/acp-client typecheck,lint, andbuild— passed before the final web-only hardening commitpnpm quality:file-sizes,pnpm quality:type-boundaries, andgit diff --check— passedpnpm quality:gitleaks:currentcould not complete because scanner output was withheld by workspace policy.36701147517attempt 2 — passed on review-fix head88da9a3022b6e351ef887534a428a75682983b16; the retry cleared an unrelated tool-rail pointer-event flake from attempt 1.36704715011— passed on final heade13a166b5b14860b99922211927dc1e0941087a4, including Playwright Visual Tests.Staging Verification (REQUIRED for all code changes — merge-blocking)
Deploy Stagingworkflow triggered manually and passed for this branchapp.sammy.party(staging) using test credentials and actively tested the applicationStaging Verification Evidence
Staging is intentionally deferred under Raphaël's scoped authorization for delegated pieces. The coordinator will arrange integrated staging with runtime bridge task
01M3RF53QVR8ZWK446ZSZAFWB6before shipment. This draft did not deploy, mutate shared staging, or activate flags.UI Compliance Checklist (Required for UI changes)
.codex/tmp/playwright-screenshots/UI Screenshot Evidence
Surface: Project chat ACP permission cards
End-to-End Verification (Required for multi-component changes)
.claude/rules/10-e2e-verification.md)Data Flow Trace
useAcpPermissionInteractionsreads the existing Cloudflare snapshot throughlistAcpInteractions, refreshes on realtime attention changes, recovers missed events on a bounded configurable cadence, and keeps the fast delivery-state cadence while the server pending bucket is nonempty.useAcpPermissionPlacementassociatestoolCallIdwith production chat rows; requests with no resolvable anchor go to the Virtuoso footer.AcpPermissionCardgates secure content on exact creator status, pending state, and deadline.useAcpPermissionCardfetches no-store detail, validates exact options, hashes the clicked exact ID, creates the answer key, and callsanswerAcpInteraction.Untested Gaps
The production chat capability is covered through mocked HTTP boundaries and real production components. A real ACP runtime fixture and integrated live staging remain for coordinator verification with runtime task
01M3RF53QVR8ZWK446ZSZAFWB6; no runtime bridge exists in this branch by scope.Post-Mortem (Required for bug fix PRs)
Parent review found missed mounted-chat requests, a pre-hash choice race, and mobile action overlap in the initial feature slice.
What broke
New permissions could remain invisible after an empty snapshot; two deferred-digest clicks could submit different choices; jump-to-latest covered the mobile Allow target.
Root cause
Refresh depended on reconnect/pending state, the choice guard started after async hashing, and permission controls did not reserve the floating control footprint.
Class of bug
Realtime invalidation, async interaction atomicity, and responsive hit-target collision.
Why it was not caught
The original tests lacked empty-to-pending realtime transitions, deferred digest concurrency, and measured mobile hit-target geometry.
Process fix included in this PR
Added real component contract tests, deterministic deferred promises, bounded request-count assertions, and Playwright rectangle plus center-point hit tests at 320px, 375px, and desktop.
Post-mortem file
N/A.
Specialist Review Evidence (Required for agent-authored PRs)
VITE_ACP_PERMISSION_RECOVERY_POLL_MSandVITE_ACP_PERMISSION_QUERY_RETRY_COUNTare consistently typed, documented, and resolved; no secret/deployment mapping appliesCodeRabbit Review Evidence (Required for agent-authored PRs)
CodeRabbit Notes
Deferred. Per the explicit handoff constraint, this PR must remain draft and must not be marked ready. CodeRabbit is requested only after the other gates and staging are complete and the PR is no longer draft; the coordinator owns that step.
Exceptions (If any)
01M3RF53QVR8ZWK446ZSZAFWB6supplies the real ACP fixture.Agent Preflight (Required)
Classification
External References
N/A: this slice reuses internal contracts from merged foundation PRs #2182/#2187 and approved v2 idea
01M3P2E0JJNQRXX020P65ZRKEJ; it adds no external API.Codebase Impact Analysis
apps/web: API client, permission snapshot hook, transient secure-detail/answer state machine, real chat placement/rendering, focused tests, and Playwright visual fixtures.packages/acp-client: removes the unused callback-basedPermissionDialogand export.packages/ui: unchanged.apps/api,packages/shared, andpackages/vm-agent: unchanged; existing contracts are consumed as-is.Documentation & Specs
Task contract and acceptance evidence are recorded in
tasks/active/2026-09-30-acp-permission-chat-ui.md. No public docs changed because global activation and runtime delivery remain outside this slice.Constitution & Risk Check
Checked Principles IV and XI plus UI, security, request-budget, and file-size rules. Polling uses a typed/documented environment override and stops in hidden tabs. Sensitive detail stays in transient component state behind creator/pending/deadline gates. Options preserve exact backend IDs/order with no preselection. The browser has no VM response path. Focused extraction keeps production functions below 50 lines and
SessionMessageView.tsxbelow 500 lines.