Greptile P1: when a new approval/clarify replaces the per-session pending
entry, the displaced prompt's notification-dedupe key was never retired.
Cleanup only removed the key of the currently stored prompt, and since
dedupe entries no longer expire on a wall-clock TTL, a stale key persisted
forever - so the same externally supplied prompt ID re-surfacing later was
wrongly suppressed as already-notified.
_rememberApprovalPending() and _rememberClarifyPending() now retire the
displaced entry's notify key on owner change. A re-render of the SAME
owner serializes to the same typed tuple key, so repeated poll ticks stay
deduped.
Regression: node-VM test drives displacement, same-owner re-render, and
clear retirement for both approval and clarify paths (fails on the prior
head, passes here).
Round-2 review fixes for the prompt-notification deduper:
- The dedupe key is now the JSON.stringify of the typed owner tuple
[kind, sid, prompt id, run owner id, mirror token]. Plain delimiter
concatenation let approval_id='x\0r\0t' (no gateway owner) collide with
approval_id='x' + run 'r' + token 't'; the gateway accepts approval_id
as an unrestricted string, so the second distinct owner was suppressed.
- Seen entries retire on prompt LIFECYCLE via the pending-clear
chokepoints (_clearApprovalPendingForSession/_clearClarifyPendingForSession
now call _retirePromptNotifyKey) instead of a 10-minute wall-clock TTL,
so a prompt left pending for hours never re-notifies while a resolved
prompt whose ID is reused legitimately notifies again.
- Entries are no longer recorded when notifications are disabled, so
enabling mid-prompt re-notifies that owner instead of the disabled
call permanently consuming it.
New regressions: key injectivity with delimiter-bearing approval_id,
24h-still-pending no-renotify, resolved-owner ID reuse, and
disabled-to-enabled retry. Full file: 15 passed (11 pre-existing).
The dedupe key was kind + prompt id only. The approval lifecycle explicitly
permits the same externally supplied approval_id in multiple sessions and in
multiple gateway runs within one session (the same identity _notifyPromptCard's
neighbors use via _approvalOwnerForPending), so the second logical owner's
prompt was silently suppressed: no notification, card still blocking the run.
Key is now kind + sid + prompt id + gateway owner (run_id +
_gateway_mirror_token when both present), mirroring the approval-owner
identity used elsewhere in this file. Repeated SSE/poll/render calls for the
same full owner stay deduped.
Regression coverage (node-VM executed): same approval_id across two sessions
notifies both; same session + id under two distinct gateway run owners
notifies twice; repeat calls for one full owner notify once. Fails on the
pre-fix key (second session suppressed), passes with it.
Live testing on the PPC exposed the real reason unfocused-tab prompt
notifications never appeared: _notifyPromptCard correctly decided to
notify, but sendBrowserNotification's own gate
(_isBackgroundedForBrowserNotification — notify only when
document.hidden) vetoed the unfocused-but-visible case. The chokepoint
has already made the visibility decision; pass forceHidden so its
decision survives, while the notifications-enabled setting is still
honored.
Case 1 was missed: an approval on a NON-active session was suppressed
while the user was focused on a different session's tab — even though
they had no idea the prompt existed. The gate now suppresses only when
the prompt's own session is actively viewed (open in pane + tab visible
+ focused, via the existing _isSessionActivelyViewed). All three user
cases now notify: non-active session, hidden tab, unfocused window.
Approval/clarify prompts block the run until answered, but only the
active-session SSE event ever sent a browser notification — and it was
gated on document.hidden at delivery time, which background-tab SSE
throttling makes unreliable (the same #4416 class fixed for completion
notices). The 1.5s fallback poll, the post-respond 'next approval'
refresh, and reload-while-pending showed the card silently.
Notification ownership moves into the card renderers
(showApprovalCard / showClarifyCard) via _notifyPromptCard: per-id
dedupe (poll ticks re-render without re-pinging), suppressed only while
the tab is visible AND focused (an unfocused-but-visible window still
pings, since nothing progresses until the prompt is answered).
The SSE done handler previously used assistantText.slice(0, 100), which
captures the start of the live-stream accumulator (early tokens and interim
updates) rather than the final answer shown after tools settle.
Add _completionNotificationPreviewText() using anchor final-answer projection
and settled message content, with a cleaned live-stream fallback. Hoist
lastAsst so background completions on non-active panes still get preview text.
Update PWA notification regression tests.
Closes#5035
Codex SHIP-WITH-FIXES: non-done terminal paths (apperror/cancel/stream-error
finalize) didn't clear _STREAM_WAS_HIDDEN, so a stale wasHidden=true could be
inherited by a later same-sid reconnect -> spurious notification. Make each
entry stream-owned ({streamId, wasHidden}) so a stale entry from a prior stream
is never trusted (reconnect keeps state only when streamId matches; done reads
via the owned entry), and add _clearStreamHidden(activeSid, streamId) on the
apperror/cancel/finalize-fallback terminal paths (belt-and-suspenders).
A background tab's SSE is throttled by Chromium, so the stream 'done' event
arrives late (after the user returns, document.hidden=false) and the
response-complete notification was suppressed by the live visibility check.
Track per-stream whether the tab was hidden at ANY point during the stream
(_STREAM_WAS_HIDDEN, set at attach, OR'd by one idempotent visibilitychange
listener, read+cleared at done) and pass forceHidden accordingly.
sendBrowserNotification bypasses ONLY the live visibility gate on forceHidden,
still honoring the user's notifications-enabled setting — so a watched stream
stays silent (matches Slack/Discord/Gmail/Claude) but a backgrounded one
notifies. Correct fix for the #4416 root cause; supersedes #4421's always-fire
approach (which would also fire when the tab was never hidden).
Co-authored-by: bergeouss <bergeouss@users.noreply.github.com>
ai... rodboev's #4133, notification-feature ONLY (dropped the unrelated
'invalidate sidebar cache on WAL' commit — that's a separate good fix to split
out). Button now reflects live Notification.permission instead of silent no-op.
Co-authored-by: nesquena-hermes <agent@nesquena-hermes>
Co-authored-by: rodboev <rodboev@users.noreply.github.com>
* Add PWA notification controls (#3229, #3196)
Rebased onto fresh master (#3229 was 281 commits behind). Routes browser
notifications through the service worker (reg.showNotification) with a direct
Notification fallback, threads the originating session deep link/tag into the
payload, and adds Enable/Send-test/permission-status controls in Settings.
notificationclick prefers an already-open target-session tab before opening a
new window. Added the 6 new i18n keys to the Polish (pl) locale block, which
the original PR missed.
Co-authored-by: AJV20 <AJV20@users.noreply.github.com>
* Apply Opus review fixes to #3229 PWA notifications
- MUST-FIX: _showPwaNotification used navigator.serviceWorker.ready, which
never settles when no SW registration ever activates (reverse-proxy MIME
mishap, SW disabled) — every notification was silently dropped, a regression
vs master. Switch to getRegistration() raced against a 2s timeout, falling
back to new Notification().
- SHOULD-FIX: notificationclick matched client.url === targetUrl exactly, but
_sessionUrlForSid copies the current query/hash into the deep link, so an
already-open session tab missed the match and spawned a duplicate window.
Compare pathnames instead.
- SHOULD-FIX: "Send test" with permission denied was a silent no-op — now
surfaces the notifications_denied toast on the force path.
- SHOULD-FIX: permission-status span went stale after granting via the button
— refresh it in requestNotificationPermission().then.
- Consistency: the request-permission grant branch now has the same
new Notification fallback as the granted branch.
- Update test_pwa_notification_controls.py to assert the pathname match.
* Stamp v0.51.346 — Release LJ (PWA notification controls #3229/#3196)
---------
Co-authored-by: nesquena-hermes <[email protected]>
Co-authored-by: AJV20 <AJV20@users.noreply.github.com>
CharlesMcquade