mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 00:49:41 +02:00
fix(custom-model,toast): surface the real apply error, and make error toasts sticky with a close button
Two related fixes, both needed to actually diagnose 'Session started on the native backend — could not apply the custom endpoint' reports from live testing: 1. runCustomModelEntry()'s apply call went through _apiJson(), which unwraps a success body but SWALLOWS a failure response entirely and returns null — discarding the one thing (error, errorCode) that would tell 'endpoint unreachable' apart from 'not a discovered model', 'remote/Docker session', or a dozen other real causes the apply route already reports distinctly. Switched to _api() so the actual response body is read on failure too, and the toast now includes the real message. 2. showToast() defaulted every toast, error or not, to a 3s auto-dismiss with no way to read it again — exactly what made the above generic message impossible to act on even before the fix above. Error toasts now default to sticky (duration: 0, no auto-dismiss) unless a caller opts into a duration, and every toast — sticky or not — gets an explicit close (x) button, since a sticky toast with no way to dismiss it would just accumulate across repeated failures. Tests: custom-model-run-menu-ui.test.ts's two apply tests updated for the _api() switch (their mocks previously stubbed _apiJson, which the apply call no longer goes through), plus a new test pinning that the real server error string reaches the toast on a failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RqZeHrRS6DYcGcGX2p9EwG
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
9a9e542a7d
commit
409a6e65f9
@@ -5484,12 +5484,25 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
return this.showToast(message, type);
|
return this.showToast(message, type);
|
||||||
},
|
},
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `duration` defaults to 0 (sticky, no auto-dismiss) for `error` toasts and
|
||||||
|
* 3000ms for everything else — an error worth a distinct visual style is
|
||||||
|
* also worth reading before it vanishes, which a fixed 3s auto-dismiss
|
||||||
|
* does not guarantee: "Session started on the native backend — could not
|
||||||
|
* apply the custom endpoint: <the actual reason>" is exactly the kind of
|
||||||
|
* message that needs a moment to read, not a glance. Every toast gets an
|
||||||
|
* explicit close button regardless of duration, since a sticky one with no
|
||||||
|
* way to dismiss it would just pile up. A caller can still override either
|
||||||
|
* default via `opts.duration` (e.g. a deliberately brief success toast, or
|
||||||
|
* a non-error one that should also stay put).
|
||||||
|
*/
|
||||||
showToast(message, type = 'info', opts = {}) {
|
showToast(message, type = 'info', opts = {}) {
|
||||||
const { duration = 3000, action } = opts;
|
const { duration = type === 'error' ? 0 : 3000, action } = opts;
|
||||||
const toast = document.createElement('div');
|
const toast = document.createElement('div');
|
||||||
toast.className = `toast toast-${type}`;
|
toast.className = `toast toast-${type}`;
|
||||||
|
|
||||||
const msgSpan = document.createElement('span');
|
const msgSpan = document.createElement('span');
|
||||||
|
msgSpan.className = 'toast-message';
|
||||||
msgSpan.textContent = message;
|
msgSpan.textContent = message;
|
||||||
toast.appendChild(msgSpan);
|
toast.appendChild(msgSpan);
|
||||||
|
|
||||||
@@ -5501,6 +5514,20 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
toast.appendChild(btn);
|
toast.appendChild(btn);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let dismissTimer = null;
|
||||||
|
const dismiss = () => {
|
||||||
|
if (dismissTimer) clearTimeout(dismissTimer);
|
||||||
|
toast.classList.remove('show');
|
||||||
|
setTimeout(() => toast.remove(), 200);
|
||||||
|
};
|
||||||
|
|
||||||
|
const closeBtn = document.createElement('button');
|
||||||
|
closeBtn.className = 'toast-close';
|
||||||
|
closeBtn.textContent = '×';
|
||||||
|
closeBtn.setAttribute('aria-label', 'Dismiss');
|
||||||
|
closeBtn.onclick = (e) => { e.stopPropagation(); dismiss(); };
|
||||||
|
toast.appendChild(closeBtn);
|
||||||
|
|
||||||
// Cache toast container reference
|
// Cache toast container reference
|
||||||
if (!this._toastContainer) {
|
if (!this._toastContainer) {
|
||||||
this._toastContainer = document.querySelector('.toast-container');
|
this._toastContainer = document.querySelector('.toast-container');
|
||||||
@@ -5514,10 +5541,9 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
|
|
||||||
requestAnimationFrame(() => toast.classList.add('show'));
|
requestAnimationFrame(() => toast.classList.add('show'));
|
||||||
|
|
||||||
setTimeout(() => {
|
if (duration > 0) {
|
||||||
toast.classList.remove('show');
|
dismissTimer = setTimeout(dismiss, duration);
|
||||||
setTimeout(() => toast.remove(), 200);
|
}
|
||||||
}, duration);
|
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -721,12 +721,20 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
const sessionId = this.activeSessionId;
|
const sessionId = this.activeSessionId;
|
||||||
if (!sessionId || sessionId === before) return;
|
if (!sessionId || sessionId === before) return;
|
||||||
|
|
||||||
const data = await this._apiJson(`/api/sessions/${sessionId}/custom-model`, {
|
// _apiJson() (used everywhere else in this file) unwraps a success body to
|
||||||
|
// its `data`, but on failure it swallows the response entirely and returns
|
||||||
|
// null — exactly the `error` text a caller needs to tell "the endpoint is
|
||||||
|
// unreachable" apart from "the CLI can't be redirected", "not one of the
|
||||||
|
// discovered models", or "this is a Docker/remote session". Go through the
|
||||||
|
// raw response here instead so a failure is diagnosable, not just present.
|
||||||
|
const res = await this._api(`/api/sessions/${sessionId}/custom-model`, {
|
||||||
method: 'POST',
|
method: 'POST',
|
||||||
body: { endpointId, modelId },
|
body: { endpointId, modelId },
|
||||||
});
|
});
|
||||||
if (!data) {
|
const data = res ? await res.json().catch(() => null) : null;
|
||||||
this.showToast(`Session started on the native backend — could not apply the custom endpoint`, 'warning');
|
if (!data || data.success === false) {
|
||||||
|
const detail = data?.error ? `: ${data.error}` : res ? ` (HTTP ${res.status})` : ' (request failed)';
|
||||||
|
this.showToast(`Session started on the native backend — could not apply the custom endpoint${detail}`, 'error');
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
this.showToast(`Pointed at ${endpointId} — restarting the session...`, 'info');
|
this.showToast(`Pointed at ${endpointId} — restarting the session...`, 'info');
|
||||||
|
|||||||
@@ -8463,6 +8463,9 @@ kbd {
|
|||||||
}
|
}
|
||||||
|
|
||||||
.toast {
|
.toast {
|
||||||
|
display: flex;
|
||||||
|
align-items: center;
|
||||||
|
gap: 0.5rem;
|
||||||
background: var(--bg-card);
|
background: var(--bg-card);
|
||||||
border: 1px solid var(--border);
|
border: 1px solid var(--border);
|
||||||
border-radius: 6px;
|
border-radius: 6px;
|
||||||
@@ -8474,6 +8477,7 @@ kbd {
|
|||||||
opacity: 0;
|
opacity: 0;
|
||||||
transition: all 0.2s ease;
|
transition: all 0.2s ease;
|
||||||
pointer-events: auto;
|
pointer-events: auto;
|
||||||
|
max-width: 420px;
|
||||||
}
|
}
|
||||||
|
|
||||||
.toast.show {
|
.toast.show {
|
||||||
@@ -8481,6 +8485,32 @@ kbd {
|
|||||||
opacity: 1;
|
opacity: 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.toast-message {
|
||||||
|
flex: 1;
|
||||||
|
/* Errors are sticky by default (showToast) precisely so a longer, specific
|
||||||
|
message survives to be read — let it wrap instead of clipping. */
|
||||||
|
white-space: pre-wrap;
|
||||||
|
word-break: break-word;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Every toast gets one, sticky or not: a sticky toast with no way to close it
|
||||||
|
would just accumulate on screen across repeated failures. */
|
||||||
|
.toast-close {
|
||||||
|
flex-shrink: 0;
|
||||||
|
background: none;
|
||||||
|
border: none;
|
||||||
|
color: inherit;
|
||||||
|
opacity: 0.6;
|
||||||
|
font-size: 1.1rem;
|
||||||
|
line-height: 1;
|
||||||
|
padding: 0 0.15rem;
|
||||||
|
cursor: pointer;
|
||||||
|
}
|
||||||
|
|
||||||
|
.toast-close:hover {
|
||||||
|
opacity: 1;
|
||||||
|
}
|
||||||
|
|
||||||
.toast-success { border-color: rgba(34, 197, 94, 0.4); }
|
.toast-success { border-color: rgba(34, 197, 94, 0.4); }
|
||||||
.toast-error { border-color: rgba(239, 68, 68, 0.4); }
|
.toast-error { border-color: rgba(239, 68, 68, 0.4); }
|
||||||
.toast-warning { border-color: rgba(234, 179, 8, 0.4); }
|
.toast-warning { border-color: rgba(234, 179, 8, 0.4); }
|
||||||
|
|||||||
@@ -307,10 +307,9 @@ describe('Custom Model Endpoint Profiles: applying a picked entry', () => {
|
|||||||
app.run = async () => {};
|
app.run = async () => {};
|
||||||
app._runInFlight = false;
|
app._runInFlight = false;
|
||||||
let applyCalled = false;
|
let applyCalled = false;
|
||||||
const realApiJson = app._apiJson.bind(app);
|
app._api = async (path: string) => {
|
||||||
app._apiJson = async (path: string, opts?: unknown) => {
|
|
||||||
if (path.includes('/custom-model')) applyCalled = true;
|
if (path.includes('/custom-model')) applyCalled = true;
|
||||||
return realApiJson(path, opts as never);
|
return { ok: true, json: async () => ({ success: true, data: {} }) };
|
||||||
};
|
};
|
||||||
|
|
||||||
await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
|
await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
|
||||||
@@ -326,9 +325,12 @@ describe('Custom Model Endpoint Profiles: applying a picked entry', () => {
|
|||||||
app.activeSessionId = 'new-session';
|
app.activeSessionId = 'new-session';
|
||||||
};
|
};
|
||||||
const calls: Array<{ path: string; body: unknown }> = [];
|
const calls: Array<{ path: string; body: unknown }> = [];
|
||||||
app._apiJson = async (path: string, opts?: { body?: unknown }) => {
|
app._api = async (path: string, opts?: { body?: unknown }) => {
|
||||||
calls.push({ path, body: opts?.body });
|
calls.push({ path, body: opts?.body });
|
||||||
return { customModel: { endpointId: 'llama-box' }, restarted: true };
|
return {
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({ success: true, data: { customModel: { endpointId: 'llama-box' }, restarted: true } }),
|
||||||
|
};
|
||||||
};
|
};
|
||||||
|
|
||||||
await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
|
await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
|
||||||
@@ -338,6 +340,33 @@ describe('Custom Model Endpoint Profiles: applying a picked entry', () => {
|
|||||||
expect(calls[0].body).toEqual({ endpointId: 'llama-box', modelId: 'qwen3' });
|
expect(calls[0].body).toEqual({ endpointId: 'llama-box', modelId: 'qwen3' });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('surfaces the real server error in the toast on a failed apply, rather than a generic message', async () => {
|
||||||
|
const { app } = bootApp({});
|
||||||
|
app.activeSessionId = 'old-session';
|
||||||
|
app.run = async () => {
|
||||||
|
app.activeSessionId = 'new-session';
|
||||||
|
};
|
||||||
|
app._api = async () => ({
|
||||||
|
ok: false,
|
||||||
|
status: 400,
|
||||||
|
json: async () => ({
|
||||||
|
success: false,
|
||||||
|
error: 'Custom model endpoints are not supported for remote (SSH) or Docker sessions yet',
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
let toastMessage: string | null = null;
|
||||||
|
let toastType: string | null = null;
|
||||||
|
app.showToast = (msg: string, type: string) => {
|
||||||
|
toastMessage = msg;
|
||||||
|
toastType = type;
|
||||||
|
};
|
||||||
|
|
||||||
|
await app.runCustomModelEntry('claude', 'llama-box', 'qwen3');
|
||||||
|
|
||||||
|
expect(toastMessage).toContain('Custom model endpoints are not supported for remote (SSH) or Docker sessions yet');
|
||||||
|
expect(toastType).toBe('error');
|
||||||
|
});
|
||||||
|
|
||||||
it('routes through run() itself, so the Run in-flight lock actually engages', async () => {
|
it('routes through run() itself, so the Run in-flight lock actually engages', async () => {
|
||||||
// CLAUDE.md, Run launch synchronization: the lock exists so a double click
|
// CLAUDE.md, Run launch synchronization: the lock exists so a double click
|
||||||
// cannot create duplicate sessions. A hardcoded dispatch table bypassing
|
// cannot create duplicate sessions. A hardcoded dispatch table bypassing
|
||||||
|
|||||||
Reference in New Issue
Block a user