[satyalyadav]

Windows drops thread toasts when their tag is the 73-character environment/thread identity. The shared web/Electron notification constructor now hashes that identity to a stable 16-character tag, preserving replacement for repeat alerts and separation between threads and environments. Native click/reveal behavior is unchanged.

Original fix by @satyalyadav in #12289. Contributor history is preserved and main is merged. Scott’s native Windows follow-up independently verified short-tag delivery and A/B/A replacement on the earlier implementation; that is historical evidence, not a new test of this checkout.

Verification at e790dc063a:

• Focused tests reduced from 42 to 39 across three existing files: removed two helper cases and one duplicate scenario, folding the regression into the existing badge integration scenario. It covers real UUIDs, same-thread replacement, different threads and environments, badge counts, and focus clearing. All five existing sound tests are retained.
• One independent full-diff review round (Opus 5.5 and GPT-6 Sol) found no blockers in this PR. The separate pre-existing PR-guide notification tag remains outside scope. All current-head CI jobs that ran passed; skipped jobs are non-blocking. Macroscope approved the pushed head, and all historical review threads are resolved.
• Without the fix, the regression receives the 73-character tag and fails. With it, all 39 tests pass. Web typecheck and scoped lint/format/diff checks pass.
• Running Linux app with real Codex completions: actual main ab6be13d43 supplied a 73-character tag. Candidate supplied 6c6f3f8f5bf49bc1 twice (16 characters); the old notification closed on replacement and the new one stayed open. Captured before/after API payloads. Capture intercepts the Notification constructor, including its permission gate; native OS presentation is not exercised. No renderer layout changes require visual comparison.
• macOS, Windows, iOS, Android, and Electron desktop were not run. Fresh native Windows toast delivery and replacement still need platform verification; the separate native reveal bridge is outside this PR.

Tests: Codex gpt-6-astra (high). Implementation and live proof: Codex gpt-6.1-sol (high). Independent full-diff reviews: Claude claude-opus-5-5 and Codex gpt-6-sol. Harness: Codex and Claude Agent through T3 Code.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[macroscopeapp]

Approvability

Verdict: Approved at e790dc0

Macroscope's review found this PR approvable — This is a small, self-contained web notification bug fix that replaces overlong Windows toast tags with stable 16-character tags while preserving thread replacement and badge behavior. Production and browser-test changes are limited to this notification path, with regression coverage for distinct threads, environments, and repeat alerts.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 477a1c06-7a41-46a3-8fb1-9ee52b3084b0

📥 Commits

Reviewing files that changed from the base of the PR and between 4749035 and 13ebd25.

📒 Files selected for processing (4)

• apps/web/src/components/ThreadNotificationCoordinator.test.tsx
• apps/web/src/components/ThreadNotificationCoordinator.tsx
• apps/web/src/threadNotifications.test.ts
• apps/web/src/threadNotifications.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The change adds a bounded hash-based notification tag helper. Desktop notification coordination now uses explicit generated tags for pending-notification replacement. Tests cover tag generation and updated coordinator expectations.

Changes: Thread notification tags

Layer / File(s)	Summary
Bounded tag generation apps/web/src/threadNotifications.ts, apps/web/src/threadNotifications.test.ts	Adds a typed FNV-1a helper that returns stable 16-character hexadecimal tags. Tests verify length, stability, and distinctness.
Coordinator tag integration apps/web/src/components/ThreadNotificationCoordinator.tsx, apps/web/src/components/ThreadNotificationCoordinator.test.tsx	The coordinator receives explicit tags, keys pending notifications by those tags, derives tags with the helper, and updates desktop notification assertions.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: bil0000

Merge Risk: ⚪ Minimal · up to 13ebd

