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

.agents/skills/review-loop/SKILL.md

@@ -0,0 +1,60 @@
+---
+name: review-loop
+description: Get an independent review of your own change from a second agent, fix what it finds, and repeat until it approves. Use before you open or update a fork pull request, or when the owner asks for a review loop. Not for reviews of pull requests by other people.
+---
+
+# Review loop
+
+The agent that wrote a change does not review it alone. A second agent reviews it in rounds.
+
+## 1. Prepare
+
+1. Commit the change on its branch. A WIP commit is fine.
+2. Record `BASE=$(git merge-base origin/main HEAD)`.
+3. Write the intent and the out-of-scope items in one or two sentences.
+4. Run the checks that cover the change. Record the results.
+
+## 2. Choose the reviewer
+
+Call `orchestrator_capabilities`. Take the first option that runs:
+
+1. A strong model of a different provider than the one that wrote the change, through `delegate_task`.
+2. A fresh subagent of your own model, through `delegate_task`.
+
+Do not hardcode a model id. If no reviewer runs, stop and report it. A self-review is not a review.
+
+## 3. Run a round
+
+Each round is a new `delegate_task` call with `mode: "async"`, `isolation: "worktree"` and a new `clientRequestId` (`review-<branch>-<first 8 characters of the head SHA when the loop starts>-r<N>`). Reuse an id only to retry the same round. Do not edit the change while a round runs. A timeout or an error is not approval.
+
+Brief for round 1:
+
+```
+Review only. Do not edit files, commit, push, comment on GitHub, or start subagents.
+Repository: <path>. Base: <BASE>. Head: <sha>. Intent: <intent>. Out of scope: <items>.
+Checks: <results>. Read `git diff <BASE>...<sha>`, the surrounding code and AGENTS.md.
+Find bugs, broken callers, mismatches with the intent, and code that the intent does not need.
+Label each finding: blocker (the intended case fails, or a security hole), should (a real bug
+that callers hit, or a simpler way to write the same change), nit (the rest).
+Start only your last message with FINAL REPORT. Reply:
+VERDICT: APPROVE or CHANGES
+R1 [blocker|should|nit] <file>:<line> <problem>. Fix: <suggestion>
+```
+
+From round 2, also send every earlier finding and its outcome: `fixed: <commit>`, `disagree: <evidence>` or `declined: <reason>`. Ask the reviewer to check only open findings and the code that changed.
+
+## 4. Answer the findings
+
+- Check each finding against the code. Reviewers are wrong sometimes.
+- Agree: make the smallest fix. Add one focused test if the fix needs it.
+- Disagree: give evidence, such as a `file:line`, a test or command output.
+- Decline nits and out-of-scope items in one line.
+- Re-run the checks and commit before the next round.
+
+## 5. Stop
+
+- Stop when the verdict is APPROVE, or when only nits are open.
+- Use 3 rounds at most. Use up to 5 only while a blocker about security, data loss or concurrency is open.
+- If a finding stays disputed after two replies, or the rounds end, show both sides to the owner. The end of the rounds is not approval.
+
+Add the result to the "Review" section of the pull request: the reviewer kind, the number of rounds, the verdict, and what the review changed. Do not name the model: the fork rules forbid model and AI credit lines.

.github/pull_request_template.md

@@ -36,4 +36,8 @@ For UI changes, include clear before/after screenshots. Include a short recordin
 when motion, timing, transitions, or interaction details are needed to demonstrate
 the change. Upload evidence to GitHub and embed or link it here. Never commit PR-only assets. -->
 
-<!-- If you used an agent, end with the model and harness that did the work. -->
+## Review
+
+<!-- Fork: say how the change was reviewed, for example the review-loop skill
+(.agents/skills/review-loop) or a subagent. Give the reviewer kind, the number of
+rounds, the verdict and what the review changed. Do not name a model or harness. -->

.github/workflows/fork-ci.yml

@@ -5,9 +5,10 @@ name: Fork CI
 # Limit: this file copies the ci.yml commands by hand. When upstream changes
 # them, update this file. A failure on main calls the repair agent. See docs/fork/operations.md "Fork CI".
 #
-# The heavy jobs run only when the push or pull request changes code. The changes job
-# compares a push with the last green run, so a cancelled or failed run never hides a
-# change. It compares a pull request with its base.
+# The changes job runs upstream's .github/scripts/ci-changes.cjs, which maps the
+# changed files to the jobs they can affect. It compares a push with the last green
+# run, so a cancelled or failed run never hides a change. It compares a pull request
+# with its base.
 
 on:
   push:
@@ -40,10 +41,25 @@ jobs:
       actions: read
       contents: read
     outputs:
-      code: ${{ steps.classify.outputs.code }}
+      code: ${{ steps.classify.outputs.typecheck }}
       files: ${{ steps.classify.outputs.files }}
-      machine_map: ${{ steps.classify.outputs.machine_map }}
+      build: ${{ steps.classify.outputs.build }}
+      test: ${{ steps.classify.outputs.test }}
+      test_packages: ${{ steps.classify.outputs.test_packages }}
+      test_web: ${{ steps.classify.outputs.test_web }}
+      test_server: ${{ steps.classify.outputs.test_server }}
+      rust: ${{ steps.classify.outputs.rust }}
     steps:
+      # Only the files ci-changes.cjs reads to build the package graph.
+      - uses: actions/checkout@v6
+        with:
+          sparse-checkout: |
+            /pnpm-workspace.yaml
+            /.github/scripts/ci-changes.cjs
+            /*/package.json
+            /*/*/package.json
+            !/.repos/
+          sparse-checkout-cone-mode: false
       - name: Classify the changed files
         id: classify
         env:
@@ -52,58 +68,37 @@ jobs:
           PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
         run: |
           set -euo pipefail
-          code() { echo "code=true" >>"$GITHUB_OUTPUT"; echo "$1: the heavy jobs run."; exit 0; }
+          # Without a list of changed files, ci-changes.cjs runs every job.
+          everything() { echo "$1: every job runs."; node .github/scripts/ci-changes.cjs | tee -a "$GITHUB_OUTPUT"; exit 0; }
           head="$GITHUB_SHA"
           case "$GITHUB_EVENT_NAME" in
             push)
               base="$(gh run list --repo "$GITHUB_REPOSITORY" --workflow fork-ci.yml --branch "$GITHUB_REF_NAME" \
                 --event push --status success --limit 1 --json headSha --jq '.[0].headSha // empty')"
-              [[ -n "$base" ]] || code "No green run to compare with"
+              [[ -n "$base" ]] || everything "No green run to compare with"
               ;;
             pull_request)
               base="$PR_BASE_SHA"
               head="$PR_HEAD_SHA"
               ;;
-            *) code "Manual run" ;;
+            *) everything "Manual run" ;;
           esac
-          # The compare API lists at most 300 files. A longer list counts as code.
-          files="$(gh api "repos/$GITHUB_REPOSITORY/compare/$base...$head" --jq '.files[].filename')" ||
-            code "The compare with $base failed"
-          (($(wc -l <<<"$files") < 300)) || code "300 or more changed files"
-          while IFS= read -r file; do
-            [[ -n "$file" ]] || continue
-            case "$file" in
-              docs/* | *.md) ;;
-              apps/machine-map/*) echo "machine_map=true" >>"$GITHUB_OUTPUT" ;;
-              *) code "$file changed since $base" ;;
-            esac
-          done <<<"$files"
-          echo "code=false" >>"$GITHUB_OUTPUT"
+          # The compare API lists at most 300 files. A longer list runs every job.
+          # A rename lists the old path too: moving a file out of a package changes that package.
+          files="$(gh api "repos/$GITHUB_REPOSITORY/compare/$base...$head" \
+            --jq '.files[] | .filename, (.previous_filename // empty)')" ||
+            everything "The compare with $base failed"
+          (($(wc -l <<<"$files") < 300)) || everything "300 or more changed files"
           { echo "files<<EOF"; echo "$files"; echo "EOF"; } >>"$GITHUB_OUTPUT"
-          echo "Only docs, markdown or apps/machine-map changed since $base: only lint and the machine map job run."
-
-  # The heavy jobs also cover apps/machine-map. This job checks it alone when nothing else changed.
-  machine_map:
-    name: Machine map
-    needs: changes
-    if: needs.changes.outputs.code == 'false' && needs.changes.outputs.machine_map == 'true'
-    runs-on: ubuntu-24.04
-    timeout-minutes: 10
-    steps:
-      - uses: actions/checkout@v6
-        with:
-          sparse-checkout: |
-            /*
-            !/.repos/
-          sparse-checkout-cone-mode: false
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: |
-            - args: ["--filter", "@t3tools/machine-map...", "--filter", "@t3tools/monorepo"]
-      - run: vp run --filter @t3tools/machine-map typecheck
-      - run: vp run --filter @t3tools/machine-map test
+          # Fork: Markdown is docs anywhere. ci-changes.cjs treats only root *.md as docs.
+          grep -v '\.md$' <<<"$files" >"$RUNNER_TEMP/changed-paths" || true
+          decision="$(node .github/scripts/ci-changes.cjs --paths "$RUNNER_TEMP/changed-paths")"
+          # As upstream: a code push to main runs every job, the backstop for a
+          # dependency the script misses. A docs-only push still skips them.
+          if [[ "$GITHUB_EVENT_NAME" == push ]] && grep -qx 'typecheck=true' <<<"$decision"; then
+            decision="$(node .github/scripts/ci-changes.cjs --paths "$RUNNER_TEMP/changed-paths" --full)"
+          fi
+          tee -a "$GITHUB_OUTPUT" <<<"$decision"
 
   lint:
     name: Lint
@@ -117,11 +112,7 @@ jobs:
             /*
             !/.repos/
           sparse-checkout-cone-mode: false
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: true
+      - uses: ./.github/actions/setup-ci-deps
       - if: needs.changes.outputs.code == 'true'
         run: vp run --filter @t3tools/desktop ensure:electron
       - if: needs.changes.outputs.code == 'true'
@@ -160,18 +151,14 @@ jobs:
             /*
             !/.repos/
           sparse-checkout-cone-mode: false
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: true
+      - uses: ./.github/actions/setup-ci-deps
       - run: vp run --filter @t3tools/desktop ensure:electron
       - run: vpr typecheck
 
   build:
     name: Build
     needs: changes
-    if: needs.changes.outputs.code == 'true'
+    if: needs.changes.outputs.build == 'true'
     runs-on: ubuntu-24.04
     timeout-minutes: 20
     steps:
@@ -182,32 +169,22 @@ jobs:
             !/.repos/
           sparse-checkout-cone-mode: false
       - run: sudo apt-get update && sudo apt-get install -y libsecret-1-dev pkg-config
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: true
+      - uses: ./.github/actions/setup-ci-deps
       - run: vp run --filter @t3tools/desktop ensure:electron
       - run: vp run build:desktop
       - run: node apps/desktop/scripts/verify-preload-bundle.mjs
 
   test:
     name: Test ${{ matrix.group }}
     needs: changes
-    if: needs.changes.outputs.code == 'true'
+    if: needs.changes.outputs.test == 'true'
     runs-on: ubuntu-24.04
     timeout-minutes: 30
     # Two runners: on one runner, the slowest packages wait for each other.
     strategy:
       fail-fast: false
       matrix:
-        include:
-          - group: apps
-            filters: --filter @t3tools/mobile --filter @t3tools/desktop --filter @t3tools/oxlint-plugin-t3code
-          - group: packages
-            filters: >-
-              --filter '!t3' --filter '!@t3tools/monorepo' --filter '!@t3tools/web'
-              --filter '!@t3tools/mobile' --filter '!@t3tools/desktop' --filter '!@t3tools/oxlint-plugin-t3code'
+        group: [apps, packages]
     steps:
       - uses: actions/checkout@v6
         with:
@@ -216,22 +193,42 @@ jobs:
             !/.repos/
           sparse-checkout-cone-mode: false
       - run: sudo apt-get update && sudo apt-get install -y --no-install-recommends build-essential libsecret-1-dev pkg-config
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: true
+      - uses: ./.github/actions/setup-ci-deps
       - run: vp run --filter @t3tools/desktop ensure:electron
       - if: matrix.group == 'packages'
         run: python3 -B .github/scripts/stage-preview-bundle.test.py
       - if: matrix.group == 'packages'
         run: node --test .github/scripts/check-nightly-release.test.cjs
-      - run: vp run --parallel --concurrency-limit 4 ${{ matrix.filters }} test
+      # TEST_PACKAGES lists the affected packages, or nothing when all of them are.
+      # t3 and @t3tools/web have their own jobs.
+      - name: Test
+        env:
+          GROUP: ${{ matrix.group }}
+          TEST_PACKAGES: ${{ needs.changes.outputs.test_packages }}
+        run: |
+          apps=(@t3tools/mobile @t3tools/desktop @t3tools/oxlint-plugin-t3code)
+          filters=()
+          if [[ -z "$TEST_PACKAGES" ]]; then
+            if [[ "$GROUP" == apps ]]; then
+              for package in "${apps[@]}"; do filters+=(--filter "$package"); done
+            else
+              filters=(--filter '!t3' --filter '!@t3tools/monorepo' --filter '!@t3tools/web')
+              for package in "${apps[@]}"; do filters+=(--filter "!$package"); done
+            fi
+          else
+            for package in $TEST_PACKAGES; do
+              group=packages
+              case " ${apps[*]} " in *" $package "*) group=apps ;; esac
+              if [[ "$group" == "$GROUP" ]]; then filters+=(--filter "$package"); fi
+            done
+          fi
+          if ((${#filters[@]} == 0)); then echo "No affected package in the $GROUP group."; exit 0; fi
+          vp run --parallel --concurrency-limit 4 "${filters[@]}" test
 
   test_web:
     name: Test Web
     needs: changes
-    if: needs.changes.outputs.code == 'true'
+    if: needs.changes.outputs.test_web == 'true'
     runs-on: ubuntu-24.04
     timeout-minutes: 20
     steps:
@@ -241,19 +238,15 @@ jobs:
             /*
             !/.repos/
           sparse-checkout-cone-mode: false
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: true
+      - uses: ./.github/actions/setup-ci-deps
       # The browser project runs React tests in headless Chromium.
       - run: node apps/web/node_modules/playwright/cli.js install --with-deps --only-shell chromium
       - run: vp run --filter @t3tools/web test
 
   test_server:
     name: Test Server ${{ matrix.shard }}
     needs: changes
-    if: needs.changes.outputs.code == 'true'
+    if: needs.changes.outputs.test_server == 'true'
     runs-on: ubuntu-24.04
     timeout-minutes: 20
     strategy:
@@ -267,18 +260,14 @@ jobs:
             /*
             !/.repos/
           sparse-checkout-cone-mode: false
-      - uses: voidzero-dev/setup-vp@v1
-        with:
-          node-version-file: package.json
-          cache: true
-          run-install: true
+      - uses: ./.github/actions/setup-ci-deps
       - run: node apps/server/node_modules/playwright-core/cli.js install --with-deps --only-shell chromium
       - run: vp run --filter t3 test --shard ${{ matrix.shard }}/${{ strategy.job-total }}
 
   rust:
     name: Rust
     needs: changes
-    if: needs.changes.outputs.code == 'true'
+    if: needs.changes.outputs.rust == 'true'
     runs-on: ubuntu-24.04
     timeout-minutes: 20
     steps:
@@ -302,7 +291,7 @@ jobs:
   # so the docs-only path passes too. A cancelled job counts as failed.
   result:
     name: Fork CI result
-    needs: [changes, lint, machine_map, typecheck, build, test, test_web, test_server, rust]
+    needs: [changes, lint, typecheck, build, test, test_web, test_server, rust]
     if: always()
     runs-on: ubuntu-24.04
     timeout-minutes: 5
@@ -320,7 +309,7 @@ jobs:
   # A pull request never calls it: its author sees the failed check.
   notify_agent:
     name: Notify repair agent
-    needs: [changes, lint, machine_map, typecheck, build, test, test_web, test_server, rust]
+    needs: [changes, lint, typecheck, build, test, test_web, test_server, rust]
     if: failure() && github.event_name != 'pull_request' && github.event.inputs.notify_agent != 'false'
     runs-on: ubuntu-24.04
     timeout-minutes: 20

docs/fork-agents.md

@@ -11,6 +11,7 @@ This guide maps owner tasks to the fork tools. It is for agents that operate the
 - Never kill a process that you found by a name, a path or a pattern. Kill only a PID that you started.
 - Never write to `~/.t3/userdata` on a machine. It is the live install. Read it or copy from it only.
 - Stage files by path only. Never use `git add -A` or `git add .`. Worktrees hold links to private files from `_shared/`.
+- Before you open a pull request, review the change with the `review-loop` skill (`.agents/skills/review-loop`). Fill the "Review" section of the pull request template.
 - Before every push, run `vp run fork:check` and fix each failed step. On a branch with an upstream merge not yet in `origin/main`, it exits without checks; run typecheck and the fork tests instead, then wait for Fork CI. See [Check a change before you push](fork/operations.md#check-a-change-before-you-push).
 - Put every fork behaviour behind a setting. A setting that only adds something without noticeable cost starts on. A setting that changes existing behavior or layout, installs or restarts anything, changes access or needs setup starts off.
 - Set the values of the owner's machines through the machine map, not through the code default.

docs/fork/operations.md

@@ -60,7 +60,8 @@ Each day at 09:00 Israel time, the VPS task "Fork daily release" promotes a nigh
 
 - A newer push to main or to a pull request cancels the Fork CI run of the older push.
 - The job "Fork CI result" fails when any Fork CI job fails or is cancelled. Branch protection requires this one check.
-- Fork CI compares a push with the commit of the last green push run, and a pull request with its base. When only `docs`, `*.md` or `apps/machine-map` changed, only the Lint job runs, on the changed files. A change in `apps/machine-map` also runs the Machine map job. A manual run always runs every job.
+- Fork CI compares a push with the commit of the last green push run, and a pull request with its base. Upstream's `.github/scripts/ci-changes.cjs` maps the changed files to packages and their dependents. A pull request runs only the jobs that those packages need. When only `docs/` or Markdown files changed, only the Lint job runs, on the changed files. A code push to main and a manual run run every job.
+- Each job installs through upstream's `.github/actions/setup-ci-deps`. A cache hit makes no request to npm.
 - The Lint job also runs `scripts/fork-check-doc-links.ts`. It fails on a relative link or an anchor that does not resolve in `README.md`, `docs/fork*`, `docs/fork/` or `apps/machine-map/*.md`.
 
 ### Repair agent