Repository navigation
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change makes Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to No unresolved issue is established; the change is ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
Collaborator
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
Contributor
 |
Collaborator
Author
|
Regarding CodeRabbit’s docstring-coverage warning: this changes a private lookup helper and adds a regression test, not a public API. The inline comment documents the membership-precedence rule; adding docstrings solely to meet the generated coverage percentage would not clarify this fix. |
akbash-bot
force-pushed
the
akbash/4293-preserve-worktree-membership
branch
from
September 17, 2026 14:55
2269665 to
2b4e44b
Compare
Collaborator
Author
|
The regression fixture now uses the checkout path returned by Git, matching the real create/open flow on macOS and Windows. Full local checks pass again. Please review the updated head. @coderabbitai review |
akbash-bot
force-pushed
the
akbash/4293-preserve-worktree-membership
branch
from
September 17, 2026 15:06
2b4e44b to
f77b753
Compare
|
✅ Action performedReview finished.
|
Collaborator
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
Collaborator
|
@coderabbitai review |
|
|
JJLiebig
self-requested a review
September 20, 2026 14:42
JJLiebig
approved these changes
Sep 20, 2026
JJLiebig
left a comment
Collaborator
There was a problem hiding this comment.
Validated locally on two machines, lgtm
kaushikgopal
added a commit
to kaushikgopal/herdr
that referenced
this pull request
Sep 21, 2026
Merged upstream/master b88e811..2836010. Zero conflicts: all six overlapping files split into disjoint regions (mouse.rs navigator scrollbar/row-click work, state.rs navigator fields and close-tab confirmation, config/model.rs clear_pane keybinding, config-reference.json keys.clear_pane, AGENTS.md detection policy, tests/mod.rs mod close_tab). No src/protocol/wire.rs change, so no protocol conflict. Notable upstream fixes absorbed: - distinguish agent completion from startup and session changes (herdrdev#4457) - honor droid scrollback clear requests in panes (herdrdev#4432) - refresh forwarded ssh agents after reconnect (herdrdev#4443) - index navigator tabs and panes per endpoint (herdrdev#4426) - preserve session layouts across shutdown and restore failures (herdrdev#4400) - confirm closing the last tab in the tui (herdrdev#4409) - reject terminal-less attach before starting a session (herdrdev#4395) - drain event subscriptions and report history loss (herdrdev#4225) - preserve explicit worktree workspace membership (herdrdev#4301) - keep workspace navigation visible in terminal theme (herdrdev#4408) - preserve delayed mouse reports with confirmed keyboard input (herdrdev#4247) - detect codex status with custom interrupt keys (herdrdev#4196) - detect kiro status from live controls and osc signals (herdrdev#4372) - recover the cursor during animated redraws (herdrdev#4404) - stabilize windows cursor redraws and settle deadlines (herdrdev#4389) - show every agent and terminal in the go to picker (herdrdev#4384) - add configurable pane screen and scrollback clearing (herdrdev#4383) - reveal selected agent when cycling the sidebar (herdrdev#4355) - preserve request ids in socket error responses (herdrdev#4353) - enable ssh compression for remote connections (herdrdev#4340) - correct grok activity detection with custom or disabled osc signals (herdrdev#4337) - preserve windows mouse capture during refresh (herdrdev#4319)
aryrabelo
added a commit
to aryrabelo/bora-herdr-ada
that referenced
this pull request
Sep 24, 2026
* fix(windows): avoid lossy native Win32 input mode (#4133)
refs #3932
refs #3702
refs #3910
* fix: keep modified enter compatible with legacy panes (#4110)
* fix: keep modified enter compatible with legacy panes
refs #4103
* fix: synchronize modified key negotiation state
refs #4103
* test: cover windows legacy modified enter
refs #4103
---------
Co-authored-by: Jonathan Liebig <jonathan.liebig@gmail.com>
* fix: avoid resending terminal cells for metadata updates (#4137)
* feat: add cursor-based editing to text inputs (#3698)
* feat: add cursor-based editing to text inputs
refs #1803
* fix: preserve dialog actions with associated text
refs #1803
* refactor: simplify text editor outcomes and tests
refs #1803
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: protect integration configs from failed writes (#3973)
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
* fix: atomically write integration configs
refs #3970
---------
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: map pixel mouse by integer cell pitch (#4140)
refs #3295
* fix: reject stale pane surfaces after reconnect (#4141)
* test: deflake recent CI failures (#4139)
* fix(windows): batch input trace diagnostics (#4146)
refs #3932
* fix(windows): prefer VT input for SSH clients (#4148)
refs #3932
* fix: reveal the full last tab in overflowing tab strips (#4152)
refs #4151
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
* fix: reveal newly focused workspaces in the machines sidebar
* fix: propagate focused pane moves to attached clients (#4159)
refs #4153
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* test: wait for runtime readiness in integration tests (#4168)
Root cause:
The handoff test starts an agent immediately after workspace creation,
before the new shell is necessarily ready. The reconnect test assumes a
fixed sidebar row and searches raw ANSI output for text that can arrive
as separate incremental cell updates.
Fix:
Wait for a shell builtin to write a readiness marker before sending the
single raw agent.start request. Exercise the gap with a delayed shell.
Locate the recovered workspace in the rendered sidebar and check the
reconstructed screen for selection and input, including overlay borders.
Do not extend existing timeouts or add retries that hide failures.
Validation:
All 25 affected group and helper tests pass. Ten repeated rounds of the
two runtime tests and the row helper pass, for 30 test executions without
retries. Standalone native just ci passes 3582 Rust tests with six default
skips, formatting, all-target Clippy, 106 maintenance tests, six architecture
tests and 39 integration-asset tests. Windows cross-lint was not run because
its SDK is not installed.
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: reconcile client locations after no-focus pane moves (#4171)
refs #4153
Root cause:
A no-focus pane move can remove the source tab without changing the
server's numeric workspace and tab coordinates. PaneMove is not classified
as a topology change, so a client viewing that tab retains its deleted ID.
Its target lookup falls back to the server default and can follow a later,
unrelated focus change instead of staying in the source workspace.
Fix:
Include PaneMove in the existing client-location reconciliation path.
Extend the existing focus-guard test with a client on the removed tab and
verify that a later server-default change cannot move its view. Preserve
the other client's view and existing error, no-op and zoom assertions.
Validation:
The minimized regression fails five times on PR #4159's unchanged head and
passes five times with the one-line fix. All 19 pane-move tests, native
lint, maintenance, architecture and integration-asset checks pass.
Full native just ci on the publication branch, including the unchanged
prerequisites from #4159 and #4168, passes 3585 Rust tests with six default
skips plus lint, 106 maintenance, six architecture and 39 integration-asset
tests. All seven docs contract tests pass. Local Windows cross-lint was
not run because its SDK is unavailable.
No geometry policy, protocol or periodic rendering behavior is changed.
* fix: preserve live selections during pane output (#4193)
refs #3841
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
* fix(windows): preserve sgr mouse encoding after native capture (#4195)
refs #4155
* fix(windows): normalize pane launch cwd casing (#4199)
* fix(windows): reject dynamic msvc runtime in packaging (#4200)
The v0.8.2 Windows archive shipped an herdr.exe that imported
VCRUNTIME140.dll and the api-ms-win-crt-* Universal CRT, none of which the
archive provides. Static CRT linkage was restored for x86_64-pc-windows-msvc
in .cargo/config.toml, but nothing verified the packaged executable, so the
dependency could silently regress.
Parse the PE import and delay-import tables and reject herdr.exe when it
depends on the dynamic Microsoft C/C++ runtime, including release and debug
variants. Check the input while staging and re-check the staged executable
before archiving so a direct archive cannot bypass the guard.
refs #3129
* fix: preserve unloaded sessions before replacement (#4190)
refs #4125
* fix: preserve macos services across logout (#4202)
refs #4100
* chore: approve vinayshah1998 as contributor
* fix: keep remote surface updates incremental (#4209)
refs #3769
refs #3745
* fix: keep local usable when remote machines stall (#4234)
refs #3903
* fix: let local agent clicks cancel remote handoffs (#4236)
refs #3903
* fix: require exact session names before deletion (#4233)
* fix: require exact session names before deletion
refs #3819
* test: isolate session deletion regression tests
refs #3819
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* test: wait for app readiness before late session injection (#4224)
Root cause:
The API socket is bound before App evaluates session restore. Waiting only
for that socket lets the late-session test inject its file during startup,
so the server may load it normally instead of exercising late-file recovery.
The missing backup then fails an otherwise correct persistence assertion.
Fix:
Wait for a read-only workspace-list request to pass through App and assert
that the workspace list is still empty before injecting the session file.
Keep the existing autosave deadline and byte-for-byte backup assertion.
Add no sleeps, retries, longer timeouts or production changes.
Validation:
The original assertion fails with unchanged master. With the barrier, five
independent runs pass. Full native just ci passes 3624 Rust tests with six
default skips, formatting, all-target Clippy, 112 maintenance tests, six
architecture tests and 39 integration-asset tests. All seven docs contract
tests pass. Local Windows cross-lint was not run because its SDK is absent.
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: offer forced removal for worktrees with submodules (#4238)
refs #1797
* ci: promote stable releases from protected previews (#4241)
* fix: address release promotion review findings
* fix: avoid workstation sdk requirement in preview checks
* docs: publish preview documentation
* release: synchronize metadata for v0.9.1
* fix: respect opencode tui.json registrations (#4250)
refs #4240
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: cache saved machine connection metadata (#4256)
refs #4252
* fix(windows): preserve VT input during mouse refresh (#4257)
refs #4251
* fix(windows): restore terminal after setup errors (#4261)
refs #4251
* fix(windows): recover VT input without changing readers (#4269)
refs #4251
* test: make windows config backup legacy fixture explicit (#4290)
backup_preserves_legacy relied on a freshly created temp file already having a legacy DACL (D:(...)). Directories that grant inheritable ACEs produce D:AI(...) instead, so the fixture failed before exercising the backup path. Install the legacy DACL explicitly, preserving the existing ACEs while clearing the auto-inherit and protected control bits.
* fix(windows): temporarily revert VT only mode on Windows (#4295)
refs #4251
* fix(windows): prefer pwsh for the default pane shell (#4297)
* fix(windows): prefer pwsh for the default pane shell
Windows panes launched Windows PowerShell 5.1 (powershell.exe) whenever [terminal] default_shell was unset, ignoring PowerShell 7 even when pwsh.exe resolved on PATH. Resolve the unset default against PATH and prefer pwsh.exe, falling back to the inbox powershell.exe. An explicit default_shell still wins.
* fix(windows): validate pwsh before using it as the default shell
portable-pty resolves the configured shell with Path::exists and passes that path to CreateProcessW, which does not fall through to later PATH entries. Selecting an invalid pwsh.exe would therefore break new panes instead of falling back. Only prefer a pwsh.exe that starts with the PE MZ magic, and return its path so the validated binary is the one launched.
* fix(windows): validate the full PE header before preferring pwsh
Checking only the DOS MZ signature still accepted truncated images, DLLs, and foreign-architecture binaries. Because portable-pty passes the resolved path straight to CreateProcessW without trying later PATH entries, validate the PE signature, the COFF header (machine, executable-image bit, not a DLL, section count), and the optional header before preferring pwsh.exe.
* fix(windows): accept native ARM64 pwsh and validate the section table
Herdr ships an x64 Windows build that also runs on Windows ARM64 under x64 emulation, so cfg!(target_arch) cannot tell whether a native ARM64 pwsh.exe is launchable. Accept every executable machine type Windows supports instead of rejecting ARM64 on the x86_64 build. Also bound-check the section table (40 bytes per declared section) after the optional header so a truncated table falls back instead of being selected.
* fix(windows): detect the native host machine for pwsh compatibility
Machine compatibility was derived from cfg!(target_arch), which reports the emulated x64 process on Windows ARM64, and then briefly accepted ARM64 on every host. Read the native machine with IsWow64Process2 in the platform layer instead: ARM64 Windows accepts ARM64/x64/x86 images, x64 accepts x64/x86, and x86 accepts only x86.
* test: skip windows symlink tests without symlink privilege (#4311)
Windows refuses file symlinks with ERROR_PRIVILEGE_NOT_HELD (1314) unless
the test process is elevated or Developer Mode is enabled, which broke
non-admin local runs. Share a helper that reports the missing privilege so
the two symlink tests skip only the symlink-dependent assertions.
* test: add local windows input gauntlet (#4298)
* test: add local windows input gauntlet
refs #4251
* test: add local windows input gauntlet
Harden elevation and channel identity checks, distinguish known host limitations, and surface incomplete cleanup.
refs #4251
* test(windows): harden input gauntlet execution
refs #4251
* test(windows): make input gauntlet self-contained
refs #4251
* test(windows): bound input gauntlet failure paths
refs #4251
* test(windows): extend input qualification gauntlet
refs #4251
* test(windows): complete input qualification matrix
refs #4251
---------
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: Jonathan Liebig <jonathan.liebig@gmail.com>
* fix(windows): preserve mouse capture during refresh (#4319)
* fix(windows): preserve mouse capture during refresh
refs #4284
* test(windows): fix default input gauntlet invocation
* fix(windows): preserve pixel mouse transitions
refs #4284
* fix: correct grok activity detection with custom or disabled osc signals (#4337)
refs #4333
* test: focus agent detection coverage on engine contracts (#4338)
* test(windows): qualify remote clipboard image paste (#4324)
* test(windows): qualify remote clipboard image paste
refs #4314
* test(windows): verify staged clipboard image contents
refs #4314
* test(windows): safely clean failed clipboard leases
refs #4314
* perf: enable ssh compression for remote connections (#4340)
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: preserve request ids in socket error responses (#4353)
* fix: preserve request ids in socket error responses
refs #4344
* docs: clarify socket error response limits
refs #4344
---------
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
* fix: reveal selected agent when cycling the sidebar (#4355)
* fix: reveal selected agent when cycling the sidebar
* fix: reveal agent after endpoint activation
* test: isolate integration assets from inherited omp environment (#4351)
refs #4346
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: Can Celik <ogulcancelik@gmail.com>
* feat: add configurable pane screen and scrollback clearing (#4383)
* feat: show every agent and terminal in the go to picker (#4384)
* fix: stabilize windows cursor redraws and settle deadlines (#4389)
* fix: prevent cursor jumps during synchronized redraws
refs #4303
* fix: settle cursor positions across delayed windows redraws
refs #4303
* fix(windows): hold jump-shaped cursor moves through the max window
* fix: bound windows cursor settling and repaint at its deadline
refs #4303
---------
Co-authored-by: Ogulcan Celik <ogulcancelik@gmail.com>
Co-authored-by: JJLiebig <jj@liebig.gg>
Co-authored-by: Axel Karlsson <axelalbertkarlsson@outlook.com>
* test(windows): distinguish clipboard paste origin in input gauntlet (#4394)
* test(windows): distinguish clipboard paste origin in input gauntlet
refs #4314
* test(windows): correct gauntlet paste-origin evidence
refs #4314
* fix: recover the cursor during animated redraws (#4404)
refs #4303
* fix: detect kiro status from live controls and osc signals (#4372)
* fix: detect kiro status from live controls and osc signals
refs #4203
* fix: handle variable-height kiro approval panels
refs #4203
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: detect codex status with custom interrupt keys (#4196)
* fix: detect codex status with custom interrupt keys
* test: remove unverified codex stale-status cases
* fix(detect): match punctuation in codex interrupt hints
Accept parentheses in remapped interrupt binding labels and simplify the cheap prefilter to a single any-gate shared by both manifest copies.
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: preserve delayed mouse reports with confirmed keyboard input (#4247)
* fix: preserve delayed mouse reports with confirmed keyboard input
refs #3480
* fix: capture geometry before replaying buffered mouse input
refs #3480
---------
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: keep workspace navigation visible in terminal theme (#4408)
* fix: keep workspace navigation visible in terminal theme
refs #4300
* fix: clear workspace highlight on directional pane focus
refs #4300
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: preserve explicit worktree workspace membership (#4301)
refs #4293
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
Co-authored-by: Can Celik <ogulcancelik@gmail.com>
* fix: drain event subscriptions and report history loss (#4225)
* fix: drain event subscriptions and report history loss
refs #4178
Root cause:
Each subscription emits at most one matching event per 100 ms poll, while
the shared event history retains only 512 entries. Bursts therefore build
a backlog, and an evicted cursor silently resumes at the oldest retained
event with no indication that earlier events were lost.
Fix:
Drain the bounded retained batch for lifecycle and agent-status streams.
Check for history loss while holding the same lock used to read events,
then return an events_lost error and close only the affected subscription.
Keep event envelopes, request shapes, endpoint codecs and wait behavior
unchanged. Preserve state filters, initial snapshots, event order and the
existing cadence for snapshot-only subscriptions.
Document resubscription and snapshot recovery in the unreleased API docs.
Add regressions for burst draining, startup-window loss, retention bounds,
status ordering and filtering, and unavailable history.
Validation:
Three socket regressions fail before the fix and pass afterward. Thirty-two
focused API tests and five repeated rounds of socket tests pass. The real
current-master comparison receives 516 of 600 events before the fix and
600 of 600 after it in about 0.97 seconds. A paused reader receives the
explicit error, and a new subscription plus snapshot recovers successfully.
Full native just ci on the dependency-inclusive branch passes 3631 Rust
tests with six default skips, formatting, all-target Clippy, 112 maintenance
tests, six architecture tests and 39 integration-asset tests. All seven
docs contract tests pass. No retries or additional exclusions were used.
Local Windows cross-lint was not run because its SDK is unavailable.
* test: cover subscription isolation across platforms
refs #4178
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: reject terminal-less attach before starting a session (#4395)
refs #4393
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: confirm closing the last tab in the tui (#4409)
* fix: confirm closing the last tab in the tui
refs #4379
* docs: clarify last-tab worktree confirmation in translations
refs #4379
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: preserve session layouts across shutdown and restore failures (#4400)
* fix: preserve session layouts across shutdown and restore failures
refs #4320
* test: update subscription fixture for pane restore errors
---------
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* docs: publish preview documentation
* perf: index navigator tabs and panes per endpoint (#4426)
* chore: approve ain3sh contributor accounts
* fix: refresh forwarded ssh agents after reconnect (#4443)
refs #1931
* fix: honor droid scrollback clear requests in panes (#4432)
Remove the droid-specific filter that discarded ED3 before PTY output
reached the terminal parser. Keeping stale scrollback beneath replayed
primary-screen frames duplicated the welcome header and old transcript.
Pass the original bytes to the parser and trackers. Add process-aware
regression coverage across PTY chunk boundaries and preserve ED2/ED3
screen-history separation and alternate-screen isolation.
Co-authored-by: ain3sh <ainesh.chatterjee@gmail.com>
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com>
* fix: distinguish agent completion from startup and session changes (#4457)
refs #3338
refs #3990
refs #3916
* fix: clear inherited terminal and agent session identity (#4461)
refs #4104
* fix: stagger restored agent startups (#4487)
* fix: stagger restored agent startups
refs #4102
* fix: report startup delay changes requiring restart
refs #4102
* fix: track opencode subagent status per pane (#4357)
refs #1362
* fix: preserve hook agent status across live handoff (#4493)
* fix: recognize codex mention completion popups (#4495)
refs #4220
* fix: reuse ssh authentication and add machine recovery commands (#4490)
* fix: reuse ssh authentication and add in-app mfa recovery
refs #3763
* refactor: move ssh recovery to machine cli
refs #3763
* fix: move worktree lookups async (#4492)
* fix: recognize Windows daemons in non-killing jobs (#4489)
refs #4309
* fix: detect npm-installed omp on windows (#4502)
refs #4313
* fix: keep synchronized pane frames atomic (#4508)
refs #2968
* fix: prevent Windows non-text keys from triggering the space prefix (#4510)
refs #4470
Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
* test(windows): capture native console input records (#4520)
refs #4470
* test: wait for complete handoff pid markers (#4538)
* test: wait for complete handoff pid markers
* test: wait for complete handoff pid markers
* fix: prevent cursor flicker on status redraw (#4554)
* fix: prevent cursor flicker on status redraw
* fix: avoid stale cursor during resize
* fix: avoid inferring codex idle from terminal output (#4563)
refs #4507
* fix: match navigator search terms independently (#4535)
* fix: match navigator search terms independently
refs #4273
* docs: remove redundant navigator search note
refs #4273
* fix(client): restore combined begin() convenience after prepare/start split
The merge silently took upstream's two-phase prepare()+start() split for
PendingEndpointActivation under the fork's begin() name, losing the
single-call semantics 17+ existing fork tests were written against (caught
by running the test suite, not by cargo check -- Rust's auto-reborrow from
&mut to & let every call site still compile against the wrong body).
Renames the split entry point back to prepare() (matches its real upstream
semantics and the one production caller in shell_runtime.rs, which needs
the split to interleave abandoning a stale pending activation between
prepare and start). Restores begin() as a #[cfg(test)]-only prepare()+start()
convenience wrapper so none of the existing test call sites need editing.
refs upstream sync b7781a67
* fix(tests): rename remaining upstream binary-name references to bora
tests/auto_detect.rs, tests/host_shutdown.rs, and tests/session_delete.rs
(new upstream test files merged by this sync) still hardcoded the
herdr-dev debug namespace string; caught by
tests/upstream_wiring.rs::no_source_file_references_the_upstream_binary_name.
Also adds the four new upstream-only platform-gated files
(src/platform/linux/{config_file_tests,shutdown}.rs,
src/platform/windows/config_backup.rs and its tests.rs) to
PLATFORM_GATED_ALLOWLIST on both macOS and Linux, caught by
no_source_file_is_orphaned_from_the_build.
refs upstream sync b7781a67
* fix(detect): restore fork's osc_title_idle rule for codex
The merge silently replaced src/detect/manifests/codex.toml with upstream's
version wholesale (identical byte-for-byte to upstream/master, no conflict
marker, because fork's own edit history to this file happened before the
prior sync's base and so carried zero local delta git could detect) --
dropping the fork's own low-priority osc_title_idle fallback rule.
Combined with upstream's unrelated 'fix: avoid inferring codex idle from
terminal output (#4563)' (which changed the state fallback for an
agent==Codex screen with no matched rule from Idle to Unknown), the missing
rule surfaced as 9 real test failures: screens that used to fall through to
an assumed-idle default now correctly report ambiguous state, except the
osc_title_idle rule that should have matched first and reported real idle
evidence was gone.
Restored verbatim from the pre-merge fork manifest (git show
ad415672:src/detect/manifests/codex.toml). All 9 previously-failing codex
detection tests pass again.
refs upstream sync b7781a67
* style: cargo fmt after post-merge fixes
refs upstream sync b7781a67
* fix: add clippy allow justification for test-only begin() convenience
refs upstream sync b7781a67
* fix: clean clippy lints introduced by the merge
Redundant closures (persist/snapshot.rs, server/headless/{lifecycle,render,
retained_surface}.rs, ui/panes.rs), cast-lossless u32/u16->u64 conversions
in the Windows PE-header sniffing helper (pane.rs), and a redundant
to_string()+implicit-clone on an already-owned String error in the new
pane.clear handler (app/api/panes.rs, part of the merged clear-screen
feature). 'just lint' is green.
refs upstream sync b7781a67
* fix(windows): add missing AgentPromptParams fields in windows-only test
Caught only by the Windows cross-target cargo check (this test body is
#[cfg(windows)]-gated so macOS/Linux never compile it); the struct gained
from_pane/when_idle/when_idle_timeout_ms/peer_pid/origin_channel from the
merge and this one windows-only fixture wasn't updated.
refs upstream sync b7781a67
* docs: add binding rule on silent semantic drift in non-conflicting merge hunks
Three instances this sync (activation.rs begin/prepare split, worktrees.rs
async-discovery rename, codex.toml whole-file silent replacement) where a
correctly-resolved conflict marker sat next to a DIFFERENT non-conflicting
hunk that changed real behavior without tripping cargo check, caught only
by running the affected test module.
refs upstream sync b7781a67
* fix(protocol): decode force_full_repaint in the surface-delta codec
decode_frame() hardcoded force_full_repaint: false instead of reading it
off the wire, but the serde-based encoder (message()) writes the real
bool value unconditionally (FrameData has no skip_serializing_if on it,
per the bincode-is-positional rule in AGENTS.md). Every encoded surface
delta therefore carried one extra trailing byte per frame (main frame and,
when present, popup frame) that the hand-written bincode::Decode reader
never consumed, desyncing every field read afterward on that connection -
observed as unrelated enum/collection decode errors ('found: 5' on
SurfaceGraphicsSource, 'popup cell update has no popup metadata',
'InvalidIntegerType'). Fixed by decoding the trailing bool in decode_frame
at the same position FrameData actually serializes it (after graphics).
Also updated the surface_delta_v1_binary_fixture_is_frozen digest: it is a
genuinely correct shape change (this fork keeps force_full_repaint, already
covered by the PROTOCOL_VERSION 22->23 bump), not a bug - same class as the
two already-updated frozen hashes in wire.rs.
refs upstream sync b7781a67
* test: port upstream's dropped OpenCode-disabled and Windows socket-marker-path coverage
The fork's shared herdr-agent-state.test.ts rewrite moved to a single
shared-import-at-module-load harness and, in doing so, dropped upstream's
entire generic `integrations`/`socketPlugins` parametrized coverage for:
- the "stays disabled without HERDR_SOCKET_PATH" guard (every integration)
- the win32 marker-path -> named-pipe endpoint mapping (every integration)
Ported the OpenCode pair into opencode/herdr-agent-state.test.ts next to its
existing per-test-fresh-import harness (captures the real connection path
instead of ignoring it). Ported Pi/Oh My Pi/Kilo's marker-path case into a
new windows-socket-marker-path.test.ts using mock.module (a raw net.
createConnection mutation, matching upstream's exact original technique,
corrupted an unrelated omp test in a different file when bun ran multiple
test files in one process - confirmed this repo always runs each test file
through its own `bun test <file>` invocation via the justfile, never
combined, so isolated per-file behavior is what actually matters).
Writing this test surfaced a real, pre-existing (predates this sync) bug:
pi/herdr-agent-state.ts never mapped HERDR_SOCKET_PATH through the win32
named-pipe convention every other integration (omp, kilo, opencode) uses -
it called createConnection(socketPath) directly, so Pi users on Windows
would never actually connect. Fixed by adding the same socketEndpoint
mapping upstream's own copy of this file already has, and aligning the
import to the same `net.createConnection` style as the other three
integrations (the file's prior `import { createConnection }` named import
is what made this un-mockable and is why no test ever caught it).
refs upstream sync b7781a67
* fix(windows): validate the staged bora.exe, not a stale herdr.exe literal
validate_stage() re-checks the staged executable's MSVC runtime linkage
after staging (it isn't hash-pinned since it changes every build), but
hardcoded the literal "herdr.exe" while stage_bundle() next to it already
stages the binary as "bora.exe" (this fork's rename) - so validate_stage
silently checked a file that was never written and only worked by accident
whenever nothing else in the stage directory happened to be named
herdr.exe. Caught by just check's maintenance-test suite once run to
completion (FileNotFoundError, not a false pass, since no herdr.exe exists
in a bora stage). Updated the swap-and-recheck test fixture to match.
refs upstream sync b7781a67
* fix(protocol)!: delete dead FrameData.force_full_repaint, restore upstream digests
Reviewer finding (M5, highest priority): 8d2fb3c3 re-blessed the frozen
generation-1 surface_delta digest to a fork-shaped value instead of fixing
the actual bug. That codec name ("surface_delta") is shared wire-protocol
surface with upstream herdr servers - a bora client attached to an upstream
remote would decode deltas one byte off, exactly the class of bug 8d2fb3c3
was fixing for THIS fork's own client, just relocated to the interop
boundary instead of eliminated. AGENTS.md "Stable client endpoint contract"
forbids re-blessing a frozen digest instead of matching it.
Root cause: FrameData.force_full_repaint has no producer and no reader
anywhere in this codebase (git grep confirms only decode.rs ever touched
it, always hardcoded to false) - it is dead weight from an earlier,
apparently abandoned repaint-forcing feature. request_full_repaint (the
caller-side half of that feature) was already removed in 27c65c27.
Deleted the field and its ~44 initializers
across 8 files. Restored both wire.rs frozen-digest tests
(client_shell_graphics_payload_codec_is_frozen, and the PaneSurface
roundtrip above it) and surface_delta.rs's frozen fixture to their exact
upstream values (verified by recomputing after the field's removal, not
by trusting upstream's line-adjacent digest, since one of upstream's own
literals had independently drifted). Removed the now-stale "22 -> 23"
comments citing ceo-bora#274 (that PROTOCOL_VERSION bump's stated reason -
retaining this field - no longer applies; kept the version at 26, see the
next commit, for the actually-incompatible removal itself).
refs upstream sync b7781a67
* Revert "fix(detect): restore fork's osc_title_idle rule for codex"
This reverts commit 9fa1f9b36756c381dea3f5c7cb4494dd680b6e05.
* fix(protocol): update hardcoded PROTOCOL_VERSION=25 fixtures to 26
Fallout from the previous commit's version bump: tests/support/mod.rs kept
its own hand-copied CURRENT_PROTOCOL constant instead of referencing
crate::protocol::PROTOCOL_VERSION, and tests/api_ping.rs asserted the
literal value directly. Regenerated docs/next/api/herdr-api.schema.json via
HERDR_UPDATE_API_SCHEMA=1.
refs upstream sync b7781a67
* fix(detect): reconcile manifest test file with upstream's consolidation
Reviewer finding (B1). Two compounding mistakes:
1. The merge resolved src/detect/manifest/tests.rs conflicts by keeping the
fork side wholesale ('ours'), silently resurrecting roughly 37 tests
upstream had deliberately deleted in its own test-suite consolidation
(7201907b 'focus agent detection coverage on engine contracts', plus
9c96f7dd's own 20-line trim) — none of those deletions were fork-specific
drift to preserve.
2. A separate earlier commit on this branch (now reverted) then restored
src/detect/manifests/codex.toml's osc_title_idle rule believing it was a
fork feature the merge dropped. git log -S proves the rule was upstream
AUTHORED (Ogulcan Celik, 82468a0e) and upstream DELETED (9c96f7dd #4563,
fixing herdrdev/herdr#4507 - Codex reported idle while actively working).
The fork never touched this file before this sync (git diff ad415672
42b6afbd is empty), so it carried zero local delta to lose.
Fix: replaced this file with upstream's current (post-consolidation) tests.rs
verbatim, then re-appended only the tests git log -S confirms are genuinely
fork-authored (9 tests: omp_manifest_reads_state_from_title_not_body,
osc_title_state_reads_only_title_rules,
osc_title_state_is_none_for_manifests_without_a_title_idle_rule, and the
6 omp_osc_title_*/omp_selection_prompt_outranks_working_title/
omp_state_less_title_has_no_matched_rule cluster), plus a local osc_explain
test helper (upstream's own copy of it, a thin explain_with_input wrapper,
was deleted alongside the OSC test section it served; every other helper
these tests need - osc_title_state, osc_title_state_for_loaded,
with_manifest_dirs, explain, parse_manifest, loaded_manifest - is still
live, unmoved code). Adjusted one API drift: the fork's
test_config_env_lock() returns a parking_lot::Mutex (no .unwrap() on
.lock()), not std::sync::Mutex.
python3 scripts/agent_detection_manifest_check.py --require-all-published
now exits 0.
refs upstream sync b7781a67
* fix(ci): restore preview.py subcommands and fix workflow_dispatch guard
Reviewer finding (B2). Three non-conflicting upstream hunks silently broke
the fork's workflow_dispatch preview model, which this sync's merge
otherwise deliberately kept (preview.yml stayed on workflow_dispatch, not
upstream's push:tags model):
1. preview.yml's Select preview commit step kept a guard
(if [ "$GITHUB_REF_NAME" != "$tag" ]) that only makes sense when the
workflow is triggered by a tag push. On workflow_dispatch,
GITHUB_REF_NAME is the branch name (main), so the guard failed on every
single run. Removed it.
2. scripts/preview.py's select-commit subcommand (and the
latest_publishable_commit/hidden_subject helpers it needs, which skip
auto-generated docs:/chore: bot commits when picking a preview
candidate) was deleted by upstream's own simplification. preview.yml
still calls it. Restored the subcommand and its helpers from the
pre-merge fork state (git show ad415672:scripts/preview.py); the fork
never touched this file itself so its pre-sync shape came entirely from
upstream's own pre-simplification code, not fork authorship.
3. Upstream also simplified build_notes() to a bare "view changes" link,
dropping the --base-version argument and the conventional-commit
grouping (Added/Fixed/Performance/Maintenance) the fork's preview
release notes use. preview.yml still passes --base-version. Restored
the richer build_notes signature, its helper functions
(commit_subjects/humanize_subject/TYPE_HEADINGS/COMMIT_RE), and the
notes subcommand's --base-version argument.
Also restored the "Prune old preview prereleases" step upstream dropped
along with its own rejected protected-preview model (review m4) - the
fork's dispatch model still publishes unbounded preview releases without
it.
scripts/test_preview.py: replaced the one test asserting upstream's
now-reverted minimal notes format with tests covering the restored rich
format (grouped-by-heading body, base-version line, and the no-commits
fallback), and kept every other test upstream's consolidation improved
(hotfix/stable range-base tests, asset archiving). 12/12 pass.
refs upstream sync b7781a67
* fix(release): restore fork's simple release-prepare/publish recipes
Reviewer finding (B3). The merge silently replaced the fork's
release-prepare/release-publish/release justfile recipes with upstream's
preview-promotion versions (new $version $preview positional args,
scripts/release.py check-source calls, and a new preview recipe that
fetches origin/master and calls scripts/release.py preview-source) even
though the conflicted hunks looked resolved - only the recipe bodies
conflicted; the signatures and the check-source call lines merged silently
non-conflicting.
scripts/release.py implements a two-stage preview-then-promote model this
fork doesn't use (it hardcodes herdr-* asset names, defaults to
herdrdev/herdr, expects origin/master, and requires annotated release tags
with Preview:/Previous-Stable: trailers the fork's own tag command never
writes) - none of that is reachable through any justfile recipe now.
Restored the three recipes verbatim from 42b6afbd:justfile (single
$version argument, no external release.py dependency, the fork's own
ancestry/clean-tree/version-match checks) and dropped the preview recipe
entirely - the fork publishes previews via preview.yml's workflow_dispatch
trigger, not a pushed tag.
Known gap, out of scope for this fix: .github/workflows/release.yml's
validate-release-source and update-nix-package jobs both still gate on
`github.repository == 'herdrdev/herdr'` (present since before this sync,
predates it) - both are dead on this fork's actual repository regardless
of scripts/release.py's own internal incompatibilities. Flagging for a
follow-up since fixing the gate without also reworking check-tag's
ASSETS/repo/branch/tag-trailer assumptions would turn a silently-skipped
job into a job that runs and always fails.
refs upstream sync b7781a67
* fix(ci): unwire release-workflows.test.ts pending a fork-shape rewrite
Reviewer finding (B4). This upstream-added test asserts an upstream-only
publishing model (tag-push triggers, herdr-* asset names, actor rechecks
gated on github.event_name push) this fork does not use, so it fails 4/5
on every run through maintenance-test (reachable from every just check and
just ci CI leg). Removed it from the maintenance-test recipe with a
comment explaining why; kept the file for reduced conflict surface on the
next upstream sync rather than deleting it.
refs upstream sync b7781a67
* fix(docs): restore docs/preview wholesale from origin/main
Reviewer finding (B6). AGENTS.md: docs/preview is Preview-CI-owned generated
output and must never be hand-edited or three-way merged - the exact source
mismatch the merge produced (40 files hybridized between the fork and
upstream's snapshot at merge time) is what node scripts/docs/preview.mjs
check now flags as an integrity failure, and Preview itself cannot repair
it while B2 (workflow_dispatch guard) stood. Restored the entire directory
from origin/main (42b6afbd) wholesale, matching the rule; the next real
Preview run is the only legitimate way this content changes.
node scripts/docs/preview.mjs check now reports the same snapshot digest
as origin/main.
refs upstream sync b7781a67
* fix(deps): minimally resolve Cargo.lock instead of full regeneration
Reviewer finding (M1). 8d2fb3c3 deleted the conflicted Cargo.lock and ran
cargo generate-lockfile, which re-resolved the whole dependency graph and
silently upgraded 92 crates that neither this fork nor upstream asked for
in this sync (tokio, clap, libc, mio, serde_json, time, uuid,
wasm-bindgen, and more) - an unreviewed, unbisectable dependency bump
riding inside a merge commit.
Restored origin/main's Cargo.lock byte-for-byte as the starting point and
let cargo resolve only the three new upstream dependencies
(unicode-segmentation, zbus, futures-util) plus their transitive closure.
Verified by diffing package/version pairs between the two locks: bora's
own version is the only pre-existing package whose primary pin changed
(0.49.2 -> 0.49.3, matching Cargo.toml); every other diff is either a
brand-new package (zbus and its 24 transitive dependencies) or a second,
additional version of a crate already pinned (syn, toml_edit,
toml_datetime, winnow each keep their old pinned version AND gain a
separate newer entry, because zbus's proc-macro chain needs a newer major
version than the rest of the tree already used - cargo lockfiles support
this natively without touching the existing pin). Zero unrelated crates
moved.
refs upstream sync b7781a67
* docs: restore fork's release-channel model and fix stale repaint mechanism
Reviewer findings (M2, m9). Two unrelated AGENTS.md sections drifted from
upstream's non-conflicting hunks:
1. Release Channels (~line 796) described upstream's tag/admin/preview-
promotion flow (previews select a commit from master and are tag-
triggered on preview-* pushes, just preview <ref>/just release
<version> <preview-tag>, release-tags ruleset, curated categorized
preview notes) - none of which this sync's merge actually kept in the
workflows (B2/B3 restored the fork's workflow_dispatch preview and
single-argument release recipes). Replaced with the fork's own
dispatch-model paragraphs (git show 42b6afbd:AGENTS.md), correcting
two pieces of pre-existing staleness while already touching this text:
default branch is main not master, and the published asset/binary
names are bora-* not herdr-* (both verified against the actual current
workflow files and package script this session).
2. The "layout changes must force a repaint" binding rule (~line 67)
still named AppState::request_full_repaint()/FrameData.force_full_repaint
as the routing mechanism; that field was deleted from the wire protocol
this session (see the force_full_repaint removal commit) and its
caller-side half was already gone before this sync (27c65c27) - the
whole described mechanism has had zero producers or readers for a
while. Rewrote the rule to point at the actual current mechanism
(outcome.repaint on ClientShellInput, consumed by compose() in the
client-owned 0.9.0 shell architecture) while keeping the original
incident history as context for why the constraint exists.
refs upstream sync b7781a67
* fix(client): confirm client-side before closing the last tab of a group
Reviewer finding (M3). open_close_confirmation had an early return -
"if tab_id.is_some() && closes_group { return false; }" - deferring to
what its own comment called "the existing server confirmation path": the
server used to answer a TabClose that would cascade-close a worktree
group with a confirmation_required error, which the client would then
turn into a confirm dialog. That server behavior was removed in
2d839ce6 (close_selected_workspace only ever closes the one selected
workspace now, unconditionally, no confirmation round trip). With no
server-side confirmation left and this client-side path disabled for
exactly the case that needed it, closing the last tab of a non-linked
group-root workspace in Repo view skipped confirmation entirely even with
confirm_close: true.
Removed the early return so this case reaches the same dialog-building
code every other close path already uses. accept_close_confirmation's
tab_target branch sends a plain TabClose (not a cascading WorkspaceClose),
matching upstream's own current close_tab.rs test expectations for this
exact scenario (last_tab_close_preserves_parent_group_and_linked_workspace_scope,
both linked and non-linked branches resolve to a plain tab close after
one Enter).
Updated the fork's own version of that same test: its non-linked branch
still encoded the removed server round trip (asserted an immediate
uncomfirmed TabClose, then manually injected a confirmation_required
endpoint error to drive the dialog open). Since the server can no longer
send that error, this was pinning dead behavior. Rewrote it to assert the
dialog opens immediately with title "Close worktree group?" and Enter
sends the tab close - the linked branch (which was already correct,
closes_group is false there since a linked closing workspace never joins
a group) is unchanged.
refs upstream sync b7781a67
* fix(input): let host-reply discard-arming preempt disambiguation hold
Reviewer finding (M4). flush_timeout()'s kitty-keyboard disambiguation hold
(host_escape_disambiguation_active && starts_with_bounded_incomplete_escape_sequence)
ran before the host-cell-size and color-scheme discard-arming branches
below it. starts_with_bounded_incomplete_escape_sequence accepts "\x1b["
plus any byte in 0x20..=0x3f - the same digit/`;` range a truncated
XTWINOPS cell-size reply's parameters are made of - so with disambiguation
active, a truncated host reply head (e.g. "\x1b[6;21" after its terminator
never arrived) matched the broader disambiguation check first and got held
forever instead of reaching the more specific check that would arm
discard_until and let it actually get discarded. Whatever the user typed
next got appended to that same held buffer and never reached the pane as
real keystrokes.
Fixed by excluding the disambiguation hold specifically when we are
awaiting a matching host reply (host_cell_size_replies_awaited with a
buffer matching starts_with_incomplete_host_cell_size_report, or
host_appearance_reply_awaited with starts_with_incomplete_host_color_scheme_report),
deferring to those more specific checks instead. Kept disambiguation's
hold in its original position for every other case (an earlier attempt
that moved the whole check to the end of the function broke the SGR-mouse
disambiguation-hold test, since some ambiguous SGR mouse prefixes also
overlap the host-reply-specific checks that would then run first).
Added a regression test mirroring the existing
malformed_host_reply_tail_preserves_following_input test with
disambiguation active - reproduced the bug first (typed characters after
the reply's terminator came back empty instead of ["a","b","c"]), then
confirmed the fix restores it. Full raw_input test suite: 110/110 pass,
including the disambiguation-vs-SGR-mouse test the first (reverted)
attempt broke.
refs upstream sync b7781a67
* fix(persist): route snapshot-history reads through the tolerant parser
Reviewer finding (m5). preserve_snapshot_history() parsed the pre-existing
on-disk session file with a strict serde_json::from_slice::<SessionSnapshot>,
bypassing the tolerant RawSessionSnapshot boundary (parse_snapshot in
snapshot.rs) that view_mode is specifically designed to go through - an
unrecognized value (a retired variant like the old ViewMode::Project,
ceo-bora#270, or a newer value written by a future build) fails a strict
parse for the WHOLE document and silently skips archiving an otherwise
perfectly readable session into history. Recovery history existing for
exactly this kind of migration edge case is the point of preserving it.
Routed both call sites (the current file and, when comparing layout
fingerprints to decide whether to skip a duplicate rotation, the previous
snapshot file) through parse_snapshot() instead. The explicit
version-too-new check became redundant (parse_snapshot already rejects
it) and was removed.
Added a regression test: writes the fixture session with view_mode
overwritten to the retired "project" string directly to the writer's
path, then asserts save() still produces a snapshot-history entry instead
of silently skipping it.
refs upstream sync b7781a67
* fix: rename remaining herdr binary-name leaks to bora
Reviewer findings (m1, m2). New user-facing hints/usages this sync's merge
brought in still said the upstream binary name:
- cli/machine.rs: "usage: herdr machine status/reconnect" (x2), the
reconnect terminal-required hint
- client/shell/machine_diagnostics.rs: the two client-side notice actions
that echo those same commands back to the user
- session.rs: "usage: herdr session attach" (x2, both branches) and the
"use the spelling shown by `herdr session list`" hint
m2: tests/session_delete.rs:36 and tests/auto_detect.rs:248 built their
own app_dir path with `if cfg!(debug_assertions) { "bora-dev" } else {
"herdr" }` - 8c623a40 fixed only the debug arm. The real production
app_dir_name() (src/config/io.rs) returns "bora" for the release profile,
matching the fork's own test harness convention elsewhere
(tests/client_mode.rs:416-420 already uses "bora"). Fixed the release arm
in both.
Fixed one regression this caused: a client-side test asserted the literal
old "herdr machine reconnect ..." notice text.
TERM_PROGRAM=herdr (src/pane.rs) is left unchanged per the review: it is
a deliberate ecosystem identifier decision the fork already made (see
AGENTS.md rename-leaks section), not a leak.
Scope note: a broader sweep (cli.rs, cli/workspace.rs, remote.rs,
update.rs and their tests) still has "usage: herdr ..." strings that
predate this sync and were not part of the reviewer's cited evidence;
left untouched as out of scope for this fix, flagged in the sync log.
refs upstream sync b7781a67
* style: cargo fmt after review-round fixes
* fix(tests): replace hardcoded protocol 25 literals with CURRENT_PROTOCOL
Reviewer finding R2-B1 (blocker). tests/cli/sessions.rs still asserted
protocol 25 in six places after the PROTOCOL_VERSION 25->26 bump. The
file only compiles under #![cfg(all(unix, not(target_os = "macos")))],
so this could not show up on a macOS run and would only fail on CI's
ubuntu-latest leg. Replaced all six literals with the shared
tests/support::CURRENT_PROTOCOL constant (already re-exported through
tests/cli/harness.rs and in scope via sessions.rs's existing
'use super::harness::*'), matching the convention every other
CURRENT_PROTOCOL-consuming test file already follows, so the next bump
cannot miss them.
Swept the whole tree (every linux/windows-gated file: tests/cli.rs,
tests/machine_api.rs, tests/auto_detect.rs, src/platform/{linux,windows}.rs,
plus everything under tests/cli/) for any other protocol-version 25
literal; none found (the remaining 25s in that set are all
Duration::from_millis(25) timing values, unrelated).
Verified with the linux cross cargo check --all-targets (0 errors).
refs upstream sync b7781a67
* fix(client): close the whole group when a group-root's last tab closes
Reviewer finding R2-M1 (major), a real regression introduced by aa14ae4d.
After aa14ae4d removed overlay_input.rs's early return, a last-tab close
of a non-linked worktree group root reaches open_close_confirmation and
correctly shows "Close worktree group? - N workspaces, M panes", but
accept_close_confirmation always built Method::TabClose whenever the
overlay carried a tab_target - it never looked at whether the dialog it
was confirming was actually the group variant. The dialog's title said
group, the accept sent a single-tab close: the fork server's
handle_tab_close closes only that one workspace, leaving every sibling
worktree in the group open.
aa14ae4d's commit message claimed upstream's own close_tab.rs test
expects a plain TabClose in this branch. That is false: upstream's test
(git show b7781a67:src/client/shell/tests/close_tab.rs, non-linked
branch) expects
Method::WorkspaceClose(params) if params.workspace_id == "ws_1" &&
params.close_group. The fork's own rewritten
last_tab_close_preserves_parent_group_and_linked_workspace_scope test
encoded the same wrong expectation (assert_tab_close in both branches)
and did not catch this. Not correcting via amend, per policy - this
commit is the correction.
Fix: ClientTabCloseConfirmation now carries the closes_group bool
open_close_confirmation already computes when it builds the dialog.
accept_close_confirmation reads it back: when true, it sends
WorkspaceClose{workspace_id, close_group:true} instead of TabClose,
matching what the dialog told the user and what the server's
close_selected_workspace group-close path actually does. Updated the
existing test to assert TabClose only for the linked-worktree branch
(correctly excluded from grouping) and added
assert_workspace_close_group for the non-linked group-root branch.
Verified: cargo check --all-targets clean; cargo nextest -E
'test(close_tab) or test(close_drags)' -> 9/9 pass, including the fixed
regression test.
refs upstream sync b7781a67
* fix(integration): bump pi and omp asset versions past v0.46.9 content changes
Reviewer findings R2-M3 (major) and R2-m1 (minor). Integration asset
versions are migration versions relative to the latest RELEASED tag
(v0.46.9, per distribution/latest.json and AGENTS.md), not relative to
this merge's parents. registry.rs treats installed_version >=
expected_version as Current, so an unmoved marker means every install
made from v0.46.9 silently never gets a real content change.
- pi/herdr-agent-state.ts: content changed since v0.46.9 (this sync
brought in upstream #3729's Windows named-pipe socketEndpoint mapping,
plus 060cdced's own fix to actually wire it for pi). Version marker
was still 9. Bumped to 10 (HERDR_INTEGRATION_VERSION marker and
PI_INTEGRATION_VERSION constant), once.
- omp/herdr-agent-state.ts: content changed since v0.46.9 (upstream
#3994's OMPCODE nested-session guard, landed via the PREVIOUS sync
8ac008ea - predates this merge, but the version was never bumped for
it either). Version marker was still 11. Bumped to 12
(HERDR_INTEGRATION_VERSION marker and OMP_INTEGRATION_VERSION
constant), once.
opencode is intentionally left at 13 (unchanged): it went 11->12->13
across the previous and this sync, i.e. two bumps against the
'bump once' convention, but that comes from matching upstream's own
plugin-path numbering rather than an extra fork bump - noted as an
accepted exception, not fixed further, per R2-m2.
Verified: cargo check --all-targets clean; cargo nextest -E
'test(integration)' -> 190/190 pass.
refs upstream sync b7781a67
* docs(agents): record the TERM_PROGRAM=herdr decision as binding
Reviewer finding m6 (minor, round 2). pane.rs still sets
TERM_PROGRAM=herdr; the earlier fix commit (40105cde) and the sync log
claimed this was left 'per the review's own guidance' citing an
AGENTS.md rename-leaks section, but no such AGENTS.md text existed -
the decision was never actually recorded anywhere. Added it to the
binary-rename bullet under 'Fork merge friction' as an explicit, dated,
binding line: TERM_PROGRAM stays herdr on purpose, because it's an
ecosystem/terminfo identifier other tools key off, not a user-facing
string.
refs upstream sync b7781a67
* fix(tests): update stale pi integration version literal in linux-gated sessions.rs
CI check (ubuntu-latest) failure: integration_commands_run_locally_when_server_is_missing
asserted "pi: current (v9)" after a fresh pi integration install, but
PI_INTEGRATION_VERSION was bumped 9->10 earlier in this sync
(0edb4320) for an unrelated reason (pi's asset content changed since
v0.46.9 with the version marker unmoved). This test file only
compiles on unix/non-macos
(tests/cli/sessions.rs:1 #![cfg(all(unix, not(target_os = "macos")))]),
so a local macOS cargo check/nextest run never saw it - same blind
spot class as R2-B1's protocol-25 literals in the same file, caught
this time by CI's ubuntu-latest leg instead of locally.
Swept the rest of this file and the other macOS-gated files
(tests/auto_detect.rs, tests/machine_api.rs, tests/cli.rs,
src/platform/{linux,windows}.rs) for other hardcoded
'current (v'/'outdated (v'/pi:/omp: literals; this was the only one.
The other protocol-version assertions in this same file already
derive from CURRENT_PROTOCOL (R2-B1).
Verified: cargo check --target x86_64-unknown-linux-gnu --all-targets
clean. Cannot run this test locally (macOS host); relying on CI's
check (ubuntu-latest) leg to confirm the runtime assertion.
refs upstream sync b7781a67
* fix(tests): allowlist new macos-only child files for the linux orphan-file check
CI check (ubuntu-latest) failure: no_source_file_is_orphaned_from_the_build
flagged src/platform/macos/bootstrap.rs and
src/platform/macos/config_file_tests.rs as unreachable from the Linux
build. Both are new upstream files added by this sync (commits
266b8e38, 5e38e5fb), correctly mod-declared inside macos.rs
(compiles fine on macOS and both cross-target checks), but macos.rs
itself is the platform-gated root on Linux and its own allowlist
entry does not cover its children - the same pattern already handled
for linux.rs's children (config_file_tests.rs, shutdown.rs) in the
macOS-side allowlist, just missing the symmetric entries on the
Linux side for these two newly-added files.
Swept every other new file from the upstream merge
(git diff --diff-filter=A ad415672..upstream/master -- 'src/**/*.rs')
for the same class of gap: the only other platform-gated new files
(platform/linux/{config_file_tests,shutdown}.rs,
platform/windows/config_backup{.rs,/tests.rs}) were already correctly
allowlisted on both sides. platform/ssh_agent.rs is #[cfg(unix)], so
it needs no entry in either list (both macOS and Linux are unix).
Verified: cargo check --target x86_64-unknown-linux-gnu --all-targets
clean; cargo nextest -E 'test(no_source_file_is_orphaned_from_the_build)'
passes locally on macOS (exercises the macOS-side branch of the same
test, unaffected by this Linux-side edit). Cannot exercise the
Linux-side branch locally; relying on CI's check (ubuntu-latest) leg.
refs upstream sync b7781a67
* fix(scripts): rename herdr.exe -> bora.exe in the windows input qualification tool
Cubic review finding (P2, confidence 10): 'just test-windows-input' cannot
run in this fork by default. scripts/test_windows_input.ps1 is a brand-new
upstream file (this sync), never existed on origin/main before. Its default
-ExePath resolution built and looked for release/herdr.exe, but this fork's
Cargo.toml renames the [[bin]] to bora, so cargo build --release produces
bora.exe; the staged package name from this fork's own
scripts/package_windows_conpty.py (unchanged, already correctly renamed) is
also bora.exe (confirmed: staging / "bora.exe" at package_windows_conpty.py:275).
So the default invocation path threw a file-not-found before ever launching
a qualification run.
This is exactly the 'binary rename in user-facing strings' class named in
AGENTS.md's Fork merge friction rule, so in scope for this sync unlike the
deeper upstream-authored logic bugs cubic also flagged in this same tool
(mouse-mode cleanup on early exit, exit-status validation) which are left
alone: this script only runs manually, is explicitly excluded from CI
('deliberately excluded from normal CI' per its own header comment), and I
have no Windows host to exercise or verify a deeper behavioral fix.
Left scripts/windows_input/report.py's 'herdr'/'herdr-remote' internal
matrix path-identifier literals alone: those are data-level case keys
threaded through report.py's own matrix/report generation logic, not a
display string, and renaming them would require touching upstream's test
taxonomy well beyond a filename swap with no way to verify the result.
Verified: no pwsh available locally to syntax-check; this is a narrow
string-literal change, cross-checked against package_windows_conpty.py's
actual output filename. python3 -m unittest scripts.test_windows_cross
still passes (unrelated but adjacent windows-tooling test, 27/27).
refs upstream sync b7781a67
* fix(server): refuse local attach to a server below MIN_COMPATIBLE_SERVER_PROTOCOL
Independent review finding F-D2 (C35, C88), most important item of the
follow-up round: PROTOCOL_VERSION 26 (29206adf, R2-M2) removed the
FrameData.force_full_repaint field and changed the PaneSurface bincode
shape. The shell.surface.v1 client-shell endpoint codec never compared
PROTOCOL_VERSION at all - only endpoint_protocol_generation and
capability names - so a 0.49.3+ client attaching to an already-running
<=0.49.2 server would silently misdecode every PaneSurface frame
instead of failing the handshake with a clear error. This also broke
docs/next/install.mdx's promise that the client shows a notice and the
old server just needs one final stop.
validate_running_server_compatibility (src/server/autodetect.rs) now
also refuses when the running server's reported protocol is below the
new MIN_COMPATIBLE_SERVER_PROTOCOL = 26 constant, reusing the existing
'needs one final server update' error path/message with a new
requirement string ('the PaneSurface wire encoding changed'). A
missing protocol (None) keeps its pre-existing permissive behavior,
matching every other capability check in this function - extracted as
server_protocol_too_old() and unit-tested directly for 25 (refused),
26 (accepted) and None (accepted, matching today).
Checked the other attach path that validates server compatibility
without going through this function: src/remote/attach.rs's
supports_endpoint_requirement (SSH remote attach) has its own
protocol field on RemoteClientStatusJson, but a pre-existing test
(saved_machine_compatibility_uses_capabilities_not_release_or_private_protocol)
proves by design that this path deliberately ignores it and gates
only on endpoint_protocol_generation/capabilities. I initially applied
the same MIN_COMPATIBLE_SERVER_PROTOCOL guard there too and it broke
that test immediately - reverted. The triage note for C88 independently
confirms remote endpoints install upstream's own herdr binary via the
SSH bridge, whose codec already matches, so the remote path's claim in
install.mdx holds without a code change; only the local same-install
case (this commit) was broken.
Verified: cargo check --all-targets clean; cargo nextest -E
'test(server::autodetect) or test(remote::attach)' -> 17/17 and 83/83
pass, including the new tests and the previously-broken-then-reverted
remote test now green again.
refs upstream sync b7781a67
* fix(remote): rename herdr --remote hints to bora --remote
cubic review finding C17(a). These four pre-existing user-facing
'herdr --remote' hints (present since before this sync, at 42b6afbd)
are the binary-rename class named in AGENTS.md 'Fork merge friction':
this fork installs and ships only bora, never herdr, so a user
following these hints verbatim would run a command that doesn't exist.
- src/remote/saved.rs:115 saved_ssh_bootstrap_command (+ its test
assertion at :194)
- src/remote/attach.rs:1292 find_installed_remote_herdr's error hint
- src/remote.rs:59 print_remote_error_hint's SSH passphrase hint
- src/remote/host.rs:57 (not in the original C17 list, included for
consistency: 'remote herdr server needs one final update... rerun
herdr --remote' is this fork's own compiled code checking its own
remote bora counterpart, not text echoed from the remote side)
Left src/remote/attach.rs:4659-4689 alone: those are
reattach_command()'s own tests, which pass a literal "herdr" *program
name* as a parametrized test input to prove the templating logic
echoes back whatever program name it's given - not an assertion that
the fork's own binary is named herdr. Left the two source-comment-only
references (config/model.rs:1191, client/handshake.rs:28) alone: Rust
doc comments aren't user-facing.
Verified: cargo check --all-targets clean; cargo nextest -E
'test(remote::saved) or test(remote::attach) or test(remote::host) or
test(src::remote)' -> 86/86 pass.
refs upstream sync b7781a67
* fix(docs): drop stale confirmation_required sentence from zh-cn/ja cli-reference
cubic review findings C38, C97. This fork has no server-side
confirmation_required response (removed in 2d839ce6; app/api/tabs.rs
closes a workspace's last tab directly). The English docs/next
cli-reference.mdx already dropped this sentence, but the merge kept
upstream's version of it in the zh-cn and ja translations, so both
described a behavior this fork doesn't have.
Deleted the trailing confirmation_required sentence from tab-close
paragraph :194 in both files so all three languages match; the
following paragraph already correctly describes the TUI confirmation
dialog for both.
Verified: python3 scripts/docs_translation_parity.py --docs-root
docs/next/website/src/content/docs -> clean, exit 0.
refs upstream sync b7781a67
* docs(agents): separate the two request_full_repaint / force_full_repaint deletions
cubic review findings C49, C91. AGENTS.md:67 conflated two different
removals as one: AppState::request_full_repaint() was deleted in
27c65c27 (the 0.9.0 client-shell rewrite, unrelated to this sync), but
FrameData.force_full_repaint (the wire-protocol field with the same
name pattern) was a separate deletion, done in this sync's own
29206adf commit (the PROTOCOL_VERSION 25->26 bump). The bullet's
wording made it look like one commit removed both.
Also AGENTS.md:8: 'Stable/preview both build from master' was stale -
this fork's default branch is main (the merge already fixed the
identical claim at :806 in the Global Contracts section, but missed
this earlier copy).
refs upstream sync b7781a67
* fix(skills): derive pre-release-audit base tag from origin/main, not stale origin/master
cubic review finding C33. origin/master is a stale 2026-08-11 branch
with no distribution/latest.json in this fork's remotes; the step
would fail outright (git show errors) before this sync brought
herdrdev/herdr#4241's audit-skill machine…
colangelo
pushed a commit
to colangelo/herdr-max
that referenced
this pull request
Sep 27, 2026
refs herdrdev#4293 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> Co-authored-by: JJ Liebig <jonathan.liebig@gmail.com> Co-authored-by: Can Celik <ogulcancelik@gmail.com> (cherry picked from commit 6c62707)
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
4 participants
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.
Issue
After a shell in a repository workspace enters a linked checkout, opening that worktree can turn the repository workspace into the worktree's workspace. Removing the worktree then closes the repository workspace and kills processes in its other tabs.
Problem
Checkout lookup accepted a shell's current directory even when the workspace was explicitly assigned to a different checkout. It could select the parent before the actual worktree workspace and overwrite the parent's assignment.
How did we fix it?
Herdr now matches an explicitly assigned workspace only to its assigned checkout. Workspaces without an explicit assignment retain directory-based discovery, including discovery from subdirectories.
Verification
Before the fix, an isolated CLI reproduction closed the parent and killed a second-tab process. Afterward, opening selected the real child, removing the parent was rejected, and removing the child preserved the parent's process. A regression test also failed before and passed afterward.
Local
just check, Linux CI, and macOS CI passed. Both review bots completed without code findings. Windows CI failed in an unrelated remote-bridge test because its descendant PID file was missing.refs #4293