Merge branch 'next' into kimi-runtime-support

2026-06-09 10:15:41 -04:00
parent c6c3a51277 86845340dc
commit 27aca73b6e
22 changed files with 641 additions and 104 deletions
--- a/tests/bug-2808-skill-hyphen-name.test.cjs
+++ b/tests/bug-2808-skill-hyphen-name.test.cjs
@@ -175,7 +175,8 @@ describe('bug-2808: SKILL.md name: uses hyphen form', () => {
    // Use the real COMMANDS_DIR as the source via .gsd-source marker.
    // installRuntimeArtifacts('claude', configDir, 'global') writes to
    // configDir/skills/ using the same converter as the shim did.
-    // With the full profile, skills are nested: gsd-ns-<router>/skills/<stem>/SKILL.md
+    // With the full profile (#924 fix), skills are FLAT: gsd-<stem>/SKILL.md
+    // (nested layout reverted for Claude — Claude Code scans only one level).
    const configDir = path.join(tmp, 'config');
    fs.mkdirSync(configDir, { recursive: true });
    fs.writeFileSync(path.join(configDir, '.gsd-source'), COMMANDS_DIR + '\n');
--- a/tests/bug-924-claude-flat-skill-layout.test.cjs
+++ b/tests/bug-924-claude-flat-skill-layout.test.cjs
@@ -0,0 +1,189 @@
+// allow-test-rule: source-text-is-the-product
+// Reads installed SKILL.md files from a real install run —
+// testing their on-disk layout tests the deployed contract.
+
+/**
+ * Regression test for bug #924.
+ *
+ * PR #883 accidentally nested concrete gsd-* skills 3 levels deep for the
+ * Claude global install:
+ *
+ *   ~/.claude/skills/gsd-ns-<router>/skills/<stem>/SKILL.md
+ *
+ * Claude Code's skills discovery scans only ONE level under ~/.claude/skills/,
+ * so nested concretes were never listed in the Skill-tool available-skills list.
+ * Direct `Skill(skill="gsd-plan-phase")` calls stopped working.
+ *
+ * Fix: revert Claude to the FLAT layout — concrete skills at the top level:
+ *
+ *   ~/.claude/skills/gsd-<name>/SKILL.md
+ *
+ * The 6 ns-* routers are also top-level entries in the flat layout (they are
+ * concrete skills themselves). No nested skills/ subdirs for Claude.
+ *
+ * Other 6 runtimes (cline, qwen, hermes, augment, trae, antigravity) stay nested.
+ */
+
+'use strict';
+
+process.env.GSD_TEST_MODE = '1';
+
+const { describe, test, before, after } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const os = require('node:os');
+
+const ROOT = path.join(__dirname, '..');
+const COMMANDS_GSD = path.join(ROOT, 'commands', 'gsd');
+
+const { installRuntimeArtifacts } = require('../bin/install.js');
+const { cleanup } = require('./helpers.cjs');
+const {
+  loadSkillsManifest,
+  resolveProfile,
+} = require('../gsd-core/bin/lib/install-profiles.cjs');
+const { applySurface } = require('../gsd-core/bin/lib/surface.cjs');
+const { resolveRuntimeArtifactLayout } = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs');
+
+const MANIFEST = loadSkillsManifest(COMMANDS_GSD);
+const RESOLVED_FULL = resolveProfile({ modes: ['full'], manifest: MANIFEST });
+
+// ---------------------------------------------------------------------------
+// #924 regression: Claude global install must use FLAT layout
+// ---------------------------------------------------------------------------
+
+describe('bug-924: claude global install uses flat skill layout (concrete skills discoverable)', () => {
+  let tmpDir;
+
+  before(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-924-claude-flat-'));
+    installRuntimeArtifacts('claude', tmpDir, 'global', RESOLVED_FULL);
+  });
+
+  after(() => {
+    if (tmpDir) {
+      try { cleanup(tmpDir); } catch { /* best-effort */ }
+    }
+  });
+
+  test('claude global: concrete skills are at the TOP LEVEL of skills/ (flat, directly discoverable)', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+    assert.ok(fs.existsSync(skillsDir), `skills/ dir must exist under ${tmpDir}`);
+
+    const topLevel = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
+
+    // Flat layout must have MANY more than 6 top-level gsd-* entries (concrete skills).
+    // Pre-#924-fix nested layout had exactly 6 (only routers). Flat must have >= 60.
+    assert.ok(
+      topLevel.length >= 60,
+      `Claude global must have >= 60 gsd-* top-level skill dirs (concrete flat layout). ` +
+      `Got ${topLevel.length}: [${topLevel.slice(0, 10).join(', ')}${topLevel.length > 10 ? ', …' : ''}]. ` +
+      'Nested layout detected — #924 regression: Claude must be flat.',
+    );
+  });
+
+  test('claude global: gsd-plan-phase is directly at the top level of skills/', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+    const planPhaseDir = path.join(skillsDir, 'gsd-plan-phase');
+    assert.ok(
+      fs.existsSync(path.join(planPhaseDir, 'SKILL.md')),
+      `skills/gsd-plan-phase/SKILL.md must exist at top level for Claude global install. ` +
+      'Concrete skill buried in nested layout — #924 regression.',
+    );
+  });
+
+  test('claude global: gsd-execute-phase is directly at the top level of skills/', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+    assert.ok(
+      fs.existsSync(path.join(skillsDir, 'gsd-execute-phase', 'SKILL.md')),
+      `skills/gsd-execute-phase/SKILL.md must exist at top level for Claude global install.`,
+    );
+  });
+
+  test('claude global: gsd-code-review is directly at the top level of skills/', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+    assert.ok(
+      fs.existsSync(path.join(skillsDir, 'gsd-code-review', 'SKILL.md')),
+      `skills/gsd-code-review/SKILL.md must exist at top level for Claude global install.`,
+    );
+  });
+
+  test('claude global: gsd-ns-workflow is at the top level as a concrete skill (no nested skills/ subdir)', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+    const nsWorkflowDir = path.join(skillsDir, 'gsd-ns-workflow');
+    assert.ok(
+      fs.existsSync(path.join(nsWorkflowDir, 'SKILL.md')),
+      `skills/gsd-ns-workflow/SKILL.md must exist at top level (router as concrete skill).`,
+    );
+
+    // In the FLAT layout, gsd-ns-workflow/ must NOT have a skills/ subdir.
+    // A skills/ subdir means nested layout was applied (the #924 regression).
+    assert.ok(
+      !fs.existsSync(path.join(nsWorkflowDir, 'skills')),
+      `skills/gsd-ns-workflow/skills/ must NOT exist in flat layout (nested layout detected — #924 regression).`,
+    );
+  });
+
+  test('claude global: no concrete skill is nested under gsd-ns-*/skills/<stem>/SKILL.md', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+    const topLevel = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-ns-'));
+
+    for (const nsDir of topLevel) {
+      const nestedSkillsDir = path.join(skillsDir, nsDir, 'skills');
+      assert.ok(
+        !fs.existsSync(nestedSkillsDir),
+        `${nsDir}/skills/ must NOT exist in Claude flat layout (#924 regression: nested layout detected).`,
+      );
+    }
+  });
+});
+
+// ---------------------------------------------------------------------------
+// #924 regression: applySurface on Claude must also preserve flat layout
+// (no re-nesting after surface update)
+// ---------------------------------------------------------------------------
+
+describe('bug-924: applySurface on claude preserves flat layout (no re-nesting)', () => {
+  let tmpDir;
+
+  before(() => {
+    tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-924-surface-'));
+    installRuntimeArtifacts('claude', tmpDir, 'global', RESOLVED_FULL);
+  });
+
+  after(() => {
+    if (tmpDir) {
+      try { cleanup(tmpDir); } catch { /* best-effort */ }
+    }
+  });
+
+  test('claude: applySurface keeps concrete skills at the top level (flat, no re-nesting)', () => {
+    const skillsDir = path.join(tmpDir, 'skills');
+
+    // Sanity: install must produce flat layout (>= 60 top-level gsd-* dirs)
+    const topLevelAfterInstall = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
+    assert.ok(
+      topLevelAfterInstall.length >= 60,
+      `Install must produce flat layout with >= 60 gsd-* dirs. Got ${topLevelAfterInstall.length}.`,
+    );
+
+    // Run applySurface (full surface → full profile)
+    const layout = resolveRuntimeArtifactLayout('claude', tmpDir, 'global');
+    applySurface(tmpDir, layout, MANIFEST);
+
+    // After applySurface: still flat
+    const topLevelAfterSurface = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
+    assert.ok(
+      topLevelAfterSurface.length >= 60,
+      `After applySurface: must still have >= 60 gsd-* top-level dirs (flat). ` +
+      `Got ${topLevelAfterSurface.length}. Re-nesting detected.`,
+    );
+
+    // gsd-plan-phase must remain directly accessible
+    assert.ok(
+      fs.existsSync(path.join(skillsDir, 'gsd-plan-phase', 'SKILL.md')),
+      'After applySurface: gsd-plan-phase/SKILL.md must remain at top level.',
+    );
+  });
+});
--- a/tests/bug-925-context-monitor-hook-event-name.test.cjs
+++ b/tests/bug-925-context-monitor-hook-event-name.test.cjs
@@ -0,0 +1,205 @@
+/**
+ * Regression test for bug #925
+ *
+ * hooks/gsd-context-monitor.js hardcodes `hookEventName: "PostToolUse"` (or
+ * "AfterTool" for Gemini) regardless of which hook event invoked it. Since
+ * PR #821 the same script is also registered under Stop, SubagentStop, and
+ * PreCompact in hooks/hooks.json. Claude Code rejects output whose
+ * hookSpecificOutput.hookEventName doesn't echo the triggering event:
+ *
+ *   "expected Stop but got PostToolUse"
+ *
+ * Fix: derive hookEventName from the parsed stdin payload's `hook_event_name`
+ * field (already available in the data object), falling back to the
+ * Gemini / non-Gemini heuristic for runtimes that don't send it.
+ */
+
+'use strict';
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const os = require('node:os');
+const path = require('node:path');
+const { execFileSync } = require('node:child_process');
+
+const MONITOR_PATH = path.join(__dirname, '..', 'hooks', 'gsd-context-monitor.js');
+
+/**
+ * Write a bridge metrics file and invoke the context monitor with the given
+ * payload fields. Returns the parsed stdout object (or null if the hook
+ * produced no output).
+ *
+ * remainingPct must be <= 35 to cross the WARNING threshold so the hook
+ * actually emits output.
+ */
+function runMonitor({ hookEventName, sessionId, remainingPct = 30, usedPct = 70, env = {} }) {
+  const bridgePath = path.join(os.tmpdir(), `claude-ctx-${sessionId}.json`);
+  fs.writeFileSync(bridgePath, JSON.stringify({
+    session_id: sessionId,
+    remaining_percentage: remainingPct,
+    used_pct: usedPct,
+    timestamp: Math.floor(Date.now() / 1000),
+  }));
+
+  const payload = { session_id: sessionId, cwd: os.tmpdir() };
+  if (hookEventName !== undefined) {
+    payload.hook_event_name = hookEventName;
+  }
+
+  let stdout = '';
+  try {
+    stdout = execFileSync(process.execPath, [MONITOR_PATH], {
+      input: JSON.stringify(payload),
+      encoding: 'utf-8',
+      timeout: 5000,
+      env: { ...process.env, ...env },
+    });
+  } catch (e) {
+    stdout = e.stdout || '';
+  } finally {
+    try { fs.unlinkSync(bridgePath); } catch { /* noop */ }
+    try {
+      fs.unlinkSync(path.join(os.tmpdir(), `claude-ctx-${sessionId}-warned.json`));
+    } catch { /* noop */ }
+  }
+
+  if (!stdout) return null;
+  return JSON.parse(stdout);
+}
+
+function makeSessionId(suffix) {
+  return `test-925-${suffix}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
+}
+
+// ─── hookEventName echoing ────────────────────────────────────────────────────
+
+describe('bug #925: context monitor echoes the invoking hook event name', () => {
+  test('hookEventName is "Stop" when payload contains hook_event_name: "Stop"', () => {
+    const out = runMonitor({ hookEventName: 'Stop', sessionId: makeSessionId('stop') });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold (remaining=30)');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'Stop',
+      `Expected hookEventName "Stop" but got "${out.hookSpecificOutput?.hookEventName}". ` +
+      'The hook must echo the hook_event_name from stdin, not hardcode "PostToolUse".'
+    );
+  });
+
+  test('hookEventName is "SubagentStop" when payload contains hook_event_name: "SubagentStop"', () => {
+    const out = runMonitor({ hookEventName: 'SubagentStop', sessionId: makeSessionId('subagent-stop') });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'SubagentStop',
+      `Expected hookEventName "SubagentStop" but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+
+  test('hookEventName is "PreCompact" when payload contains hook_event_name: "PreCompact"', () => {
+    const out = runMonitor({ hookEventName: 'PreCompact', sessionId: makeSessionId('precompact') });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'PreCompact',
+      `Expected hookEventName "PreCompact" but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+
+  test('hookEventName is "PostToolUse" when payload contains hook_event_name: "PostToolUse"', () => {
+    const out = runMonitor({ hookEventName: 'PostToolUse', sessionId: makeSessionId('posttools') });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'PostToolUse',
+      `Expected hookEventName "PostToolUse" but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+});
+
+// ─── Fallback behaviour (no hook_event_name in payload) ──────────────────────
+
+describe('bug #925: context monitor falls back to heuristic when hook_event_name absent', () => {
+  test('falls back to "PostToolUse" when hook_event_name is absent (non-Gemini)', () => {
+    const env = { ...process.env };
+    delete env.GEMINI_API_KEY;
+    const out = runMonitor({
+      hookEventName: undefined,
+      sessionId: makeSessionId('fallback-non-gemini'),
+      env: { GEMINI_API_KEY: '' }, // ensure unset
+    });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'PostToolUse',
+      `Expected fallback "PostToolUse" for non-Gemini but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+
+  test('falls back to "AfterTool" when hook_event_name is absent and GEMINI_API_KEY is set', () => {
+    const out = runMonitor({
+      hookEventName: undefined,
+      sessionId: makeSessionId('fallback-gemini'),
+      env: { GEMINI_API_KEY: 'fake-key-for-test' },
+    });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'AfterTool',
+      `Expected fallback "AfterTool" for Gemini but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+
+  test('falls back to "PostToolUse" when hook_event_name is an empty string (non-Gemini)', () => {
+    const out = runMonitor({
+      hookEventName: '',
+      sessionId: makeSessionId('fallback-empty'),
+      env: { GEMINI_API_KEY: '' },
+    });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'PostToolUse',
+      `Expected fallback "PostToolUse" for empty hook_event_name but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+
+  test('falls back to "PostToolUse" when hook_event_name is whitespace-only (non-Gemini)', () => {
+    // trim() makes "   " → "" which is falsy, so the || fallback fires
+    const out = runMonitor({
+      hookEventName: '   ',
+      sessionId: makeSessionId('fallback-whitespace'),
+      env: { GEMINI_API_KEY: '' },
+    });
+    assert.ok(out, 'hook must emit output when context is below WARNING threshold');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'PostToolUse',
+      `Expected fallback "PostToolUse" for whitespace-only hook_event_name but got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+  });
+});
+
+// ─── Critical threshold also echoes the event name ───────────────────────────
+
+describe('bug #925: critical threshold warning also uses correct hookEventName', () => {
+  test('CRITICAL warning emitted under Stop also echoes "Stop"', () => {
+    const out = runMonitor({
+      hookEventName: 'Stop',
+      sessionId: makeSessionId('critical-stop'),
+      remainingPct: 20,
+      usedPct: 80,
+    });
+    assert.ok(out, 'hook must emit output at critical threshold (remaining=20)');
+    assert.strictEqual(
+      out.hookSpecificOutput?.hookEventName,
+      'Stop',
+      `Expected hookEventName "Stop" at critical threshold, got "${out.hookSpecificOutput?.hookEventName}".`
+    );
+    assert.match(
+      out.hookSpecificOutput?.additionalContext || '',
+      /CONTEXT CRITICAL/,
+      'Output should be a CRITICAL warning at remaining=20'
+    );
+  });
+});
--- a/tests/enh-769-context-fork-effort.install.test.cjs
+++ b/tests/enh-769-context-fork-effort.install.test.cjs
@@ -4,20 +4,28 @@
 // transformation is asserted — not inspected for string presence.

 /**
- * #769 — context:fork + effort: frontmatter on heavy workflow skills.
+ * #769 — effort: frontmatter on heavy workflow skills.
+ * #921 — spawning orchestrators must NOT carry context: fork.
+ *
+ * Context: context:fork was added by #769 to protect context budget, but
+ * plan-phase, execute-phase, and autonomous are spawning orchestrators — a
+ * forked subagent has no Agent/Task tool, breaking their core function.
+ * effort: xhigh is preserved; context: fork is removed from these three.
+ * The converter still passes context: fork through if a source file has it
+ * (for any future leaf skill that legitimately needs isolation).
 *
 * Verifies:
- *   1. Source commands/gsd/autonomous.md has context: fork and effort: xhigh
- *   2. Source commands/gsd/execute-phase.md has context: fork and effort: xhigh
- *   3. Source commands/gsd/plan-phase.md has context: fork and effort: xhigh
+ *   1. Source commands/gsd/autonomous.md does NOT have context: fork, has effort: xhigh
+ *   2. Source commands/gsd/execute-phase.md does NOT have context: fork, has effort: xhigh
+ *   3. Source commands/gsd/plan-phase.md does NOT have context: fork, has effort: xhigh
 *   4. Source commands/gsd/progress.md has effort: low
 *   5. Source commands/gsd/stats.md has effort: low
- *   6. Claude global install: SKILL.md for autonomous has context: fork and effort: xhigh
- *   7. Claude global install: SKILL.md for execute-phase has context: fork and effort: xhigh
- *   8. Claude global install: SKILL.md for plan-phase has context: fork and effort: xhigh
+ *   6. Claude global install: SKILL.md for autonomous has effort: xhigh, NOT context: fork
+ *   7. Claude global install: SKILL.md for execute-phase has effort: xhigh, NOT context: fork
+ *   8. Claude global install: SKILL.md for plan-phase has effort: xhigh, NOT context: fork
 *   9. Claude global install: SKILL.md for progress has effort: low
 *  10. Claude global install: SKILL.md for stats has effort: low
- *  11. convertClaudeCommandToClaudeSkill preserves context: fork field
+ *  11. convertClaudeCommandToClaudeSkill still passes context: fork through (for non-orchestrator skills)
 *  12. convertClaudeCommandToClaudeSkill preserves effort: field
 */

@@ -33,7 +41,12 @@ const os = require('node:os');

 const { install, convertClaudeCommandToClaudeSkill } = require('../bin/install.js');
 const { cleanup } = require('./helpers.cjs');
-const { nestedSkillPath } = require('./helpers/nested-layout.cjs');
+
+// #924: Claude global install is now FLAT — concrete skills are at the top level.
+// flatSkillPath returns: <skillsRoot>/gsd-<stem>/SKILL.md
+function flatSkillPath(skillsRoot, stem) {
+  return path.join(skillsRoot, `gsd-${stem}`, 'SKILL.md');
+}

 const REPO_ROOT = path.resolve(__dirname, '..');
 const SOURCE_COMMANDS_DIR = path.join(REPO_ROOT, 'commands', 'gsd');
@@ -91,11 +104,15 @@ function runClaudeGlobalInstall(claudeHome) {

 // ─── describe 1: Source command files have correct frontmatter ────────────────

-describe('#769 source commands: heavy skills have context: fork and effort: xhigh', () => {
-  test('commands/gsd/autonomous.md has context: fork', () => {
+// #921/#922: spawning orchestrators must NOT carry context: fork — a forked
+// subagent has no Agent/Task tool, making it impossible for orchestrators to
+// spawn their required subagents. context: fork is appropriate only for leaf
+// skills that do not themselves dispatch agents. effort: xhigh is preserved.
+describe('#769/#921 source commands: spawning orchestrators have effort: xhigh but NOT context: fork', () => {
+  test('commands/gsd/autonomous.md does NOT have context: fork (#921)', () => {
    const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'autonomous.md'));
-    assert.match(fm, /^context:[ \t]*fork$/m,
-      `autonomous.md frontmatter must have context: fork\nActual:\n${fm}`);
+    assert.doesNotMatch(fm, /^context:[ \t]*fork$/m,
+      `autonomous.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`);
  });

  test('commands/gsd/autonomous.md has effort: xhigh', () => {
@@ -104,10 +121,10 @@ describe('#769 source commands: heavy skills have context: fork and effort: xhig
      `autonomous.md frontmatter must have effort: xhigh\nActual:\n${fm}`);
  });

-  test('commands/gsd/execute-phase.md has context: fork', () => {
+  test('commands/gsd/execute-phase.md does NOT have context: fork (#921)', () => {
    const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'execute-phase.md'));
-    assert.match(fm, /^context:[ \t]*fork$/m,
-      `execute-phase.md frontmatter must have context: fork\nActual:\n${fm}`);
+    assert.doesNotMatch(fm, /^context:[ \t]*fork$/m,
+      `execute-phase.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`);
  });

  test('commands/gsd/execute-phase.md has effort: xhigh', () => {
@@ -116,10 +133,10 @@ describe('#769 source commands: heavy skills have context: fork and effort: xhig
      `execute-phase.md frontmatter must have effort: xhigh\nActual:\n${fm}`);
  });

-  test('commands/gsd/plan-phase.md has context: fork', () => {
+  test('commands/gsd/plan-phase.md does NOT have context: fork (#921)', () => {
    const fm = readFrontmatter(path.join(SOURCE_COMMANDS_DIR, 'plan-phase.md'));
-    assert.match(fm, /^context:[ \t]*fork$/m,
-      `plan-phase.md frontmatter must have context: fork\nActual:\n${fm}`);
+    assert.doesNotMatch(fm, /^context:[ \t]*fork$/m,
+      `plan-phase.md is a spawning orchestrator and must NOT have context: fork (#921)\nActual:\n${fm}`);
  });

  test('commands/gsd/plan-phase.md has effort: xhigh', () => {
@@ -238,7 +255,9 @@ describe('#769 convertClaudeCommandToClaudeSkill: preserves context and effort f

 // ─── describe 3: Claude global install — SKILL.md files include new fields ────

-describe('#769 Claude global install: SKILL.md files preserve context: fork and effort:', () => {
+// #921/#922: after install, spawning orchestrators must NOT carry context: fork
+// in their emitted SKILL.md. effort: xhigh is still emitted (preserved from source).
+describe('#769/#921 Claude global install: spawning-orchestrator SKILL.md files have effort: xhigh but NOT context: fork', () => {
  let tmpDir;
  let claudeHome;

@@ -252,49 +271,49 @@ describe('#769 Claude global install: SKILL.md files preserve context: fork and
    cleanup(tmpDir);
  });

-  test('gsd-autonomous SKILL.md has context: fork after global install', () => {
+  test('gsd-autonomous SKILL.md does NOT have context: fork after global install (#921)', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'autonomous');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'autonomous');
    const fm = readFrontmatter(skillPath);
-    assert.match(fm, /^context:[ \t]*fork$/m,
-      `gsd-autonomous SKILL.md must have context: fork\nActual:\n${fm}`);
+    assert.doesNotMatch(fm, /^context:[ \t]*fork$/m,
+      `gsd-autonomous is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`);
  });

  test('gsd-autonomous SKILL.md has effort: xhigh after global install', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'autonomous');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'autonomous');
    const fm = readFrontmatter(skillPath);
    assert.match(fm, /^effort:[ \t]*xhigh$/m,
      `gsd-autonomous SKILL.md must have effort: xhigh\nActual:\n${fm}`);
  });

-  test('gsd-execute-phase SKILL.md has context: fork after global install', () => {
+  test('gsd-execute-phase SKILL.md does NOT have context: fork after global install (#921)', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'execute-phase');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'execute-phase');
    const fm = readFrontmatter(skillPath);
-    assert.match(fm, /^context:[ \t]*fork$/m,
-      `gsd-execute-phase SKILL.md must have context: fork\nActual:\n${fm}`);
+    assert.doesNotMatch(fm, /^context:[ \t]*fork$/m,
+      `gsd-execute-phase is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`);
  });

  test('gsd-execute-phase SKILL.md has effort: xhigh after global install', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'execute-phase');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'execute-phase');
    const fm = readFrontmatter(skillPath);
    assert.match(fm, /^effort:[ \t]*xhigh$/m,
      `gsd-execute-phase SKILL.md must have effort: xhigh\nActual:\n${fm}`);
  });

-  test('gsd-plan-phase SKILL.md has context: fork after global install', () => {
+  test('gsd-plan-phase SKILL.md does NOT have context: fork after global install (#921)', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'plan-phase');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'plan-phase');
    const fm = readFrontmatter(skillPath);
-    assert.match(fm, /^context:[ \t]*fork$/m,
-      `gsd-plan-phase SKILL.md must have context: fork\nActual:\n${fm}`);
+    assert.doesNotMatch(fm, /^context:[ \t]*fork$/m,
+      `gsd-plan-phase is a spawning orchestrator; its SKILL.md must NOT have context: fork (#921)\nActual:\n${fm}`);
  });

  test('gsd-plan-phase SKILL.md has effort: xhigh after global install', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'plan-phase');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'plan-phase');
    const fm = readFrontmatter(skillPath);
    assert.match(fm, /^effort:[ \t]*xhigh$/m,
      `gsd-plan-phase SKILL.md must have effort: xhigh\nActual:\n${fm}`);
@@ -302,7 +321,7 @@ describe('#769 Claude global install: SKILL.md files preserve context: fork and

  test('gsd-progress SKILL.md has effort: low after global install', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'progress');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'progress');
    const fm = readFrontmatter(skillPath);
    assert.match(fm, /^effort:[ \t]*low$/m,
      `gsd-progress SKILL.md must have effort: low\nActual:\n${fm}`);
@@ -310,7 +329,7 @@ describe('#769 Claude global install: SKILL.md files preserve context: fork and

  test('gsd-stats SKILL.md has effort: low after global install', () => {
    runClaudeGlobalInstall(claudeHome);
-    const skillPath = nestedSkillPath(path.join(claudeHome, 'skills'), 'gsd-', 'stats');
+    const skillPath = flatSkillPath(path.join(claudeHome, 'skills'),'stats');
    const fm = readFrontmatter(skillPath);
    assert.match(fm, /^effort:[ \t]*low$/m,
      `gsd-stats SKILL.md must have effort: low\nActual:\n${fm}`);
--- a/tests/install-nested-layout.test.cjs
+++ b/tests/install-nested-layout.test.cjs
@@ -32,7 +32,8 @@ const { COMMANDS_GSD, ROUTER_STEMS, routerChildren } = require('./helpers/nested
 // ---------------------------------------------------------------------------

 const NEST = [
-  { runtime: 'claude',      scope: 'global', skillsSub: 'skills',     prefix: 'gsd-' },
+  // Claude reverted to flat (#924: nested layout breaks Skill-tool discovery on Claude Code).
+  // Only the 6 runtimes below keep the nested layout.
  { runtime: 'cline',       scope: 'global', skillsSub: 'skills',     prefix: 'gsd-' },
  { runtime: 'qwen',        scope: 'global', skillsSub: 'skills',     prefix: 'gsd-' },
  { runtime: 'hermes',      scope: 'global', skillsSub: 'skills/gsd', prefix: ''     },
@@ -42,6 +43,9 @@ const NEST = [
 ];

 const FLAT = [
+  // Claude reverted to flat (#924): Claude Code scans only one level under ~/.claude/skills/
+  // so nested concretes were never discoverable by the Skill tool.
+  { runtime: 'claude',    scope: 'global', skillsSub: 'skills' },
  { runtime: 'cursor',    scope: 'global', skillsSub: 'skills' },
  { runtime: 'codex',     scope: 'global', skillsSub: 'skills' },
  { runtime: 'copilot',   scope: 'global', skillsSub: 'skills' },
@@ -200,10 +204,13 @@ for (const { runtime, scope, skillsSub, prefix } of NEST) {
 }

 // ---------------------------------------------------------------------------
-// claude extra: total top-level gsd- count must equal exactly 6
+// claude extra: total top-level gsd- count must be >= 60 (FLAT, #924)
+//
+// Pre-#924 (nested) this block asserted exactly 6 (only routers).
+// Post-#924 (flat) Claude has all concrete skills at the top level.
 // ---------------------------------------------------------------------------

-describe('claude: total top-level gsd- entries == 6', () => {
+describe('claude: total top-level gsd- entries >= 60 (flat layout, #924)', () => {
  let tmpDir;

  before(() => {
@@ -216,15 +223,15 @@ describe('claude: total top-level gsd- entries == 6', () => {
    }
  });

-  test('claude: total top-level gsd- skill entries == 6', () => {
+  test('claude: >= 60 gsd-* top-level skill entries (concrete flat layout, not nested)', () => {
    const skillsDir = path.join(tmpDir, 'skills');
    assert.ok(fs.existsSync(skillsDir), 'skills/ dir must exist');

    const topLevel = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
-    assert.strictEqual(
-      topLevel.length,
-      6,
-      `Expected exactly 6 gsd-* top-level entries under claude/skills, got ${topLevel.length}: [${topLevel.join(', ')}]`,
+    assert.ok(
+      topLevel.length >= 60,
+      `Expected >= 60 gsd-* top-level entries under claude/skills (flat layout after #924 fix). ` +
+      `Got ${topLevel.length}: [${topLevel.slice(0, 10).join(', ')}${topLevel.length > 10 ? ', …' : ''}]`,
    );
  });
 });
--- a/tests/issue-69-surface-keeps-nested.test.cjs
+++ b/tests/issue-69-surface-keeps-nested.test.cjs
@@ -8,6 +8,10 @@
 //
 // Fix (install-profiles.cts): gate nesting on full OR full-equivalent (all routerStems
 // present in the concrete Set) so that the surface path preserves nesting.
+//
+// NOTE: As of #924 Claude has been REVERTED to FLAT. This test now uses Cline as the
+// representative nested runtime. The original claude-global test below is updated to
+// assert the flat layout (>= 60 top-level gsd-* entries, concrete skills discoverable).

 'use strict';

@@ -29,18 +33,19 @@ const { resolveRuntimeArtifactLayout } = require('../gsd-core/bin/lib/runtime-ar
 const { cleanup } = require('./helpers.cjs');

 describe('issue-69: applySurface preserves nested skill layout (no re-flatten)', () => {
-  test('claude global full: applySurface keeps 6 router dirs and nested gsd-ns-workflow/skills/plan-phase/SKILL.md', (t) => {
+  // #924: Claude is now flat; use Cline as the representative nested runtime.
+  test('cline global full: applySurface keeps 6 router dirs and nested gsd-ns-manage/skills/help/SKILL.md', (t) => {
    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-69-surface-'));
    t.after(() => { try { cleanup(tmpDir); } catch { /* best-effort */ } });

    // Step 1: full install
    const manifest = loadSkillsManifest(COMMANDS_GSD);
    const resolved = resolveProfile({ modes: ['full'], manifest });
-    installRuntimeArtifacts('claude', tmpDir, 'global', resolved);
+    installRuntimeArtifacts('cline', tmpDir, 'global', resolved);

    const skillsDir = path.join(tmpDir, 'skills');

-    // Sanity: install must produce nested layout
+    // Sanity: install must produce nested layout (6 top-level router dirs)
    const topLevelAfterInstall = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
    assert.strictEqual(
      topLevelAfterInstall.length,
@@ -53,7 +58,7 @@ describe('issue-69: applySurface preserves nested skill layout (no re-flatten)',
    );

    // Step 2: applySurface (full surface, no surface state file → resolves to full)
-    const layout = resolveRuntimeArtifactLayout('claude', tmpDir, 'global');
+    const layout = resolveRuntimeArtifactLayout('cline', tmpDir, 'global');
    applySurface(tmpDir, layout, manifest);

    // Step 3: assert nested layout is preserved after applySurface
@@ -77,4 +82,50 @@ describe('issue-69: applySurface preserves nested skill layout (no re-flatten)',
      'After applySurface: gsd-plan-phase/ must NOT exist at top level (#69 re-flatten regression guard)',
    );
  });
+
+  // #924 companion: Claude must use FLAT layout and applySurface must NOT re-nest it.
+  test('claude global full: install produces flat layout and applySurface preserves it (#924)', (t) => {
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-924-69-'));
+    t.after(() => { try { cleanup(tmpDir); } catch { /* best-effort */ } });
+
+    const manifest = loadSkillsManifest(COMMANDS_GSD);
+    const resolved = resolveProfile({ modes: ['full'], manifest });
+    installRuntimeArtifacts('claude', tmpDir, 'global', resolved);
+
+    const skillsDir = path.join(tmpDir, 'skills');
+
+    // Install must produce FLAT layout (>= 60 gsd-* dirs)
+    const topLevelAfterInstall = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
+    assert.ok(
+      topLevelAfterInstall.length >= 60,
+      `Claude install must produce >= 60 gsd-* top-level dirs (flat, #924). Got ${topLevelAfterInstall.length}.`,
+    );
+
+    // gsd-plan-phase must be directly at top level
+    assert.ok(
+      fs.existsSync(path.join(skillsDir, 'gsd-plan-phase', 'SKILL.md')),
+      'After claude install: gsd-plan-phase/SKILL.md must be at top level (flat layout, #924)',
+    );
+
+    // No nested skills/ subdirs under gsd-ns-* in Claude
+    assert.ok(
+      !fs.existsSync(path.join(skillsDir, 'gsd-ns-workflow', 'skills')),
+      'After claude install: gsd-ns-workflow/skills/ must NOT exist (flat layout, no nesting, #924)',
+    );
+
+    // applySurface must preserve flat layout
+    const layout = resolveRuntimeArtifactLayout('claude', tmpDir, 'global');
+    applySurface(tmpDir, layout, manifest);
+
+    const topLevelAfterSurface = fs.readdirSync(skillsDir).filter((n) => n.startsWith('gsd-'));
+    assert.ok(
+      topLevelAfterSurface.length >= 60,
+      `After applySurface: claude must still have >= 60 gsd-* dirs (flat preserved). Got ${topLevelAfterSurface.length}.`,
+    );
+
+    assert.ok(
+      fs.existsSync(path.join(skillsDir, 'gsd-plan-phase', 'SKILL.md')),
+      'After applySurface: gsd-plan-phase/SKILL.md must remain at top level (#924)',
+    );
+  });
 });
--- a/tests/plan-phase-drift-guard.test.cjs
+++ b/tests/plan-phase-drift-guard.test.cjs
@@ -193,3 +193,70 @@ describe('plan-phase workflow: top-level spawn guard (#913)', () => {
    );
  });
 });
+
+// ─── (D) Attempt-based Agent gate (#922) ─────────────────────────────────────
+
+describe('plan-phase workflow: attempt-based Agent availability gate (#922)', () => {
+  // Extract the runtime_compatibility block for targeted assertions
+  const rtBlock = (() => {
+    const m = workflow.match(/<runtime_compatibility>([\s\S]*?)<\/runtime_compatibility>/);
+    return m ? m[1] : '';
+  })();
+
+  // Extract the "Other runtimes" clause specifically
+  const otherRuntimesClause = (() => {
+    const m = rtBlock.match(/\*\*Other runtimes[^*]*\*\*[^\n]*\n([\s\S]*?)(?=\n\*\*|$)/);
+    return m ? m[0] : rtBlock;
+  })();
+
+  test('Other runtimes clause does not authorize stopping on a self-assessed absence (#922)', () => {
+    // The pre-#922 wording ("if the Agent tool is genuinely absent") let the model
+    // self-assess and stop without ever attempting a call. The fixed wording must
+    // not contain phrasing that authorizes that pattern.
+    const forbiddenPatterns = [
+      /if the Agent tool is genuinely absent/i,
+      /if.*Agent.*genuinely absent/i,
+    ];
+    for (const pattern of forbiddenPatterns) {
+      assert.ok(
+        !pattern.test(otherRuntimesClause),
+        `plan-phase "Other runtimes" clause must not authorize stopping on a self-assessed Agent absence — ` +
+        `use attempt-based gate instead (#922). Found: ${otherRuntimesClause.trim()}`
+      );
+    }
+  });
+
+  test('Other runtimes clause pins "Always attempt the actual Agent() call" language (#922)', () => {
+    // Pin the exact contract phrase so a future edit that changes to "try to determine
+    // availability" or "check if Agent is available" does not silently reintroduce introspection.
+    assert.ok(
+      otherRuntimesClause.includes('Always attempt the actual') ||
+      otherRuntimesClause.includes('always attempt the actual'),
+      `plan-phase "Other runtimes" clause must pin "Always attempt the actual Agent() call" (or equivalent) (#922). ` +
+      `Found: ${otherRuntimesClause.trim()}`
+    );
+  });
+
+  test('Other runtimes clause pins "real tool-unavailable error" as the only valid stop signal (#922)', () => {
+    // Must tie the stop to a real returned error, not a self-assessed absence.
+    assert.ok(
+      otherRuntimesClause.includes('real tool-unavailable error') ||
+      otherRuntimesClause.includes('tool-unavailable error returned'),
+      `plan-phase "Other runtimes" clause must state only a real tool-unavailable error from Agent() authorizes stopping (#922). ` +
+      `Found: ${otherRuntimesClause.trim()}`
+    );
+  });
+
+  test('Other runtimes clause still prohibits inline role collapse (#922 preserves #913)', () => {
+    // Even after the attempt-based rewrite the clause must keep the no-inline-collapse guard.
+    const hasNoInline =
+      otherRuntimesClause.toLowerCase().includes('do not') &&
+      (otherRuntimesClause.toLowerCase().includes('inline') ||
+       otherRuntimesClause.toLowerCase().includes('collapse'));
+    assert.ok(
+      hasNoInline,
+      `plan-phase "Other runtimes" clause must still prohibit inline role collapse even with the attempt-based gate (#922). ` +
+      `Found: ${otherRuntimesClause.trim()}`
+    );
+  });
+});
--- a/tests/runtime-artifact-layout.test.cjs
+++ b/tests/runtime-artifact-layout.test.cjs
@@ -441,7 +441,7 @@ describe('stage — skills kind (claude global)', () => {
    assert.ok(entries.length >= 1, 'at least one skill dir should be staged');
  });

-  test('stage with skills="*" nests all commands/gsd/*.md under 6 routers (claude)', () => {
+  test('stage with skills="*" produces flat layout for claude (#924: reverted from nested)', () => {
    const layout = resolveRuntimeArtifactLayout('claude', FAKE_STAGE_DIR, 'global');
    const skillsKind = layout.kinds.find(k => k.kind === 'skills');
    assert.ok(skillsKind, 'should have a skills kind');
@@ -449,32 +449,22 @@ describe('stage — skills kind (claude global)', () => {
    const stagedDir = skillsKind.stage(PROFILE_FULL);
    assert.ok(fs.existsSync(stagedDir), 'stagedDir must exist');

-    // Claude is a NESTING runtime: full profile produces exactly 6 gsd-ns-* router dirs.
+    // #924: Claude is reverted to FLAT. Full profile produces >= 60 top-level gsd-* dirs.
+    // (Previously nested: exactly 6 gsd-ns-* router dirs. That broke Skill-tool discovery.)
    const topEntries = fs.readdirSync(stagedDir);
-    assert.strictEqual(topEntries.length, 6, `full profile should have exactly 6 router dirs, got ${topEntries.length}`);
+    assert.ok(
+      topEntries.length >= 60,
+      `full profile should have >= 60 top-level skill dirs (flat layout, #924), got ${topEntries.length}`,
+    );
    for (const entry of topEntries) {
-      assert.ok(entry.startsWith('gsd-ns-'), `top-level entry should be a gsd-ns-* router: ${entry}`);
-      // Each router has its own SKILL.md.
-      const routerSkillMd = path.join(stagedDir, entry, 'SKILL.md');
-      assert.ok(fs.existsSync(routerSkillMd), `router SKILL.md must exist in ${entry}`);
-      // Each router has a skills/ subdirectory with nested children.
+      assert.ok(entry.startsWith('gsd-'), `entry should start with gsd-: ${entry}`);
+      // Each skill dir has its own SKILL.md at the top level.
+      const skillMd = path.join(stagedDir, entry, 'SKILL.md');
+      assert.ok(fs.existsSync(skillMd), `SKILL.md must exist at top level in ${entry}`);
+      // No nested skills/ subdirectory: flat layout means no nesting.
      const skillsSubdir = path.join(stagedDir, entry, 'skills');
-      assert.ok(fs.existsSync(skillsSubdir), `skills/ subdir must exist in ${entry}`);
-      assert.ok(fs.statSync(skillsSubdir).isDirectory(), `${entry}/skills must be a directory`);
+      assert.ok(!fs.existsSync(skillsSubdir), `skills/ subdir must NOT exist in ${entry} (flat layout, #924)`);
    }
-
-    // Total SKILL.md files across all routers + nested children must be large (proves no skill was dropped).
-    function countSkillMdFiles(dir) {
-      let count = 0;
-      for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
-        const fullPath = path.join(dir, entry.name);
-        if (entry.isDirectory()) count += countSkillMdFiles(fullPath);
-        else if (entry.name === 'SKILL.md') count++;
-      }
-      return count;
-    }
-    const totalSkillMd = countSkillMdFiles(stagedDir);
-    assert.ok(totalSkillMd >= 60, `full profile should have >= 60 total SKILL.md files (routers + children), got ${totalSkillMd}`);
  });
 });

--- a/tests/workflow-size-budget.test.cjs
+++ b/tests/workflow-size-budget.test.cjs
@@ -82,7 +82,7 @@ const GRACE = 3000;
 // XL high-water mark is execute-phase.md — note that under LINES it was
 // plan-phase; bytes genuinely re-rank the tier, which is the point of #717.
 // actualMax=92525 (execute-phase, #913 inline-fallback scope clarification);
-// slack=475 ≤ GRACE. plan-phase.md=90501 (#913 runtime_compatibility block + label rename), new-project.md=58110.
+// slack=475 ≤ GRACE. plan-phase.md=90748 (#922 attempt-based Agent gate), new-project.md=58110.
 const XL_BUDGET = 93000;
 // LARGE high-water mark is docs-update.md. actualMax=54410 (#891 launcher shim expansion);
 // slack=1590 ≤ GRACE. quick.md=45710, autonomous.md=38030.
@@ -96,7 +96,7 @@ const DEFAULT_BUDGET = 40000;
 // pattern that future shrinks should follow. Byte counts noted for reference.
 const XL_WORKFLOWS = new Set([
  'execute-phase',  // 92525 bytes (tier high-water mark; grew in #913 inline-fallback scope clarification)
-  'plan-phase',     // 90501 bytes (grew in #913 runtime_compatibility block + label rename)
+  'plan-phase',     // 90748 bytes (grew in #922 attempt-based Agent gate)
  'new-project',    // 55850 bytes
 ]);