fix(frontend): stop showing a stale model catalog for a rejected key - #6472
fix(frontend): stop showing a stale model catalog for a rejected key#6472Aman-goel-04 wants to merge 3 commits into
Conversation
|
@Aman-goel-04 is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR fixes misleading UI in the provider connection drawer when a provider key is rejected or otherwise doesn’t yield a live model discovery result, ensuring users aren’t shown a bundled catalog as if it came from the provider and improving the credential status copy/indicator behavior.
Changes:
- Fixes
credentialStatusLinepunctuation handling so messages keep their trailing period when no model fetch occurred. - Passes discovery status into the Active Models UI and adds a fallback-catalog note when discovery didn’t return a live list.
- Updates the credential status indicator so the dot is only green for confirmed
validverdicts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| web/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsx | Passes discovery status/title into the models section and adjusts credential status indicator styling. |
| web/packages/agenta-entity-ui/src/secretProvider/ActiveModelsSection.tsx | Adds a user-visible note when the displayed model list is the bundled fallback rather than a fetched provider list. |
| web/packages/agenta-entities/tests/unit/provider-card-copy.test.ts | Updates unit expectations so credentialStatusLine(..., null) preserves the trailing period. |
| web/packages/agenta-entities/src/secret/core/cardCopy.ts | Fixes credentialStatusLine to only strip trailing periods when appending the “· N models fetched” suffix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| > | ||
| {credentialFailed ? ( | ||
| <WarningCircle size={14} className="mt-0.5 shrink-0" /> | ||
| ) : ( | ||
| ) : credentialStatus === "valid" ? ( | ||
| <span className="mt-1.5 size-1.5 shrink-0 rounded-full bg-colorSuccess" /> | ||
| ) : ( | ||
| <span className="mt-1.5 size-1.5 shrink-0 rounded-full bg-colorWarning" /> |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
web/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsx:477
- The dot is now only green for
credentialStatus === "valid", but the surrounding status line still usestext-colorSuccessfor every non-failed probe (includingunknown). That keeps unknown/indeterminate verdicts visually “successful”. Consider keying the text color offcredentialStatus === "valid"as well (and using a warning/neutral tone otherwise) so the dot and text communicate the same state.
) : credentialStatus === "valid" ? (
<span className="mt-1.5 size-1.5 shrink-0 rounded-full bg-colorSuccess" />
) : (
<span className="mt-1.5 size-1.5 shrink-0 rounded-full bg-colorWarning" />
)}
| const catalogNote = | ||
| discoveryStatus === "fetched" | ||
| ? null | ||
| : discoveryStatus == null | ||
| ? `Showing Agenta's shipped catalog. ${title}'s own list has not been fetched yet.` | ||
| : `Showing Agenta's shipped catalog. ${title}'s own list was not fetched.` | ||
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThe provider card now preserves credential punctuation, labels shipped catalog fallbacks, distinguishes unknown or pending credentials from valid credentials, and passes discovery status to the active-models section. ChangesProvider discovery clarity
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to The PR improves rejected-key messaging and explains when a shipped model catalog is shown, but the current behavior may still mislabel non-fetched model lists and show green success text for unknown or pending credentials, which can mislead users about available models and key validity; these bounded correctness issues should be fixed or explicitly accepted before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4 files. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Team
Run ID: c75f4fae-772e-4ea2-996d-093f68a1d465
📒 Files selected for processing (4)
web/packages/agenta-entities/src/secret/core/cardCopy.tsweb/packages/agenta-entities/tests/unit/provider-card-copy.test.tsweb/packages/agenta-entity-ui/src/secretProvider/ActiveModelsSection.tsxweb/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| const catalogNote = | ||
| discoveryStatus === "fetched" | ||
| ? null | ||
| : discoveryStatus == null | ||
| ? `Showing Agenta's shipped catalog. ${title}'s own list has not been fetched yet.` | ||
| : `Showing Agenta's shipped catalog. ${title}'s own list was not fetched.` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render the catalog note only for an actual catalog fallback.
discoveryStatus !== "fetched" does not prove that the displayed options came from Agenta's shipped catalog. Credential-set providers are documented as discovery- and manual-entry-only, so this text can claim a catalog is shown when it is not. Pass an explicit fallback flag and gate this note on that flag.
| ) : credentialStatus === "valid" ? ( | ||
| <span className="mt-1.5 size-1.5 shrink-0 rounded-full bg-colorSuccess" /> | ||
| ) : ( | ||
| <span className="mt-1.5 size-1.5 shrink-0 rounded-full bg-colorWarning" /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use non-success text styling for unconfirmed credentials.
The new amber dot is inside a parent span that still uses text-colorSuccess for every non-failed status. An unknown or pending verdict can therefore appear with green status text. Apply the same valid/failed/other classification to the parent text color.
|
@ashrafchowdury, please review this PR and let me know if any changes need to be made! |
Summary
Fixes #6443
The provider connection drawer was silently showing a 34-model shipped catalog for a key OpenAI had just rejected, with no indication the list wasn't real, and the rejection message ran two sentences together with no punctuation between them.
Root cause:
credentialStatusLineunconditionally stripped the API message's trailing period, on the assumption a· N models fetchedsuffix would always follow. WhenmodelCountisnull(no fetch happened, e.g. an invalid key), no suffix is appended, so the caller's separately-appended" Nothing has been saved."landed directly after the now-period-less verdict. Separately,ProviderConnectionCardsilently fell back to Agenta's bundled model catalog whenever discovery didn't return a live list, with no signal to the user that the list wasn't the key's actual models, true for a rejected key, an untested key, and a transport failure alike.This change:
credentialStatusLineinside themodelCount !== nullbranch, so a fetch-less verdict keeps its own punctuation.ActiveModelsSection("Showing Agenta's shipped catalog. X's own list was not fetched.") wheneverdiscoveryStatusisn't"fetched", so the fallback list is never presented as if it came from the key.unknownverdicts, it now only goes green on a confirmedvalidstatus.Testing
Verified locally
Ran the updated flow against a rejected OpenAI key and a valid one in the local app; confirmed the rejection message now reads as two properly punctuated sentences, and the fallback-catalog note appears under Active Models whenever a live fetch didn't happen. Confirmed
doneStateandprobeFailureMessageneeded no changes, both already routed through the fixedcredentialStatusLine/ already treatedunknowncorrectly.Added or updated tests
Updated the assertion in
provider-card-copy.test.tsforcredentialStatusLine'smodelCount === nullcase to expect the trailing period to be preserved. Fullagenta-entitiesunit suite passes (100 test files, 1460 tests). Integration suite skipped locally (noAGENTA_API_URL/AGENTA_AUTH_KEY), unrelated to this change.QA follow-up
N/A
Demo
Before: rejected-key message runs together with no punctuation, and the shipped catalog appears with no indication it isn't the key's real list.
After: message reads as two clean sentences, and a note under Active Models states the list is Agenta's shipped catalog rather than a live fetch.
Checklist