From 9d0d085a17e696bae8547e4932ded2798d3dd55d Mon Sep 17 00:00:00 2001 From: javeroff Date: Fri, 1 May 2026 12:21:06 -0400 Subject: [PATCH] fix(query/agent-skills): emit raw block instead of JSON-wrapped string (#2917) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(query/agent-skills): emit raw block instead of JSON-wrapped string The CLI dispatcher (`cli.ts`) JSON-stringifies all query handler results via `console.log(JSON.stringify(result.data, null, 2))`. For the `agent-skills` handler this produced a JSON-quoted string literal — e.g. `"\n…"` — which workflows embedded verbatim via `$(gsd-sdk query agent-skills gsd-planner)`, breaking all `` injection into spawned subagent prompts. Fix: add an optional `format: 'json' | 'text'` field to `QueryResult`. When a handler returns `format: 'text'` and `--pick` is not active, the CLI writes the string directly via `process.stdout.write` instead of JSON-stringifying it. `agentSkills` sets `format: 'text'` for non-empty blocks. Regression guard: two new CLI integration tests in `skills.test.ts` spawn the CLI as a child process and assert that (a) a mapped agent type receives the raw XML block on stdout and (b) an unmapped agent type produces the existing JSON empty-string output. Fixes #2914. Co-Authored-By: Claude Opus 4.7 * docs(changelog): add #2917 entry under Unreleased Fixed --------- Co-authored-by: Claude Opus 4.7 --- CHANGELOG.md | 1 + sdk/src/cli.ts | 8 +++- sdk/src/query/skills.test.ts | 79 +++++++++++++++++++++++++++++++++++- sdk/src/query/skills.ts | 8 ++-- sdk/src/query/utils.ts | 10 +++++ 5 files changed, 100 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb5677e5..36237111e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`gsd-sdk query agent-skills` emits raw `` block instead of JSON-wrapped string** — workflows that embed via `$(gsd-sdk query agent-skills )` were receiving a JSON-quoted string literal mid-prompt (e.g. `"\n…"`), silently breaking all `` injection into spawned subagents. The CLI dispatcher now honors an opt-in `format: 'text'` field on `QueryResult` and writes such results raw via `process.stdout.write`; `--pick` always returns JSON regardless. (#2917) - **`sketch --wrap-up` now dispatches correctly** — `/gsd-sketch --wrap-up` was silently no-oping because the flag dispatch wiring was omitted when the micro-skill entry point was absorbed in #2790. (#2949) ### Added — 1.40.0-rc.1 diff --git a/sdk/src/cli.ts b/sdk/src/cli.ts index 367b0cd77..0bcfff519 100644 --- a/sdk/src/cli.ts +++ b/sdk/src/cli.ts @@ -443,7 +443,13 @@ export async function main(argv: string[] = process.argv.slice(2)): Promise XML block workflows embed via $(...) substitution). + if (!pickField && result.format === 'text' && typeof output === 'string') { + process.stdout.write(output); + } else { + console.log(JSON.stringify(output, null, 2)); + } } } catch (err) { if (err instanceof GSDError) { diff --git a/sdk/src/query/skills.test.ts b/sdk/src/query/skills.test.ts index db5f42add..29afe4c4a 100644 --- a/sdk/src/query/skills.test.ts +++ b/sdk/src/query/skills.test.ts @@ -8,11 +8,15 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; -import { join } from 'node:path'; -import { tmpdir } from 'node:os'; +import { execSync } from 'node:child_process'; +import { join, resolve } from 'node:path'; +import { tmpdir, homedir } from 'node:os'; +import { fileURLToPath } from 'node:url'; import { agentSkills } from './skills.js'; +const CLI = resolve(fileURLToPath(import.meta.url), '../../../dist/cli.js'); + async function writeSkill(rootDir: string, name: string) { const skillDir = join(rootDir, name); await mkdir(skillDir, { recursive: true }); @@ -120,4 +124,75 @@ describe('agentSkills', () => { const r = await agentSkills(['gsd-planner'], tmpDir); expect(r.data).toBe(''); }); + + it('signals format:"text" for non-empty blocks (used by CLI dispatcher)', async () => { + await writeSkill(join(tmpDir, '.claude', 'skills'), 'a-skill'); + await writeConfig(tmpDir, { + agent_skills: { 'gsd-planner': '.claude/skills/a-skill' }, + }); + + const r = await agentSkills(['gsd-planner'], tmpDir); + expect(r.format).toBe('text'); + }); + + it('does not signal format:"text" for empty result', async () => { + const r = await agentSkills(['gsd-planner'], tmpDir); + expect(r.format).toBeUndefined(); + }); +}); + +// ─── CLI stdout integration ───────────────────────────────────────────────── +// Regression guard for the JSON-wrapping bug (#2914): the CLI must emit the +// raw block to stdout, not a JSON-quoted string. Spawns the +// CLI as a child process so the full dispatch path (including cli.ts format +// handling) is exercised. + +describe('agentSkills CLI stdout', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'gsd-skills-cli-')); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + it('writes raw block to stdout — not JSON-wrapped', async () => { + const skillDir = join(tmpDir, '.claude', 'skills', 'cli-skill'); + await mkdir(skillDir, { recursive: true }); + await writeFile(join(skillDir, 'SKILL.md'), '# cli-skill\n'); + await mkdir(join(tmpDir, '.planning'), { recursive: true }); + await writeFile( + join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ agent_skills: { 'gsd-planner': '.claude/skills/cli-skill' } }), + ); + + const stdout = execSync( + `node "${CLI}" query --project-dir "${tmpDir}" agent-skills gsd-planner`, + { encoding: 'utf-8' }, + ); + + expect(stdout).toBe( + '\nRead these user-configured skills:\n- @.claude/skills/cli-skill/SKILL.md\n', + ); + }); + + it('emits empty output (no JSON null) when agent type is unmapped', async () => { + await mkdir(join(tmpDir, '.planning'), { recursive: true }); + await writeFile( + join(tmpDir, '.planning', 'config.json'), + JSON.stringify({ agent_skills: { 'gsd-executor': ['.claude/skills/foo'] } }), + ); + + const stdout = execSync( + `node "${CLI}" query --project-dir "${tmpDir}" agent-skills gsd-planner`, + { encoding: 'utf-8' }, + ); + + // Unmapped agent → empty string → CLI falls through to JSON (""), not raw + // text. This is acceptable: workflows that embed an empty var are no-ops. + // The important invariant is that a MAPPED agent never gets JSON-wrapped. + expect(stdout.trim()).toBe('""'); + }); }); diff --git a/sdk/src/query/skills.ts b/sdk/src/query/skills.ts index 2ee4f8dd1..43168b0c3 100644 --- a/sdk/src/query/skills.ts +++ b/sdk/src/query/skills.ts @@ -123,7 +123,9 @@ export const agentSkills: QueryHandler = async (args, projectDir) => { if (validEntries.length === 0) return { data: '' }; const lines = validEntries.map((e) => `- @${e.ref}`).join('\n'); - return { - data: `\nRead these user-configured skills:\n${lines}\n`, - }; + const block = `\nRead these user-configured skills:\n${lines}\n`; + // Signal the CLI dispatcher to write raw text — workflows embed the result + // with `$(gsd-sdk query agent-skills …)` and need the XML block verbatim, not + // a JSON-quoted string (see cli.ts QueryResult.format handling). + return { data: block, format: 'text' }; }; diff --git a/sdk/src/query/utils.ts b/sdk/src/query/utils.ts index 5781e9adb..a2c5b8a6e 100644 --- a/sdk/src/query/utils.ts +++ b/sdk/src/query/utils.ts @@ -24,6 +24,16 @@ import { GSDError, ErrorClassification } from '../errors.js'; /** Structured result returned by all query handlers. */ export interface QueryResult { data: T; + /** + * Output format hint for the CLI dispatcher. + * `'text'` — write `data` as-is to stdout (no JSON-stringify). + * `'json'` (default) — JSON-stringify as usual. + * + * Only meaningful when `data` is a string and the consumer is the CLI. + * Used by `agent-skills` so workflows embedding `$(gsd-sdk query …)` receive + * a raw `` XML block rather than a JSON-quoted string. + */ + format?: 'json' | 'text'; } /** Signature for a query handler function. */