Skip to content

upstream取り込み: 安全な29コミット (安全・おそらく安全)#388

Merged
MocA-Love merged 61 commits intomainfrom
upstream/safe-batch-1
Apr 23, 2026
Merged

upstream取り込み: 安全な29コミット (安全・おそらく安全)#388
MocA-Love merged 61 commits intomainfrom
upstream/safe-batch-1

Conversation

@MocA-Love
Copy link
Copy Markdown
Owner

@MocA-Love MocA-Love commented Apr 23, 2026

Summary

upstream (superset-sh/superset) の70コミットのうち、フォーク独自機能と衝突しない29コミットをcherry-pick。

最終更新 (2026-04-24): 段階 1 / 2 / 3 を経て CI 3 種(typecheck / lint / test)実質グリーン、fork 固有機能 100% 健在、外部レビュー指摘 41 件の精査・修正まで完了した状態で仕上げ。superset-sh#3295 のみ意図的にスキップ(下記「意図的にスキップした upstream コミット」参照)。

取り込み内容

セキュリティ・依存関係:

CI/インフラ:

CLI/trpc:

Desktop(安全・おそらく安全):

Fork側のコンフリクト解決

  • package.json: fork独自の ansi_up, @vscode/ripgrep, @xyflow/react を維持しつつupstreamのdep更新を適用
  • Terminal.tsx: fork独自の TERMINAL_OPTIONS importを維持
  • useTerminalLifecycle.ts: upstreamのpaste handler変更に追従
  • page.tsx (v1 workspace): fork独自の BROWSER_RELOAD/BROWSER_HARD_RELOAD useHotkey配線を維持。OPEN_DIFF_VIEWER は V1 では登録せず、V2 側で upstream 意図通り利用(後述の「V1/V2 hotkey 分岐」参照)
  • browser-manager.ts: fork独自のimport群を維持しつつ safeOpenExternal を追加

段階 2: Silent Regression の検出と修正

コンフリクトが発生しない状態で upstream diff が fork 実装を silently 上書きしていたケースを検出・修正:

  • packages/host-service/src/trpc/router/git/git.ts (upstream fix(host-service): v2 workspace git correctness (upstream, 3-dot, numstat) superset-sh/superset#3543 で fork 実装が喪失)
    • fork が commit 184c0610b で追加していた listBranchessortOrder / pinDefault 入力と remote-only / pin-default 処理を復元
  • DashboardSidebarWorkspaceItem.tsx (upstream 取り込み時の prop 伝播不整合)
    • 親→子 への hostIsOnline / onCopyBranchName / branch 伝播を修正
  • ⌘⇧L hotkey 重複 (upstream feat(desktop): ⌘⇧L opens diff viewer in v2 workspace superset-sh/superset#3556 OPEN_DIFF_VIEWER と fork 既存 TOGGLE_EXPAND_SIDEBAR が同キー)
    • V1 page から OPEN_DIFF_VIEWER useHotkey 登録を削除(ハンドラは SEARCH_IN_FILES と同一で冗長だった)
    • RightSidebar の HotkeyLabel id を TOGGLE_EXPAND_SIDEBAR に戻し、サイドバー展開ボタンのラベルを正常化
    • V1 / V2 は別ルートなので useHotkeyenabled で自然に分岐

段階 3: 外部レビュー指摘への対応 (CodeRabbit × 4 回 + Codex P1 × 3 件 = 41 コメント)

全 41 件を分類して処理:

  • A. fork 取り込み起因の真のバグ → 修正済み (3 件)
  • B. 現 HEAD で既に解消済み (約 10 件): Codex P1 の INCEPTION_AUTH_PROVIDER_ID / getSmallModel async 互換などは段階 1 の 766404e26 / 3e3cff0cf / 7540cbc8c で対応済み
  • C. upstream 機能そのものへの指摘 (約 25 件): 本 PR (fork 取り込み) の scope 外として無視
  • D. 判断委譲 → 対応済み (3 件のうち 2 件修正、1 件保留)

A / D で実施した修正:

  1. fix(desktop): restore Windows branch in port-scanner process helpers (ff206cda4)
    • upstream 取り込みで削除された Windows プラットフォーム分岐を復元
    • getProcessName / getProcessCommandwin32 分岐と wmic / PowerShell fallback を復元(Windows ユーザーでの回帰防止)
  2. fix(chat): reject Anthropic keys in isOpenAIApiKey (7ae4ab984)
    • sk-ant- prefix の Anthropic key を OpenAI credential として誤承認する bug を修正
  3. fix(desktop): restore MainWindowEffects singleton guard for tearoff (a51729052)
  4. fix(local-db): drizzle journal の tag prefix 不整合を drizzle-kit で再生成 (3ebb91bdf)
    • 61db020efagent_preset_permissions_migrated_at migration を手動追加した際の tag prefix ズレ(idx 69/70 の tag が 0040_ / 0069_ のままだった)を v1.5.5-fork.14 state に巻き戻し、drizzle-kit generate で正しく idx 70 として再生成
    • 追加で判明した 0040_snapshot.json の prevId collision(upstream #5aaeb0f82 による上書き)も同時解消
    • v1.5.5-fork.14 には agent_preset_permissions_migrated_at が含まれないため既存配布ユーザーへの破壊的変更なし
  5. chore(plans): shipped 計画を plans/done に移動 (e7c1ffc27)
    • AGENTS.md の plan 配置ルール準拠
  6. hotkey 重複解消 (85706b4e1): 段階 2 で既述

保留 (D 分類 1 件):

意図的にスキップした upstream コミット

superset-sh#3295 fix(desktop): resolve GitHub status for branch workspaces は当 PR では revert 維持。

理由:

  • fix(desktop): resolve GitHub status for branch workspaces superset-sh/superset#3295git-status.ts を 1842 → 516 行 に簡素化し、fork が独自追加した 19 tRPC プロシージャ (getGitHubRepositoryOverview, getGitHubWorkflowRuns, rerunPullRequestChecks, replyToPullRequestComment ほか) の前提となるヘルパー群を github.ts (-319行) / cache.ts (-259行) からも全削除する内容。
  • キャッシュ TTL 短縮 (30s→10s) のようなミクロ改善だけの cherry-pick も、削除される export に fork 19 プロシージャが依存しているため単独取り込み不可。
  • 両立には fork 19 プロシージャと upstream 簡素化版を個別に再設計する別 PR でのアーキテクチャ作業が必要。

スキップしたコミット(別PRで対応)

fork 固有機能ヘルスチェック(全項目健在を grep で実機確認済み)

  • 19 tRPC プロシージャ全数
  • 依存: ansi_up / @vscode/ripgrep / @xyflow/react
  • apps/desktop/electron-builder.tsdmg.size="4g" 対策
  • SUPERSET_WORKSPACE_NAME 関連ロジック
  • Kimi Code サポート(VscodeExtensionsSettings.tsx
  • Find Search Focus 修正(PR fix(desktop): Diff Viewer / File Viewer の Cmd+F 検索フォーカス修正 #390、既に main にマージ済み)
  • TERMINAL_OPTIONS (Terminal.tsx 等 5 ファイル 14 箇所)
  • VS Code 拡張シム層 / VscodeExtensionButtons
  • listBranches sortOrder/pinDefault(本 PR で復元)
  • INCEPTION_AUTH_PROVIDER_ID
  • MainWindowEffects tearoff ガード(本 PR で復元)
  • Windows プラットフォーム分岐 (port-scanner)(本 PR で復元)

既知の残課題(本 PR の責務外、別 Issue で対応)

Test plan

  • bun install が正常に完了する
  • bun run typecheck が通る(ローカル実機確認済み、27/27 task green)
  • bun run lint が通る(biome 指摘ゼロ)
  • bun test の fail は PR upstream取り込み: 安全な29コミット (安全・おそらく安全) #388 起因ではない既存バグ / bun 環境制約のみ
  • local-db drizzle journal に不整合なし(idx 68 → 69 → 70 の tag prefix 正当化)
  • desktop dev 起動確認
  • V1 workspace で ⌘⇧L がサイドバー展開として動作する
  • V2 workspace で ⌘⇧L が diff viewer を開く
  • mermaid diagram が markdown pane でレンダリングされる
  • v2-workspaces ページがソート可能なテーブルで表示される
  • fork 独自の GitHub Repository Tools(v1 sidebar の repo タブ)が正常に動作する
  • tearoff window で重複エフェクト起動なし
  • Windows 環境で terminal port scanner が動作する(該当環境があれば)

Kitenite and others added 29 commits April 23, 2026 12:41
…uperset-sh#3565)

Observability was enabled in superset-sh#1464 but dropped when the proxy was
re-created from scratch in superset-sh#1867. Without it, every wrangler deploy
reconciles Cloudflare back to logs/traces off, which is why the
dashboard toggles kept reverting after each production deploy.

Pin invocation_logs explicitly so future config drift can't silently
disable it again. Audit logs are an account-level setting and still
need to be re-enabled in the Cloudflare dashboard separately.
Without it, Chromium rejects https://localhost:* with
ERR_CERT_AUTHORITY_INVALID on machines that never had Caddy's local root
CA installed with trust flags (e.g. fresh machines, or where the prior
`caddy run` sudo prompt was dismissed).
…-sh#3599)

* fix(workspace-client): preserve host path prefix in useWorkspaceWsUrl

new URL(path, hostUrl) resolves absolute paths against the origin, dropping
hostUrl's path component. For remote workspaces hostUrl is
"https://relay.superset.sh/hosts/<hostId>"; building the terminal WS URL
via new URL("/terminal/<id>", hostUrl) yielded
"wss://relay.superset.sh/terminal/<id>", which the relay 404s (only
/hosts/:hostId/* is routed). Swap to string concat so the prefix survives.

* fix(relay): cap to one fly machine

TunnelManager.tunnels is an in-process Map — with 2+ replicas a proxy
request routed to the replica that doesn't own the tunnel returns
503 "Host not connected". Prod was running 2 machines and serving ~half
of all /hosts/:hostId/* traffic as 503. Scaled down live; this commit
codifies it so the next deploy doesn't recreate the second machine.

Longer-term fix (shared tunnel state via Redis pub/sub) tracked in
SUPER-427.
Captures the architecture plan for SUPER-427 (relay hardening) so
phases can ship independently. Documents the fly-replay + Upstash
directory approach (reusing the existing rate-limit instance), the
truthful is_online story, streaming-proxy fix, graceful shutdown,
observability, and data-model cleanups — with rejected alternatives
recorded so we don't relitigate them.
* docs: consolidated weekly changelog — 2026-04-20

Supersedes superset-sh#3206 (2026-04-06) and superset-sh#3404 (2026-04-13), folding in this
week's v2 workspace work so three weeks of shipped changes land in one
post instead of three stale PRs.

* docs(changelog): include chat UX overhaul (superset-sh#3039) in consolidated post

* docs(changelog): add compressed chat-ux hero screenshot

* docs(changelog): refresh chat-ux hero and add v2 file tree screenshot

* docs(changelog): add brand refresh screenshot

* docs(changelog): add v2 diff viewer screenshot

* docs(changelog): v2 workspace framed as early access, add screenshot

* docs(changelog): rename title to 'v2 early access'

* docs(changelog): frame v2 as a cloud-aimed rebuild — terminal rewrite, IDE architecture

* docs(changelog): drop articles in title

* docs(changelog): light trim — drop redundant lead-ins and filler words
…t-sh#3590)

Production Electric runs on Electric Cloud and local dev runs on Docker —
the Fly.io `superset-electric` app no longer exists, so every deploy-electric
run was failing on `flyctl secrets set --app superset-electric`. Remove the
job, the preview Fly.io counterpart, and the root fly.toml. Point preview API
at the shared ELECTRIC_URL/ELECTRIC_SECRET secrets.
…#3631)

* chore(ci): pin third-party GitHub Actions to commit SHAs

Replaces every mutable tag (`@v4`, `@v2`, `@master`, etc.) on third-party
actions with the commit SHA they currently resolve to, with a trailing
`# <tag>` comment so Dependabot/Renovate can keep them updated.

Closes Tolmo findings 04e0e887, 3580d63d, 3ced71b1, 59691bf4, ce908e26,
f7d7ab2e, 8bd4a7ba, 6ab1fc7e — all rooted in the unpinned
`oven-sh/setup-bun@v2` (and other tags) reachable from
`deploy-production.yml` and `build-desktop.yml`.

114 references pinned across 13 workflow files. No logic changes.

* chore(ci): use specific release tags in pin comments

Replaces major-tracking comments (`# v4`, `# v2`, `# master`) with the
precise release tag the SHA points to (`# v4.3.1`, `# v2.2.0`, `# 1.6`).
Same SHAs, more useful comments — Dependabot/Renovate can now show
"v4.3.1 → v4.4.0" in the bump PR title instead of a no-op `v4 → v4`.
The Vercel-hosted `/api/electric/*` route is no longer used by
supported desktop clients. Bumps minimum desktop version to 1.5.0
(the first release after 2026-04-10) so any lingering client on a
pre-1.5.0 build is forced to update before it can heartbeat against
an endpoint that's about to disappear.

Drops the route, its env vars (ELECTRIC_URL/SECRET/SOURCE_*),
`@electric-sql/client` dep, Electric CORS headers, and the matching
env passthroughs in the Vercel deploy workflows. The Cloudflare
worker (apps/electric-proxy) and Fly-hosted Electric instance stay
intact.
…superset-sh#3638)

* feat(cli,trpc): organization override via header, no session mutation

CLI sends `x-superset-organization-id` when `organizationId` is set in
config. A new middleware on `protectedProcedure` validates membership and
exposes `ctx.activeOrganizationId`, falling back to the session's active
org when the header is absent. Router call sites migrate to the new ctx
field; `requireActiveOrgId`/`requireActiveOrgMembership` take `ctx`.

Adds `superset organization switch <idOrSlug>`; renames `commands/org/`
to `commands/organization/`. `auth login` picks an org when the user has
multiple memberships.

* fix(cli): narrow login error swallowing + lazy sessionActive fetch

- Scope the post-login try/catch to the non-essential user.me info
  line; let myOrganizations / writeConfig errors surface.
- Skip myOrganization round-trip for single-org users; only fetch
  the session's active org when the multi-org selector needs it as
  a default.
- Remove --organization from the global flags doc since it isn't
  wired as an actual global (use `organization switch` instead).
…ts (superset-sh#3562)

Memory leak and CPU spiral root-caused to `staleTime: 0, gcTime: 0` +
60fps polling: React Query can't dedupe or GC anything, and the render
path churns allocations every 16ms.

Restoring the React Query defaults (5min gcTime) fixes the leak. Server
poll rate is independent of perceived stream smoothness — StreamingMessageText
already reveals text client-side at 60fps from whatever buffer the server
delivers. 4fps polling keeps that buffer fed with plenty of headroom.

Also removes the `isRunning` invalidation effect — redundant when the
query is polling.

Builds on and supersedes superset-sh#3170 by @thepathmakerz, which diagnosed the
same root cause. This version takes the subtractive path (-21 lines)
instead of adaptive polling (+36).

Closes superset-sh#3049
…3642)

* feat(desktop): render mermaid diagrams in markdown pane

Wire the editable TipTap code-block node view to render a live mermaid
preview above the source when the language is `mermaid`, using the same
Streamdown plugin + theme handling as the read-only CodeBlock. The
read-only v2 path already supported mermaid; this closes the gap for
v1's editable rendered view.

* feat(desktop): toggle between mermaid preview and source in markdown pane

Default to rendered-only for mermaid code blocks with content; click the
diagram or the toolbar toggle to reveal the editable source. Empty
blocks open in source mode so users can type. The code element stays
mounted (hidden) when the preview is showing so TipTap keeps tracking
content.

* fix(desktop): polish mermaid preview in markdown pane

- Definitively hide the mermaid source via inline display:none so the
  raw code no longer shows below the preview.
- Drop the extra bg/padding frame around the Streamdown viewer so
  zoom/expand controls aren't captured by a click-to-edit wrapper.
- Unify hover toolbar as a single pill (rounded border + bg/80 +
  backdrop-blur) and move it to top-left in preview mode so it doesn't
  overlap Streamdown's own top-right toolbar.
- Override Streamdown's min-h-28 to min-h-80 so the diagram canvas is
  taller by default.

* fix(desktop): guard mermaid fence against backticks in source

Open the synthetic markdown with 4 backticks instead of 3 so a stray
triple-backtick inside the user's mermaid source can't close the fence
early and leave Streamdown with malformed input.
…t-sh#3513) (superset-sh#3554)

* fix(desktop): recover terminal from non-monospace font crash (superset-sh#3513)

Setting the terminal font to a proportional family like "Inter" blanked
the app on next launch — the bad value persisted in SQLite and xterm
couldn't lay out cells on reload, leaving no way back into settings.

- Sanitize the stored family on read: if the primary family isn't
  monospace (per canvas measurement), fall back to the default terminal
  font so a poisoned DB value can never blank the renderer.
- Hide the "Other" group and custom free-form entry in the terminal
  font picker so new selections are restricted to monospace candidates.

* fix(desktop): reject all-proportional generic terminal font stacks

Follow-up on superset-sh#3554 review. sanitizeTerminalFontFamily previously passed
any all-generic CSS value through untouched (e.g. "cursive", "sans-serif",
"monospace, sans-serif") because parsePrimaryFontFamily returns null when
no concrete family is present — same blank-window crash class as the
"Inter" report.

Refactor the sanitizer to inspect the full family list: when no concrete
primary exists, only trust the value if every entry is a monospace
generic; otherwise fall back to the default. Add regression tests.

* refactor(desktop): append ", monospace" fallback + cleaner font preview

- Always append "monospace" to the sanitized terminal font stack when it
  doesn't already end with one. Mirrors VS Code's behavior in
  src/vs/workbench/contrib/terminal/browser/terminalConfigurationService.ts
  so that if the configured primary isn't installed on this machine, the
  browser falls back to the OS monospace generic instead of a proportional
  default.
- Swap the terminal font preview from a box-drawing layout (which rendered
  as broken in proportional fonts and used tofu glyphs) to a shell session
  that demonstrates column alignment naturally.
- Drop a couple of narrating comments flagged in simplify review.

* refactor(desktop): show Nerd Fonts in the editor picker too

Nerd Fonts are monospace — the terminal-only gate was pre-existing
special-casing and reviewers pointed out it hides a widely-used class
of fonts from users picking an editor font. Drop the gate.

* fix(desktop): validate the actual CSS primary font, not the first concrete entry

Addresses the coderabbit review on b2e6a04. The sanitizer previously
skipped leading generics when picking the primary to measure, so a value
like `sans-serif, "JetBrains Mono"` passed validation because the later
concrete entry was monospace — but CSS resolves the first generic
(sans-serif) and the terminal still renders proportional.

Switch to validating families[0] (the actual CSS primary): if it's a
monospace generic, trust the stack; if it's a proportional generic, fall
back; if it's concrete, canvas-measure it. Add regression tests.
…perset-sh#3581)

* fix(desktop): restore terminal buffer after Unicode 11 activation

The persisted xterm buffer was being replayed before the Unicode11Addon
was loaded, so CJK, emoji, and ZWJ sequences got parsed with Unicode 6
cell widths. The wrong widths baked into the buffer, producing garbled
glyphs on repaint — especially visible with many Claude Code tabs open
and Chinese content (superset-sh#3572).

Mirrors VS Code's pattern: load Unicode11Addon during terminal
construction, before the first write. Also bumps @xterm/* to the
versions VS Code ships (xterm 6.1.0-beta.197, webgl 0.20.0-beta.196).

* docs: tighten Unicode 11 ordering comment
…d paste (superset-sh#3582)

* fix(desktop): terminal paste auto-submits first line when bracketed paste is off

Terminal's custom paste handler reimplemented xterm's `\r?\n → \r`
normalization to enable a chunking path that turned out to be
unnecessary. Without bracketed paste, the `\r` sequence makes the shell
execute each pasted line as Enter, so a multi-line paste would run only
the first line.

Mirror VS Code's approach: delegate to `xterm.paste()` (which handles
normalization + bracketed-paste wrapping correctly) and add a
`shouldPasteTerminalText`-style confirmation dialog for multi-line
pastes when the shell doesn't have bracketed paste mode enabled.

* drop multi-line paste warning dialog, keep only paste-handler simplification

* drop setupPasteHandler entirely, let xterm handle paste natively

* Lint

* restore minimal setupPasteHandler for Electron clipboard-event propagation

* Revert "restore minimal setupPasteHandler for Electron clipboard-event propagation"

This reverts commit f3fba94.

* remove unused isBracketedPasteRef from useTerminalLifecycle
Rename TOGGLE_EXPAND_SIDEBAR to OPEN_DIFF_VIEWER (same binding).
In v2, focus any existing diff pane or open one in a new tab, and
flip the workspace sidebar to the Changes tab. V1 keeps its existing
expand-sidebar behavior under the new ID.
…ebar (superset-sh#3619)

* fix(desktop): place new v2 workspaces at top of their project in the sidebar

New workspaces were inserted with tabOrder = max + 1, burying them at the
bottom of the project's list. Prepend with min - 1 instead so freshly
created workspaces surface at the top without disturbing existing order.

* fix(desktop): seed empty-project prepend tabOrder at 1 to match reorder helpers
…h#3655)

* fix(desktop): render pending workspaces at top of sidebar

New workspaces are inserted with a prepend tabOrder (top), but the pending placeholder was pinned to MAX_SAFE_INTEGER (bottom), so the creating/failed indicator visually separated from the row it represents. Flip the constant to MIN_SAFE_INTEGER so the pending sits alongside the just-created workspace at the top.

* chore(desktop): tighten PENDING_WORKSPACE_TAB_ORDER comment
superset-sh#3635)

* feat(desktop): add Copy Branch Name to v1 and v2 sidebar context menus

* chore(desktop): drop success toast from Copy Branch Name

* chore(desktop): restore success toast on Copy Branch Name

* fix(desktop): expose Copy Branch Name on collapsed v1 local-workspace

The collapsed branch-workspace branch returned early with only a Tooltip,
so right-click surfaced no menu. Wrap the button in a ContextMenu so Copy
Branch Name is reachable.

* style(desktop): biome auto-format collapsed branch context menu trigger
…h#3660)

* feat(desktop): redesign v2-workspaces list as sortable Linear-style table

Replace stacked card rows with a dense full-width table: Sidebar / Name /
Host / Branch / Created columns that align across projects, sortable
column headers, proper truncation on long workspace and host names, and
hover-revealed row actions.

* fix(desktop): address v2-workspaces review feedback

- Sidebar column sort direction was inverted (default "desc" put non-sidebar
  items first); use a normal boolean comparator so descending means "in
  sidebar first".
- Row onKeyDown was firing when Enter/Space bubbled from focused action
  buttons; ignore keyboard events originating from descendants.
- Offline host cell regressed to a native `title` tooltip; wrap it in the
  shared `<Tooltip>` so the indicator is keyboard-accessible and styled
  consistently with the action buttons.
- Extract SortableHeader into its own folder per repo conventions
  (one component per file) and move shared sort types into a types module.
…licks (superset-sh#3552)

* fix(desktop): refresh v2 terminal link tooltip editor label + nudge plain clicks

- LinkHoverTooltip fetched the default editor in a mount-only useEffect, so
  changing the default editor in settings left the modifier-shift label
  ("Open in Cursor", etc.) stale until the terminal pane unmounted. Refetch on
  every hover-start instead.
- Plain (no-modifier) clicks on a detected file path in the v2 terminal were
  silent, which made the modifier-key affordance undiscoverable. On a plain
  file-link click, show a transient tooltip at the click position
  ("Hold ⌘ to open · ⌘⇧ for external", or Ctrl/Ctrl+Shift off-mac). Capped at
  two shows per renderer session via a module-level counter, and suppressed
  while the modifier-hover tooltip is already visible. Uses framer-motion's
  AnimatePresence for fade in/out.

* refactor(desktop): simpler v2 terminal link tooltip labels

- "Open in editor" → "Open in pane" for the ⌘-click file case (native in-app
  file pane is what actually happens).
- Shift variant always says "Open in external editor" instead of interpolating
  the configured editor name. openFileInEditor uses the global settings
  defaultEditor (non-editor apps like Finder can't be set there), so the
  interpolated name could disagree with a user's per-project preference — the
  generic label never lies.
- Drops the getDefaultEditor fetch, defaultEditor state, and getAppOption/
  getAppLabel plumbing that went with it.

* refactor(desktop): URL ⌘-click tooltip says "Open in pane" for consistency
…et-sh#3650)

* fix(desktop): allowlist URL schemes before shell.openExternal

The browser-pane context menu and `external.openUrl` tRPC procedure
forwarded attacker-controlled URLs directly to `shell.openExternal`,
letting a `file://` href right-clicked inside a pane launch arbitrary
applications via OS URL handlers.

Adds `main/lib/safe-url` with an `{http, https, mailto}` allowlist and
routes both sinks through it. Rejects disallowed schemes on `openUrl`
with BAD_REQUEST.

* fix(desktop): catch openExternal rejections and redact URL logs

Addresses two defense-in-depth findings from PR superset-sh#3650 review:

- safeOpenExternal now try/catches shell.openExternal so fire-and-forget
  callers (browser-manager context menu) can't leak an unhandled rejection
  into the main process.
- Log only the parsed URL scheme (via externalUrlLogLabel) instead of the
  raw untrusted URL in both sinks, avoiding path/email/token spill into
  electron-log files.

---------

Co-authored-by: Satya Patel <satyapatel111@gmail.com>
…uperset-sh#3659)

`safe-url.ts` imports `shell` from electron at the module top level, so
bun's test runner can't load the file and the whole suite fails with
`SyntaxError: Export named 'shell' not found`. Moves the pure URL
helpers (`isSafeExternalUrl`, `externalUrlLogLabel`) to `scheme.ts`
and has the test import from there. `safeOpenExternal` stays in
`safe-url.ts` with the electron import; the barrel keeps the same
public surface.
…rset-sh#3629)

host.info was calling organization.getActiveFromJwt, which resolves to
organizationIds[0] on the JWT regardless of which org the host-service
is configured for. Users in multiple orgs saw the same (first)
membership name for every Host Service tray entry.

Adds organization.getByIdFromJwt and has host.info use ctx.organizationId
so each per-org host-service reports its own org.
… + dev placeholders (superset-sh#3580)

* fix(desktop): unblock AI branch/workspace naming for OAuth-only users + dev placeholders

`getSmallModel` only resolved Anthropic credentials from `apikey:anthropic`
in mastracode's auth.json. Users authed via Claude OAuth (the default Code
flow) had no api-key entry, so it returned `null` silently and naming
always fell back to a slugified prompt. A stale/dummy `ANTHROPIC_API_KEY`
in `.env` (e.g. the `=dummy` placeholder) made it worse: the env path was
preferred, the bad key 401'd, and the fallback toast hid the real reason.

Resolution order is now: env api-key → stored api-key → OAuth → openai.
Anthropic api-keys are validated to start with `sk-ant-api` and openai
keys with `sk-` so dev placeholders fall through to OAuth instead of
being sent to the API. OAuth is read directly from auth.json with refresh
done via direct HTTP to console.anthropic.com — no mastracode import (it
pulls in onnxruntime-node and broke electron-vite bundling, see superset-sh#3517).

Also cap the workspace auto-rename prompt at "20 characters or less" so
sidebar titles stay short.

Diagnostic logs (`[get-small-model] using …` / refresh failures) are kept
so future regressions surface in the dev terminal instead of silently
producing the fallback toast.

* review: address self-review feedback on small-model OAuth fix

- Fix pre-existing OpenAI provider-id mismatch: mastracode stores keys at
  apikey:openai-codex, not apikey:openai. Settings-saved OpenAI keys now
  reach the small-model path.
- Add length floor (30 chars) to api-key validation. Catches placeholders
  shorter than the prefix check would (e.g. bare "sk-ant-api").
- Drop success-path console.log calls. Keep the no-credentials warn so the
  silent-fallback regression stays visible if it ever returns.
- Tighten writeAnthropicEntry comment to acknowledge the cross-provider
  race window and document the (rare) impact + escape hatch.
- Hard-cap workspace auto-rename output at 20 chars server-side. Prompt
  asks for 20, but LLMs miscount; this guarantees the contract.
- Add provenance comment for CLAUDE_CODE_OAUTH_CLIENT_ID.
- Drop unused AnthropicOAuthHeaders type export; const is `as const` so
  callers get the same type inference.
- Change getSmallModel return type from Promise<unknown | null> to
  Promise<unknown> (null is assignable to unknown).
- Add unit tests:
  - isAnthropicApiKey / isOpenAIApiKey validation (rejects "dummy",
    rejects OAuth tokens sent as api keys, accepts real key shapes).
  - isOAuthEntry shape validation.
  - 20-char truncation behaviour in generateWorkspaceNameFromPrompt.

* test(chat): add OAuth refresh integration tests with mocked fs + fetch

Covers the previously-untested paths in anthropic-oauth.ts:
- happy path: valid non-expired token returned without refresh
- refresh: expired token + 200 → new entry persisted with refreshed access,
  refresh, and future expiry
- refresh: response without refresh_token → original refresh token preserved
- failure: 4xx, missing access_token, fetch throw → all return null without
  writing
- cross-provider preservation: refreshing anthropic does not clobber the
  openai-codex slot (the race-window concern from the self-review)

* review: address second-pass feedback on small-model OAuth fix

- Fix data-loss in writeAnthropicEntry on parse failure: distinguish
  "file missing" from "parse error" in readAuthJson; refuse to overwrite
  auth.json when existing content is unparseable (would otherwise wipe
  every non-anthropic provider slot).
- Cache the in-memory refreshed entry so we don't hammer Anthropic's OAuth
  endpoint when persistence keeps failing (read-only home dir, full disk).
- Move provider-id constants to packages/chat/src/server/shared/
  auth-provider-ids.ts; desktop re-exports for back-compat. getSmallModel
  now iterates OPENAI_AUTH_PROVIDER_IDS (covers both `openai-codex` and
  legacy `openai` slots) instead of hardcoding one.
- Drop unused AuthJsonOAuthEntry export; only isOAuthEntry is consumed
  externally.
- Comment the fs-mock isolation constraint in anthropic-oauth.test.ts.
- Drop the workspace-title server-side .slice(0, 20). The prompt still
  says "20 characters or less"; LLMs miscount but the soft hint is good
  enough — hard truncation produced ugly mid-word cuts.
- New tests:
  - data-loss prevention: corrupt auth.json → no write
  - expires_in default path (3600s) when response omits it
  - leeway boundary: token expiring within REFRESH_LEEWAY_MS triggers refresh
  - cache reuse: write failure once doesn't cause a second refresh fetch

* review: address PR review-bot feedback

- Fix EXDEV on Linux: writeAnthropicEntry now creates the temp file in
  the SAME directory as auth.json (renameSync across filesystems throws
  when /tmp is tmpfs and $HOME is on a different mount). mkdirSync the
  target dir first so initial writes on a fresh install succeed.
  Reported by greptile, coderabbitai, cubic (P1 each, independently).
- Add 10s AbortController timeout around the OAuth refresh fetch so a
  stalled token endpoint can't hang AI naming indefinitely. Reported by
  coderabbitai.
- Deduplicate getAuthJsonPath/readAuthJson: export from anthropic-oauth.ts
  and import in get-small-model.ts. getAnthropicOAuthCredential now takes
  an optional pre-parsed authData so getSmallModel doesn't read auth.json
  twice per call. Reported by greptile.
- Tests: temp-file location + mkdirSync call, refresh-timeout abort.

* fix(chat): isolate OAuth test fs mocking to prevent leakage to sibling tests

CI failure root cause: anthropic-oauth.test.ts used `mock.module("node:fs", …)`,
which is process-global in bun:test. When the chat suite ran all 19 test files
in one process, mcp-overview.test.ts (which uses real fs to set up temp dirs)
inherited my no-op mocks → 8 unrelated failures.

Fix: extract auth-storage-io.ts (small dedicated I/O wrapper for auth.json)
and add an injectable IO seam to anthropic-oauth.ts via __setIOForTests /
__resetIOForTests. The OAuth test now passes a plain mock object instead of
mocking node:fs at all. Production code keeps the same default behaviour.

Bonus: auth.path is now an injectable parameter on readAuthJson/writeAuthJson,
so auth-storage-io.test.ts can exercise real file I/O against a temp dir
without touching the user's real auth.json.

Verified: full chat suite 148/148 pass, ai-name 6/6 pass, live OAuth smoke
test still produces a real title.

* refactor(chat): use mastracode for OAuth instead of reimplementing

The custom anthropic-oauth.ts + auth-storage-io.ts that this PR added were
justified by a constraint that doesn't actually exist: mastracode is already
externalized via runtime-dependencies.ts (so electron-vite doesn't try to
bundle it) AND is already imported by 10+ other places in the codebase
(chat-service, host-service's resolveAnthropicCredential, etc). There's no
bundle-size or memory cost to importing it from get-small-model too.

Mastracode's createAuthStorage().getApiKey(providerId) handles:
- OAuth refresh via the Claude Code endpoint
- Atomic write-back to auth.json (EXDEV-safe)
- Refresh token rotation
- All the auth.json schema details

We were reimplementing all of it. ~830 lines deleted across 4 files;
~170-line get-small-model.ts replaces it.

What we keep:
- isAnthropicApiKey / isOpenAIApiKey validators (still needed for the
  ANTHROPIC_API_KEY=dummy fallthrough — the actual original bug)
- ANTHROPIC_OAUTH_HEADERS constant (these go on the inference request,
  not the OAuth flow — mastracode doesn't help here)
- auth-provider-ids.ts shared module
- Memoized AuthStorage instance (matches chat-service.ts pattern)

Verified: typecheck 25/25, full chat suite 123/123, ai-name 6/6, live OAuth
smoke test still produces a real branch name with ANTHROPIC_API_KEY=dummy
in the environment.

* fix(chat): prefer apikey:<provider> slot over main slot in get-small-model

If a user logs in with OAuth after saving an API key via Settings, the
main `anthropic` (or `openai-codex`) slot flips to OAuth and the stored
API key in `apikey:<provider>` becomes unreachable from the small-model
path — even though the user explicitly added it.

Check `getStoredApiKey()` first for both Anthropic and OpenAI before
falling back to the main slot. Matches the precedence the previous
custom implementation had and aligns with the docstring.

* chore(branch-naming): nudge LLM toward 20 chars or less

Branch names show up in compact UI surfaces (sidebar, tabs, status). The
existing "2-4 words" guidance still leaves room for a 60-char branch when
the words are long. Add an explicit char hint to keep them tight.

Soft prompt only — no server-side truncation. Same approach as workspace
title prompt.
…es with / (superset-sh#3232)

* fix: fall back to FETCH_HEAD checkout when gh pr checkout fails for branch names with /

Fixes superset-sh#3231

gh pr checkout internally runs `git checkout -b <branch> --track origin/<branch>`.
When the branch name contains `/`, git cannot resolve the tracking ref inside a
freshly created detached worktree, producing "starting point is not a branch".

The fetch succeeds — only the tracking setup fails. Catch that specific error and
fall back to `git checkout -b <localBranchName> --no-track FETCH_HEAD`.
push.autoSetupRemote=true (already set after worktree creation) handles push
tracking without needing --track.

* fix: use -B flag to force-replace branch in FETCH_HEAD fallback checkout

* fix: log fallback path when gh pr checkout fails with tracking error

---------

Co-authored-by: Ruan Gustavo Araujo da Silveira <ruan.silveira@M4Pro.local>
- LanguageServicesProvider / ScheduleFireToasts を layout.tsx に復元
  (upstream MainWindowEffects 解体で脱落していた)
- workspace-creation.ts: base branch 書き込みを remote-tracking のみに制限
  (local branch で Changes tab が壊れる回帰を防ぐ)

💘 Generated with Crush

Assisted-by: GLM-5 via Crush <crush@charm.land>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (2)
apps/desktop/src/renderer/routes/_authenticated/layout.tsx (1)

196-199: ⚠️ Potential issue | 🟠 Major

MainWindowEffects 経由の singleton 所有権ガードに戻してください。

AgentHooks / ScheduleFireToasts / WorkspaceInitEffects を直接 mount すると、tear-off window でも agent orchestration・global toast・workspace init が起動します。既存の MainWindowEffects は tear-off 所有権チェックを持っているため、ここではそれを render するのが安全です。

修正案
-import { WorkspaceInitEffects } from "renderer/screens/main/components/WorkspaceInitEffects";
-import { ScheduleFireToasts } from "renderer/features/todo-agent/ScheduleFireToasts";
 import { LanguageServicesProvider } from "renderer/providers/LanguageServicesProvider";
@@
-import { AgentHooks } from "./components/AgentHooks";
 import { GlobalTerminalLifecycle } from "./components/GlobalTerminalLifecycle";
+import { MainWindowEffects } from "./components/MainWindowEffects/MainWindowEffects";
@@
 							<LanguageServicesProvider />
-							<AgentHooks />
-							<ScheduleFireToasts />
+							<MainWindowEffects />
 							<Outlet />
-							<WorkspaceInitEffects />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/routes/_authenticated/layout.tsx` around lines 196
- 199, 現在のコードは AgentHooks / ScheduleFireToasts / WorkspaceInitEffects
を直接マウントしており、tear-off window でもこれらが起動してしまうので、既存の singleton 所有権チェックを持つ
MainWindowEffects を代わりにレンダーしてください。具体的には layout コンポーネント内で直接 <AgentHooks />,
<ScheduleFireToasts />, <WorkspaceInitEffects /> を削除し、代わりに MainWindowEffects
をレンダーするよう差し替え(MainWindowEffects は所有権ガードを実装しているため単一実行を担保します)。MainWindowEffects /
AgentHooks / ScheduleFireToasts / WorkspaceInitEffects のシンボルを参照して差し替えを行ってください。
apps/desktop/src/renderer/stores/tabs/types.ts (1)

134-312: ⚠️ Potential issue | 🔴 Critical

TabsStore インターフェースに実装されていないメソッドが残っています。

ai_summary によれば store.ts から次のアクションが削除されています(setTabColor、setFileViewerDisplayName、addDatabaseExplorerTab、addVscodeExtensionTab、addGitGraphTab、addReferenceGraphTab、addActionLogsTab、setDatabaseExplorerConnection、detachTabForTearoff、hydrateFromTearoff、hydrateReturnedTab)。一方で本ファイルの TabsStore インターフェースにはそれらが依然として宣言されたままです(L134, L161, L238–L270, L272–L275, L310–L312)。

結果として:

  1. create<TabsStore>()(...) が不足プロパティで型エラーになる(あるいは他所でキャストして握り潰されていれば警告なしに欠損)。
  2. 既存呼出側(例: useTearoffInit.tshydrateReturnedTab を呼ぶ等)があれば実行時に TypeError: ... is not a function

この PR の方針(upstream の削除を受け入れる)ならインターフェースからも削除し、呼出側も整合させるべきです。フォーク独自機能として残す場合は store.ts に実装を戻す必要があります。

#!/bin/bash
# Verify: the declared-but-removed actions are truly absent in store.ts and identify any remaining callers.
set -euo pipefail

STORE=apps/desktop/src/renderer/stores/tabs/store.ts
TYPES=apps/desktop/src/renderer/stores/tabs/types.ts

echo "=== store.ts: presence of each action name (expect 0 for removed) ==="
for name in setTabColor setFileViewerDisplayName addDatabaseExplorerTab addVscodeExtensionTab addGitGraphTab addReferenceGraphTab addActionLogsTab setDatabaseExplorerConnection detachTabForTearoff hydrateFromTearoff hydrateReturnedTab; do
  count=$(rg -nP "^\s*${name}\s*[:\(]" "$STORE" | wc -l | tr -d ' ')
  echo "  ${name}: ${count} occurrence(s) in store.ts"
done

echo
echo "=== types.ts: each action is still declared ==="
rg -nP '^\s*(setTabColor|setFileViewerDisplayName|addDatabaseExplorerTab|addVscodeExtensionTab|addGitGraphTab|addReferenceGraphTab|addActionLogsTab|setDatabaseExplorerConnection|detachTabForTearoff|hydrateFromTearoff|hydrateReturnedTab)\s*:' "$TYPES" || true

echo
echo "=== External call sites that would break at runtime if missing ==="
for name in setTabColor setFileViewerDisplayName addDatabaseExplorerTab addVscodeExtensionTab addGitGraphTab addReferenceGraphTab addActionLogsTab setDatabaseExplorerConnection detachTabForTearoff hydrateFromTearoff hydrateReturnedTab; do
  echo "--- callers of ${name} ---"
  rg -nP --type=ts --type=tsx "\b${name}\b" -g '!**/types.ts' -g '!**/store.ts' -g '!**/*.d.ts' -C1 || true
done
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/stores/tabs/types.ts` around lines 134 - 312, The
TabsStore interface still declares methods that were removed from store.ts
(setTabColor, setFileViewerDisplayName, addDatabaseExplorerTab,
addVscodeExtensionTab, addGitGraphTab, addReferenceGraphTab, addActionLogsTab,
setDatabaseExplorerConnection, detachTabForTearoff, hydrateFromTearoff,
hydrateReturnedTab); remove these declarations from the TabsStore interface in
types.ts to match store.ts, then run a project-wide search for callers of those
symbols (e.g., useTearoffInit.ts calling hydrateReturnedTab) and either (A)
update those call sites to the new API or guard/replace the behavior, or (B) if
you intend to keep the functionality, restore the corresponding implementations
in store.ts (implement functions with the exact names above) so the interface
and store remain consistent.
🤖 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/renderer/routes/_authenticated/layout.tsx`:
- Around line 28-30: Reorder the import statements in layout.tsx to satisfy
Biome's import ordering (including the newly added ScheduleFireToasts and
LanguageServicesProvider) by running Biome's organizer or the project assist
task; specifically, ensure imports like WorkspaceInitEffects,
ScheduleFireToasts, and LanguageServicesProvider are ordered per Biome rules
(you can run `biome check --write --unsafe` or the
`assist/source/organizeImports` task) so CI no longer fails.

In
`@packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts`:
- Around line 722-729: このブロックで setProgress(input.pendingId, "ensuring_repo")
を呼んだ後に localProject が見つからない場合に projectNotSetupError(input.projectId)
を投げているため、進捗レコードが残ったままになる問題があります。localProject が null の早期判定(ここで参照している変数/関数名:
setProgress, projectNotSetupError, createProgress,
sweep)を行ったら、例外を投げる前に該当する進捗をクリア/完了する処理を呼び出してから例外を投げるように修正してください(つまり
projectNotSetupError を throw する直前に進捗を削除するロジックを追加)。
- Around line 777-803: The code replaces a local default branch with its
upstream without preserving the remote, which causes comparisons to be
reconstructed as origin/${baseBranch} even when the upstream is a non-origin
remote; in function logic around resolveUpstream, asRemoteRef and startPoint
assignment, only convert the local default branch to a remote-tracking
startPoint when the upstream.remote === "origin" (i.e., guard the replacement to
origin remotes), and use async/await instead of mixing then/catch for the
rev-parse check; apply the same guard and async/await pattern to the other
similar block referenced (lines ~886-888) so downstream consumers that rebuild
refs as origin/${baseBranch} see consistent refs.
- Around line 838-850: The git worktree add call uses startPoint.shortName
(assigned to startPointArg) which can cause ambiguous ref resolution when local
and remote-tracking refs share names; instead pass the fully-qualified ref to
git.raw to avoid ambiguity. Update the git.raw invocation in the workspace
creation flow (the worktree add call that takes branchName and worktreePath) to
use startPoint.ResolvedRef.fullRef (or ResolvedRef.fullRef on the startPoint
object) for all ref kinds (local, remote-tracking, tag) rather than
startPointArg/shortName so the command always receives an unambiguous full ref.

---

Duplicate comments:
In `@apps/desktop/src/renderer/routes/_authenticated/layout.tsx`:
- Around line 196-199: 現在のコードは AgentHooks / ScheduleFireToasts /
WorkspaceInitEffects を直接マウントしており、tear-off window でもこれらが起動してしまうので、既存の singleton
所有権チェックを持つ MainWindowEffects を代わりにレンダーしてください。具体的には layout コンポーネント内で直接
<AgentHooks />, <ScheduleFireToasts />, <WorkspaceInitEffects /> を削除し、代わりに
MainWindowEffects をレンダーするよう差し替え(MainWindowEffects
は所有権ガードを実装しているため単一実行を担保します)。MainWindowEffects / AgentHooks / ScheduleFireToasts
/ WorkspaceInitEffects のシンボルを参照して差し替えを行ってください。

In `@apps/desktop/src/renderer/stores/tabs/types.ts`:
- Around line 134-312: The TabsStore interface still declares methods that were
removed from store.ts (setTabColor, setFileViewerDisplayName,
addDatabaseExplorerTab, addVscodeExtensionTab, addGitGraphTab,
addReferenceGraphTab, addActionLogsTab, setDatabaseExplorerConnection,
detachTabForTearoff, hydrateFromTearoff, hydrateReturnedTab); remove these
declarations from the TabsStore interface in types.ts to match store.ts, then
run a project-wide search for callers of those symbols (e.g., useTearoffInit.ts
calling hydrateReturnedTab) and either (A) update those call sites to the new
API or guard/replace the behavior, or (B) if you intend to keep the
functionality, restore the corresponding implementations in store.ts (implement
functions with the exact names above) so the interface and store remain
consistent.
🪄 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: 4b715a96-3056-4e66-815c-255405632edb

📥 Commits

Reviewing files that changed from the base of the PR and between b2de08d and 8da85eb.

📒 Files selected for processing (8)
  • apps/desktop/src/renderer/routes/_authenticated/layout.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx
  • apps/desktop/src/renderer/stores/tabs/store.ts
  • apps/desktop/src/renderer/stores/tabs/types.ts
  • apps/desktop/src/renderer/stores/tabs/utils.ts
  • apps/desktop/src/shared/tabs-types.ts
  • packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts
  • packages/local-db/drizzle/meta/_journal.json
✅ Files skipped from review due to trivial changes (2)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsx
  • apps/desktop/src/renderer/stores/tabs/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/local-db/drizzle/meta/_journal.json

Comment thread apps/desktop/src/renderer/routes/_authenticated/layout.tsx Outdated
Comment thread apps/desktop/src/renderer/stores/tabs/store.ts Outdated
- _journal.json コンフリクト解消(両エントリを保持)
- layout.tsx: LanguageServicesProvider / ScheduleFireToasts 復元
- workspace-creation.ts: base branch を remote-tracking のみに制限
- agent-custom.ts: 欠落していた shared モジュールを追加
- error-types.ts: ProjectNotSetupCause を追加
- git-helpers.ts: buildBranch に isRemote パラメータ追加
- v2-project.ts: requireActiveOrgMembership に ctx を渡すよう修正
- cli package.json: @superset/shared 依存を追加
- shared package.json: simple-git-unsafe export を追加
- tabs-types.ts: PaneType に "comment" を追加
- FileViewerMode に "conflict" を追加

残存する typecheck エラー: upstream batch-2 の一部コミットが
desktop renderer 側の新機能を追加したが、対応する tRPC router
実装がまだ取り込まれていないため。これらは次回の upstream
取り込み PR で解決される。

💘 Generated with Crush

Assisted-by: GLM-5 via Crush <crush@charm.land>
- tRPC手続き依存ファイルをfork版に復元(main時点でもエラーあり)
- port-scanner.ts: getProcessName/getProcessCommand を追加
- env.ts: upstream版に更新(buildTerminalEnv含む)
- registry.ts: TOGGLE_EXPAND_SIDEBAR を追加
- page.tsx: OPEN_DIFF_VIEWER のuseHotkey引数を修正
- CommentPane削除(tRPC手続き未実装)

残存エラーは全て main ブランチから存在する既知の問題。
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

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/renderer/hotkeys/registry.ts (1)

189-205: ⚠️ Potential issue | 🟠 Major

OPEN_DIFF_VIEWERTOGGLE_EXPAND_SIDEBAR のデフォルトキーが競合しています。

両方とも ⌘⇧L / Ctrl⇧AltL で、workspace page では両方の handler が有効登録されています。PR目的どおり ⌘⇧L を diff viewer に割り当てるなら、既存の TOGGLE_EXPAND_SIDEBAR はデフォルト未割当にするか別キーにしてください。

修正案
 	TOGGLE_EXPAND_SIDEBAR: {
 		key: {
-			mac: "meta+shift+l",
-			windows: "ctrl+shift+alt+l",
-			linux: "ctrl+shift+alt+l",
+			mac: null,
+			windows: null,
+			linux: null,
 		},
 		label: "Toggle Expand Sidebar",
 		category: "Layout",
 		description: "Toggle sidebar between tabs and changes view",
 	},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/hotkeys/registry.ts` around lines 189 - 205, The
two hotkey entries OPEN_DIFF_VIEWER and TOGGLE_EXPAND_SIDEBAR share the same
default key (mac: "meta+shift+l", windows/linux: "ctrl+shift+alt+l") causing
handler conflicts; update the TOGGLE_EXPAND_SIDEBAR hotkey definition to avoid
the collision by removing or clearing its default key mapping (e.g., delete or
set its key to undefined/empty for mac/windows/linux) or assign it a different
non-conflicting shortcut, and keep OPEN_DIFF_VIEWER mapped to
"meta+shift+l"/"ctrl+shift+alt+l"; locate and change the TOGGLE_EXPAND_SIDEBAR
entry in the hotkey registry to apply this fix.
🧹 Nitpick comments (4)
apps/desktop/src/main/lib/terminal/port-scanner.ts (1)

74-90: AbortSignal を受け付ける拡張は良い一方、getProcessName / getProcessCommand にも伝搬を検討

getListeningPortsForPidssignal を下位に貫通できるようになりましたが、公開 API の getProcessName / getProcessCommandsignal を受け付けません。同じ呼び出しコンテキスト (例: ペイン解決) からキャンセルしたい場面で不整合になります。

一貫性のため optional signal?: AbortSignal を追加することを推奨します。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/terminal/port-scanner.ts` around lines 74 - 90,
getListeningPortsForPids now accepts an optional AbortSignal but does not
propagate it to getProcessName/getProcessCommand; add an optional signal?:
AbortSignal parameter to the public helpers getProcessName and getProcessCommand
and thread that signal through any internal calls (e.g., from
getListeningPortsForPids -> getListeningPortsLsof/getListeningPortsWindows ->
places that call getProcessName/getProcessCommand) so callers can cancel the
entire flow; update function signatures and call sites to pass the signal along
and ensure any spawned async operations respect the signal.
apps/desktop/src/main/lib/terminal/env.ts (3)

459-459: getLocale には baseEnv を渡す方が一貫しています(任意)。

LANGLC_ALL はどちらも ALLOWED_ENV_VARS に含まれているため、rawBaseEnvbaseEnv のどちらを渡しても結果は同じになります。ただし buildTerminalEnv 内で rawBaseEnv を参照しているのはここだけで、他はすべて allowlist 済みの baseEnv を基準にしているため、baseEnv に寄せるとスナップショットから raw を取り出す必要がなくなり、意図(「ターミナルに露出する値をベースに locale を決める」)も明確になります。

♻️ 変更案
-	const { raw: rawBaseEnv, safe: baseEnv } = getProcessEnvSnapshot();
+	const { safe: baseEnv } = getProcessEnvSnapshot();
 
 	// shellEnv provides shell wrapper control variables (ZDOTDIR, BASH_ENV, etc.)
 	// These configure how the shell initializes, not the user's actual environment
 	const shellEnv = getShellEnv(shell);
-	const locale = getLocale(rawBaseEnv);
+	const locale = getLocale(baseEnv);

(このまま適用すると getProcessEnvSnapshot の戻り値で raw を使わなくなるので、必要に応じて getProcessEnvSnapshot の公開形を整理してもよいと思います。)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/terminal/env.ts` at line 459, The call to getLocale
currently passes rawBaseEnv but should use the allowlisted baseEnv to be
consistent with buildTerminalEnv's other uses; update the invocation so
getLocale(baseEnv) is used instead of getLocale(rawBaseEnv), ensuring locale is
derived from the terminal-exposed environment, and then verify any dependent
logic in buildTerminalEnv and getProcessEnvSnapshot still works (and consider
cleaning up getProcessEnvSnapshot's public shape if raw is no longer needed).

163-170: cachedMacosSystemCertAvailable は TTL/無効化なしでプロセス生存中キャッシュされます。

ユーザーが実行中に /etc/ssl/cert.pem を入れ替える/削除するといった稀なケースで古い結果を返し続けます。システム証明書バンドルの有無はアプリの稼働中に変化することはほぼ無いため実害は限定的ですが、意図がプロセス永続キャッシュであることを 1 行コメントで明示しておくと、将来 resetTerminalEnvCachesForTests に似た本番経路が必要になった際の判断が早くなります。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/terminal/env.ts` around lines 163 - 170, The
function hasMacosSystemCertBundle uses a process-lifetime cached value
cachedMacosSystemCertAvailable without TTL or invalidation; add a one-line
comment above the cachedMacosSystemCertAvailable usage (and/or above
hasMacosSystemCertBundle) stating this is an intentional process-persistent
cache and not expected to change during runtime, and reference that tests can
reset it via resetTerminalEnvCachesForTests if needed so future readers know why
no TTL/invalidation is present.

485-485: GOOGLE_API_KEYdelete は実質的にノーオペです。

baseEnvbuildSafeEnv で allowlist フィルタ済みで、ALLOWED_ENV_VARS / ALLOWED_PREFIXESGOOGLE_API_KEY は含まれていません。shellEnv (getShellEnv) も SUPERSET_ORIG_ZDOTDIR/ZDOTDIR しか返さず、terminalEnv の他のキーもリテラルで固定されているため、このパスで GOOGLE_API_KEY が混入する経路はありません。

防御的に残したい意図があるなら、将来的に allowlist へ誤って追加された場合の最終防衛線である旨をコメントで残しておくと、読み手が「なぜこの削除が必要か」を追えるのでおすすめです。そうでなければ dead code として削除しても機能上の差はありません。

♻️ 意図を明示するコメント案
+	// Defense in depth: ensure GOOGLE_API_KEY never leaks even if the allowlist
+	// or shellEnv were to change to include it in the future.
 	delete terminalEnv.GOOGLE_API_KEY;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/lib/terminal/env.ts` at line 485, The delete of
terminalEnv.GOOGLE_API_KEY is effectively dead (baseEnv is already allowlisted
by buildSafeEnv using ALLOWED_ENV_VARS/ALLOWED_PREFIXES, getShellEnv only
returns SUPERSET_ORIG_ZDOTDIR/ZDOTDIR, and terminalEnv keys are literal), so
remove the statement delete terminalEnv.GOOGLE_API_KEY; from env.ts; if you
prefer to keep a defensive guard instead, replace it with a short comment on
terminalEnv explaining that any explicit deletion is only a final-defense
against accidental future additions to the allowlist (referencing buildSafeEnv,
ALLOWED_ENV_VARS, ALLOWED_PREFIXES, and getShellEnv) so readers understand the
intent.
🤖 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/terminal/port-scanner.ts`:
- Around line 277-296: Restore platform branching so Windows does not always hit
the catch path: in getProcessName and getProcessCommand detect process.platform
=== 'win32' and call the Windows-specific implementations (e.g.,
getProcessNameWindows / getProcessCommandWindows) which should use wmic or
PowerShell fallbacks; for non-win32 keep the current execFileAsync('ps', ...)
path with EXEC_TIMEOUT_MS. Ensure both functions return the same types/values as
before on Windows (not the fallback 'unknown' / ''), and reuse execFileAsync
only for the Unix branch.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx:
- Around line 665-674: The OPEN_DIFF_VIEWER hotkey handler is opening the Search
tab instead of the diff/changes view; update the useHotkey callback (the
OPEN_DIFF_VIEWER handler) to call setRightSidebarTab(workspaceId,
RightSidebarTab.Changes) instead of RightSidebarTab.Search, while keeping the
existing logic that opens the sidebar and sets SidebarMode.Tabs (refer to
useHotkey, OPEN_DIFF_VIEWER, setRightSidebarTab, RightSidebarTab in the diff).

---

Outside diff comments:
In `@apps/desktop/src/renderer/hotkeys/registry.ts`:
- Around line 189-205: The two hotkey entries OPEN_DIFF_VIEWER and
TOGGLE_EXPAND_SIDEBAR share the same default key (mac: "meta+shift+l",
windows/linux: "ctrl+shift+alt+l") causing handler conflicts; update the
TOGGLE_EXPAND_SIDEBAR hotkey definition to avoid the collision by removing or
clearing its default key mapping (e.g., delete or set its key to undefined/empty
for mac/windows/linux) or assign it a different non-conflicting shortcut, and
keep OPEN_DIFF_VIEWER mapped to "meta+shift+l"/"ctrl+shift+alt+l"; locate and
change the TOGGLE_EXPAND_SIDEBAR entry in the hotkey registry to apply this fix.

---

Nitpick comments:
In `@apps/desktop/src/main/lib/terminal/env.ts`:
- Line 459: The call to getLocale currently passes rawBaseEnv but should use the
allowlisted baseEnv to be consistent with buildTerminalEnv's other uses; update
the invocation so getLocale(baseEnv) is used instead of getLocale(rawBaseEnv),
ensuring locale is derived from the terminal-exposed environment, and then
verify any dependent logic in buildTerminalEnv and getProcessEnvSnapshot still
works (and consider cleaning up getProcessEnvSnapshot's public shape if raw is
no longer needed).
- Around line 163-170: The function hasMacosSystemCertBundle uses a
process-lifetime cached value cachedMacosSystemCertAvailable without TTL or
invalidation; add a one-line comment above the cachedMacosSystemCertAvailable
usage (and/or above hasMacosSystemCertBundle) stating this is an intentional
process-persistent cache and not expected to change during runtime, and
reference that tests can reset it via resetTerminalEnvCachesForTests if needed
so future readers know why no TTL/invalidation is present.
- Line 485: The delete of terminalEnv.GOOGLE_API_KEY is effectively dead
(baseEnv is already allowlisted by buildSafeEnv using
ALLOWED_ENV_VARS/ALLOWED_PREFIXES, getShellEnv only returns
SUPERSET_ORIG_ZDOTDIR/ZDOTDIR, and terminalEnv keys are literal), so remove the
statement delete terminalEnv.GOOGLE_API_KEY; from env.ts; if you prefer to keep
a defensive guard instead, replace it with a short comment on terminalEnv
explaining that any explicit deletion is only a final-defense against accidental
future additions to the allowlist (referencing buildSafeEnv, ALLOWED_ENV_VARS,
ALLOWED_PREFIXES, and getShellEnv) so readers understand the intent.

In `@apps/desktop/src/main/lib/terminal/port-scanner.ts`:
- Around line 74-90: getListeningPortsForPids now accepts an optional
AbortSignal but does not propagate it to getProcessName/getProcessCommand; add
an optional signal?: AbortSignal parameter to the public helpers getProcessName
and getProcessCommand and thread that signal through any internal calls (e.g.,
from getListeningPortsForPids -> getListeningPortsLsof/getListeningPortsWindows
-> places that call getProcessName/getProcessCommand) so callers can cancel the
entire flow; update function signatures and call sites to pass the signal along
and ensure any spawned async operations respect the signal.
🪄 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: 867d97b2-6561-4460-b3ab-e6f2f5c45ee2

📥 Commits

Reviewing files that changed from the base of the PR and between 61db020 and 9eeeb63.

📒 Files selected for processing (4)
  • apps/desktop/src/main/lib/terminal/env.ts
  • apps/desktop/src/main/lib/terminal/port-scanner.ts
  • apps/desktop/src/renderer/hotkeys/registry.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx

Comment thread apps/desktop/src/main/lib/terminal/port-scanner.ts
git-blame.ts が参照しているキャッシュ機能が欠落していた。
main既存のビルドエラー。
- DashboardSidebarWorkspaceItem: hostIsOnline / onCopyBranchName / branch を
  子と hook に渡すように修正。子コンポーネントが upstream 仕様で required
  になっていた 3 つの prop を親が destructuring / 伝播し忘れていたため。
- host-service/listBranches: feat(desktop): VS Code-style branch sort order
  (184c061) で追加された sortOrder / pinDefault 入力、および remote-only
  branch 追加 + pin default 処理を復元。upstream superset-sh#3543 取り込み時に
  git.ts が上書きされて fork 実装が失われていた。API は現在の
  resolveBaseComparison / buildBranch に合わせて調整。
upstream PR superset-sh#3556 で追加された OPEN_DIFF_VIEWER と fork 既存の
TOGGLE_EXPAND_SIDEBAR が同じ ⌘⇧L に binding され、V1 workspace の
page.tsx で両方が useHotkey 登録されて発火順が不定になっていた。
加えて RightSidebar のサイドバー展開ボタンが誤って OPEN_DIFF_VIEWER
のラベルを表示していた。

- V1 page.tsx: OPEN_DIFF_VIEWER の useHotkey 登録を削除
  (ハンドラは SEARCH_IN_FILES と完全に同一で冗長だった)
- RightSidebar: HotkeyLabel id を TOGGLE_EXPAND_SIDEBAR に戻す
- V2 は OPEN_DIFF_VIEWER を引き続き使用 (upstream 意図を維持)

V1/V2 は別ルートなので useHotkey の enabled で自然に分岐し、
同じキーで異なる動作をしても競合しない。
`getProcessName` / `getProcessCommand` lost their platform branching during
the upstream merge, so on Windows they always ENOENT on `ps` and fall back to
`unknown` / `""`. pane-resolver (browser MCP) and the browser-automation
router call these without their own platform check, meaning terminal pane
resolution and browser process identification silently break on Windows.

Re-introduce the `os.platform() === "win32"` branches that delegate to
`getProcessNameWindows` and a new `getProcessCommandWindows` (wmic first,
PowerShell `Get-CimInstance` fallback — same shape as the existing name
helper).

Refs: CodeRabbit on PR #388 (comment 3132095994).
20260421-pr-detection-matcher-fix.md は既に shipped 済み。
AGENTS.md の plan 配置ルール (shipped plans move to plans/done/) に従って移動。
`isOpenAIApiKey` accepted any key starting with `sk-`, which matches
Anthropic keys (`sk-ant-…`) as well. When the Anthropic resolution path
didn't return a model (e.g. only an OAuth credential present on the
Anthropic slot) the fallback scanned auth storage for OpenAI keys and
could pick up the Anthropic key, sending it to the OpenAI API and
producing 401s.

Exclude the `sk-ant-` prefix explicitly.

Refs: CodeRabbit on PR #388 (comment 3128967779).
upstream commit 3902e3b (superset-sh#3621) が layout.tsx から MainWindowEffects wrapper
を外して AgentHooks / ScheduleFireToasts / WorkspaceInitEffects を直接 mount
するように変更していた。この変更は fork の tearoff window 前提を壊す:
tearoff でも authenticated tree が描画されるため、これら singleton エフェクト
(useDevicePresence / useCommandWatcher / tRPC subscription / workspace init
subscription) が main window と tearoff で二重に走り、agent orchestration
および workspace init が多重起動するリスクがある。

MainWindowEffects コンポーネント自体はリポジトリに残っており
(isTearoffWindow + window.shouldOwnSingletonEffects でガードする実装)、
単に layout.tsx の呼び出しだけが置き換えられていた。直接 mount を
<MainWindowEffects /> に戻すことで upstream 変更を巻き戻す。

Refs: CodeRabbit on PR #388 (Major).
61db020 で agent_preset_permissions_migrated_at migration を手動追加した際、
tag prefix を 0040_ のままにしてしまい、_journal.json / SQL ファイル名と
idx の対応が崩れていた (idx 69 が 0040_, idx 70 が 0069_ を指す状態)。
さらに upstream 5aaeb0f が 0040_snapshot.json を 0040_agent_preset_...
のスナップショットで上書きしていたため、0040 と 0044 が同じ prevId を
指す collision も発生していた。

- _journal.json を v1.5.5-fork.14 state に巻き戻し
  (idx 68=0068_add_agent_kind_codex_fields, idx 69=0069_add_crush_model)
- 0040_snapshot.json を v1.5.5-fork.14 state (illegal_squadron_supreme 時点)
  に巻き戻し
- 壊れた 0040_agent_preset_permissions_migrated_at.sql を削除
- schema は現状維持 (agentPresetPermissionsMigratedAt カラム定義あり)
- bunx drizzle-kit generate --name="agent_preset_permissions_migrated_at"
  で正しく idx 70 として 0070_agent_preset_permissions_migrated_at を再生成

v1.5.5-fork.14 には agent_preset_permissions_migrated_at 自体が含まれない
(本 PR の upstream 取り込みで初めて入る) ため、既存配布ユーザーへの
破壊的変更はない。
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants