Submodule guard: drop the duplicate shallow-clone check - #16168
Open
lawrencecchen wants to merge 2 commits into
Open
lawrencecchen wants to merge 2 commits into
lawrencecchen wants to merge 2 commits into
Conversation
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change removes a repeated shallow-repository check from ChangesSubmodule Relation Check
Linux Preflight Cancellation Diagnostic
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~5 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to The submodule check preserves its behavior, and the tests now match the gate’s cancellation, failure, and skipped diagnostics. No actionable merge risk is established. 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches 💡 3📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
#16150 made a cancelled linux-preflight report that the run has no verdict instead of 'linux preflight did not pass: cancelled'. The gate still fails, but the assertion kept the old text, so CI fast guards failed on main. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
lawrencecchen
enabled auto-merge (squash)
September 30, 2026 19:24
This was referenced Sep 30, 2026
austinywang
added a commit
that referenced
this pull request
Sep 30, 2026
The same change as #16168 (108bd10), carried here so this PR's Linux guards pass and its macOS jobs are not declined while main is red. #16150 made the ci.yml tests gate report a cancelled linux-preflight as "cancelled: linux-preflight"; the test kept the old text. Refs #15488 Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
austinywang
added a commit
that referenced
this pull request
Oct 1, 2026
… guard fetch history (#16094) * fix: pin bonsplit main with the deallocating-window hint fix main's app-host shards still abort with "objc: Cannot form weak reference to instance ... of class NSKVONotifying_NSWindow" (shard 3 of #15488 validation run 36732010954 on cmux14). manaflow-ai/bonsplit#261 (bb03f7d) fixes it, but main pins bd340ad, the hint-pill branch from #15821, which predates it. Pin bonsplit main's head, 7e5598e: it merges the hint-pill branch over bf5f051 (#268) and bb03f7d (#261), so main keeps #15821's bonsplit changes and gains the fix. The two app edits are #15942's adaptation to the performance changes that come with bf5f051: read pane tab ids through tabIds(inPane:), and correct the title-refresh comment now that bonsplit observes each tab item. Refs #15488 Co-authored-by: Leo Li <cheerleaderleo@outlook.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: say a title frame wakes only its tab's views With bonsplit observing each tab item, a title-only refresh no longer invalidates the whole tab bar subtree; the comment at the call site still said it did, contradicting the doc comment on refreshTabLabel. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(settings): add the missing try and capture that break main's compile #14868 merged 74c3a5f after its compile admission failed, so CmuxSettings, and with it the app, no longer builds on main: JSONConfigAtomicPublisher.swift:74: call can throw but is not marked with 'try' JSONConfigStore.swift:601: reference to property 'fileURL' in closure requires explicit use of 'self' to make capture semantics explicit The post-exchange rollback now uses `if try`, like the publisher's two other rollback call sites, so a failed rollback still reports sourceChangedRollbackFailed. The isTargetCurrent closure captures the store's nonisolated fileURL by value instead of the actor. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: expect the cancelled-run message from the tests gate The same change as #16168 (108bd10), carried here so this PR's Linux guards pass and its macOS jobs are not declined while main is red. #16150 made the ci.yml tests gate report a cancelled linux-preflight as "cancelled: linux-preflight"; the test kept the old text. Refs #15488 Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: let the submodule guard fetch history when GitHub can't answer The forward-only guard checks submodules out two commits deep. When an old pin sits deeper than that, it asks the GitHub compare API, which fails whenever the repository's shared Actions token is out of quota. The guard then reports "could not determine ancestry". It did so on every run of this PR (bf5f051 -> 7544622, three commits deep) and of #15942, although GitHub's compare says behind_by=10, ahead_by=0. As a last resort after the compare, the guard now fetches the missing history (commits and trees, no blobs) and decides locally. It never runs when the local check or GitHub already answered, so passing and rejected moves keep their current path. A shallow bonsplit clone at 7544622, as CI makes it, now resolves bf5f051 as forward. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): apply migrations the way production does everywhere #15423 added a CREATE INDEX CONCURRENTLY migration and taught the production migrator (migrate-planetscale.mjs) to run it outside a transaction. CI, web-validation and local databases still ran `drizzle-kit migrate`, which wraps every migration in one transaction, so main's web-db-migrations job fails with "CREATE INDEX CONCURRENTLY cannot run inside a transaction block", and `bun run db:migrate` fails for anyone with a fresh local database. The production migrator's loop moves unchanged into scripts/cloud-vm/apply-migrations.mjs, and a new scripts/db-migrate.mjs runs it against DIRECT_DATABASE_URL or DATABASE_URL. Every caller of `drizzle-kit migrate` now uses it: ci-web, web-validation, cloud-vm-guest-install, ios-streamed-validate, db-local.sh, and dev-local.sh through db-local.sh. CI now exercises the code path production runs. Checked on a scratch Postgres 14: all 93 migrations apply, a second run applies none, and cloud_vms_observed_destroy_cleanup_idx is valid. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): use the transaction's json helper in the outbox test main's web typecheck fails since #15423: tests/vm-workflows.test.ts(6219,37): error TS18047: 'sql' is possibly 'null'. The test narrows the file's `let sql` at its start, but TypeScript drops that narrowing inside the `sql.begin` callback. The insert there now uses the transaction's own `tx.json`, which is also the connection that runs the insert. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the French Actions discovery titles as invariant The same change as #16175 (ee38771), carried so this PR's static checks pass while main is red. #13232 added actions.discovery.menuTitle and actions.discovery.dialogTitle, whose French text is identical to the English, and the localization parity check fails on main. Refs #15488 Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): insert a real JSON null in the malformed cleanup-row test "Cloud VM database schema > rejects malformed transferred cleanup rows" (#15423) never ran on main, because main's migrations failed before the database behavior tests. With migrations fixed it fails: expect((insertError)?.code).toBe("23514") Expected: "23514" Received: "23502" Its first malformed value is `null`, and postgres.js binds `sql.json(null)` as SQL NULL. The NOT NULL column rejects that (23502) before the check constraint the test is about. The row under test is a JSON null document, so that case now inserts `'null'::jsonb`, and the check rejects it with 23514 like the other nine. Checked on a scratch Postgres 14 with postgres.js: sql.json(null) gives 23502, the JSON null gives 23514, all ten malformed values give 23514, and {modelPlane: true} and {homeVolume: "v"} are accepted. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(config): pass actionReferenceID on the setting-action trust path main doesn't compile since #13232 (ef75ca7) and #14868 (10e78b5) merged 13 minutes apart: Sources/CmuxConfig.swift:2816:51: error: missing argument for parameter 'actionReferenceID' in call #13232 added the required actionReferenceID field to ResolvedSurfaceTabBarButtonEntry. #14868 added a new return of that struct for a project button that shows a global setting action, without the field. That button still shows and runs the referenced action, like the ordinary resolved path below it, so it reports the same resolvedIdentifier. Actions & Launchers discovery then lists the action as placed on the tab bar. The argument shares a line to keep the file within its length budget. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(actions): name setting actions in the discovery summary The second compile error from #13232 and #14868 merging 13 minutes apart, hidden behind the first: Sources/AppDelegate+WorkspaceActionSave.swift:126:9: error: switch must be exhaustive #14868 added CmuxSurfaceTabBarButtonAction.setting, and #13232's Actions & Launchers summary switched over the enum without it. The summary's type token follows each action's cmux.json "type", so a setting preset shows "settingPreset" and any other setting change "setting". The switch is now one case per line, which keeps the file within its length budget. Every other exhaustive switch over the enum already handles .setting. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep Workspace+TitleOwnership.swift as main has it The title-frame comment tweak is cosmetic and was the only Swift change left in this PR. Without it the PR is web and CI only, so its checks don't wait on main's cmuxTests build. Refs #15488 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(web): pin the seats-follow-membership billing copy The billing panel's over-seat line is asserted here, and this test has been red on main since the dashboard SPA port: it already checks that no add-seats link is offered, and the port brought one back. Widen it to the copy the rule actually calls for, so both halves of the regression are covered. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Leo Li <cheerleaderleo@outlook.com> * fix(web): restore the seats-follow-membership copy the dashboard port dropped The Team subscription quantity follows the member count, so an over-seat line has nothing for an admin to act on: the reconciler updates Stripe on the next membership fact. That was settled in 06f4a7c, which reworded the line in all 20 locales, removed the add-seats link beside it, and dropped the members-page seat nudge. The dashboard SPA port rebuilt the billing panel from the pre-06f4a7c version at a new path, so git saw no conflict and the link came back, and the locale files went back to the soft-seat wording. `web/tests/ dashboard-billing-screen.test.tsx` has been red on main ever since, which fails the required `ci-status` on every web pull request. Restores the wording and drops the link. `seatNudge` and `seatNudgeAction` go too: the nudge they belonged to is gone from the members page and nothing reads them. `docs/team-settings-and-invites.md` already records the rule, and the stale "seats are soft" comment left hanging over an unrelated type in `team-members.tsx` is removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Leo Li <cheerleaderleo@outlook.com> * test(web): pin the new-team seat copy too The same merge-resolution path that reverted the billing panel's copy also reverted this line, and nothing asserted on it. Pin the sentence and the old wording's absence so a stale merge side fails the shard instead of shipping. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Leo Li <cheerleaderleo@outlook.com> * test(coderouter): close pinned proxy test connections Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> * fix(coderouter): handle pinned proxy body failures without hanging Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> * fix(ci): address follow-up review findings * merge: keep main's current bonsplit pin * fix(ci): harden locale and migration review follow-ups Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> * fix(ci): finish migration and locale follow-ups Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> * fix(web): preserve locale cookies during RSC navigation Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> * test(web): remove duplicate locale race case --------- Co-authored-by: Leo Li <cheerleaderleo@outlook.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com>
This branch has not been deployed
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
1 participant
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
local_relationinscripts/ci/submodule_forward_only.pychecked for a shallow clone twice, once from #15884 and once from #15924. The first check returns for every shallow clone, so the second one, inside thedivergedbranch, could never run. This removes the second copy. Behavior does not change.Also fixes
test_linux_failure_still_blocks_tests_after_macos_succeeds, which failed on main: #16150 changed the message for a cancelled linux-preflight, and the assertion kept the old text. The gate still fails for every outcome.Changelog
none
Testing
python3 -m unittest tests.test_submodule_forward_only: 15 tests pass, includingtest_shallow_clone_defers_forward_move_to_github.🤖 Generated with Claude Code
Summary by cubic
Removes the duplicate shallow-clone check in
local_relationinscripts/ci/submodule_forward_only.pyand fixes the tests-gate test that broke after #16150 changed the message for a cancelled linux-preflight.divergedbranch could never run; removing it leaves behavior unchanged.test_linux_failure_still_blocks_tests_after_macos_succeedsnow expects the cancelled-run message, and the gate still fails for every outcome.Testing
test_shallow_clone_defers_forward_move_to_github.Written for commit 108bd10. Summary will update on new commits.
Summary by CodeRabbit