Skip to content

fix: reject workspace numbers past the last workspace - #5171

Merged
ogulcancelik merged 1 commit into
masterfrom
issue/5096-workspace-number-panic
Oct 11, 2026
Merged

ogulcancelik merged 1 commit into
masterfrom
issue/5096-workspace-number-panic

Conversation

@ogulcancelik

Copy link
Copy Markdown
Collaborator

A workspace number or w_N id past the last workspace (--workspace 2 with one workspace, 999, w_999) crashed the server in worktree list, worktree open and pane move --new-tab, killing every pane. Out-of-range numeric ids now resolve to not found, so these commands return workspace_not_found like an unknown stable id does.

refs #5096

Validation:

  • reproduced the panic on 0.9.3; fixed build returns workspace_not_found for all reported commands and the server keeps running
  • new unit test fails without the fix
  • just check green

@kangal-bot

Copy link
Copy Markdown
Collaborator

@coderabbitai review
@greptileai

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c29c9f1-79f5-49e3-b7b0-1747c43faa64

📥 Commits

Reviewing files that changed from the base of the PR and between be0e73f and 4bb8ac1.


📒 Files selected for processing (1)
  • src/app/ids.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

parse_workspace_id now returns None when the resolved index is outside the current workspace list. Tests cover valid IDs for the sole workspace, zero and out-of-range IDs, and a tab ID that references a nonexistent workspace.


Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 4bb8a

No merge-blocking risk is identified; merge after normal checks.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly summarizes the main fix: rejecting workspace numbers beyond the last workspace.
Description check Passed The description directly explains the server crash, the fix, affected commands, validation results, and issue reference.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@greptile-apps

greptile-apps Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Low impact] This PR appears safe to merge.

Summary

parse_workspace_id now rejects workspace numbers past the last workspace instead of returning an invalid index.

  • Out-of-range workspace numbers resolve as not found.

Reviews (1) · Last reviewed commit: "fix: reject workspace numbers past the l..." · Reviewed by Greptile

@ogulcancelik
ogulcancelik merged commit dc220d5 into master Oct 11, 2026
8 checks passed
@ogulcancelik
ogulcancelik deleted the issue/5096-workspace-number-panic branch October 11, 2026 22:06
aryrabelo added a commit to aryrabelo/bora-herdr-ada that referenced this pull request Oct 12, 2026
Upstream commits brought in:
- fix: detect codex startup update chooser (herdrdev#5174)
- fix: confirm closing a workspace's last pane in the tui (herdrdev#5173)
- fix: keep sgr pixel mouse units consistent (herdrdev#4971)
- fix: reject workspace numbers past the last workspace (herdrdev#5171)
- fix: reject oversized canonical input on macos (herdrdev#4988)
- fix: save sessions before agents die during macos restart (herdrdev#5092)
- fix: preserve hermes plugin list structure (herdrdev#4984)
- fix: restore keyboard protocol flags after a terminal reconnect (herdrdev#5088)
- fix: reduce interactive pane echo latency (herdrdev#5149)
- fix: flush session saves before replacing persisted files (herdrdev#5152)

Conflict resolutions:
- client shell close-pane confirmation: take upstream's request_pane_close,
  keep the fork's SetPaneStatus menu action and closes_group confirmation
  field; a group-root close still confirms client-side and closes the whole
  group (the fork has no server confirmation_required path, 2d839ce), so
  upstream's close_pane group test is adapted like close_tab.rs.
- pane input: keep the fork's backpressure-preserving send paths and add
  upstream's keyboard_forwarded tracking; keep send_bytes_after and
  write_user_input next to upstream's validate_input_submission.
- pane mouse: take upstream's merged encoder/position helper.
- satisfy the fork's redundant_clone deny lint in the new macos input tests.

Sets UPSTREAM_HERDR_COMMIT to 6d697e1 and bumps bora to 0.52.3.
aceitw added a commit to aceitw/herdr that referenced this pull request Oct 18, 2026
Keep our fork features (federation, gram, api-bridge, guest, push,
notifications, agent archive/forget/transfer, accounts, staged updates)
alongside upstream's v0.9.4, v0.9.5 and unreleased fixes. Most source
conflicts keep our fork's Method enum (upstream's define_operations!
macro cannot express our cfg-gated and serde-skipped variants). Detection
manifests keep our extra rules; Codex integration scripts switch to
upstream's canonical version.

Upstream highlights:
- Persistent macOS/Linux server (SIGHUP no longer stops it)
- Session saves on system shutdown/restart
- Plugin update command
- Keyboard/IME fixes (non-Latin layouts, Ctrl+Shift, accents)
- Windows improvements (PowerShell 7 default, clipboard, notifications)
- SSH compression and bandwidth improvements
- Performance fixes (CPU, memory, hidden pane improvements)
- Codex, Claude Code, Pi, Antigravity, Grok detection updates
- New platform modules (launchd spawn, power/shutdown)
- Vendor patches (libghostty-vt cursor hyperlink, portable-pty Windows dir)

refs herdrdev#5310, herdrdev#5278, herdrdev#5236, herdrdev#5303, herdrdev#5308, herdrdev#5275, herdrdev#5280, herdrdev#5282,
     herdrdev#5277, herdrdev#5276, herdrdev#5270, herdrdev#5269, herdrdev#5252, herdrdev#5249, herdrdev#5247, herdrdev#5241,
     herdrdev#5242, herdrdev#5192, herdrdev#5232, herdrdev#5212, herdrdev#5174, herdrdev#5173, herdrdev#5171, herdrdev#5092
maciejul pushed a commit to maciejul/herdr-supercharged that referenced this pull request Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants