Repository navigation
feat(skills): pick task skills with one shared resolver, behind wizard-agent-dag - #1551
Conversation
…d-agent-dag Co-Authored-By: Claude <noreply@anthropic.com>
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a Context Mill branch:
Add Results will be posted here when complete. |
posthog-security-review-bot
Bot
added
security-review
|
✅ Security review complete — No findings above the confidence threshold, in 5 min. Add the |
|
✅ Security review complete — No findings above the confidence threshold, in 4 min. Add the |
johncwaters
left a comment
johncwaters
left a comment
There was a problem hiding this comment.
Note
Automated review. Not written by a human.
Verdict: APPROVE WITH NITS
See inline comments.
sarahxsanders
left a comment
sarahxsanders
left a comment
There was a problem hiding this comment.
I'm not seeing any major blockers in any of these PRs, so I am gonna leave general comments
centralizing skill selection makes sense to me + the existing path behind the flag makes things a little less scary :)
![posthog-security-review-bot[bot]](https://avatars.githubusercontent.com/in/4551195?s=60&v=4){kind=small}
posthog-security-review-bot
Bot
removed
the
security-review
The equivalence test also runs on the recorded menu with every tags field dropped, the shape a context-mill release without menu metadata serves. Co-Authored-By: Claude <noreply@anthropic.com>
gewenyu99
left a comment
gewenyu99
left a comment
There was a problem hiding this comment.
Initial comments, these generally loook pretty clean. Gonna spend some more time trying to break it with. claude.
| if (resolveSkillVariantId(menuSkillEntries, skillId, framework)) { | ||
| continue; | ||
| } | ||
| if (pickSkillId(skillId)) continue; |
There was a problem hiding this comment.
Not from this PR, but this check now decides more. When a skill name is gone from the menu, the user is told the instructions "failed to download" and to try again. Retrying can't help.
That's what an older flag-on wizard hits after context-mill renames a skill. The message should say the skill is missing from the menu and to update the wizard.
A tag whose key names a built-in object property, such as constructor:x, made the resolver read that property and throw. It now reads only keys the run supplies. The shared README says a run with no framework resolves no family. The fixture recorder reads the menu before it writes, so a response that is not a menu leaves the fixture alone. Co-Authored-By: Claude <noreply@anthropic.com>
…able A run whose framework reference skill cannot be installed goes on without the example and the framework rules. It now sends orchestrator reference skill unavailable with the skill and the reason. Co-Authored-By: Claude <noreply@anthropic.com>
The release carries the menu's titles, descriptions and tags, so the recorded menu is now a published one. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Adds
wizard-agent-dag, a boolean flag. With it on, the orchestrator picks each task's skills withresolveSkillin@shared/skill-resolution. With it off or unset, the orchestrator uses its menu lookup,resolveSkillVariantId. Both pick the same skill for every skill family, menu id and framework on the recorded menu.A run pinned to a context-mill release with
--context-mill-versionuses the menu lookup, flag on or off.Stack
The five PRs, in merge order:
wizard-agent-dagflag and the shared skill resolver the orchestrator uses when it is on.main. Every later PR hangs its new path off this flag.The four wizard PRs merge in that order and ship in one release with
wizard-agent-dagoff. A run with the flag off or unset behaves as it does onmain.How a pick works
A task prompt names its skills by menu id or by skill family, such as
skills: [integration, posthog-best-practices].flowchart TD name["A task names a skill"] --> id{"Is the name<br/>a menu id?"} id -- yes --> exact["That skill"] id -- no --> family["The skills whose group is the name"] family --> disagree["Drop a skill whose match key<br/>disagrees with the run's value"] disagree --> nokey["Drop a skill with no framework key"] nokey --> rank["Rank by keys that agree,<br/>then the default tag, then menu order"] rank --> pick["The first one"]A
key:valuetag on a menu entry is a match key, such asframework:nextjs. It says which projects the skill is written for, and the run supplies its values. A key the run has no value for never disagrees. An entry'sframeworkanddefaultfields count as tags, so a menu published without tags resolves the same.The run supplies one key here:
framework.What is in the PR
wizard-agent-dagjoinsWIZARD_FLAG_KEYS.isAgentDagEnabled(flags)reads it in the switchboard, and is false for a run pinned to a context-mill release.@shared/skill-resolution:resolveSkill(entries, name, matchKeys), pure.resolveSkillwith the flag on,resolveSkillVariantIdwith it off.SkillEntryreadstitle,descriptionandtags. The menu'sbuildVersionbecomes thecontext_mill_versiontag on every later event.setup wizard finishedcarries the run's wizard flags, so an outcome can be split by flag variant.orchestrator reference skill unavailablewith the skill and the reason.fixtures/context-mill/skill-menu.json, recorded withpnpm record:context-mill-fixtures.Proof of no change
src/agent/runner/sequence/orchestrator/__tests__/skill-equivalence.test.tscompares the two picks for every name a prompt can use against every framework on the menu, no framework, and an unknown framework.tagsfield dropped, the shape a release without menu metadata servesCONTEXT_MILL_MENUCONTEXT_MILL_MENU=<path to skill-menu.json>runs the same comparison against any other build.The menu lookup reads an entry's
frameworkanddefaultfields, and the resolver also reads its tags. The same test fails when an entry's tags and fields disagree, and when the recorded menu carries no tags.Test plan
pnpm typecheckpnpm lintpnpm exec vitest run: 282 files passfixtures/context-mill/skill-menu.jsonfrom the release that ships feat(menu): describe skills with title, description and tags context-mill#447. The committed menu is a local build of that PR, because the published menu has no tags yet🤖 Generated with Claude Code