feat(desktop): Windows x64 build and runtime support (#273)#423
feat(desktop): Windows x64 build and runtime support (#273)#423
Conversation
最初の Windows 対応。Windows 10/11 x64 で app が起動し、 ワークスペース/ターミナル/チャットなどのコア機能が動く状態を作る。 agent hook と一部の macOS 固有統合は Windows では skip して known limitation とし、follow-up PR で段階的に PowerShell 実装を追加する。 ## 起動基盤 (upstream superset-sh#2100 を fork 用に再実装) - scripts/postinstall.sh -> scripts/postinstall.mjs (fork の \$CI 早期 exit ロジックを維持、Windows では install:deps を非致命に) - electron-builder.ts: npmRebuild を Windows で無効化 (VS Build Tools 無しでの node-gyp 失敗を回避)、NSIS Desktop/ Start Menu ショートカットを追加 - electron.vite.config.ts: rollup banner で require("electron") 前に ELECTRON_RUN_AS_NODE をクリア、renderer に stripCrossOriginPlugin を追加 (ASAR file:// の CORS 問題でブラック画面になるのを防止) - copy-native-modules.ts: Windows で Bun が作る directory junction を recursive rmSync で削除するよう分岐 - factories/app/setup.ts: Windows でも GPU hardware acceleration を disable (ドライバ互換によるブラック画面防止) - window-loader.ts: Windows production は superset-app:// カスタム プロトコル経由でロード (file:// だと ES dynamic import が破綻する) - main/index.ts: superset-app scheme の privileged 登録 + protocol handler、api.superset.sh / PostHog / Sentry への outbound CORS バイパス - main/windows/main.ts: Windows は titleBarOverlay で閉じる/最小化 ボタンを描画、nativeTheme で dark/light 色追従、renderer console の warning/error を stdout に forward - vite/helpers.ts: defineEnv の ?? を || に変更 (空文字列 CI secret フォールバック)、stripCrossOriginPlugin を export ## macOS 固有機能の Windows fallback - play-sound.ts: Windows で PowerShell の System.Media.SoundPlayer (WAV) / MediaPlayer (他) 経由の再生分岐を追加 - ScriptsEditor.tsx: file input の accept に .ps1/.cmd/.bat を追加 - auto-updater は既に IS_FORK path で GitHub API 経由のチェックが動く ため Windows でも更新通知が出る (変更なし) ## agent-setup / host-service - agent-setup/index.ts: Windows では wrapper/hook setup を skip。 bash 専用の notify / copilot / cursor / gemini / codex テンプレートを PowerShell に完全移植する作業は follow-up PR - host-service/.../setup-terminal.ts + runtime/teardown/teardown.ts: Windows では setup.ps1 / .cmd / .bat (resp. teardown) を探して powershell.exe -File または cmd.exe /c で spawn。POSIX は従来どおり setup.sh / teardown.sh - providers/git/.../askpass.ts: Windows では findstr ベースの .cmd askpass ヘルパを生成 ## CI - .github/workflows/build-desktop.yml: windows-latest で NSIS x64 インストーラをビルドする build-windows job を追加。 git config --global core.longpaths true、Git Bash で fork 既存の bash スクリプトを動かし、bun run build:browser-mcp で superset-browser-mcp.exe を生成、electron-builder --win で .exe + .exe.blockmap + latest.yml を artifact にアップロード ## Follow-up (本 PR の対象外) - notify / copilot / cursor / gemini / codex hook テンプレートの PowerShell 版 - Windows での agent wrapper (PATH 注入, sleep-inhibitor) - Windows 11 の Mica / Acrylic vibrancy fallback - Windows NSIS auto-update 設定 (Squirrel.Windows 対応) - dev mode の deep-link 登録 (Windows レジストリ) - BUILDING.md / README の Windows 手順追記 Refs: #273
📝 WalkthroughWalkthroughこの変更は、Windows デスクトップ向けサポートを追加し、Windows 固有の起動/プロトコル、CORS 修正、音声再生、スリープ抑止、エージェントフックの Windows 化、Vite/Electron ビルド調整、NSIS インストーラー生成と Windows 用 CI ワークフローを導入します。 Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer (renderer)
participant Main as Electron Main
participant FS as File System
participant API as API Server
Note over Renderer,Main: Windows production uses superset-app:// scheme
Renderer->>Main: request superset-app://app/renderer.js (dynamic import)
Main->>FS: read ../renderer/renderer.js
FS-->>Main: file contents
Main-->>Renderer: serve JS with file://-compatible response
Renderer->>API: fetch /api/endpoint (Origin: superset-app://app)
API->>Main: routed via webRequest (persist:superset) or normal
Main->>API: remove Origin header when superset-app origin present
API-->>Main: respond (may lack CORS for superset-app)
Main->>Renderer: append Access-Control-Allow-Origin: superset-app://app and forward
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
ポエム
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81352c8248
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| buildCommand: (p) => | ||
| `powershell.exe -NoProfile -ExecutionPolicy Bypass -File ${doubleQuote(p)}`, |
There was a problem hiding this comment.
runTeardown relies on the PTY session exiting to mark teardown success, but the new Windows command string only runs powershell.exe -File ... (or cmd.exe /c ...) inside the interactive cmd shell created by createTerminalSessionInternal and never exits that parent shell. Unlike the POSIX branch (bash ... ; exit $?), this leaves the PTY open until timeout, so any workspace with .superset/teardown.ps1/.cmd/.bat will be reported as teardown failure and block normal workspace deletion unless users force-delete.
Useful? React with 👍 / 👎.
Fork (MocA-Love/superset) uses v*-fork.* tags that release-desktop.yml does not react to, so the fork would otherwise have no way to invoke the new build-windows job. Adding workflow_dispatch lets anyone kick off a manual build from the Actions tab and download the .exe / .dmg / .AppImage artifacts without cutting a real release.
…ilds VS2022 Build Tools の default workload は Spectre-mitigated CRT を含まないため、 node-pty@1.1.0 の binding.gyp と deps/winpty/src/winpty.gyp にある `'SpectreMitigation': 'Spectre'` を有効にしたまま MSBuild を回すと windows-latest ランナーで MSB8040 で失敗する。 bun patch で patches/node-pty@1.1.0.patch を追加し、3 箇所の msvs_configuration_attributes ブロックを削除 (Spectre 属性以外のオプションは CRT パッケージに依存しない)。fork 運用のままだと Windows CI の install:deps ステップでビルドが必ずコケるため、Windows NSIS リリースの 前提条件。 macOS / Linux はこのブロックを元々踏まないので副作用なし。 Refs: #273
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/components/ScriptsEditor/ScriptsEditor.tsx (1)
58-75:⚠️ Potential issue | 🔴 Critical
importFirstFileのフィルタが.ps1/.cmd/.batを拒否するため、Windows スクリプトのインポートが無音で失敗します。Line 158 で
acceptに.ps1,.cmd,.batを追加しましたが、Line 61 の正規表現/\.(sh|bash|zsh|command)$/iは変更されていません。ファイルピッカーで選択された.ps1/.cmd/.batや、同フォーマットのドラッグ&ドロップはfiles.find(...)で早期 return され、onChangeが呼ばれません。結果として「ファイル選択ダイアログでは選べるのに取り込まれない」という挙動になります。🛠 Proposed fix
const scriptFile = files.find((file) => - file.name.match(/\.(sh|bash|zsh|command)$/i), + file.name.match(/\.(sh|bash|zsh|command|ps1|cmd|bat)$/i), );Also applies to: 155-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/components/ScriptsEditor/ScriptsEditor.tsx around lines 58 - 75, The importFirstFile callback's file filter only matches Unix shells and ignores Windows scripts, so selected .ps1/.cmd/.bat files never call onChange; update the files.find regex inside importFirstFile to include Windows extensions (e.g. /\.(sh|bash|zsh|command|ps1|cmd|bat)$/i) to match those files (and similarly update the other equivalent filter used near the duplicate callback) so Windows script files picked via the file picker or drag-and-drop are accepted and their content passed to onChange.apps/desktop/src/main/lib/agent-setup/index.ts (1)
19-46:⚠️ Potential issue | 🔴 CriticalWindows での
setupDesktopAgentCapabilities()スキップについて確認されました。
DESKTOP_AGENT_SETUP_RUNNERSには bash 依存のフック実装だけでなく、JSON 設定ファイルを書き込むだけのcursor-hooks-json(~/.cursor/hooks.json)、droid-settings-json(~/.factory/settings.json)、codex-hooks-json、gemini-settings-json、mastra-hooks-jsonなども含まれています。これらは shell ラッパに依存しておらず、Windows でも実行可能です。ただし、重大な問題が確認されました。
teardown.tsの line 44-45 は Windows を考慮せずprocess.platform === "darwin" ? "/bin/zsh" : "/bin/bash"と判定するため、Windows でも/bin/bashをスポーンしようとします。このパスは Windows に存在しないためspawnは失敗します。session.tsはgetDefaultShell()でprocess.platform === "win32"を確認し Windows ではcmd.exeを使用するため問題ありませんが、teardown.tsは Windows 対応が必要です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/index.ts` around lines 19 - 46, The current setupAgentHooks() early-return on Windows prevents non-shell-dependent runners from running and teardown.ts spawns a hardcoded /bin/bash on non-mac OS, which breaks on Windows; change setupAgentHooks() to still call setupDesktopAgentCapabilities() on Windows and to execute only JSON-only runners from DESKTOP_AGENT_SETUP_RUNNERS (e.g., cursor-hooks-json, droid-settings-json, codex-hooks-json, gemini-settings-json, mastra-hooks-json) while skipping bash/zsh wrapper creation on Windows, and update teardown.ts to detect Windows (process.platform === "win32") and use an appropriate shell (e.g., cmd.exe or the same logic as session.ts's getDefaultShell()) or avoid spawning /bin/bash there so spawn() will not fail on Windows.apps/desktop/scripts/copy-native-modules.ts (1)
371-385:⚠️ Potential issue | 🟠 MajorWindows junction 対応が 379 行で抜けています。
preferNested側（285-289 行）とcopyModuleIfSymlink（127-131 行）では Windows の directory junction を考慮してrmSync(..., { recursive: true, force: true })に切り替えていますが、preferNestedではない通常経路の「既に nested が要件を満たしているが symlink の場合」にあたるこのブロックでは従来のままrmSync(nestedDependencyPath)になっています。Windows で nested 依存が junction として配置されているケースを通るとEPERM/EACCESで失敗します。他の2箇所と同じ分岐を適用してください。🛡️ 提案する修正
const nestedVersion = readInstalledModuleVersion(nestedDependencyPath); if ( nestedVersion && satisfies(nestedVersion, resolvedDependency.sourceVersionRange) ) { const nestedStats = lstatSync(nestedDependencyPath); if (nestedStats.isSymbolicLink()) { const realPath = realpathSync(nestedDependencyPath); - rmSync(nestedDependencyPath); + if (process.platform === "win32") { + rmSync(nestedDependencyPath, { recursive: true, force: true }); + } else { + rmSync(nestedDependencyPath); + } cpSync(realPath, nestedDependencyPath, { recursive: true, }); } return nestedDependencyPath; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/copy-native-modules.ts` around lines 371 - 385, The symlink removal in the block that handles an already-satisfying nested dependency uses rmSync(nestedDependencyPath) which fails on Windows junctions; change that rmSync call inside the nestedStats.isSymbolicLink() branch to use rmSync(nestedDependencyPath, { recursive: true, force: true }) (same behavior used in preferNested and copyModuleIfSymlink) so Windows directory junctions are removed safely before cpSync(realPath, nestedDependencyPath, { recursive: true }).apps/desktop/vite/helpers.ts (1)
19-27:⚠️ Potential issue | 🟡 Minor
||移行の挙動変更についての注意（例示の修正）。意図（未解決 CI シークレットの空文字を既定値で埋める）は妥当ですが、ビルド時に「ユーザーが意図的に空文字を指定した」ケースも
fallbackで上書きされる可能性があります。ただし、SENTRY_DSN_DESKTOPはfallbackパラメータなしで呼び出されているため、この例は該当しません。代わりに、SKIP_ENV_VALIDATIONなど実際にfallbackを持つ変数で、意図的な空文字値が有意な場合がないかご確認ください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/vite/helpers.ts` around lines 19 - 27, The defineEnv function currently uses value || fallback which treats intentional empty-string env values the same as undefined; review call sites (e.g., SENTRY_DSN_DESKTOP and any uses like SKIP_ENV_VALIDATION that pass a fallback) and decide if empty string should be preserved — if so, change the call site to pass undefined when missing or update usage to use nullish-coalescing (??) or an explicit check for undefined only; ensure defineEnv or its callers clearly distinguish between undefined (use fallback) and "" (preserve) for any envs where an intentional empty value is meaningful.
🧹 Nitpick comments (6)
apps/desktop/src/main/windows/main.ts (1)
336-349: titleBarOverlay の色定義が 2 箇所で重複しています。初期化（341-346 行）と
nativeTheme更新（390-394 行）で同じ色リテラルが重複しており、将来テーマ変更時の修正漏れを招きやすいです。ヘルパーに抽出すると一貫性が保てます。♻️ 提案する修正
+function getTitleBarOverlayOptions() { + const isDark = nativeTheme.shouldUseDarkColors; + return { + color: isDark ? "#1e1e1e" : "#ffffff", + symbolColor: isDark ? "#ffffff" : "#000000", + height: 35, + }; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/windows/main.ts` around lines 336 - 349, The titleBarOverlay color literals are duplicated between the initial window options (inside PLATFORM.IS_WINDOWS block) and the nativeTheme update handler; extract a helper (e.g. getTitleBarOverlayColors or computeTitleBarOverlay) that returns { color, symbolColor, height } based on nativeTheme.shouldUseDarkColors and use that helper in both the initial creation (where titleBarOverlay is set) and in the nativeTheme 'updated' handler to update the overlay, eliminating duplicated literals and keeping behavior tied to a single source of truth.apps/desktop/vite/helpers.ts (1)
78-89:crossorigin属性の正規表現がクォート付き形式のみを想定しています。Vite の現行出力は
crossorigin="anonymous"形式なので実害はありませんが、将来的にシングルクォートや裸値形式に変わった場合、crossorigin=anonymousのような裸値属性を取りこぼし、壊れた HTML（=anonymousのみ残る）になるリスクがあります。防御的に属性値の形式を広げておくと安全です。♻️ 提案する修正
- handler(html) { - if (process.platform !== "win32") return html; - return html.replace(/ crossorigin(?:="[^"]*")?/g, ""); - }, + handler(html) { + if (process.platform !== "win32") return html; + // Match optional = with double/single-quoted or bare value. + return html.replace( + / crossorigin(?:=(?:"[^"]*"|'[^']*'|[^\s"'>]+))?/g, + "", + ); + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/vite/helpers.ts` around lines 78 - 89, The regex in stripCrossOriginPlugin's transformIndexHtml.handler only matches double-quoted crossorigin attributes and can miss single-quoted or unquoted forms leaving stray "=anonymous"; update the replacement regex to match crossorigin with optional value in double quotes, single quotes, or unquoted (e.g. pattern like / crossorigin(?:=(?:"[^"]*"|'[^']*'|[^\s>]*))?/g) so the entire attribute (value if present) is removed; modify the handler in stripCrossOriginPlugin to use this broader regex when returning the transformed html.apps/desktop/scripts/copy-native-modules.ts (1)
125-131: Windows junction 解決処理を共通ヘルパに括り出してください。3 箇所で同じ
process.platform === "win32" ? rmSync(..., { recursive: true, force: true }) : rmSync(...)を繰り返しています。軽微ですが将来の抜け漏れ（上記 379 行のような）を防ぐため小さなユーティリティに切り出すのが安全です。♻️ 提案する修正
+function removeSymlinkOrJunction(target: string): void { + // Windows では Bun が direct deps を directory junction として作成するため、 + // 再帰削除が必要。POSIX の symlink は単体削除で十分。 + if (process.platform === "win32") { + rmSync(target, { recursive: true, force: true }); + } else { + rmSync(target); + } +}Also applies to: 283-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/copy-native-modules.ts` around lines 125 - 131, Extract the platform-specific deletion logic into a small helper (e.g., safeRmSync or removePathHandlingWindows) and replace the three repeated conditionals that check process.platform === "win32" and call rmSync(modulePath, { recursive: true, force: true }) vs rmSync(modulePath) with calls to that helper; implement the helper to accept a path (modulePath) and perform rmSync(path, { recursive: true, force: true }) when process.platform === "win32" otherwise rmSync(path) so all current call sites (the block using rmSync/modulePath and the similar repetition around the other occurrences) use the same centralized behavior.packages/host-service/src/trpc/router/workspace-creation/shared/setup-terminal.ts (1)
12-24: 2 ファイル間で重複している型／候補配列／doubleQuoteの共通化を検討してください。
SetupScriptCandidate/TeardownScriptCandidateの型、候補配列のディスパッチロジック、singleQuote/doubleQuoteの実装がteardown.tsと本ファイルで完全に重複しています。shared/script-candidates.tsのような共有モジュールに切り出すと、Windows 側候補の追加・修正（例: 上で指摘した teardown 側の; exit $LASTEXITCODE追加）が一箇所で完結します。Also applies to: 69-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/trpc/router/workspace-creation/shared/setup-terminal.ts` around lines 12 - 24, The POSIX_CANDIDATES/WINDOWS_CANDIDATES arrays and the types/functions SetupScriptCandidate, TeardownScriptCandidate, singleQuote, and doubleQuote are duplicated between setup-terminal.ts and teardown.ts; extract them into a new shared module (e.g., shared/script-candidates) that exports the types and the candidate arrays and quoting helpers, centralize Windows-specific command tweaks (like the teardown-specific `; exit $LASTEXITCODE`) in the shared candidates or provide a small post-process hook on the buildCommand, then replace the local declarations in setup-terminal.ts and teardown.ts with imports of the shared symbols (keeping function names: POSIX_CANDIDATES, WINDOWS_CANDIDATES, singleQuote, doubleQuote, SetupScriptCandidate, TeardownScriptCandidate) so future changes need only one update.package.json (1)
52-55:node-pty@1.1.0パッチについて: Windows ビルド設定のみの変更を確認コーディングガイドラインに従い、公開されている upstream パッケージを使用すること、カスタムパッチを避けることが原則ですが、PR の Windows CI MSB8040 回避が明示的な要件のため許容範囲です。
パッチ内容を確認したところ:
- ✅ 変更は Windows ビルド設定のみ (
OS=="win"条件内) — binding.gyp と deps/winpty/src/winpty.gyp の SpectreMitigation 属性削除- ✅ macOS/Linux のネイティブビルドは影響なし
- MSB8040 エラーと GitHub Actions の Spectre-mitigated CRT 非搭載の原因説明もパッチ内に記載済み
ただし、upstream node-pty への同様パッチ提出を検討する旨の TODO/Issue リンクが、パッチコメント内に明示されていません。後続メンテナンス向上のため、パッチまたはコミットメッセージに upstream 報告への言及を追加することを推奨します。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 52 - 55, The node-pty@1.1.0 local patch (referenced in package.json under patchedDependencies) removes SpectreMitigation attributes from binding.gyp and deps/winpty/src/winpty.gyp to avoid MSB8040 on Windows; please update the patch header or the commit message that produced the patch to add a clear note referencing an upstream report/PR or issue (e.g., mention "upstream node-pty" and link to the intended issue/PR), and include brief context mentioning MSB8040 and GitHub Actions Spectre-mitigated CRT to aid future maintainers locating binding.gyp and deps/winpty/src/winpty.gyp changes.packages/host-service/src/runtime/teardown/teardown.ts (1)
13-13:TEARDOWN_SCRIPT_REL_PATHを公開シンボルから削除する、またはPOSIX固有シンボルとしてドキュメント化してください。
export const TEARDOWN_SCRIPT_REL_PATH = ".superset/teardown.sh"はindex.tsで公開再エクスポートされていますが、外部参照がなく、POSIX候補専用の内部定数です。現在のコード設計では、プラットフォーム選択ロジック（行82-84）がPOSIX_TEARDOWN_CANDIDATESとWINDOWS_TEARDOWN_CANDIDATESを使い分けており、このシンボルの公開は設計の意図を反映していません。POSIX_TEARDOWN_CANDIDATES内部に隠匿するか、コメントで「POSIX候補のレガシー互換シンボル」と明記してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/host-service/src/runtime/teardown/teardown.ts` at line 13, TEARDOWN_SCRIPT_REL_PATH is an internal POSIX-only constant but is exported; either remove its export (make it non-exported) and move the string into the POSIX_TEARDOWN_CANDIDATES array, or keep it but mark it clearly as a POSIX-only legacy symbol with a comment and update index.ts to avoid re-exporting it; update references in the teardown module to use POSIX_TEARDOWN_CANDIDATES (or the new internal const) and ensure WINDOWS_TEARDOWN_CANDIDATES logic remains unchanged.
🤖 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-desktop.yml:
- Around line 58-62: The default for the input parameter artifact_retention_days
currently differs between the workflow_call caller (30) and the
workflow_dispatch manual trigger (14); update the workflow to make these
consistent or add an inline comment explaining the intentional difference.
Locate the artifact_retention_days declaration and either change its default to
match the other trigger (e.g., set default: 30 or 14) in the
.github/workflows/build-desktop.yml or add a clear comment next to
artifact_retention_days referencing workflow_call and workflow_dispatch and
explaining why the retention periods intentionally differ.
In `@apps/desktop/src/main/index.ts`:
- Around line 780-793: The appProtocolHandler currently builds filePath with
path.join(rendererDir, urlPath) and may allow path traversal via “..”; fix by
normalizing and resolving the requested path (use path.normalize and
path.resolve) and verify the resolved path is inside rendererDir (check
resolvedPath === rendererDir or resolvedPath.startsWith(rendererDir +
path.sep)); if the check fails, return a safe error response (e.g., 403 or a
not-found) instead of calling net.fetch on the resolved path; update the handler
referenced as appProtocolHandler (and where it’s registered with protocol.handle
and session.fromPartition(...).protocol.handle) to perform this validation
before calling pathToFileURL and net.fetch.
- Around line 798-806: appSession の CORS 設定 uses process.env.NEXT_PUBLIC_API_URL
when computing apiHost which differs from the rest of the file; replace the
direct process.env reference with mainEnv.NEXT_PUBLIC_API_URL so the packaged
build reads the correct value. Update the apiHost computation (and any use in
corsTargets) to source NEXT_PUBLIC_API_URL from mainEnv, keeping the same
fallback ("https://api.superset.sh") and ensuring references to apiHost,
corsTargets and appSession remain unchanged.
- Around line 601-611: The CSP for the custom scheme declared by scheme:
"superset-app" may not be covered by the current 'self'-based CSP and differs
from the other superset-* schemes; update the code that builds the renderer's
Content-Security-Policy to explicitly allow superset-app: in script-src and
connect-src (and media-src if other schemes include media) or set bypassCSP:
true on the scheme declaration if that is the intended behavior, and verify the
renderer bundle delivery still works without stream: true; locate the scheme
declaration (scheme: "superset-app") and the CSP-generation/registration code
that references other superset-* schemes to make the change and run the renderer
to confirm scripts, connects and media load as expected.
In `@apps/desktop/src/main/lib/play-sound.ts`:
- Around line 39-57: The Windows branch in play-sound.ts must mirror Linux's
cancellation behavior and robustly wait for MediaPlayer to load: update the
PowerShell script (the string assigned to script used in execFile) to (1) poll
until $p.NaturalDuration.HasTimeSpan becomes true with a reasonable timeout
instead of a single Start-Sleep 500ms, (2) inside the waiting and playback loop
periodically invoke a callback signal check (expose callbacks?.isCanceled via a
mechanism such as writing a sentinel file, exit code, or using PowerShell
argument/state) so playback stops when callbacks?.isCanceled?() is true, and (3)
ensure callbacks?.onComplete?.() is called only when playback finishes normally;
also make sure the WAV/MediaPlayer branches both perform the same cancellation
checks and finalization. Locate and modify the script construction and execFile
invocation around the isWav, script, and execFile(...) symbols to implement
these changes.
In `@apps/desktop/src/main/windows/main.ts`:
- Around line 386-395: The nativeTheme.on("updated", ...) listener is being
added inside MainWindow() for each new window and never removed, causing
listener leaks and potential MaxListenersExceededWarning; instead, either
register a single module-level nativeTheme.on("updated", ...) that iterates all
BrowserWindow instances and updates title bars (like the existing top-level
listener at lines ~185-191) or, if keeping the per-window handler, register the
listener and remove it when the window closes by calling nativeTheme.off inside
window.once("closed", ...) (use the same callback reference tied to
window.setTitleBarOverlay updates and guard with window.isDestroyed() as already
done).
- Around line 372-384: The console-message handler uses the old signature;
update the listener on window.webContents (the call to
window.webContents.on("console-message", ...)) to accept the new payload shape
(e.g., (event, details) where details.level is a string like
"info"|"warning"|"error"|"debug" and line is details.lineNumber), replace
numeric-level checks (level < 2 / level === 3) with string-based logic (map or
switch details.level to "verbose"/"info"/"warning"/"error", treat "debug" as
"verbose"), build the source using details.sourceId and details.lineNumber, and
log using console.error for "error" and console.warn for "warning"/others so
behavior matches the original intent.
In
`@packages/host-service/src/providers/git/CloudGitCredentialProvider/askpass.ts`:
- Around line 29-32: The escape function escapeCmdEcho currently prefixes all
matched chars (including '%') with '^', but in Windows batch files '%' must be
escaped as '%%' not '^%'; modify escapeCmdEcho so the regex still matches
/([&<>^|%])/g but use a replacer function that returns '%%' when the matched
character is '%' and '^' + ch for all other matched characters, preserving
correct escaping for cmd.exe echo/findstr.
In `@packages/host-service/src/runtime/teardown/teardown.ts`:
- Around line 27-41: The Windows teardown commands in
WINDOWS_TEARDOWN_CANDIDATES leave the parent PTY shell running so
session.pty.onExit never fires; change the buildCommand functions so the invoked
command ensures the parent cmd.exe exits with the child's status by appending a
cmd-style exit that propagates the child exit code (use %ERRORLEVEL%), ensuring
proper quoting around doubleQuote(p) and correct placement for both the
PowerShell and cmd/bat candidates; update the buildCommand closures for the
PowerShell entry (and the cmd/bat entries if needed) to wrap the invocation so
the parent shell runs an explicit "exit" with %ERRORLEVEL% after the script
finishes.
In `@patches/node-pty`@1.1.0.patch:
- Around line 1-45: The patch removes 'msvs_configuration_attributes'
'SpectreMitigation' entries from binding.gyp and deps/winpty/src/winpty.gyp,
turning off Spectre mitigation for all Windows builds; instead, update CI to
install the Spectre-mitigated CRT so you can keep the SpectreMitigation
settings: add a CI step that runs the Visual Studio installer CLI (e.g.,
vs_installer.exe or Setup-VsDevCmd) to install the
Microsoft.VisualStudio.Component.VC.Runtimes.x86.x64.Spectre component before
building, revert or avoid removing the 'SpectreMitigation' entries in
binding.gyp and winpty.gyp (msvs_configuration_attributes with
'SpectreMitigation': 'Spectre') once the runner provides the CRT, and treat the
current patch as temporary until CI is updated.
---
Outside diff comments:
In `@apps/desktop/scripts/copy-native-modules.ts`:
- Around line 371-385: The symlink removal in the block that handles an
already-satisfying nested dependency uses rmSync(nestedDependencyPath) which
fails on Windows junctions; change that rmSync call inside the
nestedStats.isSymbolicLink() branch to use rmSync(nestedDependencyPath, {
recursive: true, force: true }) (same behavior used in preferNested and
copyModuleIfSymlink) so Windows directory junctions are removed safely before
cpSync(realPath, nestedDependencyPath, { recursive: true }).
In `@apps/desktop/src/main/lib/agent-setup/index.ts`:
- Around line 19-46: The current setupAgentHooks() early-return on Windows
prevents non-shell-dependent runners from running and teardown.ts spawns a
hardcoded /bin/bash on non-mac OS, which breaks on Windows; change
setupAgentHooks() to still call setupDesktopAgentCapabilities() on Windows and
to execute only JSON-only runners from DESKTOP_AGENT_SETUP_RUNNERS (e.g.,
cursor-hooks-json, droid-settings-json, codex-hooks-json, gemini-settings-json,
mastra-hooks-json) while skipping bash/zsh wrapper creation on Windows, and
update teardown.ts to detect Windows (process.platform === "win32") and use an
appropriate shell (e.g., cmd.exe or the same logic as session.ts's
getDefaultShell()) or avoid spawning /bin/bash there so spawn() will not fail on
Windows.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/components/ScriptsEditor/ScriptsEditor.tsx:
- Around line 58-75: The importFirstFile callback's file filter only matches
Unix shells and ignores Windows scripts, so selected .ps1/.cmd/.bat files never
call onChange; update the files.find regex inside importFirstFile to include
Windows extensions (e.g. /\.(sh|bash|zsh|command|ps1|cmd|bat)$/i) to match those
files (and similarly update the other equivalent filter used near the duplicate
callback) so Windows script files picked via the file picker or drag-and-drop
are accepted and their content passed to onChange.
In `@apps/desktop/vite/helpers.ts`:
- Around line 19-27: The defineEnv function currently uses value || fallback
which treats intentional empty-string env values the same as undefined; review
call sites (e.g., SENTRY_DSN_DESKTOP and any uses like SKIP_ENV_VALIDATION that
pass a fallback) and decide if empty string should be preserved — if so, change
the call site to pass undefined when missing or update usage to use
nullish-coalescing (??) or an explicit check for undefined only; ensure
defineEnv or its callers clearly distinguish between undefined (use fallback)
and "" (preserve) for any envs where an intentional empty value is meaningful.
---
Nitpick comments:
In `@apps/desktop/scripts/copy-native-modules.ts`:
- Around line 125-131: Extract the platform-specific deletion logic into a small
helper (e.g., safeRmSync or removePathHandlingWindows) and replace the three
repeated conditionals that check process.platform === "win32" and call
rmSync(modulePath, { recursive: true, force: true }) vs rmSync(modulePath) with
calls to that helper; implement the helper to accept a path (modulePath) and
perform rmSync(path, { recursive: true, force: true }) when process.platform ===
"win32" otherwise rmSync(path) so all current call sites (the block using
rmSync/modulePath and the similar repetition around the other occurrences) use
the same centralized behavior.
In `@apps/desktop/src/main/windows/main.ts`:
- Around line 336-349: The titleBarOverlay color literals are duplicated between
the initial window options (inside PLATFORM.IS_WINDOWS block) and the
nativeTheme update handler; extract a helper (e.g. getTitleBarOverlayColors or
computeTitleBarOverlay) that returns { color, symbolColor, height } based on
nativeTheme.shouldUseDarkColors and use that helper in both the initial creation
(where titleBarOverlay is set) and in the nativeTheme 'updated' handler to
update the overlay, eliminating duplicated literals and keeping behavior tied to
a single source of truth.
In `@apps/desktop/vite/helpers.ts`:
- Around line 78-89: The regex in stripCrossOriginPlugin's
transformIndexHtml.handler only matches double-quoted crossorigin attributes and
can miss single-quoted or unquoted forms leaving stray "=anonymous"; update the
replacement regex to match crossorigin with optional value in double quotes,
single quotes, or unquoted (e.g. pattern like /
crossorigin(?:=(?:"[^"]*"|'[^']*'|[^\s>]*))?/g) so the entire attribute (value
if present) is removed; modify the handler in stripCrossOriginPlugin to use this
broader regex when returning the transformed html.
In `@package.json`:
- Around line 52-55: The node-pty@1.1.0 local patch (referenced in package.json
under patchedDependencies) removes SpectreMitigation attributes from binding.gyp
and deps/winpty/src/winpty.gyp to avoid MSB8040 on Windows; please update the
patch header or the commit message that produced the patch to add a clear note
referencing an upstream report/PR or issue (e.g., mention "upstream node-pty"
and link to the intended issue/PR), and include brief context mentioning MSB8040
and GitHub Actions Spectre-mitigated CRT to aid future maintainers locating
binding.gyp and deps/winpty/src/winpty.gyp changes.
In `@packages/host-service/src/runtime/teardown/teardown.ts`:
- Line 13: TEARDOWN_SCRIPT_REL_PATH is an internal POSIX-only constant but is
exported; either remove its export (make it non-exported) and move the string
into the POSIX_TEARDOWN_CANDIDATES array, or keep it but mark it clearly as a
POSIX-only legacy symbol with a comment and update index.ts to avoid
re-exporting it; update references in the teardown module to use
POSIX_TEARDOWN_CANDIDATES (or the new internal const) and ensure
WINDOWS_TEARDOWN_CANDIDATES logic remains unchanged.
In
`@packages/host-service/src/trpc/router/workspace-creation/shared/setup-terminal.ts`:
- Around line 12-24: The POSIX_CANDIDATES/WINDOWS_CANDIDATES arrays and the
types/functions SetupScriptCandidate, TeardownScriptCandidate, singleQuote, and
doubleQuote are duplicated between setup-terminal.ts and teardown.ts; extract
them into a new shared module (e.g., shared/script-candidates) that exports the
types and the candidate arrays and quoting helpers, centralize Windows-specific
command tweaks (like the teardown-specific `; exit $LASTEXITCODE`) in the shared
candidates or provide a small post-process hook on the buildCommand, then
replace the local declarations in setup-terminal.ts and teardown.ts with imports
of the shared symbols (keeping function names: POSIX_CANDIDATES,
WINDOWS_CANDIDATES, singleQuote, doubleQuote, SetupScriptCandidate,
TeardownScriptCandidate) so future changes need only one update.
🪄 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: CHILL
Plan: Pro
Run ID: 0d3c1c0a-f10e-4ee7-aa8c-0346d32bcab5
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/workflows/build-desktop.ymlapps/desktop/electron-builder.tsapps/desktop/electron.vite.config.tsapps/desktop/scripts/copy-native-modules.tsapps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/lib/window-loader.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/main/lib/play-sound.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/components/ScriptsEditor/ScriptsEditor.tsxapps/desktop/vite/helpers.tspackage.jsonpackages/host-service/src/providers/git/CloudGitCredentialProvider/askpass.tspackages/host-service/src/runtime/teardown/teardown.tspackages/host-service/src/trpc/router/workspace-creation/shared/setup-terminal.tspatches/node-pty@1.1.0.patchplans/20260425-windows-desktop-support.mdscripts/postinstall.mjsscripts/postinstall.sh
💤 Files with no reviewable changes (1)
- scripts/postinstall.sh
| artifact_retention_days: | ||
| description: "Number of days to retain artifacts" | ||
| required: false | ||
| type: number | ||
| default: 14 |
There was a problem hiding this comment.
artifact_retention_days のデフォルト値が workflow_call (30) と workflow_dispatch (14) で食い違っています
意図的であれば問題ありませんが、手動実行と呼び出し実行でリテンション期間が変わるのは混乱のもとなので、揃えるかコメントで理由を明記しておくとよいです。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build-desktop.yml around lines 58 - 62, The default for
the input parameter artifact_retention_days currently differs between the
workflow_call caller (30) and the workflow_dispatch manual trigger (14); update
the workflow to make these consistent or add an inline comment explaining the
intentional difference. Locate the artifact_retention_days declaration and
either change its default to match the other trigger (e.g., set default: 30 or
14) in the .github/workflows/build-desktop.yml or add a clear comment next to
artifact_retention_days referencing workflow_call and workflow_dispatch and
explaining why the retention periods intentionally differ.
| { | ||
| // Windows production loader uses this scheme so ES module dynamic imports | ||
| // (code-split route chunks) work — file:// breaks them on Windows. | ||
| scheme: "superset-app", | ||
| privileges: { | ||
| standard: true, | ||
| secure: true, | ||
| supportFetchAPI: true, | ||
| corsEnabled: true, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# CSP 設定と superset-app オリジン対応の確認
fd -e html -e ts . apps/desktop -x rg -nP -C3 'Content-Security-Policy|connect-src|script-src'Repository: MocA-Love/superset
Length of output: 2785
🏁 Script executed:
# Check if superset-app: is mentioned in any CSP directives
rg -n "superset-app" apps/desktop --type ts --type tsx --type html -C2Repository: MocA-Love/superset
Length of output: 90
🏁 Script executed:
# Look at the full index.html to see complete CSP meta tag
fd -name "index.html" apps/desktop -x cat -n {}Repository: MocA-Love/superset
Length of output: 293
🏁 Script executed:
# Check how the renderer is actually loaded from superset-app origin
rg -n "superset-app://" apps/desktop --type ts --type tsx -C3Repository: MocA-Love/superset
Length of output: 90
🏁 Script executed:
# Check security configuration in main/index.ts to understand the scheme setup better
sed -n '580,650p' apps/desktop/src/main/index.tsRepository: MocA-Love/superset
Length of output: 1749
🏁 Script executed:
# Search for superset-app usage in TypeScript files
rg "superset-app" apps/desktop --type ts -C2Repository: MocA-Love/superset
Length of output: 3646
🏁 Script executed:
# Search for superset-app:// in TypeScript files
rg "superset-app://" apps/desktop --type ts -C3Repository: MocA-Love/superset
Length of output: 2512
🏁 Script executed:
# Find and display the full index.html file
fd "index.html" apps/desktop -x cat {}Repository: MocA-Love/superset
Length of output: 2049
🏁 Script executed:
# Check what schemes are in CSP meta tag and look for superset-app
rg "Content-Security-Policy.*content=" apps/desktop --type ts -o -A1 -B1Repository: MocA-Love/superset
Length of output: 44
🏁 Script executed:
# Find all scheme registrations and their privileges
rg -B2 -A10 'scheme:.*"superset' apps/desktop/src/main/index.tsRepository: MocA-Love/superset
Length of output: 1223
🏁 Script executed:
# Check if there's any CSP bypass or special handling for superset-app
rg -n "bypassCSP|stripExtensionCsp|CSP" apps/desktop/src/main/index.ts -C2Repository: MocA-Love/superset
Length of output: 1091
🏁 Script executed:
# Look at the protocol handler implementation to understand how superset-app serves files
sed -n '450,550p' apps/desktop/src/main/index.tsRepository: MocA-Love/superset
Length of output: 2898
superset-app スキームの CSP 設定を確認してください。
superset-app が bypassCSP: true を設定していない点は意図的な設計と見えますが、他の superset-* スキーム（icon/font/ext-icon/temp-audio/workspace-media）と異なります。superset-app://app/index.html から読み込まれるレンダラでは CSP の 'self' ディレクティブが自動的に superset-app://app オリジンに解決されるため、理論上は script-src と connect-src で許可されます。ただし他のスキームが明示的に media-src に記述されている点を踏まえると、superset-app: を script-src/connect-src に明示的に追加するか、現在の 'self' ベースの設定が意図通り動作していることを確認してください。また、レンダラバンドルの配信が stream: true なしで問題ないか検証してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/index.ts` around lines 601 - 611, The CSP for the
custom scheme declared by scheme: "superset-app" may not be covered by the
current 'self'-based CSP and differs from the other superset-* schemes; update
the code that builds the renderer's Content-Security-Policy to explicitly allow
superset-app: in script-src and connect-src (and media-src if other schemes
include media) or set bypassCSP: true on the scheme declaration if that is the
intended behavior, and verify the renderer bundle delivery still works without
stream: true; locate the scheme declaration (scheme: "superset-app") and the
CSP-generation/registration code that references other superset-* schemes to
make the change and run the renderer to confirm scripts, connects and media load
as expected.
| // Windows production: serve renderer bundle through a custom protocol so | ||
| // ES module dynamic imports work (file:// breaks them on Windows). | ||
| if (PLATFORM.IS_WINDOWS && !IS_DEV) { | ||
| const rendererDir = path.join(__dirname, "../renderer"); | ||
| const appProtocolHandler = (request: Request) => { | ||
| let urlPath = new URL(request.url).pathname; | ||
| if (urlPath.startsWith("/")) urlPath = urlPath.slice(1); | ||
| const filePath = path.join(rendererDir, urlPath); | ||
| return net.fetch(pathToFileURL(filePath).toString()); | ||
| }; | ||
| protocol.handle("superset-app", appProtocolHandler); | ||
| session | ||
| .fromPartition("persist:superset") | ||
| .protocol.handle("superset-app", appProtocolHandler); |
There was a problem hiding this comment.
superset-app ハンドラでパストラバーサル対策が必要です。
urlPath は request.url から取得した値ですが、.. を含むパスが来た場合に path.join(rendererDir, urlPath) が rendererDir の外側のファイル（ユーザーのホームディレクトリ配下の任意ファイルなど）を解決し、net.fetch(file://...) でそのまま返してしまいます。レンダラ側の不具合やサードパーティスクリプトの不正なインポートで簡単にトリガーされ得るため、解決後のパスが rendererDir 配下であることを明示的に検証してください。
🛡️ 提案する修正
- if (PLATFORM.IS_WINDOWS && !IS_DEV) {
- const rendererDir = path.join(__dirname, "../renderer");
- const appProtocolHandler = (request: Request) => {
- let urlPath = new URL(request.url).pathname;
- if (urlPath.startsWith("/")) urlPath = urlPath.slice(1);
- const filePath = path.join(rendererDir, urlPath);
- return net.fetch(pathToFileURL(filePath).toString());
- };
+ if (PLATFORM.IS_WINDOWS && !IS_DEV) {
+ const rendererDir = path.resolve(__dirname, "../renderer");
+ const appProtocolHandler = (request: Request) => {
+ let urlPath = new URL(request.url).pathname;
+ if (urlPath.startsWith("/")) urlPath = urlPath.slice(1);
+ const filePath = path.resolve(rendererDir, urlPath);
+ const rel = path.relative(rendererDir, filePath);
+ if (rel.startsWith("..") || path.isAbsolute(rel)) {
+ return new Response("Not found", { status: 404 });
+ }
+ return net.fetch(pathToFileURL(filePath).toString());
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/index.ts` around lines 780 - 793, The
appProtocolHandler currently builds filePath with path.join(rendererDir,
urlPath) and may allow path traversal via “..”; fix by normalizing and resolving
the requested path (use path.normalize and path.resolve) and verify the resolved
path is inside rendererDir (check resolvedPath === rendererDir or
resolvedPath.startsWith(rendererDir + path.sep)); if the check fails, return a
safe error response (e.g., 403 or a not-found) instead of calling net.fetch on
the resolved path; update the handler referenced as appProtocolHandler (and
where it’s registered with protocol.handle and
session.fromPartition(...).protocol.handle) to perform this validation before
calling pathToFileURL and net.fetch.
| const appSession = session.fromPartition("persist:superset"); | ||
| const apiHost = new URL( | ||
| process.env.NEXT_PUBLIC_API_URL || "https://api.superset.sh", | ||
| ).host; | ||
| const corsTargets = [ | ||
| `https://${apiHost}/*`, | ||
| "https://*.posthog.com/*", | ||
| "https://*.sentry.io/*", | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# mainEnv と process.env.NEXT_PUBLIC_API_URL の使い分けを確認
rg -nP --type=ts -C2 'NEXT_PUBLIC_API_URL' apps/desktop/srcRepository: MocA-Love/superset
Length of output: 11031
🏁 Script executed:
#!/bin/bash
# Check imports at the top of apps/desktop/src/main/index.ts
head -50 apps/desktop/src/main/index.ts | grep -E "import.*mainEnv|from.*env"Repository: MocA-Love/superset
Length of output: 196
🏁 Script executed:
#!/bin/bash
# Get context around lines 798-806 and 851-855
sed -n '795,810p' apps/desktop/src/main/index.ts
echo "---"
sed -n '848,856p' apps/desktop/src/main/index.tsRepository: MocA-Love/superset
Length of output: 973
🏁 Script executed:
#!/bin/bash
# Check if mainEnv is properly exported from env.main.ts
grep -n "export" apps/desktop/src/main/env.main.ts | head -20Repository: MocA-Love/superset
Length of output: 96
NEXT_PUBLIC_API_URL の参照方法を統一してください。
同ファイル内の 853 行では mainEnv.NEXT_PUBLIC_API_URL を使用していますが、800 行では process.env.NEXT_PUBLIC_API_URL を直接参照しています。パッケージ化されたビルドでは process.env が define プラグインによって置き換わらないため、この値が未定義となり、常にフォールバック値 https://api.superset.sh が使用されます。これにより、セルフホスト API 構成での CORS 設定が機能しなくなる可能性があります。mainEnv に統一することで、開発ビルドとプロダクションビルドの両方で正しく環境変数が参照されます。
修正内容
- const apiHost = new URL(
- process.env.NEXT_PUBLIC_API_URL || "https://api.superset.sh",
- ).host;
+ const apiHost = new URL(mainEnv.NEXT_PUBLIC_API_URL).host;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const appSession = session.fromPartition("persist:superset"); | |
| const apiHost = new URL( | |
| process.env.NEXT_PUBLIC_API_URL || "https://api.superset.sh", | |
| ).host; | |
| const corsTargets = [ | |
| `https://${apiHost}/*`, | |
| "https://*.posthog.com/*", | |
| "https://*.sentry.io/*", | |
| ]; | |
| const appSession = session.fromPartition("persist:superset"); | |
| const apiHost = new URL(mainEnv.NEXT_PUBLIC_API_URL).host; | |
| const corsTargets = [ | |
| `https://${apiHost}/*`, | |
| "https://*.posthog.com/*", | |
| "https://*.sentry.io/*", | |
| ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/index.ts` around lines 798 - 806, appSession の CORS 設定
uses process.env.NEXT_PUBLIC_API_URL when computing apiHost which differs from
the rest of the file; replace the direct process.env reference with
mainEnv.NEXT_PUBLIC_API_URL so the packaged build reads the correct value.
Update the apiHost computation (and any use in corsTargets) to source
NEXT_PUBLIC_API_URL from mainEnv, keeping the same fallback
("https://api.superset.sh") and ensuring references to apiHost, corsTargets and
appSession remain unchanged.
| if (process.platform === "win32") { | ||
| if (volume === 0) { | ||
| callbacks?.onComplete?.(); | ||
| return null; | ||
| } | ||
| // PowerShell arguments are single-quoted to avoid shell injection; any | ||
| // single quote in the path is escaped per PowerShell conventions. | ||
| const escapedPath = soundPath.replace(/'/g, "''"); | ||
| const isWav = /\.wav$/i.test(soundPath); | ||
| const script = isWav | ||
| ? `$p = New-Object Media.SoundPlayer '${escapedPath}'; $p.PlaySync()` | ||
| : `Add-Type -AssemblyName presentationCore; $p = New-Object System.Windows.Media.MediaPlayer; $p.Open([System.Uri]::new('${escapedPath}')); $p.Volume = ${volumeDecimal}; $p.Play(); Start-Sleep -Milliseconds 500; while ($p.NaturalDuration.HasTimeSpan -and $p.Position -lt $p.NaturalDuration.TimeSpan) { Start-Sleep -Milliseconds 200 }`; | ||
| return execFile( | ||
| "powershell.exe", | ||
| ["-NoProfile", "-NonInteractive", "-Command", script], | ||
| { windowsHide: true }, | ||
| () => callbacks?.onComplete?.(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the play-sound.ts file
find . -path "*/apps/desktop/src/main/lib/play-sound.ts" -type fRepository: MocA-Love/superset
Length of output: 104
🏁 Script executed:
# Get the full file content
cat -n apps/desktop/src/main/lib/play-sound.tsRepository: MocA-Love/superset
Length of output: 3389
🏁 Script executed:
# Search for callback usage patterns to understand the callback structure
rg -A 5 -B 5 "callbacks\?" apps/desktop --type ts --type tsx | head -100Repository: MocA-Love/superset
Length of output: 90
🏁 Script executed:
# Check for any Linux-specific implementation or other platform handling
rg -A 10 "process.platform" apps/desktop/src/main/lib/play-sound.tsRepository: MocA-Love/superset
Length of output: 662
🏁 Script executed:
# Search for playSoundFile calls to understand how callbacks are used
rg -B 5 -A 10 "playSoundFile" apps/desktop --type ts | head -150Repository: MocA-Love/superset
Length of output: 8716
🏁 Script executed:
# Look for any tests or usage of isCanceled callback
rg "isCanceled" apps/desktop --type ts -B 3 -A 3Repository: MocA-Love/superset
Length of output: 1507
🏁 Script executed:
# Check for onProcessChange usage
rg "onProcessChange" apps/desktop --type ts -B 3 -A 3Repository: MocA-Love/superset
Length of output: 1509
Windows の isCanceled コールバック処理が欠落しており、500ms の固定待機で再生待機ループが途中終了する可能性があります。
Linux 分岐では callbacks?.isCanceled?.() をチェックして呼び出し側の中止要求に応じていますが、Windows 分岐では全く参照されていません。呼び出し側（ringtone ルーター）は中止時に currentSession?.id !== sessionId として isCanceled を渡しており、プラットフォーム間で挙動が一致していません。
また、MediaPlayer.Open() は非同期ですが、Start-Sleep -Milliseconds 500 では大容量ファイルやネットワークファイルで不十分な場合があります。その後の while 条件 $p.NaturalDuration.HasTimeSpan -and $p.Position -lt $p.NaturalDuration.TimeSpan が即座に false を返し、再生完了を待たずに終了するケースが起こり得ます。
🛠 提案される修正: HasTimeSpan 確認待機と isCanceled チェック追加
- const script = isWav
- ? `$p = New-Object Media.SoundPlayer '${escapedPath}'; $p.PlaySync()`
- : `Add-Type -AssemblyName presentationCore; $p = New-Object System.Windows.Media.MediaPlayer; $p.Open([System.Uri]::new('${escapedPath}')); $p.Volume = ${volumeDecimal}; $p.Play(); Start-Sleep -Milliseconds 500; while ($p.NaturalDuration.HasTimeSpan -and $p.Position -lt $p.NaturalDuration.TimeSpan) { Start-Sleep -Milliseconds 200 }`;
+ const script = isWav
+ ? `$p = New-Object Media.SoundPlayer '${escapedPath}'; $p.PlaySync()`
+ : `Add-Type -AssemblyName presentationCore; $p = New-Object System.Windows.Media.MediaPlayer; $p.Open([System.Uri]::new('${escapedPath}')); $p.Volume = ${volumeDecimal}; $p.Play(); $deadline = (Get-Date).AddSeconds(10); while (-not $p.NaturalDuration.HasTimeSpan -and (Get-Date) -lt $deadline) { Start-Sleep -Milliseconds 50 }; while ($p.NaturalDuration.HasTimeSpan -and $p.Position -lt $p.NaturalDuration.TimeSpan) { Start-Sleep -Milliseconds 200 }`;加えて、Windows 分岐でも isCanceled チェックを実装し、Linux 分岐と同等の中止処理をサポートしてください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/play-sound.ts` around lines 39 - 57, The Windows
branch in play-sound.ts must mirror Linux's cancellation behavior and robustly
wait for MediaPlayer to load: update the PowerShell script (the string assigned
to script used in execFile) to (1) poll until $p.NaturalDuration.HasTimeSpan
becomes true with a reasonable timeout instead of a single Start-Sleep 500ms,
(2) inside the waiting and playback loop periodically invoke a callback signal
check (expose callbacks?.isCanceled via a mechanism such as writing a sentinel
file, exit code, or using PowerShell argument/state) so playback stops when
callbacks?.isCanceled?() is true, and (3) ensure callbacks?.onComplete?.() is
called only when playback finishes normally; also make sure the WAV/MediaPlayer
branches both perform the same cancellation checks and finalization. Locate and
modify the script construction and execFile invocation around the isWav, script,
and execFile(...) symbols to implement these changes.
| if (PLATFORM.IS_WINDOWS) { | ||
| window.webContents.on( | ||
| "console-message", | ||
| (_event, level, message, line, sourceId) => { | ||
| if (level < 2) return; | ||
| const levelStr = | ||
| ["verbose", "info", "warning", "error"][level] ?? "unknown"; | ||
| const source = sourceId ? ` (${sourceId}:${line})` : ""; | ||
| const formatted = `[renderer:${levelStr}] ${message}${source}`; | ||
| if (level === 3) console.error(formatted); | ||
| else console.warn(formatted); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Electron のバージョンを確認
fd -e json package.json -x rg -nP -C1 '"electron":\s*"' {}Repository: MocA-Love/superset
Length of output: 157
🌐 Web query:
Electron console-message event signature changed deprecation version
💡 Result:
The 'console-message' event signature on WebContents was changed in Electron v34 (released January 14, 2025). The separate arguments (level, message, line, sourceId) after the event parameter are now deprecated. Properties are moved to the event object itself, with additions like 'frame' and 'lineNumber' (note: 'line' became 'lineNumber'). A deprecation warning is emitted when listeners expect more than one argument. The old signature still works but is planned for removal, currently listed under Planned Breaking API Changes (35.0). Old (deprecated): webContents.on('console-message', (event, level, message, line, sourceId) => {}) New: webContents.on('console-message', (event) => { /* use event.level, event.message, etc. */ }) Additionally, level is now a string ('info', 'warning', 'error', 'debug').
Citations:
- 1: feat: expose frame & move properties to console-message event object electron/electron#43617
- 2: https://electronjs.org/docs/latest/breaking-changes
- 3: http://www.electronproject.org/breaking-changes.html
- 4: https://github.com/electron/electron/blob/main/docs/breaking-changes.md
- 5: https://github.com/electron/electron/releases/tag/v34.0.0
- 6: electron/electron@c868963
Electron 40.8.5 では非推奨の console-message イベントシグネチャを使用しています。
Electron v34（2025年1月リリース）以降、webContents の console-message イベントシグネチャが変更され、旧形式の (event, level, message, line, sourceId) は非推奨化されました。プロジェクトが使用する Electron 40.8.5 では既に古いシグネチャです。新形式への更新が必要です。
新形式では level は数値ではなく文字列 "info" | "warning" | "error" | "debug" となり、プロパティは event オブジェクトに含まれます（line は lineNumber に変更）。
♻️ 修正例
- window.webContents.on(
- "console-message",
- (_event, level, message, line, sourceId) => {
- if (level < 2) return;
- const levelStr =
- ["verbose", "info", "warning", "error"][level] ?? "unknown";
- const source = sourceId ? ` (${sourceId}:${line})` : "";
- const formatted = `[renderer:${levelStr}] ${message}${source}`;
- if (level === 3) console.error(formatted);
- else console.warn(formatted);
- },
- );
+ window.webContents.on("console-message", (event) => {
+ const { level, message, lineNumber, sourceId } = event;
+ if (level !== "warning" && level !== "error") return;
+ const source = sourceId ? ` (${sourceId}:${lineNumber})` : "";
+ const formatted = `[renderer:${level}] ${message}${source}`;
+ if (level === "error") console.error(formatted);
+ else console.warn(formatted);
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/windows/main.ts` around lines 372 - 384, The
console-message handler uses the old signature; update the listener on
window.webContents (the call to window.webContents.on("console-message", ...))
to accept the new payload shape (e.g., (event, details) where details.level is a
string like "info"|"warning"|"error"|"debug" and line is details.lineNumber),
replace numeric-level checks (level < 2 / level === 3) with string-based logic
(map or switch details.level to "verbose"/"info"/"warning"/"error", treat
"debug" as "verbose"), build the source using details.sourceId and
details.lineNumber, and log using console.error for "error" and console.warn for
"warning"/others so behavior matches the original intent.
| // Keep the title-bar overlay contrast aligned with the OS theme — it is | ||
| // a Windows-only API so the call is safely gated. | ||
| nativeTheme.on("updated", () => { | ||
| if (window.isDestroyed()) return; | ||
| window.setTitleBarOverlay?.({ | ||
| color: nativeTheme.shouldUseDarkColors ? "#1e1e1e" : "#ffffff", | ||
| symbolColor: nativeTheme.shouldUseDarkColors ? "#ffffff" : "#000000", | ||
| height: 35, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
nativeTheme.on("updated", ...) が window ごとに追加され、クリーンアップされません。
MainWindow() は再オープン時に再度呼び出され得るため、呼び出しのたびに新しい nativeTheme リスナが増え、古いウィンドウは isDestroyed() ガードで早期 return するだけで listener 自体は残留します。時間が経つとイベント配列が膨らみ、EventEmitter の警告（MaxListenersExceededWarning）にもつながります。
また、本ファイルでは既にモジュールトップで nativeTheme.on("updated", ...) リスナを登録しており（185-191 行）、そこと同じようにウィンドウ全体を走査する方式に寄せるか、最低でも window.once("closed", ...) で off する必要があります。
🛡️ 提案する修正
- // Keep the title-bar overlay contrast aligned with the OS theme — it is
- // a Windows-only API so the call is safely gated.
- nativeTheme.on("updated", () => {
- if (window.isDestroyed()) return;
- window.setTitleBarOverlay?.({
- color: nativeTheme.shouldUseDarkColors ? "#1e1e1e" : "#ffffff",
- symbolColor: nativeTheme.shouldUseDarkColors ? "#ffffff" : "#000000",
- height: 35,
- });
- });
+ const onThemeUpdated = () => {
+ if (window.isDestroyed()) return;
+ window.setTitleBarOverlay?.({
+ color: nativeTheme.shouldUseDarkColors ? "#1e1e1e" : "#ffffff",
+ symbolColor: nativeTheme.shouldUseDarkColors ? "#ffffff" : "#000000",
+ height: 35,
+ });
+ };
+ nativeTheme.on("updated", onThemeUpdated);
+ window.once("closed", () => nativeTheme.off("updated", onThemeUpdated));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Keep the title-bar overlay contrast aligned with the OS theme — it is | |
| // a Windows-only API so the call is safely gated. | |
| nativeTheme.on("updated", () => { | |
| if (window.isDestroyed()) return; | |
| window.setTitleBarOverlay?.({ | |
| color: nativeTheme.shouldUseDarkColors ? "#1e1e1e" : "#ffffff", | |
| symbolColor: nativeTheme.shouldUseDarkColors ? "#ffffff" : "#000000", | |
| height: 35, | |
| }); | |
| }); | |
| const onThemeUpdated = () => { | |
| if (window.isDestroyed()) return; | |
| window.setTitleBarOverlay?.({ | |
| color: nativeTheme.shouldUseDarkColors ? "#1e1e1e" : "#ffffff", | |
| symbolColor: nativeTheme.shouldUseDarkColors ? "#ffffff" : "#000000", | |
| height: 35, | |
| }); | |
| }; | |
| nativeTheme.on("updated", onThemeUpdated); | |
| window.once("closed", () => nativeTheme.off("updated", onThemeUpdated)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/windows/main.ts` around lines 386 - 395, The
nativeTheme.on("updated", ...) listener is being added inside MainWindow() for
each new window and never removed, causing listener leaks and potential
MaxListenersExceededWarning; instead, either register a single module-level
nativeTheme.on("updated", ...) that iterates all BrowserWindow instances and
updates title bars (like the existing top-level listener at lines ~185-191) or,
if keeping the per-window handler, register the listener and remove it when the
window closes by calling nativeTheme.off inside window.once("closed", ...) (use
the same callback reference tied to window.setTitleBarOverlay updates and guard
with window.isDestroyed() as already done).
| /** Escape characters that would break `echo` or `findstr` in cmd.exe. */ | ||
| function escapeCmdEcho(value: string): string { | ||
| return value.replace(/([&<>^|%])/g, "^$1"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In a Windows .cmd batch file, how do you escape a literal %character in anechostatement — is it%%or^%?
💡 Result:
In a Windows .cmd batch file, you escape a literal percent sign in an echo statement by doubling it: %% (not ^%). Example: echo %% This prints a single %. If your echo (or the surrounding code) is inside a parenthesized block, or you’re dealing with other parsing phases (e.g., CALL, FOR), you may need additional escaping because ) and other characters can end a block early; but the percent character itself is still produced via %% in the batch-file parsing rules.
Citations:
- 1: https://www.robvanderwoude.com/escapechars.php
- 2: https://stackoverflow.com/questions/32150412/batch-echo-print-an-percent-sign/32150522
- 3: https://stackoverflow.com/questions/13709434/how-to-escape-percent-when-using-parameters-in-a-for-loop
- 4: https://stackoverflow.com/questions/44182247/how-to-echo-symbols-like-in-a-batch-command-processor
.cmd バッチファイルでは % を ^^% ではなく %% にエスケープする必要があります
batch ファイル内で % のリテラル出力は %% でエスケープします。^% は &/</>/|/^ には有効ですが、% には機能しません。現在のコードは ^% を使用しており、これは正確ではありません。
修正
/** Escape characters that would break `echo` or `findstr` in cmd.exe. */
function escapeCmdEcho(value: string): string {
- return value.replace(/([&<>^|%])/g, "^$1");
+ // In .cmd files, `%` must be doubled (`%%`); the caret escape does NOT work.
+ // Other specials (&, <, >, |, ^) are caret-escaped.
+ return value.replace(/%/g, "%%").replace(/([&<>^|])/g, "^$1");
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/host-service/src/providers/git/CloudGitCredentialProvider/askpass.ts`
around lines 29 - 32, The escape function escapeCmdEcho currently prefixes all
matched chars (including '%') with '^', but in Windows batch files '%' must be
escaped as '%%' not '^%'; modify escapeCmdEcho so the regex still matches
/([&<>^|%])/g but use a replacer function that returns '%%' when the matched
character is '%' and '^' + ch for all other matched characters, preserving
correct escaping for cmd.exe echo/findstr.
| const WINDOWS_TEARDOWN_CANDIDATES: TeardownScriptCandidate[] = [ | ||
| { | ||
| relativePath: ".superset/teardown.ps1", | ||
| buildCommand: (p) => | ||
| `powershell.exe -NoProfile -ExecutionPolicy Bypass -File ${doubleQuote(p)}`, | ||
| }, | ||
| { | ||
| relativePath: ".superset/teardown.cmd", | ||
| buildCommand: (p) => `cmd.exe /c ${doubleQuote(p)}`, | ||
| }, | ||
| { | ||
| relativePath: ".superset/teardown.bat", | ||
| buildCommand: (p) => `cmd.exe /c ${doubleQuote(p)}`, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Windows での既定 PTY シェルと initialCommand 処理を確認
rg -nP --type=ts -C5 'createTerminalSessionInternal|defaultShell|win32' packages/host-service/src/terminalRepository: MocA-Love/superset
Length of output: 4325
🏁 Script executed:
# Read the teardown.ts file to understand the full context and implementation
cat -n packages/host-service/src/runtime/teardown/teardown.ts | head -100Repository: MocA-Love/superset
Length of output: 3614
🏁 Script executed:
# Check for POSIX implementation to compare patterns
rg -nP --type=ts 'POSIX_TEARDOWN_CANDIDATES|bash.*exit' packages/host-service/src/runtime/teardown/Repository: MocA-Love/superset
Length of output: 390
🏁 Script executed:
# Understand how buildCommand is used and executed
rg -nP --type=ts -A10 'buildCommand' packages/host-service/src/runtime/teardown/Repository: MocA-Love/superset
Length of output: 5131
🏁 Script executed:
# Check how initialCommand is processed in createTerminalSessionInternal
rg -nP --type=ts -A20 'initialCommand' packages/host-service/src/terminal/terminal.ts | head -100Repository: MocA-Love/superset
Length of output: 1507
🏁 Script executed:
# Search for onExit handler and exit code propagation patterns
rg -nP --type=ts 'onExit|shellReadyPromise|session.pty|exited' packages/host-service/src/terminal/terminal.ts | head -60Repository: MocA-Love/superset
Length of output: 742
🏁 Script executed:
# Check how subprocess exit codes are handled in PTY environments
rg -nP --type=ts 'exitCode|ERRORLEVEL|pty.*exit' packages/host-service/src/Repository: MocA-Love/superset
Length of output: 1635
🏁 Script executed:
# Understand the PTY creation and shell invocation
rg -nP --type=ts -B5 -A10 'new Pty\|createPty\|spawnPty' packages/host-service/src/terminal/terminal.ts | head -80Repository: MocA-Love/superset
Length of output: 44
🏁 Script executed:
# Check for more context on how the terminal session handles the PTY exit
rg -nP --type=ts -B5 -A15 'session.pty.onExit' packages/host-service/src/runtime/teardown/teardown.tsRepository: MocA-Love/superset
Length of output: 576
🏁 Script executed:
# Verify the shell type conditional logic and see if parent shell type is determined
rg -nP --type=ts -B10 'createTerminalSessionInternal' packages/host-service/src/runtime/teardown/teardown.ts | tail -30Repository: MocA-Love/superset
Length of output: 715
Windows のテアダウンコマンドに親シェルの終了伝播が欠落しており、タイムアウトまで待機する可能性があります。
POSIX 側は bash '{path}' ; exit $? と明示的に親シェルを終了させ、session.pty.onExit が迅速に発火する構造になっていますが、Windows 側は powershell.exe -File ... / cmd.exe /c ... の呼び出しのみで、親の対話シェル（PTY 上の cmd.exe）は残ったままになります。その結果 onExit が発火せず、タイムアウト（TEARDOWN_TIMEOUT_MS）まで常に待たされます。
提案する修正：
Windows の既定親シェルが cmd.exe であるため、exit コマンドは cmd.exe 構文を使用する必要があります。提案された PowerShell の ; 構文は cmd.exe 下では無効です。
修正内容
const WINDOWS_TEARDOWN_CANDIDATES: TeardownScriptCandidate[] = [
{
relativePath: ".superset/teardown.ps1",
buildCommand: (p) =>
- `powershell.exe -NoProfile -ExecutionPolicy Bypass -File ${doubleQuote(p)}`,
+ `powershell.exe -NoProfile -ExecutionPolicy Bypass -File ${doubleQuote(p)} & exit !ERRORLEVEL!`,
},
{
relativePath: ".superset/teardown.cmd",
- buildCommand: (p) => `cmd.exe /c ${doubleQuote(p)}`,
+ buildCommand: (p) => `cmd.exe /c ${doubleQuote(p)} & exit !ERRORLEVEL!`,
},
{
relativePath: ".superset/teardown.bat",
- buildCommand: (p) => `cmd.exe /c ${doubleQuote(p)}`,
+ buildCommand: (p) => `cmd.exe /c ${doubleQuote(p)} & exit !ERRORLEVEL!`,
},
];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/runtime/teardown/teardown.ts` around lines 27 - 41,
The Windows teardown commands in WINDOWS_TEARDOWN_CANDIDATES leave the parent
PTY shell running so session.pty.onExit never fires; change the buildCommand
functions so the invoked command ensures the parent cmd.exe exits with the
child's status by appending a cmd-style exit that propagates the child exit code
(use %ERRORLEVEL%), ensuring proper quoting around doubleQuote(p) and correct
placement for both the PowerShell and cmd/bat candidates; update the
buildCommand closures for the PowerShell entry (and the cmd/bat entries if
needed) to wrap the invocation so the parent shell runs an explicit "exit" with
%ERRORLEVEL% after the script finishes.
| diff --git a/binding.gyp b/binding.gyp | ||
| index 5f63978b07ab50aaf7523219a2170ec737a6b5db..5944abc6d19216b9f7daf83ce4eea59e430f4fd1 100644 | ||
| --- a/binding.gyp | ||
| +++ b/binding.gyp | ||
| @@ -5,9 +5,10 @@ | ||
| ], | ||
| 'conditions': [ | ||
| ['OS=="win"', { | ||
| - 'msvs_configuration_attributes': { | ||
| - 'SpectreMitigation': 'Spectre' | ||
| - }, | ||
| + # SpectreMitigation removed: VS Build Tools default workload on GitHub | ||
| + # Actions windows-latest does not ship the Spectre-mitigated CRT, which | ||
| + # causes MSB8040 when /Qspectre is requested. Dropping the attribute | ||
| + # is consistent with the rest of the Node.js/Electron native ecosystem. | ||
| 'msvs_settings': { | ||
| 'VCCLCompilerTool': { | ||
| 'AdditionalOptions': [ | ||
| diff --git a/deps/winpty/src/winpty.gyp b/deps/winpty/src/winpty.gyp | ||
| index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..5dba57022de0a5d283aade6d718c6d5cfce7dc90 100644 | ||
| --- a/deps/winpty/src/winpty.gyp | ||
| +++ b/deps/winpty/src/winpty.gyp | ||
| @@ -40,9 +40,8 @@ | ||
| '-lshell32', | ||
| '-luser32', | ||
| ], | ||
| - 'msvs_configuration_attributes': { | ||
| - 'SpectreMitigation': 'Spectre' | ||
| - }, | ||
| + # SpectreMitigation dropped — GitHub Actions windows-latest VS Build | ||
| + # Tools lack the Spectre-mitigated CRT, producing MSB8040. | ||
| 'msvs_settings': { | ||
| # Specify this setting here to override a setting from somewhere | ||
| # else, such as node's common.gypi. | ||
| @@ -142,9 +141,7 @@ | ||
| '-ladvapi32', | ||
| '-luser32', | ||
| ], | ||
| - 'msvs_configuration_attributes': { | ||
| - 'SpectreMitigation': 'Spectre' | ||
| - }, | ||
| + # SpectreMitigation dropped — see note in the winpty-agent target above. | ||
| 'msvs_settings': { | ||
| # Specify this setting here to override a setting from somewhere | ||
| # else, such as node's common.gypi. |
There was a problem hiding this comment.
Spectre 緩和を無効化する前に、CI 側で Spectre-mitigated CRT をインストールする選択肢を検討してください。
本パッチはすべての Windows ビルド（CI だけでなくローカル開発も含む）で node-pty と winpty の Spectre 緩和をオフにします。node-pty は子プロセスの I/O を中継する性質上、Spectre 緩和が無くとも直ちに脆弱になるわけではありませんが、ビルド成果物のセキュリティ姿勢は後退します。
windows-latest ランナーでも Visual Studio Installer CLI (vs_installer.exe もしくは Setup-VsDevCmd の --add Microsoft.VisualStudio.Component.VC.Runtimes.x86.x64.Spectre) を使って Spectre-mitigated CRT を明示インストールすれば MSB8040 を回避でき、アップストリームの挙動を維持できます。フォローアップとして CI ステップ化しつつ、パッチは一時的措置として残すのが望ましいです。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@patches/node-pty`@1.1.0.patch around lines 1 - 45, The patch removes
'msvs_configuration_attributes' 'SpectreMitigation' entries from binding.gyp and
deps/winpty/src/winpty.gyp, turning off Spectre mitigation for all Windows
builds; instead, update CI to install the Spectre-mitigated CRT so you can keep
the SpectreMitigation settings: add a CI step that runs the Visual Studio
installer CLI (e.g., vs_installer.exe or Setup-VsDevCmd) to install the
Microsoft.VisualStudio.Component.VC.Runtimes.x86.x64.Spectre component before
building, revert or avoid removing the 'SpectreMitigation' entries in
binding.gyp and winpty.gyp (msvs_configuration_attributes with
'SpectreMitigation': 'Spectre') once the runner provides the CRT, and treat the
current patch as temporary until CI is updated.
Windows 上で agent lifecycle hook (notify / copilot / cursor / gemini /
codex) を実際に動作させるための PowerShell 移植。Claude / Codex / Cursor /
Gemini は global hooks.json / settings.json 経由で hook が走るようになり、
Superset 通知 (PermissionRequest / Start / Stop) がデスクトップ側に届く。
## 追加 template (templates/*.template.ps1)
- notify-hook.template.ps1 — 共通 notify 実装 (Claude pipes stdin、Codex は
argument passing)
- copilot-hook.template.ps1 — GitHub Copilot CLI の session/prompt/tool
イベント用
- cursor-hook.template.ps1 — cursor-agent hook (PermissionRequest は
`{"continue":true}` を返却)
- gemini-hook.template.ps1 — Gemini CLI hook (BeforeAgent/AfterAgent/AfterTool)
- codex-wrapper-exec.template.ps1 — codex の session-log watcher (fallback)
全テンプレートは bash 版と機能等価を目指し、HTTP は Invoke-WebRequest、
JSON 抽出は [regex]、stdin drain は [Console]::In.ReadToEnd() を使用。
sleep-inhibitor snippet は Windows では空文字列に置換 (main process 側の
powerSaveBlocker に任せる follow-up)。
## agent-wrappers-common.ts
- IS_WIN_AGENT / hookScriptExtension() / hookTemplateExtension() /
buildHookCommand() / getSleepInhibitorSnippet() を追加
- createWrapper() は Windows で early return (bash の #!/bin/bash wrapper
は generate しない)
## 各 agent wrapper
- copilot / cursor / gemini / claude-codex-opencode:
- HOOK_SCRIPT_NAME / TEMPLATE_PATH を hookScriptExtension() 経由で .ps1 に
- hooks.json / settings.json 側の command を buildHookCommand() で
`powershell.exe -NoProfile -ExecutionPolicy Bypass -File <path>` に
- Copilot hooks.json は `bash` key を Windows では `powershell` key に切替
- Claude managed hook command も PowerShell 版を追加 (SUPERSET_HOME_DIR
経由で notify.ps1 を dispatch)
- createCodexWrapper() は Windows で early return (wrapper 本体は bash
専用、hook 経由で代替)
- notify-hook.ts: NOTIFY_SCRIPT_NAME を platform-aware に
(notify.sh / notify.ps1)
## agent-setup/index.ts
- 先行 PR では Windows 全スキップしていたガードを解除
- HOOKS_DIR と OPENCODE_PLUGIN_DIR は常に作成
- BIN_DIR / ZSH_DIR / BASH_DIR は Unix のみ
- createZshWrapper / createBashWrapper は Unix のみ
- setupDesktopAgentCapabilities() は呼ぶ (各 Wrapper 系は createWrapper()
の Windows guard で自動 skip、hook 系は Windows でも動く)
## 動作範囲
Windows ユーザーが実際に hook を受け取る agent:
- Claude Code (~/.claude/settings.json)
- Codex (~/.codex/hooks.json)
- cursor-agent (~/.cursor/hooks.json)
- Gemini CLI (~/.gemini/settings.json)
Copilot CLI は project-level (.github/hooks/superset-notify.json) が
bash wrapper 経由でしか自動生成されないため、Windows では手動での
hooks.json 配置が必要 (follow-up)。
Refs: #273
Windows で agent 実行中にスリープを抑制し、Mica によるウィンドウ装飾を
ネイティブ対応させる。README に Windows preview の既知制限と手元ビルド
手順を追記。
## Sleep inhibitor (Windows)
macOS / Linux は wrapper 内で caffeinate / systemd-inhibit を起動するが、
Windows は wrapper を shipping していないため、main process 側で Electron
の powerSaveBlocker (prevent-app-suspension) を起動/停止する。
- shared/constants.ts: SUPPORTS_AGENT_SLEEP_PREVENTION に PLATFORM.IS_WINDOWS を追加。
- main/lib/agent-sleep/windows-sleep-blocker.ts (新規):
- agent lifecycle イベント (Start / PermissionRequest / Stop) に応じて
active set を管理。空なら blocker を停止、入ったら開始。
- ユーザー設定 (preventAgentSleep) が off の時は何もしない + 保持中の
blocker を解放。
- terminal exit でも対象 pane の entry を clean up。
- main/windows/main.ts: notificationsEmitter の agent-lifecycle /
terminal-exit listener から windows-sleep-blocker を呼び出す。
display-sleep は抑制しない (prevent-app-suspension のみ) — 画面を点けたまま
にするのはユーザー期待と合わないため。
## Mica fallback (Windows 11)
- main/lib/vibrancy/index.ts:
- isVibrancySupported() に IS_WINDOWS を追加。
- applyVibrancy() に Windows 分岐。BrowserWindow.setBackgroundMaterial
('mica' | 'none') を try/catch でガード (Windows 10 や 22H2 前では
silent no-op)。
- getInitialWindowOptions() の戻り型に backgroundMaterial を追加。
Windows 作成時から mica を指定してちらつきを抑える。
- isNativeContinuousBlurSupported() は macOS 限定 (Windows Mica は
on/off のみで radius 無し)。
## README
- L185 の OS 行を `macOS / Windows x64 (preview, this fork) / Linux
(untested)` に更新。
- Quick Start に MocA-Love/superset の Windows preview リンクを追加。
- Windows preview の known limitations (wrapper 未ポート、Copilot プロジェクト
hooks、Mica の Windows 11 22H2+ 要件、auto-install-on-quit、dev-mode
deep-link) を箇条書きで明示。
- ローカルでの Windows ビルド手順 (core.longpaths、bun install --frozen
--ignore-scripts、SUPERSET_WORKSPACE_NAME、電子 builder --win) を追記。
Refs: #273
# Conflicts: # apps/desktop/src/main/index.ts
…buildHookCommand
PR の buildHookCommand が全 platform で args を quote するよう変更されたのに、
Cursor hooks.json のテストだけ旧の unquoted `${path} Start` 期待値が残っていた。
空白を含む path で安全になる新しい動作に合わせて `"Start"` を期待するよう修正。
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/main/lib/agent-setup/agent-wrappers-copilot.ts (1)
49-78:⚠️ Potential issue | 🟠 MajorWindows での PowerShell 二重起動を回避する必要があります
buildHookCommand()が返すpowershell.exe -NoProfile -ExecutionPolicy Bypass -File "..." "eventName"を hooks.json の"powershell"キーに直接渡すと、Copilot CLI が この値を PowerShell で実行する際に PowerShell が二重に起動します。GitHub Copilot CLI の公式スキーマで
"powershell"キーは確実にサポートされていますが、その値はスクリプト参照またはスクリプト文字列である想定です。Windows 分岐ではbuildHookCommand()を使わず、PowerShell の呼び出し演算子&を使った形式に変更してください：& "${hookScriptPath}" "${event}"これにより、Copilot CLI が PowerShell で実行する際に単一の起動で済みます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-copilot.ts` around lines 49 - 78, The Windows branch is passing the full PowerShell invocation from buildHookCommand into the "powershell" key which causes PowerShell to be launched twice; update getCopilotHooksJsonContent so when IS_WIN_AGENT is true you do not call buildHookCommand for the Windows path but instead construct a command string using the PowerShell call operator and the script path/event (e.g. & "${hookScriptPath}" "${event}") — change the cmd helper to return that string on Windows while keeping buildHookCommand for non-Windows, and ensure the hooks object uses that adjusted cmd for the "powershell" entries so Copilot CLI receives a script reference/string rather than a full powershell.exe invocation.
🧹 Nitpick comments (6)
apps/desktop/src/main/lib/vibrancy/index.ts (1)
164-174:setBackgroundMaterialの型アサーションは不要ですElectron 40.8.5 では
BrowserWindow.setBackgroundMaterial()が公開メソッドとして含まれているため、WithSetBackgroundMaterial型アサーションと optional chaining は不要です。直接window.setBackgroundMaterial(...)を呼び出せます（try/catchは引き続き保持することで、ネイティブ層の例外は確実に捕捉できます）。例：
try { window.setBackgroundMaterial(state.enabled ? "mica" : "none"); } catch (error) { console.warn("[vibrancy] setBackgroundMaterial failed on Windows:", error); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/vibrancy/index.ts` around lines 164 - 174, Remove the unnecessary type assertion WithSetBackgroundMaterial and the optional chaining when calling setBackgroundMaterial: call window.setBackgroundMaterial(...) directly instead of casting window to WithSetBackgroundMaterial and using withMaterial.setBackgroundMaterial?.(...); keep the existing try/catch block around the call to ensure native exceptions are caught and log errors as before.apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
12-12:hookScriptExtension()で統一すると他のラッパーと整合します。他のラッパー（
agent-wrappers-gemini.ts、agent-wrappers-copilot.ts、agent-wrappers-cursor.ts）は`xxx-hook.${hookScriptExtension()}`形式で生成していますが、ここだけIS_WIN_AGENTの三項演算で書かれており、将来拡張子を増やしたときの追従漏れが起きやすい書き方です。機能的には等価ですが統一してもよいでしょう。♻️ 提案
-export const NOTIFY_SCRIPT_NAME = IS_WIN_AGENT ? "notify.ps1" : "notify.sh"; +export const NOTIFY_SCRIPT_NAME = `notify.${hookScriptExtension()}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/notify-hook.ts` at line 12, Replace the platform-specific ternary for NOTIFY_SCRIPT_NAME with the common hookScriptExtension() approach used by other wrappers: import or use hookScriptExtension() and set export const NOTIFY_SCRIPT_NAME = `notify-hook.${hookScriptExtension()}` (remove reliance on IS_WIN_AGENT here) so the filename generation is consistent with agent-wrappers-gemini.ts / agent-wrappers-copilot.ts / agent-wrappers-cursor.ts; update any imports if needed and remove the unused IS_WIN_AGENT branch.apps/desktop/src/main/lib/agent-setup/agent-wrappers-common.ts (1)
11-19:hookScriptExtensionとhookTemplateExtensionの実装が同一です。将来的にテンプレート側とスクリプト側で別々の拡張子を持つ設計余地があるという意図は読み取れますが、現状は完全に同じロジックで、呼び出し側が使い分けるメリットが薄くなっています。どちらか一方のヘルパーに集約するか、ドキュメントコメントで分離理由を明記しておくと保守上混乱がありません。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-common.ts` around lines 11 - 19, The two functions hookScriptExtension and hookTemplateExtension have identical implementations; refactor to eliminate duplication by making hookTemplateExtension delegate to hookScriptExtension (i.e., return hookScriptExtension()) or consolidate into a single exported helper used by both callers, keeping IS_WIN_AGENT as the platform check; alternatively, if you intend them to diverge later, add a clear doc comment on hookTemplateExtension explaining why it duplicates hookScriptExtension and under what conditions it should change.apps/desktop/src/main/lib/agent-sleep/windows-sleep-blocker.ts (2)
66-93: 設定 OFF かつactiveに孤児エントリが残るケースの軽微なリークLine 70-77 で
preventSleepSettingEnabled()が false のときにactive.clear()を呼ぶのはblockerId !== nullのときだけです。ensureBlocker()が何らかの例外で blockerId を埋められずactiveだけ積み上がっている状態から設定が OFF に切り替わった場合、active が永続的に残り、将来設定 ON 時にpreventSleepSettingEnabled()チェックは通るものの積み上がった stale キーに対してStopが来ないまま膨らみ続ける可能性があります。実害は小さいですが、設定 OFF 時は常にクリアする形の方が安全です。🛠 提案修正
if (!preventSleepSettingEnabled()) { - // Setting was toggled off mid-flight — drop any inhibitor we were holding. - if (blockerId !== null) { - active.clear(); - releaseBlocker(); - } + // Setting was toggled off mid-flight — drop any inhibitor we were holding. + active.clear(); + releaseBlocker(); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-sleep/windows-sleep-blocker.ts` around lines 66 - 93, The logic in handleAgentLifecycleForWindowsSleep should always clear the active set when preventSleepSettingEnabled() is false to avoid leaving stale entries; change the block that checks preventSleepSettingEnabled() so active.clear() is called unconditionally (regardless of blockerId), and only call releaseBlocker() when blockerId !== null (keeping that conditional), so ensureBlocker() / blockerId race cannot leave orphaned active entries when the setting is toggled off.
37-39: 同一ペインで複数エージェントが動作する場合、buildKeyの粒度が粗いキーが
workspaceId:tabId:paneIdの組のみのため、同じペインで並行して走る 2 つのエージェント A と B がそれぞれ Start した場合でもactiveには 1 件しか登録されません。A が Stop した時点でactive.size === 0となりブロッカーを解除してしまい、B がまだ動作中にスリープが発生し得ます。現状の Superset の構造上 1 ペイン 1 エージェントの想定であれば許容できますが、将来のマルチエージェント並走に備えるなら key にsessionId/hookSessionIdを含めることを検討してください（AgentLifecycleEvent型の拡張が必要）。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-sleep/windows-sleep-blocker.ts` around lines 37 - 39, buildKey のキー粒度が workspaceId:tabId:paneId のみで同一ペイン内の複数エージェントを区別できないため、event の一意識別子（sessionId や hookSessionId）をキーに含めるよう修正してください: AgentLifecycleEvent 型を拡張して sessionId/hookSessionId を追加し、buildKey(event: AgentLifecycleEvent) にこれらを優先して組み込み（例: `${event.workspaceId ?? "-"}:${event.tabId ?? "-"}:${event.paneId ?? "-"}:${event.sessionId ?? event.hookSessionId ?? "-"}` のような構成）し、active マップの登録・削除処理が新しいキーで正しく動作すること（Start/Stop ハンドラの呼び出し元が sessionId を渡すこと）を確認してください。apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1 (1)
10-18:{{NOTIFY_PATH}}が単引用符に挿入されているため、パス中の'が構文エラーを起こすテンプレート置換で
{{NOTIFY_PATH}}が単引用符リテラル内にそのまま埋め込まれています。ユーザ名やワークスペース名に'が含まれていた場合（Windows ではそれなりにあり得ます）、生成後の PowerShell が構文エラーになります。agent-wrappers-claude-codex-opencode.ts:buildCodexWrapperExecLineでreplaceAllする前に'を''にエスケープする、もしくは@'...'@ヒアストリング + エスケープのような防御を入れてください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1` around lines 10 - 18, The template injects {{NOTIFY_PATH}} into a single-quoted PowerShell string which breaks if the path contains a single quote; modify agent-wrappers-claude-codex-opencode.ts:buildCodexWrapperExecLine to escape single quotes in the path before performing replaceAll (convert each ' to ''), or alternatively emit the path using a PowerShell verbatim here-string (e.g. @'... '@) when building the codex wrapper exec line so that embedded single quotes won't create a syntax error; ensure the change targets the replacement logic that produces $notifyPath and $env:CODEX_TUI_SESSION_LOG_PATH in the codex-wrapper-exec.template.ps1 output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-common.ts`:
- Around line 28-39: The non-Windows branch of buildHookCommand does not quote
hookScriptPath, which breaks paths with spaces; update the return for the
non-Windows path so hookScriptPath is wrapped in quotes (use the existing
quotedArgs variable and IS_WIN_AGENT check context) ensuring the final returned
string uses `"<hookScriptPath>"` plus the quotedArgs if present; reference the
buildHookCommand function, the hookScriptPath parameter, and the quotedArgs
local variable when making the change.
In
`@apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1`:
- Around line 91-101: Initialize a safe non-zero default for $codexStatus before
the try block and ensure any failure sets it: set $codexStatus (e.g. to 1) prior
to invoking '& '{{REAL_BIN}}'', add a catch block that assigns $codexStatus from
$LASTEXITCODE (or a non-zero constant) when an exception occurs, keep the
existing finally cleanup for $WatcherJob, and then exit using $codexStatus so
failures in invoking '{{REAL_BIN}}' are not treated as success.
- Around line 24-31: Send-HookEvent builds a JSON payload in $payload but passes
it as a command-line argument to powershell.exe which breaks double-quote
quoting on PowerShell 5.1; instead pipe the payload to the child PowerShell
process so the notify script (which reads [Console]::In.ReadToEnd()) receives it
via stdin. Modify the powershell.exe invocation in Send-HookEvent to remove the
$payload argument and feed $payload into the process's stdin (e.g., "$payload |
powershell.exe -NoProfile -ExecutionPolicy Bypass -File $notifyScript") while
preserving the try/catch and Out-Null behavior.
In `@apps/desktop/src/main/lib/agent-setup/templates/copilot-hook.template.ps1`:
- Around line 1-55: The PowerShell hook template (copilot-hook.template.ps1)
contains non-ASCII characters in comments but is emitted without a UTF-8 BOM,
causing Windows PowerShell 5.1 (invoked by buildHookCommand -> powershell.exe)
to treat it as Windows-1252; fix by ensuring the build step that writes/copied
this template outputs the .ps1 with a UTF-8 BOM (or alternatively remove/replace
all non-ASCII characters in the template), i.e., update the code path that emits
copilot-hook.template.ps1 so it writes with BOM-enabled UTF8 encoding before
packaging.
In `@apps/desktop/src/main/lib/agent-setup/templates/gemini-hook.template.ps1`:
- Around line 17-18: The script assigns to the PowerShell automatic variable
$Input which is unsafe; replace that assignment with a new local variable (e.g.
$rawInput or $stdinData) and use it when extracting the JSON field. Update the
statement that calls Get-JsonStringValue (currently using $Input) to use the new
local variable (e.g. change the $GeminiEvent assignment to call
Get-JsonStringValue with $rawInput) so you avoid clobbering the automatic $Input
pipeline variable.
In `@apps/desktop/src/main/lib/vibrancy/index.ts`:
- Around line 159-178: The Windows Mica is hidden because
applyWindowsBackgroundMaterial (function applyWindowsBackgroundMaterial) always
calls window.setBackgroundColor with an opaque color; change it to set the color
to transparent ("#00000000") when state.enabled is true and keep the opaque
OPAQUE_DARK/OPAQUE_LIGHT when false (i.e., set background color conditionally
based on state.enabled). Do the same change in getInitialWindowOptions' Windows
branch where backgroundMaterial/backgroundColor are returned (ensure
backgroundColor is transparent when state.enabled and opaque otherwise). Also
update the misleading comment in applyWindowsBackgroundMaterial to not claim
renderer transparency enables Mica.
In `@README.md`:
- Line 220: Update the README entry that currently uses the invalid shorthand
flag by replacing the command string "bun install --frozen --ignore-scripts"
with the correct flag name so it reads "bun install --frozen-lockfile
--ignore-scripts"; locate the occurrence of the exact command text in README.md
and change only the flag name to --frozen-lockfile to preserve the rest of the
line.
- Around line 219-227: Replace the invalid bun flag usage in the README: change
the command string "bun install --frozen" to "bun install --frozen-lockfile" so
bun enforces installs against bun.lock and fails on mismatch; update any
occurrences of the "bun install --frozen" invocation accordingly (e.g., the
README command block containing "bun install --frozen --ignore-scripts").
---
Outside diff comments:
In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-copilot.ts`:
- Around line 49-78: The Windows branch is passing the full PowerShell
invocation from buildHookCommand into the "powershell" key which causes
PowerShell to be launched twice; update getCopilotHooksJsonContent so when
IS_WIN_AGENT is true you do not call buildHookCommand for the Windows path but
instead construct a command string using the PowerShell call operator and the
script path/event (e.g. & "${hookScriptPath}" "${event}") — change the cmd
helper to return that string on Windows while keeping buildHookCommand for
non-Windows, and ensure the hooks object uses that adjusted cmd for the
"powershell" entries so Copilot CLI receives a script reference/string rather
than a full powershell.exe invocation.
---
Nitpick comments:
In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-common.ts`:
- Around line 11-19: The two functions hookScriptExtension and
hookTemplateExtension have identical implementations; refactor to eliminate
duplication by making hookTemplateExtension delegate to hookScriptExtension
(i.e., return hookScriptExtension()) or consolidate into a single exported
helper used by both callers, keeping IS_WIN_AGENT as the platform check;
alternatively, if you intend them to diverge later, add a clear doc comment on
hookTemplateExtension explaining why it duplicates hookScriptExtension and under
what conditions it should change.
In `@apps/desktop/src/main/lib/agent-setup/notify-hook.ts`:
- Line 12: Replace the platform-specific ternary for NOTIFY_SCRIPT_NAME with the
common hookScriptExtension() approach used by other wrappers: import or use
hookScriptExtension() and set export const NOTIFY_SCRIPT_NAME =
`notify-hook.${hookScriptExtension()}` (remove reliance on IS_WIN_AGENT here) so
the filename generation is consistent with agent-wrappers-gemini.ts /
agent-wrappers-copilot.ts / agent-wrappers-cursor.ts; update any imports if
needed and remove the unused IS_WIN_AGENT branch.
In
`@apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1`:
- Around line 10-18: The template injects {{NOTIFY_PATH}} into a single-quoted
PowerShell string which breaks if the path contains a single quote; modify
agent-wrappers-claude-codex-opencode.ts:buildCodexWrapperExecLine to escape
single quotes in the path before performing replaceAll (convert each ' to ''),
or alternatively emit the path using a PowerShell verbatim here-string (e.g.
@'... '@) when building the codex wrapper exec line so that embedded single
quotes won't create a syntax error; ensure the change targets the replacement
logic that produces $notifyPath and $env:CODEX_TUI_SESSION_LOG_PATH in the
codex-wrapper-exec.template.ps1 output.
In `@apps/desktop/src/main/lib/agent-sleep/windows-sleep-blocker.ts`:
- Around line 66-93: The logic in handleAgentLifecycleForWindowsSleep should
always clear the active set when preventSleepSettingEnabled() is false to avoid
leaving stale entries; change the block that checks preventSleepSettingEnabled()
so active.clear() is called unconditionally (regardless of blockerId), and only
call releaseBlocker() when blockerId !== null (keeping that conditional), so
ensureBlocker() / blockerId race cannot leave orphaned active entries when the
setting is toggled off.
- Around line 37-39: buildKey のキー粒度が workspaceId:tabId:paneId
のみで同一ペイン内の複数エージェントを区別できないため、event の一意識別子（sessionId や
hookSessionId）をキーに含めるよう修正してください: AgentLifecycleEvent 型を拡張して
sessionId/hookSessionId を追加し、buildKey(event: AgentLifecycleEvent)
にこれらを優先して組み込み（例: `${event.workspaceId ?? "-"}:${event.tabId ??
"-"}:${event.paneId ?? "-"}:${event.sessionId ?? event.hookSessionId ?? "-"}`
のような構成）し、active マップの登録・削除処理が新しいキーで正しく動作すること（Start/Stop ハンドラの呼び出し元が sessionId
を渡すこと）を確認してください。
In `@apps/desktop/src/main/lib/vibrancy/index.ts`:
- Around line 164-174: Remove the unnecessary type assertion
WithSetBackgroundMaterial and the optional chaining when calling
setBackgroundMaterial: call window.setBackgroundMaterial(...) directly instead
of casting window to WithSetBackgroundMaterial and using
withMaterial.setBackgroundMaterial?.(...); keep the existing try/catch block
around the call to ensure native exceptions are caught and log errors as before.
🪄 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: CHILL
Plan: Pro
Run ID: 73d87290-34ed-435d-a7e8-dac54d6e268c
📒 Files selected for processing (20)
README.mdapps/desktop/electron-builder.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers-common.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers-copilot.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers-cursor.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers-gemini.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.test.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/main/lib/agent-setup/notify-hook.tsapps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1apps/desktop/src/main/lib/agent-setup/templates/copilot-hook.template.ps1apps/desktop/src/main/lib/agent-setup/templates/cursor-hook.template.ps1apps/desktop/src/main/lib/agent-setup/templates/gemini-hook.template.ps1apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.ps1apps/desktop/src/main/lib/agent-sleep/windows-sleep-blocker.tsapps/desktop/src/main/lib/vibrancy/index.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/shared/constants.ts
✅ Files skipped from review due to trivial changes (2)
- apps/desktop/src/main/lib/agent-setup/agent-wrappers.test.ts
- apps/desktop/src/main/windows/main.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/electron-builder.ts
| export function buildHookCommand( | ||
| hookScriptPath: string, | ||
| ...args: string[] | ||
| ): string { | ||
| const quotedArgs = args.map((arg) => `"${arg}"`).join(" "); | ||
| if (IS_WIN_AGENT) { | ||
| return `powershell.exe -NoProfile -ExecutionPolicy Bypass -File "${hookScriptPath}"${ | ||
| quotedArgs ? ` ${quotedArgs}` : "" | ||
| }`; | ||
| } | ||
| return quotedArgs ? `${hookScriptPath} ${quotedArgs}` : hookScriptPath; | ||
| } |
There was a problem hiding this comment.
buildHookCommand の非 Windows 分岐で hookScriptPath が引用符で囲まれていません。
Windows 側（Line 34）は "${hookScriptPath}" で括っていますが、非 Windows 側（Line 38）は裸のまま連結しています。macOS ではユーザー名（ホームディレクトリ）に空白を含むことが許容されており（例: /Users/John Doe/.superset/hooks/notify.sh）、この経路だとシェル側で単語分割されて実行に失敗します。両分岐で一貫してクオートすることを推奨します。
🛡️ 提案
export function buildHookCommand(
hookScriptPath: string,
...args: string[]
): string {
const quotedArgs = args.map((arg) => `"${arg}"`).join(" ");
if (IS_WIN_AGENT) {
return `powershell.exe -NoProfile -ExecutionPolicy Bypass -File "${hookScriptPath}"${
quotedArgs ? ` ${quotedArgs}` : ""
}`;
}
- return quotedArgs ? `${hookScriptPath} ${quotedArgs}` : hookScriptPath;
+ return quotedArgs ? `"${hookScriptPath}" ${quotedArgs}` : `"${hookScriptPath}"`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function buildHookCommand( | |
| hookScriptPath: string, | |
| ...args: string[] | |
| ): string { | |
| const quotedArgs = args.map((arg) => `"${arg}"`).join(" "); | |
| if (IS_WIN_AGENT) { | |
| return `powershell.exe -NoProfile -ExecutionPolicy Bypass -File "${hookScriptPath}"${ | |
| quotedArgs ? ` ${quotedArgs}` : "" | |
| }`; | |
| } | |
| return quotedArgs ? `${hookScriptPath} ${quotedArgs}` : hookScriptPath; | |
| } | |
| export function buildHookCommand( | |
| hookScriptPath: string, | |
| ...args: string[] | |
| ): string { | |
| const quotedArgs = args.map((arg) => `"${arg}"`).join(" "); | |
| if (IS_WIN_AGENT) { | |
| return `powershell.exe -NoProfile -ExecutionPolicy Bypass -File "${hookScriptPath}"${ | |
| quotedArgs ? ` ${quotedArgs}` : "" | |
| }`; | |
| } | |
| return quotedArgs ? `"${hookScriptPath}" ${quotedArgs}` : `"${hookScriptPath}"`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/agent-setup/agent-wrappers-common.ts` around lines
28 - 39, The non-Windows branch of buildHookCommand does not quote
hookScriptPath, which breaks paths with spaces; update the return for the
non-Windows path so hookScriptPath is wrapped in quotes (use the existing
quotedArgs variable and IS_WIN_AGENT check context) ensuring the final returned
string uses `"<hookScriptPath>"` plus the quotedArgs if present; reference the
buildHookCommand function, the hookScriptPath parameter, and the quotedArgs
local variable when making the change.
| function Send-HookEvent([string]$notifyScript, [string]$eventName) { | ||
| $payload = ('{{"hook_event_name":"{0}"}}' -f $eventName) | ||
| try { | ||
| powershell.exe -NoProfile -ExecutionPolicy Bypass -File $notifyScript $payload | Out-Null | ||
| } catch { | ||
| # Silent — lifecycle notifications must not block codex. | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PowerShell 5.1 external command argument quoting double quote bug PSNativeCommandArgumentPassing
💡 Result:
In Windows PowerShell 5.1, native/external command invocation has long-standing quoting/escaping quirks for embedded double quotes, especially because PowerShell must construct a single Windows command line string for the target process (Windows uses CommandLineToArgvW-style parsing). The practical effect is: if your argument contains embedded " (double quotes), PowerShell may strip or mis-handle them unless you apply additional escaping appropriate for the downstream native quoting rules. About your specific phrase "PowerShell 5.1 external command argument quoting double quote bug PSNativeCommandArgumentPassing": 1) PSNativeCommandArgumentPassing is a PowerShell (Core) 7.x experimental feature that fixes native argument passing on Unix and partially on Windows by changing how the command line is constructed/escaped. Issue #1995 and later work introduced this experimental feature. 2) Windows PowerShell 5.1 does not have PSNativeCommandArgumentPassing at all; you cannot enable it there. So if you’re on Windows PowerShell 5.1, you’re subject to the legacy/native argument passing behavior. 3) For PowerShell 7.x, PSNativeCommandArgumentPassing has modes (Legacy vs Standard/Windows), and there are Windows-specific “accommodations”/fallbacks for certain executables (including cmd.exe/batch-file related cases). For example, PSNativeCommandArgumentPassing has a Windows mode that selectively falls back to Legacy for compatibility with high-profile Windows CLIs. What to do (grounded in the sources): - Use an argument array (or .ArgumentList in PS7) so PowerShell passes arguments as separate argv elements rather than trying to rebuild a single quoted string (this is the common workaround for PowerShell’s “be smart” behavior around spaces/quotes). - For embedded double quotes when calling native programs from PowerShell, you may need additional escaping. PowerShell’s docs/maintainers acknowledge you sometimes must manually add backslash escaping around embedded double quotes so the native program receives literal ". - If you’re calling through cmd.exe /c (or similar), PSNativeCommandArgumentPassing can break cmd /c calls with embedded double quotes because cmd.exe expects command lines as a single string and has its own quoting rules; there are also explicit accommodations to avoid breaking common Windows patterns. Finally, note there was discussion about a “double quote / trailing backslash” edge case in the Legacy mode of $PSNativeCommandArgumentPassing (Issue #26432). That issue was closed with the conclusion that Windows PowerShell v5.1 (the “Legacy” behavior) does perform the relevant escaping, so some reports were based on a misconception—but the core takeaway remains: quoting with embedded " and trailing backslash is tricky and differs between modes/hosts. If you share your exact sample command (the native exe name and the exact argument string you’re trying to pass, including backslashes and quotes), I can tell you which workaround/escaping pattern applies and show a correct PowerShell snippet for PS 5.1 specifically.
Citations:
- 1: Arguments for external executables aren't correctly escaped PowerShell/PowerShell#1995
- 2: Native invocation using
ArgumentListPowerShell/PowerShell#14692 - 3: Implement accommodations for Windows CLIs, notably batch files and misexec-style programs as part of experimental feature PSNativeCommandArgumentPassing PowerShell/PowerShell#15143
- 4: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_powershell_exe?view=powershell-5.1&viewFallbackFrom=powershell-Microsoft.PowerShell.Core
- 5: Document the need for additional \-escaping of double quotes (") when calling external programs MicrosoftDocs/PowerShell-Docs#2361
- 6: Experimental feature PSNativeCommandArgumentPassing breaks cmd /c calls with embedded double quotes PowerShell/PowerShell#15239
- 7: Add a Windows mode for native commands that allows some commands to use legacy argument passing PowerShell/PowerShell#15408
- 8: Bug in
Legacymode of$PSNativeCommandArgumentPassing: a\(backslash) before a closing"(double quote) is unexpectedly escaped PowerShell/PowerShell#26432
🏁 Script executed:
#!/bin/bash
# Find notify-hook.template.ps1 file
find . -name "notify-hook.template.ps1" -type fRepository: MocA-Love/superset
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Read the notify-hook.template.ps1 file to verify stdin support
cat -n ./apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.ps1Repository: MocA-Love/superset
Length of output: 3838
PowerShell 5.1 での JSON ペイロード引数のクォーティング問題を修正する必要がある
PowerShell 5.1 および PSNativeCommandArgumentPassing を有効にしていない PowerShell 7.2 以前では、ネイティブコマンドへ渡す引数に含まれるダブルクォート (") の処理に既知の不具合があります。現在のコード (powershell.exe -File $notifyScript $payload) は JSON ペイロードを引数として渡しているため、引数のクォーティング処理により JSON 内の " が破損し、hook_event_name の抽出に失敗する リスクがあります。その結果、Start・PermissionRequest などのイベント通知が発火しません。
notify-hook.template.ps1 は既に line 18 で [Console]::In.ReadToEnd() によるパイプ入力に対応しているため、ペイロードを stdin 経由で渡すように修正することで、引数クォーティング問題を完全に回避できます。
修正案
function Send-HookEvent([string]$notifyScript, [string]$eventName) {
$payload = ('{{"hook_event_name":"{0}"}}' -f $eventName)
try {
- powershell.exe -NoProfile -ExecutionPolicy Bypass -File $notifyScript $payload | Out-Null
+ $payload | powershell.exe -NoProfile -ExecutionPolicy Bypass -File $notifyScript | Out-Null
} catch {
# Silent — lifecycle notifications must not block codex.
}
}🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 28-30: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
(PSAvoidUsingEmptyCatchBlock)
[warning] Missing BOM encoding for non-ASCII encoded file 'codex-wrapper-exec.template.ps1'
(PSUseBOMForUnicodeEncodedFile)
[warning] 24-24: The variable '$notifyScript' is not declared within this ScriptBlock, and is missing the 'Using:' scope modifier.
Suggested fix: Replace $notifyScript with $using:notifyScript
(PSUseUsingScopeModifierInNewRunspaces)
[warning] 24-24: The variable '$eventName' is not declared within this ScriptBlock, and is missing the 'Using:' scope modifier.
Suggested fix: Replace $eventName with $using:eventName
(PSUseUsingScopeModifierInNewRunspaces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1`
around lines 24 - 31, Send-HookEvent builds a JSON payload in $payload but
passes it as a command-line argument to powershell.exe which breaks double-quote
quoting on PowerShell 5.1; instead pipe the payload to the child PowerShell
process so the notify script (which reads [Console]::In.ReadToEnd()) receives it
via stdin. Modify the powershell.exe invocation in Send-HookEvent to remove the
$payload argument and feed $payload into the process's stdin (e.g., "$payload |
powershell.exe -NoProfile -ExecutionPolicy Bypass -File $notifyScript") while
preserving the try/catch and Out-Null behavior.
| try { | ||
| & '{{REAL_BIN}}' --enable codex_hooks @args | ||
| $codexStatus = $LASTEXITCODE | ||
| } finally { | ||
| if ($WatcherJob) { | ||
| Stop-Job -Job $WatcherJob -PassThru | Receive-Job -ErrorAction SilentlyContinue | Out-Null | ||
| Remove-Job -Job $WatcherJob -Force -ErrorAction SilentlyContinue | Out-Null | ||
| } | ||
| } | ||
|
|
||
| exit $codexStatus |
There was a problem hiding this comment.
$codexStatus が未定義のまま参照される可能性
& '{{REAL_BIN}}' で例外や実行不能エラーが発生した場合、$codexStatus = $LASTEXITCODE が実行されないまま finally に制御が移り、最終的に exit $codexStatus が exit $null（= 0）になります。これにより Codex 実行失敗がラッパーから正常終了として見えてしまいます。
🛠 提案修正
+$codexStatus = 1
try {
& '{{REAL_BIN}}' --enable codex_hooks `@args`
$codexStatus = $LASTEXITCODE
} finally {
if ($WatcherJob) {
Stop-Job -Job $WatcherJob -PassThru | Receive-Job -ErrorAction SilentlyContinue | Out-Null
Remove-Job -Job $WatcherJob -Force -ErrorAction SilentlyContinue | Out-Null
}
}
-
-exit $codexStatus
+
+exit ([int]$codexStatus)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| & '{{REAL_BIN}}' --enable codex_hooks @args | |
| $codexStatus = $LASTEXITCODE | |
| } finally { | |
| if ($WatcherJob) { | |
| Stop-Job -Job $WatcherJob -PassThru | Receive-Job -ErrorAction SilentlyContinue | Out-Null | |
| Remove-Job -Job $WatcherJob -Force -ErrorAction SilentlyContinue | Out-Null | |
| } | |
| } | |
| exit $codexStatus | |
| $codexStatus = 1 | |
| try { | |
| & '{{REAL_BIN}}' --enable codex_hooks `@args` | |
| $codexStatus = $LASTEXITCODE | |
| } finally { | |
| if ($WatcherJob) { | |
| Stop-Job -Job $WatcherJob -PassThru | Receive-Job -ErrorAction SilentlyContinue | Out-Null | |
| Remove-Job -Job $WatcherJob -Force -ErrorAction SilentlyContinue | Out-Null | |
| } | |
| } | |
| exit ([int]$codexStatus) |
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'codex-wrapper-exec.template.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/main/lib/agent-setup/templates/codex-wrapper-exec.template.ps1`
around lines 91 - 101, Initialize a safe non-zero default for $codexStatus
before the try block and ensure any failure sets it: set $codexStatus (e.g. to
1) prior to invoking '& '{{REAL_BIN}}'', add a catch block that assigns
$codexStatus from $LASTEXITCODE (or a non-zero constant) when an exception
occurs, keep the existing finally cleanup for $WatcherJob, and then exit using
$codexStatus so failures in invoking '{{REAL_BIN}}' are not treated as success.
| # {{MARKER}} | ||
| # Called by GitHub Copilot CLI hooks to notify Superset of agent lifecycle events. | ||
| # The Copilot CLI expects valid JSON on stdout; emit it ASAP so the agent is not | ||
| # blocked while we fire the notification request. | ||
| [CmdletBinding()] | ||
| param([string]$EventName) | ||
|
|
||
| $ErrorActionPreference = 'Continue' | ||
|
|
||
| # Drain stdin so the agent isn't blocked by a broken pipe. | ||
| if ([Console]::IsInputRedirected) { | ||
| [Console]::In.ReadToEnd() | Out-Null | ||
| } | ||
|
|
||
| switch ($EventName) { | ||
| 'sessionStart' { $EventType = 'Start' } | ||
| 'sessionEnd' { $EventType = 'Stop' } | ||
| 'userPromptSubmitted' { $EventType = 'Start' } | ||
| 'postToolUse' { $EventType = 'Start' } | ||
| 'preToolUse' { $EventType = 'PermissionRequest' } | ||
| default { | ||
| Write-Output '{}' | ||
| exit 0 | ||
| } | ||
| } | ||
|
|
||
| # Must output valid JSON to avoid blocking the agent. | ||
| Write-Output '{}' | ||
|
|
||
| if (-not $env:SUPERSET_TAB_ID) { exit 0 } | ||
|
|
||
| {{SLEEP_INHIBITOR_SNIPPET}} | ||
|
|
||
| $Port = if ($env:SUPERSET_PORT) { $env:SUPERSET_PORT } else { '{{DEFAULT_PORT}}' } | ||
| $Fields = [ordered]@{ | ||
| paneId = $env:SUPERSET_PANE_ID | ||
| tabId = $env:SUPERSET_TAB_ID | ||
| workspaceId = $env:SUPERSET_WORKSPACE_ID | ||
| eventType = $EventType | ||
| env = $env:SUPERSET_ENV | ||
| version = $env:SUPERSET_HOOK_VERSION | ||
| } | ||
| $Query = ($Fields.GetEnumerator() | ForEach-Object { | ||
| $value = if ($null -eq $_.Value) { '' } else { [string]$_.Value } | ||
| "{0}={1}" -f [Uri]::EscapeDataString($_.Key), [Uri]::EscapeDataString($value) | ||
| }) -join '&' | ||
|
|
||
| try { | ||
| Invoke-WebRequest -Uri "http://127.0.0.1:$Port/hook/complete?$Query" ` | ||
| -Method Get -UseBasicParsing -TimeoutSec 2 -ErrorAction Stop | Out-Null | ||
| } catch { | ||
| # Silent — the agent must not be blocked by transient notification failures. | ||
| } | ||
|
|
||
| exit 0 |
There was a problem hiding this comment.
Windows PowerShell 5.1 互換の UTF-8 BOM を付けてください。
静的解析でも指摘がありますが、このファイルにはコメント内に非 ASCII 文字（全角ダッシュ — など）が含まれる一方で BOM がありません。buildHookCommand が呼ぶ powershell.exe は Windows 10 既定の Windows PowerShell 5.1 で、BOM のない .ps1 は Windows-1252 として解釈されます。現状は非 ASCII がコメント行なので実害は出ませんが、将来テンプレートに非 ASCII の文字列リテラルが入ると文字化け／パースエラーの温床になります。ビルド時にテンプレートを UTF-8 BOM 付きで出力するか、非 ASCII を完全に避けるようにしてください。
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 51-53: Empty catch block is used. Please use Write-Error or throw statements in catch blocks.
(PSAvoidUsingEmptyCatchBlock)
[warning] Missing BOM encoding for non-ASCII encoded file 'copilot-hook.template.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/agent-setup/templates/copilot-hook.template.ps1`
around lines 1 - 55, The PowerShell hook template (copilot-hook.template.ps1)
contains non-ASCII characters in comments but is emitted without a UTF-8 BOM,
causing Windows PowerShell 5.1 (invoked by buildHookCommand -> powershell.exe)
to treat it as Windows-1252; fix by ensuring the build step that writes/copied
this template outputs the .ps1 with a UTF-8 BOM (or alternatively remove/replace
all non-ASCII characters in the template), i.e., update the code path that emits
copilot-hook.template.ps1 so it writes with BOM-enabled UTF8 encoding before
packaging.
| $Input = [Console]::In.ReadToEnd() | ||
| $GeminiEvent = Get-JsonStringValue $Input 'hook_event_name' |
There was a problem hiding this comment.
PowerShell 自動変数 $Input への代入は避ける
$Input は PowerShell の自動変数（パイプライン入力）で、ここでの代入は静的解析でも警告されています。パイプライン経由で呼び出されたとき（例えば将来 Gemini 側が実行方法を変えたとき）や、関数/スクリプトブロック内で再利用された場合に、予期しない挙動を招きます。
🛠 提案修正
-$Input = [Console]::In.ReadToEnd()
-$GeminiEvent = Get-JsonStringValue $Input 'hook_event_name'
+$StdinPayload = [Console]::In.ReadToEnd()
+$GeminiEvent = Get-JsonStringValue $StdinPayload 'hook_event_name'🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] 17-17: The Variable 'Input' is an automatic variable that is built into PowerShell, assigning to it might have undesired side effects. If assignment is not by design, please use a different name.
(PSAvoidAssignmentToAutomaticVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/agent-setup/templates/gemini-hook.template.ps1`
around lines 17 - 18, The script assigns to the PowerShell automatic variable
$Input which is unsafe; replace that assignment with a new local variable (e.g.
$rawInput or $stdinData) and use it when extracting the JSON field. Update the
statement that calls Get-JsonStringValue (currently using $Input) to use the new
local variable (e.g. change the $GeminiEvent assignment to call
Get-JsonStringValue with $rawInput) so you avoid clobbering the automatic $Input
pipeline variable.
| function applyWindowsBackgroundMaterial( | ||
| window: BrowserWindow, | ||
| state: VibrancyState, | ||
| isDark: boolean, | ||
| ): void { | ||
| type WithSetBackgroundMaterial = BrowserWindow & { | ||
| setBackgroundMaterial?: ( | ||
| material: "auto" | "none" | "mica" | "acrylic" | "tabbed", | ||
| ) => void; | ||
| }; | ||
| const withMaterial = window as WithSetBackgroundMaterial; | ||
| try { | ||
| withMaterial.setBackgroundMaterial?.(state.enabled ? "mica" : "none"); | ||
| } catch (error) { | ||
| console.warn("[vibrancy] setBackgroundMaterial failed on Windows:", error); | ||
| } | ||
| // Even when Mica is active we keep backgroundColor at the brand opaque | ||
| // value — the renderer decides per-region whether to let Mica show through. | ||
| window.setBackgroundColor(isDark ? OPAQUE_DARK : OPAQUE_LIGHT); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Electron BrowserWindow setBackgroundMaterial mica requires transparent backgroundColor 2025
💡 Result:
Yes, using Electron's BrowserWindow.setBackgroundMaterial('mica') on Windows requires a transparent backgroundColor (set to null or '#00000000') or transparent: true for the effect to apply to the client area beyond just the titlebar. Official docs do not explicitly state this, but multiple GitHub issues confirm it. To implement: - Set backgroundColor: null or '#00000000' in BrowserWindow options. - Ensure HTML/CSS body and html have transparent or no background (background: transparent; or unset). - Optionally use transparent: true, but note it may disable resizing/maximizing unless using titleBarOverlay or recent fixes (post-2025 updates). - Call win.setBackgroundMaterial('mica') after window creation. - Works on Windows 11 22H2+, improved in Electron v38+ and later fixes in 2025 (e.g., PR #45456, #46657). Example: const win = new BrowserWindow({ backgroundColor: '#00000000', // other options }); win.setBackgroundMaterial('mica'); In CSS: html, body { background: transparent !important; } Caveats (as of 2026): May fail on maximize/frameless without focus/Energy Saver off; client area shows white/opaque if backgroundColor set solidly. Recent Electron versions (37+) handle better without explicit transparent: true via internal transparency for materials.
Citations:
- 1: Windows backgroundMaterial doesn't work electron/electron#48031
- 2: [Feature Request]: Support full frame transparency with Mica background material electron/electron#38532
- 3: [Bug]: Cannot enable Background Material for the client area electron/electron#38454
- 4: https://www.electronjs.org/docs/api/browser-window/
- 5: https://electronjs.org/docs/latest/api/browser-window
- 6: feat: support Mica/Acrylic on Windows electron/electron#38163
- 7:
setBackgroundMaterial('acrylic')only takes effect whentransparentis enabled. electron/electron#49443 - 8: [Bug]:
backgroundMaterialeffect does not work on first page load electron/electron#41072
🏁 Script executed:
# Check the current state of the vibrancy file around the identified lines
cat -n apps/desktop/src/main/lib/vibrancy/index.ts | sed -n '160,180p'Repository: MocA-Love/superset
Length of output: 978
🏁 Script executed:
# Check lines 264-269 for getInitialWindowOptions
cat -n apps/desktop/src/main/lib/vibrancy/index.ts | sed -n '260,275p'Repository: MocA-Love/superset
Length of output: 750
🏁 Script executed:
# Check for OPAQUE_DARK and OPAQUE_LIGHT constants
rg -n "OPAQUE_DARK|OPAQUE_LIGHT" apps/desktop/src/main/lib/vibrancy/Repository: MocA-Love/superset
Length of output: 636
🏁 Script executed:
# Determine Electron version requirement
fd -e json package.json | xargs rg -l "electron" | head -5 | xargs -I {} cat {} | rg -A1 -B1 '"electron"'Repository: MocA-Love/superset
Length of output: 191
Windows での Mica が不透明な backgroundColor により表示されません
Lines 177 と 267 で、Mica が有効な場合でも常に不透明な backgroundColor（#151110 または #ffffff）を設定しています。Electron で Mica マテリアルを正常に機能させるには、有効時に背景色を透明（#00000000）にする必要があります。現在のコードではコンポジション層が不透明で塗りつぶされ、DWM が描画した Mica テクスチャが隠れるため、ユーザーには何も表示されません。
修正方法:
Lines 171–177（applyWindowsBackgroundMaterial）で、setBackgroundColor の呼び出しを以下のように条件付けてください:
window.setBackgroundColor(
state.enabled ? "#00000000" : (isDark ? OPAQUE_DARK : OPAQUE_LIGHT)
);同様に Lines 264–269（getInitialWindowOptions の Windows 分岐）も修正してください:
if (PLATFORM.IS_WINDOWS) {
return {
backgroundMaterial: state.enabled ? "mica" : "none",
backgroundColor: state.enabled
? "#00000000"
: (isDark ? OPAQUE_DARK : OPAQUE_LIGHT),
};
}Line 175–176 のコメント「レンダラが領域ごとに Mica を通すかどうかを判定する」は不正確です。レンダラの透明性では Mica は表示されません。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/lib/vibrancy/index.ts` around lines 159 - 178, The
Windows Mica is hidden because applyWindowsBackgroundMaterial (function
applyWindowsBackgroundMaterial) always calls window.setBackgroundColor with an
opaque color; change it to set the color to transparent ("#00000000") when
state.enabled is true and keep the opaque OPAQUE_DARK/OPAQUE_LIGHT when false
(i.e., set background color conditionally based on state.enabled). Do the same
change in getInitialWindowOptions' Windows branch where
backgroundMaterial/backgroundColor are returned (ensure backgroundColor is
transparent when state.enabled and opaque otherwise). Also update the misleading
comment in applyWindowsBackgroundMaterial to not claim renderer transparency
enables Mica.
| # Skip lifecycle scripts; native modules are materialized by the postinstall .mjs. | ||
| bun install --frozen --ignore-scripts | ||
|
|
||
| cd apps/desktop | ||
| $env:SUPERSET_WORKSPACE_NAME = 'superset' | ||
| bun run install:deps | ||
| bun run compile:app | ||
| bun run build:browser-mcp | ||
| bun run package -- --publish never --win |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the scripts referenced and where browser-mcp output is staged.
fd -t f 'package.json' apps/desktop --max-depth 2 -x cat {}
echo "---"
rg -n "build:browser-mcp|browser-mcp|superset-browser-mcp" apps/desktop -g '!**/node_modules/**' -C2
echo "---"
# Inspect electron-builder config to see if browser-mcp bin is included in extraResources/files
fd -t f 'electron-builder*' apps/desktop --max-depth 2 -x cat {}Repository: MocA-Love/superset
Length of output: 40617
🌐 Web query:
Bun install frozen-lockfile flag documentation
💡 Result:
The --frozen-lockfile flag for bun install ensures reproducible installs by using the exact package versions specified in bun.lock. If package.json dependencies disagree with the lockfile, Bun exits with an error without updating the lockfile. Usage: bun install --frozen-lockfile This is equivalent to bun ci. Commit bun.lock to version control for CI/CD use. Configuration in bunfig.toml: [install] frozenLockfile = true Note: Some users report issues in monorepos or after lockfile migrations (e.g., binary to text format in Bun 1.2+), often fixed by running bun install first and committing updates. Official docs recommend it for production reproducibility, as in Docker: RUN bun install --frozen-lockfile.
Citations:
bun install --frozen フラグを --frozen-lockfile に修正してください。
220行の bun install --frozen は無効です。正しいフラグは --frozen-lockfile です。このフラグはロックファイルに基づいた再現可能なインストールを強制し、bun.lockとpackage.jsonが不一致の場合はエラーで終了します。
修正前後
- bun install --frozen --ignore-scripts
+ bun install --frozen-lockfile --ignore-scriptsビルド順序（compile:app → build:browser-mcp → package）は正しく設定されています。package スクリプトが実行される際、自動的に prepackage フックが先に実行され、その中で build:browser-mcp が呼び出されるため、electron-builder は正しく構築されたブラウザーMCP バイナリを入手できます。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 219 - 227, Replace the invalid bun flag usage in the
README: change the command string "bun install --frozen" to "bun install
--frozen-lockfile" so bun enforces installs against bun.lock and fails on
mismatch; update any occurrences of the "bun install --frozen" invocation
accordingly (e.g., the README command block containing "bun install --frozen
--ignore-scripts").
| cd superset | ||
|
|
||
| # Skip lifecycle scripts; native modules are materialized by the postinstall .mjs. | ||
| bun install --frozen --ignore-scripts |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does bun install --frozen(without-lockfilesuffix) work in Bun 1.x, or is only--frozen-lockfile supported?
💡 Result:
No, bun install --frozen (without the -lockfile suffix) does not work in Bun 1.x. Only --frozen-lockfile is supported. Official Bun documentation explicitly describes --frozen-lockfile as the flag for reproducible installs that fail if package.json disagrees with bun.lock (sources 2, 4, 14, 18). CLI help and configuration reference it as --frozen-lockfile or frozenLockfile = true in bunfig.toml (sources 14, 18, 19). No official sources mention --frozen alone. All GitHub issues and discussions use --frozen-lockfile exclusively (sources 1, 3, 9, 10, 62, 63). Searches for --frozen without -lockfile yield no supporting evidence.
Citations:
- 1: https://bun.sh/docs/pm/cli/install
- 2: https://bun.com/docs/pm/cli/install
- 3: https://bun.sh/docs/cli/install
- 4: https://bun.sh/docs/install/
- 5: https://www.mintlify.com/oven-sh/bun/pm/install
- 6: (Regression)
bun install --frozen-lockfilekeeps reporting issues oven-sh/bun#19088 - 7:
bun cias an alias ofbun install --frozen-lockfileoven-sh/bun#5283 - 8:
bun install --frozen-lockfilekeeps reporting issues oven-sh/bun#16692 - 9:
--frozen-lockfileis saying changes to lockfile whenbun installsays there is not oven-sh/bun#20913
bun install --frozen を --frozen-lockfile に修正してください。
Bun の公式ドキュメントによると、bun install --frozen は無効です。正しいフラグは --frozen-lockfile のみが対応しています。短縮形 --frozen では lockfile に従ったインストールが機能せず、通常の install に落ちる可能性があります。README の再現性と正確性のため、line 220 を bun install --frozen-lockfile --ignore-scripts に変更してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 220, Update the README entry that currently uses the
invalid shorthand flag by replacing the command string "bun install --frozen
--ignore-scripts" with the correct flag name so it reads "bun install
--frozen-lockfile --ignore-scripts"; locate the occurrence of the exact command
text in README.md and change only the flag name to --frozen-lockfile to preserve
the rest of the line.
PR #423 が `output.banner: "delete process.env.ELECTRON_RUN_AS_NODE;"` を 追加したが、rollup の banner は全 chunk の 1:1 に prepend されるため、 shared chunk (`dist/main/chunks/*.js`) で local に `process` を再束縛する wrapper と衝突し、`ReferenceError: Cannot access 'process' before initialization` で main プロセスが落ちていた (macOS で再現)。 さらにこの statement は本来の目的 (Electron を plain Node モードで起動 させない) に対しても効かない: `require("electron")` は entry chunk の 上部 (line ~37) で実行される一方、banner は line ~144 で実行される。 Electron のランタイムモードは binary 起動時の env で確定するので、 module 実行中の delete には意味がない。 VS Code 等の host 配下で child process として Superset を起動する ケースのためのクリーンアップは、別途 child_process spawn 側で 対応する follow-up として切り出す。
1 participant
Summary
Phase 1 — 起動基盤 (upstream
superset-sh/superset#2100を fork 用に再実装)scripts/postinstall.sh→scripts/postinstall.mjs$CI早期 exit を維持、Windows ではinstall:depsを非致命にapps/desktop/electron-builder.tsnpmRebuildを Windows 無効化 (VS Build Tools 無しの node-gyp 失敗を回避)、NSIS Desktop/Start Menu ショートカット追加apps/desktop/electron.vite.config.tsrequire(\"electron\")前にELECTRON_RUN_AS_NODEをクリア、renderer にstripCrossOriginPluginを追加apps/desktop/scripts/copy-native-modules.tsrmSync(.., { recursive, force }))apps/desktop/src/lib/electron-app/factories/app/setup.tsapps/desktop/src/lib/window-loader.tssuperset-app://プロトコル経由でロード (file:// だと ES dynamic import が破綻)apps/desktop/src/main/index.tssuperset-appscheme の privileged 登録 + protocol handler、api.superset.sh / PostHog / Sentry への outbound CORS バイパスapps/desktop/src/main/windows/main.tstitleBarOverlayで閉じる/最小化ボタンを描画、nativeThemeで dark/light 色追従、renderer console を stdout forwardapps/desktop/vite/helpers.tsdefineEnvを||fallback に変更 (空文字列 CI secret 対策)、stripCrossOriginPluginを exportPhase 2 — macOS 固有機能の Windows fallback
play-sound.ts: Windows で PowerShellSystem.Media.SoundPlayer(WAV) /MediaPlayer(他) 経由の再生分岐ScriptsEditor.tsx: file input のacceptに.ps1 / .cmd / .batを追加IS_FORKpath で GitHub API 経由のチェックが動くため Windows でも更新通知が出る状態で変更不要Phase 3 — agent-setup / host-service
apps/desktop/src/main/lib/agent-setup/index.ts: Windows では wrapper/hook setup を skip。bash 専用の notify/copilot/cursor/gemini/codex テンプレートを PowerShell に完全移植する作業は follow-up PR (同一 issue)。packages/host-service/.../setup-terminal.ts/runtime/teardown/teardown.ts: Windows はsetup.ps1 / .cmd / .bat(resp. teardown) を探してpowershell.exe -Fileorcmd.exe /cで spawn。POSIX は従来どおり.sh。packages/host-service/providers/git/.../askpass.ts: Windows ではfindstr /I \"^Username\"ベースの.cmdaskpass ヘルパを生成。Phase 4 — CI
.github/workflows/build-desktop.ymlにwindows-latestのbuild-windowsjob を追加:git config --global core.longpaths true(260 char 対策)shell: bash(Git Bash で fork 既存の bash スクリプトを動かす)bun install --frozen --ignore-scripts→bun run install:deps→bun run generate:icons→bun run compile:app→bun run build:browser-mcp→bun run package --win*.exe,*.exe.blockmap,latest.yml既に Windows 対応済みで手を入れていない領域
COMSPEC,powershell.exe,PATHEXTallow-list 整備済み)wmic,Get-CimInstance,netstat -ano)superset-browser-mcp.exeappend、extraResources 対応済み)@libsql/win32-x64-msvc,@ast-grep/napi-win32-x64-msvc,@parcel/watcher-win32-*全て検証パイプライン済み)gopls.exe, Dartdart.exe)where.exe分岐)Follow-up (本 PR の対象外、Issue #273 に残す)
notify / copilot / cursor / gemini / codexhook テンプレートの PowerShell 版実装BUILDING.md/READMEの Windows 手順追記Test plan
bun run typecheck通過 (ローカル).github/workflows/build-desktop.ymlを手動ディスパッチで呼び出し、build-windowsjob が*.exe+latest.ymlを artifact として出力することを確認.exeを実行し、以下が動作することを確認:Cmd+Q相当 (Alt+F4) でアプリ終了process.platform === \"win32\"orPLATFORM.IS_WINDOWSで保護済み)Refs: #273
Summary by CodeRabbit
新機能
改善
ドキュメント