diff --git a/src/app/shared/flow-assistant/flow-assistant.css b/src/app/shared/flow-assistant/flow-assistant.css index 1072109..128485d 100644 --- a/src/app/shared/flow-assistant/flow-assistant.css +++ b/src/app/shared/flow-assistant/flow-assistant.css @@ -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; - } } diff --git a/src/app/shared/flow-assistant/flow-assistant.html b/src/app/shared/flow-assistant/flow-assistant.html index debd686..4ad4fc1 100644 --- a/src/app/shared/flow-assistant/flow-assistant.html +++ b/src/app/shared/flow-assistant/flow-assistant.html @@ -115,9 +115,14 @@ @else if (!compatibleCredentials().length) { No credentials available } @if (credentialsError()) { {{ credentialsError() }} } - @if (!credentialsLoading() && !compatibleCredentials().length) { - } @@ -185,49 +190,6 @@ } -
-
-
-

Provider credentials

-

Keys are stored in the vault and are never displayed.

-
- -
- - @if (credentialsPanelOpen()) { -
- @if (credentialsLoading()) { -

Loading credentials...

- } @else if (!credentials().length) { -

No credentials saved yet.

- } @else { - @for (credential of credentials(); track credential.id) { -
-
- {{ credential.label }} -

{{ credential.provider }} · {{ credential.active ? 'Active' : 'Inactive' }}

- @if (credential.description) {

{{ credential.description }}

} -

Key configured · last used: {{ credential.lastUsedAt || 'never' }}

-
-
- - -
-
- } - } -
- - - @if (credentialsError()) {

{{ credentialsError() }}

} - @if (credentialSaving()) {

Saving...

} - } -
- @if (assistantErrorMessage()) {

{{ assistantErrorMessage() }}

diff --git a/src/app/shared/flow-assistant/flow-assistant.spec.ts b/src/app/shared/flow-assistant/flow-assistant.spec.ts index e799eab..3977870 100644 --- a/src/app/shared/flow-assistant/flow-assistant.spec.ts +++ b/src/app/shared/flow-assistant/flow-assistant.spec.ts @@ -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(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 () => { diff --git a/src/app/shared/flow-assistant/flow-assistant.ts b/src/app/shared/flow-assistant/flow-assistant.ts index 591b587..f73af95 100644 --- a/src/app/shared/flow-assistant/flow-assistant.ts +++ b/src/app/shared/flow-assistant/flow-assistant.ts @@ -81,7 +81,6 @@ export class FlowAssistant implements OnInit, OnDestroy { readonly credentialsLoading = signal(false); readonly credentialsError = signal(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, model: string) {