[StiensWout]

ACP agents that need no sign-in kept models they stopped advertising, including a stale default. The registry treated authenticated as proof that discovery had completed, while these agents correctly report auth as unknown.

Use the discovery probe's session metadata to distinguish a complete catalog from a readiness or failed snapshot. Completed probes replace removed models and the placeholder; pending and failed probes retain known models. Authentication and custom models keep their existing behavior.

Fixes #18783. Alternative to #18807, with tests using the real driver snapshots to cover completed discovery and signed-in readiness.

Validation: both focused behavior cases fail before the fix and pass afterward. Independently ran pnpm exec vp test run src/provider/ProviderRegistry.test.ts from apps/server, 62/62 passed, plus package typecheck, targeted lint and diff checks. The author also passed driver, probe and provider compatibility tests and apps/server typecheck. No live agent session was tested.

_{Prepared for Wout by claude-opus-5-5 in T3 Code's Claude harness. Validated by gpt-6.1-sol in Codex.}

[macroscopeapp]

Approvability

Verdict: Approved at 1d6e9b1

Macroscope's review found this PR approvable — This is a small, localized ACP catalog bug fix: completed discovery replaces stale models, while pending or failed discovery preserves known entries. The added tests cover unauthenticated completed probes and authenticated readiness without introducing schema, infrastructure, or static-analysis changes.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

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: e2a30c87-67b7-433a-8dea-98be89ce471b

📥 Commits

Reviewing files that changed from the base of the PR and between 7ac952c and 1d6e9b1.

📒 Files selected for processing: 4

• apps/server/src/provider/ProviderRegistry.test.ts
• apps/server/src/provider/ProviderRegistry.ts
• packages/provider-acp-registry/src/server/driver.ts
• packages/provider-acp-registry/src/testing.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

ACP model retention now follows discovery completion rather than authentication status. Completed discovery replaces the discovered model list even when authentication is unknown. Pending and failed discovery retain previously known models.

Changes: ACP model discovery

Layer / File(s)	Summary
Snapshot-driven model retention apps/server/src/provider/ProviderRegistry.ts, apps/server/src/provider/ProviderRegistry.test.ts, packages/provider-acp-registry/src/server/driver.ts, packages/provider-acp-registry/src/testing.ts	The registry uses nativeSessions to identify incomplete ACP snapshots. Tests cover completed, pending, and failed discovery, including custom models and a default placeholder. The testing module re-exports the checked-snapshot builder. A driver comment describes how completed discovery informs model inventory.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 1d6e9

Completed discovery replaces stale models, while the inspected pending and failed paths retain known models. No actionable merge-blocking issue is established by the available evidence.

Security architecture risk: 🔵 Low · up to 1d6e9

The change removes stale model choices without changing authentication or access permissions. Remaining uncertainty concerns whether every successful discovery and live update supplies a complete catalog; an incomplete advertisement could remove valid choices.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated effect is catalog mutation for the emitting provider instance. Registry source correlation checks both instance and driver identity, and updates merge into the matching instance entry; the changed predicate does not widen that ownership scope.

Trust Boundaries and Controls

• observed — Model advertisements originate from the configured ACP process and are normalized before entering provider snapshots. Existing normalization bounds identifiers and display text, deduplicates models, and limits the model count. The PR changes retention of that metadata, not these input controls or process authority.

Resilience and Maintainability Implications

• observed — Pending and failed discovery preserve known model rows without carrying forward the previous native-session capabilities. This retains catalog availability without making cached models proof of authentication or privileged session-operation support.

Hardening Proposals

• proposed — Consider an explicit catalog-completeness contract, separate from session capabilities, if additional snapshot producers or partial live advertisements are supported. This would reduce future drift in the authority to remove cached models; it is not an observed vulnerability.

Pre-merge checks: 4

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: replacing ACP model catalogs after completed discovery.
Description check		The description explains the problem, implementation, linked issue, verification results, and testing limitations. It is mostly complete and provides sufficient context for this focused fix, although …
Linked Issues check		Issue #18783 requires completed ACP discovery to replace stale discovered models, while pending or failed discovery retains known models. ProviderRegistry.ts now retains missing ACP models only when…
Out of Scope Changes check		The changes stay within issue #18783. The registry condition fixes ACP model replacement. The driver comment documents the session-metadata contract. The testing export enables registry tests to const…

✨ 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.}