Skip to content

cmux-next: FolderChanges.events() returns once its watches are registered - #26517

Merged
teamleaderleo merged 7 commits into
feat-cmux-nextfrom
ib/folderchanges-registration
Oct 29, 2026
Merged

teamleaderleo merged 7 commits into
feat-cmux-nextfrom
ib/folderchanges-registration

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Fix-forward for #26320 (squash 39d216b), from its post-landing review, which answered FIX:

The startup race remains — FolderChanges.swift:44,87. arm() calls source.resume(), which does not guarantee registration has completed. dispatch_source(3) explicitly documents that events during this delay are missed. A write after events() returns can therefore still disappear. Establish readiness through setRegistrationHandler before promising the watch is armed; rearming needs the same guarantee.

The test remains timing-dependent — FolderChangesTests.swift:27–32. [...] Add a deterministic readiness test with delayed registration.

What changes

  • FolderChanges.events() is now async. arm() gives every dispatch source a registration handler and waits for each one before it returns, so a change made after events() returns is always delivered. The wait ends early if the calling task is cancelled, so a cancelled caller never hangs.
  • When the watch has to move, the loop waits for the new watches to register before it cancels the old ones and yields the element, so re-arming has the same guarantee.
  • BackdropCatalogStore (the only production caller) awaits events().
  • If the caller is cancelled during that wait, events() cancels the watch and returns a finished stream. The detached loop never holds descriptors for a caller that will not iterate.
  • New test FolderChangesTests.eventsReturnsOnlyOnceTheWatchesAreRegistered. An internal init sets the queue the sources target. The test holds that queue suspended so no registration handler can run, and starts events() with Task.immediate, so it runs synchronously up to its first suspension. It checks two things: events() returns only after the queue is resumed (from an ordered step log), and a write made the moment it returns produces an element. There is no sleep. With the fix the test cannot fail, however the threads are scheduled. Without the wait, events() has no suspension point and returns before the test resumes the queue, so the test fails every time.
  • Kept from the earlier review: there are no new file descriptors or sources. Cancel handlers still close every fd, and cancellation still reaches armed.cancel().

No capture: no visible UI change; this only changes when the folder watch behind the wallpaper catalog reports that it is ready.

Risk

Low. The change is contained to the folder watch behind the wallpaper catalog. events() now waits for kernel registration before returning, which is sub-millisecond.

Verification

Fleet runs pending; this section is updated with their results.

Agent session: agent session

🤖 Generated with Claude Code

…ered

Post-landing review of #26320: arm() resumed each DispatchSource and returned,
but resume() only schedules the kernel registration, and dispatch_source(3)
says events before it completes are missed, so a write right after events()
returned could still be lost. arm() now waits for every source's registration
handler (events() is async), and the loop re-arms the same way before it
yields. A new test holds the watches' queue suspended and checks events()
returns only after it is let go, and that a write the moment it returns is
seen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 366a8e7: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr26517 -f ref=ib/folderchanges-registration -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); NEW: green on the base 8acb82a; do not merge until green; see lane-rules

@github-actions github-actions Bot added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 28, 2026
…terministic

Review of the fix: the 200 ms sleep did not guarantee the task ran while the
queue was held, so a regression could still pass. Task.immediate runs events()
on the test's thread until its first suspension, which with the fix is the
wait for registration and without it is never reached before it returns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5
@blacksmith-sh

blacksmith-sh Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Found 7 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpdxav8fkh/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmptu6w8ae3/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-0ks52i created for this PR by the cmux coordinator.

… stream

Review of the fix: a cancelled caller that never iterates would leave the
detached loop holding the watch and its descriptors. events() now cancels the
watch and finishes the stream instead of starting the loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-cc8bff1c3ddb4ec353c5
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): SHIP at 366a8e7

No blocking findings. Reviewed registration readiness, rearming, cancellation, and the updated caller and tests. Validation was limited to the supplied files; tests were not run.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Orphaned: no activity for 15 h, and no lane holds a lease on it. Adopted by the orphan sweep: merged feat-cmux-next in at ce569c4. A later pass queues it for the merge hub if it is reviewed, small and red only on base reds.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): SHIP at ce569c4

No blocking findings.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 28, 2026 21:46 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 28, 2026 21:46 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 28, 2026 22:01 Inactive
@cmux-bot
cmux-bot Bot dismissed their stale review October 29, 2026 22:46

A newer verdict blocks 61e571c (no independent SHIP at the head).

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): SHIP at 61e571c

No blocking findings.

Reviewed the supplied diff and changed files. Tests were not run; access to additional repository code was blocked by the tool approval policy.

@cmux-bot cmux-bot 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.

cmux-bot approves 61e571c: an independent review passed this exact head (next-review receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@teamleaderleo
teamleaderleo merged commit 00d6d54 into feat-cmux-next Oct 29, 2026
148 of 153 checks passed
@teamleaderleo
teamleaderleo deleted the ib/folderchanges-registration branch October 29, 2026 23:43
@github-actions

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40975388675 at 43303f7 failed: cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next fleet Release compile.
Those jobs last passed at 94e3fa5. This pull request is one of 3 merged since: #26517, #27679, #26082.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40975576275 at 60a6a31 failed: cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next fleet Release compile.
Those jobs last passed at 94e3fa5. This pull request is one of 4 merged since: #26517, #27679, #26082, #27682.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions

Copy link
Copy Markdown
Contributor

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/40977234399 at 817e10c failed: cmux-next checks (god files, concurrency, crash safety, l10n), cmux-next fleet Release compile.
Those jobs last passed at 94e3fa5. This pull request is one of 4 merged since: #26517, #26082, #25151, #25420.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

teamleaderleo pushed a commit that referenced this pull request Oct 30, 2026
…ld fix) (#27758)

#26517 made FolderChanges.events() async (it returns once its watches are
registered); #27223, which landed an hour before it, calls it from the
synchronous MachineConsentChanges.events(armed:). Together they stop
CmuxNextApp from compiling (JoinConsent+App.swift:222: 'async' call in a
function that does not support concurrency; fleet job
549a2bfa50c634b6131799d8). MachineConsentChanges.events(armed:) is async
now and awaits it, so the folder watch is registered before the file
watches and the armed handshake, as its comment already promised; its
callers (the model's watch task and JoinConsentTests) await it.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
austinywang pushed a commit that referenced this pull request Oct 30, 2026
…atch

#26517 made FolderChanges.events() async (it now returns once its watches
are registered). #27223 landed alongside it with MachineConsentChanges
calling events() synchronously, so feat-cmux-next stopped compiling in
Release from 00d6d54 on (cmux-next/compile red on every landing since),
and no NEXT nightly can build a newer commit.

MachineConsentChanges.events(armed:) is now async and awaits the folder's
registration before arming the files, which keeps its documented order;
its app caller and the two tests await it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
austinywang pushed a commit that referenced this pull request Oct 30, 2026
…atch

#26517 made FolderChanges.events() async (it now returns once its watches
are registered). #27223 landed alongside it with MachineConsentChanges
calling events() synchronously, so feat-cmux-next stopped compiling in
Release from 00d6d54 on (cmux-next/compile red on every landing since),
and no NEXT nightly can build a newer commit.

MachineConsentChanges.events(armed:) is now async and awaits the folder's
registration before arming the files, which keeps its documented order;
its app caller and the two tests await it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
austinywang pushed a commit that referenced this pull request Oct 30, 2026
…atch

#26517 made FolderChanges.events() async (it now returns once its watches
are registered). #27223 landed alongside it with MachineConsentChanges
calling events() synchronously, so feat-cmux-next stopped compiling in
Release from 00d6d54 on (cmux-next/compile red on every landing since),
and no NEXT nightly can build a newer commit.

MachineConsentChanges.events(armed:) is now async and awaits the folder's
registration before arming the files, which keeps its documented order;
its app caller and the two tests await it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
austinywang pushed a commit that referenced this pull request Oct 30, 2026
…atch

#26517 made FolderChanges.events() async (it now returns once its watches
are registered). #27223 landed alongside it with MachineConsentChanges
calling events() synchronously, so feat-cmux-next stopped compiling in
Release from 00d6d54 on (cmux-next/compile red on every landing since),
and no NEXT nightly can build a newer commit.

MachineConsentChanges.events(armed:) is now async and awaits the folder's
registration before arming the files, which keeps its documented order;
its app caller and the two tests await it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
austinywang pushed a commit that referenced this pull request Oct 30, 2026
…atch

#26517 made FolderChanges.events() async (it now returns once its watches
are registered). #27223 landed alongside it with MachineConsentChanges
calling events() synchronously, so feat-cmux-next stopped compiling in
Release from 00d6d54 on (cmux-next/compile red on every landing since),
and no NEXT nightly can build a newer commit.

MachineConsentChanges.events(armed:) is now async and awaits the folder's
registration before arming the files, which keeps its documented order;
its app caller and the two tests await it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
austinywang pushed a commit that referenced this pull request Oct 30, 2026
…atch

#26517 made FolderChanges.events() async (it now returns once its watches
are registered). #27223 landed alongside it with MachineConsentChanges
calling events() synchronously, so feat-cmux-next stopped compiling in
Release from 00d6d54 on (cmux-next/compile red on every landing since),
and no NEXT nightly can build a newer commit.

MachineConsentChanges.events(armed:) is now async and awaits the folder's
registration before arming the files, which keeps its documented order;
its app caller and the two tests await it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-fb75b7af9902231028aa
@github-actions

Copy link
Copy Markdown
Contributor

Shipped in:

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 61e571c3 Deployed Oct 29, 2026 by teamleaderleo via wait for same-key tree publisher #78678
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant