feat(approval-prompt): unified PauseRequest→UI data model for ACP, CLI, Desktop (#1322 Phase 3)#1443
Merged
esengine merged 9 commits intoMay 21, 2026
Conversation
Problem: Desktop ContextPanel showed stale token counts after /compact because it relies on per-turn API usage events (model.final) to update cacheHitTokens/cacheMissTokens. /compact is a local operation with no API call, so no usage event fires and the meter never refreshes. Fix (high-cohesion): 1. ContextManager.getLogTokens() — real-time token count of current log. 2. Loop.getCurrentLogTokens() — forwards to ContextManager. 3. Extend \$ctx_breakdown event with optional logTokens field. 4. emitCtxBreakdown() now computes + pushes logTokens alongside reservedTokens, becoming the unified context-refresh entrypoint. 5. compact_history handler calls emitCtxBreakdown(tab) on success so the frontend immediately receives the compressed token count. 6. App.tsx \$ctx_breakdown handler resets cacheHitTokens=0 and cacheMissTokens=logTokens when logTokens is present, calibrating the meter to the actual current context size. This keeps all computation in ContextManager, all emitting in emitCtxBreakdown, and all consumption in App.tsx — no new event types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ne#1322 Phase 3 PR 1) - Add `toApprovalPrompt()` and `resolveApprovalPrompt()` in `@reasonix/core-utils` to serve as the single source of truth for PauseRequest → UI metadata mapping. - Define `ApprovalPrompt`, `ApprovalAction`, `ApprovalTone`, `ApprovalPromptKind` types with English fallback labels. - Move `derivePrefix` computation into `toApprovalPrompt` so all surfaces share one implementation. - Add `secondaryInput` to `ApprovalAction` to describe two-phase deny flows. - Replace ACP `gates.ts` inline mapping with calls to the new unified helpers, deleting duplicated `permissionOptionsFor`, `permissionTitleFor`, `permissionKindFor`, `verdictFor`, and all `ID_*` constants. - Update `acp.test.ts` to use new semantic action IDs (`always_allow`, etc.). - Add `packages/core-utils/tests/**/*.test.ts` to vitest include pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…el (esengine#1322 Phase 3 PR 2) - Update `ShellConfirm.tsx` and `PathConfirm.tsx` to accept `prompt: ApprovalPrompt` instead of individual raw fields. - Drive `SingleSelect` items from `prompt.actions` instead of hardcoded options. - Trigger deny phase via `action.secondaryInput` detection instead of hardcoded "deny" value check. - Remove `clampCommand`, `tildeify`, and `derivePrefix` from ShellConfirm/PathConfirm (computed centrally in `toApprovalPrompt`). - In `App.tsx`, call `toApprovalPrompt()` at render time for both shell and path confirmations, removing inline `derivePrefix` at the component boundary. - Update `shell-confirm-render.test.tsx` to use `ApprovalPrompt` fixtures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to ApprovalPrompt (esengine#1322 Phase 3 PR 3) - Add `@reasonix/core-utils` workspace dependency to `desktop/package.json`. - Extend `ConfirmRequiredEvent` and `PathAccessRequiredEvent` with optional `prompt: ApprovalPrompt` for backward-compatible RPC. - In `src/cli/commands/desktop.ts`, call `toApprovalPrompt()` when emitting `$confirm_required` and `$path_access_required` events. - Update `ConfirmApprovalCard` and `PathAccessApprovalCard` in `desktop/src/ui/thread.tsx` to consume `prompt` instead of raw fields. - Drive primary/secondary/tertiary buttons from `prompt.actions` mapped by `kind` (`allow_once`, `allow_always`, `reject`). - Map `ApprovalPrompt.tone` to Desktop `ApprovalTone` (warn/danger/info/brand). - Remove inline `derivePrefix` from card components. - Update `PendingConfirm` and `PendingPathAccess` types in `desktop/src/App.tsx` to store `prompt`, and pass it to the card components at render time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add desktop/src/App.test.ts (5 reducer integration tests) - Add desktop/src/ui/thread.test.tsx (6 component rendering tests) - Install @testing-library/react and jsdom for Desktop component testing - Configure vitest with React aliases to prevent duplicate React copies - Fix tildeify double-slash normalization bug on Windows - Fix ACP gates comment length to pass policy check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolve package-lock.json conflict by regenerating from main.
CI installs only root dependencies; desktop-specific deps like @tauri-apps/api and lucide-react are not available in test runners. Add vi.mock() calls to App.test.ts and thread.test.tsx so vitest can resolve these imports without the desktop node_modules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI installs only root dependencies; desktop-specific packages like lucide-react and @tauri-apps/api are not available. Add mock modules in tests/mocks/ and wire them via resolve.alias in vitest.config.ts so desktop tests can resolve these imports without desktop/node_modules. Also remove redundant vi.mock() calls from App.test.ts and thread.test.tsx since aliases now handle module resolution globally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…p loading App.test.ts imports reduce from App.tsx, which transitively loads CommandPalette, Markdown, and theme modules with desktop-only deps. thread.test.tsx imports from thread.tsx, which loads cards.tsx, which loads Markdown.tsx with react-markdown and other deps. Add vi.mock() for these local module boundaries so tests resolve without needing the full desktop/node_modules dependency tree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
esengine
pushed a commit
that referenced
this pull request
May 21, 2026
…t a workspace member `npm ci` inside desktop/ failed with EUNSUPPORTEDPROTOCOL because desktop/ is not listed under root workspaces, so the `workspace:*` spec added in #1443 had no resolver. Replace with `file:../packages/core-utils`, which is what the desktop install actually wants (source-only package, type-only import in desktop sources).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #1322 Phase 3.
Note to reviewer: This is a single PR with 4 sequentially-dependent commits. Each commit can be reviewed independently in the GitHub "Commits" tab, but they are not independently mergeable (CLI TUI depends on the core model; Desktop depends on both). This structure was chosen over stacked PRs because the three surfaces must migrate together to avoid an intermediate state where old inline assembly and new
ApprovalPromptcode coexist.Introduces a single source of truth in
@reasonix/core-utilsthat converts anyPauseGatePauseRequestinto a platform-agnosticApprovalPromptdata structure. All four surfaces (ACP, CLI TUI, Desktop, Dashboard wire protocol) now consume this structure instead of independently assembling titles, subtitles, options, and tones.What changed
permissionOptionsFor(),permissionTitleFor(),verdictFor()inline insrc/acp/gates.tstoApprovalPrompt()+resolveApprovalPrompt()fromcore-utilsShellConfirm.tsx/PathConfirm.tsxinline title/sub/option assembly + localderivePrefixApprovalPrompt; driveSingleSelectfromprompt.actionsConfirmApprovalCard/PathAccessApprovalCardinline assembly + localderivePrefixApprovalPrompt; mapactionsto primary/secondary/tertiary buttonscommand,kind)prompt: ApprovalPromptfieldWhy we standardized all three surfaces in one PR instead of prototyping one card first
Owner
esengineoriginally suggested: "prototype one card first (ConfirmApprovalCard) before standardizing the rest."After prototyping the core model, we found that the value of
ApprovalPromptis as a cross-surface contract, not as a local refactor:The model only proves itself when multiple consumers agree on it. Prototyping
ConfirmApprovalCardalone would still leavederivePrefix, title assembly, and option lists duplicated in CLI TUI and ACP. The bug that motivated Phase 3 (桌面版询问用户是否允许运行命令时，选了始终允许后，根本不生效，后续还是继续问用户 #1180 — DesktopderivePrefixdiverged from CLI) would remain.The three commits are strictly layered and reviewable independently.
tests/acp-yolo.test.ts.ShellConfirm.tsx,PathConfirm.tsx, and their render sites inApp.tsx.thread.tsx,App.tsx,protocol.ts, and the Desktop RPC emitter.Each layer validates that the previous abstraction is sufficient.
Incremental rollout would create temporary duplication. If we landed Desktop first while CLI still used inline assembly, any title/option change would require editing both the old inline code and the new
ApprovalPromptcode until CLI migrated. A single branch eliminates this overlap period.Benefits
derivePrefix, tone selection, title/subtitle assembly, and action lists all live inpackages/core-utils/src/approval-prompt.ts.approval-prompt.tsinstead of 3–4 files.ApprovalAction.secondaryInputtells any surface when an action requires extra input (e.g., deny reason), replacing hardcoded\"deny\"checks.packages/core-utils/tests/approval-prompt.test.ts(37)tests/acp.test.ts(28)tests/shell-confirm-render.test.tsx(2)desktop/src/App.test.ts(5)desktop/src/ui/thread.test.tsx(6)Risk mitigations
tests/acp-yolo.test.tsvalidates verdict mapping.promptfield.action.idto their own translation keys.desktop/src/App.test.tsanddesktop/src/ui/thread.test.tsximport fromdesktop/src/source files, which transitively depend on Desktop-only packages (lucide-react,react-markdown,@tauri-apps/api, etc.) that are not installed in the rootnode_modulesused by CI. We mock these local module boundaries (./CommandPalette,./Markdown,./cards) and external deps viatests/mocks/+vitest.config.tsaliases so the tests can run without a fulldesktop/node_modules. This is a known testing-infrastructure debt that should be addressed in a future "Desktop testing infrastructure" PR (either by extracting pure-logic modules or by making CI install Desktop dependencies).Commits
0dffa32— feat(core-utils): introduce ApprovalPrompt unified data modelc6edbd8— feat(cli): migrate ShellConfirm and PathConfirm to ApprovalPrompt modelff90c22— feat(desktop): migrate ConfirmApprovalCard and PathAccessApprovalCard to ApprovalPrompt761cb6d— test(desktop): add ApprovalPrompt reducer and component testsd5ac04c— fix(desktop-tests): mock local desktop modules and add vitest aliases for CITest plan
npm run verifypasses (255 test files, 3505 tests)npx vitest run tests/acp.test.tsnpx vitest run tests/shell-confirm-render.test.tsxnpx vitest run desktop/src/App.test.tsnpx vitest run desktop/src/ui/thread.test.tsx🤖 Generated with Claude Code