Skip to content

fix(acp): let configured custom models bypass the advertised-choices guard - #18169

Closed
Lokkuchakreshkumar wants to merge 1 commit into
pingdotgg:mainfrom
Lokkuchakreshkumar:fix/acp-custom-model-passthrough
Closed

Lokkuchakreshkumar wants to merge 1 commit into
pingdotgg:mainfrom
Lokkuchakreshkumar:fix/acp-custom-model-passthrough

Conversation

@Lokkuchakreshkumar

Copy link
Copy Markdown

Problem

A model id configured via a provider instance's customModels is merged into the composer catalog (modelsFromDiscovery → providerModelsFromSettings), so it is selectable — but any run with it dies as a generic provider-session-open-failure. Concretely: Cline's free tier (cline-free/*, the only working route for $0-balance accounts) is missing from the agent's discovery, can be added as a custom model, and the same cline --acp binary accepts it — yet T3 never sends it. Fixes #18164 (maintainer-triaged; direction confirmed in the reply there).

Change

  • packages/provider-acp/src/server/runtimeModel.ts: new pure helper isAllowedSessionConfigOptionValue — a value passes when it is either agent-advertised or in the instance's passthrough set.
  • packages/provider-acp/src/server/AcpSessionRuntime.ts: new optional AcpSessionRuntimeOptions.passthroughConfigValues; validateConfigOptionValue consults it before the advertised-choices allowlist. Type/boolean checks are unchanged, and the -32602 error is unchanged for everything else.
  • packages/provider-acp-registry/src/server/adapter.ts: makeAcpRegistryRuntime passes the instance's normalized customModels slugs as the passthrough set. No agent-specific branches (per the file's rule); other layer() callers (grok, probe, auth, Antigravity) are unaffected since the field is optional.
  • packages/provider-acp/src/server/runtimeModel.test.ts: 3 unit tests for the helper (advertised passes, unadvertised rejected, custom id passes while other unknown ids still rejected).

Net effect: configureSession → runtime.setModel → session/set_config_option now reaches the agent for configured custom ids, and the agent validates them. Agents that reject unknown ids behave exactly as before (the error just comes from the agent instead of the client guard).

Scope and approval

Scope is the one defect in #18164; the maintainer reply there confirms the bug in code and the fix direction ("skip or widen the advertised-choices check in validateConfigOptionValue ... for configured customModels"). No product defaults change: with empty customModels the behavior is byte-for-byte identical (empty passthrough set). The Cursor case (#13267, customs hidden at catalog level) is untouched — hiding happens before this check, and a manually configured id there would now be sent to the agent, which rejects out-of-catalog ids itself.

Verification

  • Direct ACP probe against the managed Cline 3.0.70 binary (the same binary T3 spawns): session/set_model and session/set_config_option { configId: "model", value: "cline-free/muse-spark-1.3-contributor" } both succeed (currentValue updated), and a follow-up session/prompt "hi" completes with stopReason: end_turn — no billing error. This establishes the agent side accepts exactly what T3 was refusing to send.
  • tsc --noEmit clean for @t3tools/provider-acp and @t3tools/provider-acp-registry.
  • vp test run for @t3tools/provider-acp: 8 files / 112 tests pass (incl. 3 new); @t3tools/provider-acp-registry: 65 tests pass.
  • vp fmt --check clean on all four touched files.
  • Could not check: end-to-end run through a patched T3 build (no desktop build produced here); and the follow-up suggestion in the issue reply (surfacing the -32602 text instead of the opaque open failure) is intentionally left out to keep this to one problem.

Built with Muse Spark 1.3 via the T3 Code OpenCode harness.

…guard

Custom model ids (e.g. Cline's cline-free/* free tier) are merged into the
composer catalog but rejected client-side by validateConfigOptionValue, which
allowlists session/set_config_option values against agent-advertised choices
only. Thread the instance's customModels through as passthrough values so the
agent validates them; the guard still applies to everything else.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 11, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 11, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 8a16706

Macroscope's review found this PR approvable — This is a contained ACP bug fix that lets explicitly configured custom model IDs reach the agent while retaining validation for all other values. Defaults, unrelated ACP callers, schemas, deployment settings, and static-analysis configuration remain unchanged.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 11, 2026 — with ChatGPT Codex Connector
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 11, 2026 09:02

Dismissing prior approval to re-evaluate 8a16706

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f4cda322-82ba-4643-ba25-f098b2bf95ca

📥 Commits

Reviewing files that changed from the base of the PR and between 8d199b4 and 8a16706.


📒 Files selected for processing (4)
  • packages/provider-acp-registry/src/server/adapter.ts
  • packages/provider-acp/src/server/AcpSessionRuntime.ts
  • packages/provider-acp/src/server/runtimeModel.test.ts
  • packages/provider-acp/src/server/runtimeModel.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



📝 Walkthrough

Walkthrough

The ACP session runtime now accepts configured passthrough values during configuration-option validation. The ACP Registry adapter supplies custom model slugs as those values.

Changes

ACP custom model validation

Layer / File(s) Summary
Session configuration validation
packages/provider-acp/src/server/AcpSessionRuntime.ts, packages/provider-acp/src/server/runtimeModel.ts, packages/provider-acp/src/server/runtimeModel.test.ts
The runtime accepts configured passthrough values or advertised choices. Tests cover advertised values, configured values, and unadvertised values that remain rejected.
ACP Registry passthrough values
packages/provider-acp-registry/src/server/adapter.ts
The adapter passes slugs from configured custom model entries to the runtime as passthrough values.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant ACPRegistryAdapter
  participant AcpSessionRuntime
  participant Agent
  ACPRegistryAdapter->>AcpSessionRuntime: Pass custom model slugs as passthroughConfigValues
  AcpSessionRuntime->>Agent: Forward a valid configured model value
Loading

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to 8a167

Merge Risk: ⚪ Minimal · up to 8a167

Configured custom model IDs now reach the ACP agent for validation, while other unadvertised values are still rejected as before. No actionable merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8a167

The exception accepts only explicitly configured values, but it applies beyond the intended selection change. Existing checks constrain the normal request paths, and no privilege escalation was demonstrated. Downstream enforcement and settings-write permissions remain unverified.

Retained concerns

  • Low · security · observed: The public configuration setter now admits configured custom-model slugs for unrelated select options, including modes, because passthrough membership is not bound to a model option. This is an observed widening of the client-side control, not a demonstrated authorization bypass. Advertised-value checks in the current registry orchestration constrain practical reachability, but direct setter calls rely on downstream rejection for these cross-category values.
Security review details

Security Blast Radius

  • observed — The demonstrated acceptance expansion is scoped to a runtime constructed from one provider instance's configuration. Requests use that runtime's active session identity; the changed code does not introduce a shared cross-instance allowlist.

Security Findings and Attack Paths

  • inferred — A cross-category path requires a value already present in the instance's custom-model configuration and a caller that submits it for another select option without an advertised-value precheck. The public setter would send that request, but the inspected registry paths constrain such submissions. No attacker-controlled route to a privileged outcome was established, and external rejection behavior was not verified.

Trust Boundaries and Controls

  • observed — Boolean and string-type validation remain intact. The new exception affects select-value membership before sending the request; it does not locally apply the requested value. The implementation explicitly delegates validation of configured passthrough values to the downstream ACP implementation.

Resilience and Maintainability Implications

  • inferred — Lost responses and concurrent configuration updates can leave uncertainty about remote versus local state because successful responses replace the snapshot without request-version reconciliation. These mechanisms predate the PR, and configuration notifications can refresh state. Custom values now participate in the same transition path, but no materially worsened security guarantee was established.

Hardening Proposals

  • proposed — Bind configured custom-model passthrough values to the resolved model option or model category, preserving advertised-choice rejection for unrelated select controls without relying on caller-specific filters.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the main change: allowing configured custom model IDs to bypass the advertised-choices guard.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the defect, implementation, scope, test results, manual validation, and known limitations.
Linked Issues check Passed Issue #18164 requires configured custom model IDs to reach the ACP agent. The adapter now reads normalized customModels entries and passes their slugs as passthroughConfigValues. `isAllowedSession…
Out of Scope Changes check Passed The diff contains only the runtime allowlist change, the ACP registry wiring for provider custom models, and focused unit tests. The optional runtime field leaves other callers unchanged. No product d…

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@t3dotgg

t3dotgg commented Oct 14, 2026

Copy link
Copy Markdown
Member

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing because this PR states that Muse Spark wrote code in it:

Built with Muse Spark 1.3 via the T3 Code OpenCode harness.

We do not accept code written by Gemini models or by Muse models other than Muse Wonder Full. See Models we do not accept.

To have this considered again, open a new PR with the code written by a different model and link this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Cline ACP free-tier models (cline-free/*) blocked by client-side config validation

3 participants