Skip to content

feat(herdr): integrate treehouse with the herdr runtime#44

Open
9Olive wants to merge 2 commits into
kunchenguid:mainfrom
9Olive:herdr-integration
Open

feat(herdr): integrate treehouse with the herdr runtime#44
9Olive wants to merge 2 commits into
kunchenguid:mainfrom
9Olive:herdr-integration

Conversation

@9Olive

@9Olive 9Olive commented Jun 29, 2026

Copy link
Copy Markdown

Intent

Goal: make treehouse a full first-class citizen of the herdr terminal multiplexer. Three layers: (1) entry routing - when running inside herdr, export a TREEHOUSE_HERDR=1 marker into the worktree and print a one-line message routing the agent that lands there to the /herdr skill; (2) a shipped companion treehouse agent skill that cross-routes to the /herdr skill; (3) herdr-native spawning - open each acquired worktree in its own herdr pane instead of nesting a blocking subshell in the caller's pane.

Deliberate design decisions (please treat as intended, not mistakes):

  • Preserving treehouse's signature exit-to-return UX was the central constraint. Spawning a herdr pane returns immediately, which would let the pool reclaim the worktree while the agent is still using it. The fix reuses treehouse's existing durable LEASE primitive: getHerdrRunE leases the worktree (holder "herdr") so the pool cannot reclaim it after the outer get process exits, then opens a pane running the hidden "treehouse __hold " holder; __hold runs the worktree session and calls pool.Release when its shell exits. Using a lease (not an owner reservation) is intentional precisely because the outer process exits immediately.
  • The __hold binary path is resolved via os.Executable(), not by assuming treehouse is on PATH inside the pane.
  • Graceful degradation is required and intentional: herdr CLI absent, --no-herdr / TREEHOUSE_NO_HERDR=1, os.Executable() failure, or any pane-spawn failure releases the lease and falls back to the classic in-place subshell. Behavior outside herdr is byte-for-byte unchanged.
  • SpawnHold is bounded by a 10s timeout (exec.CommandContext) so an unresponsive herdr server triggers the fallback instead of hanging treehouse get forever. (Closing this hole is the change in this revision.)
  • parsePaneID is intentionally best-effort and tolerates multiple herdr JSON shapes because the pane id is display-only.
  • The [herdr] config section (enabled/split/focus) is honored ONLY from user-level config and stripped from repo-level config, mirroring the existing hooks safety model, because it drives the user's live herdr session.
  • .gitignore patterns were anchored to /treehouse and /treehouse.exe. This is a root-cause fix: the unanchored "treehouse" pattern matched the skills/treehouse/ directory and silently kept the committed skill file out of git.
  • The e2e harness appends TREEHOUSE_NO_HERDR=1 so the suite is deterministic when run from inside a herdr session. HERDR_ENV is intentionally NOT stripped; all e2e assertions use substring matching so the routing line is harmless.

Known, user-accepted tradeoffs for this PR (already decided, please do not block on them):

  • hold-double-release-race: a TOCTOU double-release exists if a user runs "treehouse return " from outside the pane while __hold is still live and another get immediately reuses the worktree. This is pre-existing in the classic-subshell path too; the user has accepted it for this PR and a focused follow-up will add a conditional/token-guarded release in the lease/pool primitives. Out of scope here to keep this PR focused on the herdr integration.
  • routing-message-always-on-fallback: when getHerdrRunE falls back to a classic subshell, the routing message still prints because HERDR_ENV=1 is still set. This is intentional and accurate (the agent is inside herdr, just not in its own pane).

Verification already done in a live herdr session: get inside herdr spawned a real pane parsed from herdr's JSON; __hold ran the worktree session with TREEHOUSE_HERDR=1 and cwd in the worktree; status showed "leased (held by herdr)"; exiting the pane returned the worktree to the pool; get --lease printed only the path on stdout; --no-herdr / TREEHOUSE_NO_HERDR=1 / herdr-not-on-PATH all fell back to the classic subshell; return released the lease.

What Changed

  • Added internal/herdr package with SpawnHold, IsRuntime, Available, and RoutingMessage - when HERDR_ENV=1 and the herdr CLI is present, treehouse get leases the worktree and opens it in a dedicated herdr pane via a hidden __hold subcommand, with a 10s timeout so an unresponsive herdr server triggers fallback rather than hanging
  • Added [herdr] config section (enabled, split, focus) honored only from user-level config; extended cmd/get.go with getHerdrRunE/getLeaseRunE/holdAndReturn and a --no-herdr/TREEHOUSE_NO_HERDR opt-out; anchored .gitignore patterns to /treehouse and /treehouse.exe to stop the unanchored pattern from silently excluding skills/treehouse/ from git
  • Shipped a companion agent skill at skills/treehouse/SKILL.md that cross-routes agents landing in a treehouse worktree to the /herdr skill

