[rig-tasks] Add 10 rig samples — 2026-07-28 - #232
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /grill-with-docs — requesting changes on 3 correctness issues across 4 samples.
📋 Key Findings
Issues
- Shell injection (
242):urlinterpolated intoexecAsyncshell string — useexecFileinstead - Phantom output field (
243):reportWrittenclaims a file was written but no write tool orp.writeintent exists — field will always be hallucinated - Enum/handler mismatch (
241): output schema includes"other"but handler never produces it (default is"util") - Ergonomic input design (
248): input fields hold raw YAML content rather than file paths;p.readInputis the idiomatic pattern
Positive Highlights
- ✅ Diverse, realistic use cases (git, npm, file I/O, HTTP, YAML) well-matched to rig primitives
- ✅ Consistent use of
s.enum,s.record,s.optionalthroughout - ✅
repair()andsteering()addons used correctly in 247 and 249 - ✅ All samples passed typecheck
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 39.2 AIC · ⌖ 5.05 AIC · ⊞ 6.3K
Comment /matt to run again
| const { stdout } = await execAsync( | ||
| `curl -s -o /dev/null -w "%{http_code}" --max-time 5 --location "${url}"`, | ||
| ); | ||
| const code = parseInt(stdout.trim(), 10); |
There was a problem hiding this comment.
[/grill-with-docs] Shell injection risk: url is interpolated directly into a shell string passed to execAsync. A URL containing shell metacharacters (e.g. $(...), backticks) will execute arbitrary commands.
💡 Suggested fix
Use execFile to pass the URL as a separate argument, bypassing shell interpretation:
import { execFile } from "node:child_process";
const { stdout } = await promisify(execFile)("curl", ["-s","-o","/dev/null","-w","%{http_code}","--max-time","5","--location",url]);Samples teach patterns — unsafe shell interpolation here will be replicated by readers.
| - "major" if major version differs | ||
| 3. Write a markdown drift report to drift-report.md summarizing the table. | ||
| 4. Set reportWritten to true after writing.`, | ||
| output: s.object({ |
There was a problem hiding this comment.
[/grill-with-docs] The instructions say "Write a markdown drift report to drift-report.md" but the agent has no write tool and no p.write(...) prompt intent. The reportWritten output field will always be a hallucinated true. Either add a p.write intent or remove the file-writing step and the reportWritten field.
💡 Options
Option A — remove the phantom write:
// Remove "3. Write a markdown drift report to drift-report.md" from instructions
// Remove reportWritten from the output schema
output: s.object({
packages: s.array(...),
}),Option B — add a write intent:
instructions: p`...
${p.write("drift-report.md", "<the generated markdown table>")}
`,| command: s.string, | ||
| dependsOn: s.array(s.string), | ||
| category: s.enum("build", "test", "lint", "dev", "deploy", "util", "hook", "other"), | ||
| hasPreHook: s.boolean, |
There was a problem hiding this comment.
[/grill-with-docs] The output schema includes "other" in the category enum, but the parseScriptDeps tool handler never produces that value — it always falls through to "util". Either remove "other" from the schema or add an explicit else branch that returns it.
💡 Fix
// In parseScriptDeps handler, the fallthrough produces "util", never "other".
// Option A: remove "other" from the output enum:
category: s.enum("build", "test", "lint", "dev", "deploy", "util", "hook"),
// Option B: rename the default to "other" in the handler:
let category = "other";| // Agent role: diff top-level keys between two YAML config files and detect breaking changes. | ||
| const yamlConfigDiff = agent({ | ||
| model: "small", | ||
| input: s.object({ baseFile: s.string, targetFile: s.string }), |
There was a problem hiding this comment.
[/grill-with-docs] input requires the caller to pass raw YAML content as strings (baseFile: s.string), but the agent description says "diff two YAML config files". Callers will expect to pass file paths. The existing p.read intent is the idiomatic way to handle this in rig — use p.readInput("baseFile") where baseFile is a path.
💡 Suggested pattern
input: s.object({ baseFile: s.string, targetFile: s.string }), // paths, not content
instructions: p`...
Base file: ${p.readInput("baseFile")}
Target file: ${p.readInput("targetFile")}
`,This also mirrors how sample 247 (licenseHeaderChecker) uses p.readInput. Passing raw multi-KB YAML content as input strings is ergonomically awkward for callers.
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
No typecheck failures — all 10 samples passed.
Tasks run
git for-each-ref, syncclassifyBranchAgetool,s.enum(fresh/stale/dead)input: s.object, asynccheckLicenseHeadertool,repair()addonp.readInputfor both files, two chained toolsp.bash find, asyncextractPackageInfotool,steering()addon