Skip to content

ci(fork): gate Fork CI jobs per package and add a review loop - #293

Merged
Yarden-zamir merged 2 commits into
mainfrom
ci/fork-ci-upstream-gates
Oct 22, 2026
Merged

Yarden-zamir merged 2 commits into
mainfrom
ci/fork-ci-upstream-gates

Conversation

@Yarden-zamir

Copy link
Copy Markdown
Owner

🤖

Problem

Fork CI runs every heavy job for any code change, so a web-only change still runs the 8 server shards. It also calls setup-vp directly, so an npm outage fails its jobs. Upstream fixed both after the fork wrote its own CI: pingdotgg/t3code#19587 "ci: skip jobs a pull request cannot affect" and pingdotgg/t3code#23582 "ci: setup survives npm registry outages". Fork pull requests also had no fixed review step.

Change

  • The changes job feeds the fork's file list (last green push run, or the PR base) to upstream's .github/scripts/ci-changes.cjs. A PR runs only the jobs its packages need. A code push to main still runs every job. Renamed files count with their old path. Markdown anywhere counts as docs, as before.
  • The Machine map job is gone. The Test job now runs the machine map tests when it changes.
  • Every job installs through upstream's .github/actions/setup-ci-deps.
  • New review-loop skill (.agents/skills/review-loop): a second agent reviews a change in rounds before the PR. The PR template gets a "Review" section, and docs/fork-agents.md gets the rule.

Verification

  • vp run fork:check passed. actionlint and vp fmt --check pass on the workflow.
  • Ran ci-changes.cjs locally on sample lists: docs only → lint only; apps/machine-map → typecheck + machine map tests; apps/web → build, web and scripts tests, no server shards; .github/ → everything.
  • Ran the Test step's filter logic locally for both groups.
  • Not checked: a real Fork CI run. This PR is the first one.

Review

The new review loop with a reviewer from another provider: 2 rounds, approved. Round 1 found 3 issues, all fixed: renamed files lost their old path, nested Markdown ran the code jobs, and review ids could repeat across loops.

- Fork CI runs upstream's .github/scripts/ci-changes.cjs. A pull request runs
  only the jobs its packages need. A code push to main still runs every job.
  The Machine map job goes away: the Test job now covers a machine map change.
- Every job installs through upstream's .github/actions/setup-ci-deps. A cache
  hit makes no npm request (pingdotgg#23582, "ci: setup survives npm
  registry outages").
- New review-loop skill: a second agent reviews a change in rounds before the
  pull request. The pull request template gets a "Review" section.
@Yarden-zamir

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 671271d and 870fd1f.

📒 Files selected for processing: 6
  • .agents/skills/review-loop/SKILL.md
  • .github/pull_request_template.md
  • .github/workflows/fork-ci.yml
  • docs/fork-agents.md
  • docs/fork/operations.md
  • scripts/fork-check.ts

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

📝 Walkthrough

The pull request adds an agent review skill and updates its guidance and reporting template. It also changes fork CI to classify changed files, select jobs and affected packages, and include machine-map changes in repository checks.

Changes: Agent review process

Layer / File(s) Summary
Review preparation and reviewer selection
.agents/skills/review-loop/SKILL.md
The new skill defines change preparation, check execution, and selection of an available independent reviewer.
Review rounds and reporting
.agents/skills/review-loop/SKILL.md, .github/pull_request_template.md, docs/fork-agents.md
The skill defines isolated asynchronous review rounds, finding handling, and stopping rules. Agent guidance and the pull request template require recording the review method, rounds, verdict, and resulting changes.

Fork CI job selection

Layer / File(s) Summary
Change classification and outputs
.github/workflows/fork-ci.yml
The workflow compares commits based on the event and uses ci-changes.cjs outputs to select jobs and affected packages. It falls back to selecting all jobs in specified cases, including manual runs and unavailable comparisons.
Selected job execution and checks
.github/workflows/fork-ci.yml, scripts/fork-check.ts, docs/fork/operations.md
Jobs use classifier outputs and affected-package lists, and use the local setup-ci-deps action. The fork-check script includes machine-map changes in code checks. The operations documentation describes the updated selection behavior.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Event as GitHub event
  participant Workflow as fork-ci.yml
  participant Classifier as ci-changes.cjs
  participant Jobs as Selected CI jobs
  Event->>Workflow: Start workflow
  Workflow->>Classifier: Compare event-specific commits and classify changed paths
  Classifier->>Workflow: Return job outputs and affected-package lists
  Workflow->>Jobs: Run jobs selected by classifier outputs
Loading
Merge Risk: ⚪ Minimal · up to 870fd

This PR adds a review-loop skill and moves fork CI to package-aware job selection. No concrete merge-blocking defect was found. The CI behavior has not been exercised in a real Fork CI run, which is normal post-merge validation.

Architecture risk: 🔵 Low · up to 870fd

The change affects 2 systems.

Changed systems: scripts, docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — scripts (service) was modified; 1 changed file maps to changed impact.
  • observed — docs (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in docs/fork-agents.md: Adds a rule requiring a review-loop review and completion of the pull request template’s “Review” section before opening a pull request.
  • observed — Modified behavior in docs/fork/operations.md: Fork CI replaces its documentation/Markdown/machine-map path rules with upstream ci-changes.cjs package-and-dependent mapping: pull requests run jobs needed for changed packages, docs/ or Markdown-only changes run only Lint, and code pushes to main and manual runs run every job. The previous apps/machine-map-specific Machine map job trigger is removed. Jobs now use upstream’s setup-ci-deps action; cache hits make no npm request.
  • observed — Modified behavior in scripts/fork-check.ts: codeChanged now includes all changed files that are not docs or Markdown; changes under apps/machine-map/ no longer receive an exclusion.
  • observed — Modified behavior in scripts/fork-check.ts: The knip skip condition now describes only docs and Markdown changes; apps/machine-map/-only changes instead take the knip:check path.
Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the problem, changes, verification steps, limitations, and review results. It omits the required Scope and approval section and does not end with the model and harness used by… Add a Scope and approval section with the applicable issue, discussion, or maintainer approval and explain why the scope is approved. End the description with the model and harness used by the agent.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the two primary changes: package-aware Fork CI gating and the review loop.
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.
Full details: Description check

Explanation

The description explains the problem, changes, verification steps, limitations, and review results. It omits the required Scope and approval section and does not end with the model and harness used by the agent, as required by the template.

Resolution

Add a Scope and approval section with the applicable issue, discussion, or maintainer approval and explain why the scope is approved. End the description with the model and harness used by the agent.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Stopped

Pull request is closed.

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

@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Autofix was enabled. Check current status in the Coding task.

@Yarden-zamir
Yarden-zamir marked this pull request as ready for review October 22, 2026 21:48
@Yarden-zamir
Yarden-zamir merged commit d2938ec into main Oct 22, 2026
19 checks passed
@Yarden-zamir
Yarden-zamir deleted the ci/fork-ci-upstream-gates branch October 22, 2026 21:48
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