Submodule guard: ask GitHub when a shallow clone hides ancestry

[lawrencecchen]

Summary

submodule-forward-only reported a forward Bonsplit bump as diverged on #15821. GitHub compare shows the old pin 4 commits behind the new one with nothing dropped. CI checks out submodules shallowly, so both commits were present but the history joining them was not: both merge-base --is-ancestor checks failed, and local_relation returned diverged without trying the GitHub compare.

In a shallow clone, local_relation now returns unknown unless an ancestry check succeeded, so the guard falls back to the GitHub compare. A full clone keeps the local diverged result.

Changelog

none

Testing

• python3 -m unittest tests.test_submodule_forward_only: the new shallow-clone test failed before the fix (commit 1) and passes after it. All 14 tests pass.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes submodule-forward-only wrongly reporting a forward Bonsplit bump as diverged in a shallow clone, where both pinned commits exist but the history joining them does not.

local_relation now returns unknown for a shallow clone unless an ancestry check succeeded, so the guard falls back to the GitHub compare. Full clones keep the local diverged result.

^{Written for commit d0c51b8. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Submodule comparisons in shallow repositories no longer incorrectly report commits as diverged when limited history prevents determining their relationship. Comparisons in repositories with complete history continue to identify diverged commits as before.

[github-actions]

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

[coderabbitai]

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 configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8d1c6e94-e6b1-4430-b8ab-f881df08c984

📥 Commits

Reviewing files that changed from the base of the PR and between 304d346 and d0c51b8.

📒 Files selected for processing (2)

• scripts/ci/submodule_forward_only.py
• tests/test_submodule_forward_only.py

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

---

📝 Walkthrough

Walkthrough

local_relation now returns None if the shallow-status check fails or the submodule repository is shallow. A new test checks this behavior for two commits in a shallow clone.

Changes

Shallow submodule relation handling

Layer / File(s)	Summary
Shallow repository check and test scripts/ci/submodule_forward_only.py, tests/test_submodule_forward_only.py	local_relation returns None when the shallow-status check fails or reports a shallow repository. A test verifies that two commits in a shallow clone are not classified as diverged.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: teamleaderleo

Merge Risk: ⚪ Minimal · up to d0c51

The change avoids false divergence in shallow clones, while unknown ancestry still fails the check. No merge-blocking issue remains.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to d0c51

The change corrects false divergence reports without weakening forward-only enforcement. Unresolved ancestry still fails the check, and the existing credential permissions remain unchanged.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed decision affects acceptance of changed submodule pins when local ancestry is inconclusive in a shallow or unverifiable repository. It expands reliance on the existing remote comparison authority, without changing the inspected caller's token permissions or enforcement rules.

Trust Boundaries and Controls

• observed — Repository-derived submodule URLs select the owner and repository for the existing GitHub Compare request. The shallow-state change does not alter URL parsing, credential attachment, response interpretation, or the explicit rollback control.

Resilience and Maintainability Implications

• observed — Caught HTTP, URL, I/O, or JSON-decoding failures return an indeterminate relation. Unrecognized comparison results also return indeterminate, and the caller fails the guard rather than accepting the pin.

🚥 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 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (24 passed)

Check name	Status	Explanation
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 pull request changes only scripts/ci/submodule_forward_only.py and its regression test. The diff adds shallow-repository detection for submodule ancestry and does not modify Cloud terminal…
Cmux Swift Actor Isolation	✅ Passed	PASS: The pull request changes only Python files: scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. It introduces no Swift production changes, so the Swift actor isola…
Cmux Swift Blocking Runtime	✅ Passed	The pull request changes only Python code and Python tests. The authoritative diff contains no production Swift changes, so it does not introduce or expand the specified Swift blocking or timing-based…
Cmux Browser Automation Off-Main	✅ Passed	The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. The diff adds shallow Git ancestry handling and a Python regression test. It does not c…
Cmux Expensive Synchronous Load	✅ Passed	PASS: The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. The diff contains no Swift or other production UI/actor code, and it adds no sync…
Cmux Cache Substitution Correctness	✅ Passed	PASS. The pull request changes only Python files: scripts/ci/submodule_forward_only.py and its Python test. The cache substitution check applies only to production Swift, TypeScript, and JavaScript …
Cmux No Hacky Sleeps	✅ Passed	PASS. The PR changes a Python CI/runtime script and a test, so the rule scope can apply. The production diff only adds a shallow-repository check and returns unknown before commit-existence classifica…
Cmux Algorithmic Complexity	✅ Passed	The production diff adds one shallow-repository check in local_relation. It does not add nested collection scans, per-target rescans, sorting, filtering, joins, or a slower algorithm over user-owned…
Cmux Swift Concurrency	✅ Passed	The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. The diff contains no Swift or Xcode files, so it does not introduce or expand any Swift…
Cmux Swift @Concurrent	✅ Passed	PASS: The pull request changes only Python files (scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py). It introduces no Swift changes, async Swift functions, `@concurren…
Cmux Swift Package Boundaries	✅ Passed	The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. It introduces no Swift production changes, so the Swift package boundary check is not a…
Cmux Swiftpm Lockfiles	✅ Passed	The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. It does not change a SwiftPM package, Xcode project package references, .gitignore, w…
Cmux Swift Logging	✅ Passed	PASS: The pull request changes only Python files. It adds no Swift production logging and does not add or modify print, debugPrint, dump, NSLog, or other Swift diagnostics.
Cmux User-Facing Error Privacy	✅ Passed	The patch changes only scripts/ci/submodule_forward_only.py and its test. The production addition is an internal CI decision path that returns unknown for shallow repositories; it adds no user-facin…
Cmux Full Internationalization	✅ Passed	PASS: The PR changes only scripts/ci/submodule_forward_only.py and its regression test. The production diff adds shallow-repository detection and developer comments; it adds no Swift text, catalog e…
Cmux Swiftui State Layout	✅ Passed	PASS: The pull request changes only Python files (scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py). It contains no SwiftUI changes, so the SwiftUI state-layout criter…
Cmux Architecture Rethink	✅ Passed	PASS. The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py, both Python files. It adds shallow-repository detection and a regression test. Th…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS: The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. It adds no Swift code and does not add or materially change any cmux-owned window…
Cmux Source Artifacts	✅ Passed	The PR changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. These are intentional hand-written source and test files. The diff adds no logs, caches, generate…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The pull request changes only scripts/ci/submodule_forward_only.py and tests/test_submodule_forward_only.py. It changes no Swift file under a production Sources/ path, so the custom check does n…
Title check	✅ Passed	The title clearly identifies the submodule guard change and the shallow-clone ancestry problem.
Description check	✅ Passed	The description explains the problem, resulting behavior, testing, and changelog status. It omits the template's Demo Video and Checklist sections, but the core review information is complete and the …

• 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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[lawrencecchen]

Superseded: main already defers to the GitHub compare in a shallow submodule clone (added in #15884 and #15924), so this change has nothing left to add.