Drop the Provider credentials panel from the assistant
A key gets added by picking a provider that needs one, so a second place to manage the vault was redundant. The panel is gone, and with it the rotate and enable/disable paths that only it could reach - the assistant answers a provider's requirement now, it does not administer the vault. That leaves creation as the only credential operation in the app; a management screen, if one is wanted, belongs somewhere it can be found. The picker's Add credential no longer appears only when no compatible key exists: it was the panel that covered adding a second one, so limiting it to the empty case would have made that unreachable. It also carries the saving state the panel used to show. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ffbd8c192b
commit
37bf2cb853
|
|
@ -237,9 +237,7 @@
|
|||
gap: 10px;
|
||||
}
|
||||
|
||||
.assistant-credential-picker,
|
||||
.assistant-credentials-list,
|
||||
.assistant-credential-form {
|
||||
.assistant-credential-picker {
|
||||
display: grid;
|
||||
gap: 8px;
|
||||
}
|
||||
|
|
@ -248,44 +246,6 @@
|
|||
justify-self: start;
|
||||
}
|
||||
|
||||
.assistant-credential-item {
|
||||
display: flex;
|
||||
justify-content: space-between;
|
||||
gap: 12px;
|
||||
padding: 10px;
|
||||
border: 1px solid #dbe4ee;
|
||||
border-radius: 10px;
|
||||
background: #f8fafc;
|
||||
}
|
||||
|
||||
.assistant-credential-item strong,
|
||||
.assistant-credential-item p {
|
||||
margin: 0;
|
||||
}
|
||||
|
||||
.assistant-credential-item strong {
|
||||
color: #0f172a;
|
||||
font-size: 13px;
|
||||
}
|
||||
|
||||
.assistant-credential-item p {
|
||||
margin-top: 3px;
|
||||
color: #64748b;
|
||||
font-size: 12px;
|
||||
}
|
||||
|
||||
.assistant-credential-inactive {
|
||||
opacity: 0.7;
|
||||
}
|
||||
|
||||
.assistant-credential-actions {
|
||||
display: flex;
|
||||
align-items: flex-start;
|
||||
flex-wrap: wrap;
|
||||
justify-content: flex-end;
|
||||
gap: 6px;
|
||||
}
|
||||
|
||||
.assistant-llm-settings .assistant-copy {
|
||||
margin-bottom: 0;
|
||||
}
|
||||
|
|
@ -531,12 +491,4 @@
|
|||
.assistant-phase-fields {
|
||||
grid-template-columns: 1fr;
|
||||
}
|
||||
|
||||
.assistant-credential-item {
|
||||
flex-direction: column;
|
||||
}
|
||||
|
||||
.assistant-credential-actions {
|
||||
justify-content: flex-start;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -115,9 +115,14 @@
|
|||
@else if (!compatibleCredentials().length) { <mat-hint>No credentials available</mat-hint> }
|
||||
@if (credentialsError()) { <mat-error>{{ credentialsError() }}</mat-error> }
|
||||
</mat-form-field>
|
||||
@if (!credentialsLoading() && !compatibleCredentials().length) {
|
||||
<button type="button" mat-stroked-button class="assistant-add-credential" [disabled]="configurationLocked() || assistantBusy()" (click)="openCredentialForm()">
|
||||
Add credential
|
||||
@if (!credentialsLoading()) {
|
||||
<button
|
||||
type="button"
|
||||
mat-stroked-button
|
||||
class="assistant-add-credential"
|
||||
[disabled]="configurationLocked() || assistantBusy() || credentialSaving()"
|
||||
(click)="openCredentialForm()">
|
||||
{{ credentialSaving() ? 'Saving...' : 'Add credential' }}
|
||||
</button>
|
||||
}
|
||||
</div>
|
||||
|
|
@ -185,49 +190,6 @@
|
|||
}
|
||||
</section>
|
||||
|
||||
<section class="assistant-card assistant-credentials">
|
||||
<div class="assistant-starters-head">
|
||||
<div>
|
||||
<p class="assistant-label">Provider credentials</p>
|
||||
<p class="assistant-copy">Keys are stored in the vault and are never displayed.</p>
|
||||
</div>
|
||||
<button type="button" class="assistant-section-toggle" (click)="toggleCredentialsPanel()" [attr.aria-label]="credentialsPanelOpen() ? 'Collapse credentials' : 'Expand credentials'">
|
||||
{{ credentialsPanelOpen() ? '−' : '+' }}
|
||||
</button>
|
||||
</div>
|
||||
|
||||
@if (credentialsPanelOpen()) {
|
||||
<div class="assistant-credentials-list">
|
||||
@if (credentialsLoading()) {
|
||||
<p class="assistant-meta">Loading credentials...</p>
|
||||
} @else if (!credentials().length) {
|
||||
<p class="assistant-meta">No credentials saved yet.</p>
|
||||
} @else {
|
||||
@for (credential of credentials(); track credential.id) {
|
||||
<article class="assistant-credential-item" [class.assistant-credential-inactive]="!credential.active">
|
||||
<div>
|
||||
<strong>{{ credential.label }}</strong>
|
||||
<p>{{ credential.provider }} · {{ credential.active ? 'Active' : 'Inactive' }}</p>
|
||||
@if (credential.description) { <p>{{ credential.description }}</p> }
|
||||
<p>Key configured · last used: {{ credential.lastUsedAt || 'never' }}</p>
|
||||
</div>
|
||||
<div class="assistant-credential-actions">
|
||||
<button type="button" mat-stroked-button (click)="editCredential(credential)">Edit / rotate</button>
|
||||
<button type="button" mat-stroked-button [disabled]="credentialSaving()" (click)="setCredentialActive(credential, !credential.active)">
|
||||
{{ credential.active ? 'Disable' : 'Enable' }}
|
||||
</button>
|
||||
</div>
|
||||
</article>
|
||||
}
|
||||
}
|
||||
</div>
|
||||
|
||||
<button type="button" mat-stroked-button class="assistant-add-credential" (click)="openCredentialForm()">Add credential</button>
|
||||
@if (credentialsError()) { <p class="assistant-error">{{ credentialsError() }}</p> }
|
||||
@if (credentialSaving()) { <p class="assistant-meta">Saving...</p> }
|
||||
}
|
||||
</section>
|
||||
|
||||
@if (assistantErrorMessage()) {
|
||||
<section class="assistant-card assistant-error-card">
|
||||
<p class="assistant-error-message">{{ assistantErrorMessage() }}</p>
|
||||
|
|
|
|||
|
|
@ -182,11 +182,11 @@ describe('FlowAssistant credentials', () => {
|
|||
|
||||
afterEach(() => TestBed.resetTestingModule());
|
||||
|
||||
it('opens a modal wherever it is asked for, without needing the credentials panel open', async () => {
|
||||
// It used to set a flag whose form only rendered inside the credentials panel, so pressing Add
|
||||
// credential from the picker beside the model looked like it did nothing at all.
|
||||
it('opens a modal from the credential picker, with the provider prefilled', async () => {
|
||||
// It used to set a flag whose form rendered only inside a credentials panel further down, so
|
||||
// pressing Add credential beside the model looked like it did nothing at all. That panel is
|
||||
// gone: a key is added by picking a provider that needs one.
|
||||
const { component, settingsDialog } = build();
|
||||
expect(component.credentialsPanelOpen()).toBe(false);
|
||||
|
||||
await component.openCredentialForm('Gemini');
|
||||
|
||||
|
|
@ -207,7 +207,7 @@ describe('FlowAssistant credentials', () => {
|
|||
);
|
||||
expect(byKey.get('label').required).toBe(true);
|
||||
expect(byKey.get('value').required).toBe(true);
|
||||
expect(byKey.get('provider').readonly).toBe(false);
|
||||
expect(byKey.get('provider').required).toBe(true);
|
||||
});
|
||||
|
||||
it('saves what the modal returned', async () => {
|
||||
|
|
@ -234,20 +234,18 @@ describe('FlowAssistant credentials', () => {
|
|||
expect(vault.updateSecret).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('locks the provider and makes the key optional when rotating an existing one', async () => {
|
||||
// The provider is what makes a credential compatible, so rotating a key must not move it; and
|
||||
// an empty key means "keep the current one" rather than "no key".
|
||||
const { component, settingsDialog, vault } = build({
|
||||
dialogResult: { label: 'Prod key', provider: 'Gemini', description: '', value: '' }
|
||||
it('only creates: the assistant does not manage the vault', async () => {
|
||||
// It answers the requirement the chosen provider has. Rotating and disabling keys went with
|
||||
// the credentials panel, so nothing here can reach updateSecret.
|
||||
const { component, vault } = build({
|
||||
dialogResult: { label: 'Prod key', provider: 'Gemini', description: '', value: 'sk-1' }
|
||||
});
|
||||
|
||||
await component.editCredential({ id: 'cred-1', label: 'Prod key', provider: 'Gemini', active: true });
|
||||
await component.openCredentialForm('Gemini');
|
||||
|
||||
const dialog = settingsDialog.open.mock.calls[0][0] as any;
|
||||
const byKey = new Map<string, any>(dialog.fields.map((field: any) => [field.key, field]));
|
||||
expect(byKey.get('provider').readonly).toBe(true);
|
||||
expect(byKey.get('value').required).toBe(false);
|
||||
expect(vault.updateSecret).toHaveBeenCalledWith('cred-1', { label: 'Prod key', description: undefined });
|
||||
expect(vault.createSecret).toHaveBeenCalledTimes(1);
|
||||
expect(vault.updateSecret).not.toHaveBeenCalled();
|
||||
expect((component as { editCredential?: unknown }).editCredential).toBeUndefined();
|
||||
});
|
||||
|
||||
it('offers the current provider even when the list never loaded', async () => {
|
||||
|
|
|
|||
|
|
@ -81,7 +81,6 @@ export class FlowAssistant implements OnInit, OnDestroy {
|
|||
readonly credentialsLoading = signal(false);
|
||||
readonly credentialsError = signal<string | null>(null);
|
||||
readonly selectedCredentialId = signal('');
|
||||
readonly credentialsPanelOpen = signal(false);
|
||||
readonly credentialSaving = signal(false);
|
||||
readonly advancedModelsOpen = signal(false);
|
||||
readonly modelPickerOpen = signal(false);
|
||||
|
|
@ -305,29 +304,13 @@ export class FlowAssistant implements OnInit, OnDestroy {
|
|||
this.selectedCredentialId.set(credentialId);
|
||||
}
|
||||
|
||||
toggleCredentialsPanel() {
|
||||
this.credentialsPanelOpen.update((open) => !open);
|
||||
if (this.credentialsPanelOpen()) {
|
||||
void this.loadCredentials();
|
||||
void this.loadProviders();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A modal rather than a form inside the credentials panel: this is reachable from the credential
|
||||
* picker next to the model too, and there the panel it used to appear in may well be collapsed -
|
||||
* so pressing Add credential looked like it did nothing at all.
|
||||
* A modal, from the credential picker beside the model - the one place the assistant needs a key
|
||||
* at all. Creation only: the assistant no longer manages the vault, it answers the requirement
|
||||
* the chosen provider has, so there is nothing here to rotate or disable.
|
||||
*/
|
||||
async openCredentialForm(provider = this.selectedProvider()) {
|
||||
if (this.configurationLocked()) return;
|
||||
await this.openCredentialDialog(null, provider);
|
||||
}
|
||||
|
||||
async editCredential(credential: VaultSecret) {
|
||||
await this.openCredentialDialog(credential, credential.provider);
|
||||
}
|
||||
|
||||
private async openCredentialDialog(editing: VaultSecret | null, provider: string) {
|
||||
this.credentialsError.set(null);
|
||||
|
||||
// The current provider is offered even when the list has not loaded: it is the one the user
|
||||
|
|
@ -337,92 +320,48 @@ export class FlowAssistant implements OnInit, OnDestroy {
|
|||
|
||||
const fields: NodeSettingField[] = [
|
||||
{ key: 'label', label: 'Label', type: 'text', required: true, autofocus: true },
|
||||
{
|
||||
key: 'provider',
|
||||
label: 'Provider',
|
||||
type: 'select',
|
||||
required: true,
|
||||
options: providerOptions,
|
||||
// What makes a credential compatible, so rotating a key must not move it to another one.
|
||||
readonly: editing != null
|
||||
},
|
||||
{ key: 'provider', label: 'Provider', type: 'select', required: true, options: providerOptions },
|
||||
{ key: 'description', label: 'Description (optional)', type: 'text' },
|
||||
{
|
||||
key: 'value',
|
||||
label: editing ? 'New value (to rotate)' : 'API key',
|
||||
label: 'API key',
|
||||
type: 'password',
|
||||
required: editing == null,
|
||||
tip: editing
|
||||
? 'Leave empty to keep the current key.'
|
||||
: 'The value will not be shown again after saving.'
|
||||
required: true,
|
||||
tip: 'The value will not be shown again after saving.'
|
||||
}
|
||||
];
|
||||
|
||||
const result = await this.settingsDialog.open({
|
||||
title: editing ? `Edit ${editing.label}` : 'Add credential',
|
||||
title: 'Add credential',
|
||||
fields,
|
||||
initial: {
|
||||
label: editing?.label ?? '',
|
||||
provider,
|
||||
description: editing?.description ?? '',
|
||||
value: ''
|
||||
}
|
||||
initial: { label: '', provider, description: '', value: '' }
|
||||
});
|
||||
if (!result) return;
|
||||
|
||||
this.saveCredential({
|
||||
editingId: editing?.id ?? null,
|
||||
label: String(result['label'] ?? '').trim(),
|
||||
provider: String(result['provider'] ?? '').trim(),
|
||||
description: String(result['description'] ?? '').trim(),
|
||||
value: String(result['value'] ?? '')
|
||||
});
|
||||
this.saveCredential(
|
||||
String(result['label'] ?? '').trim(),
|
||||
String(result['provider'] ?? '').trim(),
|
||||
String(result['description'] ?? '').trim(),
|
||||
String(result['value'] ?? '')
|
||||
);
|
||||
}
|
||||
|
||||
private saveCredential(input: {
|
||||
editingId: string | null;
|
||||
label: string;
|
||||
provider: string;
|
||||
description: string;
|
||||
value: string;
|
||||
}) {
|
||||
const { editingId, label, provider, description, value } = input;
|
||||
if (!label || !provider || (!editingId && !value.trim()) || this.credentialSaving()) return;
|
||||
private saveCredential(label: string, provider: string, description: string, value: string) {
|
||||
if (!label || !provider || !value.trim() || this.credentialSaving()) return;
|
||||
|
||||
this.credentialSaving.set(true);
|
||||
this.credentialsError.set(null);
|
||||
const request = editingId
|
||||
? this.vault.updateSecret(editingId, {
|
||||
label,
|
||||
description: description || undefined,
|
||||
...(value.trim() ? { value } : {})
|
||||
})
|
||||
: this.vault.createSecret({ label, provider, description: description || undefined, value });
|
||||
|
||||
request.pipe(finalize(() => this.credentialSaving.set(false))).subscribe({
|
||||
next: (credential) => {
|
||||
if (credential.active && this.sameProvider(credential.provider, this.selectedProvider())) {
|
||||
this.selectedCredentialId.set(credential.id);
|
||||
}
|
||||
void this.loadCredentials();
|
||||
},
|
||||
error: (err) => this.credentialsError.set(this.backendErrorMessage(err))
|
||||
});
|
||||
}
|
||||
|
||||
setCredentialActive(credential: VaultSecret, active: boolean) {
|
||||
if (this.credentialSaving()) return;
|
||||
this.credentialSaving.set(true);
|
||||
this.credentialsError.set(null);
|
||||
this.vault.updateSecret(credential.id, { active }).pipe(
|
||||
finalize(() => this.credentialSaving.set(false))
|
||||
).subscribe({
|
||||
next: () => {
|
||||
if (!active && this.selectedCredentialId() === credential.id) this.selectedCredentialId.set('');
|
||||
void this.loadCredentials();
|
||||
},
|
||||
error: (err) => this.credentialsError.set(this.backendErrorMessage(err))
|
||||
});
|
||||
this.vault.createSecret({ label, provider, description: description || undefined, value })
|
||||
.pipe(finalize(() => this.credentialSaving.set(false)))
|
||||
.subscribe({
|
||||
next: (credential) => {
|
||||
if (credential.active && this.sameProvider(credential.provider, this.selectedProvider())) {
|
||||
this.selectedCredentialId.set(credential.id);
|
||||
}
|
||||
void this.loadCredentials();
|
||||
},
|
||||
error: (err) => this.credentialsError.set(this.backendErrorMessage(err))
|
||||
});
|
||||
}
|
||||
|
||||
setPhaseModel(phase: keyof NonNullable<AssistantLlmSelection['phaseModels']>, model: string) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue