infra: three submodules, watched releases, registry-mirrored skills - #358
infra: three submodules, watched releases, registry-mirrored skills#358marc0olo wants to merge 11 commits into
Conversation
Eleven repos were vendored as submodules purely so agents could check a fact against them. None of their bytes reach the built site, so each one was a recurring bump PR with nothing to show for it, and several drifted years of releases behind unnoticed. They are now watched instead: .sources/upstream.json records the ref the docs are verified against, scripts/check-upstream-releases.mjs compares that against the newest matching tag, and a weekly workflow opens one issue per repo that moved, carrying the ref delta and a summary of what changed. Agents verify against the pinned ref via raw.githubusercontent.com rather than main, so a review stays reproducible and a page cannot document a flag from a release whose doc links have not been adapted yet.
|
🤖 Here's your preview: https://dpfyp-oaaaa-aaaam-ahcqa-cai.icp0.io |
examples, icskills, and dotskills had no notification when their pin fell behind: motoko and internetidentity are covered by their own sync workflows, and the release check only looked at the non-vendored repos. They are now in upstream.json as a vendored group whose pin is read from the gitlink, so git stays the single source of truth for it. icskills and dotskills are both currently behind.
…tion Skills came from two submodules through a double symlink chain (.claude/skills -> .agents/skills -> .sources/<submodule>/skills). They now come from skills.internetcomputer.org via a SessionStart hook, matching dfinity/examples: a differential sync keyed on each skill's published hash, offline-safe, and it only prunes skills it installed itself. That drops the icskills submodule, the symlink farm, and a watcher entry, and raises coverage from 16 skills to 29 — including static-site, custom-domains, encrypted-maps, service-discoverability, and mops-cli. It also corrects two stale names in the AGENTS.md table: asset-canister no longer exists (static-site covers it, including the legacy recipe and migration) and vetkd is now vetkeys. technical-documentation is not on the IC registry. It changed once since the pin and only to switch its license field to MIT, so its 11 text files are vendored next to the two brand skills that were already committed, and the dotskills submodule goes too. Three submodules remain: motoko, internetidentity, examples.
marc0olo
changed the title
MIT requires the copyright and permission notice to travel with a copy. The skill's frontmatter recorded its license and source but not the notice itself, so the upstream license text now sits alongside the files and NOTICE records the attribution.
The session-start hook is Claude Code only and automatic skill loading is harness-specific everywhere, so nothing loads skills automatically elsewhere. What is portable is where to get them: the IC skills from the registry index by URL, since they are gitignored and absent from a fresh clone, and the three repo-maintained skills from their committed path. Also records that the sync replaces a local skill whose name is later published to the registry, so publishing one means deleting the local copy in the same change.
…e version The cdk-rs pin was 0.20.1, which is the published crate version, not a git tag: the repo's newest bare-semver tag is 0.19.0, two minors behind. Compared against tags, the pin could never be overtaken, so the check reported 'current' forever for the repo that governs every Rust code block. A release pin that is not among the matched tags is now a loud failure rather than silence, and cdk-rs tracks crates.io, which is where its releases actually appear. Auditing the other ten pins found no further cases. motoko-core and response-verification now diff their changelogs, which names the APIs that changed instead of leaving 'check every signature' as the whole task.
… label Auditing every watched repo against its real publishing channel found a second case beyond cdk-rs: icp-js-sdk-docs carries no tags, so its commit was standing in for an @icp-sdk/core release. Tracking npm instead shows the docs are verified against 5.4.0 while 6.1.0 is out, a major bump across every JS code block. The pin was derived from public/core/versions.json at the previously pinned commit rather than guessed. Registry-tracked entries get a registry link instead of a GitHub compare view and no file diff, because a package version is not a ref git can resolve; both were producing broken links and a failed fetch. Every issue now also carries upstream-sync, so they are findable as a class rather than only per repo.
icp-js-sdk-docs is the delivery site for js.icp.build and carries the API docs for six libraries as zips. A commit on it means the docs site was rebuilt, not that an SDK released, so it was a proxy for the thing we actually document. The two packages the docs use are watched directly, and they need different signals: icp-js-core tags match its npm versions, so tags are authoritative and its CHANGELOG is the review payload, while icp-js-canisters publishes 3.6.0 with its newest semver tag at v1.0.0, so npm is the signal there. Both pins come from what the docs assert: v5.4.0 from the mirror's public/core/versions.json at the previously pinned commit, and 3.5.0 from the stated minimum in references/application-canisters.md.
…ches Addresses the Copilot review on #358. The commit track ordered short SHAs with the version comparator, so a real update whose SHA happened to sort lower was reported as current. Demonstrated: pinned ef03698 with head 9f1e819 returned 'current'. SHAs have no order, so the pin either prefixes the head or it does not, and links now use the full SHA while only the display is shortened. The link row was inferred from whether a registry URL was set, which mislabelled the commit track's GitHub compare view as 'Registry'; it is now named explicitly, and the verify-diff guard keys off that name rather than the URL. Also: upstream.json documented only two of the four tracks, VERSIONS claimed 7-char hashes while internetidentity uses 8, the label wording omitted both the shared upstream-sync label and the vendored form, and the workflow's jq filter uses an explicit index.
Twelve watched entries meant a weekly check on repos the docs barely draw on. Measured footprint in docs/ decided each one: icp-cli 51 pages, 461 hits, plus the link slug motoko-core 63 pages, 328 hits cdk-rs 47 pages, 336 hits, plus pinned versions in prose icp-js-core 15 pages, 47 hits icp-cli-recipes 11 pages, 26 hits, all pinned recipe versions in prose icp-js-canisters 4 pages, 8 hits, with a stated minimum version The other six move to a group: templates (linked, not restated), candid (a rarely-moving spec), response-verification (two pages that link to docs.rs), chain-fusion-signer (seven mentions), papi (one page), ic-pub-key (three pages). They keep their entry and their verify path so an agent knows where to look, but carry no pin and produce no issue: verify against the latest release on demand, since the surface is small enough that the next edit to those pages catches drift. First run drops from ten issues to five.
|
All five Copilot comments addressed. One was a real bug in the script; four were accurate documentation mismatches. 1. 2. 3. 4. 5. Label wording. Correct and then some: it omitted both the shared Unrelated to the review, the watch list also shrank from 12 repos to 6 after measuring what each upstream actually underwrites in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 72 out of 73 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
AGENTS.md:220
- This section says there are "five" vendored submodules, but the table immediately below lists three (
motoko,internetidentity,examples). This will mislead anyone following the instructions.
scripts/check-upstream-releases.mjs:148 summarizeChangeusesSets of lines, which can undercount changes when the same non-empty line appears multiple times (duplicates in changelogs/headings are common). Using per-line counts will make the added/removed totals and heading detection more accurate.
| const pinned = pinnedSha.slice(0, 7); | ||
| const latest = headSha.slice(0, 7); | ||
| const name = path.replace(/^\.sources\//, ''); | ||
| const body = [ | ||
| `The \`${path}\` submodule is behind \`${repo}@${branch}\`.`, | ||
| '', | ||
| '| | |', | ||
| '|---|---|', | ||
| `| Pinned (gitlink) | \`${pinned}\` |`, | ||
| `| Branch head | \`${latest}\` |`, | ||
| `| Compare | https://github.com/${repo}/compare/${pinned}...${latest} |`, | ||
| '', | ||
| '## What to re-check', | ||
| '', | ||
| affects ?? 'No notes recorded for this submodule.', | ||
| '', | ||
| '## How to close this', | ||
| '', | ||
| '```bash', | ||
| `git -C ${path} fetch origin ${branch}`, | ||
| `git -C ${path} checkout ${latest}`, | ||
| '```', |
| // Commit SHAs have no order, so "newer" cannot be a comparison: the pin is | ||
| // a prefix of the head or it is not. Ordering them would silently report a | ||
| // real update as current whenever the new SHA happened to sort lower. | ||
| moved = !sha.startsWith(pinned); | ||
| // Links resolve against the full SHA; only the display is shortened. | ||
| compare = `https://github.com/${repo}/compare/${pinned}...${sha}`; |
Addresses the second Copilot review on #358. The vendored check had both full SHAs in hand but put 7-char prefixes in the compare link and, more importantly, in the git checkout command it asks a maintainer to run. Short SHAs are now display-only. The commit track's comment also claimed links resolve against the full SHA, while the pin side of the compare URL is whatever upstream.json records. The comment now describes that: the head side is full, the pin side stays short for readability, and GitHub resolves a prefix on either side.
|
Both comments from the second review are accurate and fixed in Short SHAs in the vendored check. Correct, and the The comment that no longer matched the code. Also correct, and it was mine from fixing the previous review: I wrote "links resolve against the full SHA" while the pin side of the compare URL is whatever |
Closes #355.
.sources/held 16 submodules. Three are opened by the build; the rest were vendored so an agent could check a fact, or to supply skills through a symlink chain. Both have cheaper answers.Result: 3 submodules, 6 watched upstreams, 6 verified on demand, skills mirrored from the registry.
Kept, because the build opens them
motoko<motokoExamples>includesinternetidentitysync-ii-spec.mjs, which generates two reference pagesexamplesremark-snippet.mjs, which extractssnippet=codeThirteen go: eleven that existed only so an agent could check a fact, plus
icskillsanddotskills, which fed the skills symlink chain. The eleven appeared only in theAGENTS.mdsource table and.agents/submodule-bumping.md, and CI initializes justexamplesandmotoko, so nothing in the build referenced any of them.What is worth watching, and what is not
Watching everything meant a weekly check on repos the docs barely draw on. Measured footprint in
docs/decided each one:icp-climotoko-corecdk-rsicp-js-coreicp-cli-recipesicp-js-canistersThe other six became a
referencegroup:icp-cli-templates(linked, not restated),candid(a rarely moving spec),response-verification(two pages, both linking to docs.rs),chain-fusion-signer(7 mentions),papi(1 page),ic-pub-key(3 pages). They keep their entry and verify path so an agent knows where to look, but carry no pin and produce no issue: verify against the latest release on demand, since the surface is small enough that the next edit to those pages catches drift. Each entry records the footprint that decided it, so promotion back towatchedis a judgment someone can re-make.Every upstream is now checked exactly once
motoko,internetidentityexamplesupstream-releases.yml,vendoredmasterupstream-releases.yml,watched.claude/sync-ic-skills.shon session startexamplespreviously had no drift notification of any kind: it tracks a branch, and the only two workflows that could have noticed belong to the other two submodules. Issues carryupstream-syncplus a per-upstream label, and a run whose title already matches an open issue does nothing, so a repo that stays behind produces no weekly noise.Watching
.sources/upstream.jsonrecords per repo: the ref the docs are verified against, where its releases appear, which file's diff is the review payload, and what to re-check.scripts/check-upstream-releases.mjswrites an issue body per repo that moved (no token needed).npm run check:upstreamruns it locally.Agents verify against the pinned ref, so a review is reproducible and a page cannot document a flag from a release whose links have not been adapted:
raw.githubusercontent.comrather thangh api .../contents/...: the API returns base64 truncated at ~2.3 KB against 71 KB for the full file, and a truncated flag reference looks complete.Where releases appear differs per repo, and getting it wrong fails silently
trackreleaseicp-cli,icp-cli-recipes,motoko-core,icp-js-corecratecdk-rsnpmicp-js-canistersFour defects found and fixed, three while building this and one from the Copilot review:
0.9.4beats0.20.1. Sorting is numeric-run aware.cdk-rswas pinned to0.20.1, which is not a tag but the published crate version, with the newest tag two minors behind. Against tags that pin could never be overtaken, so the repo governing every Rust code block would have reported "current" for ever. Areleasepin that is not among the matched tags is now a loud failure; every pin was audited.icp-js-sdk-docswas the wrong repo entirely. It is the js.icp.build delivery site, so a commit meant the docs site rebuilt. Replaced byicp-js-core(tags match npm) andicp-js-canisters(tags lag npm by two majors, so npm is the signal). Sibling repos, different correct answers.committrack ordered short SHAs with the version comparator, so an update whose SHA happened to sort lower read as "current". Demonstrated with pinef03698against head9f1e819. SHAs have no order, so it is now a prefix match, with the full SHA in links.commitis currently used by nothing: it exists for a repo with no releases at all, and after the reduction below no watched repo is in that state.First run: 5 of 7 have drifted
icp-cliv1.1.0→v1.3.0,motoko-corev2.4.0→v2.6.1,cdk-rs0.20.1→0.20.2,icp-js-corev5.4.0→v6.1.0,icp-js-canisters3.5.0→3.6.0.icp-cli-recipesandexamplesare current.Three carry real content risk:
motoko-core(two minors ofmo:core),icp-js-core(a major bump,Agent.readStaterevamped),cdk-rs. Where a changelog exists it is the review payload, because "check every API signature" is unbounded while a changelog names the symbols to grep for.icp-cliis pinned at v1.1.0 deliberately: #354 moved the doc links to the 1.3 slug, but the commands were last verified against v1.1.0, so the first watch issue carries that pass rather than hiding it.Skills
.claude/sync-ic-skills.shonSessionStart, byte-identical to the scriptdfinity/examplesruns, replaces two submodules and a double symlink chain. Coverage rises 16 → 29 and corrects two table entries pointing at skills that no longer exist upstream:asset-canister→static-site(which covers the legacy recipe and the migration) andvetkd→vetkeys.No symlinks, in either direction: Claude finds a skill whether it is a directory or a link, no other harness auto-discovers any path, and for the synced 29 a symlink is actively wrong because the sync consumes anything at a name the registry publishes (verified in a sandbox). AGENTS.md states the non-Claude path instead: fetch the registry index by URL, explicitly not
.claude/skills/, which is gitignored and empty in a fresh clone.icp-brand-design,icp-brand-voice, andtechnical-documentationstay committed. The last is not on the registry and changed once since our pin, only to relicenseAGPL-3.0-only→ MIT. That license is why it was a submodule; MIT still requires the notice to travel, so its text sits in the skill directory and a new rootNOTICErecords the attribution.Instructions updated
AGENTS.md(source material, CLI verification, the1.3slug no longer derived from a submodule pin, skills, key directories),.agents/upstream-tracking.mdreplacingsubmodule-bumping.md, andCONTRIBUTING.md.Worth challenging
icp-cli-recipestracking is a heuristic. One pin cannot express five independently versioned recipes, and sorting across tag prefixes is meaningless. Per-recipe entries would need the issue slug keyed on something other than the repo.examplescosts 265 MB for one page today (6 snippets, 2 examples). Keeping it bets on<CodeExample>spreading.ic-cdk-timersandic-cdk-executorversion separately fromic-cdkand are not tracked individually; theaffectsnote tells a reviewer to check them.referencegroup trades a prompt for less noise. If nobody edits those pages, drift sits there unreported. That is the deliberate bet: six weekly checks on repos with a 1-to-3 page footprint cost more attention than they return.Found along the way, not fixed here
ic-cdkis pinned to four different versions across the docs:0.19in 8 places, plus0.20,0.17, and0.17.1. The published crate is 0.20.2. That is a content bug rather than infrastructure, so it belongs with thecdk-rswatch issue rather than in this PR.Follow-up
#279 is unblocked: of the 11 repos its "keep as-is" bucket names, only
examplesis still a submodule, andaffectsnow carries the upstream-to-pages mapping.After merge: trigger the workflow once so the drifts land as issues, batch the pin-only bumps, then handle
icp-cliandmotoko-coreas their own PRs.