fix(agent): save conversations to DB, fix Test button hang, wire hasNext/search
## Root cause: _save_conversation() dead code + missing message persistence
### Backend: Conversation persistence (3 critical bugs)
- **app.py**: Replaced early with so _save_conversation() executes
after successful stream — was dead code on normal path
- **app.py**: Added _save_conversation call in HITL resume path (confirm/deny)
- **app.py**: Added broad that saves conversation (at least user
message) before re-raising on LLM errors (APIConnectionError etc.)
- **app.py**: _save_conversation now passes user_id from JWT (not hardcoded UUID)
and includes messages[] in payload
- **agent_conversations.py**: save_conversation endpoint now processes body.messages
and creates AgentMessage records (idempotent by msg id)
### Frontend: Agent chat sidebar wiring
- AgentChatModel.svelte.ts: added public derived getter
- AgentChatModel.svelte.ts: added method
- agent/+page.svelte: wired hasNext={model.conversationsHasNext} (was hardcoded false)
- agent/+page.svelte: wired onsearch to model.searchConversations (was no-op)
### Frontend: LLM Provider Test button hang fix
- ProviderConfig.svelte: resetForm/handleEdit now reset isTesting=false, isProbing=false
- ProviderConfig.svelte: Cancel button calls abortPendingRequests()
- ProviderConfig.svelte: Added AbortController lifecycle — cancels in-flight test/fetch
requests on modal close or provider switch, preventing stale disabled buttons
- provider_config.integration.test.ts: added 6 abort/reset invariant tests
This commit is contained in:
@@ -62,6 +62,16 @@ import { SvelteSet } from "svelte/reactivity";
|
||||
let probeResult = $state(null); // { max_images, method } or null
|
||||
let probeError = $state("");
|
||||
|
||||
// AbortController for in-flight request cancellation
|
||||
let abortController = $state<AbortController | null>(null);
|
||||
|
||||
function abortPendingRequests() {
|
||||
if (abortController) {
|
||||
abortController.abort();
|
||||
}
|
||||
abortController = new AbortController();
|
||||
}
|
||||
|
||||
function isMaskedKey(key) {
|
||||
if (!key) return false;
|
||||
return key === "********" || key.includes("...");
|
||||
@@ -78,6 +88,8 @@ import { SvelteSet } from "svelte/reactivity";
|
||||
}
|
||||
|
||||
function resetForm() {
|
||||
// Cancel any in-flight requests
|
||||
abortPendingRequests();
|
||||
formData = {
|
||||
name: "",
|
||||
provider_type: "openai",
|
||||
@@ -92,13 +104,17 @@ import { SvelteSet } from "svelte/reactivity";
|
||||
};
|
||||
editingProvider = null;
|
||||
testStatus = { type: "", message: "" };
|
||||
isTesting = false;
|
||||
availableModels = [];
|
||||
modelsLoadError = "";
|
||||
probeResult = null;
|
||||
probeError = "";
|
||||
isProbing = false;
|
||||
}
|
||||
|
||||
function handleEdit(provider) {
|
||||
// Cancel any in-flight requests from previous edit session
|
||||
abortPendingRequests();
|
||||
console.log("[ProviderConfig][Action] Editing provider", provider?.id);
|
||||
editingProvider = provider;
|
||||
// Normalize provider fields to editable form shape.
|
||||
@@ -118,10 +134,12 @@ import { SvelteSet } from "svelte/reactivity";
|
||||
max_output_tokens: provider?.max_output_tokens ?? null,
|
||||
};
|
||||
testStatus = { type: "", message: "" };
|
||||
isTesting = false;
|
||||
availableModels = [];
|
||||
modelsLoadError = "";
|
||||
probeResult = null;
|
||||
probeError = "";
|
||||
isProbing = false;
|
||||
showForm = true;
|
||||
// Auto-fetch models when editing an existing provider
|
||||
if (provider?.base_url) {
|
||||
@@ -203,7 +221,7 @@ import { SvelteSet } from "svelte/reactivity";
|
||||
if (isMaskedKey(testData.api_key)) {
|
||||
delete testData.api_key;
|
||||
}
|
||||
const result = await requestApi(endpoint, "POST", testData);
|
||||
const result = await requestApi(endpoint, "POST", testData, { signal: abortController?.signal });
|
||||
|
||||
if (result.success) {
|
||||
testStatus = { type: "success", message: $t.llm.connection_success };
|
||||
@@ -649,6 +667,7 @@ import { SvelteSet } from "svelte/reactivity";
|
||||
type="button"
|
||||
class="px-4 py-2 border rounded hover:bg-surface-muted flex-1"
|
||||
onclick={() => {
|
||||
abortPendingRequests();
|
||||
showForm = false;
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
// #region ProviderConfigIntegrationTest [C:3] [TYPE Module] [SEMANTICS llm, provider-config, integration-test, edit-flow, delete-flow]
|
||||
// @PURPOSE: Protect edit, submit, test, and delete interaction contracts in LLM provider settings UI.
|
||||
// #region ProviderConfigIntegrationTest [C:3] [TYPE Module] [SEMANTICS llm, provider-config, integration-test, edit-flow, delete-flow, abort]
|
||||
// @PURPOSE: Protect edit, submit, test, delete interaction contracts, isTesting reset, and AbortController lifecycle in LLM provider settings UI.
|
||||
// @LAYER UI (Tests)
|
||||
// @RELATION DEPENDS_ON -> [ProviderConfig]
|
||||
// @INVARIANT: Edit action keeps explicit click handler and opens normalized edit form.
|
||||
// @INVARIANT: Masked API keys are excluded from save/test/fetch-models payloads.
|
||||
// @INVARIANT: isTesting resets to false on modal close, edit switch, and form reset.
|
||||
// @INVARIANT: abortPendingRequests() aborts old controller and creates a new one.
|
||||
// @INVARIANT: testConnection passes abort signal to requestApi for cancellability.
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import fs from 'node:fs';
|
||||
@@ -90,4 +93,65 @@ describe('ProviderConfig edit interaction contract', () => {
|
||||
});
|
||||
});
|
||||
// #endregion provider_config_edit_contract_tests:Function
|
||||
|
||||
// #region provider_config_isTesting_reset_tests:Function [TYPE Function]
|
||||
// @RELATION BINDS_TO -> [EXT:frontend:ProviderConfigIntegrationTest]
|
||||
// @PURPOSE: Validate isTesting and isProbing reset, AbortController lifecycle, and Cancel button wiring.
|
||||
// @INVARIANT: isTesting = false on edit switch, form reset, and modal close.
|
||||
// @INVARIANT: abortPendingRequests() cancels in-flight and creates fresh AbortController.
|
||||
// @INVARIANT: testConnection passes AbortSignal via FetchOptions to enable pre-unmount cancellation.
|
||||
// @TEST_EDGE: resetForm includes abortPendingRequests + isTesting + isProbing reset
|
||||
// @TEST_EDGE: handleEdit includes abortPendingRequests + isTesting + isProbing reset
|
||||
// @TEST_EDGE: Cancel button calls abortPendingRequests before hiding modal
|
||||
// @TEST_EDGE: testConnection uses abortController?.signal in requestApi call
|
||||
describe('ProviderConfig abort/reset invariants', () => {
|
||||
it('resetForm calls abortPendingRequests and resets isTesting + isProbing', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('function resetForm()');
|
||||
expect(source).toContain('abortPendingRequests();');
|
||||
expect(source).toContain('isTesting = false;');
|
||||
expect(source).toContain('isProbing = false;');
|
||||
});
|
||||
|
||||
it('handleEdit calls abortPendingRequests and resets isTesting + isProbing', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('function handleEdit(provider)');
|
||||
expect(source).toContain('abortPendingRequests();');
|
||||
expect(source).toContain('isTesting = false;');
|
||||
expect(source).toContain('isProbing = false;');
|
||||
});
|
||||
|
||||
it('Cancel button calls abortPendingRequests before hiding modal', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
// Cancel button must abort pending requests, not just hide
|
||||
expect(source).toContain('abortPendingRequests();');
|
||||
expect(source).toContain('showForm = false;');
|
||||
// Cancel must appear before Save in the button group
|
||||
});
|
||||
|
||||
it('testConnection passes AbortSignal via FetchOptions to requestApi', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('requestApi(endpoint, "POST", testData, { signal: abortController?.signal })');
|
||||
});
|
||||
|
||||
it('abortPendingRequests aborts existing controller and creates a new one', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('function abortPendingRequests()');
|
||||
expect(source).toContain('abortController.abort();');
|
||||
expect(source).toContain('abortController = new AbortController();');
|
||||
});
|
||||
|
||||
it('defines abortController as nullable $state', () => {
|
||||
const source = fs.readFileSync(COMPONENT_PATH, 'utf-8');
|
||||
|
||||
expect(source).toContain('let abortController = $state<AbortController | null>(null);');
|
||||
});
|
||||
|
||||
});
|
||||
// #endregion provider_config_isTesting_reset_tests:Function
|
||||
// #endregion ProviderConfigIntegrationTest:Module
|
||||
|
||||
@@ -141,6 +141,7 @@ export class AgentChatModel {
|
||||
: "destructive",
|
||||
);
|
||||
queuePosition = $derived(this._messageQueue.length);
|
||||
conversationsHasNext = $derived(this._conversationsHasNext);
|
||||
|
||||
// ── Actions — P1 ───────────────────────────────────────────────
|
||||
|
||||
@@ -273,6 +274,28 @@ export class AgentChatModel {
|
||||
}
|
||||
}
|
||||
|
||||
async searchConversations(query: string): Promise<void> {
|
||||
log("AgentChat.Model", "REASON", "Searching conversations", { query });
|
||||
try {
|
||||
this._conversationsPage = 1;
|
||||
const res = await getAssistantConversations(
|
||||
1, 20, false, query
|
||||
);
|
||||
const items = res.items || [];
|
||||
this.conversations = items.map((item: Record<string, unknown>) => ({
|
||||
id: item.conversation_id ?? item.id ?? "",
|
||||
title: item.title ?? "",
|
||||
updated_at: item.updated_at ?? "",
|
||||
message_count: item.message_count ?? 0,
|
||||
}));
|
||||
this._conversationsHasNext = Boolean(res.has_next);
|
||||
this._conversationsPage++;
|
||||
} catch (e: unknown) {
|
||||
this.error = e instanceof Error ? e.message : "Failed to search conversations";
|
||||
log("AgentChat.Model", "EXPLORE", "Search conversations failed", {}, this.error);
|
||||
}
|
||||
}
|
||||
|
||||
async loadHistory(conversationId: string | null = null): Promise<void> {
|
||||
this.isLoadingHistory = true;
|
||||
this.error = null;
|
||||
|
||||
@@ -45,14 +45,14 @@
|
||||
conversations={model.conversations}
|
||||
currentConversationId={model.currentConversationId}
|
||||
isLoading={model.isLoadingHistory}
|
||||
hasNext={false}
|
||||
hasNext={model.conversationsHasNext}
|
||||
onselect={(id) => {
|
||||
model!.currentConversationId = id;
|
||||
model!.loadHistory(id);
|
||||
}}
|
||||
ondelete={(id) => model!.deleteConversation(id)}
|
||||
onloadmore={() => model!.loadConversations(false)}
|
||||
onsearch={() => {}}
|
||||
onsearch={(q: string) => model!.searchConversations(q)}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
@@ -95,7 +95,7 @@
|
||||
conversations={model.conversations}
|
||||
currentConversationId={model.currentConversationId}
|
||||
isLoading={model.isLoadingHistory}
|
||||
hasNext={false}
|
||||
hasNext={model.conversationsHasNext}
|
||||
onselect={(id) => {
|
||||
model!.currentConversationId = id;
|
||||
model!.loadHistory(id);
|
||||
@@ -103,7 +103,7 @@
|
||||
}}
|
||||
ondelete={(id) => model!.deleteConversation(id)}
|
||||
onloadmore={() => model!.loadConversations(false)}
|
||||
onsearch={() => {}}
|
||||
onsearch={(q: string) => model!.searchConversations(q)}
|
||||
/>
|
||||
</div>
|
||||
<div class="flex-1 bg-black/30" onclick={() => sidebarOpen = false}></div>
|
||||
|
||||
Reference in New Issue
Block a user