feat: propose the pin move when an imported skill changes upstream - #27
Merged
Merged
Conversation
`sync_external.py --check` proves the vendored copy is still the pinned commit. Nothing proved the pin is still the commit upstream maintains, and that failure is silent: a stale pin keeps every check green while what this catalog publishes falls behind what its authors ship. `check_upstream.py` asks the other half. For each upstream it reads the tip of the default branch upstream itself names through HEAD, then compares the tree object id of every pinned directory at the pin against the same path at that tip. Directory hashes rather than repository HEADs is what keeps it quiet: an upstream commit that does not touch a pinned path is not an update here, and reporting one would open a pull request that rewrites 23 `.source.json` files and changes no skill text. It needs no file content, so two commits of intel/gpu-ai-skills cost 13.75 KiB and the whole survey runs in 3 s. When a directory has moved it moves the pin everywhere the pin is written down - every entry of that upstream, the twelve characters the catalog comment quotes, and NOTICE - re-vendors with `sync_external.py --write`, and opens one pull request per upstream. The branch is named after the target commit, so a second run finds its own pull request and stops. What the pull request does not do is decide whether the new bytes should be published; that is what its checks and its reviewer are for. `upstream-sync.yml` runs it weekly in intel/skills only, or by hand with a dry-run input. GitHub starts no workflow run for anything GITHUB_TOKEN does, so with the fallback token the pull request arrives correct but unchecked: the run warns and the body says so, and an UPSTREAM_SYNC_TOKEN secret fixes it. Every way this detector can break leaves it reporting that nothing moved, which is indistinguishable from an upstream nobody touched, so `--self-test` asserts it against this tree's own catalog and `--mutate M1..M7` shows each break turning it red. It runs on every pull request in validate.yml, not only on the schedule, because a pull request that breaks it would otherwise be found out about on a Monday.
`gh pr list` and `gh pr create` with no `--repo` resolve the repository from the remotes of the checkout they run in. In CI that is one remote and the answer is right. In a maintainer's clone it is several, and gh picks by its own rules: run from a clone that has the fork and two other remotes, `gh pr list --state open` answered about a repository nobody had named, and `--open-pr` would have opened the pull request there. So the repository comes from the remote the branch is pushed to, parsed from its URL in both the https and the ssh spelling, and anything that is not a github repository is refused rather than guessed. M8 is that refusal removed. Found by running the whole propose() path against a throwaway bare remote with a stand-in gh, which is also how the branch it pushes was checked: 37 files, and the 9 skills the detector names are exactly the 9 whose files other than .source.json changed.
The workflow pushes to the repository it runs in, so the branch and the pull request are refs of the same place. Run by hand it is not: the branch goes to a fork, and gh has to be told whose branch it is or it names one of this repository's. --against separates the two, branches off the target's default rather than the fork's, and M9 is the break.
One em dash reached a print. The body says why it avoids them -- both go through the console encoding of whoever runs the tool -- and the report had no reason to be different.
The assertion passed a string starting with /tmp, which is a hardcoded-temp-path finding whether or not anything is created -- nothing is here, it is a url that must be refused. Two urls now stand for that: a relative path and a git remote on another host.
napetrov
approved these changes
Sep 23, 2026
… not none GitHub now creates pull_request runs for a pull request opened or updated with GITHUB_TOKEN in an approval-required state, started by a maintainer with "Approve workflows to run". The workflow comment, the run annotation, the generated pull request body and MAINTAINERS.md all still said such a pull request carries no checks until someone pushes to it. The annotation drops from a warning to a notice, since it now describes the intended setup. MAINTAINERS.md also names the one setting the no-secret path does need: Allow GitHub Actions to create and approve pull requests.
|
Non-blocking: detection is per-directory, but bump() re-vendors every skill in the upstream group. classify() already knows which dirs changed. Follow-up: |
Contributor
Author
Going to merge as is as working initial version. Working on another PR with some improvements for that changes checker functionality. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sync_external.py --checkproves an imported skill is still the bytes of its pinned commit. Nothing proved the pin is still the commit upstream maintains, and that failure is silent: every check here stays green while what this catalog publishes falls behind what its authors ship. This is the other half.tools/check_upstream.py— compares the tree object id of eachexternal-pathat the pin against the same path at the tip of upstream's default branch. When a pinned directory moved, it movesexternal-commiteverywhere this repository writes it down (every entry of that upstream, the twelve characters the group comment quotes, andNOTICE), re-vendors withsync_external.py --write, and opens one pull request per upstream onsync/<upstream>-<commit>.tools/upstream_git.py— the transport, now shared withsync_external.py..github/workflows/upstream-sync.yml— Sundays and Wednesdays plusworkflow_dispatchwith adry-runinput; runs only inintel/skills; the one workflow here that writes.validate.yml— the detector's self-test, beside the two self-tests already there, because a scheduled job that stops detecting is not noticed until its next run.What it does not do is decide whether the new text should be published. An import is somebody else's document, so a change in it is a change somebody made there: the pull request's checks answer the structural half and a reviewer reads the diff.
Why a directory hash and not upstream's HEAD. Most commits upstream touch none of the skills imported here, so following HEAD would open a pull request that moves 23 pins, rewrites 23
.source.jsonfiles and changes no skill text. Per-path comparison costs 13.75 KiB of transfer per upstream and the whole survey runs in about three seconds; nothing is checked out, so the same code runs offline as the self-test above.No third-party code receives the write token — plain
gitandgh, the stancesecurity.ymlalready states.Token. No secret is needed. A pull request opened with
GITHUB_TOKENgets its checks in GitHub's approval-required state, and a maintainer starts them with Approve workflows to run; every pull request this opens says so in its body. An app or account token inUPSTREAM_SYNC_TOKENonly removes that click. What it does need is the repository setting Allow GitHub Actions to create and approve pull requests: with it off, the run fails atgh pr createrather than quietly.Run, not assumed
intel/gpu-ai-skillsmoved3fb51a0b6952 -> 0b4fafd09c5e, 9 of 20 pinned directories changed, 37 files, and the level 1 gate is green on it.intel/intel-performance-skillsis at its pin and was correctly left alone.--self-test: 21 assertions against this tree's real pins, offline.--mutate M1..M9is nine distinct ways the detector can break — every one of them reports "nothing moved upstream", and every one of them turns the self-test red.SKIP intel/gpu-ai-skills: #26 (open) already proposes sync/gpu-ai-skills-0b4fafd09c5e, including the case where the head branch lives in a fork.actionlintandzizmor --min-severity=medium --min-confidence=mediumare clean.Not run yet: the workflow itself has never executed on a runner — the schedule, the empty
dry-runinput on ascheduleevent,env.SYNC_TOKEN_SETand the repository guard are static-checked only. Aworkflow_dispatchwithdry-run: trueright after this merges is what closes that, and it changes nothing when it runs. Whether that repository setting is on here is something only the first real run can show.Docs are in the same change:
MAINTAINERS.mdgains the workflow row, what the write permissions are for, the approval step and the one setting it needs, and how to run the same proposal by hand from a fork.