Risk Assessment

⚠️ Medium: The herdr integration is architecturally sound and well-tested, the lease primitive is correctly wired through acquire/release/prune/destroy, and the fallback degrades gracefully - but the double hook execution on herdr fallback is a real behavioral issue in an easily-reached failure path, and the LeasedAt serialization diverges from the stated backward-compatibility contract.

Testing

Static code review confirms all three herdr integration layers are correctly implemented: (1) TREEHOUSE_HERDR=1 marker and /herdr routing message exported in holdAndReturn when HERDR_ENV=1; (2) skills/treehouse/SKILL.md ships with a dedicated herdr section cross-routing to the /herdr skill; (3) herdr-native pane spawning via AcquireLease+SpawnHold with 10s timeout, errHerdrFallback on any failure, __hold hidden subcommand for exit-to-return UX, and .gitignore anchored to /treehouse to avoid masking the skills/ directory. Runtime test execution was blocked by the permission system - no go or make commands could be run.

  • Outcome: ⚠️ 1 warning across 1 run (3m57s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 2 warnings
  • ⚠️ internal/pool/state.go:19 - LeasedAt time.Time tagged json:"leased_at,omitempty,omitzero" will not be omitted for non-leased entries. Standard encoding/json does not omit zero struct values with omitempty (only scalars, slices, maps, and pointers are handled by isEmptyValue). The omitzero option is not recognized by encoding/json and is silently ignored. Every non-leased WorktreeEntry written to disk will contain "leased_at":"0001-01-01T00:00:00Z", contradicting the stated design goal of backward-compatible, omitted-when-zero state files. There is no operational breakage (old binaries ignore unknown fields on decode), but the format diverges from intent. Fix: change the field to *time.Time so omitempty omits the nil pointer, updating markAcquired and clearLease accordingly.
  • ⚠️ cmd/get.go:82 - post_create hooks fire twice when getHerdrRunE falls back to a classic subshell. The sequence is: (1) pool.AcquireLease acquires a worktree and runs hooks (routed to stderr); (2) herdr.SpawnHold fails; (3) pool.Release resets the worktree back to idle; (4) the caller falls through to pool.Acquire, which finds the same now-idle worktree and runs hooks a second time (to stdout/stderr). The user sees hook output twice with no intervening interaction. For idempotent hooks this is harmless; for venv-setup or dependency-install scripts that write state or emit errors on re-run, this produces confusing output or broken environments. The fallback path is presumably rare (herdr server unresponsive or binary spawn failure), but it is still a reachable code path.
⚠️ **Test** - 1 warning
  • ⚠️ Test execution is blocked by the permission system - all go test, make test, go build, and mkdir /tmp/... commands require user approval and cannot be auto-approved in this session. The tests could not be run, and no runtime evidence could be produced. The user should approve go test and mkdir /tmp commands (or add them to .claude/settings.json permissions) to enable automated test validation.
  • internal/herdr/herdr.go reviewed - SpawnHold, buildStartArgs, parsePaneID, RoutingMessage, IsRuntime, Disabled, Available all present and correctly implemented
  • internal/herdr/herdr_test.go reviewed - 8 tests covering IsRuntime, Disabled, Available, buildStartArgs (x2), parsePaneID (9 cases), RoutingMessageMentionsSkill, SpawnHoldTimesOut (150ms stub with exec sleep 30)
  • cmd/get.go reviewed - getRunE, useHerdrPane, getHerdrRunE, getLeaseRunE, holdAndReturn all present; lease-on-herdr-spawn, fallback-to-subshell, TREEHOUSE_NO_HERDR/--no-herdr opt-out, stderr-only for --lease stdout-path all confirmed
  • skills/treehouse/SKILL.md reviewed - herdr section present, /herdr skill cross-reference correct, no em dashes
  • go test ./internal/herdr/... -v - BLOCKED (permission system requires user approval for all go commands)
  • go test ./... -count=1 - BLOCKED (permission system requires user approval)
⚠️ **Document** - 3 warnings
  • ⚠️ README.md:330 - README.md Configuration section has no ### herdr subsection. The [herdr] config is documented only in the 'Using treehouse inside herdr' section above; readers who jump to Configuration miss the enabled, split, and focus options entirely. Add a ### herdr subsection after ### Hooks (before ## Development) mirroring the hooks pattern: a one-paragraph intro noting user-level-only scope, the toml block, and a link to the herdr section.
  • ⚠️ CLAUDE.md:56 - CLAUDE.md herdr integration design-decision bullet omits the SpawnHold 10s timeout, which was the key behavioral fix in this revision. The sentence after 'Any failure to spawn the pane releases the lease and falls back to the classic in-place subshell (errHerdrFallback), so behavior outside herdr is byte-for-byte unchanged.' should read: 'SpawnHold is bounded by a 10s timeout (exec.CommandContext) so an unresponsive herdr server triggers the fallback instead of blocking treehouse get forever (the spawnTimeout var is shrinkable in tests).'
  • ⚠️ AGENTS.md:56 - AGENTS.md carries the same herdr integration bullet as CLAUDE.md and has the same omission of the SpawnHold 10s timeout. Apply the identical fix: insert the SpawnHold timeout sentence after 'behavior outside herdr is byte-for-byte unchanged.' on line 56.
✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

9Olive added 2 commits June 28, 2026 21:28
… /herdr skill

When treehouse runs inside herdr (HERDR_ENV=1) and the herdr CLI is present,
`treehouse get` now opens each acquired worktree in its own herdr pane and
routes the agent that lands there to the /herdr skill, instead of nesting a
subshell blindly inside the caller's pane.

- internal/herdr: runtime detection, `herdr agent start` pane spawning, pane-id
  parsing, and the /herdr routing message. Spawning is bounded by a 10s timeout
  so an unresponsive herdr server triggers the subshell fallback instead of
  hanging `treehouse get`. Shells out only, so it builds on Windows even though
  herdr is unix-only.
- cmd/get: herdr-native get leases the worktree and opens a pane running the
  hidden __hold holder, which runs the worktree session and returns it to the
  pool when its shell exits, preserving exit-to-return even though the outer
  process exits immediately. A lease (not an owner reservation) protects the
  worktree across that exit. Any failure to spawn a pane releases the lease and
  falls back to the classic in-place subshell, so behavior outside herdr is
  unchanged. Adds --no-herdr and --no-focus; exports TREEHOUSE_HERDR=1 and the
  routing line on every herdr path (including --lease and the fallback).
- config: a [herdr] section (enabled/split/focus) honored only from user-level
  config, like hooks, because it drives the user's live herdr session.
- skills/treehouse: a companion agent skill that teaches agents to use treehouse
  and cross-routes to the /herdr skill; installable via `make install-skill`.
- docs: README "Using treehouse inside herdr" section, new flags in the CLI
  reference, AGENTS.md design notes, and treehouse.toml.example.
- tests: unit tests for the herdr package and the [herdr] config; the e2e
  harness forces TREEHOUSE_NO_HERDR=1 so the suite is deterministic when run
  from inside a herdr session.
…bsection

Apply the document-step suggestions that were deferred from the feat commit:
- AGENTS.md (and CLAUDE.md via symlink): note that SpawnHold is bounded by a
  10s timeout so an unresponsive herdr server triggers the subshell fallback.
- README.md: add a ### herdr subsection under Configuration mirroring ### Hooks,
  so readers browsing Configuration see the [herdr] enabled/split/focus options.
@9Olive

9Olive commented Jun 29, 2026

Copy link
Copy Markdown
Author

Reviewer context from the validation pass

Docs: de887b2 adds the two documentation improvements the review suggested (the SpawnHold 10s timeout note in AGENTS.md/CLAUDE.md, and a ### herdr config subsection in the README).

Deferred to a focused follow-up (#45): two warning-level findings on the herdr -> subshell fallback path were intentionally left out of this PR to keep it scoped to the integration. They share one root cause (unconditional pool.Release + the fallback's release/re-acquire dance):

  • post_create hooks run twice when getHerdrRunE falls back to a classic subshell.
  • a pre-existing TOCTOU double-release if treehouse return is run from outside the pane while __hold is live.

Verified false positive (not changed): the leased_at,omitempty,omitzero tag was flagged as "omitzero is ignored by encoding/json," but omitzero is honored since Go 1.24 (this repo is on go 1.25). Confirmed empirically: a zero entry marshals to {}, a leased entry keeps leased_at.

The feature was also verified end-to-end in a live herdr session (real pane spawn, __hold session, status showing leased (held by herdr), exit-to-return, --lease/--no-herdr/CLI-absent fallbacks).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant