fix(#851): correct Codex quick adapter for generic multi_agent_v1 schema (#958)

* fix(#851): correct Codex adapter for generic multi_agent_v1 schema The Codex skill adapter header in getCodexSkillAdapterHeader() documented typed spawn_agent(agent_type=...) as a direct, unconditional mapping for all Task()/Agent() calls. In sessions exposing only the generic multi_agent_v1 schema (message/items/fork_context — no agent_type field), this mapping is silently invalid: the orchestrator cannot natively dispatch typed gsd-planner/ gsd-executor agents and may fall back to inline execution or produce errors. Fix: Section C now requires schema detection before spawning. It documents the typed mapping as conditional on the agent_type-capable schema (e.g. multi_agent_v2) and introduces an explicitly-labeled generic-agent workaround for multi_agent_v1 sessions — read the agent TOML, inject its instructions as a role-preamble, and call spawn_agent(message=...) — clearly marking the result as NOT equivalent to typed gsd-planner/gsd-executor execution. Regression test: tests/bug-851-codex-quick-adapter-agent-type-fallback.test.cjs asserts schema-awareness language, the multi_agent_v1 fallback, the workaround label, and backward compat with the existing bug-279 typed-spawn contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore: add changeset for PR #958 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#851): keep Codex adapter block consistent with materialized skill surface The `~/.codex/agents/<agent-name>.toml` literal introduced in the #851 prose was being rewritten to the real install path by `_applyRuntimeRewrites` (the `~/.codex/` → pathPrefix substitution) before the SKILL.md was written to disk. `getCodexSkillAdapterHeader()` still returned `~/.codex/agents/...` so the test assertion (exact match between builder output and materialized file) always failed. Fix: replace the `~/.codex/agents/` literal with the runtime-neutral form `agents/<agent-name>.toml` plus a parenthetical naming `$CODEX_HOME/` — which is not matched by any rewrite pattern and survives the path-substitution step unchanged. The #851 schema-detection + generic-subagent-fallback intent is fully preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(#851): resolve active Codex config root in fallback; strengthen tests (adversarial review) - Rewrites the generic-agent workaround step 1 to explicitly describe active config root resolution (priority: $CODEX_HOME → --config-dir → --local .codex → default global dir) without the literal ~/.codex/ substring that _applyRuntimeRewrites replaces, preventing bug-3582 divergence. - Replaces OR/loose-includes test assertions in bug-851 with AND-logic checks covering all four required elements: (a) schema-detection step, (b) active-config-root resolution for the TOML path including all three override mechanisms, (c) NOT-equivalent-to-typed-gsd-planner/gsd-executor label, and (d) fail-closed rule when typed dispatch is mandatory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(#851): register bug-851/947/948/950 in lint-regression-test-names allowlist The ratchet (622e4be) bans NEW top-level bug-NNNN test files; the four sibling PRs (#851, #947, #948, #950) landed AFTER the baseline was cut, so their test files were not yet grandfathered. Add all four to the identity allowlist so lint-regression-test-names passes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-10 00:45:27 -04:00
parent 1c86368785
commit e8cfb560b2
4 changed files with 250 additions and 2 deletions
--- a/.changeset/noble-tigers-tumble.md
+++ b/.changeset/noble-tigers-tumble.md
@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 958
+---
+**`$gsd-quick` Codex adapter no longer assumes typed `spawn_agent(agent_type=...)`** — documents that typed planner/executor spawning needs the agent_type-capable Codex schema and provides a clearly-labeled generic-subagent fallback when only `multi_agent_v1` is exposed.
--- a/bin/install.js
+++ b/bin/install.js
@@ -3449,7 +3449,13 @@ Execute mode fallback:
 ## C. Task() → spawn_agent Mapping
 GSD workflows use \`Task(...)\` (Claude Code syntax). Translate to Codex collaboration tools:

-Direct mapping:
+**Schema detection (required first step):** Codex exposes two \`spawn_agent\` schemas:
+- **agent_type-capable schema** (e.g. \`multi_agent_v2\`): \`spawn_agent\` accepts \`agent_type\`, \`message\`, \`reasoning_effort\`, \`fork_context\`, etc. — typed GSD agent dispatch is available.
+- **Generic schema** (\`multi_agent_v1\`): \`spawn_agent\` accepts only \`message\`, \`items\`, \`fork_context\` — there is **no \`agent_type\` field**. Typed GSD agent dispatch is unavailable in this session.
+
+Before spawning, inspect the \`spawn_agent\` tool's visible parameter schema (via \`tool_search\` or the tool list) to determine which form is active.
+
+Typed mapping (agent_type-capable schema only):
 - \`Task(subagent_type="X", prompt="Y")\` → \`spawn_agent(agent_type="X", message="Y")\`
 - \`Agent(subagent_type="X", prompt="Y")\` → \`spawn_agent(agent_type="X", message="Y")\`
 - \`Task(model="...")\` → omit. \`spawn_agent\` has no inline \`model\` parameter;
@@ -3466,6 +3472,24 @@ Direct mapping:
  Workflows that require this isolation must fail closed or use an explicit
  manual worktree protocol before spawning (#3360).

+Generic-agent workaround (multi_agent_v1 schema — NO agent_type field):
+When only the generic \`multi_agent_v1\` schema is available, typed GSD agent dispatch
+(\`gsd-planner\`, \`gsd-executor\`, etc.) is NOT possible. This is a known Codex limitation
+(openai/codex#15250). **This workaround is NOT equivalent to typed gsd-planner/gsd-executor
+execution** — GSD agents carry project-aware prompts, audit logging, and workflow context
+that a generic subagent lacks. Use the following fallback:
+1. Resolve your active Codex config root — the directory that contains your \`config.toml\`.
+   This directory is determined in priority order: \`$CODEX_HOME\` (if set), the path given
+   by \`--config-dir\` (if passed on invocation), a local \`.codex\` directory in the current
+   project (if \`--local\` was used), or the default global config directory. Read
+   \`agents/<agent-name>.toml\` relative to that config root to extract the agent's system
+   instructions.
+2. Inject those instructions as a role-preamble into a generic \`spawn_agent(message=...)\` call.
+3. Label results and logs clearly as "generic-agent workaround" so the orchestrator and user
+   know full typed-agent guarantees are not in effect.
+4. Where typed dispatch is mandatory for correctness (e.g. worktree isolation), fail closed
+   and report the schema limitation rather than silently degrading.
+
 Spawn restriction:
 - Codex restricts \`spawn_agent\` to cases where the user has explicitly
  requested sub-agents. When automatic spawning is not permitted, do the
--- a/scripts/lint-regression-test-names.allowlist.json
+++ b/scripts/lint-regression-test-names.allowlist.json
@@ -247,6 +247,7 @@
  "bug-730-milestone-phase-details-scope.test.cjs",
  "bug-782-cline-skills-emission.test.cjs",
  "bug-783-kilo-global-skills-base.test.cjs",
+  "bug-851-codex-quick-adapter-agent-type-fallback.test.cjs",
  "bug-853-bg-dispatch-runtime-gating.test.cjs",
  "bug-866-profile-pipeline-temp-root.test.cjs",
  "bug-891-non-claude-runtime-home-fallback.test.cjs",
@@ -255,5 +256,8 @@
  "bug-924-claude-flat-skill-layout.test.cjs",
  "bug-925-context-monitor-hook-event-name.test.cjs",
  "bug-936-no-nested-spawner-wrap.test.cjs",
-  "bug-941-managed-hooks-registry-manifest.test.cjs"
+  "bug-941-managed-hooks-registry-manifest.test.cjs",
+  "bug-947-hermes-gsd-prefix.test.cjs",
+  "bug-948-state-noop-write-guard.test.cjs",
+  "bug-950-quick-summary-status-complete.test.cjs"
 ]
--- a/tests/bug-851-codex-quick-adapter-agent-type-fallback.test.cjs
+++ b/tests/bug-851-codex-quick-adapter-agent-type-fallback.test.cjs
@@ -0,0 +1,215 @@
+// allow-test-rule: source-text-is-the-product
+// Tests assert on text in bin/install.js (Codex adapter header prose) —
+// the adapter text IS the product loaded by Codex agents at runtime.
+
+'use strict';
+
+const { test, describe } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const INSTALL_JS = path.join(__dirname, '..', 'bin', 'install.js');
+const src = fs.readFileSync(INSTALL_JS, 'utf8');
+
+// Helper: extract Section C from the raw source text.
+// Anchors on the heading and ends at </codex_skill_adapter>.
+function getSectionC() {
+  const headingIdx = src.indexOf('## C. Task() → spawn_agent Mapping');
+  assert.ok(headingIdx >= 0, 'Section C heading must exist in bin/install.js');
+  const closeTag = src.indexOf('</codex_skill_adapter>', headingIdx);
+  assert.ok(closeTag >= 0, 'Section C must be followed by </codex_skill_adapter>');
+  return src.slice(headingIdx, closeTag);
+}
+
+describe('bug #851: Codex adapter documents multi_agent_v1 schema limitation and fallback', () => {
+
+  // (a) Schema-detection step: the adapter must require the agent to inspect
+  //     spawn_agent's parameter schema BEFORE deciding how to dispatch.
+  test('(a) schema-detection: adapter requires inspecting spawn_agent schema before dispatching', () => {
+    const sectionC = getSectionC();
+
+    // Must name BOTH schema variants so the agent knows what to look for
+    assert.ok(
+      sectionC.includes('multi_agent_v1'),
+      'Section C must name the multi_agent_v1 schema to identify the limited form',
+    );
+    assert.ok(
+      sectionC.includes('multi_agent_v2') || sectionC.includes('agent_type-capable'),
+      'Section C must name the typed schema (multi_agent_v2 or agent_type-capable) as the capable form',
+    );
+
+    // Must instruct schema inspection before spawning
+    assert.ok(
+      sectionC.includes('tool_search') || sectionC.includes('inspect') || sectionC.includes('schema'),
+      'Section C must instruct the agent to inspect the spawn_agent schema (via tool_search or similar)',
+    );
+
+    // All three requirements together (AND):
+    assert.ok(
+      sectionC.includes('multi_agent_v1') &&
+      (sectionC.includes('multi_agent_v2') || sectionC.includes('agent_type-capable')) &&
+      (sectionC.includes('tool_search') || sectionC.includes('inspect') || sectionC.includes('schema')),
+      'Section C must require schema-detection: name both schema variants AND instruct inspection before spawning',
+    );
+  });
+
+  // (b) Active-config-root resolution: the TOML path must describe how to
+  //     resolve the config root (honoring $CODEX_HOME / --config-dir / --local),
+  //     not imply a single fixed path.
+  test('(b) active-config-root: fallback TOML path resolves the active Codex config root', () => {
+    const sectionC = getSectionC();
+
+    // Must mention the agents/<agent-name>.toml relative path
+    assert.ok(
+      sectionC.includes('agents/<agent-name>.toml'),
+      'Section C must reference agents/<agent-name>.toml for the TOML extraction step',
+    );
+
+    // Must describe dynamic config-root resolution (at least two of the three
+    // override mechanisms, plus the word "config" to anchor context)
+    const mentionsCodexHome = sectionC.includes('$CODEX_HOME') || sectionC.includes('CODEX_HOME');
+    const mentionsConfigDir = sectionC.includes('--config-dir') || sectionC.includes('config-dir');
+    const mentionsLocal = sectionC.includes('--local') || sectionC.includes('.codex') || sectionC.includes('local');
+    const mentionsConfigRoot = sectionC.includes('config root') || sectionC.includes('config.toml') || sectionC.includes('config directory');
+
+    assert.ok(
+      mentionsCodexHome,
+      'Section C fallback must mention $CODEX_HOME for config-root resolution',
+    );
+    assert.ok(
+      mentionsConfigDir,
+      'Section C fallback must mention --config-dir for config-root resolution',
+    );
+    assert.ok(
+      mentionsLocal,
+      'Section C fallback must mention --local / .codex for config-root resolution',
+    );
+    assert.ok(
+      mentionsConfigRoot,
+      'Section C fallback must describe the concept of an active config root (config.toml or config root/directory)',
+    );
+
+    // AND: all four required elements together
+    assert.ok(
+      mentionsCodexHome && mentionsConfigDir && mentionsLocal && mentionsConfigRoot,
+      'Section C fallback must describe active-config-root resolution: $CODEX_HOME + --config-dir + --local + config-root concept (AND logic)',
+    );
+
+    // Must NOT contain the literal ~/.codex/ (would be rewritten by _applyRuntimeRewrites
+    // and cause bug-3582 to diverge)
+    assert.ok(
+      !sectionC.includes('~/.codex/'),
+      'Section C must NOT contain the literal ~/.codex/ substring (breaks bug-3582 materialization test)',
+    );
+  });
+
+  // (c) "NOT equivalent" label: the workaround must be explicitly labeled as
+  //     not equivalent to typed gsd-planner/gsd-executor execution.
+  test('(c) not-equivalent label: generic-agent workaround is labeled as NOT equivalent to typed dispatch', () => {
+    const sectionC = getSectionC();
+
+    // Must name at least one typed agent
+    const namesTypedAgent =
+      sectionC.includes('gsd-planner') ||
+      sectionC.includes('gsd-executor') ||
+      sectionC.includes('typed GSD agent') ||
+      sectionC.includes('typed gsd-');
+
+    // Must contain explicit "not equivalent" / "NOT equivalent" / negation language
+    const hasNotEquivalent =
+      sectionC.toLowerCase().includes('not equivalent') ||
+      sectionC.includes('NOT equivalent') ||
+      sectionC.includes('is NOT possible');
+
+    // Must name the workaround as a workaround, not a first-class path
+    const hasWorkaroundLabel =
+      sectionC.includes('workaround') ||
+      sectionC.includes('fallback');
+
+    assert.ok(
+      namesTypedAgent,
+      'Section C must name at least one typed GSD agent (gsd-planner, gsd-executor, or "typed GSD agent")',
+    );
+    assert.ok(
+      hasNotEquivalent,
+      'Section C must contain explicit "not equivalent" / "NOT equivalent" language for the generic-agent path',
+    );
+    assert.ok(
+      hasWorkaroundLabel,
+      'Section C must label the generic-agent path as a workaround or fallback',
+    );
+
+    // AND: all three together
+    assert.ok(
+      namesTypedAgent && hasNotEquivalent && hasWorkaroundLabel,
+      'Section C must AND: name a typed agent + label it NOT equivalent + call the generic path a workaround/fallback',
+    );
+  });
+
+  // (d) Fail-closed rule: when typed dispatch is mandatory, the adapter must
+  //     instruct the agent to fail closed and report the limitation, not silently degrade.
+  test('(d) fail-closed: adapter requires failing closed when typed dispatch is mandatory', () => {
+    const sectionC = getSectionC();
+
+    const hasFailClosed =
+      sectionC.includes('fail closed') ||
+      sectionC.includes('fail-closed') ||
+      sectionC.includes('fail_closed');
+
+    const hasReportLimitation =
+      sectionC.includes('schema limitation') ||
+      sectionC.includes('report') ||
+      sectionC.includes('not silently') ||
+      sectionC.includes('silently degrading') ||
+      sectionC.includes('silently');
+
+    const hasMandatoryContext =
+      sectionC.includes('mandatory') ||
+      sectionC.includes('required') ||
+      sectionC.includes('worktree isolation') ||
+      sectionC.includes('isolation');
+
+    assert.ok(
+      hasFailClosed,
+      'Section C must instruct fail-closed behavior (the phrase "fail closed" or equivalent)',
+    );
+    assert.ok(
+      hasReportLimitation,
+      'Section C must instruct reporting the schema limitation rather than silently degrading',
+    );
+    assert.ok(
+      hasMandatoryContext,
+      'Section C must identify a context where typed dispatch is mandatory (e.g. worktree isolation)',
+    );
+
+    // AND: all three together
+    assert.ok(
+      hasFailClosed && hasReportLimitation && hasMandatoryContext,
+      'Section C must AND: instruct fail-closed + report limitation + identify mandatory-typed-dispatch contexts',
+    );
+  });
+
+  // Regression guard: typed mapping for capable schema must still be present.
+  test('adapter still documents typed agent_type spawn for sessions that support it', () => {
+    const sectionC = getSectionC();
+
+    assert.ok(
+      sectionC.includes('agent_type-capable') || sectionC.includes('multi_agent_v2'),
+      'Section C must still document the typed schema (agent_type-capable / multi_agent_v2)',
+    );
+    assert.ok(
+      sectionC.includes('spawn_agent(agent_type=') || sectionC.includes('agent_type="X"'),
+      'Section C must still show a typed spawn_agent(agent_type=...) example for capable sessions',
+    );
+  });
+
+  // Regression guard: deferred tool discovery must remain (bug-279 contract).
+  test('adapter deferred tool discovery instruction is preserved', () => {
+    // The pre-existing bug-279 contract must remain intact
+    assert.ok(
+      src.includes('deferred') && src.includes('tool_search') && src.includes('spawn_agent'),
+      'Adapter must still instruct deferred tool discovery via tool_search before deciding to run inline',
+    );
+  });
+});