Skip to content

feat(notifications): capture service-worker push notifications for slack (t067)#5

Open
nguyenthienthanh wants to merge 3 commits into
duongdev:mainfrom
nguyenthienthanh:feat/t067-service-worker-push-capture
Open

feat(notifications): capture service-worker push notifications for slack (t067)#5
nguyenthienthanh wants to merge 3 commits into
duongdev:mainfrom
nguyenthienthanh:feat/t067-service-worker-push-capture

Conversation

@nguyenthienthanh

@nguyenthienthanh nguyenthienthanh commented Jun 8, 2026

Copy link
Copy Markdown

Stacked on #4 (t066). Merge #4 first — until then this PR's diff also shows the t066 commit. After #4 merges, this reduces to just the t067 changes.

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's push handler via registration.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

  • reconcile now also matches service_worker targets whose adapter declares a swScript (Slack), and attaches a read-only side-channel to them.
  • The SW channel injects inject/slack-sw-notify.js via Runtime.evaluate — a worker has no Page domain, so there's no document-start hook and no t066 keep-alive. The script patches ServiceWorkerRegistration.prototype.showNotification and ships the same __cdpNotify toasts the page path uses.
  • The single Slack SW serves every workspace (origin-level), so its URL carries no team id. The worker script derives the per-workspace groupKey from 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 showNotification before the next 5s reconcile attaches is missed — there's no SW-start barrier. A hardened version would use a browser-level CDP session with Target.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. See docs/tasks/067.

Tests

  • pnpm test635 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.
  • Needs HITL: the Slack SW push payload shape is a defensive guess — verify against a live push and tighten the team/channel probes using the one-time [cdp-sw-notify] sample options: console log.

🤖 Generated with Claude Code


Also in this PR — clipboard file paste (video/doc)

Bundled at the author's request (one PR on this branch). Independent of the t067 SW work.

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 referenceclipboard.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/*.

Changes:

  • core/clipboard.js: pure mimeForName (extension → MIME); converted 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{ name, type, dataUrl } (150 MB cap, main-process fs, no CORS wall).
  • remote-page.ts: generalize pasteImagepasteFile(dataUrl, name, type) preserving the real name + MIME; 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.
  • preload.js / vite-env.d.ts / cdp-web-transport.ts: wire/stub readClipboardFiles.

Not covered: file-picker <input type=file> upload remains the separate known limitation (needs DOM.setFileInputFiles over CDP).

Tests: pnpm test 634 → still green (2 new: mimeForName, pasteFile); pnpm typecheck + Biome (changed) clean.

@nguyenthienthanh

Copy link
Copy Markdown
Author

@duongdev follow-up to #4. Captures Slack's service-worker push notifications (the realm the page hook can't see). Experimental — the SW push payload probe needs a quick HITL tighten against a live push (one-time console sample included).

@nguyenthienthanh nguyenthienthanh force-pushed the feat/t067-service-worker-push-capture branch from f816d47 to a5dae4d Compare June 8, 2026 09:46
…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.
@nguyenthienthanh nguyenthienthanh force-pushed the feat/t067-service-worker-push-capture branch from a5dae4d to be78a45 Compare June 8, 2026 09:54
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant