Skip to content

fix: use upstream's bound in parse_workspace_id - #120

Merged
cameronsjo merged 2 commits into
masterfrom
claude/project-thread-vhm86r-herdr-upstream-workspace-id
Oct 21, 2026
Merged

cameronsjo merged 2 commits into
masterfrom
claude/project-thread-vhm86r-herdr-upstream-workspace-id

Conversation

@cameronsjo

Copy link
Copy Markdown
Owner

Requested by Cameron · project thread

What

Upstream fixed the out-of-range w_N / N workspace-id panic with the same filter as fork PR #81 (upstream herdrdev#5171). The 2026-10-12 sync kept the fork's copy of the line and its comment. So parse_workspace_id still diverged from upstream, with no difference in behavior.

parse_workspace_id in src/app/ids.rs now matches upstream master (143bf5f) exactly. The fork's .filter(|index| *index < len) and its four-line comment become upstream's .filter(|&index| index < len). The fork's request-path tests and the three hand-rolled < len guards stay.

In docs/fork/CHANGES.md, the #81 entry is marked retired for the parser filter, like the 2026-09-17 retired entry. A short new entry records this PR.

Check

  • The function matches upstream master byte for byte (diffed against raw.githubusercontent.com/herdrdev/herdr/master/src/app/ids.rs, master at 143bf5f).
  • cargo fmt --check: pass.
  • Not run: cargo test. The vendored libghostty-vt build fetches its Zig packages from deps.files.ghostty.org, which this session's proxy blocks. I installed Zig 0.16.0 and seeded uucode by hand (hash verified), but zlib, highway, wuffs and the themes tarball also need that host. CI covers it: cargo nextest run --locked -E 'test(positional_workspace_id) + test(numeric_workspace_ids_beyond_the_last_workspace_are_not_found)'.

Part of the owned-concessions sweep (cameronsjo/cadence-ecosystem#649).


Generated by Claude Code

Upstream fixed the out-of-range w_N / N panic with the same filter as
fork PR #81 (herdrdev#5171). The 2026-10-12 sync kept the fork's copy of the
line and its comment, so the function still diverged from upstream
with no change in behavior.

parse_workspace_id now matches upstream master exactly. The fork's
request-path tests and the hand-rolled guards stay. The ledger marks
the #81 entry retired for the parser filter.

refs #77

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016MXBYW3meTL95gcAeo2HFm
@cameronsjo cameronsjo self-assigned this Oct 20, 2026
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 53 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 60 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Essentials
  • Run ID: 6c01fdb6-9920-4ee7-877a-ea5f15eb2e00
📥 Commits

Reviewing files that changed from the base of the PR and between 0919977 and 30093a9.

📒 Files selected for processing: 2
  • docs/fork/CHANGES.md
  • src/app/ids.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

…workspace-id

docs/fork/CHANGES.md conflict only: this branch and master (#115,
#116, #117, #118, #119) each added a new section at the same spot.
Kept all of them whole, this branch's section first because the ledger
is newest first. The retired note on the #81 entry merged without a
conflict.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016MXBYW3meTL95gcAeo2HFm
@cameronsjo
cameronsjo merged commit b853848 into master Oct 21, 2026
6 checks passed
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