Conversation
|
Important Review skippedToo many files! This PR contains 81 files, which is 31 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (82)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change adds configurable hint mode, scripting actions, per-buffer user options, runtime working-directory tracking, session-aware terminal behavior, background notifications, CLI commands, tests, and updated Config API documentation. ChangesCore client behavior
Buffer options and runtime metadata
Scripting, validation, and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
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 |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/embers-client/src/input/modes.rs (1)
115-121: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUpdate test to include
hintsmode.The test
builtin_modes_include_normal_copy_search_and_selectasserts the presence ofnormal,copy,search, andselectmodes but does not check for the newly addedhintsmode. This is a coverage gap now thatHINTS_MODEis registered inbuiltin_modes().✅ Proposed fix
fn builtin_modes_include_normal_copy_search_and_select() { let modes = builtin_modes(); assert!(modes.contains_key("normal")); assert!(modes.contains_key("copy")); assert!(modes.contains_key("search")); assert!(modes.contains_key("select")); + assert!(modes.contains_key("hints")); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/input/modes.rs` around lines 115 - 121, Update the test function builtin_modes_include_normal_copy_search_and_select to also assert that builtin_modes() contains the newly registered "hints" mode, preserving the existing assertions.crates/embers-client/src/configured_client.rs (1)
540-553: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
finish_config_reloaddoesn't clear the new hints overlay state.When the active mode falls back to
NORMAL_MODEon reload,search_promptis explicitly cleared but the newhints_node/per-viewhints_stateintroduced by this PR are not. If a config reload happens whileHINTS_MODEis active,input_stateresets toNORMAL_MODE(so hint key handling no longer runs) while the hint overlay remains attached to the view and keeps rendering until something else clears it — a stuck, non-interactive overlay.🐛 Proposed fix
fn finish_config_reload(&mut self, current_mode: &str) { if self .config .active_script() .loaded_config() .modes .contains_key(current_mode) { self.input_state.clear_pending(); } else { self.input_state.set_mode(NORMAL_MODE); self.search_prompt = None; + if let Some(node_id) = self.hints_node.take() + && let Some(state) = self.client.state_mut().view_state_mut(node_id) + { + state.hints_state = None; + } } }Also applies to: 2236-2312
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/configured_client.rs` around lines 540 - 553, Update finish_config_reload to clear the hints overlay state whenever the active mode is reset to NORMAL_MODE, including removing or resetting hints_node and the per-view hints_state alongside search_prompt. Apply the same cleanup to the corresponding reload-handling logic around the alternate referenced section, using the existing hints state symbols and lifecycle methods.crates/embers-client/src/scripting/runtime.rs (1)
166-170: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winAdd
dynamic_option_stringtodocumented_context_apiimports —hint_selection()calls it here, but this module’suse super::{...}list doesn’t bring it into scope, so this won’t compile as written.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/scripting/runtime.rs` around lines 166 - 170, The documented context API is missing the dynamic_option_string import required by hint_selection(). Add dynamic_option_string to the use super import list in module documented_context_api so the function is in scope and compiles.crates/embers-cli/src/lib.rs (1)
2795-2859: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding CLI parsing coverage for
set-option/show-options.
buffer_subcommands_parse_expected_flags_and_defaultscovershistoryandrevealbut not the newSetOption/ShowOptionsvariants (including the--unset/valueconflict and the missing-value edge case above).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-cli/src/lib.rs` around lines 2795 - 2859, Extend buffer_subcommands_parse_expected_flags_and_defaults to cover SetOption and ShowOptions parsing: verify normal set-option name/value parsing, --unset behavior, rejection when --unset is combined with a value, missing-value rejection, and show-options parsing with its expected arguments/defaults. Assert the parsed BufferCommand variants and fields explicitly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/lib.rs`:
- Around line 295-306: Update the set-option handling associated with the
SetOption command to reject missing values unless unset is true; replace the
value.unwrap_or_default() behavior with explicit runtime validation that returns
a user-facing error when both value and --unset are absent, while preserving
normal setting and unsetting behavior.
- Around line 739-745: Make the `show-options` formatting tab/newline-safe by
JSON-encoding each option key and value before joining them into the
one-option-per-line output. Update the iterator in the response handling block
to use the same escaping approach as `format_buffer_details` for `title` and
`cwd`, while preserving the existing key/value output structure.
In `@crates/embers-client/src/configured_client.rs`:
- Around line 3124-3132: Extract the shared 64-item trimming logic from
push_background_notification and record_notification into an
enforce_notification_cap helper, then call it after each collection is updated.
Preserve the existing behavior of retaining only the newest 64 notifications and
use the helper for both the background sink and self.notifications.
- Around line 2248-2281: Extract the follow-output positioning calculations from
this scan block and the corresponding renderer logic into a shared helper,
including significant-line detection, content-row handling, display offset, and
displayed top-line computation. Update both this code and the renderer to call
the helper, preserving existing behavior and alignment. Use the relevant
renderer positioning function and this block’s visible-lines setup as
integration points.
In `@crates/embers-client/src/hints.rs`:
- Around line 13-15: Update the default hint-patterns doc comment to accurately
describe ordering: matches are primarily resolved by earliest start and longest
length, while pattern order only affects exact ties sharing both start and end
offsets. Locate the pattern declaration and tie-breaking logic around the
match-selection code to keep the documentation consistent.
In `@crates/embers-client/src/renderer.rs`:
- Around line 269-271: Update render_hints_overlay to enforce an upper bound on
relative_row based on the pane’s visible content height, in addition to the
existing top_line check. Before restyle_range and put_char_styled, skip matches
whose relative_row is outside the visible rows so hints cannot overwrite the row
below the pane.
In `@crates/embers-client/src/scripting/engine.rs`:
- Around line 846-874: Align set_patterns documentation and behavior: in
HintsApi::set_patterns, validate each string with regex::Regex::new before
storing it and return a positioned runtime error for invalid regexes; retain the
existing error for non-string elements. Update the doc comment to state that
invalid elements or regex patterns cause an error rather than being skipped.
In `@crates/embers-protocol/src/codec.rs`:
- Around line 3678-3689: Update the SetUserOption branch in the BufferRequest
decoder so user_option_value is validated with required(...) whenever
has_user_option_value() is true, instead of using unwrap_or(""). Preserve the
optional-field behavior when absent, and ensure malformed present-but-missing
values return ProtocolError consistently with user_option_key.
In `@crates/embers-server/src/buffer_runtime.rs`:
- Around line 1140-1147: Release the surface lock before performing the fallback
filesystem lookup in the cwd selection logic. In the code handling reported cwd,
extract or clone the value from surface.router.reported_cwd(), drop(surface),
then call self.pid.and_then(resolve_pid_cwd) only if no reported cwd was
available, preserving the existing OSC 7 preference.
In `@crates/embers-server/src/server.rs`:
- Around line 2805-2812: Update the cwd handling block to track the poller
update’s sequence and only apply cwd changes when the update is current or newer
than the buffer state, while still accepting same-sequence changes; reject stale
lower-sequence updates consistently with the pipe handling logic. Preserve the
existing change detection and render_invalidated behavior in the cwd update
path.
---
Outside diff comments:
In `@crates/embers-cli/src/lib.rs`:
- Around line 2795-2859: Extend
buffer_subcommands_parse_expected_flags_and_defaults to cover SetOption and
ShowOptions parsing: verify normal set-option name/value parsing, --unset
behavior, rejection when --unset is combined with a value, missing-value
rejection, and show-options parsing with its expected arguments/defaults. Assert
the parsed BufferCommand variants and fields explicitly.
In `@crates/embers-client/src/configured_client.rs`:
- Around line 540-553: Update finish_config_reload to clear the hints overlay
state whenever the active mode is reset to NORMAL_MODE, including removing or
resetting hints_node and the per-view hints_state alongside search_prompt. Apply
the same cleanup to the corresponding reload-handling logic around the alternate
referenced section, using the existing hints state symbols and lifecycle
methods.
In `@crates/embers-client/src/input/modes.rs`:
- Around line 115-121: Update the test function
builtin_modes_include_normal_copy_search_and_select to also assert that
builtin_modes() contains the newly registered "hints" mode, preserving the
existing assertions.
In `@crates/embers-client/src/scripting/runtime.rs`:
- Around line 166-170: The documented context API is missing the
dynamic_option_string import required by hint_selection(). Add
dynamic_option_string to the use super import list in module
documented_context_api so the function is in scope and compiles.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4e8e3940-cb15-4acd-9c8f-995c6d854986
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-50ea5e33.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
crates/embers-cli/src/lib.rs (1)
728-766: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
show-optionsjoins two JSON tokens with a plain space, unlike the tab convention used elsewhere in this file.Both key and value are now correctly JSON-encoded (fixing the earlier newline-safety concern), but they're joined with
" "rather than"\t".format_buffer_detailsusesfield\tJSON_VALUEfor exactly this reason — a fixed, unambiguous single-character delimiter. Here, if a user option value JSON-encodes to a string containing a literal space (e.g. a path with spaces), a naive whitespace-splitting consumer of this line-based output could misparse the boundary between key and value, even though a JSON-aware reader would still succeed.💡 Proposed fix: use tab to match the file's existing convention
Ok(response .buffer .user_options .iter() .map(|(key, value)| { - // JSON-encode both fields so a key or value - // containing a space, tab, or newline stays on - // one parseable line (matches format_buffer_details). format!( - "{} {}", + "{}\t{}", serde_json::to_string(key) .expect("user option keys serialize to JSON"), serde_json::to_string(value) .expect("user option values serialize to JSON"), ) }) .collect::<Vec<_>>() .join("\n"))Note:
crates/embers-cli/tests/panes.rsand any other assertions on this exact output format would need updating to match ("\"is-vim\"\t\"1\"").🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-cli/src/lib.rs` around lines 728 - 766, In BufferCommand::ShowOptions, join the JSON-encoded key and value with a tab delimiter instead of a space to match format_buffer_details and preserve an unambiguous field boundary. Update any exact-output assertions, including panes tests, to expect the tab-separated format.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/hints.rs`:
- Around line 52-71: Prevent assign_labels/scan from indexing beyond the
available label set when matches exceed two-character capacity. In assign_labels
and the scan logic, clamp the number of matches processed or otherwise limit
indexing to labels.len(), preserving consistent label assignment without panics;
add a boundary test with count just above 729 to verify safe behavior.
In `@crates/embers-client/tests/e2e.rs`:
- Around line 1584-1658: Extract the duplicated alpha/beta session and
shell-window setup from
terminal_title_follows_session_on_attach_switch_and_rename and
session_switch_actions_move_between_sessions into a shared helper such as
two_sessions_with_shells, accepting the test server and performing the four
run_cli calls; replace both inline setup blocks with calls to that helper.
---
Duplicate comments:
In `@crates/embers-cli/src/lib.rs`:
- Around line 728-766: In BufferCommand::ShowOptions, join the JSON-encoded key
and value with a tab delimiter instead of a space to match format_buffer_details
and preserve an unambiguous field boundary. Update any exact-output assertions,
including panes tests, to expect the tab-separated format.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4531e9db-ef6c-4b54-adf7-d05438e866e1
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 2409-2450: Update the None branch of select_hint so the
copied-hint notification does not include raw hint.text, since
record_notification emits it at WARN level; retain the clipboard enqueue
behavior and use a safe summary such as the notification label or text length
while preserving the user-facing copy confirmation.
- Around line 148-165: Update emit_terminal_title to remove control characters
from the session name before constructing the OSC 2 terminal-title sequence,
while preserving the existing missing-session behavior. Update select_hint so
its WARN log does not include raw hint.text; retain only safe contextual or
non-sensitive metadata.
In `@crates/embers-client/src/scripting/runtime.rs`:
- Around line 186-193: Add dynamic_option_string to the documented_context_api
module’s use super import list alongside dynamic_option_custom, so
hint_selection can resolve its helper and compile successfully.
In `@crates/embers-client/src/state.rs`:
- Line 74: Invalidate or cancel hints_state whenever the underlying buffer
snapshot or visible lines change, so HintsState coordinates and matched text
cannot be reused against different content. Update the existing state-refresh
logic around the hints_state field, preserving hints only when they remain bound
to the current snapshot version.
In `@docs/config-api/action.md`:
- Around line 317-321: Add a blank line between each fenced Rhai example’s
closing ``` and the surrounding </div> in the enter_hints example and the
corresponding examples at the referenced locations, resolving markdownlint MD031
without changing the example content.
In `@docs/config-api/registration-action.md`:
- Around line 317-322: Update the documentation generator in documentation.rs so
the template emitting each Example tabcontent block inserts the required blank
lines around the fenced code block, rather than editing the generated
registration-action.md output directly; preserve the existing enter_hints
example content and formatting.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 34dbf847-40d4-498a-8999-01fe136746ea
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/embers-server/src/server.rs (1)
2805-2945: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for same-sequence cwd updates.
Existing tests updated with
cwd: Noneverify sequence gating for title/pipe but none exercise the new same-sequence cwd acceptance path (previously the site of a major bug). Worth adding a case asserting cwd updates apply whenupdate.sequence == buffer.last_snapshot_seqand are rejected when stale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-server/src/server.rs` around lines 2805 - 2945, Add test coverage for record_buffer_update and its same-sequence cwd handling: assert a cwd change applies when update.sequence equals buffer.last_snapshot_seq, and assert a stale lower-sequence cwd update is ignored. Keep the existing title/pipe sequence-gating assertions and isolate the cwd cases with appropriate update.cwd values.crates/embers-server/src/model.rs (1)
69-85: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPersist
user_optionsin workspace state
PersistedBuffer,persisted_buffer, andrestored_bufferomituser_options, so values set viaset_buffer_user_optionwill be lost on save/load.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-server/src/model.rs` around lines 69 - 85, Update PersistedBuffer, persisted_buffer, and restored_buffer to include Buffer.user_options in workspace serialization and restoration. Ensure set_buffer_user_option values survive save/load by persisting the BTreeMap and reconstructing it when creating a Buffer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 2318-2352: Update handle_hints_key to detect when the hints
overlay has been invalidated: when hints_node no longer resolves to a view state
with hints_state, cancel or clear the hints overlay and switch input_state to
NORMAL_MODE before processing the key. Preserve the existing key handling when
hints_state is present, and ensure invalidated overlays do not leave the client
stuck in HINTS_MODE.
In `@crates/embers-protocol/src/codec.rs`:
- Around line 264-272: Add a unit test alongside the existing buffer-request
rejection tests, such as encode_buffer_request_rejects_empty_user_option_key,
that encodes a SetUserOption with an empty key and asserts it returns
InvalidMessageOwned containing "buffer_request.user_option_key must not be
empty".
In `@crates/embers-server/src/terminal_backend.rs`:
- Around line 86-92: Keep the existing OSC 7 handling and pid-based fallback
behavior, but document in the BufferRef cwd API that cwd() may be spoofed by
arbitrary PTY output because reported_cwd() takes precedence in
KeeperRuntime::status(). Add a clear warning that it must not be trusted for
security-sensitive actions such as seeding spawned-buffer directories, while
noting this behavior is intentional to support SSH and tmux-style automatic cwd.
---
Outside diff comments:
In `@crates/embers-server/src/model.rs`:
- Around line 69-85: Update PersistedBuffer, persisted_buffer, and
restored_buffer to include Buffer.user_options in workspace serialization and
restoration. Ensure set_buffer_user_option values survive save/load by
persisting the BTreeMap and reconstructing it when creating a Buffer.
In `@crates/embers-server/src/server.rs`:
- Around line 2805-2945: Add test coverage for record_buffer_update and its
same-sequence cwd handling: assert a cwd change applies when update.sequence
equals buffer.last_snapshot_seq, and assert a stale lower-sequence cwd update is
ignored. Keep the existing title/pipe sequence-gating assertions and isolate the
cwd cases with appropriate update.cwd values.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 72a71312-de05-436e-becf-b83f4ddb627b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
crates/embers-client/src/scripting/runtime.rs (1)
166-193: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMissing import breaks compilation:
hint_selectioncallsdynamic_option_string, which isn't imported.
documented_context_api'suse super::{...}list only brings indynamic_option_custom, notdynamic_option_string, buthint_selection(line 191) callsdynamic_option_string(...). This is the same issue flagged in a previous review round and remains unresolved.🐛 Proposed fix
mod documented_context_api { use super::{ - Array, Context, Dynamic, NativeCallContext, dynamic_option_custom, parse_buffer_id, - parse_floating_id, parse_node_id, with_call_position, + Array, Context, Dynamic, NativeCallContext, dynamic_option_custom, dynamic_option_string, + parse_buffer_id, parse_floating_id, parse_node_id, with_call_position, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/scripting/runtime.rs` around lines 166 - 193, Update the documented_context_api import list to include dynamic_option_string alongside dynamic_option_custom, so hint_selection can resolve the helper and compile successfully.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 1903-1961: Update spawn_run_shell so failure notifications never
include the raw command arguments from display. Replace display-based messages
in the child wait and process.spawn error paths with safe metadata, such as the
executable name or a redacted representation, while preserving the existing
warning behavior and exit/error details.
- Around line 434-448: Update the ClientChanged handling in the configured
client event flow to explicitly clear active-session state when the matching
client’s current_session_id is None. Preserve the existing set_active_session
and terminal-title behavior for Some(session_id), and ensure the detached case
removes the stale active_session_id and related state.
- Line 417: Update process_next_event_timeout() to surface queued background
failures when the idle timeout returns None, by adding a notification wakeup or
equivalent drain/dirty path for that timeout branch. Ensure run_shell child
failures are processed without waiting for a subsequent input or server event,
while preserving existing event handling and the
drain_background_notifications() behavior.
In `@crates/embers-protocol/schema/embers.fbs`:
- Around line 236-239: Remove the has_user_option_value field from the relevant
schema table, and update generated schema/codec consumers to rely solely on
user_option_value presence, preserving the distinction between None and
Some("").
In `@crates/embers-protocol/src/codec.rs`:
- Around line 4513-4517: Update the buffer option decoding around
decode_string_map so user_option_keys and user_option_values must either both be
absent or both be present; reject frames where values exist without keys with
the appropriate protocol error, while preserving normal map decoding for
matching vectors.
---
Duplicate comments:
In `@crates/embers-client/src/scripting/runtime.rs`:
- Around line 166-193: Update the documented_context_api import list to include
dynamic_option_string alongside dynamic_option_custom, so hint_selection can
resolve the helper and compile successfully.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: be544d69-8600-4a80-b5e7-ee3a22f18460
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/embers-client/src/configured_client.rs (1)
651-750: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSynchronize the interactive session before processing another queued key.
These actions update only
ConfiguredClientimmediately. The interactive loop retains its oldsession_iduntilClientChangedis polled, but drains queued input first; a following key is therefore routed to the previous session and resetsactive_session_idviaset_active_view.Update the caller from
configured.active_session_id()after actions that can cross sessions instead of waiting for event delivery.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/configured_client.rs` around lines 651 - 750, Update the interactive loop’s action-processing caller to refresh its session_id from configured.active_session_id() immediately after actions that can switch or reveal a different session, rather than waiting for ClientChanged delivery. Cover the cross-session branches in the action handler, including RevealBuffer, OpenBufferHistory, SwitchSession, LastSession, NextSession, and PrevSession, while preserving existing behavior for actions that do not change sessions.crates/embers-server/src/server.rs (1)
2931-2939: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn the refreshed cwd in snapshot responses.
capture_snapshotstill serializes the pre-syncbuffer_cwd, so a cwd change applied bysync_buffer_runtime_statuscan be missed in the response. Re-readbuffer.cwdafter the sync or use the synced value.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-server/src/server.rs` around lines 2931 - 2939, Update capture_snapshot after sync_buffer_runtime_status to serialize the refreshed buffer.cwd rather than the stale pre-sync buffer_cwd value. Re-read buffer.cwd after synchronization, or reuse the synchronized status value, so snapshot responses include cwd changes.
♻️ Duplicate comments (1)
crates/embers-client/src/scripting/runtime.rs (1)
166-193: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winCompile error:
dynamic_option_stringnot imported intodocumented_context_api.
hint_selection(lines 189-192) callsdynamic_option_string, but this module'suse super::{...}list (lines 167-170) never imports it — onlydocumented_ref_apidoes. This module-scopedusewon't resolve the name, so the crate fails to build. This was already flagged in a previous review round and is still unresolved.🐛 Proposed fix
mod documented_context_api { use super::{ - Array, Context, Dynamic, NativeCallContext, dynamic_option_custom, parse_buffer_id, - parse_floating_id, parse_node_id, with_call_position, + Array, Context, Dynamic, NativeCallContext, dynamic_option_custom, + dynamic_option_string, parse_buffer_id, parse_floating_id, parse_node_id, + with_call_position, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-client/src/scripting/runtime.rs` around lines 166 - 193, Add dynamic_option_string to the super import list in documented_context_api so hint_selection can resolve the helper and compile successfully.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 1854-1866: Update set_active_session and clear_active_session to
cancel and clear the current hints overlay whenever the active session changes
or is detached. Ensure the old hints_node no longer retains hints_state before
storing the session transition, while preserving the existing session ID
bookkeeping.
In `@crates/embers-client/src/state.rs`:
- Around line 372-377: Restrict the hints reset in apply_session_snapshot to
snapshots that advance the pane’s buffer content, using the buffer snapshot
sequence or existing content-change marker; do not clear state.hints_state for
unrelated NodeChanged/FloatingChanged resyncs. Preserve the reset behavior in
apply_buffer_snapshot and add coverage confirming session resyncs retain an
active hints overlay when visible content is unchanged.
In `@crates/embers-server/src/buffer_runtime.rs`:
- Around line 1455-1474: Add regression coverage around the poller/server update
flow containing the status-change callback, verifying cwd changes are emitted
even when sequence remains unchanged. Cover both known-to-known and
known-to-unknown transitions, preserving the distinction between Some values and
None.
---
Outside diff comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 651-750: Update the interactive loop’s action-processing caller to
refresh its session_id from configured.active_session_id() immediately after
actions that can switch or reveal a different session, rather than waiting for
ClientChanged delivery. Cover the cross-session branches in the action handler,
including RevealBuffer, OpenBufferHistory, SwitchSession, LastSession,
NextSession, and PrevSession, while preserving existing behavior for actions
that do not change sessions.
In `@crates/embers-server/src/server.rs`:
- Around line 2931-2939: Update capture_snapshot after
sync_buffer_runtime_status to serialize the refreshed buffer.cwd rather than the
stale pre-sync buffer_cwd value. Re-read buffer.cwd after synchronization, or
reuse the synchronized status value, so snapshot responses include cwd changes.
---
Duplicate comments:
In `@crates/embers-client/src/scripting/runtime.rs`:
- Around line 166-193: Add dynamic_option_string to the super import list in
documented_context_api so hint_selection can resolve the helper and compile
successfully.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9923a3d8-2198-4fc7-ae72-c877f66de1da
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/Cargo.toml`:
- Line 31: Remove the duplicate base64 entry from the dev-dependencies section
of Cargo.toml, while retaining the existing base64.workspace = true declaration
under dependencies.
In `@crates/embers-client/src/scripting/runtime.rs`:
- Around line 1073-1086: Import Dynamic into the documented_action_api module so
run_shell_argv can resolve Dynamic::from(argv). Leave the existing
parse_string_array conversion and command validation unchanged.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 72ece4d0-d156-470d-a1c2-78836cd7d3d3
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
| unicode-width.workspace = true | ||
|
|
||
| [dev-dependencies] | ||
| base64.workspace = true |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'Cargo.toml' crates/embers-client --exec cat -n {}Repository: Pajn/Embers
Length of output: 1507
Remove the duplicate base64 dev-dependency. base64.workspace = true already exists under [dependencies], so this entry is redundant and can be dropped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/embers-client/Cargo.toml` at line 31, Remove the duplicate base64
entry from the dev-dependencies section of Cargo.toml, while retaining the
existing base64.workspace = true declaration under dependencies.
Pajn
force-pushed
the
smaller
branch
2 times, most recently
from
July 13, 2026 10:21
cf89152 to
17e79ca
Compare
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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)
crates/embers-server/src/server.rs (1)
2624-2643: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply the post-sync CWD read to visible snapshots too.
capture_visible_snapshotstill fillsVisibleSnapshotResponse.cwdfromsnapshot.cwdtaken beforesync_buffer_runtime_status. Re-readbuffer.cwdafter the sync here as well soCaptureVisibledoesn't return a stale CWD when the runtime updates it in the same sequence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-server/src/server.rs` around lines 2624 - 2643, The visible snapshot path in capture_visible_snapshot still returns the pre-sync snapshot.cwd value. After sync_buffer_runtime_status, re-read the current buffer.cwd from state and use it for VisibleSnapshotResponse.cwd, falling back to the previously captured CWD when unavailable, matching the response_cwd logic used by SnapshotResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 147-158: Move the active-session reconciliation using
configured.active_session_id(), ensure_root_window(), and session_id so it runs
immediately after each dispatched action and before the next queued input is
processed, rather than only after draining the input queue. Preserve the
existing behavior for switching to a new session, marking dirty, and leaving
active-session detachment to the ClientChanged path.
In `@crates/embers-client/tests/script_actions.rs`:
- Around line 645-710: Add an enter_hints_builders_map_to_actions test alongside
session_switch_builders_map_to_actions and run_shell_builders_map_to_actions.
Define Rhai actions invoking action.enter_hints() and
action.enter_hints_with("open-url"), then assert they produce Action::EnterHints
with None and Some("open-url") respectively; first reuse any existing equivalent
coverage if present elsewhere.
---
Outside diff comments:
In `@crates/embers-server/src/server.rs`:
- Around line 2624-2643: The visible snapshot path in capture_visible_snapshot
still returns the pre-sync snapshot.cwd value. After sync_buffer_runtime_status,
re-read the current buffer.cwd from state and use it for
VisibleSnapshotResponse.cwd, falling back to the previously captured CWD when
unavailable, matching the response_cwd logic used by SnapshotResponse.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6f55baae-577c-4e38-a0e4-d3ec8be8e95e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 1854-1869: Update set_active_session so reattaching a new session
does not overwrite previous_session_id when active_session_id is None after
clear_active_session. Preserve the detached session stored by
clear_active_session, while continuing to record the currently active session
when switching directly between sessions.
- Around line 1873-1881: Update cancel_active_hints to reset the input mode to
the normal mode while tearing down the active hints overlay, in addition to
clearing hints_node and hints_state. Ensure session switches fully leave
HINTS_MODE so the next key is processed normally.
In `@crates/embers-client/tests/e2e.rs`:
- Around line 1740-1743: Strengthen the assertions in the unknown-session and
missing-previous-session test cases around the existing
configured.notifications() checks: verify that the newly appended notification
has the expected content and corresponds to the scenario, rather than only
asserting the collection length increased. Preserve the existing before-count
boundary so the assertion targets the notification added by each operation.
In `@crates/embers-client/tests/reducer.rs`:
- Around line 390-447: The duplicated HintsState fixture in
fresh_buffer_snapshot_invalidates_stale_hints and
session_resync_retains_active_hints_overlay may be extracted into a small
sample_hints_state helper returning the shared HintMatch data. Use the helper in
both tests while preserving their existing assertions and behavior.
In `@crates/embers-server/src/server.rs`:
- Around line 2624-2634: Update the response_cwd lookup in both snapshot APIs,
including capture_visible_snapshot, to distinguish a missing buffer from an
existing buffer with cwd set to None. Fall back to buffer_cwd only when
buffers.get(&buffer_id) returns no buffer; preserve None when an existing
buffer’s cwd was cleared.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 90a7d87d-74b1-400f-bfe2-a03cdcd41e6f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 2352-2358: Clear the active hints state and reset the hints mode
whenever focus changes, both in local mouse-focus handling and server-driven
FocusChanged handling. Update the relevant focus-change paths around hints_node
and input_state.set_mode so stale hints cannot remain associated with the
previously focused node.
In `@crates/embers-client/tests/e2e.rs`:
- Around line 1641-1648: Update the test around configured.emit_terminal_title
to exercise the real attach initialization path before draining terminal output,
rather than invoking the title emitter directly; preserve the assertion for the
alpha terminal-title sequence. If the attach path cannot be exercised here,
rename the test to accurately describe direct title-emission coverage.
In `@crates/embers-client/tests/fixtures/repository_config.rhai`:
- Around line 90-102: Update the user_option handling in the Vim detection logic
so an explicit "0" returns false immediately, while only the unset () value
proceeds to process-name detection. Preserve the existing true behavior for "1"
and the nvim/vim/nvr process-name matches.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6e6a7c31-6465-4610-993b-20ad2892279c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
Comment on lines
+1641
to
+1648
| // Attach: title names the active session. | ||
| configured.emit_terminal_title(alpha); | ||
| let drained = configured.drain_terminal_output().concat(); | ||
| assert!( | ||
| contains_subslice(&drained, b"\x1b]2;alpha\x07"), | ||
| "attach title missing in {:?}", | ||
| String::from_utf8_lossy(&drained) | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Exercise the attach path instead of invoking the title emitter directly.
Calling emit_terminal_title(alpha) cannot catch missing attach-time wiring. Drive the actual attach initialization path, or rename this test so it does not claim attach coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/embers-client/tests/e2e.rs` around lines 1641 - 1648, Update the test
around configured.emit_terminal_title to exercise the real attach initialization
path before draining terminal output, rather than invoking the title emitter
directly; preserve the assertion for the alpha terminal-title sequence. If the
attach path cannot be exercised here, rename the test to accurately describe
direct title-emission coverage.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/configured_client.rs`:
- Around line 348-350: Update the mouse-wheel scrolling path in configured
client event handling to call cancel_active_hints whenever scrolling replaces
visible_lines for the currently hinted pane. Preserve the existing click-focus
cancellation through cancel_active_hints, and ensure stale hints are cleared
before or alongside the new visible content.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 41e4ded5-3140-41cb-adb2-593480616363
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
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)
crates/embers-server/src/buffer_runtime.rs (1)
1125-1163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard the pid-based cwd fallback behind
running
resolve_pid_cwd(self.pid)should only run while the child is still alive. After exit, the pid can be recycled, and a later poll can pick up another process’s cwd and propagate it throughBufferRuntimeStatus.cwd/BufferRef.cwd().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/embers-server/src/buffer_runtime.rs` around lines 1125 - 1163, Update KeeperStatus::status to only invoke resolve_pid_cwd through the reported_cwd fallback while exit_code indicates the child is still running. Preserve OSC 7 reported_cwd when available, and leave cwd unset after exit rather than resolving a potentially recycled pid.
♻️ Duplicate comments (1)
docs/config-api/action.md (1)
319-322: 📐 Maintainability & Code Quality | 🟡 Minor | 💤 Low valueAdd blank lines around the fenced examples.
markdownlintreports MD031 because the closing fences are immediately adjacent to the surrounding HTML</div>. Add a blank line after each closing fence to ensure proper markdown rendering and pass the linter.
docs/config-api/action.md#L319-L322: Add a blank line after the closing ``` and before</div>.docs/config-api/action.md#L352-L355: Add a blank line after the closing ``` and before</div>.docs/config-api/action.md#L1300-L1303: Add a blank line after the closing ``` and before</div>.docs/config-api/action.md#L1332-L1335: Add a blank line after the closing ``` and before</div>.docs/config-api/action.md#L1806-L1809: Add a blank line after the closing ``` and before</div>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/config-api/action.md` around lines 319 - 322, Add a blank line between each closing fenced code block and the following HTML </div> in docs/config-api/action.md at lines 319-322, 352-355, 1300-1303, 1332-1335, and 1806-1809, preserving the examples while satisfying markdownlint MD031.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/renderer.rs`:
- Around line 243-254: Update the position fallback in the renderer flow around
follow_output_position so a missing view_state computes the default
follow-output position using the current rendered lines and content_rows, rather
than Position::default(). Preserve the existing view-specific calculation when
view_state is present, ensuring initial rendering stays at the bottom of the
buffer.
In `@crates/embers-server/src/server.rs`:
- Around line 2221-2270: Update set_buffer_user_option to track whether the
requested insertion or removal changes record.user_options: treat setting an
existing key to its current value and removing an absent key as no-ops. Return
the existing buffer record and an empty ServerEvent list when unchanged, while
preserving RenderInvalidatedEvent for actual option changes.
---
Outside diff comments:
In `@crates/embers-server/src/buffer_runtime.rs`:
- Around line 1125-1163: Update KeeperStatus::status to only invoke
resolve_pid_cwd through the reported_cwd fallback while exit_code indicates the
child is still running. Preserve OSC 7 reported_cwd when available, and leave
cwd unset after exit rather than resolving a potentially recycled pid.
---
Duplicate comments:
In `@docs/config-api/action.md`:
- Around line 319-322: Add a blank line between each closing fenced code block
and the following HTML </div> in docs/config-api/action.md at lines 319-322,
352-355, 1300-1303, 1332-1335, and 1806-1809, preserving the examples while
satisfying markdownlint MD031.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4594b97f-d405-45f8-a054-4ac34bf28902
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-client/src/renderer.rs`:
- Around line 720-783: In render_hints_overlay, compute
hints_state.typed.chars().count() once before iterating through hints and reuse
that value when determining whether each label character is typed. Replace the
per-character traversal in the typed check while preserving the existing styling
behavior.
In `@crates/embers-client/src/scripting/types.rs`:
- Line 141: Update LoadedConfig’s custom fmt::Debug implementation to include
the hints field alongside the other configuration fields, using self.hints
before the formatter finishes.
In `@crates/embers-server/src/terminal_backend.rs`:
- Around line 176-189: Update the Osc7State::CollectEscape and
Osc7State::SkipEscape handlers so an ESC byte transitions to
Osc7State::SkipEscape instead of resetting to Ground, preserving the new escape
sequence for the following ] byte. Keep the existing backslash completion and
other non-ESC termination behavior unchanged.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 06bdb4b3-b3ce-4c02-a375-067522b5ba0b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
Comment on lines
+176
to
+189
| Osc7State::CollectEscape => { | ||
| if byte == b'\\' { | ||
| self.finish(); | ||
| } | ||
| self.state = Osc7State::Ground; | ||
| } | ||
| Osc7State::Skip => match byte { | ||
| BEL => self.state = Osc7State::Ground, | ||
| ESC => self.state = Osc7State::SkipEscape, | ||
| _ => {} | ||
| }, | ||
| Osc7State::SkipEscape => { | ||
| self.state = Osc7State::Ground; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
CollectEscape/SkipEscape swallow a second ESC instead of restarting the sequence.
In Escape, a repeated ESC correctly stays armed (Line 153-154: "Stay armed on a run of ESCs"). But CollectEscape (Lines 176-181) and SkipEscape (Lines 187-189) drop unconditionally to Ground on any non-backslash byte, including ESC. If a malformed/aborted OSC (no \ terminator) is immediately followed by a genuine new escape sequence, that second ESC is consumed here and the following ] is evaluated in Ground (which only watches for ESC), so the next OSC report — including a real OSC 7 cwd report — is silently missed.
🐛 Proposed fix
Osc7State::CollectEscape => {
if byte == b'\\' {
self.finish();
+ self.state = Osc7State::Ground;
+ } else if byte == ESC {
+ // A second ESC aborts the pending ST and may itself start a
+ // fresh sequence; stay armed instead of dropping it.
+ self.state = Osc7State::Escape;
+ } else {
+ self.state = Osc7State::Ground;
}
- self.state = Osc7State::Ground;
}
Osc7State::Skip => match byte {
BEL => self.state = Osc7State::Ground,
ESC => self.state = Osc7State::SkipEscape,
_ => {}
},
Osc7State::SkipEscape => {
- self.state = Osc7State::Ground;
+ self.state = if byte == ESC {
+ Osc7State::Escape
+ } else {
+ Osc7State::Ground
+ };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Osc7State::CollectEscape => { | |
| if byte == b'\\' { | |
| self.finish(); | |
| } | |
| self.state = Osc7State::Ground; | |
| } | |
| Osc7State::Skip => match byte { | |
| BEL => self.state = Osc7State::Ground, | |
| ESC => self.state = Osc7State::SkipEscape, | |
| _ => {} | |
| }, | |
| Osc7State::SkipEscape => { | |
| self.state = Osc7State::Ground; | |
| } | |
| Osc7State::CollectEscape => { | |
| if byte == b'\\' { | |
| self.finish(); | |
| self.state = Osc7State::Ground; | |
| } else if byte == ESC { | |
| // A second ESC aborts the pending ST and may itself start a | |
| // fresh sequence; stay armed instead of dropping it. | |
| self.state = Osc7State::Escape; | |
| } else { | |
| self.state = Osc7State::Ground; | |
| } | |
| } | |
| Osc7State::Skip => match byte { | |
| BEL => self.state = Osc7State::Ground, | |
| ESC => self.state = Osc7State::SkipEscape, | |
| _ => {} | |
| }, | |
| Osc7State::SkipEscape => { | |
| self.state = if byte == ESC { | |
| Osc7State::Escape | |
| } else { | |
| Osc7State::Ground | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/embers-server/src/terminal_backend.rs` around lines 176 - 189, Update
the Osc7State::CollectEscape and Osc7State::SkipEscape handlers so an ESC byte
transitions to Osc7State::SkipEscape instead of resetting to Ground, preserving
the new escape sequence for the following ] byte. Keep the existing backslash
completion and other non-ESC termination behavior unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/embers-cli/src/interactive.rs`:
- Around line 187-193: Update the event-handling branch for poll results
returning Some(event) to call configured.drain_background_notifications() before
draining terminal output, and surface any resulting output through
drain_terminal_output(&mut configured). Preserve the existing timeout-branch
behavior and event processing.
In `@crates/embers-server/src/server.rs`:
- Around line 61-67: Update the BufferRequest::List response path to enforce an
aggregate BuffersResponse size budget, not only the per-buffer limits from
MAX_USER_OPTIONS_PER_BUFFER and related constants. Add pagination or stop adding
BufferRecords before the encoded response could exceed the 8 MiB frame cap,
while preserving valid list behavior within the budget.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 14c9d48a-c610-4eab-ad0d-0721705184a5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
Cargo.tomlcrates/embers-cli/src/interactive.rscrates/embers-cli/src/lib.rscrates/embers-cli/tests/panes.rscrates/embers-client/Cargo.tomlcrates/embers-client/src/client.rscrates/embers-client/src/configured_client.rscrates/embers-client/src/hints.rscrates/embers-client/src/input/mod.rscrates/embers-client/src/input/modes.rscrates/embers-client/src/lib.rscrates/embers-client/src/presentation.rscrates/embers-client/src/renderer.rscrates/embers-client/src/scripting/context.rscrates/embers-client/src/scripting/documentation.rscrates/embers-client/src/scripting/engine.rscrates/embers-client/src/scripting/model.rscrates/embers-client/src/scripting/runtime.rscrates/embers-client/src/scripting/types.rscrates/embers-client/src/state.rscrates/embers-client/tests/configured_client.rscrates/embers-client/tests/e2e.rscrates/embers-client/tests/fixtures/repository_config.rhaicrates/embers-client/tests/presentation.rscrates/embers-client/tests/reducer.rscrates/embers-client/tests/renderer.rscrates/embers-client/tests/script_actions.rscrates/embers-client/tests/support/mod.rscrates/embers-protocol/schema/embers.fbscrates/embers-protocol/src/codec.rscrates/embers-protocol/src/types.rscrates/embers-protocol/tests/family_round_trip.rscrates/embers-server/Cargo.tomlcrates/embers-server/src/buffer_runtime.rscrates/embers-server/src/model.rscrates/embers-server/src/protocol.rscrates/embers-server/src/server.rscrates/embers-server/src/terminal_backend.rscrates/embers-test-support/tests/buffer_runtime.rsdocs/config-api-book/404.htmldocs/config-api-book/action.htmldocs/config-api-book/buffer-ref.htmldocs/config-api-book/context.htmldocs/config-api-book/defs/registration.rhaidocs/config-api-book/defs/runtime.rhaidocs/config-api-book/event-info.htmldocs/config-api-book/example.htmldocs/config-api-book/floating-ref.htmldocs/config-api-book/hints.htmldocs/config-api-book/index.htmldocs/config-api-book/mouse.htmldocs/config-api-book/mux.htmldocs/config-api-book/node-ref.htmldocs/config-api-book/print.htmldocs/config-api-book/registration-action.htmldocs/config-api-book/registration-globals.htmldocs/config-api-book/registration-system.htmldocs/config-api-book/registration-tree.htmldocs/config-api-book/registration-ui.htmldocs/config-api-book/runtime-theme.htmldocs/config-api-book/searcher-c2a407aa.jsdocs/config-api-book/searchindex-3b02c077.jsdocs/config-api-book/searchindex-60b5c002.jsdocs/config-api-book/session-ref.htmldocs/config-api-book/system-runtime.htmldocs/config-api-book/tab-bar-context.htmldocs/config-api-book/tab-info.htmldocs/config-api-book/tabbar.htmldocs/config-api-book/theme.htmldocs/config-api-book/toc-22d094e2.jsdocs/config-api-book/toc.htmldocs/config-api-book/tree.htmldocs/config-api-book/ui.htmldocs/config-api/SUMMARY.mddocs/config-api/action.mddocs/config-api/buffer-ref.mddocs/config-api/context.mddocs/config-api/defs/registration.rhaidocs/config-api/defs/runtime.rhaidocs/config-api/hints.mddocs/config-api/index.mddocs/config-api/registration-action.md
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Store a runtime user_options map on each buffer, settable via a new BufferRequest::SetUserOption (set/unset), surfaced on BufferRecord and exposed to config scripts as buffer.user_option(key). Setting emits RenderInvalidated so clients refetch the record before the next binding dispatch. - server: user_options on Buffer + set_buffer_user_option handler - protocol: SetUserOption request + user_options on BufferRecord + codec - cli: `embers buffer set-option [-t] <key> [<value>] [--unset]` and `embers buffer show-options [-t]` - scripting: BufferRef.user_option(key); fixture is_nvim_buffer prefers it - tests: codec round-trip, server set/unset/reject, CLI round-trip, script reads option; regenerated config API docs
The runtime keeper now reports the child shell's working directory so splits and scripts see the live cwd like tmux does. Two sources, keeper side (only the keeper is guaranteed same-host as the child): - pid polling (primary, zero shell config): resolve the direct child's cwd each Status poll — /proc/<pid>/cwd on Linux, proc_pidinfo PROC_PIDVNODEPATHINFO on macOS. - OSC 7 (override when emitted): a small stateful scanner in RawByteRouter sniffs ESC]7;file://host/path(BEL|ST), percent-decodes the path, and wins over pid polling (handles ssh'd shells). cwd rides the keeper Status response (serde-defaulted for old-keeper compat) through BufferRuntimeUpdate into buffer.cwd; the existing BufferRecord.cwd / snapshot echo / Rhai buffer.cwd() are now live. Tests: OSC 7 scanner units (whole/split/ST/non-7/malformed), OSC 7 integration updating the record, ignored pid-resolution smoke test.
New action-layer bindings over the existing server switch mechanism:
- Actions SwitchSession { name }, LastSession, NextSession, PrevSession
- Rhai builders action.switch_session/last_session/next_session/prev_session
- previous_session_id tracked on ConfiguredClient via a set_active_session
helper wired through every active-session change (including own-client
ClientChanged), so last_session toggles A<->B like tmux
- Name resolution against client state (unknown name -> Notify, no detach);
Next/Prev cycle in list-sessions order, wrapping
- Fixture config binds <leader>' to last-session
Tests: Rhai builder->action mapping; real-server e2e over a live
ConfiguredClient switching by name, toggling last, cycling, and notifying
on an unknown name. Regenerated config API docs.
New client-side action that spawns a command in the user's login context:
- Action RunShell { command } + Rhai action.run_shell("wisp popup")
(string form -> /bin/sh -lc) and action.run_shell_argv([...]) (exec form)
- Dispatch spawns a detached tokio process with stdio nulled, cwd set to
the focused buffer's cwd when known, and $EMBERS_SOCKET injected so the
tool can drive Embers via the CLI. The client never awaits the child; a
background waiter surfaces a warning on non-zero exit through a drained
notification queue. Output is discarded (unlike tmux) — capture into a
popup with buffer_spawn instead
- interactive client records its socket path on the ConfiguredClient
- Fixture config binds <leader>w to run-shell "wisp popup"
Tests: Rhai builder->action mapping; real-server e2e proving spawn + env
(writes $EMBERS_SOCKET to a file) and a non-zero exit notification, with
the client staying responsive. Regenerated config API docs.
Push an OSC 2 window-title update naming the active session through the existing host-output queue (the OSC 52 path), so the host terminal's tab follows the session: - on initial attach (interactive client) - on own-client ClientChanged (session switch) - on SessionRenamed of the active session Title is fixed to the session name for now; no reset on exit (terminals restore their own, like tmux). Tests: real-server e2e asserting the drained output carries the OSC 2 sequence after attach, after a switch, and after a rename.
Client-only hint mode following the search/select overlay pattern:
- state: HintsState { matches, typed, on_select } on BufferViewState;
a new `hints` built-in mode (FallbackPolicy::Ignore)
- entry: Action EnterHints { action } + Rhai action.enter_hints() /
enter_hints_with("handler"). Dispatch scans the rendered visible lines
with a regex set, assigns home-row labels (single chars, else 2-char
pairs, last-match-first), and enters hints mode
- patterns: built-in default set (URLs, paths, SHAs, UUIDs, IPs, numbers),
overridable via the new hints.set_patterns([...]) registration API;
introduces the regex crate to embers-client
- keys: dedicated hints branch in handle_key — printable keys extend the
typed prefix and filter labels, a unique full label selects, Esc cancels
- select: default copies the text via OSC 52 + notifies; with a handler,
invokes the named action with the text on ctx.hint_selection()
- overlay: render_hints_overlay dims matched text and draws labels,
underlining the typed prefix
- fixture config binds <leader><Space> to hints
Tests: label/pattern/scan units; renderer label overlay; real-server e2e
copying a URL via OSC 52 and the callback variant via ctx.hint_selection().
Regenerated config API docs.
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
1 participant
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.
Stack
Summary by CodeRabbit
embers buffer set-option/embers buffer show-options, plus scripting support.run_shell,run_shell_argv), and entering hints.