[rig-tasks] Add 10 rig samples — 2026-07-26 - #169
Conversation
Samples added: - 160-dead-code-detector (reused: deadcd2b) - 161-openapi-spec-validator (reused: opnapi8c) - 162-api-endpoint-extractor (reused: apiext11) - 163-source-file-annotator (reused: srcann01) - 164-stale-dependency-detector (reused: nxtask1a) - 165-git-tag-timeline (reused: nxtask2b) - 166-env-variable-type-inferrer (new: envbdg01) - 167-makefile-target-extractor (new: makfilt2) - 168-test-fixture-generator (new: tstfxtr3) - 169-server-uptime-log-parser (new: srvupti4) All 10/10 typecheck passed. 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 /codebase-design — requesting changes on three correctness issues found across the new samples.
📋 Key Themes & Highlights
Issues Found
-
Shell injection in
160-dead-code-detector—symbolis interpolated directly into anexecSyncshell command without sanitisation. Any adversarial or unusual symbol name (e.g., containing$(...)) could execute arbitrary shell commands. Fix: strip non-identifier characters or usegrep -F. -
Tool/output schema mismatch in
161-openapi-spec-validator— thecheckStructuretool returns{ issues }but the output schema includesvalid: s.booleanwhich the tool never produces. The LLM must guess this field, making results unreliable. Fix: derive and returnvalidfrom the issues list in the tool handler. -
p.writemisuse in163-source-file-annotator—p.write("annotated-summary.md", "...")writes the literal placeholder string at prompt-build time, not the agent's generated summary. To persist a generated output field to disk, usep.writeOutput(field, path)instead. -
Wrong
p.writeOutputfield in168-test-fixture-generator—p.writeOutput("suggestedFileName", ...)points at the filename field (a path label) rather than the code-content field. The written file will contain a file path string, not the fixture code. Should bep.writeOutput("fixtureCode", "fixture-output.ts").
Positive Highlights
- ✅ Correct
import { repair } from "rig/addons"usage — the PR description notes this was fixed from a prior mistake, and all affected samples now import correctly. - ✅ Good use of
steering()addon in169-server-uptime-log-parserto guide crash-loop detection. - ✅ Subagent delegation patterns in samples 162, 163, 165, 168 are clean and consistent with the codebase conventions.
- ✅
s.optionalused correctly for nullable output fields throughout.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 36.4 AIC · ⌖ 4.57 AIC · ⊞ 6.3K
Comment /matt to run again
| const out = execSync( | ||
| `grep -rn "\\b${symbol}\\b" --include="*.ts" . 2>/dev/null | grep -v "^export " | grep -v node_modules | wc -l`, | ||
| { encoding: "utf-8" } | ||
| ); |
There was a problem hiding this comment.
[/codebase-design] Shell injection risk: symbol is interpolated directly into the execSync shell command, so an adversarial or malformed symbol value could execute arbitrary shell commands.
💡 Suggested fix
Strip non-identifier characters before interpolation:
const safeSym = symbol.replace(/[^a-zA-Z0-9_$]/g, '');
const out = execSync(
`grep -rn -F "${safeSym}" --include="*.ts" . 2>/dev/null | grep -v node_modules | wc -l`,
{ encoding: 'utf-8' }
);Using -F (fixed-strings) also avoids accidental regex metacharacter expansion.
| })), | ||
| issueCount: s.int, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
[/codebase-design] Output field valid is never set by checkStructure — the tool returns { issues } but the output schema expects valid: s.boolean. The LLM must infer this field's value, making it unreliable.
💡 Suggested fix
Either derive valid from the issue list in the tool handler and return it:
return { issues, valid: issues.filter(i => i.type === 'error').length === 0 };...or remove valid from the output schema and let the consumer compute it from issueCount. A mismatch between tool output and output schema is a common source of repair-loop thrashing.
| For each file, read its content, delegate to the jsDocAnnotator subagent, then write the annotated version back using p.write. Track which files were processed successfully and which were skipped. | ||
|
|
||
| ${p.write("annotated-summary.md", "# Annotation Summary\n<!-- will be filled by agent -->")} | ||
|
|
There was a problem hiding this comment.
[/codebase-design] p.write with a static template string is a prompt instruction, not a post-generation write — the agent is instructed to write a placeholder file, but the actual annotated content is never wired to that path. Use p.writeOutput(field, path) if you want to persist an output field to disk.
💡 Detail
Per the docs: p.write(path, content) embeds content known at prompt-build time as an instruction. The resulting file will literally contain the placeholder comment, not the agent's summary.
If the goal is to write the agent's summary after generation, add a field to the output schema and use:
${p.writeOutput("annotationSummary", "annotated-summary.md")}| ${p.writeOutput("suggestedFileName", "fixture-output.ts")} | ||
|
|
||
| Return only the declared output.`, | ||
| output: s.object({ |
There was a problem hiding this comment.
[/codebase-design] p.writeOutput("suggestedFileName", "fixture-output.ts") uses a static destination path — the field name suggestedFileName is passed as the output field, but the written file will always be fixture-output.ts regardless of what the agent generates. This is likely intended to be p.writeOutput("fixtureCode", "fixture-output.ts").
💡 Detail
The signature is p.writeOutput(field, path) where field must be a key in the output schema and path is the static destination. Here suggestedFileName is a s.path field (the name the agent proposes), not the code content. To write the generated code, use the field that holds the content:
${p.writeOutput("fixtureCode", "fixture-output.ts")}If you also want the destination path to be dynamic (caller-supplied), use p.writeInput(field, inputField) instead.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.p.bash,defineTool estimateUsage,repair()p.readOptional+defineTool checkStructureendpointClassifiersubagent +p.bashgrepjsDocAnnotatorsubagent +p.writep.bash npm outdated,defineTool classifyDrift,repair()timelineSummarizersubagent chain.env.examplevariable type inferrer usingp.readOptional+defineTool inferTypep.readOptional+defineTool parseTargets+repair()input s.object({sourceFile: s.path}),p.readInput,p.writeOutput, nano subagentp.bash,defineTool parseLogLine,steering()Typecheck failures
No failures this run — all 10/10 samples passed typecheck after fixing:
repair/steeringmust be imported from"rig/addons", not"rig"p.writeOutputrequires 2 arguments(field, path), not just 1Tasks run