Skip to content

fix(subagents): reject unsupported resume option overrides - #1055

Merged
agegr merged 1 commit into
agegr:mainfrom
uvforce:fix/subagent-resume-options
Oct 7, 2026
Merged

agegr merged 1 commit into
agegr:mainfrom
uvforce:fix/subagent-resume-options

Conversation

@uvforce

@uvforce uvforce commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Summary

 Agent(resume, creation_options)
- accept the options but silently keep the existing session configuration
+ reject unsupported effective overrides before dispatch
+ preserve empty defaults and resume the existing session normally

Agent accepts model and other creation options, but the resume path does not apply them. A caller can request another model and incorrectly believe the override took effect.

Make the continuation contract explicit: reject nonempty model, thinking, input_files, isolation, inherit_context: true, and nonzero max_turns before calling the runtime. Empty defaults, including max_turns: 0, retain the existing start behavior. To change creation options, start a new subagent; this does not implement model hot-swapping. The tool description and documentation now state this contract.

This is the resume-contract split from #1054, based directly on official main. It does not include or depend on that PR's turn-limit/runtime changes. The regression reuses the existing SDK integration harness instead of copying the lifecycle fixture or introducing an abstraction.

Evidence

  • Before: On unmodified upstream 6fcd7d4, the new SDK-dispatch regression fails:

    AssertionError: model must not be silently ignored.
    false !== true
    

    After: A real SDK AgentSession calls the model-only Agent tool using faux model responses. A recording runtime delegate observes no resume dispatch for any rejected option and exactly one dispatch for the compatible empty-default request. Errors name the rejected option.

  • Before: Resume coverage was bundled with the lifecycle change.
    After: This independent resume-only branch passes:

    Check Result
    Subagent extension tests 22/22 passed
    npm test 2284/2284 passed; no skips
    npm run lint Passed
    npm run build, including TypeScript checking Passed
    Production-mode browser suite 10 PASS, 0 SKIP; 1280px and 390px

The model boundary and runtime delegate are controlled in the new integration test; it exercises actual SDK tool dispatch, not a live external provider or a real child execution. Local validation used macOS/arm64 and Node 26.5.0. The unchanged session-export route still emits its dynamic-dependency build warning. Both split patches apply together, and their combined production code is byte-identical to the previously reviewed implementation.

Merge Danger

Door: two-way

No data migration, settings, dependencies, or new runtime states are introduced. Reverting restores the previous silent-ignore behavior.

Blast Radius: Resume

Callers that repeat nonempty creation options when resuming will now receive an explicit error and must omit them or create a new child. Empty-default calls remain compatible. Turn limits, terminal outcomes, provider selection, tool permissions, ownership checks, and existing session configuration are not changed.

Reject effective creation options before resuming an existing child. Preserve empty defaults, including max_turns: 0, and document the continuation contract. Reuse the existing SDK integration harness to verify rejection before dispatch and compatible continuation.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@agegr
agegr merged commit f272cd8 into agegr:main Oct 7, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants