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 50 minutes and 36 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 (6)
✨ 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 |
![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: f2486d5b26
ℹ️ 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".
| }, | ||
| ); | ||
| launchPresetCommands(launches); | ||
| launchPresetCommands(launches, { waitForMountedSession: true }); |
There was a problem hiding this comment.
Passing waitForMountedSession: true here makes launchCommandInPane skip createOrAttach and wait for markTerminalSessionReady, which is only emitted by useTerminalLifecycle after the pane is mounted. That works for the currently rendered tab, but openPreset(..., { target: "active-tab" }) is also invoked from background setup flows (e.g. workspace init), where the workspace tab can exist without being mounted; in that case the promise never resolves and the preset command is never sent. Please keep helper-side attach for these active-tab preset paths unless visibility/mount is guaranteed.
Useful? React with 👍 / 👎.
MocA-Love
changed the title
MocA-Love
changed the title
* fix v1 split pane startup sizing * Handle background
MocA-Love
force-pushed
the
upstream-merge/pr4-terminal-session-readiness
branch
from
f2486d5 to
2656d14
Compare
All 9 upstream commits have been individually cherry-picked via PR#159~#163: | Upstream | Our PR | Description | |---|---|---| | d656b7e (superset-sh#3415) | #159 (PR#1) | terminal clipboard handling | | 31fcf19 (superset-sh#3416) | #162 (PR#4) | v1 split pane startup sizing fix | | 039edf2 (superset-sh#3403) | #161 (PR#3) | Cmd+Alt+Arrow spatial pane focus | | b18a00c (superset-sh#3421) | #159 (PR#1) | v2 right sidebar toggle reactive | | 3dd1de2 (superset-sh#3420) | #161 (PR#3) | v2 diff viewer + tab title resolution | | b42a114 (superset-sh#3418) | #159 (PR#1) | CodeMirror hotkey enablement | | c925f4d (superset-sh#3422) | #160 (PR#2) | unbound defaults + restore prev/next tab/workspace | | bb12c09 (superset-sh#3419) | #163 (PR#5) | version bump 1.5.3 | | 47efa73 (superset-sh#3432) | #159 (PR#1) | pending/update-required error selectable | Fork-specific features preserved: - auto-updater (IS_FORK, GitHub Releases API) - QuitMode/cleanupMainWindowResources lifecycle - GitHubSyncService, SpreadsheetViewer - BROWSER_RELOAD / BROWSER_HARD_RELOAD / SEARCH_IN_FILES hotkeys - HotkeyCategory "Browser" - v1 deep-link navigation (useSearch/WorkspaceSearchParams) - v1 tRPC-based PREV/NEXT_WORKSPACE handlers - v1 CLOSE_TERMINAL/CLOSE_TAB hotkey handlers - v2 extra state (rightSidebarOpenViewWidth, showPresetsBar)
2 participants
Upstream Merge PR#4 - Terminal Session Readiness
upstream superset-sh#3416 を cherry-pick し、v1 split pane での preset コマンド実行バグを修正します。
取り込むコミット
改善内容の詳細
何が問題だったか
v1 split pane でプリセットコマンドが正しく実行されない：
背景タブで pane を作る場合は「helper-side attach」経由なので影響なし。フォアグラウンドの split 操作でのみ 発生する race condition でした。
どう直したか
新しい "Session Readiness" 機構を導入：
新規ファイル `lib/terminal/session-readiness.ts`
```typescript
// paneId ごとに「準備完了フラグ」と「待機中の Promise」を管理する module-level state
export function waitForTerminalSessionReady(paneId: string): Promise
// paneId がまだ ready でなければ Promise を返して待つ
// ready なら即座に resolve
export function markTerminalSessionReady(paneId: string): void
// paneId を ready 状態に、待機中の Promise を全て resolve
export function rejectTerminalSessionReady(paneId: string, error: Error): void
// 待機中の Promise を全て reject（attach 失敗時や pane close 時）
export function clearTerminalSessionReady(paneId: string): void
// ready 状態をクリア（再 attach 時）
```
`launch-command.ts` に `waitForMountedSession` オプション追加
```typescript
interface LaunchCommandInPaneOptions {
// ... 既存の項目 ...
/**
*/
waitForMountedSession?: boolean;
}
async function launchCommandInPane(options) {
if (options.waitForMountedSession) {
// session が ready になるまで待機
await waitForTerminalSessionReady(paneId);
await writeCommandInPane({ paneId, command, write, noExecute });
return;
}
// 従来のパス（helper-side attach）
// ...
}
```
`useTerminalLifecycle.ts` で readiness シグナルを発火
Terminal の attach ライフサイクルの適切なタイミングでシグナルを送る：
`useTabsWithPresets.ts` で split 操作時に `waitForMountedSession: true` を渡す
```typescript
// split 操作時の preset launch
launchPresetCommands(launches, { waitForMountedSession: true });
```
`launchPresetCommand()`, `launchPresetCommands()`, `launchFirstPresetInPane()`, `launchFirstPresetInFocusedPane()` に `options?: { waitForMountedSession?: boolean }` パラメータを追加。
`terminal-cleanup.ts` でクリーンアップ
```typescript
export const killTerminalForPane = (paneId: string): void => {
// pane が close されたときに、まだ待機中の Promise を reject
rejectTerminalSessionReady(
paneId,
new Error("Terminal pane was closed before the session became ready"),
);
electronTrpcClient.terminal.kill.mutate({ paneId }).catch(...);
};
```
これで、ユーザーが session ready 前に pane を close しても、待機中の Promise がリークせず適切に reject されます。
変更ファイル
コンフリクト解決
フォーク固有機能の保持
ユーザーへのメリット
v1 workspace での split 操作が確実に動作
リソースリーク防止
テスト結果
手動テスト計画