Skip to content

fix(doctor): ignore vanished entries during acl tree verification - #213

Merged
kyle-sexton merged 2 commits into
mainfrom
cursor/issue-114-doctor-acl-race-5554
Aug 12, 2026
Merged

fix(doctor): ignore vanished entries during acl tree verification#213
kyle-sexton merged 2 commits into
mainfrom
cursor/issue-114-doctor-acl-race-5554

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

verifyACLTree aborted the entire filepath.WalkDir when ACL.Verify returned fs.ErrNotExist for a child that had been listed moments earlier — a TOCTOU race against atomic temp-file + rename writes in state (observed.json, jobs.json). That produced a spurious acl/state (and occasionally other acl/*) healthy:false for one poll.

This change treats mid-enumeration vanish (fs.ErrNotExist on a non-root path, either as a walk error or ACL probe error) as benign and continues the walk. Real ACL faults and a missing root still fail the check.

Fixes #114

Test plan

  • TestVerifyACLTreeIgnoresVanishedChildDuringProbe — wrapped fs.ErrNotExist from ACL verifier on a listed child does not fail the walk
  • TestVerifyACLTreeFailsOnRealACLFault — non-ErrNotExist ACL faults still abort
  • TestVerifyACLTreeFailsOnMissingRoot — missing root still fails
  • TestVerifyACLTreeFailsWhenRootVanishesAfterWalkBegins — root ErrNotExist from ACL verifier after walk starts still fails
  • TestLocalDoctorInspectorACLStateCheckFailsWhenRootVanishesDuringProbe — end-to-end acl/state stays unhealthy when root vanishes before probe
  • TestVerifyACLTreeIgnoresVanishedChildDuringWalk — child removed between listing and probe stays healthy
  • TestLocalDoctorInspectorACLStateCheckStaysHealthyWhenChildVanishesDuringProbe — end-to-end acl/state doctor check stays healthy with unchanged entry count
  • go test -race ./internal/app/...

Related

Fixes #114

@cursor

cursor Bot commented Aug 11, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f03fb67b6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread internal/app/doctor_inspector.go Outdated
@kyle-sexton kyle-sexton changed the title doctor: ignore vanished entries during acl/* tree verification fix(doctor): ignore vanished entries during acl tree verification Aug 11, 2026
@kyle-sexton
kyle-sexton enabled auto-merge (squash) August 11, 2026 23:53
cursoragent and others added 2 commits August 12, 2026 00:02
verifyACLTree aborts the entire WalkDir when ACL.Verify returns
ErrNotExist for a child listed moments earlier — a TOCTOU race against
atomic temp-file + rename writes in state/jobs.json/observed.json.

Skip fs.ErrNotExist for non-root paths during walk and after ACL probe
so transient rename windows do not spuriously mark acl/* unhealthy, while
real ACL faults and a missing root still fail the check.

Fixes #114

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Only suppress ACL.Verify fs.ErrNotExist for non-root paths; the walk-error
branch already excluded root, but the verifier branch did not, so a root
removed after WalkDir lookup could spuriously mark acl/* healthy.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor
cursor Bot force-pushed the cursor/issue-114-doctor-acl-race-5554 branch from 9bcc9bd to 3e6b4e2 Compare August 12, 2026 00:02
@kyle-sexton
kyle-sexton merged commit 9bf734d into main Aug 12, 2026
27 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/issue-114-doctor-acl-race-5554 branch August 12, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doctor: acl/* checks transiently report unhealthy during concurrent state-directory writes (false-negative health alarm)

2 participants