diff --git a/.changeset/noble-tigers-tumble.md b/.changeset/noble-tigers-tumble.md new file mode 100644 index 000000000..d733e85bb --- /dev/null +++ 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. diff --git a/bin/install.js b/bin/install.js index c26731ea8..dc14fc002 100755 --- 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/.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 diff --git a/scripts/lint-regression-test-names.allowlist.json b/scripts/lint-regression-test-names.allowlist.json index c47663826..7bb0d836d 100644 --- 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" ] diff --git a/tests/bug-851-codex-quick-adapter-agent-type-fallback.test.cjs b/tests/bug-851-codex-quick-adapter-agent-type-fallback.test.cjs new file mode 100644 index 000000000..63d8456e2 --- /dev/null +++ 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 . +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('', headingIdx); + assert.ok(closeTag >= 0, 'Section C must be followed by '); + 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/.toml relative path + assert.ok( + sectionC.includes('agents/.toml'), + 'Section C must reference agents/.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', + ); + }); +});