Empty declares `flex-1 … justify-center` (empty.tsx:9), but the list pages wrapped
it in a plain `flex flex-col`, so it collapsed to its natural height and hugged the
header while api-tokens — same primitive, same "nothing yet + CTA" shape — centred
in its `flex-1` wrapper. At 1920 the CTA jumped ~350px between the two pages.
The five wrappers on knowledges, templates and flows (loading + empty states) now
carry flex-1 like the eight that already did. SidebarInset is `min-h-svh flex-1
flex-col`, so the height is there to fill. Measured at 1920: knowledges and
templates now report the same 564 centre / 24px offset as api-tokens.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The size-4 sweep stripped four icons the sweep had no right to touch, and they
rendered at lucide's default 24px: the avatar's UserIcon and the theme
switcher's Monitor/Sun/Moon in the sidebar user menu.
The sweep assumed every listed container sizes its icons at any depth. Only
Button/Toggle/CommandItem do, via [&_svg]. SidebarMenuButton and DropdownMenuItem
use [&>svg], which reaches a DIRECT child only — and these four sit deeper, inside
an AvatarFallback and inside TabsTrigger, so nothing sized them.
Found by measuring rendered icon width across every route with the menus open,
not by re-reading the selectors: the avatar icon was the only one a closed-menu
pass could see. Verified after: no icon inside a button or menu row renders over
16px except EmptyMedia's, which is meant to be large.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The remaining 20 rendered Loader2 spinners carried a deliberate size or colour, so
the earlier pass left them. They convert cleanly after all: Loader2 is a lucide
alias of LoaderCircle (`LoaderCircle as Loader2` in lucide-react's d.ts) and that
is exactly what Spinner variant="circle" renders, while Circle merges an incoming
className over its built-in animate-spin. So each site keeps its size/colour class
and only drops the now-redundant animate-spin. Every rendered spinner in the app is
the Spinner primitive now.
flow-status-icon and flow-task-status-icon keep Loader2: there it is a value in a
Record<StatusType, { icon: LucideIcon }> map, not a rendered spinner, and Spinner
is a variant wrapper rather than a LucideIcon — it does not fit that slot.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two mechanical cleanups over the same files.
<Loader2 className="size-4 animate-spin"> and <Loader2 className="animate-spin">
appeared 28 times across 20 files while Spinner variant="circle" — the same
LoaderCircleIcon with animate-spin already baked in — was the idiom elsewhere.
Loader2 spinners with a deliberate size (size-3/5/6/10/16) or a colour are left
alone: Spinner sets no size of its own and would fall back to lucide's 24px.
Separately, 51 icons carried className="size-4" inside a Button, DropdownMenuItem
or AppHeaderAction, all of which already force [&_svg]:size-4 on descendants, so
the class was a no-op. Only those 51 are stripped — the sites picked by walking
each icon's real JSX ancestors through the TypeScript AST, because an indentation
heuristic mistakes multi-line opening tags for the parent. The 40 icons with no
forcing ancestor (Alert, TabsTrigger, AppHeaderTitle, a resize handle) keep their
size-4: without it they would render at lucide's 24px.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The remove X was a raw button. It is a ghost Button now, sized to the chip's own
26px so the whole chip height is a hit target instead of the bare 14px glyph.
Negative margins keep the geometry identical: -my-[5px] and -ml-1 -mr-1.5 shrink
the margin box back to the 14px footprint the raw icon had, and [&_svg]:size-3.5
holds the X at 14px against Button's [&_svg]:size-4 descendant rule, which would
otherwise win on specificity. Measured against the old markup on the real
stylesheet: chip 26px and 107.3px wide, X 14px, X 21px from the chip's right edge
— all unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Test affordance in each agent accordion header was a raw span reimplementing
outline-button chrome by hand (rounded border px-2 py-1 + hover:bg-accent + a
manual disabled style). It now composes Button asChild variant="outline" size="xs"
over that span, so it inherits the same outline chrome as every other action
button.
It stays a span, not a button: AccordionTrigger already renders a button and a
button-in-button is invalid HTML — verified live, zero nested buttons in the DOM.
The disabled styling stays hand-rolled because a span can't be :disabled, and the
onClick keeps its stopPropagation so clicking Test still doesn't toggle the row.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The four detail-page saves rendered two ways: knowledge as AppHeaderAction (a
primary CTA that collapses to an icon on mobile), the other three as a secondary
FormSubmitButton. FormSubmitButton buys nothing here — all three sit in the
header, outside the form's FormProvider, so its useFormContext read returns null
and they already submit via form= + a manual loading prop. Its real value is the
in-<Form> subscription, which its 8 dialog/auth consumers keep using untouched.
AppHeaderAction gains an optional loading (icon->spinner + disable); the three
header saves become AppHeaderAction like knowledge, so all four are now the same
primary CTA that collapses to an icon-only button (aria-label preserved) below
md. type="submit" is explicit because Button defaults type to "button". Verified
at 480px: label hidden, 32px icon button, aria-label "Create"; at 1440: primary
fill, label shown.
Researched the alternatives first: React 19 useFormStatus can't read a
form=-associated button (react.dev; facebook/react#27980) and targets native
form actions not RHF; and a Radix Slot asChild compose throws on FormSubmitButton's
two-child array and can't inject the spinner past child-wins prop merging.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Template title was the only single-line field with neither a FormLabel nor a
FormMessage, so "Title is required" could never surface and the field was
unlabeled while knowledge/provider label theirs. It now has both — verified live:
submitting empty shows "Title is required".
Also: descriptive placeholders on the template title and editor (matching the
sentence-style placeholders elsewhere), the Rename dropdown icon to size-4 (its
siblings are size-4), and the new-flow intro to the shared block (h2 + gap-2
instead of h1 + mt-2) so every create/edit intro is identical.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The provider Type combobox rendered as a Button outline — opaque bg-background,
a ChevronsUpDown icon and font-medium text — while every select on the knowledge
form is a transparent-bg SelectTrigger with a single ChevronDown. Side by side
the Type field read as a different control.
The combobox trigger now carries bg-transparent, px-3, font-normal and a
ChevronDown size-4, matching SelectTrigger; the model combobox's addon chevron
follows suit. Measured against the canon at 1440: transparent bg, chevron-down
16px opacity .5, h-9, px-3, weight 400 — identical.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The variables and presets popovers scrolled the page once their own scroll hit
the end — overscroll-behavior defaulted to auto and a non-modal popover doesn't
lock the body. overscroll-contain keeps the momentum inside. Reproduced at
375x640: wheeling past the popover's end moved the body 0 -> 204px; with contain
it stays at 0.
The prompt variables card now shows the variable count on desktop too, matching
the preset-count badge the template card already carries.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Below the split the presets card was a 599px block that sat under the fold, past
the editor — on the new-template form the editor is empty and the presets are how
you fill it, so the primary tool was hidden behind a scroll.
The panel now mirrors the prompt variables one: on desktop it stays the card in
the split; below 1280 its header becomes a full-width secondary trigger with the
preset count, opening a non-modal popover that holds the same preset list. The
list is shared by both wrappers; only the wrapper differs. In the stack the
trigger takes the presets' desktop slot — after the title, before the editor.
Applying a preset from the popover closes it (the popover is controlled), then
either fills the form or raises the existing replace-confirm dialog when the form
already has content — verified live at 390: fill, replace-confirm, and expanding a
preset preview inside the popover all work, and the desktop split is unchanged.
presetsList drops its useMemo to take an onApplied callback; typing does not
re-render the form (FormField isolates it), so the 11 collapsibles are not on the
keystroke path — confirmed by zero slow input events while typing on desktop with
all of them mounted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Below the split, preventing the popover's open-autofocus left focus on the
trigger while the content sat in a body portal, so the first Tab landed in the
editor and the second dismissed the popover: the chips could not be reached by
keyboard at all. Before the popover existed they were inline tabbable badges, so
this was a WCAG 2.1.1 regression.
Both autofocus handlers are gone. They were written to protect the editor caret,
but ProseMirror keeps its selection across DOM blur, so they were never needed —
verified live at 390: opening now focuses the first chip, Escape returns focus to
the trigger, and clicking a variable still inserts at the caret the user left,
with the editor focused and the popover closed.
Also stop the shared chip cloud painting bg-background over the popover: dark
--background matches the page behind it, which flattened the popover to a
borderline. The two-tone well is the desktop card's business, so the fill moved
onto its wrapper, where it still contrasts bg-card.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`pnpm run prettier` is a required CI step and it exits 1 on these five files at
HEAD while passing on them at 001c6c0, so the branch is currently red. Formatting
only — no logic touched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The badge default height is 22px — text-xs' 16px line box, py-0.5 and a 1px
border — which crowds the 32px trigger. h-5 pins it to 20px; border-box leaves
18px of content for the 16px line box, so nothing clips.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The count badge sat on a bg-secondary button, where every neutral surface token
is within 1.08–1.23 contrast of it: secondary painted the pill invisible and
outline drew its border in --border, at 1.18. Only --primary separates (2.76), so
the badge is default.
The tab overrides exist because the desktop panel sits on its own surface. Below
the split they fought the shadcn defaults, so they are xl:-scoped now — the same
1280 the layout branches on. Stacked widths get bg-muted with a bg-background
active tab again; 1280+ keeps bg-background with a bg-card active tab.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The trigger is a compact secondary button carrying just its label and, on the
right, the variable count — same count-badge shape the sheet and template
headers already use.
The badge is outline, not secondary: a secondary badge paints bg-secondary onto
a bg-secondary button, which renders the count as bare text.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The header and its hint were rendered twice below the split breakpoint: once as
the trigger label and again at the top of the popover it opened.
The header belongs to the wrapper, not to the shared part. On desktop it stays
the card header; on narrow widths it becomes the trigger, and the popover now
carries only the chips. Title and hint move to constants so the two wrappers
cannot drift apart.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three follow-ups to the variables palette.
The chips composed badgeVariants onto a raw <button>, going around the primitive,
because Badge renders a div. Badge now accepts asChild through the same Slot
switch button.tsx already uses, so a chip is <Badge asChild><button>: the badge
keeps owning the styling, the button keeps native semantics, and the hand-rolled
role/tabIndex/Enter-Space handler stays gone. className has to stay on Badge —
Slot concatenates classes, so only cn() resolves font-normal against
badgeVariants' font-semibold.
Variables reads useBreakpoint itself rather than taking isDesktop from the page.
Every other consumer here does that, sidebar.tsx included, and it keeps the panel
droppable into the next page that needs one.
The narrow-width trigger is a full-width labelled button now. It carries no
chevron and no count badge: those, not the width, are what make a control read as
a value select.
Also size the popover by --radix-popover-trigger-width, the way autocomplete.tsx
does. It was 100vw wide and spilled across the sidebar between 768 and 1280.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In the stacked layout the "Available variables" cloud sat between the tabs and
the editor, pushing ~530px of chips ahead of the content it annotates. The panel
now keeps its slot and its desktop rendering, and below the split breakpoint the
same cloud moves behind a { } trigger, leaving the editor at the top.
The cloud itself is shared by both wrappers; only the wrapper differs (card vs
non-modal popover). The popover must not trap focus: insert and cycle act on the
editor's stored selection, and cycling only helps if its highlight stays visible.
Chips are real buttons now — they were divs with role=button and a hand-rolled
Enter/Space handler that the native element provides for free.
Verified live at 500px: opening keeps the caret, clicking a used variable selects
the next occurrence in view, the popover closes as the editor takes focus back,
and the desktop split is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On narrow viewports the editor was rendered between the prompt meta/tabs
and the "Available variables" panel, splitting one logical block in two and
forcing a different composition per breakpoint. Merge them into a single
panel block that the desktop split and the mobile stack both render as a
unit, so the variables stay attached to the tabs they belong to.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Consolidate every empty / loading / error / not-found / error-boundary
state off the bespoke StatusCard onto the canonical shadcn Empty (already
used by DataTable) and delete components/ui/status-card.tsx. Empty renders
borderless and centered, so the wide-viewport "letterbox" look is fixed at
the root instead of capping a solid card.
Also in this pass:
- DataTable: truncate column headers so plain-string headers in narrow
fixed-size columns (e.g. "System Prompt") stop wrapping to two lines.
- Settings > Providers / API Tokens: move the Create action into the page
header (icon-collapses on mobile) and the API-token Playground/Swagger
links into a header overflow menu; drop the cramped description row that
needed the earlier truncate/flex-wrap workarounds. Those links use a
relative href since the API base path is root-relative.
- Settings > Prompts: drop the fixed size on the Name column of both tables
so it flexes to fill (matching Providers) and truncates.
- Settings > Account: keep "Member since" on one line at mobile widths
(min-w-0 + truncate, badge shrink-0).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an Off option to the per-agent Reasoning Mode control, gated on the model's
cannotDisable capability so it appears only where a disable actually takes effect;
relax the adaptive-only lock for disablable models; disable effort/max-tokens when
Off. Reconcile a stale Off when the selected model can't disable (e.g. after typing
a custom model name) so it cannot silently persist. Map the off mode through the
form->GraphQL conversion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an explicit reasoning Off to the per-agent config, backed by the langchaingo
tri-state API. Off emits llms.WithReasoningDisabled() (the provider disable wire)
via BuildOptions, and UsesAdaptiveThinking is guarded so Off overrides the
adaptive-only auto-adaptive.
Per-model capability is derived at runtime from llms.ReasoningSupportFor and
surfaced through GraphQL ModelReasoningInfo, so the UI only offers Off where it
actually disables: cannotDisable reports when Off would be rejected (always-on
models) OR a silent no-op (an unclassified default-on model whose disable wire is
omitted), and capability is surfaced for any thinking-capable model (e.g. Gemini,
which declares thinking without a reasoning block). A new
ProviderType.ReasoningProvider() supplies the provider to the resolver.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- demuxExecStdout errors on a header cut short mid-frame (was a silent EOF that
dropped the tail) and surfaces a docker systemerr frame instead of discarding it.
- ftester drains telemetry on exit through the bounded observer.Drain instead of
two unbounded ForceFlush calls, so an unreachable collector can't hang it at exit.
- container-listing failures are sorted for a deterministic skipped-entries preview,
logged per-entry at Debug (the detail is already in the response and the endpoint
is hit on every navigation) with names quoted so control bytes in a hostile
filename can't inject into a log line; corrected the stat-failure comment
(a dangling symlink lstats fine, it doesn't fail).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The truncation slice and the demux stdout byte-cap had no docker-layer tests —
only the handler's Truncated wiring was exercised through the fake, so a
mis-slice or a dropped cap would have gone unnoticed. Extract find-output
parsing into a pure parseFindEntries and take the byte cap as a demuxExecStdout
parameter, then unit-test both boundaries (at cap / cap+1 / over-limit stream).
Also bound how many paths one container-files request may list
(maxContainerListPaths), so the per-path entry cap can't be multiplied by an
attacker-chosen path count into a large fan-out or response body.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The container-listing section still described the removed `ls -1` parse,
`pkg/queue`, and the old `[]container.PathStat` return. Update it to the current
`find -print0` + errgroup fan-out returning `ContainerDirListing{Files, Failures,
Truncated}`. Also remove the stale `WithBlock`/`DefaultDialTimeout` paragraph left
above the non-blocking `grpc.NewClient` test — its own next paragraph already
contradicted it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Container-listing polish on top of the partial-success work:
- A directory whose entries all fail to stat no longer 500s. The find exec
already proved the container alive, so ListContainerDir returns the readable
entries (possibly none) plus the per-entry failures, and the handler serves a
200 partial listing; only a cancelled request or a dir that can't be listed at
all still errors.
- Over the entry cap, list the first page and set a Truncated flag rather than
erroring with end-user copy from the docker layer. The flag flows through to
the Pull dialog, which now warns the user the directory was truncated instead
of silently showing a subset.
- The Pull dialog no longer flashes a skeleton on an in-place refetch (Refresh,
or after a Pull): the listing hook clears its rows only when the path actually
changes, so a same-path reload keeps the current rows under the loading guard.
Tests: an all-entries-failed listing returns 200 with the failures; the
Truncated flag surfaces in the response. Swagger regenerated for the new field.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three defects on the shutdown and container-listing paths:
- Telemetry drain ran the langfuse and otel sinks serially under one deadline.
langfuse's ForceFlush/Shutdown ignore the caller ctx and block on their own
timeout, so a langfuse outage burned the whole budget and the healthy otel
sink's final batch was never flushed. Add observer.Drain, which flushes then
shuts down each sink in its own goroutine bounded by the deadline; main uses
it, retiring the inline flush-then-shutdown dance.
- The partial-success listing put the raw stat/list error into the 200 body's
Failures[].Message, exposing docker-layer detail (container ids, the daemon
address) that response.Error only reveals under develop mode. Send a generic
message to the client and keep the raw text dev-gated, logged server-side.
- A path read successfully by one query could also appear in Failures when
another query failed on it, order-dependent. Reconcile at assembly: drop any
failure whose path was read successfully, so no path is in both arrays.
Also bound the exec stdout buffered before parsing so a compromised sandbox
can't stream unbounded output into memory.
Tests: Drain delivers a healthy otel sink while a langfuse sink blocks and
returns within its deadline; the listing failure message is dev-gated; a path
never lands in both Files and Failures. Live: unreachable collector -> shutdown
stays bounded (~5s) and exits cleanly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The graceful-shutdown branch only logged "cleaning up resources" and exited
without flushing telemetry, so the last buffered batch was dropped on every
SIGTERM. Drain it, with three corrections:
- main: flush then shut down the observer on shutdown, bounded by a 5s
deadline in a goroutine so an unreachable collector can't stall exit.
Flush must precede Shutdown because the langfuse observer's Shutdown
cancels its context without flushing; its final batch drains only via
ForceFlush while the context is still live.
- observer Flush/Shutdown: drain both the langfuse and otel sinks and join
their errors instead of returning after the first, which skipped the
second sink entirely when both were configured.
- telemetryClient Flush/Shutdown: attempt all three providers and always
close the grpc connection, joining errors, so a stalled provider flush
no longer skips the rest or leaks the connection.
Tests: the langfuse observer test proves Shutdown drops the buffered batch
while ForceFlush drains it to the sink; observer-level tests prove both
sinks drain and errors aggregate. Live: with an unreachable collector,
shutdown stays bounded (~5s) and exits cleanly instead of stalling ~10s.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The startup dial used grpc.WithBlock with a 10s timeout, so a set-but-
unreachable collector stalled main() for up to 10s, and once that dial failed
the client was nil forever — a collector that came up later never connected
without a restart. Switch to grpc.NewClient (non-blocking): startup never
stalls and gRPC re-establishes the connection in the background on its own.
Also build all three exporters before any provider, so the exporter-error
cleanup (closing the connection) happens before any batch/reader goroutine
starts and can't leak one. Drops the deprecated WithBlock/WithReturnConnectionError.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ContainerFiles gained a Failures list when the container listing switched to
partial-success; regenerate the Swagger docs (swag v1.8.7, the pinned version)
so generated clients see the new field.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Ignore the local `backend/vendor/` tree (187 MB, present only for the
vendored-fork build) so it can't be accidentally staged.
- Remove the architecture-table row for `pkg/queue/`, which was deleted when
the container stat fan-out moved to errgroup.
- Drop the internal "H4" report-id labels from the telemetry test — the
sentences already say what they guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
find -print0 already delimits with NUL, but the exec still ran under a TTY,
whose onlcr translation rewrites every \n in the stream to \r\n — including a
\n that is part of a filename — so a file literally named with a newline was
stat'd under the mangled name and reported unreadable. Run the exec without a
TTY and demux the resulting multiplexed stdout/stderr stream by hand (no new
dependency), so filename bytes reach the parser untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Navigating to a new directory in the Pull dialog kept showing the old files
and old "N entries could not be read" banner until the response landed (the
hook never cleared its data at fetch start), and when every entry failed the
per-entry messages were shown nowhere. Clear files/failures when a fetch
starts, and render the first few failure messages in the all-unreadable
empty state.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The multi-path container listing still fail-fasted: a single bad top-level
path (nonexistent, not a directory, list failed) returned HTTP 500 and
blanked every path that had already listed. Record each such path as a
failure and keep going so the readable paths are still returned; only a
cancelled request aborts, and if every path fails at the directory level the
request is failed as a whole. Also drop a per-entry failure whose path was
already read successfully by an overlapping query.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Parsing `ls -1` output was unsafe: under the TTY the exec allocates, GNU
coreutils shell-quotes names — so a readable file with a space, quote or
non-ASCII byte was stat'd with the quotes and reported unreadable — and
busybox wraps every entry in ANSI colour codes, so on an alpine/busybox
image the whole listing came back unreadable. Switch to `find -maxdepth 1
-print0`: literal bytes, portable across GNU and busybox, and the NUL
delimiter survives filenames containing newlines. Cap the entry count so a
directory with hundreds of thousands of files can't fan out into that many
Docker API calls, and classify a cancelled request or an all-entries-failed
listing as a directory-level error instead of a misleading empty 200.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The container file endpoint now returns HTTP 200 with the readable entries
plus a Failures list when some entries can't be stat'd, instead of failing
the whole request. Plumb those failures through useFlowContainerFiles and
show a non-blocking warning banner above the readable files ("N entries
could not be read"), reserving the full error empty-state for a true
listing failure. When nothing is readable, the empty-state says so rather
than "Directory is empty".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A single unreadable directory entry (dangling symlink, a file removed
between ls and stat, a transient /proc entry) used to fail the whole
listing with HTTP 500, blanking the file browser and discarding every
readable sibling. The frontend already expects /proc/sys to not fail
spuriously, but the backend did the opposite.
ListContainerDir now returns a ContainerDirListing{Files, Failures}: per-
entry stat errors no longer abort the batch. GetFlowContainerFiles serves
the readable entries as HTTP 200, carries the failures back in a new
ContainerFiles.Failures field, and logs each skipped entry (capped) plus a
degradation summary. Directory-level faults (not a dir, ls failed, container
gone) still return 500 — there is no partial to show. statContainerEntries
returns successes + failures instead of the lowest-index error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The deleted queue normalized workers<=0 to a default; the errgroup helper
passed the count straight to SetLimit, where 0 deadlocks the first Go() on an
unbuffered semaphore and a negative value disables the bound entirely. The
sole caller hardcodes 20 so this was latent, but clamp it back so a future
config-driven count can't hang the listing or flood the Docker daemon.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ListContainerDir was the only consumer of pkg/queue and discarded its ordered
delivery (the caller dedups and re-sorts). Replace the fan-out with a small
errgroup helper — SetLimit keeps the same 20-way bound on concurrent stats
against the Docker daemon, the caller's context propagates into each call,
results stay in input order, and the lowest-index failure is surfaced
deterministically — then delete pkg/queue. The helper was verified byte-
identical to the previous queue implementation across 600 randomized directory
shapes (sizes, failure positions, out-of-order completion) before removal.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds a fake OTLP collector so NewTelemetryClient can be exercised against a
reachable endpoint: it builds all three providers and both ForceFlush and
Shutdown export and tear down without error. Previously only the
not-configured and unreachable-dial paths were covered.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds scenarios for a non-positive worker count (must fall back to the
default, else nothing drains the queue and delivery hangs), a nil process
function (logs and drops each item without panicking), and a stable
instance id. Brings queue.go to full statement coverage and kills the
mutants that previously survived on the workers<=0 fallback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Unit coverage for NewTelemetryClient: an unset OTEL_HOST yields ErrNotConfigured,
and an unreachable collector returns within the dial timeout instead of hanging
(a TCP-accepting, silent listener drives the WithBlock path).
Also close the grpc.ClientConn on the exporter-creation error paths — a
successful dial followed by a failed exporter New() previously leaked it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The two-context design in ddd3916 was correct but fragile: the field named
`ctx` was the liveness signal goroutines must NOT bail on, while the real
cancellation signal was `stopCtx`. That inversion is the exact trap that
produced the earlier happy-path regression (bailing on `ctx`, which a normal
input-close cancels, dropped in-flight results). It also left a latent
restart-without-Stop hazard: a second Start() after input-close orphaned the old
stopCtx and shared wg, deadlocking a later Stop() (with a data race).
Collapse `ctx` into a plain `running` bool (it was only ever read via .Err(),
never awaited), keep `stopCtx` as the single cancellation signal, and guard
Start() on stopCtx so a restart requires a prior Stop() — removing both the
naming trap and the restart hazard.
Comprehensive tests (queue_scenarios_test.go): boundary-N full delivery,
contiguous-prefix-on-abandon, a randomized prefix-invariant fuzz, goroutine-leak,
restart-reject, process-error, double-stop, Running() transitions. The happy-path
tests hang on the pre-rework code and pass here (side-by-side under -race). Live:
a flow's non-empty container dirs of 1/40/100/500 files each deliver every entry;
the error path returns 500 without hanging.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous fix (97e5730) had workers and the reader bail on q.ctx, but the
reader also cancels q.ctx on a normal input-close — so the ListContainerDir
happy path regressed: after input closed (buffer names, close, read all N),
workers dropped still-undelivered results and the consumer hung waiting for the
last one. Any non-empty container directory that didn't error early hung the
file-manager request.
Split the signals: q.ctx still tracks "running" (cancelled by input-close or
Stop), and a new q.stopCtx is cancelled only by Stop(). Workers and the reader
bail on q.stopCtx, so a normal input-close drains and delivers every result
while a hard Stop() still unblocks a consumer that abandoned output. Stop()'s
already-stopped guard now checks q.stopCtx (input-close alone must not
short-circuit it, or blocked workers leak).
Tests: DeliversEveryResultAfterInputClose (hangs on the old fix, verified
side-by-side under -race) + StopHardStopsAfterInputClose; deadlock + ordering
still green under -race.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A set-but-unreachable OTEL collector hung startup: NewTelemetryClient dialed
with grpc.WithBlock() and no timeout on the deadline-free signal context, so
the process blocked forever before it ever served. And a non-ErrNotConfigured
init error from either observability client was log.Fatalf, killing the
process. Both let an OPTIONAL integration take the whole app down.
Bound the dial with a 10s timeout, and on init failure degrade to a no-op
observer with a logged warning instead of exiting. Verified live: a bad
OTEL_HOST that hung startup indefinitely now boots in ~14s with a warning and
runs a full flow; a reachable collector still boots in ~4s with no warning.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ListContainerDir aborts on the first stat error and stops reading the queue's
output channel, leaving workers blocked on the unbuffered `q.output <- result`
send and the reader blocked on a full `q.queue`; Stop() -> wg.Wait() then hung
forever. Select every pipeline send/wait on q.ctx so a stopped queue unwinds.
On stop a worker returns without msg.cancel(), so later workers also bail via
q.ctx and output ends at a contiguous prefix instead of developing gaps.
Regression test drives the exact hang (unread output -> Stop must return),
verified with -race.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The openai-compat consolidation dropped the comment warning that DeepSeek needs
the legacy top-level "reasoning_effort" string form; without it a maintainer
could add openai.WithModernReasoningFormat() to the shared opts and silently
break DeepSeek thinking mode. Restore it at the shared opts choke point.
Also fix two stale pointers: glm/qwen config.yml cited
WithPreserveReasoningContent() "in glm.go/qwen.go", but it moved to
openaicompat.go during that same consolidation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Export tokenNameSchema and pin the 100-character boundary (100 accepted, 101
rejected) plus its trim/default behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- prompts_test.go: PatchPrompt rejects a syntax error, an undeclared variable,
and a whitespace-only template over REST (the path a raw client hits when the
UI is bypassed) and does not persist them; valid templates still create/update.
- validation_test.go: validateKnowledgeFieldLengths accepts each field at its
max length and rejects one character over.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>