Repository navigation
ci: the notification-center lint also fails a global center handed to a poster - #18818
Merged
Merged
Conversation
…ent fails the lint The lint caught only a literal NotificationCenter.default.post. The off-main notice tests posted through OffMainPost.send() with .default handed in (on: .default, or a NotificationCenter = .default parameter default), and ViewBridge's own didBecomeKey observer trapped the whole swift test run (#18815). Those forms must fail the gate too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… a poster check-test-notification-centers.sh now also fails a process-global center passed as an argument (on: .default, center: NotificationCenter.default, the workspace or distributed center) or as a NotificationCenter parameter's default. The off-main notice tests posted through OffMainPost.send() that way, and ViewBridge's own didBecomeKey observer trapped the whole swift test run (#18815). The 11 existing sites, all deliberate off-main posts that prove an app observer hops to main, carry a reviewed global-notice-allow naming that risk; new ones must inject a center. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Contributor
|
Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by |
This comment has been minimized.
This comment has been minimized.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…a static-only type [cx-94dl] 35a61bd added enum HomeAppLinks (all-static public surface), which lint-ios-package-conventions.sh refuses; cdd28b4 got past the lint by making it a struct with an empty public init. URL.isChiefSubagentLink is the same check on the receiver type; behavior is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…18896) * test(ci): the base autofix regenerates without touching frozen paths (red) base-autofix.sh copies the Mac job's regenerated files into a checkout, skipping frozen paths, and adds a missing cmux-tui tree input to both lists. Red: the script does not exist yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: a base autofix bot commits generated files, tree inputs and fmt After each cmux-next run on a feat-cmux-next push, cmux-next-base-autofix repairs the mechanical reds in one commit on top of it: the generated files the Mac generated-files job rebuilt (its cmux-next-generated-patch artifact, so no fleet Mac), missing cmux-tui tree inputs and cargo fmt in cmux-tui. Frozen paths (base-autofix-frozen.txt) are reported and never written; the app FFI pin stays with the app-ffi-repin pull request. The push uses a GitHub App token minted only for the push step and a plain push that never overwrites a moved base. Dry run until the repo variable CMUX_NEXT_BASE_AUTOFIX is 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(ci): the base autofix never writes a workflow file (red) The bot pushes with GITHUB_TOKEN, which cannot push .github/workflows changes. A regenerated workflow file is skipped and reported, and a missing cmux-tui tree input (its second list is cmux-tui-artifacts.yml's paths) is reported without writing either list. Red: both are written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: the base autofix pushes as github-actions[bot] with GITHUB_TOKEN No GitHub App: the job's GITHUB_TOKEN (contents: write) pushes the repair; no ruleset or branch protection applies to feat-cmux-next. GITHUB_TOKEN cannot push workflow files, so every .github/workflows path is frozen for the bot, and missing cmux-tui tree inputs (whose second list is cmux-tui-artifacts.yml's paths) are reported, not written. Its pushes start no workflow: the next push's cmux-next run verifies the repair commit. The trigger also checks the run's workflow path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…x-tui crate (#18913) 36c60c8 moved the image paste spool into cmux-tui-image-paste, and cmux-tui-core's lib.rs re-exports it (use cmux_tui_image_paste:: image_paste;). The image staleness guard (8e23f8a, cx-4l51) only looked for crate::image_paste under cmux-tui-core, found 9 constants named CAPABILITY and failed, so release rails is red on every PR and on nightly-next. A crate:: path whose first segment lib.rs re-exports from a workspace crate now also names that crate's module file. No bead (repairs the cx-4l51 guard). Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci: format staged files in the trusted pre-commit hook * ci: guard formatter scope for staged files * ci: scope Rust formatter guard to its package * ci: use direct rustfmt and skip merge formatting * ci: pass the cmux-tui Rust edition to rustfmt
* test(ci): red test, PRs check rustfmt on the cmux-tui packages they touch PR CI never runs cmux-tui.yml (dispatch only), so #18706 landed unformatted Rust and broke base cargo fmt --check. These tests pin a helper that runs cargo fmt --check once per touched package, checks every package when a rustfmt config changes or there is no base, and names each unformatted package. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: check rustfmt on the cmux-tui packages a PR touches PR CI never runs cmux-tui.yml (dispatch only), so #18706 landed unformatted Rust and broke base cargo fmt --check. cmux-tui-spec.yml already runs on every PR and push touching cmux-tui/** on hosted Linux; its new rustfmt job runs cmux-tui-fmt-changed.py, which maps each changed .rs file or Cargo.toml to its nearest tracked package and runs cargo fmt --check on it with the pinned toolchain. No build and no dependency download. A rustfmt config or toolchain change, or no base, checks every package; vendored crates and deleted packages are skipped. A PR compares with its merge commit's first parent, a push with the commit before it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(cmux-tui): rustfmt acpmux's draft tests and the relay circuit The new rustfmt job's full run found two base packages that fail cargo fmt --check: acpmux, from #18706's draft tests and their module order, and the Cloudflare relay's circuit.rs. Formatting only, exactly the hosted rustfmt diff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(cmux-tui): leave the relay circuit to its own repair Touching the Cloudflare relay runs its lane, whose npm audit is red on base (sharp and undici advisories through wrangler, fixed only by a wrangler bump outside the stated range). That is the relay's own repair, so this PR keeps acpmux's formatting only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci: add mini-first Swift canary for cmux-next * ci: remove unit tests from Swift canary change
…e cmux-tui (#18698) * test(cmux-next): red: a PR that changes no cmux-tui fails same-tree on an unpublished merge tree #16614, #17551 and #18605 change no cmux-tui or ghostty-next path, yet same-tree cmux-tui failed them because the base tip's tree was never published (its artifacts run was cancelled). Such a PR should reuse the newest published tree in its history (tree_fetch_key, fetched through CMUX_TUI_TREE_KEY), or skip the tree jobs when none is published, never fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): same-tree cmux-tui is a hard check only for PRs that change cmux-tui Path routing records whether the pull request's own diff touches cmux-tui/ or ghostty-next. For one that does not, a merge tree that is not ready no longer fails: the probe (CMUX_TUI_TREE_BASE_FALLBACK=1) resolves the newest published tree in the merge's history and reports ready with tree_fetch_key, which the daemon tests and the Debug scheme compile fetch through CMUX_TUI_TREE_KEY; with none published it reports skipped. Pushes, dispatches and PRs that change cmux-tui keep the hard check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cmux-next): the newest-published search deepens only this commit's history A bare `git fetch --deepen=50 origin` deepens every branch of origin: on this PR's first run it took path routing past its 5-minute limit (job 113609407061) before the probe's base fallback, which then resolved in under a second. Deepening only HEAD takes about 30 s on a depth-2 clone of feat-cmux-next. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* docs(cmux-next): map UI parity reference sources * docs(cmux-next): add media preview reference source * docs(cmux-next): define reference freshness checks
* test(nightly): the notes step cannot spend the publish budget before the upload (red) Run 37611677848 spent 29 of publish-nightly's 30 minutes on release notes and was cancelled mid-upload, leaving partial release assets. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(nightly): bound the publish notes step at 10 minutes The notes take seconds since 0115b26. A regression now fails the step before any asset upload, instead of spending publish-nightly's 30 minutes and being cancelled mid-upload with partial assets left on the release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…self (#17622) * test(cmux-next): the cmux-tui gate pins and dispatches an unpublished tree itself Publisher run 37505519359 failed transiently and its retry was replaced by a newer feat-cmux-next push, so the tree was never published and lanes pushed cmux-tui-pin-* branches by hand to unblock their gates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-next): the tree gate watches the base and may pin the tree Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): the cmux-tui gate pins and dispatches an unpublished tree When two run checks find no artifacts run that will publish the tree, the gate creates cmux-tui-pin-<key12> at the publisher commit and dispatches the artifacts workflow there, once, then keeps waiting. A pull request now watches its base commit (the merge's first parent) instead of no publisher, so it neither waits 45 minutes for a tree nobody builds nor needs a lane to push a pin branch by hand. A superseded push still skips; forks keep a read-only token and no autopin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-tui): a transient wrong binary read must not fail publication Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cmux-tui): read a mismatched published binary again before failing R2 objects are write-once, so a digest mismatch on a binary the run just uploaded is a bad read. Fetch it again from a cache-busting URL up to three times, and name the response headers when it still differs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(cmux-next): a pull request gate pins a tree its base never builds Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): a pull request gate pins a tree its base never builds A pull request that changes cmux-tui has a merge tree no push builds, and the artifacts workflow's pull_request_target trigger has never started a run, so its gate waited 45 minutes with the run check off. With CMUX_TUI_TREE_AUTOPIN the gate now pins its own checkout at once and watches that run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci(cmux-next): retain base tree dispatch tests --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* ci(gallery): capture narrow responsive states * test(gallery): cover workflow matrix changes * fix(gallery): dedupe equal responsive widths * ci: shard gallery matrix renders * ci: retrigger sharded gallery validation
Resolutions: feat-cmux-next deleted the off-main notice suites (unit test deletion pass 2, 36f2381), so the nine global-notice-allow comment edits go with them. The lint change stays: it passes on base's tree. The branch's new lint test cases are dropped (no unit tests in cmux-next); base's lint test still passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7a6cbb4. Configure here.
| contents: read | ||
| timeout-minutes: 20 | ||
| env: | ||
| CMUX_NEXT_RUNNER: ${{ needs.macos-placement.outputs.runner || github.repository_owner != 'manaflow-ai' && 'macos-26' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && 'blacksmith-6vcpu-macos-26' || vars.CI_PR_POOL_OWNED == '1' && contains(fromJSON('["pull_request","push","workflow_dispatch"]'), github.event_name) && (github.run_attempt <= 2 || github.triggering_actor != 'github-actions[bot]') && vars.CI_SIDE_LANE_RUNNER || vars.MACOS_RUNNER_26 || 'blacksmith-6vcpu-macos-26') }} |
There was a problem hiding this comment.
Canary skips Blacksmith overflow fallback
Medium Severity
The new swift-canary job is placed on a mini, but its runs-on and CMUX_NEXT_RUNNER still take macos-placement.outputs.runner on every attempt. Sibling Mac jobs now require run_attempt <= 3 or a non-bot actor before reusing that label, so a bot rescue at attempt 4 can leave the minis. A canary refusal can stay queued on a dead side label instead of moving to Blacksmith.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7a6cbb4. Configure here.
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
2 participants
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.
Summary
#18797's lint caught only a literal
NotificationCenter.default.post(. The off-main notice tests posted through a helper with.defaulthanded in instead. That form let the didBecomeKey post through, and it trapped the whole swift test run in ViewBridge (#18815).check-test-notification-centers.shnow also fails these forms in Tests:NotificationCenter.default,NSWorkspace.shared.notificationCenterorDistributedNotificationCenter.default()..defaultfor a center label (on:,center:ornotificationCenter:).NotificationCenterparameter whose default is the default center.Observer registration and removal on
.defaultare still fine, and so are other.defaultarguments such asURLSession(configuration: .default).The 11 existing sites are the off-main notice suites' helpers and call sites. Each is a deliberate off-main post on the real center, there to prove an app observer hops to main (crash-elimination P1b). Each now carries a reviewed
// global-notice-allow:that names that purpose and the known risk: AppKit's own observers of the same notice can trap, as in #18815. New tests must inject a center.Testing
bash scripts/cmux-next/tests/check-test-notification-centers.test.shat 4d51423 fails "must fail: a default center as a center argument"..defaultcenter argument, spelled-out default, workspace and distributed centers as arguments, both parameter-default spellings, an unrelated.default, observer add and remove, an injected center, and a reviewed exception.scripts/cmux-next/check-test-notification-centers.shon the tree reports 11 sites before the exceptions and ok after.verify-local.py --only swift-syntax --swift-changedpasses. The test changes are comment lines only.Changelog
none
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
The notification-center lint now also fails when a process-global center is handed to a poster, not only on a literal
.postcall. The off-main notice tests posted throughOffMainPost.send()with.defaultpassed in, which let thedidBecomeKeypost through and trapped the whole swift test run in ViewBridge (#18815).Lint rules
NotificationCenter.default,NSWorkspace.shared.notificationCenter,DistributedNotificationCenter.default()) as any labeled argument, a bare.defaultfor a center label (on:,center:,notificationCenter:), and aNotificationCenterparameter whose default is the default center..default, and unrelated.defaultarguments, still pass.The merge with feat-cmux-next deleted the off-main notice suites in base, so the
global-notice-allowexceptions are gone with them; the lint change stays and passes on base's tree.Written for commit 7a6cbb4. Summary will update on new commits.
Note
Medium Risk
Changes affect merge gates, feat-cmux-next bot pushes, and cmux-tui tree selection for PRs; incorrect routing or autofix could mask failures or push unintended commits.
Overview
This PR bundles several CI, tooling, and small product fixes. The headline change from the title is
check-test-notification-centers.sh: it now flags passing a process-global notification center into a poster (labeled args,on: .default, or default parameter values), not only direct.postcalls—addressing ViewBridge traps from off-main posts via helpers (#18815).cmux-next CI adds a Swift canary tier (
swift buildon changed CmuxNext production targets) as the PR gate for small package-only Swift diffs, with Release compile advisory on those PRs. PRs that don’t touchcmux-tui/can useCMUX_TUI_TREE_BASE_FALLBACK: tree jobs fetch the newest published tree (tree_fetch_key/CMUX_TUI_TREE_KEY) or skip instead of failing. Bot rescue attempt 4 drops side-lane placement and uses Blacksmith. Acmux-next-base-autofixworkflow (dry-run untilCMUX_NEXT_BASE_AUTOFIX=1) applies generated-file patches andcargo fmtonfeat-cmux-nextpushes viabase-autofix.sh.Rust formatting in PR CI:
cmux-tui-fmt-changed.pyrunscargo fmt --checkonly on touchedcmux-tuipackages (wired intocmux-tui-spec.ymlwith unit tests). Pre-commit formats staged webviews (vp fmt) and stagedcmux-tui.rsfiles (rustfmt), and skips formatters during merge commits.Gallery PR workflow shards render jobs (4 shards), adds narrow/normal pane widths, and merges shard results before compare. Manifest verification retries remote binary digest reads with cache-busting URLs. Nightly caps the release-notes step at 10 minutes so uploads aren’t cancelled.
HomeAppLinksbecomesURL.isChiefSubagentLink. Minor fixes:image-staleness.tsfollows workspace re-exports;pin-cmux-tui.shdeepens only the current branch; docs for tiers and UI reference sources.Reviewed by Cursor Bugbot for commit 7a6cbb4. Bugbot is set up for automated code reviews on this repo. Configure here.