mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 15:39:41 +02:00
fix(custom-model): address second pre-merge review (Ark0N)
Blocker: .center-status-banner never actually disappears.
- Add `.center-status-banner[hidden] { display: none; }`, same trap as
`.home-sessions[hidden]`: the author-level `display: flex` beat the
UA `[hidden]` rule, so `dismiss()` set `el.hidden = true` and the
card stayed laid out at `opacity: 0` with its text/cancel/close
children still `pointer-events: auto` -- an invisible 442x67 click
blocker dead centre over the terminal until the page reloaded.
- Added a regression test pinning the CSS rule, and documented the
banner (10001) and the swap-confirm/context-warning modals (10010)
in CLAUDE.md's Z-index layers list.
Stale wording pointed at the reverted sticky-toast default:
- .changeset/run-menu-custom-model-picker.md, CLAUDE.md, and the
`.toast-message` comment in styles.css all still said "toasts
default to sticky" after 1f32128c put the flat 3s default back.
Reworded all three to describe the actual behaviour: one call site
passes an explicit `duration: 0`.
Smaller items from the same review:
- docs/api-reference.md said discovery failures answer
`502 OPERATION_FAILED`; OPERATION_FAILED is 422 per src/types/api.ts
and the error-code table earlier in the same file.
- The periodic re-discovery sweep (server.ts) never read
customModelEndpointsEnabled, so turning the feature off left
Codeman polling every saved endpoint forever. Added
readCustomModelEndpointsEnabled() (custom-model-routes.ts, same
shape as readPlanUsageTelemetryEnabled) and gated the interval
callback on it.
- Reverted the formatting-only Prettier pass docs/api-reference.md
picked up (table padding, *x* to _x_, JSON re-indent) by re-merging
the new Custom Model Endpoints section onto the pre-PR file, so the
diff is reviewable. No prose content was lost -- verified by diffing
the result against the pre-revert file (formatting-only) and against
the merge-base file (only the new section added).
- docs/custom-model-endpoints.md now states that a custom-model Claude
session's isolated CLAUDE_CONFIG_DIR loses the user's global
settings.json, user-level skills/agents/commands, and MCP servers
from ~/.claude.json -- only `projects` is symlinked back.
Design question left open in the review (does `confirmed: true` need
to be two flags so "launch anyway" on the context warning doesn't also
skip the llama-swap displacement warning): keeping the single flag, as
offered. The 20s displacement sweep still catches a resulting swap
after the fact, so it's a surprise rather than a silent failure, and
splitting it is real behavioural surface I have no way to verify live
in this environment.
`npm run test:browser` could not be run in this environment (no tmux,
no downloaded Playwright browser binary) -- none of its suite's files
touch code this fix changes, but it still needs a real pass before
merge, same as any frontend change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ea59JhUmHBm1gRCsiYF33R
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
1f32128ca9
commit
9982a1325f
@@ -8526,8 +8526,8 @@ kbd {
|
||||
|
||||
.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. */
|
||||
/* A sticky toast (showToast's opts.duration: 0) can carry a longer, specific
|
||||
message — let it wrap instead of clipping. */
|
||||
white-space: pre-wrap;
|
||||
word-break: break-word;
|
||||
}
|
||||
@@ -8588,6 +8588,15 @@ kbd {
|
||||
transform: translate(-50%, -50%) scale(1);
|
||||
}
|
||||
|
||||
/* `hidden` has to be re-asserted over the `display: flex` above, or `dismiss()`
|
||||
setting `el.hidden = true` does nothing (same trap as `.home-sessions[hidden]`
|
||||
below): the card stays laid out at `opacity: 0` with its text/cancel/close
|
||||
children still `pointer-events: auto`, an invisible click-blocker dead centre
|
||||
over the terminal until the page reloads. */
|
||||
.center-status-banner[hidden] {
|
||||
display: none;
|
||||
}
|
||||
|
||||
.center-status-spinner {
|
||||
flex-shrink: 0;
|
||||
width: 18px;
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
|
||||
import type { FastifyInstance, FastifyRequest } from 'fastify';
|
||||
import { ApiErrorCode, createErrorResponse, type ApiResponse } from '../../types.js';
|
||||
import { isAdmin, parseBody } from '../route-helpers.js';
|
||||
import { isAdmin, parseBody, readJsonConfig, SETTINGS_PATH } from '../route-helpers.js';
|
||||
import { isMultiUserMode } from '../../config/multiuser.js';
|
||||
import { getDataDir } from '../../config/instance.js';
|
||||
import { isBlockedWebviewUrl } from '../webview-egress-policy.js';
|
||||
@@ -565,6 +565,19 @@ function applyDiscoveredModels(host: CustomModelHost, result: DiscoveryResult):
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* `customModelEndpointsEnabled` defaults OFF (unlike `showPlanUsageLimits`'s
|
||||
* absent-means-on in `readPlanUsageTelemetryEnabled`), so mirror the frontend's
|
||||
* own gate (`session-ui.js`'s `!settings.customModelEndpointsEnabled`) rather
|
||||
* than that reader's default. Exists so the periodic re-discovery sweep in
|
||||
* server.ts can skip entirely while the feature is off, instead of polling
|
||||
* every saved endpoint forever regardless of the setting.
|
||||
*/
|
||||
export async function readCustomModelEndpointsEnabled(): Promise<boolean> {
|
||||
const settings = await readJsonConfig<Record<string, unknown>>(SETTINGS_PATH, 'settings.json', {});
|
||||
return settings.customModelEndpointsEnabled === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-discovers every saved endpoint's models, best-effort. One endpoint being
|
||||
* unreachable (powered off, wrong network) must not stop the others from
|
||||
|
||||
@@ -30,6 +30,7 @@ export { registerTabLayoutRoutes } from './tab-layout-routes.js';
|
||||
export {
|
||||
registerCustomModelRoutes,
|
||||
refreshAllCustomModelHosts,
|
||||
readCustomModelEndpointsEnabled,
|
||||
detectCustomModelSwapDisplacements,
|
||||
pruneIdleLlamaSwapLogTails,
|
||||
type CustomModelSessionLike,
|
||||
|
||||
+13
-3
@@ -191,6 +191,7 @@ import {
|
||||
registerTabLayoutRoutes,
|
||||
registerCustomModelRoutes,
|
||||
refreshAllCustomModelHosts,
|
||||
readCustomModelEndpointsEnabled,
|
||||
detectCustomModelSwapDisplacements,
|
||||
pruneIdleLlamaSwapLogTails,
|
||||
tryWebviewRefererFallback,
|
||||
@@ -2761,9 +2762,18 @@ export class WebServer extends EventEmitter {
|
||||
if (!this.testMode) {
|
||||
this.cleanup.setInterval(
|
||||
() => {
|
||||
refreshAllCustomModelHosts().catch((err) => {
|
||||
console.error('[custom-model] periodic re-discovery failed:', getErrorMessage(err));
|
||||
});
|
||||
// Reads the setting fresh on every tick, same reasoning as
|
||||
// readPlanUsageTelemetryEnabled() beside it: a live toggle takes effect
|
||||
// on the very next cycle, not just at server boot, and turning the
|
||||
// feature off actually stops the polling instead of only hiding the UI.
|
||||
void readCustomModelEndpointsEnabled()
|
||||
.then((enabled) => {
|
||||
if (!enabled) return;
|
||||
return refreshAllCustomModelHosts();
|
||||
})
|
||||
.catch((err) => {
|
||||
console.error('[custom-model] periodic re-discovery failed:', getErrorMessage(err));
|
||||
});
|
||||
},
|
||||
CUSTOM_MODEL_REDISCOVER_INTERVAL_MS,
|
||||
{ description: 'custom model endpoint re-discovery' }
|
||||
|
||||
Reference in New Issue
Block a user