Report unreachable daemon discovery errors - #74
Merged
Conversation
roborev: Combined Review (
|
roborev: Combined Review (
|
A sandbox can read a live daemon's runtime record while denying access to its endpoint. Treating that failed probe as absence lets an ensure operation start a competing daemon against the same state. Keep scanning for another reachable record, but return the first live-record probe failure when none succeeds. Callers can now distinguish definite absence from indeterminate endpoint access and avoid unsafe restart behavior. The repository hooks also had a stale analyzer and test-helper baseline that blocked this change. Bring the existing Go code and Git fixtures under the current lint, NilAway, isolation, and shuffled-test rules so the hooks can enforce those rules on later changes. Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>
PID-aware discovery uses one broad error for any live candidate it cannot safely select, not only socket permission failures. State the opt-in boundary and wrapped-cause contract so callers do not promise default protection or show the wrong recovery guidance. Pin the two compatibility edges that carry the most risk: a definite process-identity mismatch must never reach the recorded endpoint, while callers that leave PID checks disabled retain the earlier absence behavior. Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>
Git exports repository-local variables to pre-commit hooks, and developer configuration can add hooks of its own. Fixtures that inherit either source can test the caller's repository instead of the temporary repository they create. Strip repository bindings and give fixture commands empty global, system, and XDG configuration sources. Tests that need specific Git configuration now add it on top of that isolated base. Generated with OpenAI Codex Co-authored-by: OpenAI Codex <noreply@openai.com>
mariusvniekerk
force-pushed
the
t3code/improve-sandbox-daemon-handling
branch
from
August 22, 2026 14:48
3d27ab2 to
5e15444
Compare
roborev: Combined Review (
|
Three merge-request import tests set NullGlobalConfig=false and put their own GIT_CONFIG_GLOBAL into the runner environment. gitcmd.New() also sets StripEnv, which removes every GIT_* variable from that environment, so the override never reached git. Git then read the developer's real ~/.gitconfig. On a machine whose global config defines hook.* entries, the untrusted-import hook scan saw them and the tests failed; in CI they passed, but the rollback test's fsmonitor and filter configuration never loaded, so its assertions could not fail. Disable StripEnv at those three sites. Their environments already come from isolatedLifecycleBaseEnv, so the override now reaches git and the suite passes under a real developer Git configuration. Generated with Claude Code (claude-fable-5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
roborev: Combined Review (
|
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
2 participants
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.
A sandboxed client can read a live daemon's runtime record while still being unable to reach its endpoint. Discovery previously collapsed that probe failure into absence, so
Manager.Ensurecould start a competing daemon against the same state.PID-aware discovery now skips definite process-identity mismatches, keeps scanning after a failed probe, and returns a typed
UnreachableErrorwith the record, endpoint, and underlying probe failure when no later record succeeds. The manager already stops on discovery errors, so this prevents an unsafe start without adding restart policy to the shared package.The repository hooks had a stale whole-repo lint, NilAway, Testify-helper, and Git-fixture baseline that blocked the daemon change. Most of the resulting test churn is mechanical helper conversion. The fixture changes isolate global and system Git config and strip repository-local variables inherited by hooks, which makes the shuffled suite deterministic under the real commit environment.
Sources of inspiration: