✅ Done! I reviewed PR #336 and posted the review to GitHub: github.com/OpenHands/enterprise/pull/336#… Summary of findings: The core approach is sound — cleanly separating "what the SDK registers" (

ACP_PROVIDERS

) from "what Cloud offers" (

SURFACED_ACP_PROVIDERS

), and parametrizing the rejection tests off the registry diff so new upstream harnesses are covered automatically. Blocking (P1): There's a TOCTOU gap —

_validate_acp_provider_surfaced

resolves the profile and checks the provider, but the settings actually used to build the agent are re-resolved much later (

_build_start_conversation_request_for_user

/

_build_acp_start_conversation_request

). After the potentially long sandbox-start wait, a caller can overwrite the same profile ID to an unsupported provider and the later lookup builds it un-gated. Other points (P2/P3): • Redundant profile resolution on the HTTP path (router check + service check + build = up to three lookups), which also widens the P1 window. • The allowlist isn't gated by

get_deployment_mode()

— self-hosted/Replicated operators can't widen it even if they pin a wrapper for an extra harness. •

custom

bypass is only documented in the constants module, not where the gate is enforced. •

is_acp_provider_surfaced

(raw list) and

surfaced_acp_providers()

(registry-filtered) can disagree on a renamed/removed harness. Plus a small nit about rebuilding the tuple constant on every call. Overall a solid, well-tested fix with one security-relevant gap worth closing before merge.

✅ Done! Review submitted ✅ I re-reviewed PR #336 and posted an Approve. The previously-blocking P1 (profile-revision race) is now fixed —

_start_app_conversation

resolves the effective launch view once and threads that exact

UserInfo

snapshot through validation, the build, and provenance tagging, with a regression test that actually swaps the resolved revision between lookups and asserts the unsupported provider never reaches the build. The service-level enforcement covers integration-triggered starts (GitHub/GitLab/Jira/Slack), not just the HTTP endpoints. I also left four non-blocking notes (the

custom

bypass, the 400 message omitting

custom

, the import-time allowlist snapshot, and the test asserting

HTTPException

rather than the specific error type), none of which block merge. Review: github.com/OpenHands/enterprise/pull/336#…