Skip to content

LOCAL: gate only the fork's own lines on an upstream sync PR - #23

Merged
kiwidream merged 2 commits into
swaplabs/mainfrom
fork-ci/sync-pr-quality-base
Sep 29, 2026
Merged

kiwidream merged 2 commits into
swaplabs/mainfrom
fork-ci/sync-pr-quality-base

Conversation

@kiwidream

@kiwidream kiwidream commented Sep 29, 2026 •

Copy link
Copy Markdown
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​15 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​15
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​36 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​34

ELI5

A sync PR brings hundreds of upstream commits into the fork at once. The "changed code" quality gate treated every one of those upstream lines as ours, so it failed on upstream's own code and kept every sync PR red. This makes the gate on sync PRs look only at the lines the fork itself adds.

What Changed

  • Before: on the 2026-09-29 sync PR (sync: upstream 31012aeb09 into swaplabs/main #22), static analysis checked 1,052 changed files. It failed on a type assertion in worktree-activation-default-agent-chat.test.ts:63, which came from upstream fix(native-chat): an empty workspace opens the default agent as a chat when chat is the default view stablyai/orca#23693 and had already passed upstream's CI. The fork can't fix that without diverging from upstream.
  • After:
    • In .github/workflows/pr.yml, the static analysis job has a new step, run only for this repository's own sync PRs (head main or sync/upstream-*, not from a fork). It fetches the mirror main and sets ORCA_SYNC_PR_DIFF_BASE to where the PR meets it, which is the upstream commit being merged.
    • config/scripts/git-pull-request-diff-base.mjs prefers that base over the merge commit's first parent, so the changed-code and React Doctor gates see only the fork's lines: its LOCAL: changes and the conflict resolutions.
    • Every other PR, and every other job, behaves exactly as before. If the lookup fails, the step warns and the gate checks every line, as today.

Why

  • The alternative was skipping the gate on sync PRs, but then conflict resolutions and the fork's own code would go unchecked. Scoping keeps exactly those checked.
  • Why an env var the resolver reads: on pull_request runs the resolver ignores the base argument and uses the merge commit's first parent. That is correct for normal PRs, and it's why passing a different argument can't fix this.

Linked Issue

None. Fork-only; found on #22.

Visual Proof

N/A. CI only.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • git-pull-request-diff-base.test.mjs gains a case where the sync base wins over the first parent, and an empty value leaves today's behaviour.

  • vitest on the resolver plus every test that pins how pr.yml uses it: 6 files, 129 passed. That includes check-root-directory-entries, which needs GIT_CONFIG_GLOBAL=/dev/null on hosts that sign commits globally; it fails identically on swaplabs/main without that.

  • oxlint and oxfmt are clean. actionlint 1.7.7 reports only syntax newer than itself (background/wait steps, ubuntu-slim), all in upstream's existing file and none in the new step.

Not verified until it runs: the step on a real sync PR. #22 will show it once rerun on a head that includes this change.

AI Disclosure

Written by Claude Opus 5.5 (Claude Code).

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

A sync PR merges all of upstream into swaplabs/main, so the changed-code and
React Doctor gates flagged upstream's own code (1052 files on the 2026-09-29
sync, for one upstream type assertion). For this repository's sync PRs (head
`main` or `sync/upstream-*`), the static analysis job now sets
ORCA_SYNC_PR_DIFF_BASE to where the PR meets the mirror `main`, and the diff-base
resolver prefers it over the merge commit's first parent. Every other PR, and
every other job, is unchanged; a failed lookup warns and gates every line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tenki-reviewer

tenki-reviewer Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review complete. 🟡 1 medium

💬 Inline comments (1)

  • 🟡 Sync-base override fires on any PR from main — pr.yml:225

Reviewed commit: 62e1417

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T14:17:23.100664Z 13b68e4 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 62e1417475

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/pr.yml Outdated

@tenki-reviewer tenki-reviewer Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Expand to see previous review

This PR adds a ORCA_SYNC_PR_DIFF_BASE override so sync-PR diff gating can use a custom base, wired through a new CI step in pr.yml.

Comment thread .github/workflows/pr.yml
…PRs into swaplabs/main

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tenki-reviewer

tenki-reviewer Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review complete. No blocking issues — approved ✅; 1 nitpick below.

🧹 Nitpicks (1) — 🟢 1 low
  • 🟢 Sync-PR merge base fails beyond shallow depth (pr.yml:168) — The checkout comment says the gates diff against HEAD^1 so no merge base is computed (.github/workflows/pr.yml:168), but the new sync-pr-diff-base step in the same job does compute git merge-base HEAD FETCH_HEAD against a checkout and fetch both capped at --depth=50.

Adds a sync-PR diff-base override to the CI changed-line gates via an ORCA_SYNC_PR_DIFF_BASE env var computed with git merge-base, plus tests.

Reviewed commit: 13b68e4

@kiwidream
kiwidream merged commit d91ccbb into swaplabs/main Sep 29, 2026
36 checks passed
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.

1 participant