feat(desktop): switch Windows shell to native Win11 frame and add cross-platform About modal#231
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an "About" feature (menu trigger, main IPC, preload API, renderer modal), injects build SHA into the Electron build, removes Windows titlebar overlay/dynamic theming, splits desktop menu generation for macOS/Windows, and updates related tests, CSS, and i18n. (50 words) Changes
Sequence DiagramsequenceDiagram
participant Menu as Menu System
participant Main as Main Process
participant Preload as Preload
participant Renderer as App Renderer
participant Modal as About Modal
Menu->>Main: triggerAbout(window?)
Main->>Main: select target BrowserWindow
Main->>Preload: send 'about:open' IPC
Preload->>Renderer: invoke onAboutOpen callback
Renderer->>Modal: AboutModal receives open event
Modal->>Preload: call getAboutInfo()
Preload->>Main: invoke 'about:get-info'
Main->>Main: gather version, electron/chrome, buildSha
Main->>Preload: return AboutInfo
Preload->>Modal: deliver AboutInfo
Modal->>Modal: render dialog with metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/components/about-modal.tsx`:
- Around line 20-26: The dialog opens even if getAboutInfo throws or returns
undefined; wrap the async callback passed to window.api?.onAboutOpen in a
try/catch and only call setOpen(true) after verifying a non-undefined response
from window.api?.getAboutInfo (i.e., call setInfo(data) and then setOpen(true)
only when data is truthy); also handle and optionally log errors from
getAboutInfo so unsubscribe remains assigned by onMount and the dialog does not
open on failure.
In `@packages/app/src/i18n/zh.ts`:
- Line 1004: The about.title i18n entry currently reads "关于 PawWork" and must
use the repo's fully localized branding; update the "about.title" key's value to
"关于爪印" (remove the ASCII "PawWork" and use the Chinese brand "爪印") so it matches
the zh localization convention used elsewhere.
In `@packages/desktop-electron/src/main/index.ts`:
- Line 537: The call to registerAboutIpc() in index.ts must be removed and the
About IPC should be routed through the centralized IPC bootstrap in
src/main/ipc.ts: export or import the registerAboutIpc function in that file and
invoke it from the central registration function (e.g., a top-level
registerAllIpc or initIpc function) so all IPC handlers are registered from one
place; update index.ts to only call the central IPC bootstrap (not
registerAboutIpc directly) and ensure registerAboutIpc remains an exported
symbol so ipc.ts can call it.
In `@packages/desktop-electron/src/main/menu-template.test.ts`:
- Around line 29-35: The test uses a non-null assertion on the result of
find(...) when locating the Help menu which will throw a confusing runtime error
if missing; update the test that calls buildWindowsMenuTemplate and currently
assigns help via const help = tpl.find((m) => m.label === "Help")! to first
assert or guard that help is defined (e.g. expect(help).toBeDefined() or if
(!help) fail("Help menu not found in Windows template")) before accessing
help.submenu, so the failure message is explicit and the subsequent labels
extraction (help.submenu ?? []) is safe.
🪄 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: 6b810f53-fea3-41f5-80db-d7cde47f0719
📒 Files selected for processing (20)
.github/workflows/build.ymlpackages/app/src/app.tsxpackages/app/src/components/about-modal.tsxpackages/app/src/components/titlebar.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/index.csspackages/app/src/shell-frame-contract.test.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu-template.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/window-chrome.test.tspackages/desktop-electron/src/main/window-chrome.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.ts
💤 Files with no reviewable changes (3)
- packages/desktop-electron/src/main/window-chrome.test.ts
- packages/desktop-electron/src/main/window-chrome.ts
- packages/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: unit-windows-opencode-config-project
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-desktop
- GitHub Check: typecheck
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-app
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 Additional context used
📓 Path-based instructions (2)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/i18n/en.tspackages/app/src/components/titlebar.tsxpackages/app/src/shell-frame-contract.test.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.tsx
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/index.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.ts
🧠 Learnings (16)
📓 Common learnings
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:56.086Z
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.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
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.
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:23.931Z
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`.
📚 Learning: 2026-04-22T05:32:29.012Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:32:29.012Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.ts
📚 Learning: 2026-04-24T17:08:44.294Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.ts
📚 Learning: 2026-04-24T17:12:23.931Z
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:23.931Z
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/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.tspackages/app/src/app.tsx
📚 Learning: 2026-04-22T09:32:58.310Z
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.
Applied to files:
packages/app/src/i18n/en.tspackages/desktop-electron/src/main/windows.tspackages/app/src/app.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/i18n/en.tspackages/app/src/components/titlebar.tsxpackages/app/src/shell-frame-contract.test.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.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/i18n/en.tspackages/app/src/components/titlebar.tsxpackages/app/src/shell-frame-contract.test.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.tsx
📚 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/main/ipc.ts : Main process should register IPC handlers in `src/main/ipc.ts`
Applied to files:
packages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc/about.ts
📚 Learning: 2026-04-24T05:39:56.086Z
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:56.086Z
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/components/titlebar.tsx
📚 Learning: 2026-04-23T17:02:35.873Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/app/src/index.css:95-97
Timestamp: 2026-04-23T17:02:35.873Z
Learning: In the pawwork repository, Stylelint is intentionally not configured (per AGENTS.md, only linting is enforced and the Biome formatter is disabled). When reviewing CSS files, do not raise Stylelint rule-based issues (e.g., `declaration-empty-line-before`) because they are false positives and not enforced by the project toolchain.
Applied to files:
packages/app/src/index.css
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/app/src/shell-frame-contract.test.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `data-component`, `data-action`, or semantic roles for selectors instead of CSS class names or IDs
Applied to files:
packages/app/src/shell-frame-contract.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use locator assertions like `toBeVisible()`, `toHaveCount(0)`, and `toHaveAttribute(...)` for normal UI state verification
Applied to files:
packages/app/src/shell-frame-contract.test.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/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/windows.tspackages/app/src/app.tsx
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/desktop-electron/src/main/menu.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/desktop-electron/src/main/menu.test.ts
🔇 Additional comments (28)
packages/app/src/index.css (1)
181-181: Linux-only frame selector is correctly scoped.Line 181 now limits the inset frame chrome to Linux, which aligns with the native Windows frame switch.
.github/workflows/build.yml (1)
317-317: Build SHA injection in CI is a good addition.Line 317 correctly passes the workflow commit SHA into the Electron build environment.
packages/app/src/components/titlebar.tsx (1)
94-94: Conditional drag-region attribute matches the Windows native-frame move.Line 94 correctly omits
data-shell-drag-regionon Windows while keeping behavior for other platforms.packages/app/src/i18n/en.ts (1)
1033-1038: About translation keys are complete and consistent.Lines 1033-1038 provide the full
about.*set needed by the new About modal.packages/desktop-electron/src/main/index.ts (1)
480-481: About menu trigger wiring looks correct.Line 480 cleanly forwards the selected
BrowserWindowinto the About trigger path.packages/desktop-electron/src/main/menu.test.ts (2)
2-13: Test dependency shape update is correct.Lines 2-13 correctly align test imports/mocks with the new macOS menu builder and About trigger dependency.
127-214: Mac-specific template calls are updated consistently.These call-site updates correctly migrate test coverage to
buildMacosMenuTemplate.packages/app/src/shell-frame-contract.test.ts (2)
16-16: Linux-only frame selector contract is correctly updated.Line 16 now validates the intended Linux-specific frame rule.
56-60: New titlebar contract test covers the intended regressions.Lines 56-60 properly guard both the removed Windows placeholder and the conditional drag-region attribute.
packages/desktop-electron/src/main/menu-template.test.ts (2)
1-27: LGTM — Well-structured test coverage for the menu template refactor.The test setup with
stubDepscorrectly implements all requiredMenuTemplateDepsproperties, including the newtriggerAbout. ThebaseOptionstyping withlocale: "en" as constensures proper type narrowing.
37-60: Good regression tests for accelerator migration and macOS role preservation.The accelerator test (lines 37-47) properly validates the
Cmd/Option→CmdOrCtrl/Altmigration mentioned in the PR objectives. The macOS tests correctly verify the system About panel is retained viarole: "about".packages/desktop-electron/src/preload/index.ts (1)
83-90: LGTM — Clean About API surface.The
getAboutInfoandonAboutOpenmethods correctly wire up to the IPC channels defined inipc/about.ts. The unsubscribe pattern matches other event handlers likeonMenuCommand.Minor note: The wrapper
const wrapped = () => handler()could explicitly ignore the event parameter (like(_: unknown) => handler()) for consistency with other handlers, but this is purely stylistic sinceabout:opensends no data.packages/desktop-electron/src/main/windows.ts (2)
111-113: LGTM — Appropriate cleanup for Windows loading window.Removing the menu from the Windows loading window via
win.removeMenu()prevents a flickering or unnecessary menu bar on what should be a transient splash screen. This aligns with the PR's goal to streamline the Windows shell experience.
78-85: LGTM — Correct platform-specific window chrome handling.macOS correctly retains
titleBarStyle: "hidden"with custom traffic light positioning, while Windows now uses the native frame (no special overrides). The spread pattern cleanly applies platform-specific options.packages/app/src/app.tsx (3)
74-87: LGTM — Window API types correctly updated.The optional chaining (
getAboutInfo?,onAboutOpen?) correctly handles non-Electron environments. The removal ofsetTitlebaraligns with the broader titlebar cleanup.
240-243: LGTM — AboutModal correctly positioned in component tree.The modal is mounted within
LanguageProvidercontext (required foruseLanguage) and renders as a sibling toprops.children, ensuring it's available app-wide without interfering with the main content hierarchy.
45-45: 🧹 Nitpick | 🔵 Trivial
AboutInfotype is duplicated across renderer and preload.The
AboutInfotype is defined in bothpackages/app/src/components/about-modal.tsxandpackages/desktop-electron/src/preload/types.ts. Consider importing from a shared location to avoid drift.One approach: export from
types.tsand re-export through the preload surface so the app can import from the desktop-api package.⛔ Skipped due to learnings
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`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:23.931Z 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`.Learnt from: Astro-Han Repo: Astro-Han/pawwork PR: 211 File: packages/opencode/src/provider/models.ts:113-179 Timestamp: 2026-04-24T06:50:02.712Z Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.Learnt from: Astro-Han Repo: Astro-Han/pawwork PR: 99 File: packages/desktop-electron/src/renderer/i18n/index.ts:30-35 Timestamp: 2026-04-21T13:45:45.149Z Learning: In Astro-Han/pawwork, the locale normalization helpers in `packages/app/src/context/language.tsx` (`normalizeLocale`) and `packages/desktop-electron/src/renderer/i18n/index.ts` (`parseLocale`) are intentionally kept separate and have different fallback shapes: `normalizeLocale` always returns a concrete `Locale` (falling back to `"en"`), while `parseLocale` returns `Locale | null` so the desktop shell can decide whether to fall back to browser detection. Do not suggest extracting a shared normalization helper across these two runtimes.Learnt from: Astro-Han Repo: Astro-Han/pawwork PR: 98 File: packages/desktop-electron/src/main/menu-labels.ts:1-2 Timestamp: 2026-04-22T05:32:29.012Z Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.packages/desktop-electron/src/main/ipc/about.ts (2)
1-26: LGTM — Clean About IPC implementation.The handler correctly leverages Electron's
process.versionsfor runtime info and provides sensible"unknown"fallbacks. ThetriggerAboutfunction properly handles the optionalbrowserWindowparameter with a fallback to the focused window.
10-12: Build SHA may show "unknown" in development — expected per PR notes.The
PAWWORK_BUILD_SHAenv var is only injected during CI release builds. For local development, this will display "unknown" which is acceptable as noted in the PR's known follow-ups.packages/desktop-electron/src/main/menu.ts (2)
17-35: LGTM — Clean adapter between template and Electron menu APIs.The
wrapClicksfunction correctly adapts the customMenuItemTemplateabstraction to Electron'sMenuItemConstructorOptions. Theinstanceof BrowserWindowcheck on line 29 properly handles the fact that Electron's click handler may receiveBaseWindoworundefined, ensuring type safety for handlers expectingBrowserWindow.
37-59: LGTM — Platform-specific menu creation with proper dependency injection.The early return for Linux (line 38) correctly implements the "Linux menu-free" spec. The
openExternalwrapper with error logging (lines 42-46) adds resilience without cluttering the template builders.packages/desktop-electron/src/preload/types.ts (2)
24-30: This is the canonicalAboutInfotype location.The preload types file is the correct authoritative source for IPC type definitions. The duplicate definitions in
ipc/about.tsandabout-modal.tsxshould ideally import from here to prevent drift. (Related to earlier comment onapp.tsx.)
97-98: LGTM — API surface correctly extended.The
getAboutInfoandonAboutOpensignatures match the implementation inpreload/index.tsand follow established patterns for async handlers and event subscriptions respectively.packages/app/src/components/about-modal.tsx (2)
15-16: Signal usage is acceptable here.Per project convention,
createStoreis preferred only when signals represent coupled state updated together in the same transaction. Here,openandinfoare updated in sequence but within the same logical flow, making separate signals a reasonable choice.
30-52: LGTM — Accessible dialog with proper localization.The dialog uses Kobalte's Dialog primitives (
Dialog.Title,Dialog.CloseButton) which provide correct ARIA semantics. All user-facing strings use thelanguage.t()helper with keys that are defined in bothen.tsandzh.tslocalization files.packages/desktop-electron/src/main/menu-template.ts (3)
1-24: Menu callback contract update looks good.Making
browserWindowoptional in the template click signature and threadingtriggerAboutthroughMenuTemplateDepskeeps the About action explicit without forcing a focused window at the type level.
34-145: macOS template split is clean.This keeps the macOS-only roles and window menu behavior localized in one builder while still normalizing shared accelerator syntax.
148-220: Windows menu composition looks solid.Separating the Windows template makes the native-frame transition easier to maintain, and the Help submenu wiring for updates/About is clear and consistent.
There was a problem hiding this comment.
Code Review
This pull request introduces a new "About" modal for the desktop application, including the necessary IPC handlers, translations, and UI components. It also refactors the Electron menu system to provide platform-specific templates for macOS and Windows, standardizing accelerators using CmdOrCtrl. Additionally, the custom Windows titlebar overlay logic and the setTitlebar API have been removed. Feedback focuses on a missing "Go" menu in the Windows template and an inconsistent keyboard shortcut for creating new sessions.
Astro-Han
commented
Apr 25, 2026
Astro-Han
commented
Apr 25, 2026
Astro-Han
commented
Apr 25, 2026
9513c4a to
e477651
Compare
Contributor
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 @.github/workflows/build.yml:
- Line 317: CI sets PAWWORK_BUILD_SHA but the app reads
process.env.PAWWORK_BUILD_SHA at runtime; packaged Electron doesn't preserve
that, so add PAWWORK_BUILD_SHA to the main build defines in
packages/desktop-electron/electron.vite.config.ts (alongside OPENCODE_CHANNEL
and PAWWORK_FEEDBACK_FORM_URL) and then update the about IPC handler in
packages/desktop-electron/src/main/ipc/about.ts to read the inlined define
(PAWWORK_BUILD_SHA) instead of process.env.PAWWORK_BUILD_SHA so the build-time
SHA is available in packaged builds.
In `@packages/desktop-electron/src/main/ipc/about.ts`:
- Around line 23-25: triggerAbout currently sends "about:open" to the focused/
clicked BrowserWindow which can be a loading window that doesn't mount the About
bridge; update triggerAbout to select the main app shell window instead of
blindly using BrowserWindow.getFocusedWindow(). Locate the function triggerAbout
and replace the target resolution with logic that iterates
BrowserWindow.getAllWindows() to find the window that hosts the app shell (e.g.,
check webContents.getURL() or other identifying property used for the main app
window) and send "about:open" to that window; if no app shell is found, fall
back to the focused window as a last resort. Ensure the change still uses the
same IPC channel "about:open".
🪄 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: f651230e-8abd-4068-9e45-7d13ec8244c5
📒 Files selected for processing (14)
.github/workflows/build.ymlpackages/app/src/app.tsxpackages/app/src/components/about-modal.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu-template.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.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). (3)
- GitHub Check: smoke-macos-arm64
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.tsx
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/index.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/main/menu-template.ts
🧠 Learnings (22)
📓 Common learnings
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:56.086Z
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.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
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/`), 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`.
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:23.931Z
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`.
📚 Learning: 2026-04-22T05:32:29.012Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:32:29.012Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/menu-template.ts
📚 Learning: 2026-04-24T17:08:44.294Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-24T17:12:23.931Z
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:23.931Z
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/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.ts
📚 Learning: 2026-04-22T09:32:58.310Z
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.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/app.tsxpackages/desktop-electron/src/main/windows.tspackages/app/src/components/about-modal.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/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.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/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-23T15:10:26.994Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/pages/layout/pawwork-sidebar.tsx:139-139
Timestamp: 2026-04-23T15:10:26.994Z
Learning: In Astro-Han/pawwork, the icon registry in `packages/ui/src/components/icon.tsx` uses **bare (unquoted) TypeScript object keys** for single-word icon names (e.g., `pin:` at line 102, `workspace:` at line 99) and **quoted keys** only for hyphenated names (e.g., `"review-active":`). When verifying icon registration with `rg`, always search for both forms — use a pattern like `rg -n "(\"${key}\"|${key}:)"` — otherwise bare-key icons will be missed and incorrectly flagged as unregistered.
Applied to files:
packages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.ts
📚 Learning: 2026-04-24T05:39:56.086Z
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:56.086Z
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/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-23T17:02:39.474Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/ui/src/theme/themes/pawwork.json:32-33
Timestamp: 2026-04-23T17:02:39.474Z
Learning: In Astro-Han/pawwork, `#FF5910` is the locked PawWork brand orange used for `button-primary-base` (and `--accent-brand` in `index.css`, theme token `interactive`/`primary`). The `text-on-interactive-base: `#FFFFFF`` on `button-primary-base: `#FF5910`` combination produces a contrast ratio of ~3.14:1. This intentionally fails WCAG AA normal-text (4.5:1) but passes AA non-text (SC 1.4.11, 3:1) and AA large-text (SC 1.4.3, 3:1). The tradeoff is accepted for brand consistency; do NOT flag this contrast pair as a WCAG violation in future reviews. If a specific surface requires stricter contrast, it should be addressed individually (e.g., larger text, ghost variant, outline button) rather than changing the palette.
Applied to files:
packages/app/src/i18n/zh.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-23T17:02:40.117Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/app/src/index.css:95-97
Timestamp: 2026-04-23T17:02:40.117Z
Learning: In Astro-Han/pawwork, Stylelint is NOT configured in the project. Per AGENTS.md, only linting (not formatting) is used, and the Biome formatter is explicitly disabled. Do NOT raise Stylelint-based review comments (e.g., `declaration-empty-line-before`) in this repo, as they are false positives from the static analysis environment and not enforced by the project toolchain.
Applied to files:
packages/app/src/i18n/zh.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-21T13:45:45.149Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 99
File: packages/desktop-electron/src/renderer/i18n/index.ts:30-35
Timestamp: 2026-04-21T13:45:45.149Z
Learning: In Astro-Han/pawwork, the locale normalization helpers in `packages/app/src/context/language.tsx` (`normalizeLocale`) and `packages/desktop-electron/src/renderer/i18n/index.ts` (`parseLocale`) are intentionally kept separate and have different fallback shapes: `normalizeLocale` always returns a concrete `Locale` (falling back to `"en"`), while `parseLocale` returns `Locale | null` so the desktop shell can decide whether to fall back to browser detection. Do not suggest extracting a shared normalization helper across these two runtimes.
Applied to files:
packages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.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/preload/index.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/windows.ts
📚 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/`), 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-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/index.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/main/menu-template.ts
📚 Learning: 2026-04-24T13:03:10.835Z
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:10.835Z
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/index.tspackages/desktop-electron/src/main/menu-template.test.ts
📚 Learning: 2026-04-20T17:03:40.214Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:40.214Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Applied to files:
packages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-24T06:50:02.712Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 211
File: packages/opencode/src/provider/models.ts:113-179
Timestamp: 2026-04-24T06:50:02.712Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.
Applied to files:
packages/desktop-electron/src/main/index.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.test.ts
📚 Learning: 2026-04-20T14:21:56.373Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 71
File: packages/app/src/components/session/session-status-connections.tsx:146-147
Timestamp: 2026-04-20T14:21:56.373Z
Learning: In the Astro-Han/pawwork repository (SolidJS app), `sync.data.config` is always initialized to `{}` at `packages/app/src/context/global-sync.tsx` line 71 and is never `undefined` at runtime. Non-optional property access like `sync.data.config.plugin` is intentional and consistent with the pattern used in `packages/app/src/components/status-popover-body.tsx` line 243. Do not flag `sync.data.config.plugin` as needing optional chaining.
Applied to files:
packages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.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/components/about-modal.tsx
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `modKey` from utils for cross-platform keyboard shortcuts (Meta on Mac, Control on Linux/Windows)
Applied to files:
packages/desktop-electron/src/main/menu-template.ts
🔇 Additional comments (10)
packages/app/src/i18n/zh.ts (1)
1004-1008: About zh localization entries look correct.The new About keys are complete and consistent with the existing dictionary style.
packages/app/src/i18n/en.ts (1)
1033-1037: About en localization entries are complete and aligned with the new modal.packages/desktop-electron/src/main/index.ts (1)
59-60: About IPC/menu integration is wired correctly in bootstrap.Direct registration plus menu-trigger plumbing looks consistent and cohesive.
Based on learnings: In Astro-Han/pawwork (
packages/desktop-electron/src/main/), the bootstrap entry (index.ts) should directly call each module’s exportedregister*Ipc()function.Also applies to: 480-481, 537-537
packages/desktop-electron/src/main/menu.test.ts (1)
2-14: Test updates correctly follow the macOS-specific menu builder migration.Mock deps and template usage are consistent with the new menu-template contract.
Also applies to: 127-227
packages/desktop-electron/src/preload/index.ts (1)
83-90: About preload bridge implementation is solid.
getAboutInfoandonAboutOpenare correctly exposed with proper listener teardown.packages/desktop-electron/src/main/windows.ts (1)
111-113: Win32 loading-window menu handling looks correct for the native frame transition.packages/app/src/app.tsx (1)
83-85: Renderer-side About wiring is correctly integrated.The
window.apitypings andAboutModalplacement align with the new preload/event flow.Also applies to: 240-243
packages/desktop-electron/src/main/menu-template.test.ts (1)
45-55: Nice regression guard for cross-platform accelerators.The recursive accelerator scan should catch future regressions back to bare
Cmd/Optiontokens on Windows.packages/app/src/components/about-modal.tsx (1)
44-59: Good failure handling around the About bridge.The
try/catchplus early return keeps the shared dialog stack from opening an empty panel when the preload API is missing orabout:get-inforejects.packages/desktop-electron/src/preload/types.ts (1)
25-30: 🧹 Nitpick | 🔵 TrivialMove
AboutInfointo a shared IPC contract.This shape is re-declared here, in
packages/desktop-electron/src/main/ipc/about.ts, and inpackages/app/src/components/about-modal.tsx. The next field change can drift across main/preload/renderer and only fail at runtime. Importing one shared contract from all three layers would make this boundary much safer.⛔ Skipped due to learnings
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/`), 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`.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`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:23.931Z 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`.Learnt from: Astro-Han Repo: Astro-Han/pawwork PR: 211 File: packages/opencode/src/provider/models.ts:113-179 Timestamp: 2026-04-24T06:50:02.712Z Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.Learnt from: Astro-Han Repo: Astro-Han/pawwork PR: 99 File: packages/desktop-electron/src/renderer/i18n/index.ts:30-35 Timestamp: 2026-04-21T13:45:45.149Z Learning: In Astro-Han/pawwork, the locale normalization helpers in `packages/app/src/context/language.tsx` (`normalizeLocale`) and `packages/desktop-electron/src/renderer/i18n/index.ts` (`parseLocale`) are intentionally kept separate and have different fallback shapes: `normalizeLocale` always returns a concrete `Locale` (falling back to `"en"`), while `parseLocale` returns `Locale | null` so the desktop shell can decide whether to fall back to browser detection. Do not suggest extracting a shared normalization helper across these two runtimes.
e477651 to
f75ef44
Compare
Contributor
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/desktop-electron/src/main/menu-template.ts`:
- Around line 178-189: Extract shared submenu builders for repeated menu blocks:
create helper functions like buildEditSubmenu(roleLabel) and
buildGenericSubmenu(roleLabelList, roleLabel) (or separate
buildViewSubmenu/buildGoSubmenu) that return the submenu array used by the menu
templates, then replace the duplicated submenu literals in the "edit" menu
(label "edit") and the similar "view" and "go" menus with calls to those
helpers; ensure the helper accepts the roleLabel function and any
platform-specific differences (e.g., terminal accelerator) as parameters so
macOS and Windows templates can pass a small platform override while keeping a
single source of truth for the common items.
🪄 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: 5c46f47b-afb0-4959-9efe-88d6440cfe93
📒 Files selected for processing (17)
.github/workflows/build.ymlpackages/app/src/app.tsxpackages/app/src/components/about-modal.tsxpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/electron.vite.config.tspackages/desktop-electron/src/main/env.d.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu-template.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/preload/types.tspackages/opencode/test/github/build-workflow.test.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: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-app
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-app
- GitHub Check: typecheck
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: smoke-macos-arm64
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (4)
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/env.d.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/preload/types.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.tsx
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/test/github/build-workflow.test.ts
packages/opencode/test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
packages/opencode/test/**/*.test.{ts,tsx}: Use thetmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup. Useawait usingsyntax to ensure automatic cleanup when the variable goes out of scope.
When using thetmpdirfunction with git repository support, pass thegit: trueoption to initialize a git repo with a root commit.
Use theconfigoption intmpdirto write anopencode.jsonconfig file during test setup by passing a partial Config.Info object.
Use theinitoption intmpdirto define custom setup functions that can return extra data accessible viatmp.extra, and use thedisposeoption for custom cleanup logic.
UsetestEffect(...)fromtest/lib/effect.tsfor tests that exercise Effect services or Effect-based workflows.
Useit.effect(...)when the test should run withTestClockandTestConsole. Useit.live(...)when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers fromfixture/fixture.tsover building manual runtimes in tests: usetmpdirScoped()for scoped temp directories,provideInstance(dir)(effect)for low-level binding without directory creation,provideTmpdirInstance(...)for single temp instance binding, orprovideTmpdirServer(...)for tests that also need the test LLM server.
Defineconst it = testEffect(...)near the top of the test file and keep the test body insideEffect.gen(function* () { ... }). Yield services directly withyield* MyService.Serviceoryield* MyTool.
Avoid customManagedRuntime,attach(...), or ad hocrun(...)wrappers in Effect tests whentestEffect(...)already provides the runtime.
When a test needs instance-local state, preferprovideTmpdirInstance(...)orprovideInstance(...)over manualInstance.provide(...)inside Promise-style tests.
Files:
packages/opencode/test/github/build-workflow.test.ts
🧠 Learnings (24)
📓 Common learnings
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:56.086Z
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.
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/`), 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`.
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.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
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:23.931Z
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`.
📚 Learning: 2026-04-24T17:12:23.931Z
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:23.931Z
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:
.github/workflows/build.ymlpackages/desktop-electron/electron.vite.config.tspackages/desktop-electron/src/main/env.d.tspackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/ipc/about.tspackages/app/src/app.tsxpackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.ts
📚 Learning: 2026-04-24T13:03:10.835Z
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:10.835Z
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:
.github/workflows/build.ymlpackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.ts
📚 Learning: 2026-04-20T14:21:56.373Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 71
File: packages/app/src/components/session/session-status-connections.tsx:146-147
Timestamp: 2026-04-20T14:21:56.373Z
Learning: In the Astro-Han/pawwork repository (SolidJS app), `sync.data.config` is always initialized to `{}` at `packages/app/src/context/global-sync.tsx` line 71 and is never `undefined` at runtime. Non-optional property access like `sync.data.config.plugin` is intentional and consistent with the pattern used in `packages/app/src/components/status-popover-body.tsx` line 243. Do not flag `sync.data.config.plugin` as needing optional chaining.
Applied to files:
.github/workflows/build.ymlpackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-24T17:08:44.294Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:44.294Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
Applied to files:
.github/workflows/build.ymlpackages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-20T17:03:40.214Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 73
File: packages/opencode/src/cli/cmd/tui/context/sync.tsx:486-489
Timestamp: 2026-04-20T17:03:40.214Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/cli/cmd/tui/context/sync.tsx`), `sync.ready` returning `true` when `process.env.OPENCODE_FAST_BOOT` is set is intentional. The plugin-facing data properties `state.config` (initialized to `{}`) and `state.provider` (initialized to `[]`) expose safe-empty defaults, so they are safe to access before bootstrap completes. Do not flag these as needing null-guards or conditional patterns to match `vcs` — the difference is intentional because `vcs` starts as `undefined` while the others have initialized defaults. Changing this would alter the plugin API contract without a concrete failing case.
Applied to files:
.github/workflows/build.ymlpackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-22T09:32:58.310Z
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.
Applied to files:
.github/workflows/build.ymlpackages/desktop-electron/electron.vite.config.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/app/src/app.tsxpackages/desktop-electron/src/main/windows.tspackages/app/src/components/about-modal.tsxpackages/desktop-electron/src/preload/types.ts
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
.github/workflows/build.ymlpackages/opencode/test/github/build-workflow.test.tspackages/desktop-electron/src/main/menu-template.test.ts
📚 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/env.d.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/ipc/about.tspackages/desktop-electron/src/main/windows.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.ts
📚 Learning: 2026-04-22T05:32:29.012Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 98
File: packages/desktop-electron/src/main/menu-labels.ts:1-2
Timestamp: 2026-04-22T05:32:29.012Z
Learning: In Astro-Han/pawwork, the app i18n layer (`packages/app/src/i18n/`) only contains `en.ts` and `zh.ts`, and `normalizeLocale` (in `packages/app/src/context/language.tsx`) only returns `"en"` or `"zh"`. The desktop `MenuLocale = "en" | "zh"` union in `packages/desktop-electron/src/main/menu-labels.ts` is intentionally limited to these two locales and is not a broader restriction — do not flag it as overly restrictive or suggest adding other locales.
Applied to files:
packages/app/src/i18n/en.tspackages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/menu.tspackages/desktop-electron/src/main/menu.test.tspackages/desktop-electron/src/main/menu-template.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/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.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/i18n/en.tspackages/app/src/i18n/zh.tspackages/app/src/app.tsxpackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-23T15:10:26.994Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/pages/layout/pawwork-sidebar.tsx:139-139
Timestamp: 2026-04-23T15:10:26.994Z
Learning: In Astro-Han/pawwork, the icon registry in `packages/ui/src/components/icon.tsx` uses **bare (unquoted) TypeScript object keys** for single-word icon names (e.g., `pin:` at line 102, `workspace:` at line 99) and **quoted keys** only for hyphenated names (e.g., `"review-active":`). When verifying icon registration with `rg`, always search for both forms — use a pattern like `rg -n "(\"${key}\"|${key}:)"` — otherwise bare-key icons will be missed and incorrectly flagged as unregistered.
Applied to files:
packages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.ts
📚 Learning: 2026-04-24T05:39:56.086Z
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:56.086Z
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/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.tspackages/desktop-electron/src/main/ipc/about.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-23T17:02:39.474Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/ui/src/theme/themes/pawwork.json:32-33
Timestamp: 2026-04-23T17:02:39.474Z
Learning: In Astro-Han/pawwork, `#FF5910` is the locked PawWork brand orange used for `button-primary-base` (and `--accent-brand` in `index.css`, theme token `interactive`/`primary`). The `text-on-interactive-base: `#FFFFFF`` on `button-primary-base: `#FF5910`` combination produces a contrast ratio of ~3.14:1. This intentionally fails WCAG AA normal-text (4.5:1) but passes AA non-text (SC 1.4.11, 3:1) and AA large-text (SC 1.4.3, 3:1). The tradeoff is accepted for brand consistency; do NOT flag this contrast pair as a WCAG violation in future reviews. If a specific surface requires stricter contrast, it should be addressed individually (e.g., larger text, ghost variant, outline button) rather than changing the palette.
Applied to files:
packages/app/src/i18n/zh.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-23T17:02:40.117Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/app/src/index.css:95-97
Timestamp: 2026-04-23T17:02:40.117Z
Learning: In Astro-Han/pawwork, Stylelint is NOT configured in the project. Per AGENTS.md, only linting (not formatting) is used, and the Biome formatter is explicitly disabled. Do NOT raise Stylelint-based review comments (e.g., `declaration-empty-line-before`) in this repo, as they are false positives from the static analysis environment and not enforced by the project toolchain.
Applied to files:
packages/app/src/i18n/zh.tspackages/app/src/components/about-modal.tsx
📚 Learning: 2026-04-21T13:45:45.149Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 99
File: packages/desktop-electron/src/renderer/i18n/index.ts:30-35
Timestamp: 2026-04-21T13:45:45.149Z
Learning: In Astro-Han/pawwork, the locale normalization helpers in `packages/app/src/context/language.tsx` (`normalizeLocale`) and `packages/desktop-electron/src/renderer/i18n/index.ts` (`parseLocale`) are intentionally kept separate and have different fallback shapes: `normalizeLocale` always returns a concrete `Locale` (falling back to `"en"`), while `parseLocale` returns `Locale | null` so the desktop shell can decide whether to fall back to browser detection. Do not suggest extracting a shared normalization helper across these two runtimes.
Applied to files:
packages/app/src/i18n/zh.tspackages/desktop-electron/src/main/index.tspackages/desktop-electron/src/main/menu-template.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : When using the `tmpdir` function with git repository support, pass the `git: true` option to initialize a git repo with a root commit.
Applied to files:
packages/opencode/test/github/build-workflow.test.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/github/build-workflow.test.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/index.tspackages/desktop-electron/src/preload/index.tspackages/desktop-electron/src/main/windows.ts
📚 Learning: 2026-04-24T06:50:02.712Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 211
File: packages/opencode/src/provider/models.ts:113-179
Timestamp: 2026-04-24T06:50:02.712Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/provider/models.ts`), `PublishModel` and `PublishProvider` intentionally duplicate fields from `Model` and `Provider` with looser optionality. The split is a deliberate behavior boundary: `PublishModel`/`PublishProvider` (with `.passthrough()`) are used for lenient publish-time validation of incoming catalogs, while `Model`/`Provider` are used for strict runtime normalization. Do not flag this duplication as a maintenance burden or suggest extracting a shared base schema — the explicit separation is intentional and tied to the models-refresh reliability path introduced in PR `#211`.
Applied to files:
packages/desktop-electron/src/main/index.tspackages/app/src/components/about-modal.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/`), 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-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/components/about-modal.tsx
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `modKey` from utils for cross-platform keyboard shortcuts (Meta on Mac, Control on Linux/Windows)
Applied to files:
packages/desktop-electron/src/main/menu-template.ts
🔇 Additional comments (12)
packages/desktop-electron/src/main/windows.ts (1)
111-113: Verify the Electron behavior: doesMenu.setApplicationMenu()apply the menu to the loading window despite priorremoveMenu()call.The bootstrap sequence confirms that on Windows,
win.removeMenu()is called when the loading window is created (lines 111–113), butMenu.setApplicationMenu()is invoked later viawireMenu()inopenMainWindow()(line 371) while the loading window is still open (closed only at line 441).The concern depends on Electron's behavior: whether
setApplicationMenu()reapplies the menu to windows that previously hadremoveMenu()called on them. If it does, the loading window should be re-cleared after menu setup. If Electron respects the explicitremoveMenu()call, no additional action is needed.packages/desktop-electron/src/main/index.ts (1)
59-60: About IPC bootstrap and menu wiring are correctly integrated.The new About path is coherently wired end-to-end from menu action to main-process IPC registration.
Based on learnings:
packages/desktop-electron/src/main/index.tsis the intended bootstrap point that directly calls each feature module’s exportedregister*Ipc()function.Also applies to: 480-481, 537-537
packages/desktop-electron/src/preload/index.ts (1)
83-90: Preload About bridge implementation is clean and safe.The API surface and unsubscribe behavior are correct and match expected preload event patterns.
packages/app/src/app.tsx (1)
45-46: About modal app-level integration is correctly placed.Typing and provider placement are aligned so the modal can consume the shared dialog stack safely.
Also applies to: 83-85, 240-243
packages/desktop-electron/src/main/menu-template.test.ts (1)
22-68: Menu template regression coverage is strong for this change set.The new tests validate platform menu structure, About presence, and accelerator token correctness in a focused way.
packages/desktop-electron/src/preload/types.ts (1)
25-30: Preload API type contract is consistent and well-scoped.
AboutInfoand the new About methods are clearly modeled and align with the implemented IPC bridge.Also applies to: 97-98
packages/app/src/components/about-modal.tsx (1)
40-60: About modal event flow and failure handling are solid.Subscription lifecycle, guarded info fetch, and shared dialog-stack usage are all implemented correctly.
packages/desktop-electron/src/main/menu.ts (1)
17-35: Platform menu split and click rebinding are implemented correctly.The template adaptation and platform gating preserve expected behavior while enabling Windows menu support cleanly.
Also applies to: 38-60
packages/desktop-electron/src/main/ipc/about.ts (1)
15-33: About IPC module is cohesive and correctly scoped.Handler registration and app-shell window targeting are well implemented for cross-platform About triggering.
packages/desktop-electron/src/main/menu-template.ts (3)
1-25: LGTM!The type updates correctly model Electron's menu click handler signature and properly extend
MenuTemplateDepswith the newtriggerAboutdependency for the cross-platform About flow.
34-146: LGTM!The function rename to
buildMacosMenuTemplateis appropriate, and the accelerator migration toCmdOrCtrl/Altpatterns correctly enables cross-platform shortcut resolution. The existing `Ctrl+`` for terminal toggle on macOS follows VS Code convention.
148-233: LGTM!The Windows menu template correctly implements Windows conventions: About in Help (with
triggerAboutwiring), Quit in File, and no macOS-specific items like "front" or the app-name menu. The accelerators usingCmdOrCtrl/Altpatterns will resolve correctly on Windows.
7 tasks
This was referenced Apr 27, 2026
11 tasks
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
1 participant
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.
Summary
frame: true), kill the frameless+overlay setup; remove the unusedsetTitlebarIPC chain end to end.buildMacosMenuTemplate,buildWindowsMenuTemplate); install Win/macOS application menu via dispatch; Linux remains menu-free.role:aboutpanel; Windows + Linux ship a Kobalte Dialog modal (AboutModal) wired through newabout:get-info/about:openpreload bridge.Cmd/Optionliterals toCmdOrCtrl/Altso Windows shortcuts resolve correctly without a second template.Why
Windows users currently see: extra app-drawn border, right-corner buttons pushed inboard by the WCO placeholder, and no menu bar (so no in-app discovery for About / Check for Updates / file actions). This PR aligns the Windows shell with native Win11 conventions and exposes a parallel About entry point.
Related Issue
No issue filed; captured directly via brainstorming + spec.
How To Verify
```bash
bun --cwd packages/desktop-electron run typecheck
bun --cwd packages/app run typecheck
bun --cwd packages/desktop-electron test
bun --cwd packages/app test
```
All pass locally (295 desktop-electron, 571 app, including new `menu-template.test.ts` with 5 cases and `shell-frame-contract.test.ts` regression for the linux-only inset selector + Win-no-placeholder).
macOS visual smoke (manually verified):
Screenshots or Recordings
No new visible UI on macOS (regression-only). Windows visuals validated via design comparison; real-device verification deferred to release smoke per PawWork policy (no Windows runner in `desktop-smoke.yml`).
Checklist
Known follow-ups (not in scope)
Summary by CodeRabbit
New Features
Chores