refactor(skill): drop the validator from the porting skill - #181
Open
userAugustos wants to merge 6 commits into
Open
userAugustos wants to merge 6 commits into
userAugustos wants to merge 6 commits into
Conversation
report_port shelled out to the skill's own validate.py, which made the skill's file layout part of what `canyonos build` depends on. It now calls run_validate over .car in this process, so the verdict comes from the same checks `canyonos validate` runs and the skill directory holds nothing build looks for.
The skill carried its own validator and a `validation/` package beside it, and every runtime fact those checks needed was a second copy of a constant that canyonos_core already owns: the image Python version, the flat runtime module names, the base requirement lists, the builtin yaml type names. A copy that lives in a skill directory drifts from the runtime silently, because nothing builds or tests against it. `canyonos validate` reads those constants from canyonos_core directly, so the skill no longer needs to carry any of them. `prepare.py` stays: it runs before there is a `.car` for the CLI to work on. tests/test_skill_smoke.py was the only test importing the deleted package, and the skill directory was an input of canyonos-python#test only for its sake, so both go with it. A new test walks the installed skill source and fails on any .py but prepare.py, so the mirror cannot creep back.
The reference docs told the agent to run a file the skill no longer ships, and cited findings by the retired V0xx/W0xx codes. Step 4 now runs `canyonos validate` and then `canyonos test --rebuild`, and the one cited code that survived the move -- V033, the package re-export that stubs cannot satisfy -- is named CAR-PACKAGE-REEXPORT. The other citations named checks that no longer exist, so each is dropped together with the sentence that only carried it; the runtime facts they hung off are still true and stay. This is a surgical pass over the invocation and the codes, not the rewrite of step 4. A test greps the shipped references for the retired scheme, so a code that no command reports cannot be cited again.
The tests that keep a validator and the retired finding codes out of the skill run under canyonos#test, whose inputs were the package defaults alone. A stray .py or a V0xx citation in the skill directory left that hash untouched, so CI would replay a cached pass over a skill nobody had checked. The task now hashes the skill tree alongside the package: 45 inputs became 56, and editing a reference moves the hash. The E402 exemption named the porting validator, which is gone. It names what still needs it: the controller modules, and the tests that load them, put generated grpc stubs on sys.path before importing them.
`canyonos validate` has no warning severity -- every finding blocks and the command exits 1 while one remains -- so the handoff had nothing to list under "validator warnings", and the instruction not to hide them had drifted below the `canyonos test --rebuild` block where it read as being about that command. The verdict sentence claimed less than step 4 now does: `canyonos test --rebuild` deploys locally and serves one prompt, which is exactly what the report should say, and what the approval question and the checklist already said. "Gap validation" and "the gap validator" outlived the heading that defined them, so each site names `canyonos validate`. The one-sentence "Validation boundary" section folds into the paragraph it qualified.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
userAugustos
marked this pull request as ready for review
September 22, 2026 19:42
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
1 participant
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.
What was done
The porting skill no longer carries its own copy of the platform rules; it runs the CLI's checks instead.
Details
Part of #164 (step 2d). Stacked on #180 (base branch
feat/cli-validate); merge #178 and #180 first..claude/skills/porting-to-canyonos/validate.pyandvalidation/(2,134 lines) and the one test that imported them. The skill directory is nowSKILL.md,prepare.pyandreferences/.stub_generator.IMAGE_PYTHON,AGENT_FLAT_MODULES,WORKFLOW_FLAT_MODULES,RESERVED_CONTEXT_NAMES), base requirements and platform pins (BASE_*_REQUIREMENTS,PLATFORM_PINS), builtin argument types (schema.BUILTIN_TYPE_NAMES). The import→distribution table and the stdlib list only fed the dropped W006 heuristic and go with it.canyonos buildtakes its verdict fromcanyonos validatein-process instead of shelling out to a file in the skill directory. Note: verifying a port therefore needs core installed beside the CLI or Docker with the core image — the released binary alone can no longer verify offline (it could not deploy offline either).canyonos validate, followed bycanyonos test --rebuildas the install-and-import step; V/W-code citations map to theirCAR-*names or are removed with the sentence that only existed to cite them; "gap validation" is gone as a term. This is a surgical pass — the stale env-file and streaming sections and the shrink of the skill are step 4 of Refactor: Skill file, diagnosis and testing goal #164.prepare.py, and no reference cites a retiredV0xx/W0xxcode.turbo.jsonhashes the skill directory into the CLI test task so those guards cannot be skipped by the cache, and drops it from the Python task that no longer reads it.How to test it
ls .claude/skills/porting-to-canyonos/→SKILL.md,prepare.py,references/, nothing else.grep -rn -E "validate\.py|validation/|\b[VW]0[0-9]{2}\b|gap valid" .claude/skills packages scripts→ no hits..pyinto the skill directory, or pasteV033back into a reference →packages/cli/tests/test_skill_source.pyfails, andturbo run test --dry=jsonshows the skill files among the CLI test task's hashed inputs..car:canyonos build --agent claude --scope local -yends with the verdict ofcanyonos validate; the findings are the same onescanyonos validateprints on its own.Generated by Claude Code