Task 1: profile definitions and the harness-equivalence declaration - #58
Merged
Conversation
Turns the spec's profile vocabulary into data a maintainer can review and challenge: one file per profile, so a duplicate definition is a detectable condition rather than a last-wins YAML key. Anchors come from SC-02 and core from SC-03. Everything beyond the anchors is a judgment call made under one constraint: at a 200k Claude Code window a deployable realization is roughly core plus one overlay (~13 skills), so an overlay that tries to cover a category is not deployable at all. Overlays run three to four members and each member is the direct sibling of an anchor - api-design to create-cli, repo-hardening to secure-code, fetchmd as the ingestion step deep-research needs. Twenty-one catalog skills belong to no overlay, which is the design and not a gap. full carries the "*" sentinel rather than a pinned list, which would go stale the moment anyone authors a skill. The equivalence file declares only the three Codex-bundled pairs dojo actually ships a member for, each with the evidence for the claim; local-review against Codex's review-agent is deliberately left undeclared and flagged, because guessing there empties part of the baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
definitions.py answers what the reviewed data says and whether it is consistent against skills.json. It does not compose a selection into a realization - that is Task 2 - so nothing here takes a harness, a target, or a budget. Three decisions are load-bearing. Duplicate YAML keys raise instead of taking the last value, since a repeated members: key would silently discard the reviewed list above it. The non-core count is checked before the anchors: every anchor is itself non-core, so the reverse order would make the count rule unreachable and therefore untestable. And the file set is validated against SC-02's vocabulary in both directions, so a stray copy or a per-project fork cannot enter the vocabulary by being dropped in the directory. Tests restate the spec's constants rather than importing them - importing would only check the module against itself - and assert full against len(skills.json["skills"]) computed at test time. Every rejection test asserts the message names the offending profile or member. All 27 validation rules were mutation-probed; three had no failing test and are now covered, the sharpest being equivalence_identity's sort, which the loader's own sort was masking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of the Task 1 implementation. Two changes to the data, three to the docs it was built from. Withdrew the gpt-imagen <-> codex imagegen declaration. It was the only entry resting on a capability judgement across differing names, and the judgement is unverified: dojo's pins gpt-image-2 and exposes inpainting, masking, and batch output, none of which the bundled description claims. The asymmetry decides it -- a wrong declaration silently removes a selected skill, an undeclared collision is reported -- and it buys nothing today because gpt-imagen is in no overlay. Kept as a candidate with the comparison it still needs. review-agent is not a Codex-listed entry. It exists at ~/.codex/skills/.system/review-agent/ and appears in neither the Task 0 capture nor a live re-probe. The spec and plan both named it as bundled; only listed entries can displace a member. This is the filesystem-is-not-the-listing rule catching this contract's own prose. The plan's equivalence seed contradicted its own rule: it named six Codex skills to seed from while the next sentence forbids declaring a pair whose dojo member does not exist, and three of the six have no dojo counterpart. Rewritten so the rule governs, with the observed-in-a-listing condition made explicit. Also corrected: "8 rejection cases" (27 implemented and probed), a stale Create entry for __init__.py, and a new note that the overlay non-core count must be checked before the anchors or it can never fire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four gaps, two of which would have misled whoever implements the next task. Task 4 never carried the resolved-path rule. Codex reports a symlink's target rather than the link, so an unresolved comparison against the cwd finds project scope nowhere and returns a confident zero. That finding was recorded in Task 0's executed block but not in Task 4's own steps, where someone implementing it would actually look. Task 2 was still written as though definitions.py did not exist, including its own expansion of full's "*" sentinel. Task 1 ships resolved_members; a second expansion is a second thing that can disagree with the first. Task 0's Files list named fixtures that were never created under those names, including the viral capture deleted for publishing a private project's skill names into a public repo. Replaced with what shipped, plus the rule that no fixture may be a capture of another repository and that pseudonymisation must preserve byte length. Suite baseline 309 -> 391 in four places, with a note to assert a floor rather than the literal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478e086170
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
PR #58 review, P2, confirmed: load_definitions accepted a two-member `core`. A test already asserted core.yaml's contents, but that protects this checkout only. load_definitions is what a runtime consumer calls -- Task 2's resolve, and later the verifier -- against whatever core.yaml is on disk, so a narrowed baseline would have flowed straight through into a realization missing the delivery safeguards every deployable composition is promised. The asymmetry was the actual defect: SC-02's anchors were enforced in code from the start, and SC-03's core is the same kind of contract constant and more load-bearing, since core is mandatory everywhere while an overlay is optional. core.yaml's own description already claimed changing it was a contract revision rather than a file edit; nothing enforced that. Rejection names what moved -- missing versus unexpected -- because a narrowed baseline and a widened one are different mistakes. The test restates the SC-03 set independently, so editing the constant alone fails. Mutation-probed both ways: disabling the check fails one test, corrupting the constant fails four and errors nine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 0 shipped two probes with full argparse entrypoints, which raised the question of whether dojo wants a real CLI. Logged rather than acted on, because Task 8 already creates bin/dojo -- `dojo profiles verify --all` is a literal contract term -- and building one before then means designing the same executable twice. The inventory is the useful part: 47 .py files carry an argparse entrypoint, but they are three populations, not one. Roughly 13 are machine-invoked by hooks and CI, two of them on every Bash tool call, where a wrapper is a new runtime dependency rather than a convenience. Roughly 30 are skill-owned, where the SKILL.md naming the command is already the interface. Only the remainder -- the probes, the standardizer pair at 14 and 16 arguments, skills_health -- are human-invoked and would benefit. Task 8 carries a revisit note so the width is decided there rather than accreted. If the plan is descoped short of Task 8, this becomes a standalone decision and the probes are the only part that clearly earns a CLI alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Follows Task 0 (#56) and spec revision 11 (#57). Suite 350 → 391.
What landed
Eight profile definitions (one file each, so a duplicate
nameis a detectable condition rather than a silently last-wins key),harness-equivalences.yaml,scripts/profiles/definitions.py, andprofiles/README.md.Overlays are deliberately 3–4 members, not categories.
core(8) plus any one overlay lands at 11–12, inside the ~13-skill figure a 200k Claude Code window allows. An overlay of nine would not be deployable at all.create-cli⚓secure-codeapi-designrepo-hardeningdeep-research⚓research-architectfetchmddesign-critique⚓web-design-guidelinesfrontend-designobsidian-markdown⚓session-retrocompound-docsgh-commit-push-pr⚓vercel-deployvercel-preview-logsskill-creator⚓skill-standardizerskill-evalsEach non-anchor member is a sibling of an anchor along the axis the anchor sets —
api-designiscreate-cliat the service boundary,repo-hardeningissecure-codeat repo scope,frontend-designis the only producer among two auditors,vercel-preview-logsis the only skill that reads back what a deployment did. 21 of 48 catalog skills belong to no overlay, which is correct rather than a gap. Every member carries an inline reason; every file records what was deliberately excluded.The mutation probe found three holes
All 27 validation rules were probed by breaking each and confirming a test fails. Three did not:
members: core— plausible YAML, neither the sentinel nor a list, and it would have been iterated character by character into four one-letter "members".equivalence_identitywithout itssorted()passed everything, because every test reached identity through the loader, which sorts first. Order-independence was being provided by the wrong function and the guarantee was untested.All three closed. That third one is the same shape as Task 0's
sent := loadedhole — a property tested only through a caller that already satisfies it.Also built in, and worth stating because it is invisible otherwise: the non-
coremember count is checked before the anchors. Every anchor is itself a non-coreskill, so an overlay holding its anchors can never fail the count — with the checks reversed, that rule could not fire on any input.Review changes
Withdrew the
gpt-imagen↔ Codeximagegendeclaration. It was the only entry resting on a capability judgement across differing names, and the judgement is unverified: dojo's pinsgpt-image-2and exposes inpainting, masking, and batch output, none of which the bundled description claims. The asymmetry decides it — a wrong declaration silently removes a selected skill, an undeclared collision is reported — and it buys nothing today sincegpt-imagenis in no overlay. Kept as a candidate with the comparison it needs.Two declarations remain, both same-name same-capability:
skill-creator,skill-installer.Three things Task 1 falsified in its own inputs
review-agentis not a Codex-listed entry. It exists at~/.codex/skills/.system/review-agent/and appears in neither the Task 0 capture nor a live re-probe. Spec and plan both named it as bundled. Only listed entries can displace a member — this is the filesystem-is-not-the-listing rule catching this contract's own prose.__init__.pywas listed asCreatethough it shipped in Task 0.Verification
pytest tests/ -q→ 391 passedcd /tmp,HOME=/tmp/fake-home,PATH=/usr/bin:/bin) → identicalvalidate_spec.pyPASS,validate_plan.pyPASSfullresolves againstlen(skills.json)computed at test time🤖 Generated with Claude Code