doctor: a presence file that was never written is not fresh - #10
Closed
schickling-assistant wants to merge 1 commit into
Closed
doctor: a presence file that was never written is not fresh#10schickling-assistant wants to merge 1 commit into
schickling-assistant wants to merge 1 commit into
Conversation
read_state is deliberately permissive and maps a missing status file to offline, which is indistinguishable from an agent that deliberately set itself offline. doctor only rejected unknown, so an agent whose presence nothing has ever written passed a check literally named "presence fresh" — and printed "presence fresh (is offline)" while no status file existed. Check for the file first and report its absence as its own problem, naming the actual condition. read_state's documented read semantics are untouched, and a deliberately-offline agent still passes. Whether an agent that declares no ding should be exempted from this is a separate question I did not presume to answer.
Contributor
|
Thank you, @schickling-assistant. Closing this without merge because the same missing-presence distinction is already on Doctor now reports an absent status file as The PR branch is based on the older 49fd943 tree, so merging it now would duplicate/conflict with the accepted implementation. |
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.
Fixes #5.
read_stateis deliberately permissive and maps a missing status file toOffline(src/status.rs:74-77), which is indistinguishable from an agent that deliberately set itself offline. Doctor's gate only rejectedUnknown(src/main.rs:952), so an agent whose presence nothing has ever written passed a check literally named presence fresh — and printed✓ <id> presence fresh (isoffline)while nostatusfile existed at all, followed by✓ all checks passed.The change. Doctor checks for the file first and reports its absence as its own problem, naming the actual condition (
nostatusfile — nothing is maintaining this agent's presence).read_state's documented permissive read semantics are untouched, and a deliberately-offlineagent still passes —offlineis a settable state and being offline on purpose is healthy; only absent is not.Tests (
tests/doctor.rs, a new file so it will not conflict with in-flight edits elsewhere). Both cases run against an identical catalog and differ only in whether the presence file exists:4e25f56.offline→ still✓ … presence fresh (isoffline). Passes on4e25f56and after, so the permissive case is pinned against regression.One thing I deliberately did not decide. An agent that declares no
dingwill now show this problem. That may be exactly right (doctor cannot observe it, and says so), or you may want such agents exempted — it is a one-line refinement on top and it felt like your call rather than mine.You may have this in unpushed work already; happy for it to be closed in favour of your version.
Note:
codex_hooks,materialize,native_onlyandtransport_isolationalready fail on a pristine4e25f56in my environment (missing shims / sandbox —native_only's doctor case shells out to/bin/sleep, which does not exist here). The failure set is identical before and after this change.