Conversation
Two platform faults kept the herdr adapter from placing any worker on a Windows host. The launcher-identity same-session proof compares the control-socket path herdr injects against the one `session list --json` reports. Herdr on Windows reports a drive-letter path, and the canonicaliser accepted only POSIX absolute paths, so it refused every spawn with "reports an unusable socket path". Both sides report the identical drive-letter value, so the proof succeeds once that spelling is accepted. Backslashes are folded to forward slashes first, which also makes the two Windows spellings of one socket compare equal. jq on Windows opens stdout in text mode and ends every record with CRLF. A command substitution drops only the trailing carriage return, so single-value reads come back clean while multi-line reads keep an interior CR on every line but the last. Consumers that split such a capture then carry a stray CR into an id passed back to herdr, or into an operator-facing message - which is what the workspace-ambiguity refusal did. The three multi-line captures are stripped at their source; single-value reads are already clean and are deliberately left alone. Stripping happens after capture rather than through a pipe wherever jq's own exit status decides a parse failure.
…able The presentation lock namespace had to be owned by this user AND be exactly mode 700. Git Bash mounts NTFS `noacl` by default, where chmod is a silent no-op and every directory reads 755, so the assertion could never hold. The namespace was therefore permanently unobtainable, and because teardown_herdr_preflight_target resolves that lock for every herdr endpoint, teardown refused forever with no manual recovery - a finished task could not be collected, its metadata could not be cleared, and supervision stayed permanently "needed" as a result. Ownership and the symlink refusal are unchanged and are relaxed by neither branch. The exact-700 assertion is still required verbatim wherever the filesystem can store it. Only where the filesystem provably cannot is ownership accepted as the available guarantee. Capability is probed rather than inferred from platform or mount options: create a directory, ask for 0700, read back what was stored. A filesystem that discards the request answers for itself, so no uname or /etc/fstab parsing decides a security-relevant branch. The relaxation is narrow in practice as well as in code: on that mount Git Bash maps /tmp to the user's own profile temp directory (`usertemp`), so the namespace is not in a shared location and Windows' access control, rather than the emulated mode bits, is what actually guards it. Also repairs test_presentation_session_lock_path_is_shared_across_homes, which failed on unmodified main on such a host for this same reason.
Owner
|
Speaking as Kun's firstmate: closing this as stale. It has been waiting on a contributor update for 14+ days with no author push or comment. Reopen if you want to pick it back up. |
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.
Herdr is the only runtime available on the Windows host these were found on (tmux, zellij, orca and cmux are all absent), so every dispatch goes through it. Three POSIX-only assumptions in the adapter each refused work outright. Two of them are fixed here; both are narrow and independently useful on any platform.
1. Socket path assumed POSIX
fm_backend_herdr_canonical_socket_pathaccepted only paths starting with/. Herdr on Windows reportsC:\Users\...\AppData\Roaming\herdr\herdr.sockin bothHERDR_SOCKET_PATHandherdr session list --json, so the launcher-identity check refused every spawn with "reports an unusable socket path". Both sides report the identical value, so accepting the drive-letter spelling is sufficient. Separators fold so the two Windows spellings compare equal, including when the directory no longer resolves.2. jq emits CRLF
jq on that host opens stdout in text mode:
printf '{"a":["x","y"]}' | jq -r '.a[]'returnsx\r\ny\r\n. Command substitution strips only the trailing CR, so single-value reads are clean but multi-line reads keep an interior CR on every line but the last. That corrupted ids split out of multi-line captures and operator-facing messages. Three multi-line captures in the adapter now strip at source.Not audited here: jq is used throughout
bin/, so other scripts capturing multi-line jq output on Windows may have the same defect.3.
mkdir -m 700cannot produce mode 700 on NTFSThe presentation lock namespace had to be owned by this user and be exactly mode
700. Git Bash mounts NTFSnoaclby default, wherechmodis a silent no-op and every directory reads755, so the assertion could never hold. The namespace was permanently unobtainable, and becauseteardown_herdr_preflight_targetresolves that lock for every herdr endpoint, teardown refused forever with no manual recovery: a finished task could not be collected, its metadata could not be cleared, and supervision stayed permanently "needed" as a result.Ownership and the symlink refusal are unchanged. The exact-700 assertion is still required verbatim wherever the filesystem can store it; only where the filesystem provably cannot is ownership accepted as the available guarantee.
Capability is probed, not inferred: create a directory, ask for
0700, read back what was stored. A filesystem that discards the request answers for itself, so nounameor/etc/fstabparsing decides a security-relevant branch. The relaxation is also narrow in practice: on that mount Git Bash maps/tmpto the user's own profile temp directory (usertemp), so the namespace is not in a shared location and Windows access control, rather than emulated mode bits, is what actually guards it.This also repairs
test_presentation_session_lock_path_is_shared_across_homes, which fails on unmodifiedmainon such a host.Testing
tests/fm-backend-herdr.test.shagainst this branch on the Windows host: 169 pass, 0 fail, exit 0 - the full suite runs clean to completion. Unmodifiedmainaborts at test 20 on the CRLF fault, so most of the suite was never reached there at all.New coverage accompanies each fix: drive-letter socket acceptance, unchanged POSIX and refusal behaviour, separator folding when the directory is gone, and the probed-capability namespace check.
Two notes for anyone else running the suite on Windows:
dofork: child -1 ... Resource temporarily unavailableretry noise from Git Bash partway through. That is fork-retry churn, not failure; given enough time the suite completes cleanly. A short timeout will make it look like a hang.bin/fm-lint.shcould not be run for CI parity. Lint review is worth a close look.A fourth Windows fault is not addressed here:
herdr pane getreports noforeground_cwdfield at all, so the worktree-entry poll cannot succeed. A candidate fix exists but is deliberately held back until a real spawn is observed to succeed.🤖 Generated with Claude Code
https://claude.ai/code/session_01F7L1m56txHs7hrNcTeq7AP