fix: [#781] marker-guard non-strict on bridge-merge push to main#782
fix: [#781] marker-guard non-strict on bridge-merge push to main#782anandgupta42 merged 1 commit intomainfrom
Conversation
The PR-side marker-guard already runs in non-strict mode for `upstream/merge-*` head_refs, but after a squash-merge of that PR, the push event to main has no head_ref / second-parent / branch-name signal — it falls back to `--strict` and fails on the hundreds of upstream files the bridge merge brings in (98 false-positive warnings on the v1.4.0 bridge merge, run 25264283463). Detect bridge / upstream-merge commits in the pushed range by subject (`grep -qiE '(bridge|merge) upstream'`) and downgrade strict->non-strict for those pushes. Warnings still surface in job output; the PR-side review already gated marker integrity for the change. Validated locally: - regex matches commit 3cc3d4c (v1.4.0 bridge merge) - analyze.ts --markers --base <prev-main> exits 0 (97 warnings shown) - --branding and --require-markers --strict both pass on main HEAD - marker-parser tests: 22/22 passing - ci.yml is valid YAML Closes #781
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughThe CI workflow now detects bridge/merge-upstream commits in the pushed range by matching commit subjects against a pattern. When such commits are found, the marker check runs in non-strict mode; otherwise, strict mode is enforced. This prevents false positives when squash-merging upstream bridge PRs to main. ChangesMarker Guard Bridge Detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 448-451: Update the git-log subject matching used in the
if-condition that runs bun run script/upstream/analyze.ts: replace the current
grep regex '(bridge|merge) upstream' with a broader pattern that accepts either
a space or a hyphen and also matches hyphenated forms like bridge-merge; for
example change the grep invocation in the if block to use a regex such as
'(bridge|merge)(?:[ -]upstream|[-]merge)?' so commits like "bridge-merge",
"bridge-upstream" and "merge upstream" are all detected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 48ea5b70-88db-46f6-a39a-e552d6d4568a
📒 Files selected for processing (1)
.github/workflows/ci.yml
| if git log --format=%s "${{ github.event.before }}..${{ github.sha }}" \ | ||
| | grep -qiE '(bridge|merge) upstream'; then | ||
| echo "Bridge/upstream-merge commit detected in push range — running marker check in non-strict mode" | ||
| bun run script/upstream/analyze.ts --markers --base "${{ github.event.before }}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
CURRENT_RE='(bridge|merge) upstream'
PROPOSED_RE='(^|[[:space:][:punct:]])(upstream/merge-[^[:space:]]*|merge[[:space:]/_-]+upstream|bridge[[:space:]/_-]+(merge|upstream))([[:space:][:punct:]]|$)'
echo "Recent commit subjects:"
git log -n 200 --format='%s' | sed 's/^/ - /'
echo
echo "Matches with CURRENT_RE:"
git log -n 200 --format='%s' | grep -niE "$CURRENT_RE" || true
echo
echo "Matches with PROPOSED_RE:"
git log -n 200 --format='%s' | grep -niE "$PROPOSED_RE" || trueRepository: AltimateAI/altimate-code
Length of output: 281
🏁 Script executed:
cd /tmp && git clone --quiet https://github.com/AltimateAI/altimate-code.git && cd altimate-code
sed -n '448,451p' .github/workflows/ci.ymlRepository: AltimateAI/altimate-code
Length of output: 397
🏁 Script executed:
cd /tmp/altimate-code && rg -n "(bridge|merge) upstream" .github/workflows/ci.ymlRepository: AltimateAI/altimate-code
Length of output: 50
🏁 Script executed:
cd /tmp/altimate-code && rg -n "Bridge/upstream-merge\|git log.*grep.*qiE" .github/workflows/ci.ymlRepository: AltimateAI/altimate-code
Length of output: 50
🏁 Script executed:
cd /tmp/altimate-code && wc -l .github/workflows/ci.yml && rg -n "git log" .github/workflows/ci.ymlRepository: AltimateAI/altimate-code
Length of output: 97
🏁 Script executed:
cd /tmp/altimate-code && sed -n '440,460p' .github/workflows/ci.ymlRepository: AltimateAI/altimate-code
Length of output: 1431
Broaden bridge/upstream subject matching to prevent false negatives on pushed commits.
Line 449's regex (bridge|merge) upstream only matches space-separated patterns. The repository's actual commit subjects use hyphenated variants like bridge-merge (confirmed in recent commits), which won't match. This causes strict mode to run on bridge pushes, reintroducing the original CI failure.
The proposed regex correctly matches these variants and should be applied before merge.
Suggested patch
- if git log --format=%s "${{ github.event.before }}..${{ github.sha }}" \
- | grep -qiE '(bridge|merge) upstream'; then
+ BRIDGE_UPSTREAM_RE='(^|[[:space:][:punct:]])(upstream/merge-[^[:space:]]*|merge[[:space:]/_-]+upstream|bridge[[:space:]/_-]+(merge|upstream))([[:space:][:punct:]]|$)'
+ if git log --format=%s "${{ github.event.before }}..${{ github.sha }}" \
+ | grep -qiE "$BRIDGE_UPSTREAM_RE"; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 448 - 451, Update the git-log subject
matching used in the if-condition that runs bun run script/upstream/analyze.ts:
replace the current grep regex '(bridge|merge) upstream' with a broader pattern
that accepts either a space or a hyphen and also matches hyphenated forms like
bridge-merge; for example change the grep invocation in the if block to use a
regex such as '(bridge|merge)(?:[ -]upstream|[-]merge)?' so commits like
"bridge-merge", "bridge-upstream" and "merge upstream" are all detected.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
What does this PR do?
Fixes the
Marker Guardjob failing on push-to-mainimmediately after a bridge / upstream-merge PR is squash-merged.The PR-side path of the workflow already runs
analyze.ts --markersin non-strict mode forupstream/merge-*head_refs. But once such a PR is squash-merged, the push event ontomainhas no head_ref / second-parent / branch-name signal, so the workflow falls back to:For a bridge merge that overlays hundreds of upstream files, the diff legitimately contains piles of upstream code without
altimate_changeblocks, and strict mode fails. That's exactly what happened on the v1.4.0 bridge merge:⚠ Found 98 file(s) with unmarked custom code→--strict mode — failing CIThis patch detects bridge / upstream-merge commits in the pushed range by commit subject (
grep -qiE '(bridge|merge) upstream') and downgrades strict→non-strict for those pushes. Warnings are still printed in the job output; the PR-side review already gated marker integrity for the change before it landed.The
Branding leak auditandRequire-markers regression backstopsteps remain unconditional — the only thing being relaxed is the strict diff-based marker check on push, and only when the push contains a bridge/upstream-merge commit.Type of change
Issue for this PR
Closes #781
How did you verify your code works?
Re-ran the marker tooling locally against the exact range from the failing run (
941c8cae21..3cc3d4cb68onorigin/main):git log --format=%s 941c8cae21..3cc3d4cb68 origin/main | grep -qiE '(bridge|merge) upstream'→ matches the squash-merge subjectfeat: bridge upstream v1.4.0 across history rewrite + 3 backports + adversarial test suite (#757). Without--strict, the workflow takes the new bridge-merge path.bun run script/upstream/analyze.ts --markers --base 941c8cae21f124f509da6cc0ca0313a06473e50cprints 97 warnings and exits 0 (vs strict mode exiting 1).mainHEAD:bun run script/upstream/analyze.ts --branding→ exit 0, "All blocks properly closed".bun run script/upstream/analyze.ts --require-markers --strict→ exit 0, "All behavior-patched files have markers" (38/38).cd script/upstream && bun test→ 22 pass, 0 fail.python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))"→ OK.bridge upstream/merge upstream. Dependabot, release, and ordinary feature commits do not contain these phrases, so non-bridge pushes still take the strict path.Checklist
Summary by cubic
Make the
Marker Guardjob run in non-strict mode on pushes tomainthat include a bridge/upstream-merge commit, preventing false failures after squash merges. Normal pushes remain strict.analyze.ts --markers --base <before>without--strict.--brandingand--require-markers --strictunchanged; warnings still surface in job output.Written for commit 94e7a65. Summary will update on new commits.
Summary by CodeRabbit