fix(fm-backend): Herdr container detection via HERDR_SOCKET_PATH fallback - #5241
cloud-practitioner wants to merge 9 commits into
Conversation
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.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| derived=$(fm_backend_herdr_socket_session_name "$HERDR_SOCKET_PATH") | ||
| [ -n "$derived" ] && { printf '%s' "$derived"; return 0; } | ||
| fi | ||
| printf 'default' |
There was a problem hiding this comment.
If a forwarded socket is mounted at a container-local path such as /run/herdr.sock, this pattern cannot recover the named session and silently returns default. Herdr operations are then routed with --session default, so detection can select Herdr from the forwarded socket but create or address a disconnected default server instead of the session behind that socket. Require the canonical .../sessions/<name>/herdr.sock mount shape or fail closed when a socket-only path cannot identify its session.
Knowledge Base Used:
|
Speaking as Kun's firstmate: triage on HEAD contract-class: restore. Main tip VISION (brief): One captain/interface aligns (backend just works in the container shape). Authority aligns. Scripts/agents aligns. Restart N/A. Delegation N/A. Fleet-outlives-vendor aligns (Herdr adapter completeness). Scope aligns. Align: fewer silent tmux fallbacks inside Herdr-forwarded containers. Attestation: MISMATCH — body binds Waiting on author: re-run |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Closing: not needed. The Herdr backend can be selected explicitly through the documented configuration (docs/configuration.md), so no detection fallback is required. |
2 participants
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 (cloud-practitioner#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.