fix(server): recover stale Codex approval callbacks - #5195
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 Plus 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 |
ApprovabilityVerdict: Approved 80d72a4 This is a straightforward bug fix that extends error detection to recognize an additional error message pattern for Codex approval callbacks. The change is self-contained: it adds a new string pattern to an existing error detection function and normalizes case sensitivity. No new behavior paths are introduced. You can customize Macroscope's approvability policy. Learn more. |
## What's Changed * fix(server): recover stale Codex approval callbacks by @luckyPipewrench in pingdotgg/t3code#5195 ## New Contributors * @luckyPipewrench made their first contribution in pingdotgg/t3code#5195 **Full Changelog**: pingdotgg/t3code@v0.0.36-nightly.20260827.1204...v0.0.36-nightly.20260827.1205 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.36-nightly.20260827.1205
## What's Changed * fix(grok): improve skills, plans, usage, and turn reliability by @t3dotgg in pingdotgg/t3code#8358 * fix(server): recover stale Codex approval callbacks by @luckyPipewrench in pingdotgg/t3code#5195 * test(server): remove duplicate missing worktree test by @t3-code[bot] in pingdotgg/t3code#8252 * fix(server): replay all un-applied events during projection bootstrap by @krutftw in pingdotgg/t3code#7538 * test: remove low-signal test files by @t3-code[bot] in pingdotgg/t3code#8397 * test: prune trivial error and layout tests by @t3-code[bot] in pingdotgg/t3code#8400 * Fix Android adaptive launcher icon by @colonelpanic8 in pingdotgg/t3code#4332 * feat(web): split provider settings into list and editor by @t3dotgg in pingdotgg/t3code#8380 * fix(codex): accept Codex 0.150 account plans by @gsimone in pingdotgg/t3code#8447 * fix(tooling): allow ignored-only staged changes by @juliusmarminge in pingdotgg/t3code#8468 * fix(mobile): keep iOS home header stable by @juliusmarminge in pingdotgg/t3code#8467 * fix(web): stop showing red x summaries for ordinary tool failures by @t3dotgg in pingdotgg/t3code#8395 * fix(mobile): refine Git action toast glass styling by @juliusmarminge in pingdotgg/t3code#8399 * fix(desktop): allow preview automation in agent-created threads by @t3dotgg in pingdotgg/t3code#8483 * test(web): remove redundant cache key test by @t3-code[bot] in pingdotgg/t3code#8484 * fix(release): move nightly schedule to minute 38 by @t3dotgg in pingdotgg/t3code#8509 * fix(web): stabilize the provider settings editor by @t3dotgg in pingdotgg/t3code#8472 * fix(web): open GitHub pull requests in browser when loading fails by @t3dotgg in pingdotgg/t3code#8507 * fix(codex): show sub-agent models by @t3dotgg in pingdotgg/t3code#8502 * feat(analytics): report connected client platforms by @t3dotgg in pingdotgg/t3code#8481 * feat(server): accept PDF, ZIP, and other file uploads up to 50MB by @t3dotgg in pingdotgg/t3code#8235 * feat(web): toggle a thread's pin from the keyboard by @ipanasenko in pingdotgg/t3code#8440 * fix(web): add back button to project settings by @StiensWout in pingdotgg/t3code#8168 * refactor(mobile): compile semantic themes for Uniwind by @juliusmarminge in pingdotgg/t3code#7327 * fix(desktop): Cache Runtime locally on WSL Filesystem, dramatically improving launch times by @ikifar2012 in pingdotgg/t3code#5769 * fix(mobile): show OpenCode model sources in picker by @juliusmarminge in pingdotgg/t3code#8573 * fix(clients): honor project default models in new threads by @anirudhsama in pingdotgg/t3code#6011 * fix(mobile): show file actions on Android by @none23 in pingdotgg/t3code#8215 * fix(connect): explain DPoP connection failures by @extoci in pingdotgg/t3code#8351 * feat(web): make the sidebar project filter a searchable combobox by @SunkenInTime in pingdotgg/t3code#5931 * fix(server): a draft can retry its first send after a failed bootstrap by @shivamhwp in pingdotgg/t3code#8226 * fix(desktop): stop hidden previews draining battery by @Bil0000 in pingdotgg/t3code#8567 * fix(desktop): oauth popups open from the browser preview by @walid-baharwal in pingdotgg/t3code#8435 * fix(web): keep long task drawers usable on small screens by @shivamhwp in pingdotgg/t3code#8313 * fix(opencode): handle child approvals, stops, and model catalogs by @t3dotgg in pingdotgg/t3code#8480 * fix: make thread auto-settling opt-in by @shivamhwp in pingdotgg/t3code#8321 * fix(web): stop session activity timing test from blocking releases by @t3dotgg in pingdotgg/t3code#8585 * fix(mobile): show composer menus when starting a task by @juliusmarminge in pingdotgg/t3code#8587 * fix(web): show the configured stash shortcut by @UtkarshUsername in pingdotgg/t3code#8437 * feat(web): add toggleable confirmation before unpinning a thread by @UtkarshUsername in pingdotgg/t3code#7313 * fix: restore automatic thread settling defaults by @t3dotgg in pingdotgg/t3code#8596 * fix(mobile): restore composer glass and rounded shadows by @juliusmarminge in pingdotgg/t3code#8597 ## New Contributors * @luckyPipewrench made their first contribution in pingdotgg/t3code#5195 * @krutftw made their first contribution in pingdotgg/t3code#7538 * @colonelpanic8 made their first contribution in pingdotgg/t3code#4332 * @ikifar2012 made their first contribution in pingdotgg/t3code#5769 * @walid-baharwal made their first contribution in pingdotgg/t3code#8435 **Full Changelog**: pingdotgg/t3code@v0.0.35...v0.0.36 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.36
Merges 58 upstream commits (`badae6a5c` → `6a9d9f988`, upstream v0.0.34 → v0.0.36) through the `fork-upstream-merge` skill. `merge-stats.mjs` reconciles exactly: **431 files landed** against **431 in the upstream range**, no gap to explain. Fork delta is 611 files. ## Conflicts Three, plus one git resolved silently and wrongly. - **`Sidebar.tsx`** — upstream replaced the project picker's `Menu`/`MenuRadioGroup` with a searchable `Combobox` (pingdotgg#5931). Took upstream's structure and re-applied only the `FEATURES.projectManagement` gate on "New project", now carrying the `// Fork:` marker it had been missing. Upstream has since grown the mobile touch-target span itself, so the Mobile Touch Delta has nothing left to re-apply here — one convergence, unprompted. - **`ProviderSettingsPanel.tsx`** — upstream split provider settings into list and editor (pingdotgg#8380, pingdotgg#8472), moving `ProviderLastChecked` and the refresh button out of `headerAction` into the list footer. Took upstream's and re-applied the gate as `!readOnly && FEATURES.serverAdministration`. - **`pnpm-lock.yaml`** — took upstream's and re-ran `vp i`, which restored the fork's `@t3tools/moatless-api` workspace edge. - **`packages/contracts/src/orchestration.test.ts`** — auto-merged, no marker, broken. Both sides appended `OrchestrationMessage` to the same import list and the same `decodeOrchestrationMessage` const at different offsets, so git took both. Surfaced as a parse error in lint, typecheck and test at once. This is the case `merge-stats.mjs`'s conflict-candidate list exists to catch. `HostedBrowserFrame.tsx` (fork-only) also needed the `renderingActive` prop pingdotgg#8567 made required. Upstream suspends a parked webview unless background audio, PiP or a recording still needs it painted; a frame has none of those to read and is the app's only copy of the preview page, so it passes `true` and keeps today's behavior. ## Sweep Five keyword hits, all false positives: - `apps/web/src/connection/clientMetadata.ts` + test — reports the client's OS/browser/device on connect (pingdotgg#8481). Auth-adjacent, but it rides `ClientPresentation` on the relay and remote-bearer bootstraps; the fork's primary environment sends none of it. - `packages/client-runtime/src/relay/errorPresentation.ts` + test, `connection/errors.test.ts` — explain DPoP failures (pingdotgg#8351). Relay only, and T3 Connect is decided out. ## Feature classification **Usable as-is** - Searchable project-filter combobox in the sidebar (pingdotgg#5931). - Long task drawers stay usable on small screens (pingdotgg#8313) — directly relevant, the fork's phone story is `apps/web` in mobile Safari/Chrome. - Toggleable confirmation before unpinning a thread (pingdotgg#7313); toggle thread pin from the keyboard. - Back button in project settings (pingdotgg#8168); the configured stash shortcut is shown (pingdotgg#8437). - No more red-x summaries for ordinary tool failures (pingdotgg#8395); PRs open in the browser when loading fails (pingdotgg#8507). - Project default models are honored in new threads (pingdotgg#6011). - Provider settings split into list and editor (pingdotgg#8380, pingdotgg#8472) — landed, though `/settings/providers` is itself gated behind `serverAdministration`. **Unsupported in Moatless / needs implementation** - **Non-image file attachments** (pingdotgg#8235) — a turn may now carry any file up to 50MB, advertised as `capabilities.fileAttachments.maxUploadBytes` and sized by `PROVIDER_SEND_TURN_MAX_FILE_BYTES`. Moatless advertises neither this nor `attachmentUploads`, so the composer's attach affordance stays off. Costs nothing today (upstream's own web composer offers images only; `ChatAttachment` widened just far enough to typecheck), but it is the second capability key to report when uploads land. Extends the existing **Attachment uploads** entry in `docs/fork/gaps.md`. - **Connected-client platform analytics** (pingdotgg#8481) and **DPoP failure explanations** (pingdotgg#8351) — relay and T3 Connect surfaces, already decided out of the fork. No new WS methods entered the contract in this range, so no new `UnsupportedMethodError` union entries. `unsupported-methods.mjs` reports `ADD 0`, `DROP 1` (`scripts.run`), `KEEP 2`. The `scripts.run` DROP is **not** actioned — it is the documented exception in the gaps register: that union entry answers for `apps/server`, which still stubs the method (verified surviving in `ws.ts`), not for Moatless. **Backend behavior to consider reproducing in Moatless** - **Replay all un-applied events during projection bootstrap** (pingdotgg#7538) — a one-line pipeline fix upstream; Moatless runs its own projection and the same class of bug applies. - **A draft can retry its first send after a failed bootstrap** (pingdotgg#8226) — Moatless owns turn start, and a draft wedged by a failed bootstrap is the same dead end here. - **Recover stale Codex approval callbacks** (pingdotgg#5195) — Moatless owns the agent runtime in the sandbox. - **Stop querying Claude context usage after turns** (pingdotgg#8610) — a cheap correctness/cost fix in the same place Moatless polls. - **Server-side accept and size-limit non-image uploads** (pingdotgg#8235), the other half of the gap above. - Auto-settling churn (pingdotgg#8321 made it opt-in, pingdotgg#8596 reverted) nets to no change — noted so the next merge does not re-derive it. ## Verification `verify.mjs`: `tripwires`, `fmt:check`, `lint`, `typecheck` pass. `test` reports `@t3tools/web` **flaky, passed alone** — in the full run `MessagesTimeline.test.tsx` skipped all 34 tests on a 30s module-import timeout under `ChatMarkdown.tsx`; alone the package is 297 files / 3117 tests green. Load, not the merge. `inventory-check.mjs` clean. Model: Claude Opus 5, via Claude Code in a Moatless sandbox. --- Moatless task: https://moatless.soaplabstest.com/tasks/a0c041bb-1426-4591-9296-6a4b0cfa2eff
What Changed
Normalize Codex's provider-specific missing approval callback error into T3 Code's existing provider-neutral stale approval failure at the provider command reactor boundary.
This keeps web, desktop, mobile, projection counts, and thread settling on the existing shared recovery path without teaching clients about a Codex-specific error string.
Why
Approval cards are durable, but provider callbacks are in memory. After a Codex session restarts or is recovered, responding to an old card can return
Unknown pending Codex approval request. That error was not recognized by the approval classifier, so T3 persisted the raw failure and left the impossible approval pending.The normalized failure clears the stale request without emitting
approval.resolvedor executing the rejected command.Checklist
Note
Low Risk
Small change to error-string classification in the approval response path; behavior aligns with existing stale-approval recovery and does not alter auth or data handling.
Overview
Extends the provider command reactor’s stale approval classifier so Codex’s in-memory callback errors are treated like existing “unknown pending approval” failures.
isUnknownPendingApprovalRequestErrornow matchesunknown pending codex approval request(alongside the generic approval/permission strings), andCause.prettymatching is case-insensitive so mixed-case provider details still classify correctly. When matched, failures still surface as the sharedStale pending approval request: …activity—not a fakeapproval.resolvedand not executing the command.The reactor test was updated to simulate Codex’s
item/requestApproval/decisionerror shape and asserts thatprovider.approval.respond.failedcarries the normalized stale detail and that noapproval.resolvedactivity is emitted.Reviewed by Cursor Bugbot for commit 80d72a4. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix
isUnknownPendingApprovalRequestErrorto recover stale Codex approval callbacksExtends the detection logic in
isUnknownPendingApprovalRequestErrorin ProviderCommandReactor.ts to match the phrase'unknown pending codex approval request'in addition to the existing phrase. The pretty-printed cause message is now lowercased before matching, making all checks case-insensitive.Macroscope summarized 80d72a4.