[core] Allow Sites terms disclosure under never policy#28098
[core] Allow Sites terms disclosure under never policy#28098yansenzhou-oai wants to merge 1 commit into
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8914b89b4
ℹ️ 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".
| AskForApproval::Granular(granular_config) if !granular_config.allows_mcp_elicitations() => { | ||
| return result; | ||
| } | ||
| AskForApproval::Never |
There was a problem hiding this comment.
Require a visible response before accepting Sites terms
For app-server clients with mcp_elicitations_auto_deny (Xcode 26.4), request_mcp_server_elicitation auto-returns Accept without an event. This Never path tells the model Sites terms were accepted without showing the disclosure; require sent before accepting. guidance
Useful? React with 👍 / 👎.
|
|
||
| fn auth_elicitation_message(auth_failure: &CodexAppsConnectorAuthFailure) -> String { | ||
| if auth_failure.is_sites_publication_terms_disclosure() { | ||
| return "Review the ChatGPT Sites Terms to continue.".to_string(); |
There was a problem hiding this comment.
Use terms-specific UI for Sites disclosure
For the TUI path, this new Sites case only changes the elicitation message, but AppLinkView::from_codex_apps_auth_url_parts still classifies every Codex Apps auth failure as Auth and hardcodes sign-in instructions/actions (app_link_view.rs shows “Sign in…” plus “Open sign-in URL” / “I already signed in”). When Sites publication terms are required under Never, users are prompted with a sign-in flow rather than terms-review copy, so add a terms-specific kind/metadata or route it through the external-action UI.
Useful? React with 👍 / 👎.
Summary
AskForApproval::Neversuppresses ordinary approval promptsWhy
The Codex desktop client pauses a personal user's first Sites tool invocation while it shows a versioned legal disclosure. Full Access maps to
AskForApproval::Never, but that policy currently suppresses every connector elicitation. The exception must therefore be narrow enough to keep generic auth prompts disabled while allowing only the trusted Sites terms challenge.Test plan
just test -p codex-mcp auth_elicitationjust test -p codex-core codex_apps_auth_elicitationjust test -p codex-core sitesjust fix -p codex-mcpjust fix -p codex-corejust fmtThe broader
codex-corecrate run was also attempted earlier; the focused tests above passed, while unrelated sandbox and missing-test-binary cases in the broad run are not caused by this change.