The removal guard compares a regeneration against the checked-in
versions.json and refuses to write when any skill disappears. That is
right for the case it was built for -- cuopt-multi-objective-exploration
vanishing on 2026-08-03 while still registered -- but it also fires on
the routine case of a team retiring a skill.
It fired today. Isaac for Healthcare deregistered five catheter skills
in c3168ca, the sync pruned them in #538, and the regeneration refused.
The workflow never passes --allow-removals and has no input to set it,
so there is no way to record that a removal was intended. Because this
is the last generation step, the run exits non-zero and the create-PR
step is skipped, stranding metadata.json, skills.sh.json and
benchmarks.json even though those were generated correctly.
It also cannot self-heal: the comparison is against a checked-in file
that can only be updated by the regeneration that keeps failing, so
every subsequent hourly run fails identically and re-comments on the
tracking issue.
Registration separates the two cases exactly. A skill absent from
components.d was retired on purpose; a skill still registered that
disappears anyway is the failure the guard exists to catch. This is the
same distinction PR #522 drew in aggregate_benchmarks.py for the same
class of false alarm, and it reuses that helper rather than restating
the rule.
An empty registry is treated as a parse failure, not as "nothing is
registered", so a bad parse cannot silently disable the guard.
--allow-removals is unchanged as a manual override.
Verified against main at 0b26974: the real case now writes 346 skills
and names the five retired ones; hiding a still-registered skill still
exits 1. 27 tests pass.
Signed-off-by: mosheabr <moshea@nvidia.com>
Consumers that mirror skills have no way to learn a mirrored skill
changed. The catalog has no releases or tags, metadata.json carries no
version or digest, and the README's Version column is markdown and
per-component. Detecting that 4 of 30 mirrored skills moved means
walking 343 directories and diffing them.
Add versions.json at repo root: one entry per skill with a content
digest, the last catalog commit touching it, and that commit's date.
A consumer stores the digests it mirrored and compares on refresh.
The digest comes from each skill's signed manifest, not from the
upstream sha the sync observed. When content changes without a
signature refresh the sync reverts the skill and the old content stays
published; keying on skill.oms.sig describes what is published, so a
reverted update never surfaces as an update. Coverage is 343/343 and
every manifest's subject name matches its directory.
Sorting the file list before hashing keeps the digest stable across a
re-sign that shuffles manifest order. A NUL between name and digest
stops two different file lists serializing identically.
Removals are gated: if a skill disappears from a regeneration the
generator refuses to write unless --allow-removals is passed. A
generated file staying schema-valid with one fewer entry is how
cuopt-multi-objective-exploration silently vanished from metadata.json
on 2026-08-03.
Kept separate from metadata.json deliberately. That file is taxonomy,
changes rarely, and is partly AI-enriched; this is mechanical and
changes hourly. Separating them stops taxonomy churning on every sync,
keeps a provenance bug away from the marketplace index, and keeps AI
out of the path for data that has nothing to do with it.
The check job's checkout gains fetch-depth: 0 — the commit fields read
git log per skill directory, which a shallow clone cannot answer.
22 unit tests cover digest stability, order independence, separator
collisions, removal detection and schema closure.
Refs #491
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
The gate existed to stop a skill passing onboarding and then being
dropped by the hourly sync, but it diverged from that sync in both
directions.
Card: the sync requires skill-card.md by name. Accepting SKILLCARD.yaml
or card.yaml let a skill clear onboarding that the next sync would
silently remove. Both spellings do occur in the catalog, but always
alongside skill-card.md, which all 350 published skills carry -- no
skill relies on an alternate.
Evals: the sync accepts evals.json at any depth, or any *.json under
evals/ or eval/. Requiring evals/evals.json exactly would have blocked
rag-blueprint, rag-eval and rag-perf, which ship eval/*.json and which
the sync carries today.
Verified against the catalog: all 350 skills pass the corrected rules.
Signed-off-by: mosheabr <moshea@nvidia.com>
An onboarding PR adds components.d/<slug>.yml pointing at skill
directories in a product repo, and nothing on the PR looks at them.
verify_content_integrity.py checks this repository's tree, which is
sound; the content being onboarded lives elsewhere and goes unexamined.
The PR goes green on evidence unrelated to what is being merged.
The cost lands after the merge. The sync recomputes each signed file's
digest and refuses the skill on any mismatch: an existing skill reverts,
a new skill is dropped outright, having no earlier version to fall back
to. The PR is closed by then, so the failure surfaces nowhere — the
catalog looks correct and the skill is simply absent.
That happened twice on 2026-08-31. paidf-augmentation (#501) merged
green and was dropped by the next sync; paidf-orchestration (#507)
carried the same defect in all four declared skills and was caught only
because someone verified the signatures by hand during review. In every
case the file was BENCHMARK.md, regenerated after the signing run so the
signature covered an earlier copy.
Add a check that follows the pointer. For each declared path it fetches
the source at the component's ref and verifies that signed files still
hash to their signed digests, that the required artifacts exist, and
that BENCHMARK.md reports PASS backed by real measurements.
Blocking scope is limited to paths the PR adds. Around 30 catalog skills
already carry drift (#216, #357); failing a PR for drift its author
neither caused nor can fix would make the gate an obstacle rather than a
signal. Pre-existing paths are still checked and reported, never fatal.
The content check reuses verify_content_integrity.verify_skill rather
than reimplementing digest comparison, and the benchmark check reuses
aggregate_benchmarks.parse_benchmark, which already handles the v1/v2/v3
report layouts — matching the verdict by hand reads v2's methodology
bullet as a passing verdict on every v2 report.
Verified against the live repositories: replaying paidf-orchestration at
5e4fd6b, its state during #507's review, flags BENCHMARK.md on all four
skills and nothing else, reproducing the table in #508.
Refs #508
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Self-review of the exemption found two ways it removed more than the
orphan it was aimed at.
Keyed by skill name, the exemption could take a live skill with it. A
rename leaves the old and new dirs carrying the same skill name in their
reports, so the name is not a key: the two entries already overwrote each
other when building the comparison, and excluding the orphan by name
excluded the registered dir too. Both dirs sit on disk together whenever
prune-orphans hits PRUNE_CAP and deletes nothing, so this is reachable,
not theoretical. Keyed by catalog_dir it is not — dir names are unique by
construction.
With no registrations parsed, every dir looked like an orphan. A single
malformed components.d file would have exempted the whole catalog and
disabled the guard everywhere. prune-orphans already refuses to delete
anything when the declared set comes back empty, for exactly this
reason; this mirrors that rail.
Both cases now have tests, and both tests fail against the previous
commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
`component` is the one field in benchmarks.json not parsed from a
BENCHMARK.md. It is joined in from components.d at generation time, so it
moves the instant a PR edits a registration — while benchmarks.json
cannot be brought up to date until the sync has renamed the directories
on disk. A components.d-only PR is therefore always transiently stale in
that field, by construction.
Both halves of the tool treated that transient as a defect, and together
they formed a deadlock. #519 renamed catalog_dirs — a components.d-only
change by design, since the sync writes the renamed dirs and
prune-orphans deletes the old ones:
* --check regenerated, saw component values move, and failed;
* the remedy it printed was to regenerate, which hit the null guard:
the deregistered dirs were still on disk, now matched no component,
and went null — "component: 0 -> 2 nulls";
* so the check demanded a regeneration the generator refused to run.
There was no state of that PR both correct and green. The only escape
was hand-deleting the catalog dirs, which then required a signing run
that cannot fire on a fork PR. The second gate existed only because the
first forced the author into it, and it blocked a release for a day.
Two changes, one idea — a stale join is not lost data:
* --check ignores component when comparing, and says so. Every other
field, and the set of skills present, still must match exactly.
* The null guard excludes dirs no longer registered anywhere. They are
orphans that prune-orphans deletes in the next sync commit; their
component is null because the deregistration worked.
Registration is read with a dedicated helper rather than
load_component_map(), which only sees entries declaring a component name.
An exception listed without one would have read as an orphan and quietly
lost the guard's protection.
The two existing guard fixtures registered no component at all, which
made their skill an orphan under the new rule. That fixture never
matched reality — an unregistered dir is pruned by the next sync — so
they now register it. They assert exactly what they did before.
Verified by replaying #519's tree: --check passes with a note,
regeneration succeeds, and a genuine null regression on a registered
skill still refuses to write.
Refs #519
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
benchmarks.json, metadata.json, skills.sh.json and the skills/ tree are
generated by the sync and the metadata bot, not maintained by hand. They
were edited here only to get past the Generate Skill Metadata check,
which fails on a components.d-only rename for reasons unrelated to this
change:
aggregate_benchmarks.py --check validates benchmarks.json against the
skills/ dirs present on the PR. The sync has not run on a PR, so those
dirs still carry their old names. Deregistering the old catalog_dir
leaves them matching no components.d entry, their component field goes
null, and the null guard then refuses to regenerate:
Refusing to write benchmarks.json: fields lost values.
component: 0 -> 2 nulls
The check says regenerate; the generator refuses to. Deleting the old
dirs by hand was the only way out, and that in turn required a signing
run (require-nvskills-ci), which cannot fire on a fork PR.
Restoring the generated files leaves this PR as the two components.d
edits it should be. skills/ is untouched, so the signing requirement no
longer applies. The metadata check stays red and is bypassed knowingly;
after merge the sync renames the directories, prune-orphans removes the
old ones, and the metadata bot regenerates benchmarks.json.
The null guard is fixed separately so the next rename does not hit this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
The null-rate guard has been failing metadata regeneration since
2026-08-28, filing into #489 and leaving benchmarks.json stale.
pass_threshold_pct is parsed from the "- Pass threshold: N%" line that
v1/v2 cards carry and v3 cards dropped. Scraping the v3 glossary
sentence instead is not an option -- it is byte-identical across skills
with 4, 5 and 18 tasks, so it would stamp a fabricated value on every v3
skill. The field is therefore known to be drifting to null, one skill at
a time, as teams re-run CI.
That makes the guard fire on an expected transition rather than on a
parser break: it tripped on cuopt-server-api-python and would trip again
on each of the ~100 skills still carrying a v1/v2 value.
Add MIGRATING_FIELDS and filter at the call site, so
null_rate_regressions stays a faithful reporter and the drift is still
printed -- just not treated as blocking. Every other field stays
guarded, including when it regresses in the same run.
Not using --allow-null-regressions: that disables the guard entirely,
and the guard is what caught the profile regression in #485.
Remove the exemption once SkillEvaluator emits the threshold as a real
per-run field and the parser reads it again.
The CLI guard tests probed with pass_threshold_pct, which would have
made them pass for the wrong reason after this change. Repointed to
environment and added coverage for the exemption: that it does not
block, that it does not mask a co-occurring regression, and that the
drift is still reported.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
The TAO re-sign in #482 materially changed skills/tao-run-automl, so the
checked-in metadata.json drifted from its source. Regenerated with
--no-ai to match what the check validates.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
SkillEvaluator 0.2.1 redesigned BENCHMARK.md and stopped emitting two
fields the aggregator read. The metadata job has failed on every sync
since TAO's 37 re-signed skills landed in #482, because the null-loss
guard added in #468 correctly refused to write the result.
The two fields need different answers:
profile — retired. v1/v2 emitted '- NVSkills-Eval profile: external'.
NVSkills-Eval is an internal name and does not belong in a published
report, so v3 dropping it was right. Carrying a permanently-null column
named after an internal tool is worse than removing it.
pass_threshold_pct — kept, and left null on v3. It is a real provenance
value that v3 turned into template prose: the same '50%' sentence
appears on skills with 4, 5 and 18 tasks, so scraping it would stamp one
number on every skill regardless of what it was evaluated against. The
existing NO_FABRICATION note already says so. Asking SkillEvaluator to
emit it as a real field again is tracked separately.
benchmarks.json regenerated once with --allow-null-regressions to land
both changes. A strict --check passes against the result, so the guard
stays armed for the next format change.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Reword the onboarding affirmation and the CONTRIBUTING license section
to state what contributors must confirm without describing team-internal
review mechanics. Replace an internal link in the content-integrity
failure message with a pointer to CODEOWNERS, and refresh two stale tool
names in comments.
Verbiage only, no functional change:
- the PR template is not machine-read by any workflow
- all four touched YAML files parse clean
- every run: block in both workflows still passes bash -n
- the DCO v1.1 (a)-(d) text in CONTRIBUTING.md is untouched
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
SkillEvaluator's report format changed in July 2026. It stopped emitting
`NVSkills-Eval profile` and `Pass threshold`, both of which the aggregator
reads, so those columns in benchmarks.json have been emptying one skill at a
time as each is re-evaluated: 0% null through June, 57% in July, 100% for
every skill evaluated in August. Nothing failed. The regeneration succeeded,
the schema stayed valid, --check passed, and the data quietly degraded for
six weeks before anyone noticed.
Neither field is recoverable from a v3 report. `profile` is gone entirely.
The only "50%" left is a glossary sentence that is byte-identical across
skills with 4, 5 and 18 tasks, so it is template prose rather than a
measurement — scraping it would stamp an unverified pass threshold on every
skill in the file whose purpose is recording provenance. Both stay null until
SkillEvaluator emits them as real fields; a NO_FABRICATION note records why.
What v3 does add is stronger provenance the parser was discarding:
- evaluator_version which build produced the score
- dataset_digest sha256 of the exact task set, unique per skill
- validation_status pipeline outcome for the run
Capturing these follows the existing multi-pattern idiom already used for
`tasks`, so v1/v2 reports are unaffected and simply leave them null.
The generic fix is null_rate_regressions(), which compares a regeneration
against the committed benchmarks.json and refuses to write when any field
lost values, counting only skills present in both so new additions cannot
false-fire. --allow-null-regressions lands a deliberate retirement without
requiring a code edit under pressure. Verified against the real catalog: it
does not fire today, it does fire when the July drift is replayed over 20
reports, and it flags the exact fields lost in PR #466.
Adds the first tests for .github/scripts (16 cases, two real BENCHMARK.md
fixtures, one per format). Confirmed no regression: every pre-existing field
is byte-identical across all 341 skills.
Also adds the SPDX header aggregate_benchmarks.py was missing; it was the
only .py in .github/scripts/ without one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
The --no-ai case asserted on main()'s bare exit code. --check returns 1
for output drift as well as for skill warnings, so any PR that edits
skill content failed this test for reasons unrelated to enrichment.
PR #445 is the first to hit it: 70 changed files including SKILL.md at
62+/147-, which legitimately drifts the checked-in metadata.
Neutralise diff_text for the duration of the run so the exit code is
attributable to the warning gate alone. Drift is already covered by
case 4.
A first attempt simply excused drift with 'rc == 0 or drifted'. That was
worse than the original bug: removing the --no-ai gate entirely still
passed, because drift was present. Mutation-checked properly this time.
Verified against PR #445's content: reproduces the failure without this
change, passes with it, and still fails when the --no-ai gate is removed
-- on both #445's tree and a clean one.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
The previous commit made any unbuilt skill non-zero in both modes. That
is wrong for --no-ai: PR CI runs without an inference key by design, so
every newly synced skill arrives there unenriched and would red-line all
open PRs until the regenerate job ran. Caught by CI on this branch --
three freshly synced hsb-ip skills failed the check step.
Gate the non-zero exit on AI being available. Warnings still print in
both modes; a skill that cannot be enriched WITH AI available still
fails. Adds test cases pinning both halves so neither can regress.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
The recovery and reporting paths only execute when a skill cannot be
built, so a healthy catalog never exercises them and a regression stays
invisible until it drops a skill from the published catalog.
- add test_generate_skill_metadata.py: induces enrichment failures against
a stub inference API, no network, nothing written to the repo. Covers
amendment-failure-keeps-entry, amendment-success, unbuildable-new-skill
omitted-and-warned, and unbuildable-published-skill recovered
byte-identically. Fixtures are selected from real metadata.json entries
at runtime so a rename or retirement does not break them.
- wire it into the PR check job, ahead of the byte-stability check.
- the issue-body script still read warnings.omitted_skills, which the key
rename left dangling; it would have silently reported zero affected
skills on every partial run. Read stale_skills, and correct the copy:
recovered skills are stale, not excluded.
Mutation-checked: disabling recovery fails 3 assertions, routing
amendment failures back into skill_warnings fails 2.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Moving the warning block above the --check early return dropped its
return 1, so a run that omitted a skill would have exited 0. Set the
code once and return it from both the --check and write paths: outputs
are still written so the catalog does not lose a skill, but CI cannot
read a partial success as a pass.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Addresses review feedback on #408.
- workflow: set -o pipefail. Piping the generator into tee reported tee's
exit status, so a failing generator was read as rc=0 and the failure
path never ran. This is what let the original silent drop through CI.
- generator: when a skill cannot be rebuilt, reuse its last-good entry
from the checked-in metadata.json (schema-validated first) instead of
dropping it. Only entries with no valid prior are omitted, and those
are the only ones that fail the run.
- generator: an amendment failure on an already-complete entry is no
longer reported as an omission. The entry ships unchanged with its
existing values, so it is recorded as a retained notice, not a warning.
- generator: print retained notices and stale-skill warnings before the
--check early return, so PR CI surfaces them too.
- rename the warnings JSON key omitted_skills -> stale_skills to match
what the entries now mean.
Verified with an induced-failure harness driving a stub inference API:
11/11 assertions pass (entry kept + values preserved on amendment
failure; byte-identical recovery from baseline; genuinely-new unbuildable
skill still omitted and still fails the run; published skill unbuildable
-> catalog written, not delisted, failure surfaced).
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
A skill classified materially_changed is sent back to the AI client to see
whether its existing metadata values should be amended. Every required
field already holds a valid value at that point, so the amendment is
optional — but an EnrichmentError returned None, dropping the skill from
metadata.json and skills.sh.json entirely.
The generator does report this, printing a PARTIAL SUCCESS block naming the
skill. What it does not do is stop: it exits 0, writes the reduced file, and
the regeneration PR opens as normal. The output is schema-valid and
internally consistent, so --check passes and the round-trip validation is
handed the missing path as skipped_paths.
This happened on 2026-08-03 to cuopt-multi-objective-exploration, legitimately
materially_changed after the 15:04 sync. The enrichment call returned HTTP 403
three minutes after the identical call had succeeded; 403 was not in the
retryable set, so it was terminal. Run 30842167516 emitted 323 skills, run
30842382996 emitted 322 and opened a PR deleting the entry.
Three changes:
- On enrichment failure in the amendment path, keep the existing metadata and
record a warning instead of returning None.
- Fail the run if a discovered skill produces no entry, when that skill was in
the previous metadata.json or when AI enrichment was available. Skills
dropped via metadata-exclusions.yaml never reach this point. Because the
workflow opens its validation issue on a non-zero exit, this also gives the
case an issue without new wiring.
- Retry 403 alongside 429 and 5xx. The gateway returns 403 for transient edge
rejections as well as real permission failures; retrying costs ~7s on a
genuine failure and saves an enrichment on a transient one.
Verified against the real catalog with a stubbed API, main vs this branch:
403 then 200 before: terminal after 1 call
after: recovered on call 2, 5 fields returned
persistent 403 before: 1 call, no "attempts" in message
after: 4 calls, terminal, message cites attempts
amend-path failure before: entry dropped
after: entry kept, 5 metadata fields intact
forced drop, write before: "updated (322 skills)", exit 0
after: VALIDATION FAILED naming the skill, exit 1
Normal run unchanged: metadata.json unchanged (323 skills), both drift gates
exit 0.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Two corrections, neither of which had been reflected in the generated
files:
metadata.json picks up the cuopt-multi-objective-exploration description
reworded in the 8/3 re-sign.
benchmarks.json is regenerated against 937d0e3, which made "±0 points"
parse as 0.0 rather than None. Dimensions with no change had been
dropped from the average-uplift denominator, so every skill carrying a
v2 report was reporting an inflated figure. Six skills move, all
downward and all more accurate:
nemo-automodel-distributed-training 40.88 -> 32.7
nemo-relay-plugin-observability 38.0 -> 34.2
nemo-relay-get-started 36.89 -> 33.2
nemo-relay-debug-runtime-integration 30.0 -> 24.0
cuopt-multi-objective-exploration 29.3 -> 28.2
portfolio-optimization 19.0 -> 17.1
Skill count and row count are unchanged at 323 and 3000.
Signed-off-by: Moshe Abramovitch <moshea@nvidia.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mosheabr