[rig-tasks] Add 10 rig samples — 2026-08-16 - #434
Conversation
- 421: html-anchor-extractor - 422: ts-optional-chaining-counter - 423: git-file-size-tracker - 424: json-pretty-printer - 425: ts-interface-method-counter - 426: git-tag-annotation-extractor - 427: ts-wildcard-reexport-detector - 428: markdown-link-classifier - 429: package-json-field-auditor - 430: parallel-git-stats-workflow All 10/10 typecheck pass. 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 correctness bugs found in tool implementations.
📋 Key Themes & Highlights
Key Themes
- Correctness bug (426): Tagger date regex never matches — git stores timestamps as Unix epoch, not ISO dates.
datealways returns"". - Schema mismatch (430):
call.jsonoutput schema usess.record(s.int)but the sub-agents return{ authors: Record<string,number>, totalCommits: number }— the schema needs to mirror the actual objects. - Shell injection (423, 426): LLM-controlled
filePath/tagNameinterpolated directly intoexecSyncstrings. UsespawnSyncwith an args array instead. - Regex fragility (425): Interface body regex
[^}]*silently drops methods from interfaces with nested generic types.
Positive Highlights
- ✅ Solid variety: HTML, TypeScript, git, JSON, markdown, and workflow patterns
- ✅ Sample 430 cleanly demonstrates the
Promise.allparallelcall()pattern — good canonical reference - ✅ Consistent addon usage (
repair()/steering()) across all samples - ✅ All 10 samples passed typecheck
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 65.9 AIC · ⌖ 4.62 AIC · ⊞ 6.3K
Comment /matt to run again
| const body = execSync(`git cat-file tag "refs/tags/${tagName}" 2>/dev/null || echo ""`, { encoding: "utf-8" }); | ||
| if (/BEGIN PGP/.test(body)) return { type: "signed" as const, subject: "", date: "" }; | ||
| const subjectMatch = body.match(/^subject (.+)$/m); | ||
| const dateMatch = body.match(/^tagger .+ (\d{4}-\d{2}-\d{2})/m); |
There was a problem hiding this comment.
[/grill-with-docs] The tagger date regex will never match — git stores tagger timestamps as Unix epoch (1697000000 +0000), not YYYY-MM-DD. dateMatch always returns null, so date is always "" for every annotated tag.
💡 Fix
Parse the epoch integer and convert it:
const dateMatch = body.match(/^tagger .+ (\d{10,})/m);
const date = dateMatch ? new Date(parseInt(dateMatch[1], 10) * 1000).toISOString().slice(0, 10) : "";| topExtension: ${topExtension ?? ""} | ||
| overallHealth: ${health}`, | ||
| s.object({ | ||
| authorStats: s.record(s.int), |
There was a problem hiding this comment.
[/grill-with-docs] Schema mismatch in call.json output: s.record(s.int) on line 68 expects a flat Record<string, number> but authorStats is { authors: Record<string, number>, totalCommits: number }. The LLM is being asked to write a value it has structured data for, but the schema tells it to discard the structure.
💡 Fix
Use schemas that match the actual agent outputs:
authorStats: s.object({ authors: s.record(s.int), totalCommits: s.int }),
fileStats: s.object({ extensions: s.record(s.int), totalFiles: s.int }),| description: "Get the byte size of a file at a specific git revision using git cat-file.", | ||
| parameters: s.object({ revision: s.string, filePath: s.string }), | ||
| handler({ revision, filePath }: { revision: string; filePath: string }) { | ||
| try { |
There was a problem hiding this comment.
[/grill-with-docs] Shell injection risk: revision and filePath are interpolated directly into a shell command with execSync. If the agent passes paths containing spaces, quotes, or ;, the command will misbehave or execute unintended commands.
💡 Fix
Use spawnSync to pass arguments as an array, bypassing the shell entirely:
import { spawnSync } from "node:child_process";
const result = spawnSync("git", ["cat-file", "-s", `${revision}:${filePath}`], { encoding: "utf-8" });
const size = parseInt(result.stdout?.trim() ?? "0", 10);
return { size: isNaN(size) ? 0 : size };| parameters: s.object({ tagName: s.string }), | ||
| handler({ tagName }: { tagName: string }) { | ||
| try { | ||
| const objType = execSync(`git cat-file -t "refs/tags/${tagName}" 2>/dev/null || echo lightweight`, { encoding: "utf-8" }).trim(); |
There was a problem hiding this comment.
[/grill-with-docs] Same shell injection pattern as 423: tagName is interpolated into execSync shell commands. A tag named foo```; rm -rf .git would be passed directly to the shell.
💡 Fix
Use spawnSync for both calls:
import { spawnSync } from "node:child_process";
const typeResult = spawnSync("git", ["cat-file", "-t", `refs/tags/${tagName}`], { encoding: "utf-8" });
const objType = typeResult.status === 0 ? typeResult.stdout.trim() : "lightweight";| while ((m = ifaceRe.exec(content)) !== null) { | ||
| const name = m[1]; | ||
| const body = m[2]; | ||
| const methods = (body.match(/\w+\??\s*\([^)]*\)/g) ?? []); |
There was a problem hiding this comment.
[/grill-with-docs] The regex /interface\s+(\w+)[^\{]*\{([^}]*)\}/gs won't match interfaces whose bodies contain nested braces (e.g. { key: Record<string, { a: string }> }). The greedy [^}]* stops at the first }, producing an empty or partial body.
💡 Fix
For a sample, acknowledging the limitation in a comment is fine. If correctness matters, consider a brace-counting approach:
// Simple depth-aware body extraction
function extractInterfaceBody(src: string, start: number): string {
let depth = 0, i = start;
while (i < src.length) {
if (src[i] === '{') { depth++; }
else if (src[i] === '}') { if (--depth === 0) return src.slice(start + 1, i); }
i++;
}
return "";
}Or note the limitation with a comment so readers understand the scope.
Summary
Added 10 new rig sample files to
skills/rig/samples/.?.and??operators across TypeScript filesexport * fromwildcard re-export patternsTypecheck failures
No failures this run. All 10/10 samples passed typecheck.
Task 10 required two fixes during authoring:
callmust come frombody: async ({ call }) => {}parameter, notimport { call } from "rig".WorkflowMetarequires bothnameanddescriptionfields.Tasks run