* fix(workflows): agent-skills query keys must match subagent_type Eight workflow files called `gsd-sdk query agent-skills <KEY>` with a key that did not match any `subagent_type` Task() spawns in the same workflow (or any existing `agents/<KEY>.md`): - research-phase.md:45 — gsd-researcher → gsd-phase-researcher - plan-phase.md:36 — gsd-researcher → gsd-phase-researcher - plan-phase.md:38 — gsd-checker → gsd-plan-checker - quick.md:145 — gsd-checker → gsd-plan-checker - verify-work.md:36 — gsd-checker → gsd-plan-checker - new-milestone.md:207 — gsd-synthesizer → gsd-research-synthesizer - new-project.md:63 — gsd-synthesizer → gsd-research-synthesizer - ui-review.md:21 — gsd-ui-reviewer → gsd-ui-auditor - discuss-phase.md:114 — gsd-advisor → gsd-advisor-researcher Effect before this fix: users configuring `agent_skills.<correct-type>` in .planning/config.json got no injection on these paths because the workflow asked the SDK for a different (non-existent) key. The SDK correctly returned "" for the unknown key, which then interpolated as an empty string into the Task() prompt. Silent no-op. The discuss-phase advisor case is a subtle variant — the spawn site uses `subagent_type="general-purpose"` and loads the agent role via `Read(~/.claude/agents/gsd-advisor-researcher.md)`. The injection key must follow the agent identity (gsd-advisor-researcher), not the technical spawn type. This is a follow-up to #2555 — the SDK-side fix in that PR (#2587) only becomes fully effective once the call sites use the right keys. Adds `sdk/src/workflow-agent-skills-consistency.test.ts` as a contract test: every `agent-skills <slug>` invocation in `get-shit-done/workflows/**/*.md` must reference an existing `agents/<slug>.md`. Fails loudly on future key typos. Closes #2615 * test: harden workflow agent-skills regex per review feedback Review (#2616): CodeRabbit flagged the `agent-skills <slug>` pattern as too permissive (can match prose mentions of the string) and the per-line scan as brittle (misses commands wrapped across lines). - Require full `gsd-sdk query agent-skills` prefix before capture + `\b` around the pattern so prose references no longer match. - Scan each file's full content (not line-by-line) so `\s+` can span newlines; resolve 1-based line number from match index. - Add JSDoc on helpers and on QUERY_KEY_PATTERN. Verified: RED against base (`f30da83`) produces the same 9 violations as before; GREEN on fixed tree. --------- Co-authored-by: forfrossen <forfrossensvart@gmail.com>
This commit is contained in:
98
sdk/src/workflow-agent-skills-consistency.test.ts
Normal file
98
sdk/src/workflow-agent-skills-consistency.test.ts
Normal file
@@ -0,0 +1,98 @@
|
||||
/**
|
||||
* Contract test: every `gsd-sdk query agent-skills <slug>` invocation in
|
||||
* `get-shit-done/workflows/**\/*.md` must reference a slug that exists as
|
||||
* `agents/<slug>.md` at the repository root.
|
||||
*
|
||||
* A mismatch produces a silent no-op at runtime — the SDK returns `""` for an
|
||||
* unknown key, and the workflow interpolates the empty string into the spawn
|
||||
* prompt, so any `agent_skills.<correct-slug>` configuration in
|
||||
* `.planning/config.json` is silently ignored. This test prevents regression.
|
||||
*
|
||||
* Related: https://github.com/gsd-build/get-shit-done/issues/2615
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync, readdirSync, statSync } from 'node:fs';
|
||||
import { dirname, join, relative } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const repoRoot = join(__dirname, '..', '..');
|
||||
const workflowsDir = join(repoRoot, 'get-shit-done', 'workflows');
|
||||
const agentsDir = join(repoRoot, 'agents');
|
||||
|
||||
/**
|
||||
* Matches a full `gsd-sdk query agent-skills <slug>` invocation and captures
|
||||
* the slug. Requires a token boundary before `gsd-sdk` and a word boundary
|
||||
* after the slug so that prose references (e.g. documentation mentioning the
|
||||
* string "agent-skills") do not produce false positives. The `\s+` between
|
||||
* tokens accepts newlines, so commands wrapped across lines still match.
|
||||
*/
|
||||
const QUERY_KEY_PATTERN = /\bgsd-sdk\s+query\s+agent-skills\s+([a-z][a-z0-9-]*)\b/g;
|
||||
|
||||
interface QueryUsage {
|
||||
readonly file: string;
|
||||
readonly line: number;
|
||||
readonly slug: string;
|
||||
}
|
||||
|
||||
/** Recursively collects all `.md` file paths under `dir`. */
|
||||
function walkMarkdown(dir: string): string[] {
|
||||
const out: string[] = [];
|
||||
for (const entry of readdirSync(dir)) {
|
||||
const full = join(dir, entry);
|
||||
if (statSync(full).isDirectory()) {
|
||||
out.push(...walkMarkdown(full));
|
||||
} else if (entry.endsWith('.md')) {
|
||||
out.push(full);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/** Returns the set of agent slugs defined by `<slug>.md` files in `dir`. */
|
||||
function collectAgentSlugs(dir: string): Set<string> {
|
||||
return new Set(
|
||||
readdirSync(dir)
|
||||
.filter((name) => name.endsWith('.md'))
|
||||
.map((name) => name.replace(/\.md$/, '')),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts every `gsd-sdk query agent-skills <slug>` usage from the given
|
||||
* markdown files. Runs the regex over each file's full content (not line by
|
||||
* line) so wrapped commands still match, then resolves the 1-based line number
|
||||
* from the match index.
|
||||
*/
|
||||
function collectQueryUsages(files: readonly string[]): QueryUsage[] {
|
||||
const usages: QueryUsage[] = [];
|
||||
for (const file of files) {
|
||||
const content = readFileSync(file, 'utf8');
|
||||
for (const match of content.matchAll(QUERY_KEY_PATTERN)) {
|
||||
const index = match.index ?? 0;
|
||||
const line = content.slice(0, index).split('\n').length;
|
||||
usages.push({ file, line, slug: match[1]! });
|
||||
}
|
||||
}
|
||||
return usages;
|
||||
}
|
||||
|
||||
describe('workflow agent-skills query consistency', () => {
|
||||
it('every `agent-skills <slug>` query refers to an existing `agents/<slug>.md`', () => {
|
||||
const validSlugs = collectAgentSlugs(agentsDir);
|
||||
const workflowFiles = walkMarkdown(workflowsDir);
|
||||
const usages = collectQueryUsages(workflowFiles);
|
||||
const invalid = usages.filter((u) => !validSlugs.has(u.slug));
|
||||
|
||||
const report = invalid
|
||||
.map((u) => ` ${relative(repoRoot, u.file)}:${u.line} — unknown slug '${u.slug}'`)
|
||||
.join('\n');
|
||||
|
||||
expect(
|
||||
invalid,
|
||||
invalid.length
|
||||
? `Found ${invalid.length} agent-skills query keys with no matching agents/<slug>.md:\n${report}`
|
||||
: '',
|
||||
).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user