fix: resolve seven review findings across ipc, input, grid, and session restore - #70
Conversation
…on restore - ipc: raise client frame bound to the message bound so a default-sized getText response is not rejected on receipt - ipc: validate #rrggbb as ASCII hex before slicing (non-ASCII panicked) - input: honor modifyOtherKeys=2 (CSI 27;mods;cp~) in the legacy encoder - input: keep the Alt ESC prefix on Ctrl+Alt C0 bytes - grid: DECSTR clears IRM; RIS keeps the configured default cursor style - app: save/restore each tab group's selected tab, not only the focused one Claude-Session: https://claude.ai/code/session_01GBe292wmocEXM8SjBzjGq7
…t it on RIS
- modifyOtherKeys=2 now encodes the logical (Shift/layout-translated) key,
matching xterm: Ctrl+Shift+1 on US reports 33 ('!'), not 49
- RIS (ESC c) clears the modifyOtherKeys=2 flag alongside kitty keyboard state
- tests: shifted symbols/layout characters and full-reset restoring legacy bytes
Claude-Session: https://claude.ai/code/session_01GBe292wmocEXM8SjBzjGq7
![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: 621b3154d4
ℹ️ 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".
| continue; | ||
| } | ||
| if let Some(state) = group.first().and_then(|id| self.windows.get(id)) { | ||
| state.window.select_tab_at_index(saved.focused_tab); |
There was a problem hiding this comment.
When restoring on macOS, if a tab before the saved selection fails to spawn, the loop continues and group contains only the compacted successful tabs, while saved.focused_tab still indexes the original saved list. Passing that stale index here therefore selects a later tab—or skips selection when it is now out of bounds—so background groups can restore the wrong selected tab. Preserve each successful tab's original index or remap the saved index before calling this method.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
Summary
getTextresponse is not rejected; validate#rrggbbas ASCII hex before slicing (non-ASCII panicked)CSI 27;mods;cp~) in the legacy encoder, reporting the Shift/layout-translated codepoint like xterm; keep the Alt ESC prefix on Ctrl+Alt C0 bytesTest plan
cargo test -p noa-app input(158 passed)cargo test -p noa-grid(604 passed)cargo clippy --workspace— 4 warnings, all pre-existing on mainCSI > 4;2 msendsESC [27;6;33~; session restore keeps per-group selected tabhttps://claude.ai/code/session_01GBe292wmocEXM8SjBzjGq7