Repository navigation
Show Claude's usage-limit question in the desktop chat - #1140
alexeyzimarev merged 4 commits into
Conversation
The menu is terminal chrome and the choices depend on the account, so the daemon reads the live screen. A choice sends the printed digit, and a chat message is refused while the menu is up.
PR Summary by QodoSurface Claude usage-limit choices in desktop chat
AI Description
Diagram
High-Level Assessment
Files changed (21)
|
Code Review by Qodo
1. Old menu clicks send digits
|
| /// the option set depends on the account, and the transcript never records it. A match is the | ||
| /// title plus at least two of the choices the CLI actually offers, so a sentence that merely | ||
| /// quotes one of them does not raise a question. | ||
| internal sealed class ClaudeUsageLimitDetector { |
There was a problem hiding this comment.
4. Claude parser sits outside its harness 📘 Rule violation ⌂ Architecture
ClaudeUsageLimitDetector embeds Claude-only prompts and choices while declaring the generic Capacitor.Cli.Daemon.Services namespace. The detector is instantiated only for Claude terminal agents, while analogous vendor parsers live under Harness/<Vendor>, so future vendor-specific changes are split across two locations.
Agent Prompt
## Issue description
`ClaudeUsageLimitDetector` contains Claude-specific terminal parsing but is located in the generic daemon services directory, contrary to the required vendor harness organization.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ClaudeUsageLimitDetector.cs[1-91]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[3043-3045]
## Recommended Fix
Move the detector to `src/Capacitor.Cli.Daemon/Harness/Claude/ClaudeUsageLimitDetector.cs`, change its namespace to `Capacitor.Cli.Daemon.Harness.Claude`, and update the orchestrator and tests to import the new namespace.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
This scraper is called from the orchestrator read loop, next to ConsentDialogDetector, which already reads the same Claude PTY from Services. Harness/Claude is the launcher and the policies. Moving only this type would split the two scrapers.
| async Task ChooseUsageLimitAsync(int index) { | ||
| if (index is < 1 or > 9) return; | ||
| UsageLimitError = ""; | ||
| var sent = await _input.SendKeyAsync((byte)('0' + index), _lifetimeToken); | ||
| if (!sent && !_lifetimeToken.IsCancellationRequested) | ||
| UsageLimitError = "The terminal is not attached, so that choice was not sent."; |
There was a problem hiding this comment.
3. Old menu clicks send digits 🐞 Bug ≡ Correctness
ChooseUsageLimitAsync receives only an option index and sends it without checking that the notice and option which created the command are still current. A status update can clear or replace the menu after a click is dispatched, allowing that stale callback to write its digit into the replacement menu or ordinary terminal input.
Agent Prompt
Issue description
A choice callback can outlive the usage-limit menu that created it and write a stale digit to the terminal.
Fix Focus Areas
- src/Capacitor.App/ViewModels/ChatTabViewModel.cs[654-675]
- src/Capacitor.App/ViewModels/TerminalTabViewModel.cs[218-226]
Recommended Fix
Associate every choice command with a menu generation or immutable notice snapshot. Cancel its token when `ApplyUsageLimit` replaces or clears that snapshot, and verify the generation immediately before raw input is sent, including after waiting for an in-flight delivery.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
A second click on the same notice no longer sends: the choice stays taken until the notice changes, including across the wait for an in-flight paste. A click that starts after the notice has been replaced is a new command. A send that has already passed that check is not cancelled if the menu disappears while a paste is draining. Escape has the same shape.
The digit is written before the menu leaves the screen, so another click would land on the next prompt.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d115cd7d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.WhenAnyValue(x => x.HasUsageLimitQuestion), | ||
| (text, can, readOnly, limit) => can && !readOnly && !limit && !string.IsNullOrWhiteSpace(text)); |
There was a problem hiding this comment.
Recheck the usage-limit gate after attachment uploads
The new limit check only participates in CanExecute at command start. If a send with attachments is already awaiting UploadAsync when Claude renders the usage-limit menu, execution resumes and calls _input.SendAsync without checking HasUsageLimitQuestion again; for a terminal-backed chat that pastes the original message into the live Claude menu instead of sending a chat turn. Recheck the limit immediately before dispatching the prompt (and leave the draft/chips staged) after any await.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The send now checks the menu again after the upload, before the text is dispatched. The draft and the chips stay staged. A_send_still_uploading_when_the_usage_limit_appears_is_not_pasted_into_the_menu holds the upload, raises the menu, then releases it.
| } | ||
| } | ||
|
|
||
| return new UsageLimitNoticeDto(UsageLimitKinds.Blocked, summary ?? "Usage limit reached", Prompt, options); |
There was a problem hiding this comment.
Require an actual limit banner before surfacing choices
A normal Claude response that happens to print What do you want to do? followed by two numbered known choices is accepted as a blocking menu even if no usage/limit/reset banner is present: summary is optional and this fallback creates a usage-limit notice. This can occur when a user asks Claude to reproduce or explain the menu, and it disables the composer and routes clicks as raw menu keys until the screen changes. Require the adjacent limit banner (or another menu-specific terminal marker) before returning a notice.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
A notice now requires the limit line above the title. The title and two known choices without that line are not a question, including when the choice text itself contains "limit". A_title_and_two_choices_without_a_limit_line_is_not_a_question pins it.
The upload can finish after the menu is on screen, and the text would be typed into it. The draft and the chips stay put.
The title and two known choices are also what a session prints when asked to show the menu. The limit line above the title is what makes it the real prompt.
No GitHub issue — AI-3151
What & why
Claude's session-limit prompt exists only as a terminal menu, so the desktop session stays running and a chat message is pasted into that menu. The daemon reads the live screen and the chat shows the same question. A choice sends the digit Claude printed. Sending a message is refused until the menu leaves the screen.
The option set depends on the account (stop and wait, wait and continue, ask an admin, add funds, upgrade), so the card uses the lines on screen rather than a fixed list.
Where to look
A match is the limit line above the title, plus two known choices, on the current screen. Cursor movement overwrites cells, so a stripped scrollback would keep a menu that is already gone. Other harnesses do not ask this question; the notice stays empty for them.
Note: The daemon running this build has to be the one attached. An older daemon never publishes the notice, so the chat still looks like an ordinary session while the menu is up.
Verification
dotnet run --project test/Capacitor.Cli.Daemon.Tests.Unit -- --treenode-filter "/*/*/ClaudeUsageLimitDetectorTests/*"— 7 passed.dotnet run --project test/Capacitor.Cli.Core.Tests.Unit -- --treenode-filter "/*/*/StatusIpcJsonTests/*"— 21 passed.dotnet run --project test/Capacitor.App.Tests.Unit -- --treenode-filter "/*/*/ChatTabViewModelTests/A_usage_limit*"— 2 passed.A_send_still_uploading_when_the_usage_limit_appears_is_not_pasted_into_the_menu— 1 passed. The smoke test of the usage-limit card — 1 passed.TrayViewModelTests— 60 passed.DesktopNotificationCoordinatorTests— 32 passed.SessionStatusDotsTests— 10 passed.ChatComposerTests— 10 passed.