Skip to content

Handle submodule entries in push_signed_commits by falling back to git push#26298

Merged
pelikhan merged 3 commits intomainfrom
copilot/fix-submodule-updates-handling
Apr 14, 2026
Merged

Handle submodule entries in push_signed_commits by falling back to git push#26298
pelikhan merged 3 commits intomainfrom
copilot/fix-submodule-updates-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

Submodule entries (gitlink mode 160000) appeared as regular diff entries but cannot be pushed via the GraphQL createCommitOnBranch mutation — attempting to do so would silently produce corrupt commits.

Changes

actions/setup/js/push_signed_commits.cjs

  • Extract srcMode alongside existing dstMode from git diff-tree --raw output (needed to detect submodule deletions, where dstMode is 000000)
  • Detect mode 160000 across all four diff status branches (Delete, Rename, Copy, Add/Modify) and throw to trigger the existing git push fallback:
// Delete: submodule removal
if (srcMode === "160000") { ... throw }

// Rename: gitlink on either side
if (srcMode === "160000" || dstMode === "160000") { ... throw }

// Copy / Add / Modify: new gitlink
if (dstMode === "160000") { ... throw }

actions/setup/js/push_signed_commits.test.cjs

  • Adds an integration test that creates a real gitlink entry via git update-index --add --cacheinfo 160000,..., verifies GraphQL is never called, and confirms the commit lands on the remote via the git push fallback.

…igned_commits.cjs

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/308925ea-5090-4ed0-9c34-5bf1dd1f6e0a

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix push_signed_commits.cjs to handle submodule updates gracefully Handle submodule entries in push_signed_commits by falling back to git push Apr 14, 2026
Copilot AI requested a review from pelikhan April 14, 2026 21:42
@pelikhan pelikhan marked this pull request as ready for review April 14, 2026 21:44
Copilot AI review requested due to automatic review settings April 14, 2026 21:44
@github-actions github-actions Bot mentioned this pull request Apr 14, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents corrupt signed commits when a commit includes submodule (gitlink, mode 160000) entries by detecting gitlinks during the diff-tree pre-scan and forcing the existing git push fallback path instead of attempting the GraphQL createCommitOnBranch mutation.

Changes:

  • Parse srcMode from git diff-tree --raw so submodule deletions (where dstMode is 000000) can be detected.
  • Detect gitlink mode 160000 across delete/rename/copy/add-modify cases and throw to trigger the git push fallback.
  • Add an integration test that creates a gitlink entry via git update-index --cacheinfo 160000,... and asserts GraphQL is not called and fallback push succeeds.
Show a summary per file
File Description
actions/setup/js/push_signed_commits.cjs Adds srcMode parsing and gitlink (160000) detection to force git push fallback for submodule changes.
actions/setup/js/push_signed_commits.test.cjs Adds an integration test covering submodule/gitlink detection and verifying GraphQL is bypassed.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment on lines 167 to +172
if (status === "D") {
// mode 160000 = gitlink (submodule); GitHub GraphQL createCommitOnBranch does not support submodules
if (srcMode === "160000") {
core.warning(`pushSignedCommits: submodule change detected in ${filePath}, falling back to git push`);
throw new Error("submodule change detected");
}
}
deletions.push({ path: filePath });
if (srcMode === "160000" || dstMode === "160000") {
core.warning(`pushSignedCommits: submodule change detected in ${renamedPath}, falling back to git push`);
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

Excellent test quality

Metric Value
New/modified tests analyzed 1
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (100%)
Duplicate test clusters 0
Test inflation detected No (37 test lines / 19 production lines ≈ 1.95:1)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
should fall back to git push and warn when commit contains a submodule entry actions/setup/js/push_signed_commits.test.cjs:721 ✅ Design None

Test Quality Highlights

should fall back to git push and warn when commit contains a submodule entry (actions/setup/js/push_signed_commits.test.cjs:721)

Classification: Design test (behavioral contract)

What design invariant does this test enforce? When pushSignedCommits encounters a git submodule entry (mode 160000), it must never invoke the GitHub GraphQL API (which doesn't support submodules) and must instead fall back to git push, emitting the appropriate warning messages.

What would break if deleted? A regression that silently tried to push a submodule via GraphQL (and either failed or corrupted the commit) would go undetected. The fallback path is exactly the new behavior introduced in this PR.

Strengths:

  • Uses real git operations (git update-index --cacheinfo 160000,...) — no shortcuts
  • Mocks are scoped correctly to external dependencies only: makeMockGithubClient() (GitHub GraphQL API) and mockCore (GitHub Actions runtime)
  • Assertions cover three complementary layers:
    1. API-level: expect(githubClient.graphql).not.toHaveBeenCalled() — confirms the fallback path was taken
    2. User-visible output: warning messages contain the expected submodule path and the "falling back" phrase
    3. End-to-end state: verifies the commit is actually present on the remote after the git-push fallback, making this a true integration assertion

Language Support

Tests analyzed:

  • 🟨 JavaScript (*.test.cjs): 1 test (vitest)

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). The single new test exercises the full behavioral contract of the submodule fallback path with real git operations and end-to-end state verification.


📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

🧪 Test quality analysis by Test Quality Sentinel · ● 532.9K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The new test for submodule fallback is a well-structured behavioral contract test using real git operations and end-to-end state verification.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review comments

…ing with both paths

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/407bea3c-da48-4b10-814f-66cc383482c2

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 14, 2026

@copilot review comments

Addressed both reviewer comments in commit 0de51dd:

  1. Updated the pre-scan comment to list 160000 (submodules/gitlinks) as an unsupported mode alongside symlinks and executable bits.
  2. Improved the rename-branch warning to include both source and destination paths: submodule change detected in ${filePath} -> ${renamedPath}.

@pelikhan pelikhan merged commit f109ff0 into main Apr 14, 2026
@pelikhan pelikhan deleted the copilot/fix-submodule-updates-handling branch April 14, 2026 22:21
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.

[plan] Handle submodule updates gracefully in push_signed_commits.cjs by falling back to git push

3 participants