Skip to content

sync-skill: stack on open sync PRs, ask instead of stopping, and match CI locally - #128

Merged
zpyoung merged 8 commits into
mainfrom
zpyoung/sync-skill-handsoff
Oct 19, 2026
Merged

zpyoung merged 8 commits into
mainfrom
zpyoung/sync-skill-handsoff

Conversation

@zpyoung

@zpyoung zpyoung commented Oct 19, 2026

Copy link
Copy Markdown
Owner

ELI5

The daily upstream sync kept needing a person. It stopped for days whenever an older sync PR was still open, and it found its CI failures one at a time, each costing a 20–60 minute CI round. This PR makes the sync skill build on the open PR instead of stopping. It also makes the skill ask the user and wait when it needs a decision, and run locally the checks CI would otherwise fail on.

What Changed

Before: an unattended sync stopped at Step 2 whenever an older sync: absorb upstream <tag> PR was open. PR #120 sat green and unmerged for 68 hours while three daily runs stopped behind it. Any human decision ended the run. After the push, v1.4.230's PR took 7 hours and 8 CI rounds to go green. Four of those rounds were failures in the static analysis job, one new step per round, because that job stops at its first failure and unit tests wait behind it.

After:

  • Skill (.claude/skills/sync-upstream/SKILL.md):
    • An older open sync PR becomes the base to stack on, and the run closes it once the new PR is up.
    • Escalations go to the user through orca ask with a 12-hour wait, and a run never ends its turn while its PR is open.
    • Step 6 sweeps the release for upstream identity: the signing team, runners and repository.
    • Step 8 adds a clean-tree check, the two PR guards on committed HEAD, a verify-routing check, and a local replay of the preflight job.
    • Step 10 treats pr.yml as gating and every other workflow as advisory. It waits with one background command.
    • Step 11 merges after a 15-minute advisory window with --match-head-commit.
    • One writer per sync PR, and fix commits are staged by path.
    • The fix policy may remove a pr.yml job the release cannot route, with a ledger entry to restore it later.
    • The run merges lesson PRs that change only skill files once their guard and gate are green.
  • New config/scripts/fork-sync-gate/ (Tier 1, declared as feature fork-sync-gate):
    • check-pr-verify-selections.mjs proves every job verify requires is routed by pr-code-change-scope.mjs and that every script pr.yml runs exists. Against v1.4.230's tag pr.yml it reports the dependency_isolation defect that failed every PR.
    • replay-pr-preflight.mjs reads the preflight job from pr.yml and runs every step it can locally, in one pass, in about 3 minutes. It skips the typecheck and steps whose condition only CI can evaluate, and names each skip.
    • wait-pr-checks.mjs is a single background wait. It exits on the first gating failure, on a head moved by another writer, on a closed PR, or once the gate is green and the advisory window has closed.
  • pr.yml: drops a reference to win32-test-lane-registration.test.mjs, which the fork deletes. The new check found it.
  • .gitignore: ignores the release-JavaScript benchmark's default output directory, which a fix commit on sync: absorb upstream v1.4.230 #122 swept in. Declared as a seam with its residual raised to +15/-2.
  • Lesson PRs sync-skill: catch balanced -X ours seam damage, and stop typechecking the retired mobile-web project #99, sync-skill: stop early on an undecided sync escalation, and force-add ignored tree-conflict paths #114 and sync-skill: stack a directed sync on an open sync PR; recognize tag-tip workflow swaps #125 are merged into this branch. GitHub marks them merged when this lands.

Why

I read every session of the v1.4.230 sync (the lead, 10 re-home agents, the CI-driving sessions, and the PR-sitter sessions) and ranked what cost the most time. The user picked these two batches: stall-proofing and local CI parity. They also made the policy calls: stack and close rather than stop, merge with a 15-minute cap on advisory checks, allow removing an unroutable job with a ledger entry, auto-merge lesson PRs, and ask and wait up to 12 hours. I put the CI parity in scripts that read pr.yml, not in a list kept in the skill, so they follow upstream's step changes automatically. A hand-kept list is exactly what drifted on the fork's localization wrapper.

Not in this PR: the resolution tooling (a scripted pr.yml transform, re-home helpers) and the PR-sitter fixes. The user deferred both.

Linked Issue

N/A (fork maintenance)

Visual Proof

N/A: tooling and skill text only, no UI change.

Testing

  • check-pr-verify-selections.mjs passes on this branch. Against v1.4.230's tag pr.yml it reports dependency_isolation and its missing test file.
  • replay-pr-preflight.mjs runs end to end on this branch (every replayable step green). Before the lint fixes it caught 4 findings in the new files themselves.
  • wait-pr-checks.mjs checked against real PRs: merged sync: absorb upstream v1.4.230 #122 exits 4, green sync-skill: stack a directed sync on an open sync PR; recognize tag-tip workflow swaps #125 exits 0, and a wrong head exits 3.
  • Root-directory guard, fork ownership guard, --verify-seams and --verify-residuals all pass, and typecheck:node passes.
  • 13 new unit tests pass on the remote test sandbox.

AI Disclosure

Written by Claude (Opus 5.5) in Claude Code at the user's direction.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

The scripts are Node and run anywhere the sync runs. replay-pr-preflight.mjs needs bash, which every GitHub runner and Git for Windows has. Nothing here touches the app, SSH, or mobile.

Checklist

  • I explained what changed and why
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)

…sync pushes

- check-pr-verify-selections: every job verify requires has a selection
  pr-code-change-scope.mjs emits, and every script pr.yml runs exists
  (it names v1.4.230's unroutable dependency_isolation job)
- replay-pr-preflight: runs pr.yml's preflight steps locally in one pass
  instead of one CI round per failing step
- wait-pr-checks: one background wait that exits on the first gating
  failure, a moved head, or a green gate after an advisory window
- pr.yml: drop the mobile CI tools step's reference to a test the fork
  deletes, which the selection check found
- .gitignore: ignore the release-JavaScript benchmark's default output dir
…and match CI locally

From the v1.4.230 retro, which read every session of that sync:
- an open older-tag sync PR is now the base to stack on and close, not a
  hard stop that stalled the sync for days three times
- escalations put the decision to the human through orca ask and wait up
  to 12h; a run never ends its turn while its PR is open
- Step 8 adds a clean-tree check, the verify-selection check, both PR
  guards and a local replay of pr.yml's preflight job
- Step 6 sweeps the release for upstream signing, runner and repo identity
- Step 10 defines pr.yml as gating and every other workflow as advisory,
  waits with one background command, and Step 11 merges after a 15-minute
  advisory window with --match-head-commit
- one writer per sync PR; fix commits stage by path
- the fix policy may remove a pr.yml job the release cannot route, with a
  ledger entry to restore it
- qualifying skill-only lesson PRs are merged by the run
Copilot AI balanced review requested due to automatic review settings October 19, 2026 20:34

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@zpyoung
zpyoung merged commit 1e66e4f into main Oct 19, 2026
35 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.

2 participants