feat(notifications): capture service-worker push notifications for slack (t067)#5
Open
nguyenthienthanh wants to merge 3 commits into
Open
Conversation
Author
f816d47 to
a5dae4d
Compare
…ner (t066) Background tabs on the remote browser silently stopped delivering notifications: Chromium freezes idle background tabs (~5 min), which pauses the page JS the capture script hooks (window.Notification), so only the active tab kept notifying. - Keep-alive: the side-channel now sends Page.setWebLifecycleState active on attach and re-applies it every reconcile. This un-freezes the tab without making it visible (per the CDP spec the state only governs freeze, not document.visibilityState), so Slack still treats the tab as hidden and keeps firing desktop notifications. Lives in core/ so the headless web server benefits too. - Favicon: the OS notification banner and the macOS dock icon now carry the source app's favicon (newest-unread; cleared when unread -> 0). dockOverlayIcon(list) is pure; main fetches the favicon bytes (no CORS wall) and composites in the renderer (decodes .ico, no canvas taint). Service-worker push capture (registration.showNotification, a separate realm) is out of scope here -> t067.
…ack (t067) Slack delivers many notifications from its service worker's push handler via registration.showNotification — a realm the page hook (window.Notification) can't reach, so they were silently missed. - The side-channel now also attaches to a matching service_worker target (Slack adapter swScript) and injects inject/slack-sw-notify.js via Runtime.evaluate (a worker has no Page domain, so no document-start hook and no t066 keep-alive), patching ServiceWorkerRegistration.prototype.showNotification to ship the same __cdpNotify toasts. - The single Slack SW serves every workspace (origin-level), so the SW URL has no team id; the script derives the per-workspace groupKey from the notification payload (defensive probe, logged once for HITL). Known gap: a worker that spins up fresh on a push and fires before the next 5s reconcile attaches is missed (no SW-start barrier). Documented in docs/tasks/067; a hardened version would use a browser-level Target.setAutoAttach waitForDebuggerOnStart. Stacked on t066 (duongdev#4) — merge that first.
a5dae4d to
be78a45
Compare
…images
Pasting a copied file (e.g. a video from Finder) pasted the file's icon
thumbnail instead of the file. Root cause: the paste path assumed clipboard
*image bits* and never a *file reference* — clipboard.readImage() returns a
copied file's Quick Look icon (non-empty), so that branch won and pasteImage
shipped the icon PNG; on web the native paste handler filtered items to
image/ only, dropping video/*.
- core/clipboard.js: add pure mimeForName (extension -> MIME); convert the
module to CommonJS so the CJS main process can require it (aligns with the
rest of core/).
- main.js: new cdp:read-clipboard-files IPC reads the real file via the
clipboard's public.file-url and returns { name, type, dataUrl } (150 MB
cap, main-process fs, no CORS wall).
- remote-page.ts: generalize pasteImage -> pasteFile(dataUrl, name, type),
preserving the real name + MIME so upload targets accept a video;
pasteImage is now a thin wrapper.
- app.tsx: Cmd+V tries files -> image bits -> text; web paste accepts any
file kind off the native paste event, not just images.
- preload.js / vite-env.d.ts / cdp-web-transport.ts: wire/stub
readClipboardFiles.
Also bundles the notification favicon/dock-icon assets (build/notif-icons,
package.json packaging allowlist) and the accompanying notifications-sidechain
/ notifications refactor.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The page hook (
window.Notification,slack-notify.js) only sees notifications Slack fires from the page realm. Slack also delivers many notifications from its service worker'spushhandler viaregistration.showNotification(...)— a separate JS realm the page script can't reach. Those were silently missed (called out as out-of-scope in t064/t066).Changes
reconcilenow also matchesservice_workertargets whose adapter declares aswScript(Slack), and attaches a read-only side-channel to them.inject/slack-sw-notify.jsviaRuntime.evaluate— a worker has noPagedomain, so there's no document-start hook and no t066 keep-alive. The script patchesServiceWorkerRegistration.prototype.showNotificationand ships the same__cdpNotifytoasts the page path uses.groupKeyfrom the notification payload (defensive probe; logs one sample to the worker console for HITL tightening).Known limitation (documented, not fixed here)
A worker that spins up fresh on a push and fires
showNotificationbefore the next 5s reconcile attaches is missed — there's no SW-start barrier. A hardened version would use a browser-level CDP session withTarget.setAutoAttach({ waitForDebuggerOnStart: true })to attach + inject before the worker runs. The t066 page keep-alive keeps the registration warm enough to stay listed across reconciles in the common case. Seedocs/tasks/067.Tests
pnpm test— 635 passing (6 new: SW attach / no-Page-domain / no-keep-alive / ingest+groupKey / drop-on-vanish / page+SW coexist).pnpm typecheck, Biome (changed files) — clean.team/channelprobes using the one-time[cdp-sw-notify] sample options:console log.🤖 Generated with Claude Code
Also in this PR — clipboard file paste (video/doc)
Problem: pasting a copied file (e.g. a video from Finder) pasted the file's icon thumbnail, not the file. Root cause: the paste path assumed clipboard image bits, never a file reference —
clipboard.readImage()returns a copied file's Quick Look icon (non-empty), so that branch won andpasteImageshipped the icon PNG; on web the native paste handler filtered items toimage/only, droppingvideo/*.Changes:
core/clipboard.js: puremimeForName(extension → MIME); converted to CommonJS so the CJS main process canrequireit (aligns with the rest ofcore/).main.js: newcdp:read-clipboard-filesIPC reads the real file via the clipboard'spublic.file-url→{ name, type, dataUrl }(150 MB cap, main-process fs, no CORS wall).remote-page.ts: generalizepasteImage→pasteFile(dataUrl, name, type)preserving the real name + MIME;pasteImageis now a thin wrapper.app.tsx: Cmd+V tries files → image bits → text; web paste accepts any file kind off the native paste event.preload.js/vite-env.d.ts/cdp-web-transport.ts: wire/stubreadClipboardFiles.Not covered: file-picker
<input type=file>upload remains the separate known limitation (needsDOM.setFileInputFilesover CDP).Tests:
pnpm test634 → still green (2 new:mimeForName,pasteFile);pnpm typecheck+ Biome (changed) clean.