[rig-tasks] Add 10 rig samples — 2026-08-06 - #360
Conversation
Adds samples covering: - JSON fixture anonymizer (371) - CSV to Markdown table converter (372) - TOML config section analyzer (373) - Git checkpoint summarizer (374) - TypeScript literal union extractor (375) - Markdown heading validator (376) - TypeScript barrel module writer (377) - Git remote metadata inspector (378) - OS environment variable scanner (379) - Sequential commit classifier workflow (380) 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 — requesting changes on 4 correctness issues and 2 design misalignments with SKILL.md conventions.
📋 Key Themes & Highlights
Key Themes
- Schema/implementation mismatch (×2):
379declares avalueparam that is never read;371'sidanonymizer is non-deterministic despite acting like a hash. - SKILL.md convention violations (×1):
375usesp.bash("find ...")for path discovery instead of the prescribedp.glob(pattern). - Output schema accuracy (×1):
378keys branch count per-remote but only fetches it fororigin. - Workflow idiom (×1):
380usesreturn nullas a failure sentinel, which is type-unsafe and unnecessary sincecall()throws on failure. - Validation gap (×1):
376's heading validator never checks that the document starts with H1.
Positive Highlights
- ✅ All 10 samples typecheck cleanly after the PR's own fixes — good discipline.
- ✅ The workflow sample (
380) correctly usesworkflow({ meta, body })withcall,phase, and sequenced agents. - ✅
defineToolconsistently usess.object({ ... })for parameters andas conston literal returns — both SKILL.md requirements. - ✅ Good variety of addon usage:
repair()where retries help,steering()where a final-turn nudge is more appropriate.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 37.9 AIC · ⌖ 4.63 AIC · ⊞ 6.3K
Comment /matt to run again
| description: "Classify an environment variable by its name and value into a category.", | ||
| parameters: s.object({ name: s.string, value: s.string }), | ||
| handler({ name }) { | ||
| if (name === "PATH" || name.endsWith("_PATH") || name.endsWith("_HOME")) return "path" as const; |
There was a problem hiding this comment.
[/grill-with-docs] The value parameter is declared in the tool schema but never used in the handler — the handler only inspects name. This misleads callers who may expect value-based classification.
💡 Fix: drop the unused parameter or use it
Either remove value from the schema:
parameters: s.object({ name: s.string }),
handler({ name }) { ... }Or use it for value-based heuristics (e.g. detecting secret-like tokens by pattern). An unused declared parameter breaks the schema/implementation contract.
| // Agent role: Scan TypeScript files to find and catalog all string literal type union definitions. | ||
| const tsLiteralUnionExtractor = agent({ | ||
| model: "small", | ||
| instructions: p`Find TypeScript files: ${p.bash("find . -name '*.ts' -not -path '*/node_modules/*' | head -30")}. Use extractLiteralUnions on each file to collect all string literal type unions.`, |
There was a problem hiding this comment.
[/grill-with-docs] Uses a find shell command where SKILL.md prescribes p.glob(pattern) for workspace path discovery. This bypasses the declarative intent layer and couples the sample to unix-style shell.
💡 Replace `p.bash("find ...")` with `p.glob`
instructions: p`Find TypeScript files: ${p.glob("**/*.ts")}. Use extractLiteralUnions on each file to collect all string literal type unions.`,SKILL.md: "Discover workspace paths → p.glob(pattern) returns paths only; then delegate one path at a time to a subagent". This also removes the fragile head -30 cap.
|
|
||
| Use classifyRemote for each remote URL and return results keyed by remote name.`, | ||
| output: s.record(s.object({ | ||
| url: s.string, |
There was a problem hiding this comment.
[/grill-with-docs] The branchCount in the output schema is sourced from git ls-remote --heads origin (hardcoded to origin), but results are keyed by remote name — non-origin remotes will always show 0 or a stale value for branchCount.
💡 Suggestion
Either scope the output to a single remote (output: s.object({ url, type, branchCount })) or drop branchCount from per-remote records and report it as a top-level value for origin only. As-is, the sample teaches a misleading pattern where a record field appears per-remote but is actually only computed for one.
| phase("Collect"); | ||
| const step1 = await call(commitCollector, "Collect recent commits."); | ||
| if (!step1) return null; | ||
| phase("Classify"); |
There was a problem hiding this comment.
[/grill-with-docs] return null is used as an early-exit sentinel when call() fails, but null is not in the workflow body's declared return type (the final commitAggregator returns a structured object). This teaches a pattern that will cause a type error if the body's return type is ever validated.
💡 Suggestion
Either throw on unexpected null (making the failure explicit) or propagate the result directly without a null guard, since call() in rig workflows throws on failure rather than returning null. The if (!step1) return null guard is unnecessary and type-unsafe.
phase("Collect");
const step1 = await call(commitCollector, "Collect recent commits.");
phase("Classify");
const step2 = await call(commitClassifier, { commits: step1.commits });
phase("Aggregate");
return call(commitAggregator, step2);| const h1Count = headings.filter((h: { level: number; text: string }) => h.level === 1).length; | ||
| if (h1Count > 1) issues.push(`Multiple H1 headings found (${h1Count})`); | ||
| for (let i = 1; i < headings.length; i++) { | ||
| if (headings[i].level > headings[i - 1].level + 1) { |
There was a problem hiding this comment.
[/grill-with-docs] The heading-skip detection starts at i = 1 comparing against the prior heading, but never checks whether the first heading is H1. A document starting at H2 is silently accepted as valid.
💡 Add a first-heading check
if (headings.length > 0 && headings[0].level !== 1) {
issues.push(`Document does not start with H1 (starts at H${headings[0].level})`);
}This aligns with the stated purpose of "validate heading structure" and makes the tool consistent with common Markdown linting rules.
| const lower = fieldName.toLowerCase(); | ||
| if (lower.includes("email")) return "redacted@example.com" as const; | ||
| if (lower.includes("name")) return "John Doe" as const; | ||
| if (lower.includes("phone")) return "XXX-XXXX" as const; |
There was a problem hiding this comment.
[/grill-with-docs] The id anonymizer uses a non-deterministic hash of the original value's char codes, meaning the same fixture anonymized twice will produce different numeric IDs. This breaks the stated purpose of creating a stable, shareable sanitized fixture.
💡 Use a deterministic transform
Replace the Math.abs(...) formula with a stable hash (e.g. djb2 or a fixed seed), or simply return a constant like "00000000". The current formula is also non-obvious — a deterministic but opaque numeric result is better than a pseudo-random one.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures (fixed before commit)
Task 7 — Unused
import { join } from "node:path"causedTS6133. Removed.Task 9 — Unused
import { homedir } from "node:os"causedTS6133. Removed.Task 10 — Multiple workflow API errors:
callcannot be imported from"rig"— must be destructured from thebodyparameter.agents:is not a validworkflow()field (it's foragent()).metais required inWorkflowSpec.workflow({ meta: {...}, body: async ({ call, phase }) => {...} }).Tasks run