|
| 1 | +# ACP Parity Completion - Execution Spec v2 |
| 2 | + |
| 3 | +Date: 2026-02-13 |
| 4 | +Status: Implementation-ready |
| 5 | +Scope: Finish remaining OpenCode ACP parity with minimal blast radius |
| 6 | + |
| 7 | +## 1. Goal |
| 8 | + |
| 9 | +Complete the remaining ACP migration gaps so the app behaves consistently for foreground chat, background helper prompts, model/effort selection, plan updates, and UI/docs surfaces. |
| 10 | + |
| 11 | +## 2. Ground Truth (Inspected) |
| 12 | + |
| 13 | +- ACP spawn/init is live: `src-tauri/src/backend/app_server.rs:330`, `src-tauri/src/backend/app_server.rs:490` |
| 14 | +- ACP event translation exists: `src-tauri/src/backend/event_translator.rs:94` |
| 15 | +- Core thread methods already use ACP: `src-tauri/src/shared/codex_core.rs:77`, `src-tauri/src/shared/codex_core.rs:148`, `src-tauri/src/shared/codex_core.rs:379` |
| 16 | +- Background helpers still call Codex protocol (must migrate): `src-tauri/src/shared/codex_aux_core.rs:280`, `src-tauri/src/shared/codex_aux_core.rs:325`, `src-tauri/src/shared/codex_aux_core.rs:390` |
| 17 | +- `session/update` translation currently emits to sink; background callback routing for translated events is missing: `src-tauri/src/backend/app_server.rs:421` |
| 18 | +- `plan` updates are not mapped; unknown session updates are dropped: `src-tauri/src/backend/event_translator.rs:194` |
| 19 | +- Frontend already handles `turn/plan/updated`: `src/features/app/hooks/useAppServerEvents.ts:294` |
| 20 | +- Stale Codex path text remains in settings: `src/features/settings/components/sections/SettingsCodexSection.tsx:586` |
| 21 | + |
| 22 | +## 3. Product Decisions (Locked) |
| 23 | + |
| 24 | +- Hidden helper sessions: hide locally by default in thread list. |
| 25 | +- URL images: support enabled with strict fetch policy. |
| 26 | +- Model/effort set failure: continue prompt send, warn once. |
| 27 | + |
| 28 | +## 4. Risk Register |
| 29 | + |
| 30 | +- R1 [CRIT]: Prompt event cross-wiring if multiple `session/prompt` overlap per workspace. |
| 31 | + - Mitigation: serialize prompt sends per workspace session (single in-flight prompt lock). |
| 32 | +- R2 [CRIT]: Background helper deadlock if translated events never hit callback channel. |
| 33 | + - Mitigation: route translated events by `threadId` to `background_thread_callbacks` before sink emit. |
| 34 | +- R3 [CRIT]: SSRF/oversize/invalid content from URL image fetch. |
| 35 | + - Mitigation: strict URL + network + MIME + timeout + byte-cap policy. |
| 36 | +- R4 [HIGH]: False `turn/completed` on prompt failure. |
| 37 | + - Mitigation: emit `turn/completed` only on success; emit error path otherwise. |
| 38 | +- R5 [HIGH]: ACP model-setting method uncertainty. |
| 39 | + - Mitigation: capability probe + fallback; never block prompt. |
| 40 | + |
| 41 | +## 5. Implementation Plan (PR-sized) |
| 42 | + |
| 43 | +### PR1 - Background helper ACP migration |
| 44 | + |
| 45 | +Files: |
| 46 | +- `src-tauri/src/shared/codex_aux_core.rs` |
| 47 | + |
| 48 | +Changes: |
| 49 | +- Replace `thread/start` with `session/new` in `run_background_prompt_core`. |
| 50 | +- Replace `turn/start` with `session/prompt`. |
| 51 | +- Remove ACP-nonexistent `thread/archive`; use local hidden-session registry cleanup only. |
| 52 | +- Keep background thread hide behavior (`codex/backgroundThread`) unchanged. |
| 53 | + |
| 54 | +Acceptance: |
| 55 | +- Commit message generation and run metadata generation both return valid text. |
| 56 | +- No protocol error logs from old method names. |
| 57 | + |
| 58 | +### PR2 - Route translated updates to background callbacks |
| 59 | + |
| 60 | +Files: |
| 61 | +- `src-tauri/src/backend/app_server.rs` |
| 62 | + |
| 63 | +Changes: |
| 64 | +- In `session/update` branch, after translation: |
| 65 | + - inspect translated event `params.threadId` |
| 66 | + - if callback exists for thread id, send to callback channel |
| 67 | + - otherwise emit to app event sink |
| 68 | +- Preserve existing behavior for non-`session/update` notifications. |
| 69 | + |
| 70 | +Acceptance: |
| 71 | +- Background helper collects streamed deltas and terminal signal without requiring UI sink path. |
| 72 | + |
| 73 | +### PR3 - Prompt lifecycle correctness + prompt serialization |
| 74 | + |
| 75 | +Files: |
| 76 | +- `src-tauri/src/backend/app_server.rs` |
| 77 | +- `src-tauri/src/shared/codex_core.rs` |
| 78 | + |
| 79 | +Changes: |
| 80 | +- Add per-`WorkspaceSession` prompt mutex/guard to allow one in-flight prompt at a time. |
| 81 | +- In `send_user_message_core`, enforce lifecycle: |
| 82 | + - emit `turn/started` before send |
| 83 | + - on prompt error, emit error event and return error payload/result |
| 84 | + - emit `turn/completed` only after successful prompt return |
| 85 | + |
| 86 | +Acceptance: |
| 87 | +- No `turn/completed` emitted for failed prompt paths. |
| 88 | +- Concurrent send attempts are serialized and do not mix item/turn ids. |
| 89 | + |
| 90 | +### PR4 - Model/effort parity (best-effort) |
| 91 | + |
| 92 | +Files: |
| 93 | +- `src-tauri/src/shared/codex_core.rs` |
| 94 | + |
| 95 | +Changes: |
| 96 | +- Compare requested model against cached current model. |
| 97 | +- If changed, attempt ACP model-set request before `session/prompt`. |
| 98 | +- If unsupported/failure, continue prompt and emit one warning per workspace/session. |
| 99 | +- Map effort only if ACP supports variant/effort semantics in current protocol response metadata. |
| 100 | + |
| 101 | +Notes: |
| 102 | +- Method name/shape is UNVERIFIED; implementation must include fallback and not hard fail. |
| 103 | + |
| 104 | +Acceptance: |
| 105 | +- Prompt send always proceeds when model-set fails. |
| 106 | +- Warning emitted once for repeated failures. |
| 107 | + |
| 108 | +### PR5 - URL image strict fetch pipeline |
| 109 | + |
| 110 | +Files: |
| 111 | +- `src-tauri/src/shared/codex_core.rs` |
| 112 | + |
| 113 | +Changes: |
| 114 | +- In `build_acp_prompt_parts`, for `http/https` image URLs: |
| 115 | + - allow only `http`/`https` |
| 116 | + - block localhost/link-local/private CIDR targets |
| 117 | + - enforce timeout |
| 118 | + - enforce max response size |
| 119 | + - require image MIME |
| 120 | + - base64 encode bytes to ACP `image` part |
| 121 | +- On policy violation, return explicit user-facing error. |
| 122 | + |
| 123 | +Acceptance: |
| 124 | +- Valid public image URL works. |
| 125 | +- Blocked host/type/size/timeout produce deterministic error. |
| 126 | + |
| 127 | +### PR6 - Complete event translation for plan updates |
| 128 | + |
| 129 | +Files: |
| 130 | +- `src-tauri/src/backend/event_translator.rs` |
| 131 | + |
| 132 | +Changes: |
| 133 | +- Add `sessionUpdate: "plan"` mapping to `turn/plan/updated`. |
| 134 | +- Payload contract: |
| 135 | + - `params.threadId` |
| 136 | + - `params.turnId` |
| 137 | + - `params.explanation` |
| 138 | + - `params.plan` |
| 139 | +- Ensure shape remains compatible with `normalizePlanUpdate`. |
| 140 | + |
| 141 | +Acceptance: |
| 142 | +- Frontend plan UI updates with ACP plan events. |
| 143 | + |
| 144 | +### PR7 - Hidden helper session filtering |
| 145 | + |
| 146 | +Files: |
| 147 | +- `src-tauri/src/shared/codex_core.rs` |
| 148 | +- supporting local state module(s) as needed |
| 149 | + |
| 150 | +Changes: |
| 151 | +- Persist helper-created session IDs in local workspace metadata. |
| 152 | +- Filter those IDs from default `list_threads_core` output. |
| 153 | +- Keep data reversible (toggle/debug path can expose all sessions later). |
| 154 | + |
| 155 | +Acceptance: |
| 156 | +- Background helper sessions do not appear in normal thread list by default. |
| 157 | + |
| 158 | +### PR8 - Frontend and docs cleanup |
| 159 | + |
| 160 | +Files: |
| 161 | +- `src/features/settings/components/sections/SettingsCodexSection.tsx` |
| 162 | +- `src/services/tauri.ts` |
| 163 | +- `README.md` |
| 164 | +- `docs/app-server-events.md` |
| 165 | + |
| 166 | +Changes: |
| 167 | +- Remove/hide Codex-only UI/API surfaces not used in MVP (skills/collab/rate-limit/login paths). |
| 168 | +- Correct stale config path copy to OpenCode path. |
| 169 | +- Update README status to current migration phase. |
| 170 | +- Update events doc to ACP reality and current supported mapping. |
| 171 | + |
| 172 | +Acceptance: |
| 173 | +- No stale Codex path language in settings/docs for global config location. |
| 174 | +- Removed surfaces no longer shown in main user flows. |
| 175 | + |
| 176 | +## 6. Test Plan |
| 177 | + |
| 178 | +### Rust unit tests |
| 179 | + |
| 180 | +- `event_translator.rs` |
| 181 | + - `plan` -> `turn/plan/updated` |
| 182 | + - chunk parsing keeps meaningful whitespace behavior |
| 183 | +- `app_server.rs` |
| 184 | + - translated `session/update` routed to callback when callback exists |
| 185 | +- `codex_aux_core.rs` |
| 186 | + - background helper success/error/timeout cleanup |
| 187 | +- `codex_core.rs` |
| 188 | + - model-set attempted only when model changes |
| 189 | + - model-set failure does not block send |
| 190 | + - URL image policy allow/deny cases |
| 191 | + - turn lifecycle success vs failure |
| 192 | + |
| 193 | +### Integration tests |
| 194 | + |
| 195 | +- Foreground chat + background helper in same workspace without event cross-talk |
| 196 | +- Hidden helper sessions excluded from thread list |
| 197 | +- Plan updates visible in existing UI path |
| 198 | + |
| 199 | +### Manual verification |
| 200 | + |
| 201 | +- Start prompt, force error, verify no false completed state |
| 202 | +- Generate commit message and run metadata successfully |
| 203 | +- Send prompt with URL image (valid + blocked cases) |
| 204 | +- Change model, verify best-effort behavior with one warning on failure |
| 205 | + |
| 206 | +### Validation commands (to run during implementation) |
| 207 | + |
| 208 | +- `npm run typecheck` |
| 209 | +- `npm run test` |
| 210 | +- `cd src-tauri && cargo check` |
| 211 | +- `cd src-tauri && cargo test` |
| 212 | + |
| 213 | +## 7. Rollout and Backout |
| 214 | + |
| 215 | +### Rollout |
| 216 | + |
| 217 | +- Stage 1: PR1-PR3 (correctness baseline) |
| 218 | +- Stage 2: PR4-PR6 (feature parity) |
| 219 | +- Stage 3: PR7-PR8 (cleanup + docs) |
| 220 | + |
| 221 | +### Monitoring |
| 222 | + |
| 223 | +- Count unknown dropped ACP `sessionUpdate` types |
| 224 | +- Count model-set failures |
| 225 | +- Count URL image policy rejections by reason |
| 226 | +- Track prompt lifecycle mismatches (`started` without terminal event) |
| 227 | + |
| 228 | +### Backout |
| 229 | + |
| 230 | +- Disable URL image fetch path and revert to explicit validation error mode. |
| 231 | +- Disable hidden-session filtering if visibility regressions are reported. |
| 232 | +- Disable model-set pre-call while keeping prompt send path intact. |
| 233 | + |
| 234 | +## 8. Out of Scope |
| 235 | + |
| 236 | +- Reworking reducer contracts or protocol translation into frontend. |
| 237 | +- Broad naming refactors of internal `codex_*` modules. |
| 238 | +- New daemon/app feature surfaces outside ACP parity gaps. |
| 239 | + |
| 240 | +## 9. UNVERIFIED Items and How to Verify |
| 241 | + |
| 242 | +- ACP model-set RPC name and payload |
| 243 | + - Verify by wire-capture against current `opencode acp` while switching model. |
| 244 | +- ACP effort/variant semantics |
| 245 | + - Verify using `session/new` metadata and successful model/variant switch response. |
| 246 | +- ACP session archive/delete capabilities |
| 247 | + - Verify protocol docs or wire capture before designing non-local archival behavior. |
0 commit comments