fix(threads): keep completed questions closed across clients - #241
fix(threads): keep completed questions closed across clients#241leoisadev1 wants to merge 1 commit into
Conversation
Web and mobile each reduced pending approvals and questions, so a request answered on one client could reopen on the other when activities arrived out of order. Stopping a turn also left native callback questions pending after the session was ready. Share derivePendingRequests in client-runtime, including Akeru toolName/args and Codex stale-callback fragments. On turn completion or abort, record user-input.resolved for still-open native questions on that turn and leave message-mode questions alone. Adapted from T3 Code pingdotgg#10123 and pingdotgg#9851. Made with Grok 4.6 High in Grok Build via Orca.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
vouch:trusted
Greptile SummarySummary
Merge safetyDo not merge until native-question cleanup preserves ordinary response failures so terminal processing can resolve the outstanding question. The approval-detail and lifecycle-query concerns are non-blocking, but should be addressed to preserve clear approval prompts and avoid thread-history-dependent cleanup cost. Confidence Score: 4/5Not safe to merge until terminal native-question cleanup resolves questions after ordinary response failures. A reproduced terminal-state defect can leave clients with a stale pending question. Two additional reproduced concerns affect approval clarity and terminal cleanup efficiency. Files Needing Attention: apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts requires correction before merge. packages/client-runtime/src/pendingRequests.ts and apps/server/src/persistence/Layers/ProjectionThreadActivities.ts need follow-up.
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const options = Array.isArray(payload.options) | ||
| ? payload.options.filter(isProviderApprovalOption) | ||
| : []; | ||
| const args = payload.args ?? toolArgsByCallId.get(requestId); |
There was a problem hiding this comment.
When approval.requested arrives before its matching tool.started, the reducer looks up arguments before they have been recorded and never enriches the existing approval later. The web approval panel then displays the generic Command approval label instead of the command or routine being approved. This is a non-blocking usability regression that makes approvals harder to evaluate.
Artifacts
- A narrow TypeScript reproduction invokes the current pending-approval reducer with matching approval and tool activities in both caller orders, showing the order-dependent behavior.
- The captured command displays the authored reproduction that compares tool-before-approval with approval-before-tool activity order, documenting the executed test scope.
- The executed reducer output preserves `git status --short` when the tool arrives first but renders `Command approval` when approval arrives first, confirming the regression.
| } else if ( | ||
| activity.kind === "user-input.resolved" || | ||
| activity.kind === "provider.user-input.respond.failed" | ||
| ) { | ||
| pendingRequestIds.delete(requestId); |
There was a problem hiding this comment.
A normal provider.user-input.respond.failed response currently removes the native request ID from terminal cleanup. When that turn subsequently completes or aborts, no user-input.resolved activity is added, so clients continue showing a question from a turn that has already ended. Only failures identifying a stale or unknown request should remove the ID before terminal cleanup.
Artifacts
- The authored Vitest case drives a native request, ordinary response failure, and terminal failed turn; it asserts the captured broken service contract.
- The executed targeted Vitest command completed with exit code 0 and logged zero terminal resolutions with pending user input still true, confirming the defect.
| FROM projection_thread_activities | ||
| WHERE thread_id = ${threadId} | ||
| AND kind IN ( | ||
| 'user-input.requested', | ||
| 'user-input.resolved', | ||
| 'provider.user-input.respond.failed' | ||
| ) | ||
| ORDER BY | ||
| CASE WHEN sequence IS NULL THEN 0 ELSE 1 END ASC, | ||
| sequence ASC, | ||
| created_at ASC, | ||
| activity_id ASC |
There was a problem hiding this comment.
Terminal cleanup loads, decodes, and orders user-input lifecycle records from every turn in the thread, then discards requests that do not belong to the terminal turn. With 1,000 historical turns, the query returned 2,001 rows and required a temporary sort even though only one current-turn row was needed. This is a non-blocking cost that grows with thread history; filter by turn_id and add an index for the thread/turn lifecycle lookup.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
- The authored Bun SQLite script seeds historical turns and executes both the current query and a turn-filtered indexed comparison, showing the scope difference.
- The captured command prints the authored validation script that creates the matching schema and performs the query-plan checks, showing the executed setup.
- The executed current-query run returned 2,001 lifecycle rows across 1,001 turns and used only the thread-created index with a temporary sort, showing history-proportional work.
- The executed comparison returned one row for the terminal turn and used the composite covering index, showing the bounded alternative.
- The captured source command prints the terminal-cleanup caller, the changed query, and the two shipped activity indexes, showing that the terminal turn is known but not queried.
- The targeted provider-runtime test was started but did not complete within the validator execution window and was terminated, so it supplies no pass/fail conclusion.
|
This is Leo's agent. Independent verification confirms the ordinary-response-failure blocker on ea5eb8c. A focused regression creates a native question through provider ingestion, dispatches an ordinary network answer-failure activity, completes the turn, drains ingestion and reads the projected thread shell. hasPendingUserInput remains true instead of false. The regression fails in 99 ms; the 205 existing focused reducer and ingestion tests pass. The existing worker already has the scoped repair request but remains blocked by 503 errors and exhausted weekly quota. No repair or merge is claimed. |
1 participant
Problem
Web and mobile each reduced pending approvals and questions. Mobile missed some stale-failure fragments and
dynamic_tool_callmapping, so a request answered on one client could stay open on another when activities arrived out of order. Stopping a turn also left native callback questions pending after the session was already ready.Live bot and group chats use
useBotThreadRuntimeanduseGroupThreadRuntime, not ChatView.Changes
packages/client-runtimeasderivePendingRequests.toolNameand toolargson pending approvals.user-input.resolvedfor still-open native questions on that turn. Message-mode questions stay pending.Adapted from T3 Code pingdotgg#10123 and pingdotgg#9851. Not a bulk cherry-pick. No async-question UI, compaction, or restart continuation.
Verification
vp test run packages/client-runtime/src/pendingRequests.test.ts apps/web/src/session-logic.test.ts apps/mobile/src/lib/threadActivity.test.ts --maxWorkers=2: 114 tests after the shared-module fix; 113 passed on the first run except one malformed-option case that was then corrected.vp test run packages/client-runtime/src/pendingRequests.test.ts apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts --maxWorkers=2: 101 passed, including turn completed/interrupted/failed/aborted dismissals and preserved newer plus message-mode questions.client-runtime, web, mobile, and server typechecks passed (existing Effect suggestions only).No paid provider session was used. Browser verification is not claimed for this server/client-reducer change.
Implemented with Grok 4.6 High in Grok Build via Orca.