[rig-tasks] Add 10 rig samples — 2026-08-25 - #487
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 — commenting on correctness issues in tool implementations. No blocking structural problems; the samples are well-formed and diverse.
📋 Key Themes & Highlights
Key Themes
- Error return mismatch (465, 468): Two tools return
{ error: "..." }on failure, but the agent output schema doesn't include anerrorfield. This silently forces repair cycles and sets a misleading pattern for readers. - Spread regex double-counting (464): Both
objectSpreadsandarraySpreadslookaheads include,, so spreads in multi-element literals are counted twice — the most common case. - Enum regex multi-line gap (462):
[^}]*stops at the first}, missing multi-lineconst enumbodies which are idiomatic TypeScript. call.jsonvs structured agent (461): TheSummarizephase encodes all data as a freeform prose string; other workflow samples demonstrate the cleanerinput: s.object({...})pattern.- Unbounded
p.readInputfor log files (463): Inlining an access log into the prompt context is impractical at scale; worth noting or bounding in the sample.
Positive Highlights
- ✅ Good spread of patterns:
repair+steering, parallelPromise.all, sequential two-stage workflows, custom tools withnode:cryptoandnode:fs. - ✅
469(reflog classifier) is a clean, idiomatic two-stage sequential workflow — good reference for that pattern. - ✅ All 10 samples pass typecheck; import paths are correct.
- ✅
s.pathused appropriately for file path parameters throughout.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 55.7 AIC · ⌖ 5.09 AIC · ⊞ 6.3K
Comment /matt to run again
| const { readFile } = await import("node:fs/promises"); | ||
| const src = await readFile(filePath, "utf8"); | ||
| const objectSpreads = (src.match(/\.\.\.[a-zA-Z_$][a-zA-Z0-9_$]*(?=\s*[,}])/g) ?? []).length; | ||
| const arraySpreads = (src.match(/\.\.\.[a-zA-Z_$][a-zA-Z0-9_$]*(?=\s*[,\]])/g) ?? []).length; |
There was a problem hiding this comment.
[/grill-with-docs] Both regex lookaheads include , so every ...foo, (spread before a comma) is counted in both objectSpreads and arraySpreads, silently doubling all spreads in multi-element literals.
💡 Suggested fix
Use mutually exclusive lookaheads — } for object, ] for array:
const objectSpreads = (src.match(/\.\.\.[a-zA-Z_$][a-zA-Z0-9_$]*(?=\s*})/g) ?? []).length;
const arraySpreads = (src.match(/\.\.\.[a-zA-Z_$][a-zA-Z0-9_$]*(?=\s*\])/g) ?? []).length;Spreads followed by , are context-ambiguous from a pure-regex perspective; document that limitation or tally them separately as ambiguousSpreads.
| .filter((line: string) => line.length > 0) | ||
| .map((line: string) => { | ||
| const [name, value] = line.split("=").map((s: string) => s.trim()); | ||
| return { name, value: value ?? null }; |
There was a problem hiding this comment.
[/grill-with-docs] The regex /const\s+enum\s+(\w+)\s*\{([^}]*)\}/g doesn't handle multi-line enum bodies where the closing } appears on its own line — common in TypeScript. [^}]* is greedy but stops at the first }, so enums with nested expressions or trailing commas followed by whitespace may be skipped or incorrectly truncated.
💡 Suggested approach
Use the s (dotAll) flag so . matches newlines, or replace [^}]* with [\s\S]*? for a non-greedy match across lines:
const enumRe = /const\s+enum\s+(\w+)\s*\{([\s\S]*?)\}/g;This is a sample, so a note in the instructions or a comment in the code acknowledging this limitation would also be acceptable.
| summary: s.string, | ||
| }), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
[/grill-with-docs] call.json receives a large prose prompt that embeds JSON via template literals. If any JSON.stringify value is long or contains special characters the resulting string becomes hard to read and can drift from the intended semantics as a sample.
💡 Preferred pattern: structured input agent
Other workflow samples (e.g. 469) pass structured data to a typed input: agent instead of embedding it in a freeform call.json string. Consider making Summarize a proper agent with input: s.object({...}) so the pattern being demonstrated is consistent with the rest of the samples collection.
| else if (status >= 300 && status < 400) statusClass = "3xx"; | ||
| else if (status >= 400 && status < 500) statusClass = "4xx"; | ||
| else if (status >= 500 && status < 600) statusClass = "5xx"; | ||
| else statusClass = "other"; |
There was a problem hiding this comment.
[/grill-with-docs] p.readInput("logFile") inlines the log file content into the prompt, which is unbounded. A real access log can be hundreds of MB — the intent is clearly to use parseLogLine on each line, but the instructions ask the model to first read the whole file into context before calling the tool.
💡 Suggested alternative
Pass the path to the tool and read line-by-line inside the handler instead of via p.readInput. The tool signature already accepts a line string, so add a companion readLogLines tool (or use p.bash("head -1000 ...") to bound the input) so the sample doesn't imply full-file inlining as the pattern.
| }); | ||
|
|
||
| // Agent role: scan .git/hooks for installed hook scripts and report their properties. | ||
| const gitHookFileScanner = agent({ |
There was a problem hiding this comment.
[/grill-with-docs] When analyzeHookFile catches a filesystem error it returns { error: "could not read hook" }, but the tool's return type is inferred as string (JSON). The output schema for hooks is s.record(s.object({ shebang, executable, hookType, lineCount })), so if the model faithfully includes errored hooks in the record the validation will fail and trigger repair.
💡 Options
- Return a sentinel object that still matches the schema (e.g.
shebang: "", executable: false, hookType: "other", lineCount: 0) and set anerrorflag outside the record. - Return an empty string / throw so the model skips the path entirely.
- Add an optional
errorfield to the schema so the repair cycle knows what happened.
As a sample this matters because readers will copy the error-handling pattern.
| const buf = await readFile(filePath); | ||
| const hash = createHash("sha256").update(buf).digest("hex"); | ||
| const sizeBytes = buf.length; | ||
| return JSON.stringify({ hash, sizeBytes }); |
There was a problem hiding this comment.
[/grill-with-docs] Same pattern as 465: hashFile returns { error: "could not read file" } on failure, but the agent's hashes output schema expects s.record(s.object({ hash: s.string, sizeBytes: s.int })). An error entry would fail validation and force a repair cycle, which is misleading to readers about the correct error-handling idiom.
💡 Consistent alternative
Return schema-compatible data on error (e.g. hash: "", sizeBytes: -1) or omit failed entries and let the instructions note this. The hashFile tool is otherwise a clean example of node:crypto usage.
| const key = line.slice(0, eqIdx).trim(); | ||
| const val = line.slice(eqIdx + 1).trim(); | ||
| if (key) result[key] = val; | ||
| } |
There was a problem hiding this comment.
[/grill-with-docs] parseIniSection takes the full file content as a string parameter plus a target section name. The agent must first read the file (via p.readInput), then pass the raw string to the tool for each section — meaning the full content is sent once in the prompt and once per tool call. For large configs this multiplies token usage.
💡 Alternative design
A parseAllSections tool that reads the file path and returns all sections at once would be more efficient and a better demonstration of the "deep tool" principle from /codebase-design: one tool call, rich return value. The current design is useful as an educational example of iterative tool use, but a brief comment acknowledging the trade-off would help readers.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
No final typecheck failures. 8/10 tasks initially failed with
Cannot find module 'rig/addons'— fixed by importingrepair/steeringdirectly from"rig".Tasks run