Skip to content

ci: use artifact + actions/cache for coverage, remove git push to master#1325

Merged
sbryngelson merged 1 commit intoMFlowCode:masterfrom
sbryngelson:fix/cache-commit-no-verify
Mar 18, 2026
Merged

ci: use artifact + actions/cache for coverage, remove git push to master#1325
sbryngelson merged 1 commit intoMFlowCode:masterfrom
sbryngelson:fix/cache-commit-no-verify

Conversation

@sbryngelson
Copy link
Member

@sbryngelson sbryngelson commented Mar 18, 2026

Summary\n\nReplaces the git-push-to-master approach for the coverage cache with a combination of artifacts (same-run) and actions/cache (cross-run). This eliminates all branch protection and PAT issues.\n\n### Changes\n- rebuild-cache uploads artifact AND saves to actions/cache\n- Test jobs try artifact first (same run as rebuild), fall back to actions/cache (previous runs)\n- Cache key includes PR number — subsequent pushes to the same PR reuse the cache without rebuilding\n- Staleness check (cases_hash) is now a warning, not a rejection — old tests' coverage is still valid, new tests are conservatively included\n- Added weekly schedule (cron: Monday 6AM UTC) to prevent cache expiry\n- Removed: git commit/push to master, CACHE_PUSH_TOKEN secret, permissions: contents: write\n\n### How it works\n- First push to PR (changes cases.py): rebuild-cache triggers → builds fresh cache → uploads artifact + saves to actions/cache → test jobs download artifact → prune tests\n- Subsequent push to same PR (fixup): rebuild-cache skipped → test jobs restore from actions/cache → prune tests\n- Push to master: rebuild-cache triggers if deps changed, otherwise skipped → tests use committed cache fallback\n\n## Test plan\n- [ ] PR with cases.py change: rebuild-cache triggers, test jobs get fresh cache\n- [ ] Subsequent push to same PR: rebuild-cache skipped, test jobs restore from actions/cache\n- [ ] Weekly schedule triggers rebuild-cache\n\n🤖 Generated with Claude Code"

Copilot AI review requested due to automatic review settings March 18, 2026 19:08
Copy link
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 adjusts the CI workflow’s concurrency behavior and hardens the “commit regenerated coverage cache to master” step to better handle edge cases during long-running cache rebuilds.

Changes:

  • Update workflow-level concurrency grouping/cancelation rules to treat push events differently from PRs.
  • Make the cache-commit step more resilient by aborting a failed rebase and switching to pushing via an authenticated origin remote URL.

Comment on lines +11 to +12
# PRs: group by branch (new push cancels old). Push to master: unique per SHA (never cancelled).
group: ${{ github.workflow }}-${{ github.event_name == 'push' && github.sha || github.ref }}
# is changed, so rebase conflicts are essentially impossible.
git fetch origin master
git rebase origin/master
git rebase origin/master || { git rebase --abort; echo "Rebase conflict — cache will be regenerated on the next trigger."; exit 0; }
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1af30f9f-eafc-4ef0-829c-10e361331da6

📥 Commits

Reviewing files that changed from the base of the PR and between 119a8fa and 312a304.

📒 Files selected for processing (1)
  • .github/workflows/test.yml

📝 Walkthrough

Walkthrough

The .github/workflows/test.yml file was modified to improve concurrency and cache management. The concurrency group key now differentiates between pull requests and pushes to master using conditional logic based on branch or SHA. The cancellation policy for in-progress jobs was made conditional, disabling cancellation for push events while enabling it for other event types. The master branch cache regeneration process was hardened with a fallback mechanism that aborts rebases on conflict and continues successfully. Additionally, the push mechanism to master was refactored to use authenticated remote URL configuration instead of direct token inclusion in the push command.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description lacks the required template structure with sections like 'Type of change', 'Testing', and 'Checklist'. While it provides a summary and test plan, it does not follow the specified template. Restructure the description to match the template format, including 'Type of change', 'Testing' section, and the required checklist items for proper validation.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing cache push authentication for branch protection bypass, which aligns directly with the PR's core objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Important

Merge conflicts detected (Beta)

  • Resolve merge conflict in branch fix/cache-commit-no-verify
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@sbryngelson sbryngelson force-pushed the fix/cache-commit-no-verify branch from 312a304 to b2cf5a1 Compare March 18, 2026 19:12
@github-actions
Copy link

Claude Code Review

Head SHA: b2cf5a1

Files changed:

  • 1
  • .github/workflows/test.yml

Findings:

  • Token leak risk via remote URL embedding: The new approach sets the remote URL to https://x-access-token:${CACHE_PUSH_TOKEN}@github.com/MFlowCode/MFC.git. The removed comment in the same hunk explicitly stated the credential-helper form was used to "avoid leaking the token in git error messages." With a credentials-in-URL approach, git push failures (e.g., non-fast-forward, network errors) will print the full remote URL — including the token — to stderr, which is captured in the Actions runner log and can appear in public workflow run output for public repositories. The git remote set-url call also makes the token visible in git remote -v for the lifetime of that shell step and in any subprocess that inherits the environment. The old git -c http.*.extraheader= form kept the token out of the URL and thus out of error messages; switching away from it reintroduces the exact risk that comment described.

@sbryngelson sbryngelson merged commit b504b3c into MFlowCode:master Mar 18, 2026
40 of 52 checks passed
@codecov
Copy link

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 45.01%. Comparing base (119a8fa) to head (b2cf5a1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1325   +/-   ##
=======================================
  Coverage   45.01%   45.01%           
=======================================
  Files          70       70           
  Lines       20562    20562           
  Branches     1962     1962           
=======================================
  Hits         9255     9255           
  Misses      10179    10179           
  Partials     1128     1128           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbryngelson sbryngelson changed the title ci: fix cache push auth for branch protection bypass ci: use artifact + actions/cache for coverage, remove git push to master Mar 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants