Repository navigation
Revert "fix: solve #3028 — forward DA1 query responses during shell init" - #3127
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request removes tests validating headless emulator response forwarding during shell initialization and modifies the session logic to skip forwarding when shell readiness is "pending". Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
✨ 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! 🎉 |
Greptile SummaryThis PR reverts #3030, which had forwarded headless-emulator terminal query responses (DA1, DSR) to the subprocess during shell init. The revert adds an early-return guard in the Key concerns:
Confidence Score: 2/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Fish shell starts\nSends DA1 query ESC-c] --> B{attachedClients.size === 0?}
B -- No --> C[Renderer xterm handles DA1\nForwards response to shell]
B -- Yes --> D[HeadlessEmulator processes DA1\nGenerates response via onData]
D --> E{shellReadyState === pending?\nNEW GUARD added by this revert}
E -- Yes --> F[🚫 Response DROPPED\nFish waits up to 10s for reply\nIssue 3028 reintroduced]
E -- No --> G[sendWriteToSubprocess\nResponse forwarded to shell]
F --> H[After 10s timeout:\nFish disables cursor-shape\nand reflow detection]
Reviews (1): Last reviewed commit: "Revert "fix: solve #3028 — forward DA1 q..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // The headless emulator responds to terminal queries (e.g. DA) | ||
| // when no renderer client is attached. During shell init we drop | ||
| // these — they'd land in the pre-ready stdin queue and appear as | ||
| // typed text like "?62;4;9;22c" once flushed. After a client | ||
| // attaches the renderer's xterm handles all terminal queries. |
There was a problem hiding this comment.
sendWriteToSubprocess bypasses the preReadyStdinQueue
The comment justifies dropping headless-emulator responses during pending by saying they'd "land in the pre-ready stdin queue and appear as typed text like ?62;4;9;22c once flushed." This is factually incorrect.
The onData callback calls this.sendWriteToSubprocess(data) directly, not this.write(data). Only write() routes through preReadyStdinQueue. The emulator's response would be sent straight to the subprocess's stdin — it would never touch the pre-ready queue. If the real motivation is something else (e.g. the headless emulator generates an incorrect or incomplete DA1 response that confuses fish/zsh), the comment should say so.
| this.subprocess && | ||
| this.subprocessReady | ||
| ) { | ||
| if (this.shellReadyState === "pending") return; |
There was a problem hiding this comment.
By returning early when shellReadyState === "pending", this change drops all headless-emulator responses (DA1, DSR, etc.) during the critical shell-init window. Fish shell sends a DA1 query (\x1b[c) at startup and waits up to 10 seconds for a response before disabling cursor-shape changes and reflow detection.
The SHELL_READY_TIMEOUT_MS is 15 seconds, so when a session starts without a renderer client attached, fish's 10-second DA1 timeout will always fire before the shell-ready marker arrives, degrading the shell experience every single headless startup.
The original PR #3030 was specifically introduced to fix this. If this revert is needed because #3030 introduced a separate regression, that regression should be documented here (and ideally addressed separately) so the trade-off is clear.
…g shell init (ParadevAI#3030)" (ParadevAI#3127) This reverts commit 3194568.
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。 取り込み済み (cherry-pick / 手動移植): - be22b46 ParadevAI#3125 — スキップ (下記参照) - 88bc7fb ParadevAI#3127 — Revert DA1 ✓ - 92d0ff9 ParadevAI#3054 — DA1 fix ✓ - c48450e ParadevAI#3093 — file viewer pane fix ✓ - fffa8db ParadevAI#3128 — version 1.4.7 ✓ - 589a7c7 ParadevAI#3136 — fuzzy scorer (ハイブリッド方式) ✓ - ceb8c81 ParadevAI#3150 — Electron 40.8.5 ✓ - 8922b94 ParadevAI#3137 — terminalId分離 ✓ - c7508e5 ParadevAI#3152 — GitHub無料化 ✓ - 2b91f11 ParadevAI#3155 — v2 terminal theme ✓ - b8b11af ParadevAI#3154 — TUI dimension fix ✓ - 7599ace ParadevAI#3149 — v2 sidebar file tree (手動統合) ✓ - 4d7c612 ParadevAI#3174 — DnD重複削除 ✓ - 864977d ParadevAI#3157 — Host Service分離 ✓ 意図的にスキップ: - be22b46 ParadevAI#3125 (GitHub polling簡素化) フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と 設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と github-sync-service.ts のFORK NOTEを参照。 ゴースト・マージ復元 (revert 134cfd5 で消失した内容): - 538f306 ParadevAI#3120 — Patch vuln ✓ - 1588d20 ParadevAI#3108 — terminal lifecycle分離 ✓ - 59426f6 ParadevAI#3122 — file tree + FilePane + Alert refactor (手動統合) ✓ - 10d9a5d ParadevAI#3097 — tiptap line-height ✓ - 337a9ae ParadevAI#3121 — Codex hooks削除 ✓
Reverts #3030
Summary by cubic
Reverts the change that forwarded headless emulator DA/DSR responses during shell init, restoring the previous behavior of dropping them while the shell is pending. This avoids queued query replies appearing as typed text; once a client attaches, the renderer handles terminal queries.
Written for commit fd4d431. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests