[rig-tasks] Add 10 rig samples — 2026-07-27 - #211
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /grill-with-docs and /codebase-design — requesting changes on three correctness issues in the sample code.
📋 Key Themes & Highlights
Key Themes
- Unreachable enum value (
dirtystate in 203): the syncparseWorktreePorcelaintool can't check git status, so the enum variant it declares is never reachable — misleading for readers learning sync vs async tool patterns. - Unused parameter (205):
nameis declared inclassifyDependencyparameters but silently ignored in the handler — a subtle signal that goes against the "clean, self-explanatory" code style goal. - Empty steering call (208):
steering()without amessagegenerates a no-op re-prompt; should either be given a message or replaced withrepair()alone.
Positive Highlights
- ✅ All 10 samples pass typecheck on first attempt — excellent quality baseline
- ✅ Good coverage of diverse patterns: async/sync tools, repair, steering, subagents,
p.readOptional,p.write,s.unknown,s.enum - ✅
208-release-note-enricher.mdcleanly demonstrates theinput+defineTool+ multi-addon pattern - ✅
202-ts-interface-conflict-checker.mdis a well-formed asyncdefineToolexample - ✅
s.pathused correctly foroutputPathin 209
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 50 AIC · ⌖ 4.62 AIC · ⊞ 6.3K
Comment /matt to run again
| tools: [lookupTicketMetadata], | ||
| maxTurns: 5, | ||
| addons: [steering(), repair()], | ||
| }); |
There was a problem hiding this comment.
[/grill-with-docs] steering() is called without a message argument — the addon has no guidance to steer toward. An empty steering prompt is unlikely to help the model self-correct.
💡 Add a message or replace with repair()
Supply a concrete steering message:
addons: [steering({ message: "Ensure riskLabel and all section headings are populated." }), repair()],Or if the intent is just parse-correction, repair() alone is sufficient.
| path: s.string, | ||
| branch: s.optional(s.string), | ||
| state: s.enum("locked", "bare", "clean", "dirty"), | ||
| })), |
There was a problem hiding this comment.
[/grill-with-docs] The parseWorktreePorcelain tool receives the porcelain output via its output parameter, but the instructions say to check for uncommitted changes and set state to dirty — the tool never does this check. The sync handler has no way to run git status per worktree, so the dirty state in the output schema can never actually be populated.
💡 Options to reconcile this
Option A: Remove dirty from the enum (the tool can only observe locked/bare/clean from porcelain output), then let the agent instructions drive a follow-up p.bash("git status --porcelain \"<path>\"") check.
Option B: Change parseWorktreePorcelain to an async handler that shells out to git status per entry — similar to 202-ts-interface-conflict-checker.md.
As written, the dirty state is unreachable and the example may mislead readers about what sync tools can do.
| }), | ||
| handler({ inDependencies, inDevDependencies, inPeerDependencies }) { | ||
| if (inPeerDependencies) return "peer"; | ||
| if (inDevDependencies) return "dev"; |
There was a problem hiding this comment.
[/grill-with-docs] The classifyDependency tool ignores its name parameter entirely — it's declared in parameters but never destructured or used in handler. This is misleading for readers learning tool patterns.
💡 Either use the parameter or remove it
If the name is used only for display in the output (already in directDeps[].name), drop it from the tool parameters:
parameters: s.object({
inDependencies: s.boolean,
inDevDependencies: s.boolean,
inPeerDependencies: s.boolean,
}),Or destructure it and surface it in the return value if it serves a purpose.
| changedKeys: s.record(s.object({ | ||
| baseline: s.unknown, | ||
| actual: s.unknown, | ||
| })), |
There was a problem hiding this comment.
[/grill-with-docs] The p.write("config-patch.json", "PATCH_CONTENT") placeholder is a string literal, not the actual field from the output schema. Readers following this pattern may expect to see how the agent's structured output flows into the write intent — but here the written content is hardcoded, making the connection between the changedKeys output and the file write opaque.
💡 Clarify the relationship between output and write intent
If the intent is to write the changedKeys object as JSON, show that in the placeholder text or a comment:
// p.write writes the agent-generated JSON patch; the output schema captures the parsed form
${p.write("config-patch.json", "{}")}This makes the sample more instructive about how p.write and structured output coexist in the same agent.
| model: "small", | ||
| instructions: p`Delegate API extraction and prose cleanup to the named subagents. Merge their results: the apiExtractor returns the list of API names, and proseCleanup returns rewritten prose. Combine into a final output and write the refactored content to ${p.write("docs/refactored.md", "REFACTORED_CONTENT")}. Count the changes made to the prose.`, | ||
| output: s.object({ | ||
| extractedApis: s.array(s.string), |
There was a problem hiding this comment.
[/grill-with-docs] s.path is used for outputPath in the output schema, which is good — but docsRefactorCoordinator instructions say to write to docs/refactored.md via p.write("docs/refactored.md", "REFACTORED_CONTENT"). The REFACTORED_CONTENT placeholder does not reference the proseCleanup subagent's result in any visible way. Readers may not understand how subagent return values flow into write intents.
💡 Make the data flow explicit
Since p.write placeholders are instructions to the model, the instructions text should explicitly say which subagent result becomes the content:
instructions: p`Delegate API extraction to apiExtractor and prose cleanup to proseCleanup. Take the rewritten prose from proseCleanup and write it to ${p.write("docs/refactored.md", "<!-- rewritten prose from proseCleanup -->")}. Count the changes.`,This makes the sample self-documenting about the subagent → write data flow.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
No failures — all 10 programs passed typecheck on first attempt.
Tasks run