Skip to content

Revert "Bedrock model discovery and validation" - #10176

Merged
lifeizhou-ap merged 1 commit into
mainfrom
revert-9997-bedrock-model-discovery
Jul 1, 2026
Merged

Revert "Bedrock model discovery and validation"#10176
lifeizhou-ap merged 1 commit into
mainfrom
revert-9997-bedrock-model-discovery

Conversation

@lifeizhou-ap

Copy link
Copy Markdown
Collaborator

Reverts #9997

This PR has reference to goosed api. We are in a progress to launch release that UI connects to ACP server without goosed, so revert this for now. Once our release is done, we can come back to fix the dependency on goosed for this PR

@lifeizhou-ap
lifeizhou-ap requested a review from jamadeo July 1, 2026 23:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ff5d2984d

ℹ️ 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".

Comment on lines +218 to 219
let model_exists = entry.default_model == model_id
|| entry.models.iter().any(|model| model.id == model_id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve custom models for refreshable providers

When saving defaults for a refreshable provider such as openai/OpenAI-compatible endpoints, a user can type a model that is not in the cached inventory; SwitchModelModal still supports that custom entry and sends it through acpSaveDefaults, but this check now rejects anything not already listed. That makes ACP desktop unable to save valid custom/compatible model IDs unless the inventory happens to contain them, regressing the manual custom-model flow for refreshable providers.

Useful? React with 👍 / 👎.

Comment on lines 426 to +432
const success = await changeModel(sessionId, modelObj);
if (success) {
onModelSelected?.(modelObj.name, modelObj.provider || '');
trackModelChanged(modelObj.provider || '', modelObj.name);
onClose();
}
} finally {
setIsSubmitting(false);

onClose();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the switch dialog open when model switching fails

If changeModel returns false (for example because acpSaveDefaults or setSessionConfigOption failed and already showed an error toast), this unconditional close still dismisses the modal. In that failure path the model was not changed, so closing the dialog removes the user's chance to correct the provider/model selection and makes the failed switch look like it completed.

Useful? React with 👍 / 👎.

@lifeizhou-ap
lifeizhou-ap enabled auto-merge July 1, 2026 23:30
@lifeizhou-ap
lifeizhou-ap added this pull request to the merge queue Jul 1, 2026
Merged via the queue into main with commit 0d4b692 Jul 1, 2026
25 checks passed
@lifeizhou-ap
lifeizhou-ap deleted the revert-9997-bedrock-model-discovery branch July 1, 2026 23:58
lifeizhou-ap added a commit that referenced this pull request Jul 2, 2026
* main:
  Revert "Bedrock model discovery and validation" (#10176)
  Improve ACP auth and origin defaults (#9886)
  Bedrock model discovery and validation (#9997)
  chore(deps): bump actions-rust-lang/setup-rust-toolchain from 1.16.1 to 1.17.0 (#10052)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants