upstream/2026-04-26 PR5: 追加 4 commits 取り込み (terminal + automation)#440
Merged
upstream/2026-04-26 PR5: 追加 4 commits 取り込み (terminal + automation)#440
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughこのPRは、ターミナルのタイトル検出・同期機能とレンダリング改善を追加します。WebSocketを通じてサーバーから送信されるターミナルタイトルをクライアント側で受信・表示し、オートメーションの日付表示をタイムゾーン対応のフォーマットに統一します。フォント読み込み後のリフィット機構も実装されます。 Changes
Sequence DiagramsequenceDiagram
participant Host as ホストサービス
participant WS as WebSocket
participant Transport as TerminalTransport
participant Registry as RuntimeRegistry
participant UI as TerminalSessionDropdown
Host->>Host: PTYデータを受信
Host->>Host: scanForTerminalTitle()でタイトルを検出
alt タイトル変更検出
Host->>WS: { type: "title", title: "New Title" }
end
WS->>Transport: titleメッセージを受信
Transport->>Transport: transport.title = "New Title"
Transport->>Transport: titleListenersコールバック実行
Registry->>Registry: onTitleChange()で変更を監視
UI->>Registry: useSyncExternalStoreでタイトル購読
Registry-->>UI: "New Title"を返す
UI->>UI: runtimeTitleを更新しトリガーをレンダリング
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate 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! 🎉 |
Drop the timestamp from the trigger title, widen the trigger and menu, push the chevron to the trailing edge, and unify dropdown text sizes.
* Fix automation timezone scheduling * Stabilize automation timezone formatting tests
* Add terminal session titles * Use renderer xterm for terminal titles * Avoid terminal title echo * Address terminal title review feedback * Fix replay terminal title recovery * Avoid sending cached terminal title on reconnect * Match ConEmu OSC 9 title parsing * Clear stale title cache before replay * Revert terminal naming implementation * Implement terminal session naming * Instrument terminal title updates * Make terminal title logs unconditional * Prefer live terminal titles in dropdown * Remove obsolete terminal naming handoff * Address terminal title review comments * Reduce terminal title update overhead * Remove terminal title debug logs
MocA-Love
force-pushed
the
upstream/batch-2026-04-26-pr-e
branch
from
6e0ca12 to
6e625f1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/shared/src/rrule.test.ts (1)
14-29: 任意：hourの部分一致アサーションが緩すぎる可能性。
expectDateTimePartsはtoContainで個別パートをチェックしていますが、hour: "6"のような一桁文字列はyear: "2026"の中にも含まれるため、フォーマッタが時刻部分で別の時間を吐いてもテストは通ってしまいます（toISOString()のアサーションでUTC瞬間はピン留めされているので致命傷ではありませんが、表示崩れには気付けません）。時刻周辺の区切り文字を含めて照合すると、ローカル表示のリグレッションを確実に拾えます。
♻️ 提案：時刻文字列を区切り込みで突き合わせる
- expectDateTimeParts( - formatDateTimeInTimezone(next, "America/Los_Angeles", { - locale: "en-US", - }), - { - month: "Apr", - day: "25", - year: "2026", - hour: "6", - minute: "00", - dayPeriod: "AM", - timeZoneName: "PDT", - }, - ); + const formatted = formatDateTimeInTimezone(next, "America/Los_Angeles", { + locale: "en-US", + }); + expect(formatted).toContain("Apr 25, 2026"); + expect(formatted).toContain("6:00 AM"); + expect(formatted).toContain("PDT");
nextOccurrences側の3件の比較も同じ要領で"6:00 AM"単位で揃えると、サマータイム切り替わり後のローカル時刻保持を1行で表現できます。Also applies to: 304-329
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/rrule.test.ts` around lines 14 - 29, The current expectDateTimeParts helper uses toContain on individual parts so short hour strings like "6" can falsely match other fields (e.g., "2026"); update expectDateTimeParts to assert the hour using surrounding delimiters (e.g., include the separator or full time token like "6:00" or "6:00 AM") instead of raw "6" and keep other part checks strict, and apply the same delimiter-aware matching approach to the nextOccurrences comparisons referenced in the tests (the three comparisons around nextOccurrences) so local time formatting regressions are reliably caught.apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts (1)
400-402:string | null | undefinedのトライナリ意味論を JSDoc で残しておくと親切
undefined（transport 未受信／切断後）とnull（サーバが明示的にクリア）を消費側 (TerminalSessionDropdownのruntimeTitle !== undefined判定) で区別しているため、戻り値の意味を簡潔にコメントで残しておくと、今後この API を使うコードが trinary を誤って null 一本化してしまう事故を防げます。📝 ドキュメント追加例
+ /** + * Returns the current terminal title from the WS transport. + * - `undefined`: transport has not received a title message yet + * (initial state or post-disconnect). + * - `null`: server explicitly cleared the title (e.g. ConEmu reset). + * - `string`: current title. + */ getTitle(terminalId: string, instanceId?: string): string | null | undefined { return this.getEntry(terminalId, instanceId)?.transport.title; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts` around lines 400 - 402, Add a brief JSDoc to getTitle explaining the trinary return semantics (undefined = transport not yet received or connection dropped, null = server explicitly cleared the title, string = actual title) so callers (e.g., TerminalSessionDropdown checks runtimeTitle !== undefined) can reliably distinguish undefined vs null; update the comment above getTitle(terminalId: string, instanceId?: string) to document these three cases and reference that it returns this.getEntry(... )?.transport.title.packages/shared/src/terminal-title-scanner.ts (2)
89-104: OSC 0/2 が空ペイロードの場合null(reset) として扱われる挙動を JSDoc に追記推奨。
parseTitlePayloadで OSC 0;/OSC 2; のペイロードが空文字列のとき、normalizeTerminalTitleがnullを返すため、結果としてタイトル reset 扱いになります。一方、上の JSDoc では reset を「OSC 9;3; BEL/ST」のみと記載しているため、読み手の期待と乖離する可能性があります。挙動自体は妥当（多くの端末で空タイトル = デフォルト復帰）と思われるので、ドキュメントに 1 行追記しておくのが親切です。📝 提案差分
/** * Scan PTY output for terminal title OSC sequences. * * Supported sequences: * - OSC 0;<title> BEL/ST * - OSC 2;<title> BEL/ST * - OSC 9;3;<title> BEL/ST (ConEmu tab title) * - OSC 9;3; BEL/ST reset * + * Empty/whitespace-only or all-control-character titles for OSC 0/2 are + * normalized to `null` and treated as a reset by consumers. + * * OSC may be encoded as ESC ] or the single-byte C1 introducer. * ST may be encoded as ESC \ or the single-byte C1 terminator. */Also applies to: 106-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/terminal-title-scanner.ts` around lines 89 - 104, Update the JSDoc for parseTitlePayload to document that OSC 0 and OSC 2 with an empty payload are treated as a title reset (i.e., parsed to null via normalizeTerminalTitle); mention that reset behavior is already explicitly documented for OSC 9;3; BEL/ST and now also applies to empty OSC 0/2 payloads. Reference parseTitlePayload and normalizeTerminalTitle in the doc so readers can find the implementation and the function that returns null for empty titles; apply the same JSDoc note to the analogous block around lines handling OSC 9 parsing (the second occurrence noted in the comment).
42-57:getUtf8ByteLengthは標準のTextEncoderで置換可能です（任意）。実装は正しいですが、
TextEncoder().encode(value).lengthの方が標準ライブラリに依存でき、サロゲート/結合文字の扱いも明示的になります。ホットパスではないため必須ではありませんが、保守性の観点で置き換えを検討してもよいでしょう。♻️ 提案差分
-function getUtf8ByteLength(value: string): number { - let bytes = 0; - for (const char of value) { - const codePoint = char.codePointAt(0) ?? 0; - if (codePoint <= 0x7f) { - bytes += 1; - } else if (codePoint <= 0x7ff) { - bytes += 2; - } else if (codePoint <= 0xffff) { - bytes += 3; - } else { - bytes += 4; - } - } - return bytes; -} +const utf8Encoder = new TextEncoder(); +function getUtf8ByteLength(value: string): number { + return utf8Encoder.encode(value).length; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/terminal-title-scanner.ts` around lines 42 - 57, Replace the manual UTF‑8 length calculation in getUtf8ByteLength with the standard TextEncoder path: have getUtf8ByteLength(value: string) return the byte length computed by new TextEncoder().encode(value).length so the function uses the standard library encoder (handling surrogates/combining characters) instead of the custom loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/automations/update/command.ts`:
- Line 86: The update path currently assigns targetHostId: options.device which
leaves CLI users unable to clear the host (null) because create uses
options.device ?? null; change the update behavior to treat an explicit empty
string as intent to clear by setting targetHostId: options.device === "" ? null
: options.device (or alternatively support a new --clear-device boolean flag and
set targetHostId: null when that flag is present) so the update handler still
receives undefined for "no change", string for a set value, and null for an
explicit clear; update any parsing/option definitions accordingly where
options.device is defined.
---
Nitpick comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts`:
- Around line 400-402: Add a brief JSDoc to getTitle explaining the trinary
return semantics (undefined = transport not yet received or connection dropped,
null = server explicitly cleared the title, string = actual title) so callers
(e.g., TerminalSessionDropdown checks runtimeTitle !== undefined) can reliably
distinguish undefined vs null; update the comment above getTitle(terminalId:
string, instanceId?: string) to document these three cases and reference that it
returns this.getEntry(... )?.transport.title.
In `@packages/shared/src/rrule.test.ts`:
- Around line 14-29: The current expectDateTimeParts helper uses toContain on
individual parts so short hour strings like "6" can falsely match other fields
(e.g., "2026"); update expectDateTimeParts to assert the hour using surrounding
delimiters (e.g., include the separator or full time token like "6:00" or "6:00
AM") instead of raw "6" and keep other part checks strict, and apply the same
delimiter-aware matching approach to the nextOccurrences comparisons referenced
in the tests (the three comparisons around nextOccurrences) so local time
formatting regressions are reliably caught.
In `@packages/shared/src/terminal-title-scanner.ts`:
- Around line 89-104: Update the JSDoc for parseTitlePayload to document that
OSC 0 and OSC 2 with an empty payload are treated as a title reset (i.e., parsed
to null via normalizeTerminalTitle); mention that reset behavior is already
explicitly documented for OSC 9;3; BEL/ST and now also applies to empty OSC 0/2
payloads. Reference parseTitlePayload and normalizeTerminalTitle in the doc so
readers can find the implementation and the function that returns null for empty
titles; apply the same JSDoc note to the analogous block around lines handling
OSC 9 parsing (the second occurrence noted in the comment).
- Around line 42-57: Replace the manual UTF‑8 length calculation in
getUtf8ByteLength with the standard TextEncoder path: have
getUtf8ByteLength(value: string) return the byte length computed by new
TextEncoder().encode(value).length so the function uses the standard library
encoder (handling surrogates/combining characters) instead of the custom loop.
🪄 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: ab4e8ea2-5ad1-48f5-8655-12d317b3adf2
📒 Files selected for processing (19)
apps/desktop/src/renderer/lib/terminal/terminal-addons.tsapps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/lib/terminal/terminal-runtime.tsapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/automations/$automationId/components/AutomationDetailSidebar/AutomationDetailSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxpackages/cli/src/commands/automations/create/command.tspackages/cli/src/commands/automations/format.tspackages/cli/src/commands/automations/list/command.tspackages/cli/src/commands/automations/resume/command.tspackages/cli/src/commands/automations/update/command.tspackages/host-service/src/terminal/terminal.tspackages/shared/package.jsonpackages/shared/src/rrule.test.tspackages/shared/src/rrule.tspackages/shared/src/terminal-title-scanner.test.tspackages/shared/src/terminal-title-scanner.tspackages/trpc/src/router/automation/schema.tsplans/20260425-v2-terminal-rendering-divergences.md
| dtstart: options.dtstart ? new Date(options.dtstart) : undefined, | ||
| agentConfig, | ||
| targetHostId: options.device ?? null, | ||
| targetHostId: options.device, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# automation.update 入力スキーマ上の targetHostId の取り扱いを確認
fd -t f 'schema.ts' packages/trpc | xargs rg -nP -C3 'targetHostId'
# サーバ側ハンドラでの undefined / null 分岐の有無を確認
rg -nP -C5 '\btargetHostId\b' --type=ts -g '!**/node_modules/**'Repository: MocA-Love/superset
Length of output: 34529
undefined の透過で一貫性は改善されましたが、デバイスをクリアできなくなった点を確認してください。
options.device をそのまま渡すことで、--device 未指定時に targetHostId を更新しない挙動となり、name/prompt などの他の部分更新フィールドと整合します。スキーマは targetHostId: z.string().uuid().nullish() として undefined | null | string を受け付け、サーバ側の update ハンドラも input.targetHostId === undefined ? existing.targetHostId : input.targetHostId として undefined を「変更なし」に扱います。
ただし、create コマンドが options.device ?? null で null に変換するのに対して、update コマンドは undefined のままのため、CLI から targetHostId を null（クリア）に設定する手段が失われています。運用上でホスト指定を解除する必要がある場合は、--device "" や --clear-device フラグの追加を検討してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/automations/update/command.ts` at line 86, The
update path currently assigns targetHostId: options.device which leaves CLI
users unable to clear the host (null) because create uses options.device ??
null; change the update behavior to treat an explicit empty string as intent to
clear by setting targetHostId: options.device === "" ? null : options.device (or
alternatively support a new --clear-device boolean flag and set targetHostId:
null when that flag is present) so the update handler still receives undefined
for "no change", string for a set value, and null for an explicit clear; update
any parsing/option definitions accordingly where options.device is defined.
MocA-Love
added a commit
that referenced
this pull request
Apr 26, 2026
PR1〜PR5 (#435 #436 #437 #438 #440) で 13 commits 全件 cherry-pick + 手動 conflict 解消で取り込み済み。 本コミットは git 履歴上 behind=0 とするための ours マージ記録。 取り込み済み 13 commits: - 1f55c62 Fix host service restart adoption (superset-sh#3732) - 0fe65d2 test(desktop): remove host-service-coordinator test (superset-sh#3734) - 3012b5a Add optimistic Electric collection updates (superset-sh#3722) - c272a51 fix(desktop): drop branch row from v2 sidebar workspace item (superset-sh#3733) - c2f3fdc feat(desktop): add fade-edge mask utilities (superset-sh#3735) - 682d07c fix v2 terminal osc links (superset-sh#3736) - 7c0d22b feat(ports): surface remote host-service ports in the sidebar (superset-sh#3676) - 6a3be2d [codex] Stabilize v2 terminal resize (superset-sh#3739) - 8928ac6 [codex] Improve v2 pane header responsiveness (superset-sh#3737) - 5fe3d22 refactor(desktop): tidy v2 terminal session dropdown (superset-sh#3743) - 66c23d6 Fix automation timezone scheduling (superset-sh#3738) - 16e270c [codex] Add terminal session titles (superset-sh#3740) - 583fa5d fix(desktop): refit v2 terminal after font settle (superset-sh#3742)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
upstream (superset-sh/superset) の追加 4 commits を取り込む PR5。前バッチ (PR1〜PR4) のマージ後に upstream に追加された v2 terminal 系 3 件と automation timezone 1 件をまとめて取り込む。base は PR4 (
upstream/batch-2026-04-26-pr-d) なので、PR4 マージ後の main から見ると差分は 4 commits + 1 conflict resolution のみ。取り込み内容
5fe3d225166c23d62c16e270c63packages/shared/src/terminal-title-scanner.ts新規。583fa5d9bonRendererChangecallback 追加。Fork 側のコンフリクト解決
terminal-addons.ts(583fa5d)installRectangleRendererAlphaPatch(webglAddon)（vibrancy 対応の必須 patch、xterm WebGL renderer の explicit-bg cells に alpha を適用）options.onRendererChange?.()呼び出し追加（renderer 切替通知）onRendererChangeを patch 直後に呼び出し。TerminalSessionDropdown.tsx (16e270c、merge-tree が予測した conflict)
cherry-pick実行時には auto-merge 成功（PR4 head 基準で 3-way merge が成立）。手動解消不要。Fork 固有機能ヘルスチェック
installRectangleRendererAlphaPatch) 完全保持検証
bun install: ✅ (5757 packages, 55s)bun run typecheck: ✅ (28/28 successful)bun run lint: ✅ (Biome: 4371 files, no fixes applied)bun run --filter @superset/desktop compile:app: ✅ (electron-vite build, 3m 20s, exit 0)upstream 取り込み状況
このマージで
behind upstream/main = 0に到達。Test plan
installRectangleRendererAlphaPatch健在性確認）Summary by CodeRabbit
リリースノート
新機能
バグ修正
テスト