Skip to content

test(feed): wait for zero-wait Codex permission acceptance - #16536

Merged
teamleaderleo merged 3 commits into
mainfrom
fix/feed-codex-attention
Oct 2, 2026
Merged

teamleaderleo merged 3 commits into
mainfrom
fix/feed-codex-attention

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

FeedCoordinatorTests.zeroWaitCodexPermissionSurfacesTransientNeedsInputAttention, added in #15926, fails on main with attention.events.count → 0. The product change works as intended. The test is racy: ingestBlocking(waitTimeout: 0) acknowledges when the event is enqueued, and the ingress lane accepts it later on its execution queue. That ordering is the documented zero-wait contract (zero-wait telemetry must acknowledge before asynchronous acceptance). The test read the attention recorder as soon as ingestBlocking returned, so it usually ran before the main-actor acceptance that surfaces attention.

The test now passes an onAccepted callback and waits for it before checking the recorder. No product code changes.

Testing

CI; no local build per team rule.

Changelog

none

🤖 Generated with Claude Code

https://claude.ai/code/session_01FrR7YbsQtGw2eFtDeyiTcK


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


Summary by cubic

Fixes the racy zeroWaitCodexPermissionSurfacesTransientNeedsInputAttention test, which read the attention recorder before zero-wait ingress had asynchronously accepted the event.

  • Waits for the onAccepted callback before asserting on the attention hook.
  • No product code changes.

Written for commit 38b64bb. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Tests
    • Updated the zero-wait permission test to verify that asynchronous acceptance is recorded after the immediate return.

…cking attention

Zero-wait Feed ingress acknowledges at enqueue and accepts asynchronously on
the ingress lane, so the test raced the delivery and saw no attention event.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FrR7YbsQtGw2eFtDeyiTcK
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (3)
.github/review-bot-rules/test-determinism.md — configured
.github/review-bot-rules/swift-architectural-rethink.md — configured
.github/review-bot-rules/source-control-artifacts.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 50409231-383a-488d-9981-47ae2d10c8d9

📥 Commits

Reviewing files that changed from the base of the PR and between 8b8762a and 38b64bb.

📒 Files selected for processing (1)
  • cmuxTests/FeedCoordinatorTests.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The zero-wait Codex permission test now waits for the asynchronous onAccepted callback before it checks recorded attention events.

Changes

Codex permission acceptance test

Layer / File(s) Summary
Wait for asynchronous acceptance
cmuxTests/FeedCoordinatorTests.swift
The test signals a separate semaphore from onAccepted and waits for it after zero-wait ingestion returns, before checking attention events.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to 38b64

