feat(skills): add migrate skill and plugin manifest - #16
Conversation
ms-shashank
left a comment
There was a problem hiding this comment.
I have reviewed this against SKILL_STYLE.md. It is close, and one rule in the body works against rule 5. That is the only thing I want changed.
First, the CI red is not yours. The check job has been failing on every run in this repo, including the pushes to main after #13 and #15 merged, and the failing job reports zero steps and has no log at all. That is the Actions billing on the org, not your branch. Do not chase it. I raised it and it is on me to get it cleared.
What is right. tools: full matches the method, which mutates files, runs codemods and runs test suites. That is the one you and I got wrong once before, so I checked it first. workspace: required, capability: Coding with no provider named, order: 70 continuing the increment of ten, and command: migrate colliding with nothing already merged. All ten frontmatter fields are present, which is what the strict check wants. The registry commands per ecosystem, the file:line citation requirement, the codemod-first rule and the paired before-and-after counts are all disciplines rather than advice: I can look at an output and say whether each was followed.
The unverifiable-network sentence is exactly rule 1 done properly, because you gave the literal line to say rather than telling the model to be honest.
The one change. This, in the baseline section:
If any test fails before starting, STOP. Never start a migration on a failing baseline. Report the failure and refuse to proceed until the baseline is green or pre-existing defects are resolved.
Rule 5 says a skill must never become an excuse to refuse work or punt back to the user. A repository with three unrelated failing tests is the normal case, not the exception, and this rule makes the skill refuse the entire migration and send someone off to fix tests that have nothing to do with the dependency they asked about. It is the rule 5 failure in its clearest form.
It also is not needed for the discipline. The point of a paired run is that you compare two sets. A baseline of "38 passed, 3 failed, and here are the three names" is a perfectly good baseline, and equivalence then means those same three fail afterwards and no new ones do. That is a stronger claim than "everything was green twice", because it survives the repositories people actually have. OxCode's own verifier works this way: it captures a baseline carrying failingNames and computes regressions against it rather than refusing to run.
Suggested replacement, and take the wording where you like it:
Record the baseline as a set, not a verdict. Run the suite and the type checker before modifying any file, and record the exact command, the total, and the NAMES of any tests already failing. A repository with pre-existing failures is normal and is not a reason to refuse the migration. Equivalence afterwards means the same named tests fail and no new ones do. Stop only when the baseline cannot be run at all, or when a test that already fails covers the code being migrated, in which case say which test and why it blocks.
Keep the judgement-call stop exactly as it is. That one is not the skill doing less: it names the alternatives, the trade-offs and the affected file:line before it stops, which is rule 5's "name the decisive next step" done right.
One smaller note, no change needed unless you want it. Under the codemod section, "inspect the diff produced by the codemod" is advice by your own test, because I cannot look at an output and tell whether it happened. "State the exact command executed" on the same line is checkable. Either give the inspection an observable ("name the files the codemod changed and the transformation applied to each") or drop it.
The pydantic worked example is good and it is doing real work. Keep it.
Push that one change and I will merge.
| - Record the exact test command executed, total tests run, and passing status. | ||
| - If any test fails before starting, **STOP**. Never start a migration on a | ||
| failing baseline. Report the failure and refuse to proceed until the baseline | ||
| is green or pre-existing defects are resolved. |
There was a problem hiding this comment.
This is the one rule I want changed, and it is rule 5 in the style guide: a skill must never become an excuse to refuse work or punt back to the user.
A repository with three unrelated failing tests is the normal case, not the exception. As written the skill refuses the whole migration and sends someone off to fix tests that have nothing to do with the dependency they asked about.
It is also not needed for the discipline. The point of a paired run is that you compare two sets, and OxCode's own verifier already works this way: it captures a baseline carrying failingNames and computes regressions against it rather than refusing to run.
| is green or pre-existing defects are resolved. | |
| - Record the baseline as a set, not a verdict. Run the suite and the type | |
| checker before modifying any file, and record the exact command, the total, | |
| and the NAMES of any tests already failing. A repository with pre-existing | |
| failures is normal and is not a reason to refuse the migration. Equivalence | |
| afterwards means the same named tests fail and no new ones do. | |
| - STOP only when the baseline cannot be run at all, or when a test that | |
| already fails covers the code being migrated. Say which test and why it | |
| blocks, rather than refusing on a count. |
Keep the judgement-call stop further down exactly as it is. That one is not the skill doing less: it names the alternatives, the trade-offs and the affected file:line before it stops, which is rule 5's "name the decisive next step" done right.
|
Updated in latest commit:
|
Closes #7
Summary
Adds the
migrateskill and plugin manifest for upgrading dependencies and frameworks with proven test equivalence.What task was tested & what changed in the answer
npm view,pip index,cargo search) before modifying files.file:linecitations.What was deliberately left out and why
capability: Coding.