Typing `:help` crashes vimcode in both the TUI and GTK front-ends

[JDonaghy]

Summary

Typing :help (bare, no topic) in both the TUI and GTK front-ends crashes vimcode.

Repro

1. Launch vimcode (TUI: cargo run; GTK: cargo run --features gtk / the GTK binary).
2. From normal mode, press : to enter command mode.
3. Type help and press Enter.
4. → vimcode panics / crashes.

Both front-ends are affected, which points at shared engine code rather than a front-end renderer.

Suspected location

Engine::cmd_help in src/core/engine/buffers.rs:1582 handles :help. After building the help text it:

let buf_id = self.buffer_manager.create();
if let Some(state) = self.buffer_manager.get_mut(buf_id) {
    state.buffer.content = ropey::Rope::from_str(&text);
}
self.split_window(SplitDirection::Vertical, None);   // <-- likely crash path
self.active_window_mut().buffer_id = buf_id;

The crash most likely comes from split_window / active_window_mut or from rendering a freshly-created buffer that has no per-window cursor/scroll state (e.g. cursor position out of bounds, unwrap on a missing window/layout entry). :help <topic> may share the same path. Please capture the actual panic message + backtrace (RUST_BACKTRACE=1) and fix the root cause — do not just swallow the panic.

Expected behavior

:help opens the help text in a new split window (a scratch/help buffer) without crashing, matching the existing :help <topic> intent.

Acceptance (black-box test required)

Per CLAUDE.md, ship a black-box regression test that drives the running app and asserts on rendered output:

• A TUI-harness test (drive :help + Enter through the real event → handle → render path) that asserts no panic and that the help buffer content is visible on screen.
• If feasible in the same PR, a GTK/offscreen-harness assertion as well; at minimum an engine-level test that execute_command("help") returns without panicking and creates the help buffer + split.

Also verify :help topics, :help keys, :help commands, :help explorer, and an unknown topic (:help bogus) all behave (open buffer or set the "No help for ..." message) without crashing.

[JDonaghy]

Coordinator Assignment

Machine: precision
Repo: vimcode
Files: (unspecified)
Do not touch: (nothing else in flight)

Briefing

Fix the :help crash (issue #596). Typing :help and pressing Enter crashes vimcode in BOTH the TUI and GTK front-ends — so the bug is almost certainly in shared engine code, not a front-end renderer.

Before coding: reproduce it and capture the real panic. Run with RUST_BACKTRACE=1 and drive :help through the TUI (and, if you have a display, GTK). Record the panic message + backtrace in the PR.

Prime suspect: Engine::cmd_help at src/core/engine/buffers.rs:1582. After creating the help buffer it calls self.split_window(SplitDirection::Vertical, None) then self.active_window_mut().buffer_id = buf_id. The crash is likely in split_window / active_window_mut, or in rendering a brand-new buffer whose window has no valid cursor/scroll state. Fix the ROOT CAUSE — do not just catch/swallow the panic.

Acceptance (required, per CLAUDE.md black-box rule):

• Add a black-box regression test that drives :help + Enter through the real TUI event → handle → render path and asserts NO panic and that the help buffer content renders. Use the existing TUI test harness — don't stand up a live TTY.
• Also cover :help topics, :help keys, :help commands, :help explorer, and an unknown topic (:help bogus) — each must open a buffer or set the "No help for ..." message without crashing.
• cargo build AND cargo test must exit 0 from the repo root before you finish.

Scope: engine + TUI test. Do NOT edit README/CHANGELOG or other shared docs (files_forbidden: README.md, CHANGELOG.md, HISTORY.md). Stay in file scope. Write your own unit/internal tests as needed; do not touch tests/acceptance/**.

[JDonaghy]

Coordinator: Assignment Complete

Machine: precision
Status: done
Exit code: 0
Duration: 21m 27s
Log: /home/john/.coord/logs/360f43fbbb99.log

[JDonaghy]

⏱ Needs attention

Machine: elitebook
Assignment: cf3b844a0b54
Issue: #596
Reason: Running too long

Running 62.0m, past the 60m threshold for type='smoke'.

Detection + surfacing only — nothing was killed or reassigned. A human should take a look; use coord log <id> to see what it's doing, or coord retry/coord stop to intervene.

[JDonaghy]

Coordinator: Assignment Complete

Machine: elitebook
Status: done
Exit code: 0
Duration: —

Summary

Smoke test PASSED for #596 (:help crash fix): verified bare :help, all topic variants (topics/keys/commands/explorer/bogus), and GTK front-end all open the help split with no crash. Built from scratch worktree since no prebuilt artifact was available.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Correct root-cause fix (bounds guard in update_bracket_match) + defensive cursor/scroll resets across split_window call sites; solid regression tests covering all :help topics + unknown-topic case

Verdict: approve

Review findings

Review: #596 — :help crashes vimcode (TUI + GTK)

Branch: issue-596-typing-help-crashes-vimcode-in-both-the
Diff reviewed: git diff origin/develop...origin/issue-596-typing-help-crashes-vimcode-in-both-the
(merge-base is 0cee3cd, 2 commits behind origin/develop tip 1b3b9cb, but those commits don't touch any file in this diff, so no conflict risk.)

Summary of the change

• src/core/engine/motions.rs (update_bracket_match): adds a bounds guard —
  if line >= self.buffer().len_lines() { self.bracket_match = None; return; }
  before calling self.buffer().line_to_char(line).
• src/core/engine/buffers.rs: after every split_window(...) call that
  immediately reassigns active_window_mut().buffer_id to a freshly-created
  scratch buffer (cmd_help, cmd_git_status, cmd_git_blame, cmd_diffsplit,
  the two cmd_diffsplit-adjacent hunk-view helpers, and the markdown/diff
  preview helpers), the cursor and scroll position are now explicitly reset to
  Cursor::default() / scroll_top = 0 / scroll_left = 0.
• src/tui_main/render_impl.rs: 6 new regression tests driving the real
  handle_key → execute_command → render_tui path for :help,
  :help topics, :help keys, :help commands, :help explorer, and
  :help bogus.

Root-cause verification (I traced this independently, not just trusting the PR's comments)

split_window_with_new_first (src/core/engine/windows.rs) clones the
current window's view (cursor + scroll) into the new window whenever the
new window starts out pointing at the same buffer id:

if new_buffer_id == current_buffer_id {
    new_window.view = self.active_window().view.clone();
}

cmd_help (pre-fix) called split_window(..., None) — which takes this
branch — and only afterward pointed the new window's buffer_id at the
freshly built (short) help-text buffer, leaving the inherited cursor/scroll
values from the original (possibly much longer) buffer in place.

Right after every keypress, keys.rs (handle_key, ~line 465) unconditionally
calls self.update_bracket_match(), which — pre-fix — did
self.buffer().line_to_char(line) with no bounds check, where line is the
stale (out-of-bounds) cursor line. ropey::Rope::line_to_char panics when
line > len_lines(). That's the actual panic path for both front-ends, since
both TUI and GTK route through the same shared Engine::handle_key. Good,
correct root-cause diagnosis — confirmed by reading windows.rs and the
keys.rs call site myself.

The motions.rs guard is the real fix (a backend-neutral, engine-level fix —
correctly aligned with CLAUDE.md's platform-neutrality rule). The
Cursor::default() resets scattered through buffers.rs are useful
defense-in-depth (they also prevent an out-of-bounds scroll — although
render.rs:9471 already separately clamps scroll_top for the main render
path, so that half is arguably belt-and-suspenders) but are not strictly
required for the crash itself to stop.

Checklist

• Tests: Yes — 6 new TUI-harness tests in render_impl.rs, each driving
  real key events end-to-end (:, then characters, then Return) and
  rendering, matching the issue's "black-box, drive the real event→handle→render
  path" requirement. They cover the full acceptance list: bare :help,
  topics, keys, commands, explorer, and unknown topic (bogus,
  verifying no split is opened and the "No help for" message is set). I
  confirmed Settings::default().match_brackets == true (so the guarded code
  path is actually exercised in these tests — the tests would panic on
  pre-fix code) and that handle_key's signature matches how the tests call
  it, and that test_engine/render_tui are pre-existing helpers (no
  fabricated test scaffolding).
• Scope: Stays within the issue. No unrelated refactors. Single commit,
  fix(#596): prevent panic in :help and other split-window commands,
  matching the repo's commit convention.
• Security: None applicable — no new I/O, path handling, or command
  execution; pure in-memory buffer/cursor bookkeeping.
• Platform-neutrality: Compliant. The actual fix lives entirely in
  src/core/engine/ (shared engine code used by both backends); the only
  backend-touched file is a TUI test module, which is fine (tests aren't the
  "per-backend production code" the rule prohibits) and is the right place to
  exercise the handle_key→render pipeline black-box style.

Minor / non-blocking observations (not requesting changes for these)

1. GTK-side regression coverage is still open. The issue's acceptance
   criteria said "if feasible in the same PR" for a GTK/offscreen assertion,
   so this isn't a hard blocker, and the root-cause fix in motions.rs is
   backend-agnostic so it does fix the GTK crash too (both backends funnel
   through the same Engine::handle_key → update_bracket_match). Still,
   there's no test at all exercising the GTK render path for :help; worth a
   follow-up issue if the team wants belt-and-suspenders coverage there.
2. A few other active_window_mut().buffer_id = ... call sites after
   split_window were not touched (ext_panel.rs:2544/2743/2804,
   buffers.rs:662/open_commit_file_diff). I checked each: they either
   already reset cursor.line/col explicitly, or open a brand-new
   Window::new(...) (which starts with a fresh default view, not a cloned
   one) rather than reusing split_window's clone-the-current-view path, so
   none of them appear to reproduce this specific bug. Flagging only because
   the pattern is easy to miss again in future call sites — the new
   motions.rs bounds guard is the actual safety net that makes this class of
   bug non-fatal regardless of which call site is at fault, which is the
   right layer for the fix.
3. cmd_help's Cursor::default() reset block duplicates a 5-line pattern
   six times across buffers.rs in this diff. A small helper (e.g.
   fn reset_window_view(&mut self, buf_id: BufferId)) would reduce
   duplication, but this is a style nit, not a defect — happy to leave as-is
   given the PR is a targeted bugfix.

Verdict

The root-cause fix is correct, minimal, backend-neutral, and well-tested
against the full acceptance list in the issue. No blocking issues found.

[JDonaghy]

Coordinator: Assignment Complete

Machine: dellserver
Status: done
Exit code: 0
Duration: —

Summary

Rebased issue-596 branch (1 commit, fa29fa4) cleanly onto origin/develop, no conflicts. Full test suite (cargo test --no-default-features) passed: 0 failed across all suites. Pushed with --force-with-lease. verify-merge clean (develop-ahead=0, no foreign commits). Merged via coord merge --repo vimcode: PR #598 opened and merged, issue #596 closed.