feat: replace in-app Toast notifications with native OS notifications#334
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (1)packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2026-04-20T14:36:08.774ZApplied to files:
📚 Learning: 2026-04-24T17:12:26.774ZApplied to files:
📚 Learning: 2026-04-25T09:19:33.633ZApplied to files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughAdds native OS notifications with explicit fallback to in-app toast when OS permission is denied, removes focus/visibility suppression and persistent in-app toast tracking, and introduces a preload↔main IPC pair to trigger Dock/taskbar attention (flash/bounce) from the renderer. Changes
Sequence DiagramssequenceDiagram
participant User
participant Renderer as Renderer Process
participant Preload
participant Main as Main Process
participant OS as OS Notification
participant DockTask as Dock/Taskbar
User->>Renderer: Trigger notification event
Renderer->>OS: Create native Notification()
alt OS Permission Granted
OS-->>User: Show native notification
Renderer->>Preload: call flashFrame()
Preload->>Main: invoke "flash-frame"
alt macOS
Main->>DockTask: app.dock.bounce()
else Windows/Linux
Main->>DockTask: win.flashFrame(true)
Note over Main: wait 1000ms
Main->>DockTask: win.flashFrame(false)
end
else OS Permission Denied
Renderer->>Renderer: console.warn + showToast(title, description)
Renderer-->>User: Display in-app fallback toast
end
sequenceDiagram
participant Session as Session System
participant Layout as Layout Component
participant Notifier as Notification Emission
Session->>Layout: question.asked / permission.asked
Layout->>Layout: check alertedAtBySession throttle & active session match
alt Background session & throttle expired
Layout->>Notifier: emit notification (native or fallback)
Layout->>Layout: update alertedAtBySession timestamp
else Active session or throttle active
Layout->>Layout: skip emitting notification
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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)
Review rate limit: 0/3 reviews remaining, refill in 51 minutes and 2 seconds. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements a fallback to in-app toasts when OS notification permissions are denied and introduces a window flashing feature for desktop notifications to attract user attention. It also refactors notification handling by removing legacy session-based toast logic. Feedback focuses on restoring missing persistence and actions in the new fallback toast, preventing a potential crash in the Electron main process by checking if the window is destroyed before resetting the flash state, and ensuring the window only flashes when it is not currently focused to avoid unnecessary distractions.
Astro-Han
left a comment
There was a problem hiding this comment.
I left one inline comment for a notification regression that is not covered by the existing comments.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/entry.tsx`:
- Around line 64-73: The current check treats any non-"granted" result the same;
update the logic around the permission variable returned by
Notification.requestPermission() so that you only fall back to showToast(...)
when permission === "denied" (explicit denial); if permission === "default"
simply return silently and do not show the in-app toast. Locate the block using
permission, showToast, title, and description and change the conditional
accordingly so denied triggers the toast and default exits without UI.
In `@packages/desktop-electron/src/main/ipc.ts`:
- Around line 403-409: The flash timer is overwritten unconditionally causing
earlier timers to cancel newer flashes; before calling setTimeout to turn off
win.flashFrame, clear any existing timer for that BrowserWindow (e.g., check and
clear a stored timer id on the window or in a Map keyed by win.id), then assign
the new timer id back to that storage; in the timeout callback call
win.flashFrame(false) and remove/clear the stored timer id so subsequent
notifications properly reset the stop-timer per window (use clearTimeout on the
stored id and ensure unique storage name like flashTimer or a dedicated Map).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ec34922-b277-47c5-a2df-89eab0621ee2
📒 Files selected for processing (6)
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsxpackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/renderer/index.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-opencode
- GitHub Check: unit-windows-opencode-server-tools
🧰 Additional context used
📓 Path-based instructions (3)
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/renderer/index.tsxpackages/desktop-electron/src/main/ipc.ts
packages/desktop-electron/src/main/ipc.ts
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Main process should register IPC handlers in
src/main/ipc.ts
Files:
packages/desktop-electron/src/main/ipc.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsx
🧠 Learnings (8)
📚 Learning: 2026-04-20T14:36:08.774Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.774Z
Learning: Applies to packages/desktop-electron/src/**/*.{ts,tsx,js,jsx} : Renderer process should only call `window.api` from `src/preload`
Applied to files:
packages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/renderer/index.tsx
📚 Learning: 2026-04-25T09:19:33.633Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 231
File: packages/desktop-electron/src/main/index.ts:537-537
Timestamp: 2026-04-25T09:19:33.633Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/`), the convention for IPC registration is that `index.ts` (the bootstrap entry) directly calls each registration function. `registerIpcHandlers({...})` in `src/main/ipc.ts` is a single fat options-bag with inline handler bodies. New, cohesive IPC features (e.g., About) are placed in sub-modules like `src/main/ipc/about.ts`, which own their own exported types, helpers, and a `register*Ipc()` function. The bootstrap in `index.ts` calls each `register*Ipc()` directly — this is intentional, not fragmentation. Do NOT suggest routing sub-module IPC registrations through `ipc.ts`.
Applied to files:
packages/desktop-electron/src/preload/index.ts
📚 Learning: 2026-04-24T17:12:26.774Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/desktop-electron/electron-builder.config.ts:14-18
Timestamp: 2026-04-24T17:12:26.774Z
Learning: In Astro-Han/pawwork, the `localizedMacDisplayNameByChannel` map in `packages/desktop-electron/electron-builder.config.ts` is intentionally kept separate from `localizedAppDisplayName` in `packages/desktop-electron/src/main/app-display-name.ts`. The former is a build-time packaging helper; the latter is a runtime UI helper that localizes the current app name by locale. Coupling them would introduce a build-time dependency on runtime main logic. Do not suggest deduplicating or sharing this mapping — the explicit local table is covered by focused regression tests in `electron-builder-app-update.test.ts`.
Applied to files:
packages/desktop-electron/src/renderer/index.tsx
📚 Learning: 2026-04-25T09:19:30.734Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 231
File: packages/desktop-electron/src/main/index.ts:537-537
Timestamp: 2026-04-25T09:19:30.734Z
Learning: In Astro-Han/pawwork (packages/desktop-electron/src/main/), follow the IPC registration convention: the bootstrap entry (packages/desktop-electron/src/main/index.ts) should directly call each module’s exported register*Ipc() function. Do not route/centralize these sub-module IPC registrations through src/main/ipc.ts. Keep sub-module IPC features cohesive (e.g., src/main/ipc/about.ts should own its types/helpers and expose register*Ipc()), and allow index.ts to aggregate by calling each register*Ipc() directly.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-25T12:52:35.631Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/desktop-electron/src/main/ipc.ts:238-263
Timestamp: 2026-04-25T12:52:35.631Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/ipc.ts`), `deps.getServerReadyData()` (backed by `serverReady.promise` in `index.ts`) resolves once at server startup and remains settled; it is not expected to reject in practice. Do not flag the absence of a try-catch around it in the `export-session` IPC handler — the network/fetch layer in `server-client.ts` already has a 10-second AbortController timeout and returns a typed `{ok: false, error}` payload, covering the real failure modes.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsx
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsx
📚 Learning: 2026-04-24T03:51:56.211Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:56.211Z
Learning: In Astro-Han/pawwork E2E tests (packages/app/e2e/fixtures.ts), `project.prompt(text)` internally calls `trackSession(next.sessionID, active.directory)` after the prompt submission is observed and the active session is resolved. Tests that obtain a `sessionID` from `project.prompt()` do NOT need to call `project.trackSession(sessionID)` manually — it is already registered for teardown. Calling it again would be duplicate ownership.
Applied to files:
packages/app/src/pages/layout.tsx
Astro-Han
left a comment
There was a problem hiding this comment.
Please close the remaining notification UX gaps before this PR lands. The direction matches #332, but the implementation still needs to make the failure paths as reassuring as the main path: 1. Web fallback: when OS notification permission is denied, the fallback Toast should preserve the old ability to go to the affected session instead of only showing a passive message. 2. Desktop fallback: if desktop OS notifications are unavailable or denied, PawWork should still give the user an in-app fallback instead of silently losing the permission/question/session alert. 3. Windows/Linux flash cleanup: the delayed stop for taskbar flashing should not call into a destroyed window, and repeated notifications should reset the stop timer cleanly per window. These three items are the difference between replacing the blocking Toasts and accidentally losing important alerts when system notifications cannot be delivered.
…check, improve flash timer
Review Comments AddressedP0 - Critical Issues1. Restore current-session guard (Astro-Han, line 486)
P1 - High Priority Issues2. Only fallback to Toast for denied permission (CodeRabbit, line 64-73)
3. Check win.isDestroyed() in flash timer (Gemini, line 407-409)
4. Reset flash timer per window (CodeRabbit, line 403-409)
P2 - Medium Priority Issues (YAGNI Analysis)5. Desktop fallback for failed notifications (Astro-Han, line 260)
6. Flash only when window not focused (Gemini, line 263)
7. Restore Toast actions and persistence (Gemini, line 67-71)
P3 - Low Priority Issues8. Icon inconsistency (Gemini, line 67-71)
Summary: Fixed 4 critical/high priority issues that could cause crashes, UX regressions, or logic errors. Declined 4 medium/low priority issues based on YAGNI principle - they add complexity for edge cases that rarely occur or don't significantly improve UX. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/src/pages/layout.tsx`:
- Around line 480-498: The child-session suppression logic incorrectly treats
undefined params.id as a match (undefined === undefined) and suppresses
root-session notifications; in the question.asked branch replicate the guard
used above by only computing currentSession/isCurrentSession and checking
session?.parentID when params.id is truthy (i.e., require params.id before
comparing to session?.parentID), so update the logic around currentSession,
isCurrentSession, workspaceKey(currentDir()), props.sessionID and
session?.parentID in the question.asked block and only call platform.notify when
!isCurrentSession after applying that guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3bf451eb-3bb5-4137-bf46-e200a818e043
📒 Files selected for processing (3)
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsxpackages/desktop-electron/src/main/ipc.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-opencode
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-desktop
- GitHub Check: unit-app
- GitHub Check: typecheck
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (3)
packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Renderer process should only call
window.apifromsrc/preload
Files:
packages/desktop-electron/src/main/ipc.ts
packages/desktop-electron/src/main/ipc.ts
📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)
Main process should register IPC handlers in
src/main/ipc.ts
Files:
packages/desktop-electron/src/main/ipc.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsx
🧠 Learnings (13)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
📚 Learning: 2026-04-25T09:19:30.734Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 231
File: packages/desktop-electron/src/main/index.ts:537-537
Timestamp: 2026-04-25T09:19:30.734Z
Learning: In Astro-Han/pawwork (packages/desktop-electron/src/main/), follow the IPC registration convention: the bootstrap entry (packages/desktop-electron/src/main/index.ts) should directly call each module’s exported register*Ipc() function. Do not route/centralize these sub-module IPC registrations through src/main/ipc.ts. Keep sub-module IPC features cohesive (e.g., src/main/ipc/about.ts should own its types/helpers and expose register*Ipc()), and allow index.ts to aggregate by calling each register*Ipc() directly.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-25T12:52:35.631Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 234
File: packages/desktop-electron/src/main/ipc.ts:238-263
Timestamp: 2026-04-25T12:52:35.631Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/ipc.ts`), `deps.getServerReadyData()` (backed by `serverReady.promise` in `index.ts`) resolves once at server startup and remains settled; it is not expected to reject in practice. Do not flag the absence of a try-catch around it in the `export-session` IPC handler — the network/fetch layer in `server-client.ts` already has a 10-second AbortController timeout and returns a typed `{ok: false, error}` payload, covering the real failure modes.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-20T14:36:08.774Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.774Z
Learning: Applies to packages/desktop-electron/src/**/*.{ts,tsx,js,jsx} : Renderer process should only call `window.api` from `src/preload`
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-27T10:33:12.228Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:108-169
Timestamp: 2026-04-27T10:33:12.228Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/session/prompt.ts` and `packages/opencode/src/session/processor.ts`, PR `#264`), the loop-gate race condition between `buildLoopContext()` and `recordSyntheticBlock`/`recordSyntheticStop` is intentionally handled via idempotence guards (re-check sigKey presence / `hasStopped` inside the record helpers) rather than a full per-parent `Effect.Mutex`. Threading a `Map<MessageID, Mutex>` through the processor was considered too large a surface change for this edge case; the residual TOCTOU window only produces extra synthetic parts with no behavioral drift on the "turn ends" contract. A code comment documents the trade-off and points to a full-mutex follow-up if the race is observed in practice. Do NOT re-flag the absence of a per-parent mutex as a blocking issue in future reviews.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-24T13:03:14.694Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 222
File: packages/desktop-electron/src/main/index.ts:686-692
Timestamp: 2026-04-24T13:03:14.694Z
Learning: In `packages/desktop-electron/src/main/index.ts`, the `checkForUpdates()` function intentionally uses recursive self-invocation for the "Retry" path in the update-check failure dialog. This is mandated by the v5.2 design spec (`#213`): "Await the retry recursion and log any rejection so support can see repeated failures." Because retries are user-paced (require a button click), all prior async frames have already unwound through microtasks before the next attempt, so there is no stack/frame-nesting problem in practice. Do not suggest refactoring this to an iterative loop.
Applied to files:
packages/desktop-electron/src/main/ipc.ts
📚 Learning: 2026-04-21T16:57:25.580Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 102
File: packages/opencode/src/config/agent.ts:108-119
Timestamp: 2026-04-21T16:57:25.580Z
Learning: In `packages/opencode/src/config/agent.ts` (Astro-Han/pawwork), `ConfigPermission.Info` only accepts permission objects or the three action strings `"ask"`, `"allow"`, `"deny"`, and transforms those action strings into `{ "*": action }` before `normalize()` runs. By the time `normalize()` is reached, `configuredPermission` is always either `undefined` or a `Record<string, Rule>` — never a raw arbitrary string. The `Object.assign(permission, configuredPermission)` pattern is therefore safe. Do not flag it as corrupting string permission references.
Applied to files:
packages/app/src/entry.tsx
📚 Learning: 2026-04-24T05:39:58.329Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:58.329Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Applied to files:
packages/app/src/entry.tsx
📚 Learning: 2026-04-26T15:35:36.505Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 245
File: packages/opencode/src/question/index.ts:21-24
Timestamp: 2026-04-26T15:35:36.505Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/question/index.ts`), `Question.Option.description` is intentionally **required** (`z.string()`, not `.optional()`). This matches the upstream opencode contract and all current callers (e.g. `PlanExitTool`) always supply a description. The defensive `<Show when={props.description}>` rendering in `session-question-dock.tsx` is a standard guard, not a signal that the field is intended to be optional. Do NOT suggest making `Option.description` optional without a dedicated follow-up that covers schema + tool description + dock copy + tests.
Applied to files:
packages/app/src/entry.tsx
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsx
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/entry.tsxpackages/app/src/pages/layout.tsx
📚 Learning: 2026-04-29T13:27:28.494Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:28.494Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/session/session-view-controller.test.ts` and related Solid + Bun test files), the Bun + Solid test environment does NOT reliably exercise `createMemo((current) => ...)` signal recomputation the way a browser runtime does. Adding signal-driven transition tests (e.g., using `createSignal` + `createRoot` to flip reactive inputs) is misleading in this environment because the reactive invalidation/recomputation path is not faithfully replicated. The correct strategy is: cover pure transition logic with plain unit tests (e.g., `nextSessionViewState`), and cover the browser reactive path with E2E tests (e.g., session-switch E2E spec). Do NOT re-flag the absence of signal-driven `createSessionViewController` tests in this environment as a gap.
Applied to files:
packages/app/src/pages/layout.tsx
📚 Learning: 2026-04-23T15:26:07.250Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:07.250Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
Applied to files:
packages/app/src/pages/layout.tsx
🔇 Additional comments (2)
packages/desktop-electron/src/main/ipc.ts (2)
395-395: Per-window timer map is the right fix for burst notifications.This prevents older stop-timers from turning off newer flashes on the same window.
397-419:flash-framehandler correctly handles window scoping and timer cleanup.Good use of
BrowserWindow.fromWebContents(event.sender)for multi-window correctness, plus guarded unflash on destroyed windows.As per coding guidelines: Main process should register IPC handlers in
src/main/ipc.ts.
Summary
Replace in-app Toast notifications with native OS notifications + Dock/taskbar flash feedback. This removes the annoying bottom-right popups that block UI elements.
Changes:
notifyfunctions so system notifications fire regardless of window focusflashFrameIPC with platform-specific behavior (macOS: Dock bounce, Windows/Linux: taskbar flash)Why
The current notification system uses in-app Toast popups in the bottom-right corner that:
Users want native OS notifications like Codex App, which don't interfere with the app UI.
Related Issue
Closes #332
How To Verify
Manual testing:
bun run dev:desktopScreenshots or Recordings
N/A — notification behavior change, no visual UI change
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Improvements
Bug Fixes