[rig-tasks] Add 10 rig samples — 2026-08-19 - #453
Conversation
- 431: OS env variable scanner with defineTool + s.enum categories - 432: Sequential commit pipeline workflow (collect→classify→aggregate) - 433: TOML config key extractor with defineTool + repair - 434: CSV column stats reporter with defineTool + s.optional - 435: Git worktree analyzer with steering+repair addons - 436: TypeScript narrowing pattern detector with async defineTool - 437: Git reflog summarizer with s.enum action classification - 438: TypeScript export reporter with p.writeOutput - 439: Parallel project health workflow with call.json + Promise.all - 440: Markdown frontmatter extractor with steering+repair 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 /diagnosing-bugs and /codebase-design — requesting changes on correctness issues and one pattern misuse.
📋 Key Themes & Highlights
Issues Found
p.glob-in-instructionsmisuse (436, 438) —p.globinjects a text list into the prompt; it doesn't deliver a runtime array the tool can iterate over. Samples 436 and 438 both use this incorrectly when their tools require actual file I/O."dirty"status never populated (435) —classifyWorktreehandler only returns"clean"or"detached", but the output schema declares"dirty"as a valid enum value. ThedirtyCountoutput will always be 0.- CSV numeric filter bug (434) —
values[0] !== ""guards off the first element only; a column starting with an empty cell silently disqualifies the whole column from numeric detection. writeOutput+reportPathambiguity (438) — the lonep.writeOutputsample is also the most complex instruction block; the intent is good but the interaction betweenp.glob,p.bash, andp.writeOutputin one template needs simplification.- Unfiltered
envin prompt (431) —p.bash("env")dumps all env vars including secrets into the LLM prompt; worth a warning comment at minimum.
Positive Highlights
- ✅ Good variety: tools with sync handlers, async
defineTool, workflow pipelines, andcall.json. - ✅ Consistent use of
repair()alongside complex output schemas. - ✅
440-markdown-frontmatter-extractoris a clean example ofsteering()+repair()with optional fields. - ✅
432-sequential-commit-pipelinedemonstrates a crisp three-phase workflow with proper null guards after eachcall.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 44.9 AIC · ⌖ 4.89 AIC · ⊞ 6.3K
Comment /matt to run again
| parameters: s.object({ name: s.string, values: s.array(s.string) }), | ||
| handler({ values }) { | ||
| const nums = values.map(Number).filter((n) => !isNaN(n) && values[0] !== ""); | ||
| const uniqueCount = new Set(values).size; |
There was a problem hiding this comment.
[/diagnosing-bugs] Off-by-one in the numeric filter: values[0] !== "" always checks the first element of the full column array, so a column whose first value is empty string will suppress all numeric detection even when later rows are valid numbers.
💡 Fix
const nums = values
.filter(v => v !== "")
.map(Number)
.filter(n => !isNaN(n));Then guard with nums.length === values.filter(v => v !== "").length to decide "all numeric".
| description: "Classify a git worktree entry into type and status.", | ||
| parameters: s.object({ path: s.string, isBare: s.boolean, branch: s.optional(s.string), isDetached: s.boolean }), | ||
| handler({ isBare, branch, isDetached }) { | ||
| const type = isBare ? ("bare" as const) : branch === "main" || branch === "master" ? ("main" as const) : ("linked" as const); |
There was a problem hiding this comment.
[/diagnosing-bugs] The classifyWorktree handler sets status to only "clean" or "detached" — it never returns "dirty", but the output schema lists s.enum("clean", "dirty", "detached"). The agent will always report 0 dirty worktrees regardless of actual state.
💡 Suggestion
Either remove "dirty" from the output enum (simplify the contract), or pass a isDirty parameter and set status accordingly:
parameters: s.object({ path: s.string, isBare: s.boolean, branch: s.optional(s.string), isDetached: s.boolean, isDirty: s.boolean }),
handler({ isBare, branch, isDetached, isDirty }) {
const status = isDetached ? "detached" : isDirty ? "dirty" : "clean";
...
}|
|
||
| // Agent role: Count exported functions per TypeScript file and write a JSON report. | ||
| const tsExportReporter = agent({ | ||
| model: "small", |
There was a problem hiding this comment.
[/codebase-design] The instructions combine p.glob, p.bash, and p.writeOutput in a single giant p\...`template. PerSKILL.mdruleINV:p-write-no-path, p.write()/p.writeOutput()contributes a write instruction to the prompt and does not return the path — butreportPath` is also declared as an output field. The agent is being asked to both emit the path and write the file from one blended instruction, which is ambiguous and fragile.
💡 Suggestion
Split concerns: use p.writeOutput to write the file and hard-code the output path in the schema:
instructions: p`Find TypeScript files: ${p.glob("src/**/*.ts")}. Count exported symbols: ${p.bash("...")}. Write the report: ${p.writeOutput("reportPath", "export-report.json")}.`,
output: s.object({
fileCount: s.int,
reportPath: s.path, // populated by writeOutput
topExporter: s.optional(s.string),
}),But note: p.writeOutput writes the content the agent generates; there is no p.bash result to pipe into it here. Consider using p.write("export-report.json", ...) with a fixed path and removing reportPath from the output, or restructure the agent to first collect data then write.
| // Agent role: Detect TypeScript type narrowing patterns across source files. | ||
| const tsNarrowingDetector = agent({ | ||
| model: "small", | ||
| instructions: p`Find TypeScript source files: ${p.glob("src/**/*.ts")}. Use scanNarrowingPatterns on each file. Return counts per file and totals.`, |
There was a problem hiding this comment.
[/codebase-design] p.glob is a declarative placeholder that resolves into a prompt instruction (a list of paths injected as text) — it does not pass a live array to the agent. Using it directly in instructions asks the LLM to iterate over the paths and call scanNarrowingPatterns on each, but scanNarrowingPatterns performs real readFile I/O. The LLM will fabricate file paths rather than getting the real list.
💡 Suggestion
Per SKILL.md: "p.glob returns paths only; then delegate one path at a time to a subagent using p.readInput(\"path\")". For per-file tools, structure the agent so the glob is in input and use p.bashEach or a workflow loop to invoke the tool per file, or use p.bash("find src -name *.ts") and have the tool do the reading itself.
|
|
||
| // Agent role: Count exported functions per TypeScript file and write a JSON report. | ||
| const tsExportReporter = agent({ | ||
| model: "small", |
There was a problem hiding this comment.
[/codebase-design] Same p.glob-in-instructions pattern as #436: p.glob("src/**/*.ts") resolves to injected path text, not a runtime array the agent can iterate over. Combined with p.bash(grep ...) on a fixed path, the two inputs are redundant and the LLM is likely to ignore one.
💡 Suggestion
Since this sample is showcasing p.writeOutput, simplify instructions to just the bash grep and the writeOutput call — drop the redundant glob:
instructions: p`Count exported symbols per file: ${p.bash("grep -rc export| // Agent role: Scan all OS environment variables and classify them by type. | ||
| const osEnvScanner = agent({ | ||
| model: "small", | ||
| instructions: p`Scan environment variables: ${p.bash("env")}. Use classifyEnvVar on each variable and return a full summary.`, |
There was a problem hiding this comment.
[/codebase-design] The p.bash("env") call will inject the full environment dump — including secrets like GITHUB_TOKEN — into the LLM prompt context. This is a security risk unique to env-scanner samples.
💡 Suggestion
Consider redacting sensitive variables in the bash command before injecting:
p.bash("env | grep -v 'TOKEN\\|SECRET\\|KEY\\|PASSWORD'")Or document in a comment that this sample should only be run in non-sensitive environments.
| ]); | ||
| phase("Score"); | ||
| const healthScore = await call.json( | ||
| `Given scriptCount=${pkgResult?.scriptCount ?? 0}, strict=${tsResult?.strict}, paths=${tsResult?.paths}, esModuleInterop=${tsResult?.esModuleInterop}: compute an overall health score 0-100.`, |
There was a problem hiding this comment.
[/diagnosing-bugs] The health score prompt embeds runtime values via string interpolation (pkgResult?.scriptCount ?? 0) rather than passing a structured input to call.json. If pkgResult or tsResult is null (prior call.json failed), the score agent receives scriptCount=0, strict=undefined with no way to distinguish "not available" from "false", leading to misleading scores.
💡 Suggestion
Either guard with an early return when either upstream result is null, or pass a structured object:
if (!pkgResult || !tsResult) return null;
const healthScore = await call.json(
p`Given this project data: ${JSON.stringify({ ...pkgResult, tsconfig: tsResult })}, compute an overall health score 0-100.`,
s.int,
);1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.defineTool+s.enumcategories +repairdefineTool, nesteds.record,repairdefineTool,s.optional(s.number)for min/max/meansteering()+repair()addonsdefineTool+node:fs/promisesp.bash --format+s.enumaction typesp.writeOutput(first sample for this intent)Promise.all+call.jsonsteering()+repair()+s.optionalTypecheck failures
None — all 10 programs passed typecheck on first attempt.
Tasks run