refactor(app): model pending questions as a live condition (#1199) - #1200
Conversation
Pure model + helpers for a single cross-project index of question tool calls currently awaiting the user, keyed by (directory, askSessionID, messageID, callID). Entries carry a resolved rootSessionID for badge attribution; helpers cover upsert (rising-edge add), retract by every removal path, authoritative directory reconcile (hydrate prune), distinct-root badge counting, and per-directory asking sessions for session-trim preserve. Backs moving question liveness out of the persisted notification store: badge/trim/alert derive from this index while the dock and sidebar render from child-store parts. No wiring yet.
A running, ready question part is the renderable truth for an open session, so the dock and sidebar 'asking' pip read it directly via findRunningExternalResultQuestion. Drop findDescendantExternalResultQuestion's pendingQuestions index fallback and isKnownStalePendingQuestion: reload/cold-open already recovers because hydratePendingExternalResults writes the /external-result trio's parts back into the cache. Removes the index's role as a dock backfill, leaving it free to become a pure cross-project condition signal. Tests drop the orphan-index cases (no message/part row) and keep the part-derived tree walk: active session, child fallback, out-of-tree isolation, terminal-part exclusion, transitive grandchildren.
Move "which question tool calls are waiting on the user" out of each child store's external_result_question map into one non-persisted global index owned by createPendingQuestionController. The controller ingests the SSE stream for every directory (covering background projects with no child store), reconciles the authoritative GET /external-result snapshot on bootstrap/reconnect, resolves each question's root session once, and emits a rising-edge alert for live arrivals (no consumer yet — notification wiring follows). The dock and sidebar already render purely from child-store parts (prior commit); this index now feeds only the cross-project signals parts cannot: session-trim preservation via preserveSessionIDs and root attribution. The hydrate path returns the active question list for the controller to reconcile and prunes stale local question parts so the part-derived dock stops showing answered-while-away questions. trimSessions swaps its externalResultQuestion option for a generic preserveSessionIDs set. Dedupe the duplicated GlobalStore type: bootstrap.ts imports the canonical one from ../global-sync instead of redeclaring a drifting copy (the drift is what first surfaced the missing pendingQuestions field). Verification: tsgo -b clean; bun test of global-sync, blockers, and layout suites (373 pass).
A question awaiting the user is a live condition, not a point-in-time event, so persisting it as a `type: "question"` notification was the root modeling error behind issue #1199: the persisted entry could outlive the question and strand a stale unread dot / Dock badge after the question was answered. Drop the persisted question notification entirely (the QuestionNotification type, its `ask` identity, dropQuestions, and the second alertedQuestionCalls dedupe lifecycle). The notification list now holds only turn-complete and error events. Question presence is already rendered live from child-store parts (sidebar "asking" pip) and tracked by the global pending-question index; this commit makes notification a consumer of that index instead of a parallel source of truth: - OS alert (sound / notification / Dock attention) fires from globalSync.onQuestionAlert — a single rising-edge signal the controller emits once per question and never on hydrate/reconnect, so a restart with an outstanding question does not re-nag. Root attribution and the answered-mid-resolution guard now live in the controller, not here. - The Dock badge unions unseen turn-complete/error sessions (launch-scoped) with the live pending-question roots (pendingRootSessionIDs, never launch-scoped) via the new pure badgeSessionCount helper. notification-derive.ts collapses to that one helper; the question lifecycle helpers it used to hold moved to the controller in the prior commit. Verification: tsgo -b clean; bun test (app package, 1803 pass); eslint clean on the changed source files.
github-actions
Bot
added
app
Astro-Han
added
task
📝 WalkthroughWalkthroughAdds a dedicated in-memory pending-question index + controller, wires bootstrap reconciliation and global SSE events into it, removes legacy external_result_question state, updates session-trim and blocker lookup to use derived pending roots, and refactors notifications to use live alerts plus unified badge counting. ChangesPending Questions Live Index and Alert System
Sequence DiagramsequenceDiagram
participant EventStream
participant PendingController
participant ChildStore
participant GlobalSDK
participant GlobalStore
participant NotificationSystem
EventStream->>PendingController: message.part.updated / other events
PendingController->>GlobalStore: upsert / remove pending question
PendingController->>ChildStore: resolveParentID(sessionID)?
alt child-store knows parent
ChildStore-->>PendingController: parent session id
else fallback
PendingController->>GlobalSDK: client.session.get(sessionID)
GlobalSDK-->>PendingController: session parent id | undefined
end
PendingController->>GlobalStore: setPendingQuestionRoot(...)
PendingController->>NotificationSystem: onAlert(...) (rising-edge)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/context/global-sync.test.ts, packages/app/src/context/global-sync.tsx, packages/app/src/context/global-sync/bootstrap.test.ts, packages/app/src/context/global-sync/bootstrap.ts, packages/app/src/context/global-sync/child-store.ts, packages/app/src/context/global-sync/event-reducer.test.ts, packages/app/src/context/global-sync/event-reducer.ts, packages/app/src/context/global-sync/external-result-question.ts, packages/app/src/context/global-sync/pending-question-controller.test.ts, packages/app/src/context/global-sync/pending-question-controller.ts, packages/app/src/context/global-sync/pending-question-index.test.ts, packages/app/src/context/global-sync/pending-question-index.ts, packages/app/src/context/global-sync/session-cache.test.ts, packages/app/src/context/global-sync/session-cache.ts, packages/app/src/context/global-sync/session-trim.test.ts, packages/app/src/context/global-sync/session-trim.ts, packages/app/src/context/global-sync/types.ts, packages/app/src/context/notification-derive.test.ts, packages/app/src/context/notification-derive.ts, packages/app/src/context/notification.tsx, packages/app/src/pages/layout/pawwork-session-commands.test.ts, packages/app/src/pages/layout/sidebar-items.tsx, packages/app/src/pages/session/blockers/running-external-result-question.ts, packages/app/src/pages/session/blockers/use-session-blockers.test.ts, packages/app/src/pages/session/blockers/use-session-blockers.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors pending question tracking by replacing the directory-specific external_result_question state with a global pendingQuestions index. This allows tracking questions across all directories, including background projects, and simplifies the dock/sidebar rendering to derive directly from the local message/part cache. Feedback is provided regarding a critical runtime TypeError in global-sync.tsx where accessing .session on existing?.[0] requires optional chaining to prevent crashes when existing is undefined.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
The dedupe key for in-flight root resolution joined directory and question id with a literal NUL byte, which made Git classify the whole controller file as binary (`Bin 0 -> 6423 bytes`) and hid it from line-level review and text tooling. Restore the intended printable space separator; runtime behaviour is unchanged (the key is only an internal Set lookup). Caught by codex review.
The persisted notification store key (notification.v1) is unchanged, so a `type:"question"` entry written by a pre-#1199 build can still be on disk after upgrade. buildNotificationIndex and pruneNotifications filtered only by `viewed`/time, so such an entry — even for an already-answered question — kept feeding the unread index and stranded a sidebar unread dot (the exact bug #1199 set out to remove, reintroduced via migration). Filter to live notification types (turn-complete | error) at the two load boundaries: buildNotificationIndex (so the unread index never counts a legacy question) and pruneNotifications (so the stale entries are dropped from disk on first load). Move the index builder + types into notification-derive.ts as pure helpers so the migration filter is unit-testable without rendering the provider. New tests: isLiveNotification, and buildNotificationIndex proving a persisted type:"question" entry yields zero session/project unread while a turn-complete still counts. Caught in code review.
1 participant
Summary
Rework how "a question is waiting on the user" is modeled across the app so the condition has a single source of truth. Before, the same condition was tracked twice: once per child store (
external_result_question, live + hydrate) and once as a persistedtype:"question"notification (drives badge + sidebar dot). The persisted copy is the modeling error — a point-in-time log entry standing in for a live condition, so its retraction had to be hand-maintained across every removal path, and any missed path left the unread dot / Dock badge claiming a session still needed input after the question was answered.The four commits:
feat(app): add global pending-question condition index— a pure, non-persisted modeldirectory → askSessionID → questions, keyed by the server identity(directory, askSessionID, messageID, callID).refactor(app): render question dock/sidebar purely from parts— the dock and sidebar "asking" pip derive only from child-store parts (the authoritative renderable truth for an open session); the orphan-index fallback is gone.refactor(app): drive pending questions from a global index— acreatePendingQuestionControlleringests the SSE stream for every directory (covering background projects with no child store), reconciles the authoritativeGET /external-resultsnapshot on bootstrap/reconnect, resolves each question's root session once, and emits a rising-edge alert. The child stores dropexternal_result_question;trimSessionsswaps its question-specific option for a genericpreserveSessionIDsset.refactor(app): model pending questions as a live condition, not an event— the notification layer drops the persisted question entirely and becomes a consumer of the index: OS alert viaonQuestionAlert, Dock badge via the union of unseen turn-complete/error sessions and live pending-question roots.Net result: parts are the renderable truth; the global index is the single cross-project condition signal (Dock badge, root attribution, trim preserve, hydrate prune, rising-edge alert); nothing about a question is persisted.
Why
Closes the root cause in #1199: a pending question is a live condition, not an event. Two parallel sources of truth for one condition meant the persisted side could drift from the live side, stranding a stale dot/badge. Collapsing to one non-persisted index that is rebuilt from the authoritative server snapshot on every reload removes the whole class of "forgot to retract" bugs.
Related Issue
Closes #1199
Human Review Status
Pending
Review Focus
message.part.updatedis intentionally not in the SSE replay buffer, so reload recovery for parts must come from hydrate (GET /external-result). The load-bearing pieces arehydratePendingExternalResults(returns the active list and prunes stale local running-ready parts, scoped to a pre-fetch snapshot so a question arriving mid-fetch is not pruned) and the controller'sreconcile.onQuestionAlertexactly once per question identity and never on hydrate/reconnect, so a restart with an outstanding question does not re-nag. The notification provider only decides surface/visibility.badgeSessionCount) and trim preservation (preserveSessionIDsviapendingSessionIDsForDirectory).Risk Notes
Notificationis nowturn-complete | erroronly, but the store key (notification.v1) is unchanged, so atype:"question"entry written by a pre-[Task] Notifications: model pending questions as live blockers (hydrate + full retraction) #1199 build can still be on disk after upgrade. Such entries are actively filtered out at the two load boundaries (buildNotificationIndexandpruneNotificationsviaisLiveNotification) so an already-answered question can never strand a sidebar unread dot, and they are dropped from disk on first load. Covered by a regression test.platform.setBadgeCount) and OS attention (platform.requestAttention) are macOS/Linux only; their call signatures are unchanged — only the badge count source changed. No updater/packaging/signing surface touched.How To Verify
tsgo -b(packages/app): clean.bun test(packages/app, full suite): 1806 passed, 0 failed (4142 assertions). New:pending-question-controller.test.ts(8),pending-question-index.test.ts(13),notification-derive.test.tsisLiveNotification+buildNotificationIndexlegacy-question regression (3). Rewritten:notification-derive.test.tsbadge union (9),bootstrap.test.tshydrate returns-active + part-prune,event-reducer.test.ts(question-index maintenance removed),session-trim/session-cache(preserveSessionIDs).type:"question"notifications on load (review P1).eslint(changed source files): clean.bun run snap sidebar-unread; reviewed grid PNG (light + dark) — unread dot renders correctly under the narrowedNotificationshape.bun run dev:desktopbooted clean (main/preload/renderer built, sidecar + server ready, init done) with the new notification + global-sync wiring; no init errors. Live Dock-badge increment + OS alert need a running agent question and are left to manual QA; their logic is covered by the controller rising-edge andbadgeSessionCountunit tests.Screenshots or Recordings
sidebar-unreadsnap (light + dark): the orange unread dot renders on the unread row; the viewed/selected row shows its timestamp. No visual regression — the sidebar JSX is unchanged; only the data feeding the unread dot was narrowed (questions now surface via the live "asking" pip, not the persisted dot).Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.