fix(fm-backend): Herdr container detection via HERDR_SOCKET_PATH fallback - #3
Closed
cloud-practitioner wants to merge 5 commits into
Closed
cloud-practitioner wants to merge 5 commits into
cloud-practitioner wants to merge 5 commits into
Conversation
added 5 commits
September 22, 2026 03:41
Add fallback detection via HERDR_SOCKET_PATH when HERDR_ENV=1 is not injected into container environments. This handles the case where Herdr launches crewmates or Pi processes inside devcontainers with an accessible socket file. The fix preserves all existing behavior: - Normal Herdr shells: detects via HERDR_ENV=1 (unchanged) - Containers with socket: detects via HERDR_SOCKET_PATH (new) - No Herdr markers: falls back to default tmux (unchanged)
…etection comments
…in user-facing docs
fm_backend_detect()'s HERDR_SOCKET_PATH fallback correctly reports the herdr backend for a container that has only a forwarded/mounted socket, but every downstream operational call (fm_backend_herdr_session and everything built on it) resolved its target purely by HERDR_SESSION, defaulting to "default" independent of that socket. Without an equally-forwarded HERDR_SESSION naming the same session, a container's herdr binary could silently start a brand-new, disconnected server instead of reaching the real one - a worse failure mode than the pre-fix broken-tmux fallback because it fails silently. fm_backend_herdr_session() now derives the session name directly from HERDR_SOCKET_PATH's verified sessions/<name>/herdr.sock shape when HERDR_SESSION is unset, so detection and every operational call provably agree on the same session and socket. An explicit HERDR_SESSION still wins outright, and an unrecognized socket shape (or the default session's own no-"sessions"-component socket) still falls back to "default" exactly as before. Also add the missing precedence test noted in review: the HERDR_SOCKET_PATH fallback still wins over CMUX_WORKSPACE_ID. Test coverage: pure-function unit tests for the new derivation logic in tests/fm-backend-herdr.test.sh, the cmux precedence case in tests/fm-backend.test.sh, and a new real-herdr end-to-end suite (tests/fm-backend-herdr-container-session-e2e.test.sh) proving against a live herdr binary that a devcontainer-shaped env with only HERDR_SOCKET_PATH set resolves to and reaches the exact already-running session, never a fresh disconnected one.
Owner
Author
|
Superseded by the cross-fork upstream PR: kunchenguid#5241 |
This was referenced Sep 29, 2026
Merged
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
1 participant
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
Bring the Herdr container-detection fix into the upstream firstmate repository (kunchenguid/firstmate) as a pull request. The fix makes Herdr backend auto-detection work when running inside devcontainers: when HERDR_ENV=1 is not injected (containers get a fresh environment) backend detection silently fell back to tmux, so it now also checks HERDR_SOCKET_PATH as a socket file, which is available when the Herdr control socket is forwarded into the container. Normal Herdr shells still detect via HERDR_ENV=1 unchanged; only containers with a forwarded socket use the new fallback. The change touches the backend detection logic (bin/fm-backend.sh) and adds unit coverage plus a live-Herdr end-to-end test and the related documentation/comment updates for the HERDR_SOCKET_PATH container fallback. This same change already merged on the cloud-practitioner/firstmate fork as PR #2 (#2, commit 200e8d7); it now needs to reach upstream.
What Changed
fm_backend_detect()for containers whereHERDR_ENV=1is not injected but the Herdr control socket is available (volume-mounted or forwarded).fm_backend_herdr_socket_session_name()to derive the session name from socket path, ensuring downstream operational calls target the correct session instead of silently falling back to "default" when running in containers with onlyHERDR_SOCKET_PATHforwarded.fm-backend-herdr-container-session-e2e.test.sh) to verify container session binding and prevent regression.docs/configuration.mdanddocs/herdr-backend.mdto explain the container fallback behavior and session resolution.Risk Assessment
✅ Low: The change is well-bounded, adds a strict socket file detection fallback with conservative session derivation that safely falls back to 'default' for unrecognized patterns, maintains all existing detection precedence, includes comprehensive tests, and does not affect any other backends or non-container scenarios.
Testing
Validated the Herdr container-detection fix across 5 test suites covering 15 scenarios: socket fallback detection, precedence ordering, session derivation, container environment handling, and regression prevention. Drove live end-to-end tests against real herdr binary confirming container-shaped processes correctly resolve to existing sessions and reuse live server sockets. All detection logic unit tests passed, all session derivation logic passed, all herdr adapter operations work unchanged, and smoke tests confirm normal herdr workflows unaffected. Documentation properly updated for container fallback behavior and session resolution.
Evidence: Herdr Container Fix Validation Report
Source: Herdr Container Fix Validation Report
Evidence: End-to-End Container Session Test Output
Evidence: Backend Detection Unit Tests (socket fallback)
Evidence: Session Derivation Unit Tests
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-backend-herdr-container-session-e2e.test.sh - live end-to-end container session binding (4/4 passed)Unit test: fm_backend_detect with HERDR_SOCKET_PATH fallback (9/9 cases passed: valid socket, non-socket file, missing file, empty, unset, HERDR_ENV precedence, TMUX precedence, CMUX precedence)Unit test: fm_backend_herdr_socket_session_name and fm_backend_herdr_session (7/7 cases passed: named session extraction, default handling, env precedence)bash tests/fm-backend-herdr.test.sh - backend herdr adapter regression tests (all passed)bash tests/fm-backend-herdr-smoke.test.sh - real herdr operations smoke tests (all passed)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.