fix(backend): enable Herdr detection in devcontainers via HERDR_SOCKET_PATH fallback - #2
Merged
Merged
Conversation
added 4 commits
September 22, 2026 00:06
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.
This was referenced Sep 22, 2026
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
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
Fix Herdr backend auto-detection when running in devcontainers. When Herdr launches crewmates or Pi agents in containers, HERDR_ENV=1 is not injected (containers get fresh env), causing backend detection to fail and silently fall back to tmux. The fix adds a fallback check for HERDR_SOCKET_PATH as a socket file, which is available when forwarded into containers. Preserves existing behavior: normal Herdr shells use HERDR_ENV=1 unchanged, containers with sockets detect via HERDR_SOCKET_PATH, environments without Herdr markers fall back to tmux.
What Changed
Added a fallback check in
fm_backend_detect()to detect Herdr viaHERDR_SOCKET_PATHwhen running in containers whereHERDR_ENV=1is not injected (e.g., when Herdr spawns crewmates or Pi agents inside a devcontainer). The check validates thatHERDR_SOCKET_PATHnames an actual Unix domain socket file (using the-Stest), maintaining strict requirements for detection accuracy.Updated backend detection precedence documentation in
docs/architecture.md,docs/configuration.md, anddocs/herdr-backend.mdto clarify thatHERDR_SOCKET_PATHserves as a fallback for container environments while preserving the innermost-first nesting rule (TMUX > HERDR_ENV > HERDR_SOCKET_PATH > CMUX_WORKSPACE_ID).Added comprehensive test coverage (
test_backend_detect_herdr_socket_fallback) with 8 test cases validating socket detection, signal assignment, strict socket-file validation, precedence rules, and rejection of non-socket files, missing paths, and empty/unset values.Risk Assessment
✅ Low: The fix is well-bounded, preserves existing behavior through correct precedence ordering, uses standard POSIX shell tests, and the prior round's comment fixes are accurate and sufficient.
Testing
Exercised all 7 required scenarios for HERDR_SOCKET_PATH fallback detection via live unit tests with real Unix domain sockets: valid socket detection for containers (primary use case), strict rejection of non-sockets and missing files, boundary handling of empty values, correct precedence preservation (HERDR_ENV > HERDR_SOCKET_PATH, TMUX > both), and test isolation fixes to prevent environment bleed. All 21 backend detection tests pass, including new and existing coverage.Evidence: Test Results Summary
Source: Test Results Summary
Evidence: Implementation Changes Diff
Source: Implementation Changes Diff
Evidence: Detailed Validation Report
Source: Detailed Validation Report
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
bin/fm-backend.sh:99- Comment states 'HERDR_ENV=1 alone (no $TMUX) selects herdr' but code now also selects herdr via HERDR_SOCKET_PATH fallback. Update comment to reflect: 'HERDR_ENV=1 or HERDR_SOCKET_PATH (when a socket is available) alone (no $TMUX) selects herdr.'bin/fm-backend.sh:137- Comment lists FM_BACKEND_DETECT_SIGNAL possible values as 'TMUX, HERDR_ENV, CMUX_WORKSPACE_ID, bundle-id, or ancestry' but omits HERDR_SOCKET_PATH, which is now set by the new container fallback (line 162). Update to: 'TMUX, HERDR_ENV, HERDR_SOCKET_PATH, CMUX_WORKSPACE_ID, bundle-id, or ancestry'🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-backend.test.sh (21 passing backend detection tests)test_backend_detect_herdr_socket_fallback: Valid socket detection ✓test_backend_detect_herdr_socket_fallback: Non-socket file rejection ✓test_backend_detect_herdr_socket_fallback: Missing file rejection ✓test_backend_detect_herdr_socket_fallback: Empty/unset value rejection ✓test_backend_detect_herdr_socket_fallback: HERDR_ENV precedence ✓test_backend_detect_herdr_socket_fallback: TMUX precedence (innermost-first) ✓test_backend_detect_precedence: All existing detection combinations (6 nested cases) ✓test_backend_name_precedence: FM_BACKEND, config/backend, autodetect chain ✓✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.