Skip to content

test(daemon): plant a scheduler owner only once its task ended - #4182

Merged
ScriptedAlchemy merged 1 commit into
masterfrom
fix/4165-concurrent-reenable
Oct 17, 2026
Merged

ScriptedAlchemy merged 1 commit into
masterfrom
fix/4165-concurrent-reenable

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Fixes #4165.

Root cause

The test was wrong. Production was not.

The test plants a finished scheduler owner, then runs two concurrent re-enables. Its barrier was a oneshot sent from inside the planted task's body. That signal comes before the task ends. The native runtime marks the task finished only after the body returns. On a loaded worker, the re-enables can run in that window. observed_scheduler_lifecycle then reads the planted owner as Running, so both re-enables return RunningNotified. That is correct for a live owner. The earlier assertions still pass because the planted task still reads as live. Only the final pair assertion fails, as the issue reports.

Change

  • The test awaits the planted task's NativeTaskOutcome::Completed before it inserts the handle. The oneshot is gone.
  • The final assertion prints the outcome pair. The issue noted it did not.

Verification


Devin Review

The concurrent re-enable test planted a finished scheduler owner, but its
barrier was a signal sent from inside the task body. The native runtime
marks the task finished only after that body returns, so a loaded worker
left the owner reading as live. Both re-enables then notified it and the
test saw (RunningNotified, RunningNotified), which is correct production
behavior for a live owner.

Await the planted task's outcome before inserting it, and print the
outcome pair when the final assertion fails.

Fixes #4165
@changeset-bot

changeset-bot Bot commented Oct 17, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b5b1cc2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@ScriptedAlchemy
ScriptedAlchemy merged commit 350c7e3 into master Oct 17, 2026
15 of 20 checks passed
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.

test(daemon): concurrent re-enable scheduler test flakes 2/10 on master

1 participant