Repository navigation
🧭 feat: Let Hosts Declare Per-Call Subagent Arguments - #596
Conversation
Hosts can now declare optional, host-validated string arguments on a lazy subagent config (`SubagentConfig.hostArgs`). The subagent tool advertises them as optional properties (enums unioned across subagents, each type's own values listed beside it), the executor checks a call's values against the selected subagent's declaration before an execution opens, and the lazy resolver receives them as `SubagentResolveContext.hostArgs`. Values are bound to the durable execution: the invocation binding and the resume manifest carry a SHA-256 digest of them (never the values), so a reconstruction with different arguments is rejected before the resolver runs, and detached tasks include the digest in their request fingerprint. A resolver can refuse a value with `SubagentHostArgumentError`, which the parent sees as a fixed message naming the argument. Hosts that declare nothing get the same schema, description, fingerprints and manifests as before.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a7449d947
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isBoundedText(value, maxLength) && | ||
| (pattern == null || pattern.test(value)) |
There was a problem hiding this comment.
When a host declares a vulnerable pattern such as (a+)+$, the model controls the string passed to this synchronous RegExp.test, so a non-matching value can block the Node event loop. The 256-character value cap is not protective: even 28 a characters followed by ! took over a second locally, and longer allowed inputs grow exponentially. Validate declarations for unsafe regex constructs or evaluate patterns with a non-backtracking/timeout-bounded engine before exposing this to model input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ad7f03c: pattern support is removed. Free-form host arguments are bounded strings only (at most 256 characters, no control characters), a declaration that carries pattern is rejected at normalization, and any format check belongs to the host resolver, which already re-validates every value. No model input reaches a host-supplied regex.
| const [pattern] = entry.patterns; | ||
| return { | ||
| type: 'string', | ||
| description: entry.description, | ||
| maxLength: entry.maxLength, | ||
| ...(entry.patterns.size === 1 && pattern != null ? { pattern } : {}), |
There was a problem hiding this comment.
When a free-form declaration uses an unanchored pattern such as [a-z]+, JSON Schema interprets the emitted pattern as a substring match, while checkValue wraps the same declaration with ^(?:...)$ and requires a full-string match. Consequently, schema validation or constrained generation can accept a value such as abc!, only for the executor to reject the supposedly schema-valid call. Emit the same anchored expression in the tool schema that runtime validation uses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ad7f03c by removing pattern entirely (see the P1 thread), so the schema and runtime validation agree: free-form properties advertise only maxLength, which runtime enforces identically.
A host-declared pattern ran model-controlled input through a synchronous regex, so a backtracking-prone pattern could block the event loop, and the schema emitted it unanchored while runtime validation anchored it. Free-form host arguments are now bounded strings only (length and no control characters); declarations carrying a pattern are rejected and the resolver owns any format check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad7f03c8f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| resumeExecution != null && | ||
| resumeExecution.hostArgsDigest !== | ||
| getSubagentHostArgsDigest(params.hostArgs) |
There was a problem hiding this comment.
On a durable replay with an already persisted settlement, ToolNode calls getSettledToolOutput, and restoreSettledToolOutput can return the checkpointed result without ever reaching this comparison in executeOnce. If the reconstructed call keeps its tool-call ID but changes a host argument such as machine or workspace, the old result is therefore silently reused instead of returning the invocation-changed failure. Apply the digest check in the settled-output restoration path as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this one. A settled replay is keyed to the parent tool call recorded in the checkpointed AIMessage, and the host arguments are part of that call's args, so a replayed call cannot carry different values without being a different message. The persisted digest is compared on the paths where a child is resolved or settled against a bound invocation (executeOnce and persistSettledToolOutput). Comparing call.args here would also be wrong in the other direction: they are the pre-hook args, while the bound digest is taken from the post-hook values the tool actually received, so a PreToolUse hook that legitimately rewrote a host argument would make every replay fail. This mirrors how the replay path already treats description, which it also does not re-compare.
| value.length > 0 && | ||
| value.length <= maxLength && |
There was a problem hiding this comment.
For free-form arguments containing astral Unicode characters, this runtime bound disagrees with the emitted JSON Schema: JSON Schema maxLength counts Unicode code points, whereas JavaScript value.length counts UTF-16 code units. For example, a declaration with maxLength: 1 advertises "😀" as valid, but the executor rejects it as length 2, so constrained/schema-validated model output can still fail at execution. Count code points here, such as via iteration over the string, to enforce the same limit the schema advertises.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f572f5c: free-form lengths are counted in code points, matching JSON Schema maxLength; a test covers an astral-plane value at the boundary.
| for (const value of values) { | ||
| if (!isBoundedText(value, SUBAGENT_HOST_ARG_LIMITS.valueLength)) { |
There was a problem hiding this comment.
When a host declares a whitespace-only enum member such as " ", isBoundedText accepts it and the generated schema advertises it as a valid choice, but pickSubagentHostArgInput later treats that same value as omitted because value.trim() === ''. The resolver therefore never receives a schema-valid selection. Reject whitespace-only enum members here, or preserve them during input extraction, so declaration and execution semantics agree.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f572f5c: blank enum values are rejected at declaration time, so every advertised choice survives input extraction.
| for (const name of names) { | ||
| const value = values[name]; | ||
| if (value == null) { |
There was a problem hiding this comment.
If a host declares an otherwise valid name that exists on Object.prototype, such as constructor, toString, or valueOf, omitting that optional argument reads the inherited function here rather than undefined, so the call is rejected with must be a string instead of allowing the host default. Check that the raw input owns the property before reading it, or use a null-prototype lookup object.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f572f5c: input extraction reads only own properties, so an omitted argument named like a prototype member (e.g. constructor) is treated as omitted.
| const HOST_ARGS_DESCRIPTION = | ||
| '\n\nOPTIONAL ARGUMENTS:\n- Some types accept extra arguments, listed in brackets after the type. Each is optional: omit it to let the host choose, and pass only a value listed for the selected type.'; |
There was a problem hiding this comment.
When any declaration is free-form, this instruction still tells the parent to pass only a listed value, while the per-type summary represents that argument merely as worktree: text. That conflicts with the schema, which permits arbitrary bounded text, and can cause the model to omit the argument or send the literal text instead of the requested path or identifier. Describe listed choices as applying only to enum arguments and explicitly permit bounded text for free-form ones.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f572f5c: the summary now reads name: any text up to N characters for free-form arguments, and the instruction distinguishes listed values from free text.
| if ( | ||
| typeof argument !== 'string' || | ||
| !HOST_ARGUMENT_NAME_PATTERN.test(argument) | ||
| ) { |
There was a problem hiding this comment.
If a resolver mistakenly throws SubagentHostArgumentError with a reserved built-in name such as subagent_type or description, this duplicate syntax-only check accepts it and produces a model-facing instruction to omit a required built-in field, leading subsequent retries to fail schema validation rather than recover. Reuse the declaration name validator here so typed rejections can name only valid host arguments.
AGENTS.md reference: AGENTS.md:L32-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f572f5c: typed refusals reuse the declaration name validator, so a refusal naming a built-in argument falls back to the generic startup failure; covered by a test.
- Count free-form lengths in code points, as JSON Schema maxLength does. - Reject blank enum values, which input extraction treats as omitted. - Read only own properties of the tool input, so prototype names such as constructor are omitted rather than rejected. - Tell the model that free-form arguments take any short text. - Accept a typed resolver refusal only for a declarable argument name.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f572f5c544
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return { | ||
| type: 'string', | ||
| description: entry.description, | ||
| maxLength: entry.maxLength, |
There was a problem hiding this comment.
For a free-form argument, the generated schema enforces only maxLength, so a schema validator or constrained model can accept a value containing a newline or another control character. The executor later rejects that same value because checkValue delegates to isBoundedText, which forbids control characters, making schema-valid tool calls fail at runtime. Add a fixed safe schema constraint for the runtime restriction, or otherwise keep schema and execution validation aligned.
AGENTS.md reference: AGENTS.md:L32-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f0b8712: free-form properties now also advertise a fixed SDK pattern, ^[^\u0000-\u001f\u007f]*$ (no host input, linear-time), which is exactly the runtime's control-character rule; length is already aligned via code-point maxLength.
| if (!entry.seenValues.has(value)) { | ||
| entry.seenValues.add(value); | ||
| entry.values.push(value); |
There was a problem hiding this comment.
When multiple subagents declare the same argument with disjoint enums, each declaration may contribute the full 64 allowed values and this loop appends all of them without checking the merged size. Because the number of subagent configs is not bounded, the emitted property enum and tool prompt can grow arbitrarily large despite enumValues: 64, potentially exceeding provider schema or context limits. Reject an oversized merged enum or apply a separate global cap before emitting the schema.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f0b8712: the enum merged across subagents is capped at 256 distinct values (SUBAGENT_HOST_ARG_LIMITS.mergedEnumValues) and building the tool throws beyond it; covered by a test.
| failure: SubagentHostArgumentFailure | ||
| ): string { | ||
| const { argument } = failure; | ||
| const omit = `Omit "${argument}" to let the host choose, or pass another listed value.`; |
There was a problem hiding this comment.
When a resolver rejects a free-form argument such as worktree, this formatter tells the model to pass another listed value even though that declaration has no listed choices. The main tool description now distinguishes free-form arguments, but this remaining failure-path instruction contradicts it and gives the model no actionable retry guidance. Use wording that applies to both declaration kinds, or tailor the message using the selected argument specification.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f0b8712: refusal messages now say "Omit it to let the host choose, or pass a different value", which holds for both free-form and enumerated arguments.
| if (typeof argument !== 'string' || !isSubagentHostArgName(argument)) { | ||
| return undefined; |
There was a problem hiding this comment.
Fresh evidence at the current head is that this shared validator checks only syntax and reserved names, not whether the argument was declared by the selected subagent and actually supplied in this invocation. A resolver typo such as machin, or a refusal for an omitted host-defaulted argument, is therefore exposed as a specific model-facing instruction to omit or replace a field the model never passed and may not have in its schema. Compare the failure name with the selected config and validated hostArgs; otherwise fall back to the generic startup failure.
AGENTS.md reference: AGENTS.md:L32-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f0b8712: a typed refusal is honored only when its argument was supplied in this invocation's validated host args (which already implies the selected subagent declares it); otherwise the failure falls back to the generic startup message. Covered by a test where the resolver refuses an argument the call did not pass.
- Advertise free-form arguments' no-control-character rule with a fixed, linear-time schema pattern so schema and runtime validation agree. - Cap the enum merged across subagents at 256 distinct values. - Honor a typed resolver refusal only for an argument this call supplied. - Word refusals so they apply to free-form and enumerated arguments.
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
A parent agent can currently only pick which subagent to run (
subagent_type). Where that child runs is decided entirely by the host. In LibreChat this caused a real failure: a parent working in a BYOM workspace for one repository delegated a review to a reviewer subagent, and the child started in a different code environment where it could not see the PR.This PR adds a generic, host-agnostic extension point so a host can let the parent choose per call, without the SDK knowing what the choice means.
What changes
Declaring arguments. A lazy subagent config (
resolveAgentInputs+configId) may declarehostArgs: optional string arguments, each with a model-facing description and either anenumor a bounded free-form string (maxLength; the resolver owns any format check).Tool schema.
buildSubagentToolParamsadds one optional property per declared name. Enums are unioned across subagents (first declaration's description wins), each type's own values are listed in brackets beside it in the description (free-form ones asany text up to N characters), and a shortOPTIONAL ARGUMENTSnote is added. Free-form properties advertise their code-pointmaxLengthand a fixed, linear-time pattern for the no-control-character rule, so schema and runtime validation agree.requiredis unchanged.Validation before resolution. The tool handler reads only declared, own properties (null and blank count as omitted, other non-strings are rejected).
SubagentExecutor.execute/executeInBackgroundthen check the values against the selected subagent's declaration before an execution opens: a value outside its enum, a free-form value that breaks the bounds, or an argument declared only by a sibling returns a model-visibleError: ...and the resolver never runs.Delivery. The resolver receives the validated, frozen values as
SubagentResolveContext.hostArgs(the key is absent when the call passed none).Determinism and replay. Values are bound to the durable execution:
SubagentInvocationBindinggainshostArgsDigest, so a duplicate dispatch of the same tool call with different values gets the existing "invocation changed" failure instead of sharing the in-flight child.SubagentResumeExecution(the resume manifest) recordshostArgsDigest— a SHA-256 of the canonical values, never the values themselves. A reconstruction whose call carries different (or no) values is rejected before the resolver runs. A malformed digest invalidates the manifest like any other malformed field.resolvedArgsand refuses to settle against a bound invocation with a different digest, mirroring the existing description check.The child's execution ID and checkpoint thread addresses are unchanged: they remain keyed by the parent tool call, so in-flight checkpoints survive an SDK upgrade.
Refusing a value at resolve time. A resolver can throw
new SubagentHostArgumentError(argument, 'unavailable' | 'not_allowed'). The parent sees a fixed message naming the argument (never the value or error text), e.g.Subagent error: The requested "machine" is unavailable right now. Omit "machine" to let the host choose, or pass a different value.A refusal is honored only for an argument the call actually supplied; otherwise the generic startup failure applies. Diagnostics classify it as the newhost_argument_rejectedcause, and the specific message survives the detached-task boundary viaSubagentResolutionError.hostArgument.Bounds. At most 8 arguments per subagent and 16 distinct names; enums of 1–64 unique, non-blank values of at most 256 characters without control characters, and at most 256 distinct values once merged across subagents; free-form values capped at 256 code points; descriptions at most 1024 characters. Host-supplied regex patterns are not accepted (a declaration carrying
patternis rejected), so no model input reaches a host regex. Built-in names (intent,description,subagent_type,run_in_background,subagent_thread_id) are reserved. Eager, self-spawn and graph configs that declarehostArgsare rejected during normalization, since no resolver would receive them.Compatibility
Hosts that declare nothing get byte-identical schemas, descriptions, invocation bindings, background fingerprints and resume manifests.
JsonSchemaTypegains optionalpatternandmaxLength(only the SDK's fixed free-form pattern usespattern).Versioning is left to the release process (this repo bumps versions in separate release commits).
Testing
src/tools/subagent/__tests__/hostArgs.test.ts: schema/description unchanged without declarations; enum union and per-type summaries; every declaration bound; lazy-only normalization; value checks; digest canonicalization; resolver delivery and omission; rejection before any execution opens; invocation binding against a concurrent different call; manifest digest written without values; same-digest resume reuses the persisted execution ID; changed or omitted values on resume are rejected; malformed digest invalidates the manifest; typed resolver refusal for foreground and detached tasks; background fingerprint conflict and reuse.src/specs/subagent-host-args.test.ts: a fullRunthrough the realToolNodeand LangChain schema validation — the parent's choice reaches the resolver, omission falls back to the host default, and an argument the selected type does not declare returns a model-visible error.npx tsc --noEmit,eslintand import sorting on touched files,npm run check:circular-deps, and the tools/subagent/graph suites (2240 tests) pass.LibreChat consumes this to let a parent choose the machine/workspace for a subagent per call (follow-up PR in danny-avila/LibreChat).