This test change waits for asynchronous acceptance before checking attention events. The callback ordering and bounded wait leave no actionable merge-blocking risk.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary change: updating the feed test to wait for zero-wait Codex permission acceptance.
Description check ✅ Passed The description explains the race, the test fix, testing status, and changelog entry. The optional demo section and checklist are omitted, but the core required information is complete for this test-o…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The authoritative diff changes only cmuxTests/FeedCoordinatorTests.swift. It adds an acceptance callback and semaphore wait to a zero-wait feed test. It does not change Cloud terminal creation…
Cmux Swift Actor Isolation ✅ Passed PASS. The review-scoped diff changes only cmuxTests/FeedCoordinatorTests.swift. It adds a test semaphore and an onAccepted callback wait. No production Swift code changes occur, so the production …
Cmux Swift Blocking Runtime ✅ Passed PASS: The pull request changes only cmuxTests/FeedCoordinatorTests.swift. It adds a DispatchSemaphore and a bounded wait inside a test to await the existing onAccepted callback. The Swift bloc…
Cmux Browser Automation Off-Main ✅ Passed PASS: The pull request changes only cmuxTests/FeedCoordinatorTests.swift. The diff adds a semaphore and onAccepted wait to a FeedCoordinator test. It does not add or reroute any browser.* socket…
Cmux Expensive Synchronous Load ✅ Passed The pull request changes only cmuxTests/FeedCoordinatorTests.swift. The diff adds a semaphore and an existing onAccepted callback wait to a test. It does not change production Swift code or add an…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only cmuxTests/FeedCoordinatorTests.swift, a test file. It adds an acceptance callback wait and does not replace any authoritative production read with a cache or oppo…
Cmux No Hacky Sleeps ✅ Passed PASS. The PR changes only cmuxTests/FeedCoordinatorTests.swift. It adds a semaphore and waits for the onAccepted callback, which is event-driven test synchronization. It does not add a fixed sleep…
Cmux Algorithmic Complexity ✅ Passed PASS: The only changed file is cmuxTests/FeedCoordinatorTests.swift, and the diff adds a semaphore callback wait inside a test. The algorithmic-complexity rule explicitly passes test-only scaffoldin…
Cmux Swift Concurrency ✅ Passed PASS. The only changed file is cmuxTests/FeedCoordinatorTests.swift. The diff adds a semaphore and an existing onAccepted callback to synchronize a zero-wait test before asserting attention delive…
Cmux Swift @Concurrent ✅ Passed PASS. The PR changes only cmuxTests/FeedCoordinatorTests.swift; it adds a DispatchSemaphore and a synchronous onAccepted callback to an existing ingestBlocking call inside `DispatchQueue.globa…
Cmux Swift Package Boundaries ✅ Passed The pull-request diff changes only cmuxTests/FeedCoordinatorTests.swift. It adds a test semaphore and waits for onAccepted; it does not change production Swift code or Swift package boundaries. Th…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes only cmuxTests/FeedCoordinatorTests.swift. The diff contains no Package.swift, Package.resolved, .gitignore, Xcode project, or workflow changes. It therefore introduces no…
Cmux Swift Logging ✅ Passed The pull request changes only a Swift test. The added lines add a semaphore, an onAccepted callback, and an expectation; they add no print, debugPrint, dump, NSLog, file logging, or Logger…
Cmux User-Facing Error Privacy ✅ Passed PASS — the authoritative diff changes only cmuxTests/FeedCoordinatorTests.swift. It adds a semaphore and onAccepted wait to a test; it makes no production change and creates no cmux end-user error…
Cmux Full Internationalization ✅ Passed The pull request changes only cmuxTests/FeedCoordinatorTests.swift. The changes add a test semaphore, callback, comment, and assertions. No production code, user-facing Swift text, catalog, web UI, …
Cmux Swiftui State Layout ✅ Passed PASS. The PR changes only cmuxTests/FeedCoordinatorTests.swift. The added lines add a semaphore and an onAccepted callback to a test. They do not introduce SwiftUI state, layout measurement, lazy/…
Cmux Architecture Rethink ✅ Passed PASS. The PR changes only cmuxTests/FeedCoordinatorTests.swift. It adds a test-only DispatchSemaphore and waits for the existing onAccepted callback before reading the recorder. The architectura…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The pull request changes only cmuxTests/FeedCoordinatorTests.swift. The change adds a semaphore and onAccepted wait to a test-only fixture. It introduces no NSWindow, NSPanel, `NSWindowControl…
Cmux Source Artifacts ✅ Passed PASS. The pull request changes only cmuxTests/FeedCoordinatorTests.swift. The diff adds test synchronization with an onAccepted callback and a comment. This is intentional hand-written test source…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The pull request changes only cmuxTests/FeedCoordinatorTests.swift. No changed Swift file is under a production Sources/ path. The diff adds a test-local semaphore and onAccepted callback wait; …
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Subagent review at ecbca1c: approve. On the zero-wait path, enqueueZeroWaitAcceptance reaches surfaceBlockingDecisionAttention (the attention observer) inside acceptOnMainActor, before onAccepted is invoked. onAccepted fires for this codex permission event, so the semaphore wait removes the race without hiding a missing delivery.

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 1 file

Re-trigger cubic

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

CI failure attribution

CI passes on 38b64bb9ee (run 36944172123 attempt 1).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

@cursor

cursor Bot commented Oct 2, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Green at 38b64bb; landing. Resource check: test-only. It adds one semaphore wait capped at 2 s inside a single test and changes no product code, so no runtime CPU, memory or disk impact.

@teamleaderleo
teamleaderleo merged commit fcda4f0 into main Oct 2, 2026
53 checks passed
@teamleaderleo
teamleaderleo deleted the fix/feed-codex-attention branch October 2, 2026 00:36
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Merge receipt for 38b64bb9ee: every check was green at merge (15 verified; 17 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Oct 2, 2026
0bfd027 test(cloud): fix the Cloud header and moved-panel focus tests that never ran (manaflow-ai#16539)
c5c4345 localization: accept numbered placeholders in any order (manaflow-ai#16376)
456edeb fix(settings): replace custom sidebar mockups with real previews (manaflow-ai#16569)
98dc3ab Prototype: cmux Cloud as a remote MCP server (manaflow-ai#16568)
6c22525 test(remote): isolate tmux stale-surface fixture (manaflow-ai#16566)
3ec9918 Re-land "fix(coderouter): initialize Cloud VM account pools (manaflow-ai#16397)" (manaflow-ai#16572)
2b895a5 Fix browser paste routing with terminal text box beta (manaflow-ai#6380) (manaflow-ai#16560)
2bd3455 localization: check Swift defaultValue literals against their catalog en value (manaflow-ai#16396)
c43086e test(cli): expect --mark-read to mark every listed inbox message (manaflow-ai#16537)
fcda4f0 test(feed): wait for zero-wait Codex permission acceptance before checking attention (manaflow-ai#16536)
7d57a03 fix(remote): evict stale persistent SSH bridge leases (manaflow-ai#16558)
d630cb8 docs: add protected-folder diagnostics for tmux sessions (manaflow-ai#12219)
7dceaac test: create cwd fixtures that new terminals now resolve on disk (manaflow-ai#16538)
28cc575 docs: cover surface resume binding CLI contract (manaflow-ai#16473)
5c7dca1 Fix idle zsh PR probes triggering chpwd hooks (manaflow-ai#16553)

# Conflicts:
#	.github/workflows/ci-guards.yml
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.

1 participant