Support herdr as a terminal workspace manager beside tmux #427

Merged
mph merged 58 commits from IW-422 into main 2026-08-22 07:54:50 +00:00
Owner

Closes IW-422.

Adds herdr as a second terminal workspace manager beside tmux, behind a port
that either backend implements. Two phases: phase 1 introduced the port and
moved tmux behind it with no behaviour change; phase 2 adds the herdr backend,
per-process backend selection, and an iw doctor check.

What changes for a user

Nothing, unless they opt in. tmux stays the default. IW_WORKSPACE_BACKEND
selects a manager explicitly; HERDR_ENV=1 selects herdr when the variable is
unset. An unrecognised value is a hard error, never a silent fallback — a typo
must fail loudly.

iw doctor gains a terminal workspace check that reports the manager the
commands would actually drive, and names a bad value when there is one.

Breaking change

SessionContext loses sessionName, and it is plugin-facing, so VERSION
moves 0.9.0 → 0.10.0. kanon needs a lockstep one-line fix at
hook-test/src/ClaudeSessionTest.scala:20; its hook itself compiles unchanged.

Shape

  • core/model/ — the reply decoders and HerdrLookup, which holds the real
    decisions: the checkout path outranks the label, a failure on the path route
    never falls through to the label, and ambiguity is always a failure rather
    than a reason to pick the first workspace.
  • core/adapters/HerdrAdapter and TmuxAdapter, each with pure argv
    builders that both the operations and the printed recovery hints render, so
    an operation and its hint cannot drift. This fixed a real defect on the tmux
    side: the hand-written hint omitted -L <socket>, so under IW_TMUX_SOCKET
    a copy-pasted hint attached to the wrong tmux server.
  • core/commands/ — the two port bindings and the per-process selection.

dashboard/ is deliberately untouched — it has no invoking terminal, so
environment detection cannot work there and it needs a configured backend
instead. That is IW-423.

What was established by probing rather than reasoning

Four open items were closed against a live herdr server, and three of them
corrected the phase context, which had reached its answers by elimination:

  • workspace focus answers workspace_info, not ok.
  • worktree open needs a source, and --cwd supplies it. workspace create
    produces a workspace carrying no worktree object, which the parent lookup
    could never find again, so every run would build another parent.
  • create must ask git for the repo root. HERDR_WORKSPACE_ID answers a
    different question — which repository the caller sits in, not which one the
    worktree belongs to. They differ whenever the worktree is of another
    repository, and create failed until this was found by running it.
  • Focus is server-wide and attaching also switches it, so focus-before-attach
    is correct.

Testing

  • Unit: +102 across both phases
  • Tool contract: +16 herdr cases, gated on IW_CONTRACT_HERDR=1 and herdr on
    PATH. Read-only, because herdr has no socket isolation — the one case that
    writes server-wide focus carries its own opt-in.
  • E2E: every test/*.bats setup() exports IW_WORKSPACE_BACKEND=tmux, landed
    in phase 1 before any herdr adapter existed, so a suite run started inside
    herdr cannot drive the real server.
  • Full gate exits 0 with 190 assertions. core.test 186/186,
    dashboard.test 241/241, dashboard.itest 240/240.

A pre-existing flaky test was fixed on the way: TmuxAdapterTest failed about
two runs in three because it asserted on pane_current_path, which is the pane
process's live cwd and answers with the caller's directory until the login shell
has started. session_path is what tmux fixes from -c at creation. Proven by
mutation and by eleven consecutive green runs.

Review

Six single-skill reviews per phase. Phase 1: zero Critical over two iterations.
Phase 2: five Critical (three distinct defects), all fixed in f453abb:

  • isCurrent compared two Options with ==, so None == None answered true
    and iw rm refused every removal with "you are in its workspace" for a user
    in no workspace at all. Found independently by five of the six reviewers.
  • The doctor check bound a probe that runs which with stdout inherited,
    printing a stray path above every report — a regression against the baseline.
  • exists dropped a failure silently, and iw rm is the one caller with no
    next effectful call, so an unreachable server orphaned a workspace with no
    message.

Two committed fixtures also carried an internal host name and the abbreviated
real capture path; both are redacted and the documented redaction guard is
widened, since a guard built from remembered literals would let the next capture
repeat it.

Cleared by probing rather than assumption: command injection (hostile labels
were sent to the live binary and every one arrived as a value, never a flag),
hostile JSON, environment trust, -Wunused:all, and a full assertion audit —
no assertion was deleted, loosened or downgraded anywhere in the diff.

Review packets: review-phase-01-20260820-111839.md,
review-phase-02-20260821-095022.md.

Still open

LESSONS.md carries entries awaiting triage with Michal, including several
follow-ups that should leave this repository as their own issues. That triage
has not happened yet and should land on this branch before merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD

Closes IW-422. Adds `herdr` as a second terminal workspace manager beside tmux, behind a port that either backend implements. Two phases: phase 1 introduced the port and moved tmux behind it with no behaviour change; phase 2 adds the herdr backend, per-process backend selection, and an `iw doctor` check. ## What changes for a user Nothing, unless they opt in. tmux stays the default. `IW_WORKSPACE_BACKEND` selects a manager explicitly; `HERDR_ENV=1` selects herdr when the variable is unset. An unrecognised value is a hard error, never a silent fallback — a typo must fail loudly. `iw doctor` gains a `terminal workspace` check that reports the manager the commands would actually drive, and names a bad value when there is one. ## Breaking change `SessionContext` loses `sessionName`, and it is plugin-facing, so `VERSION` moves 0.9.0 → 0.10.0. kanon needs a lockstep one-line fix at `hook-test/src/ClaudeSessionTest.scala:20`; its hook itself compiles unchanged. ## Shape - `core/model/` — the reply decoders and `HerdrLookup`, which holds the real decisions: the checkout path outranks the label, a failure on the path route never falls through to the label, and ambiguity is always a failure rather than a reason to pick the first workspace. - `core/adapters/` — `HerdrAdapter` and `TmuxAdapter`, each with pure argv builders that both the operations and the printed recovery hints render, so an operation and its hint cannot drift. This fixed a real defect on the tmux side: the hand-written hint omitted `-L <socket>`, so under `IW_TMUX_SOCKET` a copy-pasted hint attached to the wrong tmux server. - `core/commands/` — the two port bindings and the per-process selection. `dashboard/` is deliberately untouched — it has no invoking terminal, so environment detection cannot work there and it needs a configured backend instead. That is IW-423. ## What was established by probing rather than reasoning Four open items were closed against a live herdr server, and three of them corrected the phase context, which had reached its answers by elimination: - `workspace focus` answers `workspace_info`, not `ok`. - `worktree open` needs a source, and `--cwd` supplies it. `workspace create` produces a workspace carrying no `worktree` object, which the parent lookup could never find again, so every run would build another parent. - `create` must ask git for the repo root. `HERDR_WORKSPACE_ID` answers a different question — which repository the *caller* sits in, not which one the worktree belongs to. They differ whenever the worktree is of another repository, and `create` failed until this was found by running it. - Focus is server-wide and attaching also switches it, so focus-before-attach is correct. ## Testing - Unit: +102 across both phases - Tool contract: +16 herdr cases, gated on `IW_CONTRACT_HERDR=1` and herdr on PATH. Read-only, because herdr has no socket isolation — the one case that writes server-wide focus carries its own opt-in. - E2E: every `test/*.bats` `setup()` exports `IW_WORKSPACE_BACKEND=tmux`, landed in phase 1 before any herdr adapter existed, so a suite run started inside herdr cannot drive the real server. - Full gate exits 0 with 190 assertions. `core.test` 186/186, `dashboard.test` 241/241, `dashboard.itest` 240/240. A pre-existing flaky test was fixed on the way: `TmuxAdapterTest` failed about two runs in three because it asserted on `pane_current_path`, which is the pane process's live cwd and answers with the caller's directory until the login shell has started. `session_path` is what tmux fixes from `-c` at creation. Proven by mutation and by eleven consecutive green runs. ## Review Six single-skill reviews per phase. Phase 1: zero Critical over two iterations. Phase 2: five Critical (three distinct defects), all fixed in `f453abb`: - `isCurrent` compared two `Option`s with `==`, so `None == None` answered true and `iw rm` refused every removal with "you are in its workspace" for a user in no workspace at all. Found independently by five of the six reviewers. - The doctor check bound a probe that runs `which` with stdout inherited, printing a stray path above every report — a regression against the baseline. - `exists` dropped a failure silently, and `iw rm` is the one caller with no next effectful call, so an unreachable server orphaned a workspace with no message. Two committed fixtures also carried an internal host name and the abbreviated real capture path; both are redacted and the documented redaction guard is widened, since a guard built from remembered literals would let the next capture repeat it. Cleared by probing rather than assumption: command injection (hostile labels were sent to the live binary and every one arrived as a value, never a flag), hostile JSON, environment trust, `-Wunused:all`, and a full assertion audit — no assertion was deleted, loosened or downgraded anywhere in the diff. Review packets: `review-phase-01-20260820-111839.md`, `review-phase-02-20260821-095022.md`. ## Still open `LESSONS.md` carries entries awaiting triage with Michal, including several follow-ups that should leave this repository as their own issues. That triage has not happened yet and should land on this branch before merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD
mph added 44 commits 2026-08-21 10:24:28 +00:00
Support herdr as a terminal workspace manager beside tmux

Artifacts:
- analysis.md

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closed by inspection and read-only probe:
- pane id comes from `herdr pane list --workspace`, not `workspace get`
- the IW_* env filter is inert on both backends (os-lib merges env)
- no compatibility alias needed; the plugin API never exposes CommandEnv
- attach is dead code under environment selection

Artifacts:
- analysis.md
- LESSONS.md

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolved:
- analysis.md: Port and operation names -> TerminalWorkspaceOps, env.workspace;
  sessionName dropped from SessionContext (no hook reads it)
- analysis.md: herdr attach path -> attach is real code; selection and isInside
  stay separate concepts so a configured default cannot orphan the branch
- analysis.md: worktree open vs workspace create -> always worktree open under a
  parent workspace, creating the parent when missing
- analysis.md: dashboard TmuxAdapter call -> stays tmux-only; IW-423 filed for
  the configured backend
- analysis.md: backend override -> IW_WORKSPACE_BACKEND names the backend and
  wins over detection

Estimates revised 10-15h -> 13-19h to cover the create path and a real attach.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Support herdr as a terminal workspace manager beside tmux

2 phases, 13-19h:
- Phase 1: Workspace port + tmux backend (5-7h)
- Phase 2: Herdr backend + selection + doctor (8-12h)

Artifacts:
- tasks.md
- review-state.json (tasks_ready)

Also records three review-state CLI defects hit while generating this
index, incl. `--needs-attention false` setting the flag to true.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue: IW-422
Phase: 1 - Workspace port + tmux backend

This commit marks the checkpoint for phase 1 context generation.
Use wf-implement IW-422 to return to this point.

🤖 Generated with Claude Code
Also records the review-state --checkpoint format defect in LESSONS.md.

🤖 Generated with Claude Code
Michal decided Risk 5 in favour of backend-specific manual-recovery hints
over neutral phrasing: a hint exists to be copy-pasted, so it is worthless
once it stops naming a runnable command.

Hints are pure attachHint/focusHint/leaveHint methods on the
TerminalWorkspaceBackend enum, not operations on TerminalWorkspaceOps; the
port gains `backend` so commands can reach them.

🤖 Generated with Claude Code
Michal rejected the enum design: a hint and the operation it describes are one
piece of knowledge, so the domain layer must not hold a second copy of the
command text.

Hints become attachHint/focusHint/leaveHint on TerminalWorkspaceOps, each
adapter rendering from the same argv its operation runs. This uncovered a live
defect - the hand-written hints omit tmux's -L <socket> - now fixed in phase 1
scope and pinned by a pure argv test. Two further copies in the dashboard are
recorded in LESSONS.md for IW-423.

Herdr keeps the same two-layer adapter shape as tmux, since IW-423 gives the
dashboard a herdr caller eventually.

🤖 Generated with Claude Code
Workspace port + tmux backend

Artifacts:
- phase-01-tasks.md

Task generation checked the context against the source and found the
leak-closing test cannot be a harness test: FakeHookOps.runSessionHooks
returns a scripted result without entering SessionHooks. The context's
step 5 and its acceptance criterion now name a direct SessionHooks unit
test instead.

🤖 Generated with Claude Code
Workspace port + tmux backend
Some checks failed
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 8s
CI / lint (pull_request) Successful in 7s
CI / compile (pull_request) Successful in 1m26s
CI / dashboard-build (pull_request) Successful in 1m0s
CI / test (pull_request) Has been cancelled
8d57ad62d7
- TmuxOps becomes TerminalWorkspaceOps with manager-neutral names and manual-recovery hints
- Pure backend selection rule with IW_WORKSPACE_BACKEND precedence
- SessionHooks reaches the terminal through the port instead of TmuxAdapter
- Attach and focus hints render the argv the operation runs and no longer drop -L socket
- SessionContext loses sessionName so VERSION moves to 0.10.0
chore(IW-422): update review-state for phase 01
All checks were successful
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 7s
CI / lint (pull_request) Successful in 8s
CI / compile (pull_request) Successful in 1m26s
CI / dashboard-build (pull_request) Successful in 59s
CI / test (pull_request) Successful in 9m21s
997f0fec72
workflow(wf): record phase-pr CI-wait resume gap for IW-422
All checks were successful
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 8s
CI / lint (pull_request) Successful in 7s
CI / compile (pull_request) Successful in 1m22s
CI / dashboard-build (pull_request) Successful in 1m1s
CI / test (pull_request) Successful in 9m21s
1c50fcedae
Reviewed-on: #424
Issue: IW-422
Phase: 2 - Herdr backend + selection + doctor

This commit marks the checkpoint for phase 2 context generation.
Use wf-implement IW-422 to return to this point.

Corrects analysis.md against live herdr probes: already_open is a
boolean field on the worktree_opened reply, session names come from
socket_path in `herdr session list --json`, herdr reports failure as a
JSON envelope on stderr, SessionHooks.run takes the runCommand
capability, and the CommandEnv member sits at LiveCommandEnv.scala:747.

Records the three decisions taken before implementation: Phase 2 stays
whole, the pure doctor decision lives in core/model/, and a Left from
select is carried by UnavailableWorkspaceOps. Path-based workspace
matching lands in this phase, which forces isCurrent to compare ids.

🤖 Generated with Claude Code
Herdr backend + selection + doctor

Live-server probes lead the list. Three of the four open items need a
mutating probe or a terminal outside herdr, so they block the decoder
and adapter groups.

Two parts of open item 4 are closed read-only against the running
server and recorded in LESSONS.md: herdr rejects a label where it
documents an id, and the detach keystroke is Ctrl+B, Q.

Artifacts:
- phase-02-tasks.md
- LESSONS.md

🤖 Generated with Claude Code
Recorded ten reply fixtures from a running server, protocol 19. Captured
in a throwaway repo under the scratchpad, so none carries a client name.
Every probe workspace was closed again and the focused workspace was
left where it was.

Two findings contradict phase-02-context.md, which is corrected here:

- workspace focus answers workspace_info, not ok. The context reached ok
  by elimination from the schema and said so; the elimination was wrong,
  because a reply variant can be reused across commands. close does
  answer ok.
- worktree open needs a source. With neither --workspace nor --cwd,
  herdr takes the caller's workspace and refuses with
  linked_worktree_source when that is a linked worktree - the normal
  case for iw start. The parent-creating call must carry --cwd.

Open item 2 is answered: worktree open --cwd builds the parent and it
keeps full worktree metadata, so parentOf can find it again.
workspace create leaves no worktree object and is dropped from the
design.

Open item 1 and open item 4 part 2 stay open. Both need Michal.

🤖 Generated with Claude Code
Adds HerdrReply and the four decoded value types. Every entry point
answers Either, so a malformed reply is an ordinary failure and never
an exception across the port.

Tested against the ten recorded replies, not against hand-written JSON.
Three shapes the recordings pin and imagination would have missed:

- workspace focus answers workspace_info, not ok
- WorkspaceInfo.worktree is an ABSENT key, not a null value
- session list carries no envelope, and the field is socket_path

🤖 Generated with Claude Code
Adds HerdrLookup. The checkout path is the identity of the directory iw
manages; the label is a display name anyone can type. A worktree opened
through herdr's own UI carries a foreign label and only the path route
reaches it.

Ambiguity is a failure everywhere, never a reason to pick the first
candidate: byLabel, byCheckoutPath, activePane and bySocket all refuse.
parentOf keeps Right(None) - create a parent - apart from Left - several
workspaces claim this root.

A Left from the path route does not fall through to the label. Answering
by label there would name a workspace holding a different directory.

The lookups read no filesystem. Both sides of a path comparison must
arrive canonicalised, and the scaladoc says so.

🤖 Generated with Claude Code
The check now reports the manager the commands will actually drive,
instead of always reporting tmux. The decision is a pure function in
core/model with a unit test over all five outcomes; the scala-cli script
keeps only the wiring, so the check gets the test checkTmux never had.

A stub records WHICH CLI name each branch probes for. Without that, the
herdr branch could silently ask about tmux and report on a manager the
commands would not use - the exact failure this check exists to prevent.

An unrecognised IW_WORKSPACE_BACKEND is reported by name and probes for
nothing, because no manager was picked. Verified live: tmux, herdr and
bogus all report correctly, and bogus exits 1 with no stack trace.

checkWorkspaceManagerWith takes no ProjectConfiguration. No project
setting reaches this decision, and the sibling checks take a config
because they use one.

Also adds HERDR_SOCKET_PATH to Constants.EnvVars and rewrites the four
PURPOSE headers in commands/ that still named tmux.

🤖 Generated with Claude Code
TmuxAdapterTest runs real tmux against the default server, so it reads
the developer's live sessions and fails about two runs in three. Both
the test and createSession are byte-identical to main, so it predates
this branch. Recorded with proposed home "new tracker issue"; not fixed
here, because phase-02-context.md forbids touching Tmux.scala.

Also marks groups 2, 3, 4, 9 and 10 complete in the phase task list, and
records which open items the live probe closed.

🤖 Generated with Claude Code
The recording stub used a var, which scalafix DisableSyntax.var rejects
on push. The behavioural form is better anyway: make only one manager
available and assert the verdict. A branch asking about the wrong
manager reads the wrong answer and reaches the wrong verdict, so the
assertion pins the name with no recording at all.

🤖 Generated with Claude Code
The first entry blamed missing tmux socket isolation and proposed
setting IW_TMUX_SOCKET. Measurement disproved it: with a private socket
the test failed three runs out of three, and it fails on its own, so
parallel load is not the cause either.

The real cause is a race with the shell. pane_current_path is the pane
process's real cwd and only becomes the -c directory once the login
shell has started:

  new-session -d -c /home/mph, queried at once -> the caller's cwd
  the same, queried two seconds later          -> /home/mph

The wrong fix is recorded beside the right one so nobody spends the hour
again.

🤖 Generated with Claude Code
createSession sets working directory failed about two runs in three. It
asked tmux for pane_current_path, which is the pane process's live cwd:
it answers with the caller's directory until the login shell has started,
and follows the shell afterwards.

    new-session -d -c /home/mph, queried at once -> the caller's cwd
    the same, queried three seconds later        -> /home/mph
    then `cd /tmp` inside the pane               -> /tmp
    session_path, all three times                -> /home/mph

session_path is the session's working directory, fixed at creation and
never moved, which is what the test's name claims to check.

Verified both ways: six consecutive full core.test runs green, and a
mutation test - dropping -c from createSession makes the case fail, so
the assertion still catches a real regression.

Socket isolation was the first guess and it was wrong; with a private
socket it failed three runs out of three. Recorded in LESSONS.md beside
the real cause.

core/adapters/Tmux.scala is untouched.

🤖 Generated with Claude Code
Focus is server-wide and focus-before-attach works, confirmed twice with
different targets. Attaching also switches focus, which is consistent
with that order. There was never a second option: attachSession blocks
until detach, so focus-after-attach is not expressible.

The hint fallbacks are decided. A field labelled "run this to recover"
holds a runnable line, never an error message.

All four open items are now closed.

🤖 Generated with Claude Code
Pure argv builders are the single source for both execution and the
printed hints, mirroring TmuxAdapter, so an operation and its hint
cannot drift.

Three shapes the recorded replies forced:

- focusWorkspace decodes workspace_info, not ok. close decodes ok.
- runInPane decodes nothing: pane run writes no output, so exit 0 is
  the whole success signal.
- parentOpenArgv carries --cwd. Without --workspace and without --cwd
  herdr takes the caller's workspace as the source and refuses with
  linked_worktree_source whenever that is a linked worktree, which is
  the normal case for iw start. workspace create is not used at all.

HerdrReply gains errorMessage, so a failed command reports the server's
own message and falls back to raw stderr rather than inventing a
description of it.

Exercised against the live server: every read operation decodes, and
getWorkspace("zzzz") returns Left("workspace zzzz not found").

No IW_* environment filter, as the port carries no such contract.

🤖 Generated with Claude Code
LiveHerdrOps implements all twelve members. Every operation that needs
an id goes through one resolve step, so path-first addressing is stated
once. isCurrent compares ids, never labels: a workspace opened through
herdr's own UI carries a foreign label, and a label comparison would let
iw rm close the workspace the user sits in.

attach focuses then attaches. Focus is server-wide and attaching lands
the client in the focused workspace, both verified 2026-08-21.

The hints render the argv their operations run. When the id lookup
fails they fall back to a line that still works - the workspace picker
for focus, session list for attach - because a field labelled "run this
to recover" must hold a runnable line.

repoRootOf asks git, not the caller's herdr workspace. The context
specified herdr first; implemented that way create failed with "worktree
path not found", because HERDR_WORKSPACE_ID answers which repository the
CALLER sits in, not which one workDir belongs to. The two agree only
when workDir is in the caller's own repo. Recorded in LESSONS.md.

WorkspaceBackend reads the environment once and serves both consumers.
runSessionHooks still receives the function value, so core/adapters
imports core/commands nowhere. UnavailableWorkspaceOps carries a bad
IW_WORKSPACE_BACKEND through the port instead of throwing, which would
kill iw doctor - the command whose job is to report it.

Exercised against the live server end to end, in a throwaway repo, and
every workspace it made was closed again.

🤖 Generated with Claude Code
The contract suite is read-only throughout. herdr has no equivalent of
tmux's -L socket isolation, so a contract run must never create or close
workspaces on a developer's live session. Gated by IW_CONTRACT_HERDR=1
and by herdr being on PATH; both gate conditions verified to skip.

Sixteen cases pass against a live server, protocol 19. The two that earn
their keep most:

- ResponseResult offers ok and offers neither workspace_focused nor
  workspace_closed, and workspace focus is observed answering
  workspace_info. That is the pin the schema alone could not give,
  because a schema says which variants exist, not which one a command
  sends.
- An unknown workspace exits non-zero with its error envelope on stderr,
  which keeps HerdrAdapter's failure mapping honest.

Also adds the start.bats bad-value case and documents the new suite and
its gate in docs/testing.md.

🤖 Generated with Claude Code
The previous form compared against null, which scalafix DisableSyntax
rejects. Exhaustiveness over the enum is already a compile error, so the
assertion worth making is that no two backends were wired to the same
object by a copy-paste slip.

🤖 Generated with Claude Code
isCurrent compared two Options with ==, so None == None answered true and
`iw rm` refused every removal with "you are in its workspace" for a user in
no workspace at all. Five of the six reviewers found this independently. The
decision moves to HerdrLookup.isCurrent, anchored on the current id, with the
four cases pinned.

The doctor check bound CommandRunner.isCommandAvailable, which runs `which`
with the JVM's stdout inherited and printed a stray path above every report.
It now binds ProcessAdapter.commandExists, which pipes, and doctor.bats asserts
the leak is gone.

exists() dropped a Left from listWorkspaces silently. Rm.scala reads false as
"no workspace to close" and removes the worktree without calling close, so an
unreachable server orphaned a workspace with no message. Both predicates now
report.

Also from the review:
- redact capture residue from two fixtures, which carried an internal host
  name and the abbreviated real capture path, and widen the README guard so
  the next capture cannot repeat it
- catch a missing herdr before the call, not around it: os-lib spawns the
  child from its own thread, so an IOException there prints a stack trace the
  caller cannot catch
- split the reply mapping out as a pure function and pin its four branches
- separate commandName from envValue, so the check cannot report a manager
  installed while the commands reach something else
- gate the contract suite's focus probe, which writes server-wide focus
- correct a scaladoc claiming already_open is decoded; it is never read
- pin the null-worktree branch, the has() label-with-no-path case, and the
  paneRunArgv quoting limitation
- drop two temporal comments CLAUDE.md forbids

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD
workflow(wf): record what the phase 2 review round taught
Some checks failed
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 11s
CI / lint (pull_request) Successful in 9s
CI / test (pull_request) Has been cancelled
CI / dashboard-build (pull_request) Has been cancelled
CI / compile (pull_request) Has been cancelled
6973e8ed50
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD
chore(IW-422): mark phase 2 complete and open PR #427
All checks were successful
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 13s
CI / lint (pull_request) Successful in 9s
CI / compile (pull_request) Successful in 1m29s
CI / dashboard-build (pull_request) Successful in 1m10s
CI / test (pull_request) Successful in 11m5s
e4199039e5
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD
mph referenced this pull request from a commit 2026-08-21 10:40:11 +00:00
chore(IW-422): record CI green on PR #427
All checks were successful
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 8s
CI / lint (pull_request) Successful in 8s
CI / compile (pull_request) Successful in 1m23s
CI / dashboard-build (pull_request) Successful in 1m1s
CI / test (pull_request) Successful in 9m36s
04502091ed
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JhUqj7NTLf2ZeY24jFTDeD
docs(IW-422): add the review packet and the release notes
All checks were successful
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 7s
CI / lint (pull_request) Successful in 7s
CI / compile (pull_request) Successful in 1m23s
CI / dashboard-build (pull_request) Successful in 1m1s
CI / test (pull_request) Successful in 10m38s
5fb1699925
Completion-flow artifacts for the final PR: a full-branch review packet
covering both phases, and user-facing release notes in Czech for 0.10.0.

🤖 Generated with Claude Code
mph added 10 commits 2026-08-21 20:59:52 +00:00
BATS `run` merges the command's stderr into `$output`, so every `--json`
assertion parsed a stream that was never only stdout. Anything a process
writes to stderr broke them — here the JVM's `NOTE: Picked up
JDK_JAVA_OPTIONS` reminder, which the java launcher emits on stderr by
specification as an anti-misuse warning whenever the variable is set.

The five affected tests now use `run --separate-stderr`, which fills
`$output` from stdout alone. `bats_require_minimum_version 1.5.0` declares
the feature level the flag needs.

With `JDK_JAVA_OPTIONS` set, the suite is 190 ok / 0 failures. It no longer
needs `env -u JDK_JAVA_OPTIONS`, a workaround that ran the whole suite with
the environment's deliberate heap cap discarded.

Also corrects a claim in doctor.bats that the JDK notice reaches stdout.
The `which` leak that test guards is genuine stdout pollution; the JDK
notice is not, so the comparison was false.

🤖 Generated with Claude Code
`SessionHooks` took `(String, String) => Either[String, Unit]`. The type
kept `adapters/` free of a `commands/` import, but it carried no meaning:
both parameters are String, so transposing them compiles and fails only at
runtime, and the roles survived in scaladoc alone.

`RunInWorkspace` names the capability and names its parameters. It is
declared beside its only consumer, so the hook runner states what it needs
and `adapters/` still depends on `model/` alone. Callers pass
`workspace.runCommand` unchanged.

Both parameters stay String, so transposition is still possible. Opaque
types for the workspace name and the command close that, and also the
herdr label/id split; filed as its own issue.

🤖 Generated with Claude Code
Promoted:
- adapters take the capability they call, not the port → core/CLAUDE.md + kanon note #175
- assert JSON on stdout, not merged output → fixed at source (8a4a2ec) + kanon note #174

Deleted:
- bats missing from this container: already false at triage; real problem is an
  unpinned toolchain, filed as #428
- SessionHooks imports TerminalWorkspaceOps: superseded, the edge was closed and
  its proposed rule was the opposite of what was learned

Filed:
- #428 pin the development and CI toolchain with a Nix flake
- #429 opaque types for workspace names, herdr ids and commands

🤖 Generated with Claude Code
`TerminalWorkspaceChecks` held both the pure verdict and the binder that
reads `sys.env` and probes PATH. The binder pulled `ProcessAdapter` into
`core/model/`, which `core/CLAUDE.md` forbids, and made the file header's
"pure decision" claim untrue.

The binder moves to `commands/start.hook-doctor.scala`, which is where the
check is assembled and which already may import adapters — the same shape
`issue.hook-doctor.scala` and `github.hook-doctor.scala` use. `core/model/`
now imports nothing.

The pure function and its unit tests are unchanged. The two doctor BATS
tests exercise the live binding and stay green.

Review finding W11 raised this and accepted it on the `GitHubHookDoctor`
precedent. That file is the remaining instance; filed separately.

🤖 Generated with Claude Code
Promoted:
- a pure function and its binder are not the same layer → core/CLAUDE.md (model/)
  + kanon note #177; merged with the model/-imports-adapters finding
- a fake stub no test asserts on is not coverage → docs/testing.md + kanon note #176

Filed:
- #430 move the GitHubHookDoctor binder out of core/model/

🤖 Generated with Claude Code
Promoted:
- probe a binary before os.proc, never around it → core/CLAUDE.md (adapters)
  + kanon note #178

Filed:
- #431 phase-pr must attach to an existing PR on Forgejo instead of failing on 409
- #432 TmuxAdapter must refuse a missing tmux binary instead of throwing

The os.proc entry was normalised — its fields were out of order and unlabelled.

🤖 Generated with Claude Code
`CommandRunner.isCommandAvailable` probed with `s"which $command".!`, which
inherits the JVM's stdout, so the resolved path printed above whatever the
command was writing. It is the default argument of roughly thirty call sites
across `GitHubClient`, `GitLabClient` and `GitHubHookDoctor`, so every `gh`
and `glab` prerequisite check could put a stray line into a `--json` reply
and break a `jq` assertion on it.

It now answers through `ProcessAdapter.commandExists`, which pipes both
streams and validates the name first. Two probes existed with names giving
no hint that one was safe and one was not; now there is one behaviour.

The only visible change is that a path-shaped argument is refused instead
of executed. No caller passes one.

🤖 Generated with Claude Code
`TmuxAdapter` shelled out at seven sites without checking that tmux is on
PATH. os-lib spawns the child from its own thread, so the resulting
`IOException` printed an uncatchable stack trace on
`subprocess-shutdown-hook-monitor` — the calling thread could not stop it.
Reachable on any machine without tmux, which is the default backend.

Every operation now consults a memoised `commandExists` probe and answers
through the channel it already returns: `sessionExists` and
`currentSessionName` report nothing, the `Either` members report the missing
binary by name. `isInsideTmux` reads the environment and never shelled out,
so it is unchanged.

`HerdrAdapter` closed the same exposure in this issue. tmux went unnoticed
because it is nearly always installed when selected, while herdr can be
selected by an environment variable alone.

The probe is a defaulted parameter on each operation, following the
`isCommandAvailable` idiom in `GitHubClient` and `GitLabClient`, so
`TmuxMissingBinaryTest` can ask what each one answers with the binary gone.

`RunInWorkspace` gains `@FunctionalInterface`. Eta-expanding into it warns
without the annotation, and scala-cli builds `core` at Scala 3.3.7 with
warnings as errors — a gate the Mill test run does not apply.

Closes #432.

🤖 Generated with Claude Code
Both fixed here rather than filed:
- the unsafe command probe now delegates to ProcessAdapter.commandExists (687d563)
- TmuxAdapter refuses a missing binary instead of throwing (3c70bb5), closing #432

Entry 9 also repeated the JDK_JAVA_OPTIONS claim corrected in entry 1. The JVM
notice is on stderr; the `which` leak is genuine stdout. Corrected in place.

🤖 Generated with Claude Code
workflow: triage lessons for IW-422 (entries 10-12) — walk complete
Some checks failed
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 7s
CI / lint (pull_request) Successful in 8s
CI / test (pull_request) Has been cancelled
CI / dashboard-build (pull_request) Has been cancelled
CI / compile (pull_request) Has been cancelled
4488a819b8
Promoted:
- Option == Option cannot express "is this the current X" → kanon note #179
  (redirected from core/CLAUDE.md: five of six reviewers already caught it)
- match the shape, not the literals, when redacting a recorded fixture →
  kanon note #180; the source fix was already applied in the fixture README

Retained:
- the pre-commit format-check segfault: rare, not reproducible on demand, held
  for recurrence with its error text and conditions intact

Also redacts an internal host name that had survived in this issue's own record
of the fixture leak, in LESSONS.md and the phase 2 review. These files ride in
the PR to main on a repository mirrored to GitHub.

No entries remain pending.

🤖 Generated with Claude Code
chore(IW-422): update review-state to all_complete
All checks were successful
CI / contract (pull_request) Has been skipped
CI / format (pull_request) Successful in 7s
CI / lint (pull_request) Successful in 8s
CI / compile (pull_request) Successful in 1m26s
CI / test (pull_request) Successful in 9m42s
CI / dashboard-build (pull_request) Successful in 1m4s
8eba1ca575
mph merged commit c0b110d659 into main 2026-08-22 07:54:50 +00:00
Sign in to join this conversation.
No description provided.