chore(repo): add author-migration skill - #36413
Merged
Merged
Conversation
Contributor
|
View your CI Pipeline Execution ↗ for commit 247372e
☁️ Nx Cloud last updated this comment at |
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
polygraph-app
Bot
force-pushed
the
nxc-4618
branch
from
July 21, 2026 12:16
aea4062 to
861af83
Compare
leosvelperez
force-pushed
the
nxc-4618
branch
2 times, most recently
from
July 23, 2026 07:09
1db4a55 to
71f39c0
Compare
Authoring a first-party migration involves conventions that are easy to get wrong and expensive to discover from source each time: entry naming and versioning, the migrations.json runtime contract (version gating, requires, implementation resolution, prompt/documentation handling), packageJsonUpdates group semantics, return-value channels, idempotency requirements, and the colocated file set including agentic prompts. The skill encodes those conventions with pointers to the source of truth and live exemplars, plus templates for the migrations.json entry, spec file, agentic prompt runbook, and documentation file, and a pre-PR checklist. It triggers when writing or editing migrations, packageJsonUpdates entries, migration prompts or docs, porting upstream framework migrations, or deciding whether a breaking change needs a migration.
…sting The skill was exercised against four shipped-migration scenarios (swc-cli bump, typescript-eslint v8 rules, Angular 22 buildable workspace, jest verify-typecheck), re-run after amendments, and compared against no-skill control runs in history-isolated clones. The gaps that surfaced are folded back into the guidance: - requires cannot express an OR of package names; recognize umbrella vs scoped installs and gate in code (paired packageJsonUpdates groups). - Preserve jsonc comments with jsonc-parser instead of readJson/updateJson, and wire new runtime deps into package.json and ng-package.json. - Edit only files that locally declare an inherited setting; never mutate shared bases from one consumer's resolution. - Install new deps at the generators' versions-constant, not the workspace's installed version. - Match backtick-quoted object keys in codemods. - Mandatory spec fixtures: adjacent multi-edit offsets and local-overrides-inherited precedence. - Re-check .md claims against the final implementation and verify tool-behavior claims from source (also a checklist item). - Build tree paths with joinPathFragments/posix helpers. - Bump the repo's own pin when a migration bumps a package the repo uses. - Handle every extends form the host resolver accepts (string, array, package specifiers, cycles). - Scope discovery by project graph only for executor-derived file sets; scan the tree for name-matched user configs. - Prompt runbooks must cover the bare no-options form.
A source-check of the skill against the migrate runtime and the cited migrations surfaced two exemplar citations that contradict the rules they illustrate and two runtime-contract claims that drift from the implementation: - annotate the vite dual-world exemplar: it returns a GeneratorCallback and uses a bare entry key, both of which the canon forbids; copy only its scan - swap the storybook version-admission exemplar from the 23.1.0 group (a within-major ceiling raise) to 22.1.0 (a true major admission) - state the real install cadence: once after the run by default, per migration only under --create-commits and agentic runs - attribute runner selection to the run-time re-read of the unmerged migrations.json, not the fetch-phase fold
…nario testing Four new test scenarios surfaced gaps the previous rounds missed: - documentation .md is required on every entry, prompt-only migrations included; a prompt .md never doubles as documentation - compatibility claims come from published package metadata (peerDependencies, engines), not upstream prose - codemods must enumerate hand-written authoring shapes and classify files by resolved import bindings, not substring matches - handles/covers claims in .md files need a backing spec case - requires bounds asymmetry explained via the collection-time mechanism: entries gate destination-only (>=N), groups always carry both bounds
…ven and codemod-first
… skill The skill called the deterministic half of a migration a codemod, a term the Nx docs and the migrate source never use for it: the documented vocabulary is migration generator, with generator-only, prompt-only, and hybrid as the kinds, and codemod in the repo refers to external npx-runnable tools. Rename accordingly across SKILL.md, the templates, and the runtime contract. Codemod stays only for upstream tools (the Run upstream codemod treatment and the runbook rule), the old Codemod table row and section become Source transform (the kind names would over-claim there, since config edits and conditional dep changes are also migration generators), and one gloss binds the general-usage term to the documented one.
…t rules Round-7 validation runs (jest, maven, nest, devkit scenarios) surfaced gaps the skill text either caused or failed to prevent: - forbid inferring a packageJsonUpdates group's requires posture from sibling groups; re-derive per admission (the ungated rspack 21.4.0 group next to its gated react twin is the counterexample) - relax the entry-key canon to a soft convention: keys stay unique and slug-bearing but follow the file's dominant form; document that JSON.parse silently drops the earlier of two duplicate keys and that a version part in the key is a release-level hint, not tied to the version field - tighten the version ask-step: a re-stamped version is still a train choice, and a run counts as non-interactive only when there is no channel to ask - add migration-homing guidance and the collection-scope contract (installed versions resolve by node resolution from the workspace root) - require materialized option values to land where the tool reads them, never as undeclared target options riding executor passthrough - add two mandatory spec rules: a drift check for frozen copies of another module's canonical set, and case-by-case coverage when a migration statically replicates deleted runtime behavior
…view A ten-reviewer audit of the skill (correctness against the migrate runtime, internal coherence, published skill-authoring guidance, token efficiency, steering quality) surfaced a set of verified issues: - fix three stale or wrong facts: assertValidMigrationPaths now has a file-granular orphan check (only .md files and helpers escape it), the generated migrations file carries the full entry rather than package+name, and devkit glob is deprecated in place, not deleted - close checklist and template drift: the pre-PR checklist now covers all seven mandatory spec cases, first-migration wiring, and the shipped-fix version re-stamp; the spec skeleton points at the single mandatory-case list instead of restating a subset; the prompt-only entry template gains the documentation key it was contradicting the canon by omitting - state where entries live (the generators section) in SKILL.md and the template, and show the full new-file shape - widen the treatment table's Nothing row to match the prose, add the ported-vs-source-transform tiebreaker, cover the next==latest train case, and give the between-branches classification gap a default - sharpen undecidable wording (inferred-plugin scan trigger, generator-schema check for keepExistingVersions/skipFormat, ifPackageInstalled as a live but unused runtime gate) and align terminology (packageJsonUpdates group, target train) - trim restated facts (landing-version semantics, tree-util import note, cli/schema bullet) and rephrase the rspack ungated-group citation as a historical fact so an upstream fix does not falsify it
…r-migration skill
The runtime contract missed the agentic validation flow: generator-only
migrations that produce changes get an agent validation step that receives
the captured generator output and any returned agentContext, and may apply
minor in-scope fixes. The skill now steers every migration with skipped or
residual work to return { nextSteps, agentContext }, not just hybrids, and
documents that nextSteps never reaches the agent while agentContext also
surfaces on stdout when running inside an outer agent.
Prompt-only entries get no spec file (nothing imports a prompt .md and no
harness exercises prompt content); the spec canon, checklist, layout, and
spec skeleton now say so. Every generators entry ships a documentation .md
set on its documentation key, and the docs-site wording follows the
key-driven rendering (nothing inferred from the implementation basename),
which also retires the docs-site-leak rationale for distinct prompt
filenames in favor of the documentation file owning the implementation's
basename.
Also corrects the overbroad claim that path validation misses any wrong
dist shape (it fails shapes whose stripped path misses the source tree)
and notes that prompt-runbook exemplars vary their heading names.
…migration skill The skill restated checks the repo validators enforce: path resolution, orphaned files, duplicate entry keys, the published dist shape, and packageGroup membership. Section 5 now instructs running the validators (plugin test/lint plus the conformance check) and the pre-PR checklist keeps only the judgment items no validator covers, such as whether a path names the right file, train and `requires` semantics, spec coverage, and .md claim accuracy. Target-train arithmetic moves from prose into scripts/compute-target-versions.mjs, anchored on `npm view nx dist-tags` alone; the git tag listing added nothing the dist-tags do not carry. The script prints each candidate train with the recommended default marked and guards the mid-promotion window where a stable `next` sits above `latest`. The local-registry walkthrough now includes the `npm adduser` login step from CONTRIBUTING.md, and the `nx migrate` invocations run through `npx`.
…gration skill The spec skeleton's idempotency example had no arrange step, so both reads returned null and the comparison passed for any non-idempotent migration; the negative test used an undeclared variable and the positive test carried only commented-out assertions. All three tests now run as written: a shared arrange writes the trigger shape, the negative test declares its fixture, and the idempotency test anchors on a non-null read before comparing runs. compute-target-versions.mjs now fails closed with the hand-compute message on npm failures, non-JSON or null payloads, missing tags, and a prerelease latest (which previously produced a backdated major option), with a 30s timeout on the registry call. The comment claiming semver resolves from the working directory was wrong (ESM resolves from the script's own location) and is corrected along with the SKILL.md clause. Guidance corrections: the fail-open canon distinguishes unparseable files (skip and report in agentContext) from unrecognized ones (skip silently) and the malformed-input spec case asserts the report; three cited exemplars get caveats for their utils/versions imports; section 3 states entry values use the published dist-prefixed shape; the agentContext note matches the runtime contract's outer-agent behavior; section 5 cites the migration-groups conformance rule; the frontmatter declares allowed-tools like the sibling skills.
The multi-version-compliance skill prescribed bilateral (>=N <M) requires ranges at the migration-entry level and condemned all in-body version guards. Both are wrong against the runner: entry gates evaluate once at collection time against the version the package lands on in the same run (installed only as fallback), so a bound encoding the source window skips exactly when the run bumps past the cap (the shipped storybook skip bug), and an OR of alternative package names cannot be expressed in requires at all (the shipped eslint fix moved that check into the migration body). Scope the bilateral default to packageJsonUpdates windows, prescribe destination gating for migration entries (lower bound alone unless the migration is inapplicable at or above a cap, like the next-15 instructions entry), carve the inexpressible-OR case out of the in-body anti-pattern, and refresh the stale reference to the pruned update-typescript-eslint-v8.13.0 entry. The author-migration skill's own "lower bound only" rule gets the same upper-bound nuance, and the two skills now cross-link.
…tion skill The migrations-json template opened with "replace the placeholders" while its JSON blocks use concrete example values and no placeholders at all; the intro now states that values are illustrative and the key sets and path shapes are the contract. alwaysAddToPackageJson appeared in both packageJsonUpdates examples unexplained, and the documentation template never said the sample-code-changes section can be omitted for changes with no code shape. Migration documentation renders on nx.dev but sits outside every docs validation: vale's scope is astro-docs/src/content only and the site synthesizes migration pages from the colocated .md files at build time without linting them (a shipped doc already fails the Nx.Headings sentence-case rule). The documentation template's own Sample Code Changes heading taught that violation; it is now sentence case, and the template and section 6 point at the docs style guide with a note that vale does not reach these files today.
…ngs in author-migration skill The pre-PR checklist required "no upper bound on the entry's own gate", contradicting the section 2 rule that permits one when the migration is inapplicable at or above the cap (the next-15 instructions gate is a valid instance). The checklist item now names the source-window case as the defect and defers to section 2 for the valid exception. The packageJsonUpdates ladder rules only covered rungs shipped together in oldest-source-first key order. A rung backfilled at a later nx version sits after the existing upper rungs, so its cohort lands on the intermediate major with the upper groups permanently behind it. Document the choice between re-offering the upper rungs (with landing-gated companion entries re-stamped) and deliberately holding the cohort.
FrozenPandaz
approved these changes
Jul 24, 2026
FrozenPandaz
pushed a commit
that referenced
this pull request
Jul 29, 2026
## Current Behavior There is no repo guidance for authoring migrations. Each new migration is written by copying whatever sibling looks closest, which reproduces stale patterns (wrong dist path shapes, backdated versions, missed packageJsonUpdates groups) and misses conventions that only live in reviewers' heads. ## Expected Behavior A Claude Code skill at `.claude/skills/author-migration/` covers the authoring flow end to end: decomposing a change into migration needs, version and `requires` gating, scaffolding, implementation canon (codemods, config edits, dependency updates, prompt and hybrid migrations), spec requirements, docs, and a pre-PR checklist. Companion files document the `nx migrate` runtime contract, deprecated patterns with recognition signatures, and entry/spec/doc templates. The guidance was validated through scenario testing: child agents authored migrations at the parent commits of 16 shipped migration PRs (plus a no-skill control round), their output was graded against what actually shipped, and the skill was amended after each round. <!-- polygraph-session-start --> --- [View session information ↗](https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/nxc-4618-98faa68a) <!-- polygraph-session-end --> (cherry picked from commit 205a32b)
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
2 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.
Current Behavior
There is no repo guidance for authoring migrations. Each new migration is written by copying whatever sibling looks closest, which reproduces stale patterns (wrong dist path shapes, backdated versions, missed packageJsonUpdates groups) and misses conventions that only live in reviewers' heads.
Expected Behavior
A Claude Code skill at
.claude/skills/author-migration/covers the authoring flow end to end: decomposing a change into migration needs, version andrequiresgating, scaffolding, implementation canon (codemods, config edits, dependency updates, prompt and hybrid migrations), spec requirements, docs, and a pre-PR checklist. Companion files document thenx migrateruntime contract, deprecated patterns with recognition signatures, and entry/spec/doc templates.The guidance was validated through scenario testing: child agents authored migrations at the parent commits of 16 shipped migration PRs (plus a no-skill control round), their output was graded against what actually shipped, and the skill was amended after each round.
View session information ↗