Skip to content

fix(review): prevent lost provider events and expose stalled progress - #53

Merged
BarretoDiego merged 1 commit into
mainfrom
fix/ai-review-progress
Sep 8, 2026
Merged

BarretoDiego merged 1 commit into
mainfrom
fix/ai-review-progress

Conversation

@BarretoDiego

Copy link
Copy Markdown
Owner

AI review could remain on “Analyzing batch 1 of 2” for 20 minutes even after the provider finished or requested approval. The review executor subscribed directly to the adapter’s single-consumer event queue, competing with ProviderService and other reviews. Its filter discarded events consumed for other threads, while approvals and completion events could be taken by another consumer.

Read-only diagnosis of the reported PR #52 review found the last Codex event at 18:49:31 local time: waitingOnApproval, followed by a command approval request. The UI still claimed Working. An earlier review’s provider log contained turn completion while its review record eventually timed out.

Changes

  • Subscribe to ProviderService’s canonical broadcast stream, filtered by thread and provider instance, preserving independent delivery to normal thread processing and reviews.
  • Keep the isolated read-only session and decline additional permissions. Surface failed approval responses and event-consumer failure/closure instead of silently waiting; bound approval responses to 10 seconds. The overall 20-minute deadline now includes session startup and turn admission.
  • Use reasoning events as public liveness signals without exposing their contents. Update activity timestamps/duration and close outstanding activity rows when an invocation ends.
  • Show elapsed time, last update age, a warning after a minute without updates, explicit cancellation progress/errors, and current stages in the history selector. Render history timestamps in the client’s local timezone. Avoid claiming that a silent provider is actively inspecting code.

Validation

  • 19 backend AI review tests and 3 review text tests passed.
  • The executor regression uses a real single-consumer Queue feeding a broadcast with an ordinary consumer alongside the reviewer, verifying that both receive completion and approvals are declined. Covers approval failure, activity callback failure, stream closure and startup timeout with virtual time.
  • Server, web, desktop and mobile typechecks passed. Targeted lint, formatting and diff checks passed.
  • Web, server and Electron builds passed; isolated development server started successfully with the new dependency wiring.
  • Browser inspection of the actual status/timeline components with an explicit waiting-state fixture verified elapsed time, last update age and the stale-progress warning. This was a visual fixture, not a new live provider review.

The existing live review and installed application were not modified or restarted. Cancel the old review and retry after deploying the updated server/client. No review comments were published. The one-minute warning indicates missing updates, not proof that the provider process has deadlocked.

Implemented with GPT-6 through the Codex harness.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Sep 8, 2026
@BarretoDiego
BarretoDiego merged commit e3617b3 into main Sep 8, 2026
7 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant