Skip to content

fix: clear hand-written validate --all violations, correct the mo:base check - #357

Merged
raymondk merged 1 commit into
mainfrom
infra/validate-clean
Aug 27, 2026
Merged

fix: clear hand-written validate --all violations, correct the mo:base check#357
raymondk merged 1 commit into
mainfrom
infra/validate-clean

Conversation

@marc0olo

@marc0olo marc0olo commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

npm run validate (--all) reported 11 errors on main. This clears the 4 that are in hand-written pages. The remaining 7 are in a synced file and are tracked in #356.

  • 2 em-dashes replaced in prose (data-persistence.mdx, chain-key-tokens.mdx)
  • mo:base check corrected in scripts/validate.js

Why the mo:base check changed

checkForbiddenPatterns skips fenced code, so the old /mo:base/ pattern could only ever match prose. That is backwards for this rule:

  • an actual import Buffer "mo:base/Buffer"; lives inside a fence and was never flagged;
  • naming the legacy library in prose is legitimate and was flagged. Both hits on main were the base-to-core migration table in data-persistence.mdx ("Use these in preference to the legacy mo:base equivalents", and the Replaces (mo:base) column header), which is the same mapping AGENTS.md itself spells out.

The rule now matches the import path mo:base/ and opts into fence checking via a per-rule includeFences flag, so the other two patterns keep their prose-only behaviour.

Verified both directions:

$ node scripts/validate.js docs/__tmp-check.md
docs/__tmp-check.md: line 9: "mo:base/" import is banned — use "mo:core" instead

with the fixture containing an import inside a fence plus a prose mention; only the import is reported. A scan of docs/ confirms there are no mo:base/ imports being missed today.

Verification

npm run validate goes from 11 errors to 7 (all 7 in docs/references/internet-identity-spec.md, synced from dfinity/internet-identity, see #356). npm run build passes.

Merge order

These three are independent in intent but share one file, so they have a preferred order: #354, then #357, then #358.

PR Why here
1 #354 CLI docs links to the 1.3 slug Content only, no dependencies, and the smallest diff.
2 #357 clear the hand-written validate --all violations Touches scripts/validate.js, data-persistence.mdx, chain-key-tokens.mdx; no overlap with the other two. Merging it here drops validate --all on main from 11 errors to 7.
3 #358 .sources/ reduction Largest diff, and it shares docs/guides/canister-management/lifecycle.mdx with #354 (different lines: a CLI link there, a stale .sources/ pointer here). Last means at most one rebase, on a cleaner validate baseline.

The remaining 7 errors after #357 are all in docs/references/internet-identity-spec.md, which is generated by sync-ii-spec.mjs. They clear when dfinity/internet-identity#4284 merges and the next spec sync runs; that PR is independent of the order here.

@marc0olo
fix: clear hand-written validate --all violations, correct the mo:bas…
1d99b9e 
…e check

Replaces two em-dashes in prose, and fixes the mo:base rule so it matches
imports (mo:base/) inside code fences instead of legitimate prose mentions
of the legacy library in base-to-core migration tables.
@marc0olo
marc0olo requested a review from a team as a code owner August 26, 2026 11:22
This was referenced Aug 26, 2026
Closed
Merged
Merged
@raymondk
raymondk merged commit 8466502 into main Aug 27, 2026
7 checks passed
@raymondk
raymondk deleted the infra/validate-clean branch August 27, 2026 12:37
marc0olo added a commit that referenced this pull request Aug 27, 2026
@marc0olo
infra: three submodules, watched releases, registry-mirrored skills (#…
4174594 
…358)

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, 10 watched entries across 6 upstreams, 7
verified on demand, skills mirrored from the registry.**

## Kept, because the build opens them

| Submodule | Resolved by |
|---|---|
| `motoko` | synced pages, plus 52 `<motokoExamples>` includes |
| `internetidentity` | `sync-ii-spec.mjs`, which generates two reference
pages |
| `examples` | `remark-snippet.mjs`, which extracts `snippet=` code |

Thirteen go: eleven that existed only so an agent could check a fact,
plus `icskills` and `dotskills`, which fed the skills symlink chain. The
eleven appeared only in the `AGENTS.md` source table and
`.agents/submodule-bumping.md`, and CI initializes just `examples` and
`motoko`, 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:

| Watched | Pages | Hits | Also |
|---|---|---|---|
| `icp-cli` | 51 | 461 | the docs-site link slug |
| `motoko-core` | 63 | 328 | |
| `cdk-rs` | 47 | 336 | pinned versions in prose |
| `icp-js-core` | 15 | 47 | |
| `icp-cli-recipes` | 11 | 26 | every hit is a pinned recipe version in
prose; one entry per recipe |
| `icp-js-canisters` | 4 | 8 | a stated minimum version |

The other six became a `reference` group: `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 to `watched` is a judgment someone can re-make.

## Every upstream is now checked exactly once

| Upstream | Checked by | Produces |
|---|---|---|
| `motoko`, `internetidentity` | their existing sync workflows,
unchanged | a bump PR |
| `examples` | `upstream-releases.yml`, `vendored` | an issue when the
gitlink falls behind `master` |
| 10 watched entries | `upstream-releases.yml`, `watched` | an issue
when a newer release exists |
| 7 reference repos | nothing | nothing; verified against the latest
release on demand |
| 29 IC skills | `.claude/sync-ic-skills.sh` on session start | nothing;
not committed |

`examples` previously 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 carry `upstream-sync` plus 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.json` records 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.mjs`
writes an issue body per repo that moved (no token needed). `npm run
check:upstream` runs 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:

```bash
curl -sL https://raw.githubusercontent.com/dfinity/icp-cli/<pinned>/docs/reference/cli.md
```

`raw.githubusercontent.com` rather than `gh 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

| `track` | Latest from | Repos |
|---|---|---|
| `release` | git tags matching an anchored pattern | `icp-cli`,
`motoko-core`, `icp-js-core`, and one entry per `icp-cli-recipes` recipe
|
| `crate` | crates.io | `cdk-rs` |
| `npm` | npm registry | `icp-js-canisters` |

Seven defects found and fixed: three while building this, one from
Copilot, three from review:

1. **Lexicographic sorting reported downgrades.** These repos tag in
four shapes; as strings `0.9.4` beats `0.20.1`. Sorting is numeric-run
aware.
2. **`cdk-rs` was pinned to `0.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. A `release` pin that
is not among the matched tags is now a **loud failure**; every pin was
audited.
3. **`icp-js-sdk-docs` was the wrong repo entirely.** It is the
js.icp.build delivery site, so a commit meant the docs site rebuilt.
Replaced by `icp-js-core` (tags match npm) and `icp-js-canisters` (tags
lag npm by two majors, so npm is the signal). Sibling repos, different
correct answers.
4. **`icp-cli-recipes` monitoring was a no-op for four of five
recipes.** Refs compare by numeric run, so `rust-` against
`static-site-` compares textually and only the alphabetically-last
prefix could ever be reported, while the entry read as watched. Each
recipe now has its own entry, pin, issue and label, which immediately
surfaces four releases the docs are behind.
5. **The pin guard covered only the `release` track.** A `crate` or
`npm` pin in the wrong shape (`v0.20.1` for a crate that publishes
`0.20.1`) compared as older for ever. Registry pins are now checked
against the published version list, and crates.io reads
`max_stable_version` so a prerelease is not reported as the latest
release.
6. **`summarizeChange` counted by set membership**, so an added line
that already appeared elsewhere in the file read as unchanged. It now
counts occurrences: `cli.md` v1.1.0 to v1.3.0 reports 14 added rather
than 11.
7. **The `commit` track ordered short SHAs** with the version
comparator, so an update whose SHA happened to sort lower read as
"current". Demonstrated with pin `ef03698` against head `9f1e819`. SHAs
have no order, so it is now a prefix match, with the full SHA in links.

`commit` is 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: 9 of 11 have drifted

`icp-cli` v1.1.0→v1.3.0, `motoko-core` v2.4.0→v2.6.1, `cdk-rs`
0.20.1→0.20.2, `icp-js-core` v5.4.0→v6.1.0, `icp-js-canisters`
3.5.0→3.6.0, and four recipes: `asset-canister` v2.2.1→v2.3.0, `rust`
v3.3.0→v3.4.0, `motoko` v5.0.0→v5.1.0, `prebuilt` v2.0.0→v2.1.0. The
`static-site` recipe and `examples` are current.

Three carry real content risk: `motoko-core` (two minors of `mo:core`),
`icp-js-core` (a major bump, `Agent.readState` revamped), `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-cli` is 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.sh` on `SessionStart`, byte-identical to the
script `dfinity/examples` runs, 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) and
`vetkd` → `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`, and `technical-documentation`
stay committed. The last is not on the registry and changed once since
our pin, only to relicense `AGPL-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 root `NOTICE` records the
attribution.

## Instructions updated

`AGENTS.md` (source material, CLI verification, the `1.3` slug no longer
derived from a submodule pin, skills, key directories),
`.agents/upstream-tracking.md` replacing `submodule-bumping.md`, and
`CONTRIBUTING.md`.

## Worth challenging

- **The workflow itself has never run.** The script is exercised
locally; the Actions job (app token, label creation, supersede logic) is
not.
- **`examples` costs 265 MB** for one page today (6 snippets, 2
examples). Keeping it bets on `<CodeExample>` spreading.
- **`ic-cdk-timers` and `ic-cdk-executor` version separately** from
`ic-cdk` and are not tracked individually; the `affects` note tells a
reviewer to check them.
- **`setup.sh` now prunes stale `.sources/` directories.** An existing
clone keeps them after a submodule is removed, and a frozen
`.sources/icp-cli/` looks authoritative while being exactly what the
verify-at-the-pinned-ref rule exists to prevent. Only untracked
directories are touched.
- **The `reference` group 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.
- **The three committed skills are watched by nothing.**
- **The non-Claude skill path is an instruction, not a mechanism.**
Claude gets skills regardless; every other harness only by compliance.

## Found along the way, not fixed here

`ic-cdk` is pinned to four different versions across the docs: `0.19` in
8 places, plus `0.20`, `0.17`, and `0.17.1`. The published crate is
0.20.2. That is a content bug rather than infrastructure, so it belongs
with the `cdk-rs` watch issue rather than in this PR.

## Follow-up

#279 is unblocked: of the 11 repos its "keep as-is" bucket names, only
`examples` is still a submodule, and `affects` now 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-cli` and `motoko-core` as
their own PRs.

## Merge order

These three are independent in intent but share one file, so they have a
preferred order: **#354, then #357, then #358.**

| | PR | Why here |
|---|---|---|
| 1 | #354 CLI docs links to the 1.3 slug | Content only, no
dependencies, and the smallest diff. |
| 2 | #357 clear the hand-written `validate --all` violations | Touches
`scripts/validate.js`, `data-persistence.mdx`, `chain-key-tokens.mdx`;
no overlap with the other two. Merging it here drops `validate --all` on
`main` from 11 errors to 7. |
| 3 | #358 `.sources/` reduction | Largest diff, and it shares
`docs/guides/canister-management/lifecycle.mdx` with #354 (different
lines: a CLI link there, a stale `.sources/` pointer here). Last means
at most one rebase, on a cleaner validate baseline. |

The remaining 7 errors after #357 are all in
`docs/references/internet-identity-spec.md`, which is generated by
`sync-ii-spec.mjs`. They clear when
[dfinity/internet-identity#4284](dfinity/internet-identity#4284)
merges and the next spec sync runs; that PR is independent of the order
here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@raymondk raymondk raymondk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marc0olo @raymondk