Windows notifications now use tags within the stated platform limit while retaining per-environment and per-thread replacement behavior. No actionable merge-blocking risk remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		[#12287] The coordinator now derives each browser notification tag from threadNotificationTag(environmentId, thread.id). The helper hashes the environmentId:threadId pair and returns a fixed 16-ch…
Out of Scope Changes check		All reviewed changes support [#12287]. The helper implements the short digest, the coordinator preserves notification replacement behavior with the digest key, and the tests cover the required propert…
Title check		The title clearly and concisely identifies the main change: shortening thread notification tags to remain within the Windows toast limit.
Description check		The description explains the problem, implementation, scope, verification results, platform limitations, and test evidence. It provides sufficient context for review, although it does not use the temp…

Full details: Linked Issues check

Explanation

[#12287] The coordinator now derives each browser notification tag from threadNotificationTag(environmentId, thread.id). The helper hashes the environmentId:threadId pair and returns a fixed 16-character hexadecimal value. The coordinator uses the same tag for Notification.tag and the pending-notification map key, so replacement and badge tracking remain scoped per environment and thread. Tests cover the tag length, stability, and distinction for different environments and threads. The reported 32 unit tests and tsc --noEmit also pass.

Full details: Out of Scope Changes check

Explanation

All reviewed changes support [#12287]. The helper implements the short digest, the coordinator preserves notification replacement behavior with the digest key, and the tests cover the required properties. No unrelated UI or platform behavior changes are present in the reviewed changes.

Full details: Description check

Explanation

The description explains the problem, implementation, scope, verification results, platform limitations, and test evidence. It provides sufficient context for review, although it does not use the template section headings explicitly.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

_{Comment @coderabbitai help to get the list of available commands.}

[ScottN-PV]

I tested this patch on Windows. Completion banners and sound worked after reloading the app, but clicking a notification did not bring T3 to the foreground.

• Setup: Windows 11 Home 25H2, build 26200.9457; T3 built from source with Electron 44.4.2. The app loaded at t3code-dev://app, using separate test settings and conversations.
• Code tested: the changes from PR commit 13ebd2589a2970ecd759fc90061e2e3d2d226f30, applied to upstream commit 5781b5240bd5d2e21c651f6b228975ac40cbd67b.
• Tag-length comparison: I created two notifications through the development app's JavaScript Console. A notification with tag t3-control appeared in Windows Notification Center. A notification with a tag consisting of 73 x characters failed: Electron logged a Windows error, The size of the notification tag is too large. (HRESULT: -2143420138). Windows' notification-history API contained the short-tag notification and no long-tag notification. Do not disturb was On for both tests; I was checking storage in Notification Center, not whether a banner appeared.
• With the patch and Do not disturb Off, real Codex completion notifications displayed banners. Electron logged the shortened app tag 828d2b52f503fb00. The first test had no audible sound. After reloading T3 with Ctrl+R, both banner and sound worked in two further tests: one with T3 minimized, and one with T3 unminimized behind another application. I have not established why sound was absent on the first attempt.
• Automated checks: before applying the patch, 16 notification-coordinator tests passed. With the patch, all 19 tests in the coordinator and tag-helper test files passed. Lint on the changed files and the web package's typecheck also passed.

Clicking the banner did not bring T3 to the foreground in either window state. Clicking its Notification Center entry also failed to bring T3 forward in the minimized test. Electron logged Notification clicked, but that does not prove T3's click-handling code ran. I had to click T3's taskbar icon. The agent then appeared to continue for about one second before the final response became visible. I do not know whether the agent was still working or the interface was catching up. This PR does not directly change the code that handles notification clicks. I haven't established whether the click failure existed before this patch.

These results support the tag-length fix and confirm banner/sound delivery with the patch in this development setup. Click-to-open remains unresolved. I have not tested a patched Windows installer build, whether notifications from different conversations remain separate, macOS/Linux, or T3 in a standalone Windows browser. I have no video recording; the evidence consists of manual observations and local diagnostic logs.

AI assistance: OpenAI Codex, using Codex CLI 0.155.1 through its app-server integration in the T3 Code desktop app; model: gpt-6-astra; reasoning effort: medium. Codex assisted with test setup, running automated checks, interpreting diagnostic logs, and drafting this report. I performed the manual Windows checks, reported the visible and audible outcomes, and reviewed the report before posting.

[satyalyadav]

Thanks for the detailed Windows verification, and for isolating the tag failure with the 73-character tag. On click-to-open: this PR only changes the tag value and the pending-notification map key, so it does not touch click handling, and your report does not establish it as a regression. It is worth checking on unpatched main, and if it reproduces there it belongs in its own issue with that repro. If you have time, the behavior this PR actually changes is replacement, so a check that two different conversations stay separate and that a second completion replaces the first would be useful evidence.

[ScottN-PV]

Thanks for the suggested checks. I ran an automated native Windows follow-up using the same isolated source build: PR 13ebd2589a2970ecd759fc90061e2e3d2d226f30 applied to baseline 5781b5240bd5d2e21c651f6b228975ac40cbd67b, Electron 44.4.2.

Real Codex turns were started through the isolated server's local API while the desktop client stayed in the background. Windows' notification-history API showed:

• First completion in conversation A: one entry for A.
• Completion in conversation B: two entries, A and B, with distinct tags.
• Second completion in A: still two entries. A reused its original tag and replaced its previous entry; B's tag and notification XML were unchanged. I changed A's conversation title before the second turn so the replacement could be distinguished in the stored notification body.

The application tags were 7fd41c906aca1aa6 for A and 1436fb8f1893e7c7 for B. Electron's log also recorded the old notification being hidden when A completed again. A local coordinator test covering A → B → A passed alongside the 16 existing coordinator tests.

This verifies separation and replacement in native Windows notification history for that development build. It does not add a visual banner/sound check.

I also tested clean, unpatched main at c26119ada3565dda55bc3e86fdf47c7f9088e4b5, in a separate worktree and copied disposable profile, with Electron 44.4.2. A real turn completed while the renderer was unfocused and notification permission was granted. Electron attempted the original 73-character application tag, and Windows rejected it with The size of the notification tag is too large. (HRESULT: -2143420138). Notification history was unchanged: no entry for that completion appeared. Consequently there was no new notification to click, so this does not establish click-to-open behavior on unpatched main. I agree that the earlier click observation is not evidence of a regression from this PR. No Windows notification settings were changed for these checks.

AI assistance: OpenAI Codex; harness: Codex CLI 0.155.1 app-server integration; host/interface: T3 Code desktop app; model: gpt-6-astra; reasoning/effort: medium. Contribution: local API test setup, automated test execution, Windows notification-history inspection, evidence comparison, and drafting this reply. Human involvement: requested the follow-up and automatic testing, and confirmed that the model, effort, runtime version, and host remained the same as in the earlier verification. These follow-up results are automated observations; no new manual Windows checks were performed. Attribution settings are user-confirmed; runtime metadata was not independently captured again for this follow-up.

[satyalyadav]

Thanks for the thorough follow-up, Scott. The A to B to A history check plus the unpatched-main control is exactly the evidence this needed, and I appreciate you closing the click-to-open question with it. I have added a summary of your verification to the PR description.

Dismissing prior approval to re-evaluate 13ebd25

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one does not say which model or harness was used to create it. If this change is really important, we recommend rebuilding the PR with a newer model and noting the model and harness in the PR description.

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

Dismissing prior approval to re-evaluate e5e842c

Dismissing prior approval to re-evaluate 05c0fe8

[juliusmarminge]

Note

This comment is posted by Julius' dot

The short-tag fix has native Windows verification. The new reveal bridge fixes a separate failure: notifications already arrive, but clicking them doesn't restore the window. Julius's triage asked to leave click handling unchanged. Has a maintainer approved combining these fixes? Otherwise, please keep this PR to the tag repair and split the reveal fix under the one-problem rule. For the reveal fix, attach a short before/after recording of banner and Notification Center clicks; the reported retest currently has no recording, as required for interaction changes.

Dismissing prior approval to re-evaluate 54a3420

[juliusmarminge]

Taking over this one. I’m keeping the notification tag fix focused, updating it against main, and retaining the existing Windows verification with fresh focused checks.

Dismissing prior approval to re-evaluate e790dc0