fix(bin): remove fork-only Herdr HERDR_SOCKET_PATH container detection fallback - #26
Merged
Merged
Conversation
…n fallback Herdr detection now matches upstream: HERDR_ENV=1 (set through the devcontainer environment as upstream's configuration docs describe) selects Herdr, and fm_backend_herdr_session reads HERDR_SESSION with a default fallback. Removes the socket-path detection branch, the socket-derived session name, their docs, unit tests, and the real-Herdr container e2e test.
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
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.
Intent
Remove the Herdr container detection fix from the fork https://github.com/cloud-practitioner/firstmate main; the environment is set from devcontainer.json as upstream https://github.com/kunchenguid/firstmate's configuration documentation instructs. Context: this is the fork-only HERDR_SOCKET_PATH fallback detection added by fork PRs #2 and #7 to #10 and restored onto main by fork PR #25, including its follow-up test fix 4b2c1da if that only serves the fallback. After the change, Herdr detection behaves exactly as upstream and relies on the documented environment configuration. Every other fork fix stays (Bitbucket PR support, the zsh fix, the test fixes).
What Changed
fm_backend_detectinbin/fm-backend.shno longer picks Herdr just becauseHERDR_SOCKET_PATHpoints to a socket, and it no longer reportsFM_BACKEND_DETECT_SIGNAL=HERDR_SOCKET_PATH. Herdr is auto-detected only fromHERDR_ENV=1when$TMUXis not set, the same as upstream.bin/backends/herdr.sh, the session name no longer comes from the socket path (fm_backend_herdr_socket_session_nameis removed).fm_backend_herdr_sessionnow returns${HERDR_SESSION:-default}.tests/fm-backend.test.sh, the socket-derived session tests intests/fm-backend-herdr.test.sh, and the wholetests/fm-backend-herdr-container-session-e2e.test.shfile along with its entry inbin/fm-test-run.sh. The remaining detection tests no longer unsetHERDR_SOCKET_PATH.docs/architecture.md,docs/configuration.mdanddocs/herdr-backend.mdno longer mention the container fallback.🤖 Generated with Claude Code
Provenance
Removed, by origin:
HERDR_SOCKET_PATHdetection branch infm_backend_detectand its header comments (bin/fm-backend.sh): fork PR fix(backend): enable Herdr detection in devcontainers via HERDR_SOCKET_PATH fallback #2, refined in Fix/herdr container detection #7-Fix/herdr container detection #10, restored by feat(bin): restore fork fixes (Bitbucket Cloud PRs, Herdr containers, zsh) onto upstream main #25.fm_backend_herdr_socket_session_nameand the socket-derivedfm_backend_herdr_session(bin/backends/herdr.sh): fork PRs Fix/herdr container detection #7-Fix/herdr container detection #10 (session binding follow-up to fix(backend): enable Herdr detection in devcontainers via HERDR_SOCKET_PATH fallback #2).docs/architecture.md,docs/configuration.md,docs/herdr-backend.md: fork PRs fix(backend): enable Herdr detection in devcontainers via HERDR_SOCKET_PATH fallback #2 and Fix/herdr container detection #7-Fix/herdr container detection #10.test_backend_detect_herdr_socket_fallbackand the extraHERDR_SOCKET_PATHunsets intests/fm-backend.test.sh: fork PRs fix(backend): enable Herdr detection in devcontainers via HERDR_SOCKET_PATH fallback #2 and Fix/herdr container detection #7-Fix/herdr container detection #10.tests/fm-backend-herdr.test.sh,tests/fm-backend-herdr-container-session-e2e.test.sh, and itsbin/fm-test-run.shfamily entry: fork PRs Fix/herdr container detection #7-Fix/herdr container detection #10.path->socket_pathrename lived only inside the removed helper, and its zsh test only exercised the socket-derived session lookup.Deliberately kept from the same fork history: the zsh fixes (
fm_backend_sourcepositional sibling list from #17, theBASH_SOURCE[0]:-$0root inbin/backends/herdr.sh, and the zsh all-backends sourcing test), Bitbucket PR support, the lint memory cap, the per-home task temp root, and the Herdr recovery lock wait.Local note:
tests/fm-test-run.test.shfailed locally both on this branch and on the unchanged base, with different failures (branch: a--jobsadmission check and a background-session fixture timing out after a slowfm-session-lock-ancestryrun; base:rubymissing to parse the CI workflow YAML), so they look environmental; CI passes.Risk Assessment
✅ Low: The change only removes fork-only code. Against upstream aedb7bb, the HERDR_SOCKET_PATH detection fallback, the socket-derived session name and their docs and tests are all gone, and no references to them remain. The remaining diffs in the touched files belong to the fork fixes that are meant to stay: the zsh sourcing fix, Bitbucket support and the per-home temp root.
Testing
I provisioned a disposable fm-lab-* Herdr session and ran firstmate's detection code inside a real pane. Herdr injects HERDR_ENV=1, HERDR_SESSION and HERDR_SOCKET_PATH there, and firstmate detected herdr with signal HERDR_ENV and the lab session. With only HERDR_SOCKET_PATH set, pointing at a live socket, the change returns undetected and falls back to tmux with session "default", exactly as upstream does. Base 76a9f28 detected herdr through the removed HERDR_SOCKET_PATH signal in that same environment. With HERDR_ENV, HERDR_SESSION and HERDR_SOCKET_PATH passed into the container environment, as upstream's docs describe, a real container_ensure call reached the existing lab server without restarting it (same socket inode). Nested tmux still wins over Herdr. A file comparison found the three detection functions identical to upstream main; because that was a source diff rather than a live run, the scenario is marked untested. The targeted fm-backend, Herdr-adapter and Bitbucket test files pass, and the zsh sourcing fix still works. I tore down the lab session and removed every temp directory; the worktree is clean. This is a CLI change with no UI, so there are text transcripts and no screenshots.
bash tests/fm-pr-bitbucket.test.shrc=0 (fm-pr-bitbucket.test.log); none of the Bitbucket files are touched by the changeEvidence: Detection probes inside a real Herdr lab pane (change vs base contrast)
Source: Detection probes inside a real Herdr lab pane (change vs base contrast)
Evidence: Herdr pane read-back of the probe run
Source: Herdr pane read-back of the probe run
Evidence: container_ensure under documented env reaches the live lab server (same socket inode)
Source: container_ensure under documented env reaches the live lab server (same socket inode)
Evidence: Upstream parity of detection functions
Source: Upstream parity of detection functions
Evidence: zsh sourcing check
Source: zsh sourcing check
Evidence: tests/fm-backend.test.sh log
Source: tests/fm-backend.test.sh log
Evidence: tests/fm-backend-herdr.test.sh log
Source: tests/fm-backend-herdr.test.sh log
Evidence: tests/fm-pr-bitbucket.test.sh log
Source: tests/fm-pr-bitbucket.test.sh log
Evidence: Detection probe excerpt
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-pr-bitbucket.test.shrc=0 (fm-pr-bitbucket.test.log); none of the Bitbucket files are touched by the changebin/fm-herdr-lab.sh name/provision/run/teardownon the throwaway session fm-lab-detect-2347999-13979 (default session left alone; tripwire check passed at teardown)Detection probe (fm_backend_detect / fm_backend_name / fm_backend_herdr_session) run inside a real lab Herdr pane viaherdr pane run, in four env shapes: native, socket-only container, documented passthrough, and nested tmuxSame socket-only probe against agit archiveof base 76a9f28, to show the removed behaviorfm_backend_herdr_container_ensure /tmprun inside the lab pane with a disposablebin/fm-lab-home.sh createFM_HOME and the documented HERDR_ENV/HERDR_SESSION/HERDR_SOCKET_PATH env; socket inode compared before and afterFunction-level diff of fm_backend_detect, fm_backend_name and fm_backend_herdr_session against upstream kunchenguid/firstmate main fetched withgh apibash tests/fm-backend.test.shbash tests/fm-backend-herdr.test.shbash tests/fm-pr-bitbucket.test.shzsh:source bin/fm-backend.sh && fm_backend_source herdrfollowed by detection and session calls✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.