Fix/herdr container detection - #9
Merged
Merged
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.
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.
No description provided.