Skip to content

sync-skill: label protocol-bump syncs at creation, retarget an open PR in place - #130

Merged
zpyoung merged 1 commit into
mainfrom
sync-lessons/v1.4.233
Oct 20, 2026
Merged

zpyoung merged 1 commit into
mainfrom
sync-lessons/v1.4.233

Conversation

@zpyoung

@zpyoung zpyoung commented Oct 20, 2026

Copy link
Copy Markdown
Owner

ELI5

Two things the v1.4.232/v1.4.233 sync run (PR #129) learned the hard way, written into the sync skill so the next run doesn't repeat them.

What Changed

  • Step 9: when the release bumps the daemon protocol, pass --label allow-runtime-launcher-protocol-bump to gh pr create. PR sync: absorb upstream v1.4.233 #129's first run failed cross-version wire compatibility with FAIL: split the protocol bump and the runtime launcher change into separate PRs, at daemon PROTOCOL_VERSION: base 49, candidate 50 and four launcher files. Upstream made those changes in separate PRs; the sync diff carries them as one. sync: absorb upstream v1.4.224 #115 and sync: absorb upstream v1.4.230 #122 needed the same label. pr.yml has no labeled trigger, so labeling after the first run only counts from the next push, which cost a CI round here.
  • Fix policy, upstream-defect bullet: how to prove an upstream defect (pnpm test:sandbox --shards=1 --source-ref=<tag> -- <test> reproduced session-scanner-scope-cwd-facts at bare v1.4.232) and how to retarget with the PR already open: merge the newer tag into the same branch by Steps 5–8, push, retitle. The skill can't create a branch, so a second PR isn't an option. v1.4.233 (test(ai-vault): stop the cold-authority scope test depending on the clock stablyai/orca#31098) fixed the test, and sync: absorb upstream v1.4.233 #129 merged both tags as 9bd674f464.

Why

Both edits sit in the step that owns the decision rather than in sync-lessons.md, because each is a rule the next run has to act on. Neither touches the hard safety rules or the fix policy's never-list.

Linked Issue

N/A. Sync-skill lesson from PR #129.

Visual Proof

N/A. Documentation only.

Testing

config/scripts/sync-upstream-file-ownership.test.mjs reads this skill. The only script it names is check-runtime-launcher-protocol-ratchet.mjs, which exists, and PR CI runs the test.

… an open PR in place

PR #129's first CI run failed cross-version wire compatibility on the
runtime-launcher protocol ratchet: v1.4.232 bumped the daemon protocol (49->50)
and touched launcher files in separate upstream PRs, which a sync diff carries
as one. The allow-runtime-launcher-protocol-bump label is the sanctioned path
(#115, #122 used it), but pr.yml has no labeled trigger, so adding it late cost
a CI round. Step 9 now says to pass the label to gh pr create when
daemon-protocol-version.ts moved.

The same PR then failed session-scanner-scope-cwd-facts. Running the test at
the bare tag with test:sandbox --source-ref=v1.4.232 reproduced it, proving an
upstream defect, and v1.4.233 (stablyai#31098) fixed it. The fix policy said to retarget
but not how with a PR open and no permission to create a branch; it now does.
Copilot AI balanced review requested due to automatic review settings October 20, 2026 22:07

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 c9feb5c into main Oct 20, 2026
12 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