Handle non-resumable pending user input#2766
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
github-actions
Bot
added
vouch:unvouched
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR expands “stale/unknown pending user input request” detection to handle additional error message variants (including the Codex-specific wording) and updates tests accordingly.
Changes:
- Added new substring matches for “unknown pending user input request” (no hyphen) and “unknown pending codex user input request”.
- Updated web and server logic to recognize the new error detail formats.
- Updated tests to use the new Codex error detail string.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/session-logic.ts | Extends stale/unknown pending request detection to include Codex wording variants. |
| apps/web/src/session-logic.test.ts | Updates test fixture detail string to match new provider error format. |
| apps/server/src/orchestration/Layers/ProviderCommandReactor.ts | Broadens matching for unknown pending user-input errors (including Codex variant) with lowercasing. |
| apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts | Updates test to assert behavior using the new Codex error detail string. |
| apps/server/src/orchestration/Layers/ProjectionPipeline.ts | Extends pending user input count logic to include new “unknown pending …” variants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const detail = error.detail.toLowerCase(); | ||
| return ( | ||
| detail.includes("unknown pending user-input request") || | ||
| detail.includes("unknown pending user input request") || | ||
| detail.includes("unknown pending codex user input request") | ||
| ); | ||
| } | ||
| return Cause.pretty(cause).toLowerCase().includes("unknown pending user-input request"); | ||
| const message = Cause.pretty(cause).toLowerCase(); | ||
| return ( | ||
| message.includes("unknown pending user-input request") || | ||
| message.includes("unknown pending user input request") || | ||
| message.includes("unknown pending codex user input request") | ||
| ); |
There was a problem hiding this comment.
a helper feels like the wrong thing, these really should not be string matching so much... there's not enough use of types in this app and state drifts constantly as a result
ApprovabilityVerdict: Approved Straightforward bug fix that adds additional error message patterns to detect non-resumable user input requests. The changes are mechanical string pattern additions with no new capabilities or behavioral changes beyond improved error detection. The review comment about code duplication is a style concern, not a correctness issue. You can customize Macroscope's approvability policy. Learn more. |
mjc
force-pushed
the
pending-user-input-fix
branch
from
84c9167 to
e724823
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
What Changed
Why
Submitting answers to a stale/non-resumable pending user-input request could look like it did something, then leave the user stuck on the same prompt. This keeps the fix small and consistent with the existing string-matching recovery shape already used for stale pending provider callbacks.
This is also one instance of a broader state-modeling problem. I want to make these invalid states unrepresentable; there are about a dozen bugs like this or more that I run into regularly, and they make the app pretty unusable. I would be down to do the sweeping refactor required to address state drift between the UI and the server, and between different parts of the UI itself, instead of relying on projected stale state recovery.
Testing:
bun fmtbun lint(existing warnings only)bun typecheckbun run testChecklist
Note
Low Risk
Low risk: changes are limited to broadened string-matching for provider error details and additional tests, with no schema or control-flow refactors.
Overview
Handles non-resumable user-input callbacks as stale. The server
ProviderCommandReactorand projection pipeline now recognize additional "unknown pending user input"/"unknown pending codex user input" error phrasings and convert them into stale failures that clear pending user-input state.Aligns UI derivation and tests. Web session logic is updated to treat the same error details as stale, and new/updated tests cover the Codex wording plus a new projection test asserting
pending_user_input_countis cleared after the failure.Reviewed by Cursor Bugbot for commit e724823. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Handle non-resumable pending user input by recognizing additional provider error messages
Extends error detection in three places to treat
'unknown pending user input request'and'unknown pending codex user input request'(case-insensitive) as signals that a pending user-input request cannot be resumed:isUnknownPendingUserInputRequestErrornow matches these new phrases in both the error detail and cause message.derivePendingUserInputCountFromActivitiestreats failure activities containing these phrases as closing the corresponding request, zeroingpending_user_input_count.isStalePendingRequestFailureDetailin the web client applies the same matching so stale requests are filtered from displayed pending inputs.Macroscope summarized e724823.