fix(agent): honor sessions_spawn ACP model overrides

[felix-miao]

Fixes #70200

Summary

• Problem: sessions_spawn accepted a model parameter, but the ACP runtime branch dropped it before session initialization.
• Why it matters: callers asking for ACP sessions with an explicit model silently got the agent default instead of the requested model.
• What changed: ACP spawn now forwards model, initializes ACP sessions with runtimeOptions.model, and persists those options through the existing ACP manager path.
• What did NOT change (scope boundary): this PR does not add ACP support for thinking; it only fixes model override handling.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes sessions_spawn: model parameter silently dropped on ACP runtime branch #70200
• Related sessions_spawn: model parameter silently dropped on ACP runtime branch #70200
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: the ACP branch in sessions_spawn read model from tool input but never forwarded it into spawnAcpDirect, and the ACP initialize path had no way to accept initial runtime options.
• Missing detection / guardrail: ACP spawn tests covered cwd, mode, and resumeSessionId, but not explicit model overrides.
• Contributing context (if known): ACP already had persisted runtime-options support for per-session config, so this was a wiring gap rather than a missing backend capability.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: src/agents/tools/sessions-spawn-tool.test.ts, src/agents/acp-spawn.test.ts, src/acp/control-plane/manager.test.ts
• Scenario the test should lock in: ACP sessions_spawn with model carries the override through tool dispatch, ACP spawn initialization, and ACP manager runtime-option persistence.
• Why this is the smallest reliable guardrail: the regression happened across the tool -> spawn -> manager seam, so each handoff now has focused coverage without requiring a live ACP backend.
• Existing test that already covers this (if any): None.
• If no new test is added, why not: N/A

User-visible / Behavior Changes

ACP sessions_spawn calls with runtime="acp" now honor explicit model overrides instead of silently falling back to the target agent default model.

Diagram (if applicable)

Before:
sessions_spawn(model=foo, runtime=acp) -> spawnAcpDirect() -> initializeSession() -> agent default model

After:
sessions_spawn(model=foo, runtime=acp) -> spawnAcpDirect(model=foo)
  -> initializeSession(runtimeOptions.model=foo) -> ACP session runs with requested model

Security Impact (required)

• New permissions/capabilities? (Yes/No) No
• Secrets/tokens handling changed? (Yes/No) No
• New/changed network calls? (Yes/No) No
• Command/tool execution surface changed? (Yes/No) No
• Data access scope changed? (Yes/No) No
• If any Yes, explain risk + mitigation:

Repro + Verification

Environment

• OS: macOS darwin arm64
• Runtime/container: local source checkout
• Model/provider: ACP target agents, explicit model override path
• Integration/channel (if any): ACP session spawn
• Relevant config (redacted): default local dev config

Steps

1. Call sessions_spawn with runtime: "acp" and an explicit model.
2. Observe the ACP child session initialization path.
3. Verify the requested model is persisted in ACP runtime options and used for the child session.

Expected

• ACP child sessions honor the requested model override.

Actual

• Before this change, ACP child sessions silently ignored model and used the target agent default.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Human Verification (required)

• Verified scenarios: ran targeted tests for the sessions tool, ACP spawn path, and ACP manager path; committed through scripts/committer, which also ran pnpm check:changed --staged.
• Edge cases checked: ACP model forwarding without thread binding; runtime-option persistence during ACP session initialization.
• What you did not verify: live ACP backend behavior against a real external harness.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? (Yes/No) Yes
• Config/env changes? (Yes/No) No
• Migration needed? (Yes/No) No
• If yes, exact upgrade steps:

Risks and Mitigations

• Risk: ACP backends that reject model config options could still fail later when runtime controls apply.
  • Mitigation: this change only restores the documented override path and uses the existing ACP runtime-options/control plumbing that already handles backend capability checks.

[greptile-apps]

Greptile Summary

This PR fixes ACP spawn model overrides by threading a model parameter through sessions-spawn-tool → spawnAcpDirect → initializeAcpSpawnRuntime → AcpSessionManager.initializeSession, where it is now forwarded as runtimeOptions and applied via the existing validateRuntimeOptionPatch path. All three changed layers have corresponding unit tests added.

Confidence Score: 5/5

Safe to merge; the fix is minimal and well-tested with no P0/P1 issues.

The change is a focused, correctly-implemented bug fix with new unit tests at each layer. The only finding is a P2 style concern about a latent cwd erasure edge case that does not affect any current caller.

No files require special attention.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/acp/control-plane/manager.core.ts
Line: 314-317

Comment:
**`cwd: input.cwd` silently erases `runtimeOptions.cwd` when `input.cwd` is `undefined`**

`validateRuntimeOptionPatch` uses `Object.hasOwn` to detect keys, so spreading `cwd: input.cwd` after `...input.runtimeOptions` will always produce a `cwd` key in the patch object — even when `input.cwd` is `undefined`. That causes `validateRuntimeOptionPatch` to set `next.cwd = undefined`, erasing any `cwd` a caller put inside `runtimeOptions`.

No current caller passes `cwd` through `runtimeOptions` (they all use the top-level `input.cwd` parameter), so there is no breakage today. But the spread order is a latent trap as `runtimeOptions` support grows. Conditionally including `cwd` makes the intent explicit and protects future callers:

```typescript
const initialRuntimeOptions = validateRuntimeOptionPatch({
  ...input.runtimeOptions,
  ...(input.cwd !== undefined ? { cwd: input.cwd } : {}),
});
```

```suggestion
      const initialRuntimeOptions = validateRuntimeOptionPatch({
        ...input.runtimeOptions,
        ...(input.cwd !== undefined ? { cwd: input.cwd } : {}),
      });
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(agent): honor ACP spawn model overri..." | Re-trigger Greptile}

[felix-miao]

Confirmed root cause in src/agents/tools/sessions-spawn-tool.ts: the ACP path read model from tool input but never forwarded it into spawnAcpDirect, so ACP child sessions silently fell back to the target agent default model.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef3b7e2db4

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[steipete]

Landed via temp rebase onto main.

• Gate: pnpm test src/acp/control-plane/manager.test.ts src/agents/acp-spawn.test.ts src/agents/tools/sessions-spawn-tool.test.ts; pnpm check:changed
• Source head: 29a6fb6
• Merge commit: 449cad5

Thanks @felix-miao!