Files
multica/server/internal/handler/agent_builder_test.go
Jiayuan Zhang b99b04bb86 feat(onboarding): Mika issue-first onboarding (#6378)
* feat(onboarding): Mika issue-first onboarding

Replaces the starter-agent welcome with one conversation: onboarding creates
Mika, the workspace's built-in Chief of Staff, and opens a real chat whose
first turn is a product-authored kickoff hidden from the transcript. Every
workspace — first and subsequent — is created through this flow.

Mika is a system agent, not an agent-template instance. Her product prompt is
//go:embed-ed and composed at claim time, so a release updates it without
touching any workspace's row; the row holds only the workspace's own notes.
Creation is server-owned and idempotent under a per-workspace advisory lock,
and archiving a system agent is rejected.

This is the pre-merge half of the branch, squashed while rebasing onto main.
Replaying its fifteen commits individually meant re-deriving each one against
a main they were never written for; the net change reconciles against today's
main in four files, so it is reconciled once, here.

Three of those four are main moving under the branch: MUL-5573 took
quick-actions generation server-side and dropped QuickActionsDisabled /
RegenerateQuickActionsFor from the task payload and the SendDirectChatMessage
signature, so this takes main's shape and keeps only the onboarding entry
point. The fourth keeps main's OnboardingLogoutButton wrapper around the
flow's new mode/onCancel props.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): let quick-action chips carry the opening's examples

Chat now renders agent-suggested follow-up actions as buttons under a reply
(MUL-5149), and the onboarding kickoff qualifies for that suggestion pass
with no extra wiring — it is a direct chat turn on a web session with a
non-empty reply.

That made the opening's fourth beat redundant: Mika wrote a three-to-five
line menu of example tasks, then three chips appeared underneath offering
the same thing. The prose menu is the worse half — a member has to retype a
line they read, but can send a button — so the beat is gone and the opening
budget drops with it. Measured on a real local run: the first reply went
from 307 to 208 characters, and the chips still arrived 21s after the
kickoff.

The questionnaire profile stays in the kickoff. The suggestion pass resumes
the same provider session, so the profile now steers the chips as well as
the reply; only the sentence naming its purpose changes.

Co-authored-by: multica-agent <github@multica.ai>

* refactor(onboarding): cut the step rails down to what the screen can't already show

The right rail carried more than twice the words of the column members were
actually working in — 42 vs 36 on the workspace step, 91 vs 34 on the runtime
step — and it got heavier the further in the flow went, which is backwards.

Step 3's rail was the clearest case. Its "Good to know" section promised the
runtime was swappable and that more could be added later; the step's own lede
already said both in nine words rather than forty-two, on the same screen. Its
60-word definition of "agent runtime" sat beside a list of named, online
runtimes under a headline reading "This computer is connected" — by then it
answers a question the member has stopped asking. What survives is the one
thing the screen does not show: what that background process is.

Step 2's rail keeps the workspace preview card, which does show something not
otherwise visible, and drops the bullet lists — promises the product is about
to keep on its own.

The freed words did not move to the rail; one moved into the main column. On
the create path "Mika" was never introduced before Step 3 used the name twice,
once on the primary button, so the lede now names the role in an appositive
right above that button. Mika stays ungendered, as everywhere else in the
product.

Net across the three regions: 167 words to 89.

Co-authored-by: multica-agent <github@multica.ai>

* refactor(onboarding): drop the right rail from the workspace and runtime steps

Every remaining rail item was either something the screen already showed or
something the product was about to do anyway, so the column was costing a
member's attention without answering a question they had. Removing it leaves
each step a single full-width column — the shape the questionnaire step has
always had, and the only step nobody has complained reads as sparse.

Gone with it: RuntimeAsidePanel, the workspace preview card and its entity
rows, and 27 copy keys per locale. The two runtime paths (desktop
runtime-connect, web platform-fork) shared that panel, so both lose it in one
move and stay identical.

The welcome step keeps its column — it holds an illustration, not prose.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): put every step's header and content on one measured axis

Removing the right rail left the four steps geometrically inconsistent in
four ways, all of which the member sees as things moving while they advance.

The header carried the horizontal padding itself, so it ran flush to the
window edge while the content column stayed centred. A 480px rail had been
absorbing that difference; without it the two were 267px apart on a 1283px
window, and since StepHeader is justify-between the step indicator floated
off at the far right, ~270px from anything it labelled.

The rest compounded it: the measure changed between steps (920px on the
questionnaire, 620px after), the header bar and content block used different
vertical padding per step, and padding living inside a max-w box made the
reading width jump from 508px to 620px at the lg breakpoint.

All four now come from step-shell.tsx. Padding belongs to the gutter, never
to a measured box, so the reading width is constant from ~700px up. The
header measures on STEP_FRAME on every step, so the one element that survives
each transition never moves; content picks STEP_FRAME or STEP_COLUMN by what
it holds, and both centre, so a step that needs the width still sits on the
header's centreline. Vertical rhythm is one value.

The header was near-identical in four files, which is how it drifted in the
first place, so it is now one component. Its test pins the invariant that
broke — padding out of the measured box, header measured on the frame — and
fails if either is put back.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): give the runtime step the frame measure and cut its copy

Nine runtimes in a 620px column meant truncated names, five rows, and
scrolling to reach the last four, with ~330px of dead space on each side.
This is the case step-shell's wider measure exists for, so the step takes
STEP_FRAME — which also puts it on exactly the header's measure — and the
card grid gains a third column at lg. Nine runtimes now land in three rows.

Copy went with it. The headline was two sentences over two lines; the first
one, "This computer is connected", is already said louder by the "9 agent
runtimes · all online" row directly beneath it, so only the instruction
remains. The lede was 44 words and five lines — I had grown it myself adding
Mika's introduction — and is 20 now, still naming the role. The
remote-computer note drops from 25 words to 15.

Prose stays capped at 620px inside the wider frame; a 920px measure is for
the card grid, not for reading.

The found-phase test keyed on the headline copy, so a copy edit read as a
behaviour regression. It now asserts the runtime count row, which is the
signal the test is actually about.

Co-authored-by: multica-agent <github@multica.ai>

* refactor(onboarding): put the workspace step on the shared frame

It was the only step still measuring at STEP_COLUMN, so its eyebrow, headline
and footer CTA started and ended ~150px inside where every other step's did —
the page margins visibly moved when you advanced from step 1 to step 2.

The step now sits on STEP_FRAME like the other two, with a new STEP_MEASURE
capping the prose and the form inside it. Matching the frame is not the same
as widening the field: a workspace name does not want an 800px input, so the
form keeps its reading measure and left-aligns to the frame instead. The
footer row spans the frame, which is what puts the CTA in the same place on
all three screens.

STEP_MEASURE deliberately does not centre — centring would pull the content
off the frame's left edge, undoing the alignment. Its test pins that.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): close the gap above the step CTA and put Log out back on one row

Two defects that read as one thing on screen: a large empty block sitting
directly above the primary button.

The card list was capped at STEP_MEASURE along with the form. That cap exists
so a workspace name does not get an 800px input — a good reason for a text
field and a bad one for selection cards, which are a list like the runtime
grid. Capped, they stopped 299px short of the CTA, leaving a void above the
button. The cards take the frame now; only the form keeps the reading measure.

Log out came in from main as `fixed right-8 top-8`, pinned to the window
corner. Its own comment says the fixed position exists to survive the flow's
full-bleed layouts — which is what the measured frame replaced, so it landed
outside the measure and above Back / Step N of N as a second header row. It
now rides the header row on the frame.

StepShellHeader takes it as a `trailing` slot rather than rendering it:
calling useLogout inside the shared header forced a QueryClient into five
step test files just to render a header bar. The flow injects it, matching
how runtimeInstructions is already threaded, and the header stays
presentational.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): make step 3 about Mika, with the runtime as a sub-decision

The step was titled after its dependency — "Pick an agent runtime" — while
the thing actually being created was named only in the grey lede. A member
reached a button reading "Start with Mika" without having been told who that
is, and said so: being confused there is not a failure to read carefully, it
is the page putting the lead role in a footnote.

So the subject changes rather than the step count. The headline names the
outcome, a card carries the introduction with a mark on it, and the runtime
list drops to a labelled sub-section under "Where should Mika run?". Reading
order becomes: who you are getting, she needs a machine, pick one, start.

MikaIntro sits above the phase switch so the subject holds still while the
runtime block below cycles through scanning / found / empty, and each phase's
own heading drops from h1 to h2 now that the page has a real h1.

Mika does not exist yet at this point — she is created on commit — so the
card cannot render her stored avatar. It reuses the mark the Runtimes page
already uses for "Start with Mika", so the two entry points read as the same
thing.

No fourth screen: the introduction and the only decision on this screen are
one beat, and splitting them would add a step between finishing setup and the
payoff. The defect was never a missing screen, it was an invisible
introduction.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): pick the runtime and the model from dropdowns

Nine runtimes as cards took three rows and left nowhere to put a second
decision. Two dropdowns fit both on one screen, and the model is a real
choice the card grid had no room for.

Both controls already existed on the agents surface — RuntimePicker and
ModelDropdown, the same pair create-agent-dialog uses — so this reuses them
rather than growing a parallel picker. ModelDropdown owns its own discovery,
grouping and unsupported-runtime states, and selecting a different runtime
clears the model because models are per-runtime.

The model now reaches the agent: POST /api/agents/mika takes an optional
model, CreateSystemUserAgent writes it to the column the agent table already
had, and empty still means "whatever the runtime defaults to" — which is what
every deployment without per-agent model support gets anyway.

Also drops the "No local runtime, or prefer a remote computer?" note. It said
in twenty-five words what the Skip button next to it says by existing, and it
appeared on all three phases.

currentUserId comes in as a prop rather than from the auth store: reading the
store inside the step broke six tests that render it without one, the same
coupling the header slot avoided.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): stop gendering Mika in the intro card

Mika is ungendered everywhere else in the product — the agent instructions,
the onboarding skill and every other locale string avoid a pronoun. The intro
card I added last round reintroduced one in two languages ("she turns it into
an issue" / "她会把它变成一个 issue"), which the Chinese screenshot made
obvious. All four locales now read around it.

Co-authored-by: multica-agent <github@multica.ai>

* feat(ui): port the ReUI stepper primitive

Groundwork for rebuilding onboarding on the @reui/onboarding-3 interaction
model: a persistent vertical stepper with named steps and click-to-return
navigation, which our horizontal "Step N of 3" dots cannot express.

Routed to components/ui rather than the vendor's components/reui namespace —
it is our code now — and rewritten to the role-named type scale (text-xs ->
text-caption, text-sm -> text-label / text-caption, dropping the leading-none
the token already supplies). No other convention fixes were needed: shadcn had
already rewritten the imports, "use client" survived because main's
components.json now sets rsc: true, and it pulls no npm dependency we do not
already have (@base-ui/react is declared).

The block's other twelve registry dependencies are primitives we already own,
so only this one was installed.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): rebuild the step shell as a named progress rail

Onboarding's chrome was a row of dots and a "Step 2 of 3" counter. It told
a member how much was left but never what was coming, so every step arrived
unannounced -- the same reason the runtime step read as a surprise even after
it was retitled "Meet Mika". Naming all three steps up front makes onboarding
legible as a whole before the first field is filled in.

StepShellHeader becomes StepShell, and it owns the window rather than a strip
at the top: rail on the left, the step's own content scrolling on the right.
That lets the four steps drop an identical wrapper/DragStrip/header/<main>
preamble, including the scroll-fade wiring that was duplicated verbatim in
all four and is now set up once.

The rail is built on the ported ReUI stepper, but deliberately not as a
tablist: that component defaults to role=tablist with each trigger owning
aria-controls on a panel id, which is right for a stepper that renders its
own panels and wrong here, where the panel is a routed step and those ids
would dangle. It uses the presentational slots and marks position with
aria-current instead.

Only completed steps are clickable -- moving forward has to run the current
step's validation and submit, so the rail would skip it. New-workspace mode
gets no rail navigation at all: it enters at the workspace step and, once
that workspace exists, every step behind it is gone. Same invariant
runtimeStepBack already enforced for the Back button.

step_header.step_of is replaced by step_nav across all four locales; Mika
stays ungendered in each.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): standardise the steps on the shared UI primitives

Follow-up to the rail, fixing what the rail exposed.

The workspace form was three hand-rolled `flex flex-col gap-1.5` stacks with
their own label sizing and a bare <p> for the slug error -- exactly what
Field/FieldLabel/FieldError/FieldDescription standardise. The manual version
had already drifted: its labels were caption-sized and muted while every
other form in the product labels at body weight.

The platform fork wrapped on STEP_COLUMN while the steps before it wrapped
on STEP_FRAME. Both measures centre, so 620px and 920px put their left edges
~150px apart and the headline visibly jumped right on arrival. It now sits on
the frame and caps its own content, which is the pattern step-shell already
documents.

The About you eyebrow read "About you" -- now the rail's label for that very
step, so the page said its own name twice, once in grey caps and once in the
headline under it. Dropped, along with the locale key. The other steps keep
theirs because they say something the rail doesn't ("Connect a computer",
"Workspace creation is disabled").

Log out is `inline` on every step, and inline now means "on the rail", which
is an inverted surface -- the muted/destructive pair it used unqualified is
mixed from the light palette, so it was rendering as near-invisible grey on
black.

e2e: the smoke spec asserted "Step 1 of 3", text the rail replaced. It now
asserts the rail's named steps and which one is aria-current, and carries on
into the runtime step so all three get captured. Two pre-existing bugs in
that spec surfaced while fixing it: the zh-Hans case pinned its locale cookie
to a hardcoded port, and it advanced with getByRole("button").first(), which
is the pinned Log out button -- so that case had been signing the user out
and asserting against a login redirect.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): rebuild the rail as the ReUI inset panel

The first pass kept the block's idea -- a named vertical rail -- and dropped
most of its structure. Side by side with onboarding-3 the gap was obvious:
no brand lockup, no inset panel, no texture, steps jammed against the top
edge, numbered chips instead of the ring/check/dot progression, stub
separators instead of one continuous track, and a bare Log out where the
block has a footer row.

This ports the structure properly:

- Inset panel -- the aside carries the padding and the panel is rounded with
  a hairline ring, instead of a dark rectangle bleeding to the window edge.
- Brand lockup top-left (the existing MulticaIcon plus a wordmark), Back
  demoted to an icon button top-right, which is where the block puts it.
- Step list centred in the remaining height rather than stacked under Back.
- Indicators follow the block: filled + check when done, ring + dot when
  current, faint ring when upcoming. Numbers move to sr-only text, since the
  ring already encodes position and the digit was redundant next to a label.
- One continuous hairline behind each row instead of a stub between rows.
- DotSphere ported to packages/ui as the panel's texture. It is decorative,
  reads no product state, and already honours prefers-reduced-motion.

Two fixes fell out of doing it properly:

`.dark` is a plain class selector in our token sheet, so scoping it to the
panel redefines the custom properties for that subtree and `bg-background` /
`text-muted-foreground` mean the right thing inside it. That replaces the
hand-mixed `text-background/60` shades of the first pass -- which is what had
made Log out near-invisible -- and it is why the button is back on ordinary
tokens here.

StepperSeparator hardcodes a 3rem height for vertical navs, so an absolutely
positioned track overshot its row and drew the line straight through the next
indicator. Overridden with the same variant rather than !important, so
tailwind-merge drops theirs.

DotSphere cycles three constant arrays by `index % length`. Under
noUncheckedIndexedAccess that is `T | undefined`, so they are typed as
non-empty tuples and index 0 is the fallback -- no non-null assertions.

Co-authored-by: multica-agent <github@multica.ai>

* feat(onboarding): put the content pane on the block's type and layout

The rail matched onboarding-3 and the pane it sat next to did not, so the
two halves read as different products: serif display headlines and a grey
uppercase eyebrow on the right, the block's sans hierarchy on the left.

Typography now maps onto the block exactly, and it lands on our scale
without rounding -- ReUI's `text-xl/7` heading is our `text-title-lg`
(20/28) and its `text-sm/5` supporting line is our `text-body` (14/20).
StepHeading owns both, so no step hand-rolls a headline again.

The eyebrows are gone. The block has no such slot, and with the rail naming
every step, a grey uppercase label above a headline saying the same thing
was the third name for one screen. The workspace step's disabled-state
wording was the only eyebrow carrying information the headline lacked, and
that variant already exists as its own headline copy.

Geometry collapses from three competing measures -- a 920px frame, a 620px
column, an in-frame cap -- to one 28rem column. Three measures is what let
the platform fork sit ~150px right of every other step. STEP_MEASURE stays
for capping a single control inside the column.

Actions move into StepFooter: full-width, stacked, pinned to the bottom of
the column. In a 28rem column the old right-aligned inline bar left the
primary action floating mid-screen instead of where the eye finishes the
form. The column is `min-h-full` rather than centred by the pane, because
`items-center` on a scroll container clips the top of anything taller than
the viewport and these steps do overflow on short windows.

Questionnaire options become the block's wrapping chips. They were
full-width cards in a 4-column grid; inside a 28rem column that grid had
nowhere to go, and stacking all 18 as rows turned one screen into a long
scroll. Chips are the block's own answer for a many-option question. This
also reaches the workspace source-backfill prompt, which shares the
component -- intentionally, it is the same question in the same style.

MikaIntro moves onto StepHeading + Item for the same reason.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): drop the rail clear of the macOS traffic lights

The rail is a dark inset panel and it started at the window's top-left
corner, which is exactly where macOS draws the traffic lights -- so the
close/minimise/zoom buttons sat on top of the dark surface.

The underlying mistake was structural. The desktop shell rule asks for a
single DragStrip as the first flex child of a full-window view; this had two
hand-rolled strips instead, one inside each pane, and neither was first. The
sidebar's was 28px of internal padding trying to duck under the traffic
lights from inside a panel that had already begun above them, which cannot
work -- the panel's own background was the thing being overlapped.

One DragStrip now spans the window above both panes. The panel starts below
it (48px strip + the aside's inset, measured at 64px on a wide window against
traffic lights that end around 32px), the whole band stays draggable, and the
two ad-hoc strips are gone.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): make every block in the column share one width

Reported on the workspace step: the name field ended short of the
description above it. Measured rather than eyeballed -- the heading,
description and footer all render at 448px, the form at 384px, because the
FieldGroup still carried STEP_MEASURE.

STEP_MEASURE made sense against the old 920px frame, where a full-width
input would have been absurd. Against a 28rem column it does nothing but
misalign, so both remaining uses are gone -- the workspace form and the
runtime picker -- and the constant with them. A single measure that no step
can locally narrow is the whole point of the column; leaving the knob
exported invites the same drift back.

Two stale measures went with it. The runtime phase views still capped their
ledes at max-w-[620px], inert inside a 448px column, and sized them
text-body-lg against StepHeading's text-body; their h2 was text-title-lg,
the same size as the h1 above it. Both now match the shared scale.

Guarded in e2e rather than a unit test. The shell test renders a stub child,
so asserting "no narrow cap inside the column" there would pass whatever the
real steps do. The new spec walks all three steps and compares rendered
geometry -- it fails on the reported bug and passes after the fix, which is
what makes it worth keeping.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): stop the whole window re-fading on every step change

Every step rendered its own <StepShell>. Because each step is a different
component type, React tore the shell down and built a new one on each
transition: the "persistent" rail remounted, its canvas restarted, and the
shell replayed `animate-onboarding-enter` -- a 0.4s fade from opacity 0
across the entire window. That full-window re-fade is the flash.

Measured before changing anything: tagging the live <aside> and switching
steps showed the attribute gone, and the shell root still reporting
animation-name `onboarding-enter` after the switch.

The upstream block has no step-change motion at all -- no animate-*, no
transitions, no framer-motion, no AnimatePresence. Its sidebar and section
live in one component and only the step body swaps. This does the same: the
flow owns a single StepShell and the steps render content. The shell's
entrance fade now runs once, on entering onboarding, which is what it was
for.

`backDisabled` was the one thing blocking the hoist -- the shell needs it but
only the workspace step knows the create request is in flight. It reports
upward through `onBusyChange`, with an unmount cleanup: a successful create
advances immediately, so without clearing the flag the next step would open
with Back and the rail dead.

Guarded in e2e by tagging the rail and content nodes and asserting they
survive a step change, plus a single DotSphere canvas rather than one per
visited step.

Co-authored-by: multica-agent <github@multica.ai>

* chore: drop a stray commit-message file

Co-authored-by: multica-agent <github@multica.ai>

* i18n(zh): retranslate onboarding and fix cross-file inconsistencies

Reported as "生硬" on the onboarding Chinese. Reviewed all 217 onboarding
strings against the Chinese voice guide in conventions.mdx, then swept the
other 24 locale files for the same classes of problem.

Three were mistranslations, not stiffness. The worst: an agent runtime was
described as the AI coding tool "我们接管的" — take over — where the English
says "we connect to". Also a "推荐" that exists in no source string, and
"12 calls" (user interviews) rendered as "12 通电话".

Punctuation, decided from the repo's own zh docs rather than guessed:
「」 is forbidden by the guide and appeared 16 times; 破折号 spacing was
split 26 spaced vs the rest unspaced, and the docs run 152 unspaced to 35,
so unspaced wins. Four strings had a stray space inside Chinese text
("正在跳转到 工作区").

67 English strings had two or more Chinese translations. Most are
legitimate — weekday pickers use single characters where labels spell them
out, 飞书/Lark are deliberate regional variants, and "Name" is 姓名 for a
person and 名称 for an object. 42 were arbitrary and are now unified.

ja/ko got the same consistency pass on the clear-cut cases only, kept
parallel with the zh choices. Their punctuation was deliberately left alone:
「」 is standard Japanese quoting, so the zh rule does not transfer.

Edits are applied to the raw file text rather than through a JSON
round-trip, which reformatted compact one-line objects and turned ~20 real
edits into a 97-line diff.

Co-authored-by: multica-agent <github@multica.ai>

* chore: drop a stray commit-message file

Co-authored-by: multica-agent <github@multica.ai>

* feat(mika): use the unicorn emoji as Mika's placeholder avatar

Mika shipped with a hand-rolled data-URI SVG — a sparkle glyph on a dark
rounded square — which only that one constant knew how to produce. Agents
already have an emoji avatar convention (`emoji:` marker in avatar_url,
owned by agentEmojiAvatarPrefix on the server and parseAvatarEmoji on the
client), so this reuses it instead: ActorAvatar renders the emoji as text and
no surface needs to special-case her.

The two cards that stand in for Mika before the agent row exists move with
it — the onboarding intro card and the Runtimes "Start with Mika" card. Both
were drawing the same sparkle mark, and leaving them would mean a member sees
one face during onboarding and a different one the moment Mika is created.
They now share MIKA_PLACEHOLDER_EMOJI, which carries a pointer to the server
constant so the two cannot drift apart silently.

The dark square went with the sparkle: it was built to frame a white line-art
glyph, and an emoji on it reads badly. These use bg-muted, matching how
ActorAvatar already frames an emoji avatar.

Placeholder until Mika has real artwork.

Co-authored-by: multica-agent <github@multica.ai>

* i18n: stop the skip path promising things it does not do

Two claims in the runtime-skip copy that the code does not back, both found
while walking the flow end to end.

"Enter your workspace in read-only mode" — there is no workspace read-only
concept on the server. The only READ ONLY in handlers is Postgres transaction
isolation on the issue-table and search queries. What actually exists is
admission.go's ReasonAgentRuntimeRequired, which blocks dispatch when no
runtime is connected. So the sentence's second half was already true and
enforced; the first half promised a restriction that does not exist — a
member who skips can create issues, comment, edit fields and invite people
exactly as normal. Same over-claim in cloud_waitlist.intro_warning.

"We've added one task" — the skip path creates an *issue* (it lands on the
Issues board as NOR-1), and task/issue are distinct entities in this product.
Now says issue.

All four locales.

Co-authored-by: multica-agent <github@multica.ai>

* i18n: reframe Mika as your first agent teammate

Requested copy change on the step 3 headline.

The rail's description for that step moves with it. It is the step's own
subtitle and sits on the same screen as the headline, so leaving it saying
"Your Chief of Staff" would have put two different framings of Mika side by
side on one page.

Three "Chief of Staff" references are deliberately untouched, because
dropping the title everywhere is a positioning call rather than a copy fix:
the role chip on the intro card (name + title reads fine under the new
headline), and the web fork's lede, which is a different screen.

Chinese follows the glossary (Agent -> 智能体) rather than the mixed-language
phrasing in the request.

Co-authored-by: multica-agent <github@multica.ai>

* fix(runtimes): let the member pick the runtime before Mika is created

"Start with Mika" provisioned immediately on
`runtimes.find(online) ?? runtimes[0]`. One machine commonly registers every
agent CLI installed on it — nine on the box this was reported from — so "the
first online one" is an arbitrary pick, and Mika could end up bound to a CLI
the member never intended to run their Chief of Staff on. Rebinding after the
fact is more work than choosing up front.

The action now opens a dialog with the same two controls onboarding already
uses for this decision, RuntimePicker and ModelDropdown, so the same choice
reached from a different entry point is asked the same way. The old heuristic
survives only as the dialog's initial selection, which makes it visible and
changeable instead of silent.

Model resets when the runtime changes, because models are per-runtime and a
value picked for the previous one may not exist on the next.

Co-authored-by: multica-agent <github@multica.ai>

* refactor(runtimes): one component for "which runtime should Mika use"

Three surfaces asked this same question and had drifted apart: desktop
onboarding and the Runtimes page offered a runtime plus a model, while the
web CLI dialog offered only a runtime. `step-platform-fork` called
`onNext(picker.selected)` with no second argument, so connecting through the
web terminal path silently created Mika on whatever model the runtime
defaulted to, while the desktop step let you choose. Two of the three also
re-implemented "changing the runtime clears the model"; the third simply
lacked it.

MikaRuntimeChoice now owns the pair and that reset rule, so no caller can
forget it and the three entry points cannot drift again. The web CLI path
gains model selection, which is the behaviour change here.

`layout` is a prop rather than a single unified presentation because the
difference is real: the CLI dialog lists machines because that is the moment
they appear one at a time after `multica setup`, and a collapsed dropdown
hides exactly the feedback that dialog exists to give. Everything below the
list is identical.

compact-runtime-row moves from onboarding/ to runtimes/ so imports only flow
onboarding -> runtimes rather than both ways.

Creation is deliberately left alone. A and B still funnel through
`handleRuntimeNext`, which also runs saveQuestionnaire, completeOnboarding
and onComplete; the Runtimes page must not do any of that, since that member
is already onboarded. Merging those would leak onboarding completion into a
non-onboarding surface.

The platform-fork test now needs a QueryClientProvider, because the dialog
renders a model dropdown that queries the runtime's model list, and its
onNext assertion moves to the two-argument signature.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): stop the download card claiming a result it cannot know

Clicking "Use this computer" set `downloaded` unconditionally, which flipped
the card to "Opening the download page..." / "Opened in a new tab." Neither
is knowable: `window.open` is called with `noopener`, and per spec that
returns null whether the tab opened or a popup blocker ate it, so a blocked
click still produced a card asserting a tab had opened.

The same flag was also write-once, so the transient "Opening..." became a
terminal state — come back to the tab later and it still says the page is
opening. And because the swapped title wraps to two lines, the card grew
10px and pushed the two cards under it down, which is what made the click
read as a page refresh in the first report.

The card now states its intent up front — "Opens in a new tab — pick your
platform there" — which is true before the click, after it, and when the
popup never appears. The state, both `_after` strings and `hint_downloaded`
are gone from all four locales.

Its test asserted the flip, so it now asserts the opposite: the mocked
window.open returns null (the blocked case) and the card must be unchanged.
Measured after the change: primary card 330px before and after, and the
cards below it do not move.

Co-authored-by: multica-agent <github@multica.ai>

* fix(desktop): stop one worktree in a thousand booting into a blank window (#6436)

Worktree renderer ports are `5174 + cksum(path) % 1000`, a 5174-6173 window
that contains exactly one port Chromium refuses to navigate to: 6000, the X11
port on its restricted list. A worktree whose path hashes to offset 826 gets a
healthy Vite server on 6000 and an Electron window that fails the load with
ERR_UNSAFE_PORT -- so it reads as a renderer bug, not a port one, and the only
way out was setting DESKTOP_RENDERER_PORT by hand.

Restricted ports in the window are now remapped into the block immediately
above it (6000 -> 6174). Sending them past the end rather than shifting them by
one keeps the offset -> port mapping injective, so two worktrees still cannot
land on the same port and race for it.

Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): close the Mika multi-member and failure-recovery gaps

From Emacs's review of #6378. All seven findings reproduced.

The two blocking server bugs were the same shape: Mika is one agent per
workspace but sessions and ownership are per member.

- StartMikaOnboarding required the agent owner, so the member who lost
  the CreateMikaAgent race — the race that handler's advisory lock exists
  to survive — got a valid Mika, opened a valid session, and then a 403.
  Mika is created workspace-visible and workspace-invocable; the session
  gate and canInvokeAgent were already the checks that matter.
- The onboarding session was resolved client-side by listing sessions and
  creating one on a miss, matched on the localized title.
  LockWorkspaceForChatSessionCreate is FOR KEY SHARE precisely so
  concurrent creators do not block, so two tabs each opened their own
  conversation with its own kickoff, and switching language between a
  failed attempt and its retry opened another. It is now get-or-create
  server-side under a per-(workspace, member) advisory lock, keyed on
  (workspace, creator, agent), returned alongside the agent.

Also:

- The skipped-runtime welcome dismissed itself silently when provisioning
  the guide issue failed. The signal is not persisted and onboarding is
  already complete, so a blip was terminal. It now offers a retry.
- The Runtimes recovery card gated on `agents.length === 0`, so creating
  any ordinary agent hid the only surface that can mint a Mika — the
  generic endpoint accepts no system_key. Gated on Mika's absence.
- CompactRuntimeRow ignored `disabled`; the CLI dialog was already
  passing it, so the runtime could change mid-submit. It is a real
  <button> now, which also gets focus and Enter/Space for free.
- The rail never went below 15rem while the content pane kept its gutter,
  leaving ~87px of form at 375px. It is hidden under md, where a compact
  bar carries the step name and the Back button instead.
- Dropped a stray __pycache__ artifact I had committed by accident.

Each new test was checked against the bug it covers: reinstating the
owner gate, the title-keyed lookup, or the dropped disabled prop makes
the corresponding test fail.

Co-authored-by: multica-agent <github@multica.ai>

* fix(onboarding): make the Mika entrypoint survive a partial bootstrap

From Emacs's second review of #6378.

Bootstrapping Mika is three server steps — provision the agent, open the
member's session, enqueue the opening turn — and the last two can fail
after the agent commits. Two things then conspired: the server reported
success with the session omitted, and the Runtimes card gated on the
agent existing. The agent's own `agent:created` broadcast invalidates the
agent list, so the card (and its open dialog) was torn down the instant
step one succeeded, and never came back on reload — the agent is durable,
the rest was not. The member was left holding a Mika they could not start.

- The endpoint now fails when the session cannot be resolved. Every step
  is idempotent, so a retry converges; handing back a half-built flow the
  caller cannot distinguish from a finished one does not.
- The entrypoint is gated on the member's own state — does this member
  have a Mika conversation that was actually kicked off — rather than on
  the workspace having an agent. That is the question the card answers,
  and it is true again for every partial state above.

Also restores the Log out escape hatch below `md`. Hiding the rail last
round took its footer with it, which stranded every step but Welcome with
no way out on a narrow screen; the compact bar now renders the same slot,
so `sidebarFooter` is `chromeFooter`.

Each new test was checked against its bug: the old agent-only gate fails
three of the memberNeedsMikaSetup cases, and dropping the footer prop
fails the chrome test.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: Lambda <lambda@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-08-06 14:21:02 +08:00

1377 lines
51 KiB
Go

package handler
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/multica-ai/multica/server/internal/service"
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
func TestAgentBuilderInstructionsConstrainModelsToRuntimeCatalog(t *testing.T) {
for _, requirement := range []string{
"AVAILABLE RUNTIME MODELS",
"Never use a model label as the id",
"never invent a model id",
} {
if !strings.Contains(agentBuilderInstructions, requirement) {
t.Fatalf("agent builder instructions missing model constraint %q", requirement)
}
}
}
func TestCreateAgentBuilderSessionCreatesIsolatedHiddenBuilder(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `
DELETE FROM agent
WHERE workspace_id = $1 AND kind = 'system' AND system_key LIKE 'agent_builder:%'
`, testWorkspaceID)
})
create := func(model string) CreateAgentBuilderSessionResponse {
w := httptest.NewRecorder()
testHandler.CreateAgentBuilderSession(w, newRequest(http.MethodPost, "/api/agent-builder/sessions", map[string]any{
"runtime_id": testRuntimeID,
"model": model,
}))
if w.Code != http.StatusCreated {
t.Fatalf("CreateAgentBuilderSession: expected 201, got %d: %s", w.Code, w.Body.String())
}
var response CreateAgentBuilderSessionResponse
if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil {
t.Fatalf("decode response: %v", err)
}
if response.SessionID == "" || response.BuilderAgentID == "" {
t.Fatalf("missing builder identifiers: %+v", response)
}
return response
}
first := create("builder-model-a")
second := create("builder-model-b")
if first.BuilderAgentID == second.BuilderAgentID {
t.Fatalf("builder sessions unexpectedly shared an agent: %s", first.BuilderAgentID)
}
if first.SessionID == second.SessionID {
t.Fatalf("each creation flow must receive a fresh chat session")
}
var kind, systemKey, firstModel string
if err := testPool.QueryRow(context.Background(), `
SELECT kind, system_key, model FROM agent WHERE id = $1
`, first.BuilderAgentID).Scan(&kind, &systemKey, &firstModel); err != nil {
t.Fatalf("load builder agent: %v", err)
}
if kind != "system" || !strings.HasPrefix(systemKey, "agent_builder:") {
t.Fatalf("unexpected builder identity kind=%q system_key=%q", kind, systemKey)
}
if firstModel != "builder-model-a" {
t.Fatalf("first builder model was mutated: got %q", firstModel)
}
w := httptest.NewRecorder()
testHandler.ListAgents(w, newRequest(http.MethodGet, "/api/agents", nil))
if w.Code != http.StatusOK {
t.Fatalf("ListAgents: %d: %s", w.Code, w.Body.String())
}
var listed []AgentResponse
if err := json.Unmarshal(w.Body.Bytes(), &listed); err != nil {
t.Fatalf("decode agent list: %v", err)
}
for _, agent := range listed {
if agent.ID == first.BuilderAgentID {
t.Fatalf("system builder leaked into the user-facing agent list")
}
}
// Knowing the ID must not expose system infrastructure through the public
// Agent detail/update/archive loaders.
w = httptest.NewRecorder()
req := withURLParams(newRequest(http.MethodGet, "/api/agents/"+first.BuilderAgentID, nil), "id", first.BuilderAgentID)
testHandler.GetAgent(w, req)
if w.Code != http.StatusNotFound {
t.Fatalf("GetAgent(system): expected 404, got %d: %s", w.Code, w.Body.String())
}
// Deleting the private Builder chat also removes its session-scoped hidden
// Agent, so completed/cancelled flows do not accumulate infrastructure rows.
w = httptest.NewRecorder()
req = withURLParams(newRequest(http.MethodDelete, "/api/chat/sessions/"+first.SessionID, nil), "sessionId", first.SessionID)
req = withChatTestWorkspaceCtx(t, req)
testHandler.DeleteChatSession(w, req)
if w.Code != http.StatusNoContent {
t.Fatalf("DeleteChatSession(builder): expected 204, got %d: %s", w.Code, w.Body.String())
}
var remaining int
if err := testPool.QueryRow(context.Background(), `SELECT count(*) FROM agent WHERE id = $1`, first.BuilderAgentID).Scan(&remaining); err != nil {
t.Fatalf("count deleted builder: %v", err)
}
if remaining != 0 {
t.Fatalf("builder agent survived chat deletion")
}
}
func TestCreateAgentAttachesSkillsWithoutCreatingAWelcomeChat(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
var skillID string
if err := testPool.QueryRow(ctx, `
INSERT INTO skill (workspace_id, name, description, content, config, created_by)
VALUES ($1, 'Atomic Create Skill', '', '# Atomic', '{}'::jsonb, $2)
RETURNING id
`, testWorkspaceID, testUserID).Scan(&skillID); err != nil {
t.Fatalf("create skill fixture: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent WHERE workspace_id = $1 AND name = 'Atomic Skill Agent'`, testWorkspaceID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM skill WHERE id = $1`, skillID)
})
w := httptest.NewRecorder()
testHandler.CreateAgent(w, newRequest(http.MethodPost, "/api/agents", map[string]any{
"name": "Atomic Skill Agent",
"runtime_id": testRuntimeID,
"skill_ids": []string{skillID},
}))
if w.Code != http.StatusCreated {
t.Fatalf("CreateAgent: expected 201, got %d: %s", w.Code, w.Body.String())
}
var response AgentResponse
if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil {
t.Fatalf("decode response: %v", err)
}
if len(response.Skills) != 1 || response.Skills[0].ID != skillID {
t.Fatalf("create response did not include attached skill: %+v", response.Skills)
}
var chatSessions int
if err := testPool.QueryRow(ctx, `
SELECT count(*) FROM chat_session WHERE agent_id = $1
`, response.ID).Scan(&chatSessions); err != nil {
t.Fatalf("count chat sessions: %v", err)
}
if chatSessions != 0 {
t.Fatalf("chat sessions after agent create = %d, want 0", chatSessions)
}
}
// newBuilderSession starts a builder conversation on testRuntimeID and registers
// cleanup for the carrier agents the flow creates.
func newBuilderSession(t *testing.T) CreateAgentBuilderSessionResponse {
t.Helper()
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `
DELETE FROM agent
WHERE workspace_id = $1 AND kind = 'system' AND system_key LIKE 'agent_builder:%'
`, testWorkspaceID)
})
w := httptest.NewRecorder()
testHandler.CreateAgentBuilderSession(w, newRequest(http.MethodPost, "/api/agent-builder/sessions", map[string]any{
"runtime_id": testRuntimeID,
"model": "model-pinned-to-runtime-a",
}))
if w.Code != http.StatusCreated {
t.Fatalf("CreateAgentBuilderSession: expected 201, got %d: %s", w.Code, w.Body.String())
}
var session CreateAgentBuilderSessionResponse
if err := json.Unmarshal(w.Body.Bytes(), &session); err != nil {
t.Fatalf("decode create response: %v", err)
}
return session
}
// newTestRuntime inserts an extra runtime in the fixture workspace so switch
// tests have somewhere to move to.
func newTestRuntime(t *testing.T, name, status string) string {
t.Helper()
var runtimeID string
if err := testPool.QueryRow(context.Background(), `
INSERT INTO agent_runtime (
workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, owner_id, last_seen_at
)
VALUES ($1, NULL, $2, 'cloud', $3, $4, 'switch test runtime', '{}'::jsonb, $5, now())
RETURNING id
`, testWorkspaceID, name, strings.ToLower(strings.ReplaceAll(name, " ", "_")), status, testUserID).Scan(&runtimeID); err != nil {
t.Fatalf("create runtime %q: %v", name, err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, runtimeID)
})
return runtimeID
}
func switchBuilderRuntime(t *testing.T, sessionID, runtimeID string) *httptest.ResponseRecorder {
t.Helper()
w := httptest.NewRecorder()
req := withURLParams(
newRequest(http.MethodPatch, "/api/agent-builder/sessions/"+sessionID+"/runtime", map[string]any{
"runtime_id": runtimeID,
}),
"sessionId", sessionID,
)
testHandler.SwitchAgentBuilderRuntime(w, req)
return w
}
func TestSwitchAgentBuilderRuntimeRebindsCarrier(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
session := newBuilderSession(t)
target := newTestRuntime(t, "Builder Switch Target", "online")
w := switchBuilderRuntime(t, session.SessionID, target)
if w.Code != http.StatusOK {
t.Fatalf("SwitchAgentBuilderRuntime: expected 200, got %d: %s", w.Code, w.Body.String())
}
var response SwitchAgentBuilderRuntimeResponse
if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil {
t.Fatalf("decode switch response: %v", err)
}
if response.RuntimeID != target {
t.Fatalf("response runtime = %q, want %q", response.RuntimeID, target)
}
// The carrier is what stamps a chat task's runtime, so this row — not the
// client's local selection — is the fix.
var boundRuntimeID, boundRuntimeMode string
var boundModel pgtype.Text
if err := testPool.QueryRow(ctx, `
SELECT runtime_id::text, runtime_mode, model FROM agent WHERE id = $1
`, session.BuilderAgentID).Scan(&boundRuntimeID, &boundRuntimeMode, &boundModel); err != nil {
t.Fatalf("load builder carrier: %v", err)
}
if boundRuntimeID != target {
t.Fatalf("carrier runtime = %q, want %q", boundRuntimeID, target)
}
if boundRuntimeMode != "cloud" {
t.Fatalf("carrier runtime_mode = %q, want cloud", boundRuntimeMode)
}
if boundModel.Valid {
t.Fatalf("carrier model should be cleared on rebind, got %q", boundModel.String)
}
// Left deliberately stale: the daemon only resumes a stored provider session
// when this pointer matches the claiming task's runtime, so keeping the old
// value is what makes the new runtime start a fresh session.
var sessionRuntimeID string
if err := testPool.QueryRow(ctx, `
SELECT runtime_id::text FROM chat_session WHERE id = $1
`, session.SessionID).Scan(&sessionRuntimeID); err != nil {
t.Fatalf("load chat session: %v", err)
}
if sessionRuntimeID != testRuntimeID {
t.Fatalf("chat_session.runtime_id = %q, want the original %q", sessionRuntimeID, testRuntimeID)
}
}
func TestSwitchAgentBuilderRuntimeRejectsOfflineTarget(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
offline := newTestRuntime(t, "Builder Switch Offline", "offline")
if w := switchBuilderRuntime(t, session.SessionID, offline); w.Code != http.StatusConflict {
t.Fatalf("offline target: expected 409, got %d: %s", w.Code, w.Body.String())
}
}
func TestSwitchAgentBuilderRuntimeRejectsWhileReplyPending(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
session := newBuilderSession(t)
target := newTestRuntime(t, "Builder Switch Pending Target", "online")
if _, err := testPool.Exec(ctx, `
INSERT INTO agent_task_queue (agent_id, chat_session_id, status, priority, context, runtime_id)
VALUES ($1, $2, 'running', 2, '{}'::jsonb, $3)
`, session.BuilderAgentID, session.SessionID, testRuntimeID); err != nil {
t.Fatalf("insert pending task: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE chat_session_id = $1`, session.SessionID)
})
if w := switchBuilderRuntime(t, session.SessionID, target); w.Code != http.StatusConflict {
t.Fatalf("pending reply: expected 409, got %d: %s", w.Code, w.Body.String())
}
}
func TestSwitchAgentBuilderRuntimeRejectsNonBuilderSession(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
target := newTestRuntime(t, "Builder Switch Foreign Target", "online")
// A user-authored agent changes runtime through the agent update path; this
// endpoint must not be a second way in.
var userAgentID, userSessionID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent (
workspace_id, name, description, runtime_mode, runtime_config,
runtime_id, visibility, permission_mode, max_concurrent_tasks, owner_id
)
VALUES ($1, 'Builder Switch User Agent', '', 'cloud', '{}'::jsonb, $2, 'workspace', 'public_to', 1, $3)
RETURNING id
`, testWorkspaceID, testRuntimeID, testUserID).Scan(&userAgentID); err != nil {
t.Fatalf("create user agent: %v", err)
}
if err := testPool.QueryRow(ctx, `
INSERT INTO chat_session (workspace_id, agent_id, creator_id, title, runtime_id)
VALUES ($1, $2, $3, 'Not a builder', $4)
RETURNING id
`, testWorkspaceID, userAgentID, testUserID, testRuntimeID).Scan(&userSessionID); err != nil {
t.Fatalf("create user chat session: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, userSessionID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, userAgentID)
})
if w := switchBuilderRuntime(t, userSessionID, target); w.Code != http.StatusNotFound {
t.Fatalf("user agent session: expected 404, got %d: %s", w.Code, w.Body.String())
}
var stillBound string
if err := testPool.QueryRow(ctx, `SELECT runtime_id::text FROM agent WHERE id = $1`, userAgentID).Scan(&stillBound); err != nil {
t.Fatalf("reload user agent: %v", err)
}
if stillBound != testRuntimeID {
t.Fatalf("user agent runtime changed to %q; the builder endpoint must never touch it", stillBound)
}
}
func listBuilderSessions(t *testing.T) ListAgentBuilderSessionsResponse {
t.Helper()
w := httptest.NewRecorder()
testHandler.ListAgentBuilderSessions(w, newRequest(http.MethodGet, "/api/agent-builder/sessions", nil))
if w.Code != http.StatusOK {
t.Fatalf("ListAgentBuilderSessions: expected 200, got %d: %s", w.Code, w.Body.String())
}
var response ListAgentBuilderSessionsResponse
if err := json.Unmarshal(w.Body.Bytes(), &response); err != nil {
t.Fatalf("decode list response: %v", err)
}
return response
}
func insertChatMessage(t *testing.T, sessionID, role, content string) {
t.Helper()
if _, err := testPool.Exec(context.Background(), `
INSERT INTO chat_message (chat_session_id, role, content)
VALUES ($1, $2, $3)
`, sessionID, role, content); err != nil {
t.Fatalf("insert chat message: %v", err)
}
}
// The studio stopped deleting builder sessions on navigation, so this list is
// the only route back to one. It must carry the carrier's runtime: the client
// seeds its picker from it, and a wrong answer reintroduces MUL-5163 (picker
// shows A while every message runs on B).
func TestListAgentBuilderSessionsReturnsCarrierRuntime(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
insertChatMessage(t, session.SessionID, "user", "Create a release manager")
response := listBuilderSessions(t)
var found *AgentBuilderSessionSummary
for i := range response.Sessions {
if response.Sessions[i].SessionID == session.SessionID {
found = &response.Sessions[i]
break
}
}
if found == nil {
t.Fatalf("builder session %s missing from the list", session.SessionID)
}
if found.RuntimeID != testRuntimeID {
t.Fatalf("runtime_id = %q, want the carrier's runtime %q", found.RuntimeID, testRuntimeID)
}
if found.LastMessageContent != "Create a release manager" {
t.Fatalf("last_message_content = %q", found.LastMessageContent)
}
if found.LastMessageRole != "user" {
t.Fatalf("last_message_role = %q, want user", found.LastMessageRole)
}
}
func saveBuilderDraft(t *testing.T, sessionID string, draft any) *httptest.ResponseRecorder {
t.Helper()
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(),
`DELETE FROM agent_builder_draft WHERE chat_session_id = $1`, sessionID)
})
w := httptest.NewRecorder()
req := withURLParams(
newRequest(http.MethodPut, "/api/agent-builder/sessions/"+sessionID+"/draft",
map[string]any{"draft": draft}),
"sessionId", sessionID,
)
testHandler.SaveAgentBuilderDraft(w, req)
return w
}
// The half of the configuration the user types by hand had no durable home
// before this: it lived in React state and died on every reload. The list is
// where it comes back, so the two have to be tested together.
func TestSaveAgentBuilderDraftRoundTripsThroughTheList(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
insertChatMessage(t, session.SessionID, "user", "Create a release manager")
if w := saveBuilderDraft(t, session.SessionID, map[string]any{
"name": "Release manager",
"instructions": "Ship carefully",
}); w.Code != http.StatusNoContent {
t.Fatalf("SaveAgentBuilderDraft: expected 204, got %d: %s", w.Code, w.Body.String())
}
for _, summary := range listBuilderSessions(t).Sessions {
if summary.SessionID != session.SessionID {
continue
}
var stored map[string]any
if err := json.Unmarshal(summary.Draft, &stored); err != nil {
t.Fatalf("decode stored draft %q: %v", string(summary.Draft), err)
}
if stored["name"] != "Release manager" {
t.Fatalf("stored draft name = %v", stored["name"])
}
return
}
t.Fatalf("builder session %s missing from the list", session.SessionID)
}
// One conversation has one current configuration. Accumulating rows per save
// would make "which one is current" a question the read path has to answer.
func TestSaveAgentBuilderDraftOverwritesInPlace(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
session := newBuilderSession(t)
saveBuilderDraft(t, session.SessionID, map[string]any{"name": "First"})
saveBuilderDraft(t, session.SessionID, map[string]any{"name": "Second"})
var rows int
var name string
if err := testPool.QueryRow(ctx, `
SELECT count(*), max(draft->>'name') FROM agent_builder_draft WHERE chat_session_id = $1
`, session.SessionID).Scan(&rows, &name); err != nil {
t.Fatalf("read stored drafts: %v", err)
}
if rows != 1 {
t.Fatalf("stored draft rows = %d, want exactly one per conversation", rows)
}
if name != "Second" {
t.Fatalf("stored draft name = %q, want the latest save", name)
}
}
// Without the carrier gate this endpoint would be a way to hang arbitrary JSON
// off any chat session the caller happens to own.
func TestSaveAgentBuilderDraftRejectsNonBuilderSession(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
var userAgentID, userSessionID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent (
workspace_id, name, description, runtime_mode, runtime_config,
runtime_id, visibility, permission_mode, max_concurrent_tasks, owner_id
)
VALUES ($1, 'Builder Draft User Agent', '', 'cloud', '{}'::jsonb, $2, 'workspace', 'public_to', 1, $3)
RETURNING id
`, testWorkspaceID, testRuntimeID, testUserID).Scan(&userAgentID); err != nil {
t.Fatalf("create user agent: %v", err)
}
if err := testPool.QueryRow(ctx, `
INSERT INTO chat_session (workspace_id, agent_id, creator_id, title, runtime_id)
VALUES ($1, $2, $3, 'Ordinary chat', $4)
RETURNING id
`, testWorkspaceID, userAgentID, testUserID, testRuntimeID).Scan(&userSessionID); err != nil {
t.Fatalf("create user chat session: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, userSessionID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, userAgentID)
})
if w := saveBuilderDraft(t, userSessionID, map[string]any{"name": "X"}); w.Code != http.StatusNotFound {
t.Fatalf("ordinary chat: expected 404, got %d: %s", w.Code, w.Body.String())
}
}
// agent_builder_draft carries no chat_session FK, so nothing reclaims the row
// on its own. A draft outliving its conversation is a leak the user can never
// see, let alone clear.
func TestDeleteChatSessionPrunesTheBuilderDraft(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
session := newBuilderSession(t)
saveBuilderDraft(t, session.SessionID, map[string]any{"name": "Doomed"})
w := httptest.NewRecorder()
req := withURLParams(
newRequest(http.MethodDelete, "/api/chat/sessions/"+session.SessionID, nil),
"sessionId", session.SessionID,
)
// DeleteChatSession reads the workspace off the context the middleware
// normally fills, not the header.
req = withChatTestWorkspaceCtx(t, req)
testHandler.DeleteChatSession(w, req)
if w.Code != http.StatusNoContent {
t.Fatalf("DeleteChatSession: expected 204, got %d: %s", w.Code, w.Body.String())
}
var remaining int
if err := testPool.QueryRow(ctx, `
SELECT count(*) FROM agent_builder_draft WHERE chat_session_id = $1
`, session.SessionID).Scan(&remaining); err != nil {
t.Fatalf("count stored drafts: %v", err)
}
if remaining != 0 {
t.Fatalf("stored draft rows after delete = %d, want 0", remaining)
}
}
// newSaveDraftRequest prepares a save the way saveBuilderDraft does but hands
// the pieces back, so the two tests below can run the handler on their own
// goroutine — both need a save in flight while another transaction holds the
// session row.
func newSaveDraftRequest(t *testing.T, sessionID string, draft any) (*httptest.ResponseRecorder, *http.Request) {
t.Helper()
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(),
`DELETE FROM agent_builder_draft WHERE chat_session_id = $1`, sessionID)
})
return httptest.NewRecorder(), withURLParams(
newRequest(http.MethodPut, "/api/agent-builder/sessions/"+sessionID+"/draft",
map[string]any{"draft": draft}),
"sessionId", sessionID,
)
}
func countBuilderDrafts(t *testing.T, sessionID string) int {
t.Helper()
var n int
if err := testPool.QueryRow(context.Background(), `
SELECT count(*) FROM agent_builder_draft WHERE chat_session_id = $1
`, sessionID).Scan(&n); err != nil {
t.Fatalf("count stored drafts: %v", err)
}
return n
}
// runSaveDraftAgainst holds the session row in a transaction, runs `hold`
// inside it, then starts a save and proves the save is waiting rather than
// racing. Returns once the save has finished, after the transaction commits.
//
// Asserting that the save blocks is the point: before the fix it read the
// session, sailed past the uncommitted transaction and wrote its row, so the
// outcome depended on which statement won rather than on the lock.
func runSaveDraftAgainst(t *testing.T, sessionID string, w *httptest.ResponseRecorder, req *http.Request, hold string) {
t.Helper()
ctx := context.Background()
tx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin holder transaction: %v", err)
}
defer tx.Rollback(ctx)
// The same row and lock mode DeleteChatSession and SetChatSessionArchived
// take, as their transaction's first statement.
if _, err := tx.Exec(ctx, `SELECT id FROM chat_session WHERE id = $1 FOR UPDATE`, sessionID); err != nil {
t.Fatalf("lock chat session: %v", err)
}
if _, err := tx.Exec(ctx, hold, sessionID); err != nil {
t.Fatalf("hold statement %q: %v", hold, err)
}
done := make(chan struct{})
go func() {
defer close(done)
testHandler.SaveAgentBuilderDraft(w, req)
}()
select {
case <-done:
t.Fatalf("save finished while the session row was held: %d %s", w.Code, w.Body.String())
case <-time.After(500 * time.Millisecond):
}
if err := tx.Commit(ctx); err != nil {
t.Fatalf("commit holder transaction: %v", err)
}
select {
case <-done:
case <-time.After(10 * time.Second):
t.Fatalf("save never returned after the holder committed")
}
}
// The client autosaves on a debounce, so a save is routinely in flight when the
// user confirms "Discard this draft" — and because the conversation is
// addressable by URL, a second tab can autosave at any moment while this one
// discards. Unlocked, the save's read and its write straddled the delete: the
// upsert landed after the conversation was gone, and agent_builder_draft has no
// chat_session FK to reject it. What survived was the configuration the user had
// just explicitly discarded, invisible to the UI and reachable by no prune but
// the workspace teardown.
func TestSaveAgentBuilderDraftLosesToAConcurrentDelete(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
// Deliberately no draft saved first. A pre-existing row would make the
// upsert contend on that row's own lock, and the test would pass for a
// reason that has nothing to do with the session lock under test.
w, req := newSaveDraftRequest(t, session.SessionID, map[string]any{"name": "Discarded"})
runSaveDraftAgainst(t, session.SessionID, w, req,
`DELETE FROM chat_session WHERE id = $1`)
if w.Code != http.StatusNotFound {
t.Fatalf("save after the conversation was deleted: expected 404, got %d: %s", w.Code, w.Body.String())
}
if n := countBuilderDrafts(t, session.SessionID); n != 0 {
t.Fatalf("orphaned draft rows = %d, want 0", n)
}
}
// Same race against the other writer of chat_session.status. Creating the agent
// archives the conversation, and the studio's last autosave can still be in
// flight — an unlocked status check let it write a draft onto a session that is
// already read-only, where the drafts list will never show it again.
func TestSaveAgentBuilderDraftLosesToAConcurrentArchive(t *testing.T) {
if testHandler == nil || testPool == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
w, req := newSaveDraftRequest(t, session.SessionID, map[string]any{"name": "Too late"})
runSaveDraftAgainst(t, session.SessionID, w, req,
`UPDATE chat_session SET status = 'archived' WHERE id = $1`)
if w.Code != http.StatusBadRequest {
t.Fatalf("save after the conversation was archived: expected 400, got %d: %s", w.Code, w.Body.String())
}
if n := countBuilderDrafts(t, session.SessionID); n != 0 {
t.Fatalf("draft rows on an archived conversation = %d, want 0", n)
}
}
// A rebind moves the carrier without touching chat_session.runtime_id, which is
// deliberately left stale as the daemon's resume pointer. Reading the list from
// that column instead of the carrier would resume the conversation onto a
// runtime that no longer executes anything.
func TestListAgentBuilderSessionsFollowsARuntimeSwitch(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
insertChatMessage(t, session.SessionID, "user", "Create a release manager")
target := newTestRuntime(t, "Builder List Switch Target", "online")
if w := switchBuilderRuntime(t, session.SessionID, target); w.Code != http.StatusOK {
t.Fatalf("SwitchAgentBuilderRuntime: expected 200, got %d: %s", w.Code, w.Body.String())
}
for _, summary := range listBuilderSessions(t).Sessions {
if summary.SessionID != session.SessionID {
continue
}
if summary.RuntimeID != target {
t.Fatalf("runtime_id = %q after switch, want %q", summary.RuntimeID, target)
}
return
}
t.Fatalf("builder session %s missing from the list", session.SessionID)
}
// A session opened and abandoned untouched is not a draft. Listing it would put
// an empty row in front of the user on every accidental entry into the flow.
func TestListAgentBuilderSessionsSkipsUntouchedConversations(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
for _, summary := range listBuilderSessions(t).Sessions {
if summary.SessionID == session.SessionID {
t.Fatalf("untouched builder session %s must not be listed", session.SessionID)
}
}
}
// The configuration form is editable from the moment the session exists and
// autosaves, so a user can open the builder, type a name and leave before the
// first turn. Keying "is this a draft" on messages alone made that work
// unreachable: the row existed and nothing could reach it.
func TestListAgentBuilderSessionsIncludesASavedDraftWithNoMessages(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
session := newBuilderSession(t)
saveBuilderDraft(t, session.SessionID, map[string]any{"name": "Typed but never sent"})
for _, summary := range listBuilderSessions(t).Sessions {
if summary.SessionID != session.SessionID {
continue
}
if summary.LastMessageContent != "" || summary.LastMessageRole != "" {
t.Fatalf("expected an empty last message, got %q/%q",
summary.LastMessageRole, summary.LastMessageContent)
}
if summary.RuntimeID != testRuntimeID {
t.Fatalf("runtime_id = %q, want the carrier's runtime %q", summary.RuntimeID, testRuntimeID)
}
return
}
t.Fatalf("builder session %s with a saved draft is missing from the list", session.SessionID)
}
// Ordinary chats must never leak into the creation-draft list: their carrier is
// a user agent, and the studio would try to resume a conversation it cannot
// drive.
func TestListAgentBuilderSessionsExcludesOrdinaryChats(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
var userAgentID, userSessionID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent (
workspace_id, name, description, runtime_mode, runtime_config,
runtime_id, visibility, permission_mode, max_concurrent_tasks, owner_id
)
VALUES ($1, 'Builder List User Agent', '', 'cloud', '{}'::jsonb, $2, 'workspace', 'public_to', 1, $3)
RETURNING id
`, testWorkspaceID, testRuntimeID, testUserID).Scan(&userAgentID); err != nil {
t.Fatalf("create user agent: %v", err)
}
if err := testPool.QueryRow(ctx, `
INSERT INTO chat_session (workspace_id, agent_id, creator_id, title, runtime_id)
VALUES ($1, $2, $3, 'Ordinary chat', $4)
RETURNING id
`, testWorkspaceID, userAgentID, testUserID, testRuntimeID).Scan(&userSessionID); err != nil {
t.Fatalf("create user chat session: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM chat_message WHERE chat_session_id = $1`, userSessionID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, userSessionID)
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent WHERE id = $1`, userAgentID)
})
insertChatMessage(t, userSessionID, "user", "Hello")
for _, summary := range listBuilderSessions(t).Sessions {
if summary.SessionID == userSessionID {
t.Fatalf("ordinary chat %s must not appear as a creation draft", userSessionID)
}
}
}
// The regression this whole change exists for: a send that loaded the agent
// before a rebind committed must still enqueue on the runtime the session is
// bound to NOW. SendDirectChatMessage is handed the stale agent on purpose here
// — that is exactly what its caller does when the two requests interleave.
func TestSendDirectChatMessageUsesCurrentlyBoundRuntime(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
created := newBuilderSession(t)
target := newTestRuntime(t, "Builder Send Rebind Target", "online")
sessionUUID := parseUUID(created.SessionID)
session, err := testHandler.Queries.GetChatSession(ctx, sessionUUID)
if err != nil {
t.Fatalf("load chat session: %v", err)
}
staleAgent, err := testHandler.Queries.GetAgent(ctx, parseUUID(created.BuilderAgentID))
if err != nil {
t.Fatalf("load builder carrier: %v", err)
}
if uuidToString(staleAgent.RuntimeID) != testRuntimeID {
t.Fatalf("carrier should start on %q, got %q", testRuntimeID, uuidToString(staleAgent.RuntimeID))
}
if w := switchBuilderRuntime(t, created.SessionID, target); w.Code != http.StatusOK {
t.Fatalf("SwitchAgentBuilderRuntime: expected 200, got %d: %s", w.Code, w.Body.String())
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE chat_session_id = $1`, created.SessionID)
})
sent, err := testHandler.TaskService.SendDirectChatMessage(
ctx, session, staleAgent, parseUUID(testUserID), "hello after the switch", nil, "member", parseUUID(testUserID),
)
if err != nil {
t.Fatalf("SendDirectChatMessage: %v", err)
}
if got := uuidToString(sent.Task.RuntimeID); got != target {
t.Fatalf("task runtime = %q, want the rebound runtime %q — a stale in-flight send must not resurrect the old runtime", got, target)
}
}
// holderBackendPID returns the server-side PID of the backend serving tx, so a
// waiter can later be attributed to this specific lock holder.
func holderBackendPID(t *testing.T, ctx context.Context, tx pgx.Tx) int {
t.Helper()
var pid int
if err := tx.QueryRow(ctx, `SELECT pg_backend_pid()`).Scan(&pid); err != nil {
t.Fatalf("read holder backend pid: %v", err)
}
return pid
}
// waitForWaiterBlockedBy blocks until some backend is waiting on a lock held by
// holderPID, which is how the interleaving tests below observe "the other side
// is parked on our chat_session row" without guessing at timings.
//
// pg_blocking_pids is what makes this specific. `go test ./...` runs package
// test binaries in parallel against one DATABASE_URL, so a probe for "any
// Lock-waiting backend" could match an unrelated package and let the holder
// commit early — after which the path under test would start clean, read the
// committed state, and pass even with its lock removed. Attributing the waiter
// to this transaction's PID removes that false-green path.
//
// Returns false only after the deadline with no attributable waiter, which is
// the signal that the path under test never took the lock. A probe error is
// fatal rather than swallowed: a permissions or connectivity failure must not
// be reported as "that path is not locking".
func waitForWaiterBlockedBy(t *testing.T, holderPID int, timeout time.Duration) bool {
t.Helper()
deadline := time.Now().Add(timeout)
for {
var waiting int
if err := testPool.QueryRow(context.Background(), `
SELECT count(*) FROM pg_stat_activity
WHERE datname = current_database()
AND state = 'active'
AND wait_event_type = 'Lock'
AND $1::int = ANY(pg_blocking_pids(pid))
`, holderPID).Scan(&waiting); err != nil {
t.Fatalf("probe pg_stat_activity for waiters blocked by pid %d: %v", holderPID, err)
}
if waiting > 0 {
return true
}
if time.Now().After(deadline) {
return false
}
time.Sleep(20 * time.Millisecond)
}
}
// A rebind that has not committed yet must hold off a concurrent send, and the
// send must then observe the NEW runtime. This is the half of the protocol the
// lock owns: with the lock removed the send reads straight through the
// uncommitted rebind under READ COMMITTED, sees the pre-switch runtime, and
// enqueues the reply on the runtime the user just switched away from.
func TestSendDirectChatMessageWaitsForUncommittedRebind(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
created := newBuilderSession(t)
target := newTestRuntime(t, "Builder Interleave Send Target", "online")
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE chat_session_id = $1`, created.SessionID)
})
session, err := testHandler.Queries.GetChatSession(ctx, parseUUID(created.SessionID))
if err != nil {
t.Fatalf("load chat session: %v", err)
}
// The agent as the send handler would have loaded it: still on runtime A.
staleAgent, err := testHandler.Queries.GetAgent(ctx, parseUUID(created.BuilderAgentID))
if err != nil {
t.Fatalf("load builder carrier: %v", err)
}
// Hold an uncommitted rebind, exactly as SwitchAgentBuilderRuntime does.
tx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin rebind tx: %v", err)
}
defer tx.Rollback(context.Background())
holderPID := holderBackendPID(t, ctx, tx)
qtx := testHandler.Queries.WithTx(tx)
if _, err := qtx.LockChatSessionForRuntimeBind(ctx, session.ID); err != nil {
t.Fatalf("lock chat session: %v", err)
}
targetRuntime, err := testHandler.Queries.GetAgentRuntimeForWorkspace(ctx, db.GetAgentRuntimeForWorkspaceParams{
ID: parseUUID(target),
WorkspaceID: parseUUID(testWorkspaceID),
})
if err != nil {
t.Fatalf("load target runtime: %v", err)
}
if _, err := qtx.RebindAgentBuilderRuntime(ctx, db.RebindAgentBuilderRuntimeParams{
ID: staleAgent.ID,
RuntimeID: targetRuntime.ID,
RuntimeMode: targetRuntime.RuntimeMode,
}); err != nil {
t.Fatalf("rebind carrier: %v", err)
}
type sendResult struct {
task db.AgentTaskQueue
err error
}
results := make(chan sendResult, 1)
go func() {
sent, err := testHandler.TaskService.SendDirectChatMessage(
context.Background(), session, staleAgent, parseUUID(testUserID),
"sent while the rebind was still open", nil, "member", parseUUID(testUserID),
)
if err != nil {
results <- sendResult{err: err}
return
}
results <- sendResult{task: sent.Task}
}()
if !waitForWaiterBlockedBy(t, holderPID, 10*time.Second) {
select {
case got := <-results:
t.Fatalf("send completed (err=%v, runtime=%q) while an uncommitted rebind held the chat_session lock; the send path is not taking the lock",
got.err, uuidToString(got.task.RuntimeID))
default:
t.Fatalf("send never blocked on the chat_session lock held by pid %d; the send path is not taking the lock", holderPID)
}
}
select {
case got := <-results:
t.Fatalf("send returned (err=%v) before the rebind committed", got.err)
default:
}
if err := tx.Commit(ctx); err != nil {
t.Fatalf("commit rebind: %v", err)
}
select {
case got := <-results:
if got.err != nil {
t.Fatalf("SendDirectChatMessage after commit: %v", got.err)
}
if runtimeID := uuidToString(got.task.RuntimeID); runtimeID != target {
t.Fatalf("task runtime = %q, want the rebound runtime %q", runtimeID, target)
}
case <-time.After(10 * time.Second):
t.Fatal("send did not complete after the rebind committed")
}
}
func TestSendDirectChatMessageRejectsSessionArchivedWhileWaitingForLock(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
agentID := createHandlerTestAgent(t, "Chat Send Archive Race", []byte("[]"))
sessionID := createHandlerTestChatSession(t, agentID)
session, err := testHandler.Queries.GetChatSession(ctx, parseUUID(sessionID))
if err != nil {
t.Fatalf("load chat session: %v", err)
}
agent, err := testHandler.Queries.GetAgent(ctx, parseUUID(agentID))
if err != nil {
t.Fatalf("load chat agent: %v", err)
}
tx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin archive tx: %v", err)
}
defer tx.Rollback(context.Background())
holderPID := holderBackendPID(t, ctx, tx)
if _, err := tx.Exec(ctx, `
UPDATE chat_session SET status = 'archived' WHERE id = $1
`, sessionID); err != nil {
t.Fatalf("archive chat session: %v", err)
}
results := make(chan error, 1)
go func() {
_, err := testHandler.TaskService.SendDirectChatMessage(
context.Background(), session, agent, parseUUID(testUserID),
"must not enqueue after archive", nil, "member", parseUUID(testUserID),
)
results <- err
}()
if !waitForWaiterBlockedBy(t, holderPID, 10*time.Second) {
t.Fatal("send did not wait for the session archive transaction")
}
if err := tx.Commit(ctx); err != nil {
t.Fatalf("commit archive: %v", err)
}
select {
case err := <-results:
if !errors.Is(err, service.ErrChatSessionArchived) {
t.Fatalf("send after archive error = %v, want ErrChatSessionArchived", err)
}
case <-time.After(10 * time.Second):
t.Fatal("send did not return after archive committed")
}
var taskCount, messageCount int
if err := testPool.QueryRow(ctx, `
SELECT count(*) FROM agent_task_queue WHERE chat_session_id = $1
`, sessionID).Scan(&taskCount); err != nil {
t.Fatalf("count chat tasks: %v", err)
}
if err := testPool.QueryRow(ctx, `
SELECT count(*) FROM chat_message WHERE chat_session_id = $1
`, sessionID).Scan(&messageCount); err != nil {
t.Fatalf("count chat messages: %v", err)
}
if taskCount != 0 || messageCount != 0 {
t.Fatalf("archived send persisted tasks=%d messages=%d, want zero", taskCount, messageCount)
}
}
func TestDeleteBuilderSessionLocksAgentBeforeTasks(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
created := newBuilderSession(t)
taskID := insertPendingChatTask(t, created.BuilderAgentID, created.SessionID, "queued")
claimTx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin claim tx: %v", err)
}
defer claimTx.Rollback(context.Background())
qtx := testHandler.Queries.WithTx(claimTx)
agent, err := qtx.GetAgentForClaimUpdate(ctx, parseUUID(created.BuilderAgentID))
if err != nil {
t.Fatalf("lock builder agent: %v", err)
}
holderPID := holderBackendPID(t, ctx, claimTx)
deleteReq := withURLParam(
newRequestAs(testUserID, http.MethodDelete, "/api/chat/sessions/"+created.SessionID, nil),
"sessionId",
created.SessionID,
)
deleteReq = withChatTestWorkspaceCtx(t, deleteReq)
deleteCodes := make(chan int, 1)
go func() {
w := httptest.NewRecorder()
testHandler.DeleteChatSession(w, deleteReq)
deleteCodes <- w.Code
}()
if !waitForWaiterBlockedBy(t, holderPID, 10*time.Second) {
t.Fatal("delete did not wait for the builder agent lock")
}
claimed, err := qtx.ClaimAgentTask(ctx, db.ClaimAgentTaskParams{
AgentID: agent.ID,
PrepareLeaseSecs: 30,
})
if err != nil {
t.Fatalf("claim while delete waits for agent lock: %v", err)
}
if got := uuidToString(claimed.ID); got != taskID {
t.Fatalf("claimed task = %q, want %q", got, taskID)
}
if err := claimTx.Rollback(ctx); err != nil {
t.Fatalf("release claim transaction: %v", err)
}
select {
case code := <-deleteCodes:
if code != http.StatusNoContent {
t.Fatalf("delete returned %d, want 204", code)
}
case <-time.After(10 * time.Second):
t.Fatal("delete did not complete after agent lock released")
}
}
// The mirror image: a send that has not committed yet must hold off a rebind,
// and the rebind must then see the now-visible pending task and refuse. Without
// the lock on the switch side, GetPendingChatTask cannot see the uncommitted
// task, so the switch reports success while a reply is already in flight on the
// old runtime — the same "UI says B, execution is A" split, one turn later.
func TestSwitchAgentBuilderRuntimeWaitsForUncommittedSend(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
created := newBuilderSession(t)
target := newTestRuntime(t, "Builder Interleave Switch Target", "online")
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE chat_session_id = $1`, created.SessionID)
})
// Hold an uncommitted send: the chat_session lock plus its task row, in the
// order SendDirectChatMessage takes them.
tx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin send tx: %v", err)
}
defer tx.Rollback(context.Background())
holderPID := holderBackendPID(t, ctx, tx)
qtx := testHandler.Queries.WithTx(tx)
if _, err := qtx.LockChatSessionForRuntimeBind(ctx, parseUUID(created.SessionID)); err != nil {
t.Fatalf("lock chat session: %v", err)
}
if _, err := tx.Exec(ctx, `
INSERT INTO agent_task_queue (agent_id, chat_session_id, status, priority, context, runtime_id)
VALUES ($1, $2, 'queued', 2, '{}'::jsonb, $3)
`, created.BuilderAgentID, created.SessionID, testRuntimeID); err != nil {
t.Fatalf("insert in-flight task: %v", err)
}
codes := make(chan int, 1)
go func() {
w := httptest.NewRecorder()
req := withURLParams(
newRequest(http.MethodPatch, "/api/agent-builder/sessions/"+created.SessionID+"/runtime", map[string]any{
"runtime_id": target,
}),
"sessionId", created.SessionID,
)
testHandler.SwitchAgentBuilderRuntime(w, req)
codes <- w.Code
}()
if !waitForWaiterBlockedBy(t, holderPID, 10*time.Second) {
select {
case code := <-codes:
t.Fatalf("switch returned %d while an uncommitted send held the chat_session lock; the switch path is not taking the lock", code)
default:
t.Fatalf("switch never blocked on the chat_session lock held by pid %d; the switch path is not taking the lock", holderPID)
}
}
if err := tx.Commit(ctx); err != nil {
t.Fatalf("commit send: %v", err)
}
select {
case code := <-codes:
if code != http.StatusConflict {
t.Fatalf("switch returned %d after the send committed, want 409 — a reply is in flight", code)
}
case <-time.After(10 * time.Second):
t.Fatal("switch did not complete after the send committed")
}
// And the carrier must be untouched: a refused switch cannot half-apply.
var boundRuntimeID string
if err := testPool.QueryRow(ctx, `SELECT runtime_id::text FROM agent WHERE id = $1`, created.BuilderAgentID).Scan(&boundRuntimeID); err != nil {
t.Fatalf("reload builder carrier: %v", err)
}
if boundRuntimeID != testRuntimeID {
t.Fatalf("carrier runtime = %q after a refused switch, want the original %q", boundRuntimeID, testRuntimeID)
}
}
func TestSwitchAgentBuilderRuntimeEnforcesRuntimeAndSessionOwnership(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
// A plain member, so canUseRuntimeForAgent's owner/admin bypass does not
// apply — the fixture user is the workspace owner and may legitimately use
// anyone's private runtime.
var plainMemberID string
if err := testPool.QueryRow(ctx, `
INSERT INTO "user" (name, email) VALUES ('Builder Switch Plain Member', 'builder-switch-plain@multica.ai')
RETURNING id
`).Scan(&plainMemberID); err != nil {
t.Fatalf("create plain member user: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM "user" WHERE id = $1`, plainMemberID)
})
if _, err := testPool.Exec(ctx, `
INSERT INTO member (workspace_id, user_id, role) VALUES ($1, $2, 'member')
`, testWorkspaceID, plainMemberID); err != nil {
t.Fatalf("add plain member: %v", err)
}
// The fixture runtime is private to the workspace owner, so start this
// member's session on a public one.
var publicRuntimeID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent_runtime (
workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, owner_id, visibility, last_seen_at
)
VALUES ($1, NULL, 'Builder Switch Public', 'cloud', 'builder_switch_public', 'online', 'public', '{}'::jsonb, $2, 'public', now())
RETURNING id
`, testWorkspaceID, testUserID).Scan(&publicRuntimeID); err != nil {
t.Fatalf("create public runtime: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, publicRuntimeID)
})
// That member's own builder session, so the creator gate passes and the
// runtime gate is what we are actually testing.
createW := httptest.NewRecorder()
testHandler.CreateAgentBuilderSession(createW, newRequestAs(plainMemberID, http.MethodPost, "/api/agent-builder/sessions", map[string]any{
"runtime_id": publicRuntimeID,
}))
if createW.Code != http.StatusCreated {
t.Fatalf("CreateAgentBuilderSession as plain member: expected 201, got %d: %s", createW.Code, createW.Body.String())
}
var created CreateAgentBuilderSessionResponse
if err := json.Unmarshal(createW.Body.Bytes(), &created); err != nil {
t.Fatalf("decode create response: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `
DELETE FROM agent
WHERE workspace_id = $1 AND kind = 'system' AND system_key LIKE 'agent_builder:%'
`, testWorkspaceID)
})
// The workspace owner's private runtime is not a legal target for them.
var privateRuntimeID string
if err := testPool.QueryRow(ctx, `
INSERT INTO agent_runtime (
workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, owner_id, visibility, last_seen_at
)
VALUES ($1, NULL, 'Builder Switch Private', 'cloud', 'builder_switch_private', 'online', 'private', '{}'::jsonb, $2, 'private', now())
RETURNING id
`, testWorkspaceID, testUserID).Scan(&privateRuntimeID); err != nil {
t.Fatalf("create private runtime: %v", err)
}
t.Cleanup(func() {
_, _ = testPool.Exec(context.Background(), `DELETE FROM agent_runtime WHERE id = $1`, privateRuntimeID)
})
forbiddenW := httptest.NewRecorder()
testHandler.SwitchAgentBuilderRuntime(forbiddenW, withURLParams(
newRequestAs(plainMemberID, http.MethodPatch, "/api/agent-builder/sessions/"+created.SessionID+"/runtime", map[string]any{
"runtime_id": privateRuntimeID,
}),
"sessionId", created.SessionID,
))
if forbiddenW.Code != http.StatusForbidden {
t.Fatalf("someone else's private runtime: expected 403, got %d: %s", forbiddenW.Code, forbiddenW.Body.String())
}
// And a session the caller does not own is not theirs to rebind, whatever
// their workspace role — this one is issued by the workspace owner.
target := newTestRuntime(t, "Builder Switch Ownership Target", "online")
if w := switchBuilderRuntime(t, created.SessionID, target); w.Code != http.StatusForbidden {
t.Fatalf("someone else's session: expected 403, got %d: %s", w.Code, w.Body.String())
}
var boundRuntimeID string
if err := testPool.QueryRow(ctx, `SELECT runtime_id::text FROM agent WHERE id = $1`, created.BuilderAgentID).Scan(&boundRuntimeID); err != nil {
t.Fatalf("reload builder carrier: %v", err)
}
if boundRuntimeID != publicRuntimeID {
t.Fatalf("carrier runtime = %q after refused switches, want the original %q", boundRuntimeID, publicRuntimeID)
}
}
// The probe above is only as good as its attribution: a database-wide "is
// anything waiting on a lock?" check would match another package's test binary
// on the shared DATABASE_URL and let the interleaving tests commit their holder
// early, turning them green even with the lock under test removed. This pins the
// property directly — a waiter blocked by a DIFFERENT backend must not count.
func TestWaitForWaiterBlockedByIgnoresUnrelatedWaiters(t *testing.T) {
if testHandler == nil {
t.Skip("database not available")
}
ctx := context.Background()
mine := newBuilderSession(t)
theirs := newBuilderSession(t)
// Our holder: locks our own session and blocks nobody.
holderTx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin holder tx: %v", err)
}
defer holderTx.Rollback(context.Background())
holderPID := holderBackendPID(t, ctx, holderTx)
if _, err := holderTx.Exec(ctx, `SELECT id FROM chat_session WHERE id = $1 FOR UPDATE`, mine.SessionID); err != nil {
t.Fatalf("hold our own session lock: %v", err)
}
// An unrelated holder on a different row, plus a backend parked behind it —
// the shape of another package's test running against the same database.
otherTx, err := testPool.Begin(ctx)
if err != nil {
t.Fatalf("begin unrelated holder tx: %v", err)
}
defer otherTx.Rollback(context.Background())
otherPID := holderBackendPID(t, ctx, otherTx)
if _, err := otherTx.Exec(ctx, `SELECT id FROM chat_session WHERE id = $1 FOR UPDATE`, theirs.SessionID); err != nil {
t.Fatalf("hold unrelated session lock: %v", err)
}
blocked := make(chan struct{})
go func() {
defer close(blocked)
waiterTx, err := testPool.Begin(context.Background())
if err != nil {
return
}
defer waiterTx.Rollback(context.Background())
_, _ = waiterTx.Exec(context.Background(), `SELECT id FROM chat_session WHERE id = $1 FOR UPDATE`, theirs.SessionID)
}()
// The unrelated waiter is genuinely parked, so a database-wide probe would
// fire here.
if !waitForWaiterBlockedBy(t, otherPID, 10*time.Second) {
t.Fatal("the unrelated waiter never blocked; this test cannot prove anything")
}
// Attributed to our holder, it must not.
if waitForWaiterBlockedBy(t, holderPID, 500*time.Millisecond) {
t.Fatal("probe matched a waiter blocked by another backend; the interleaving tests could commit their holder early and pass with the lock removed")
}
if err := otherTx.Rollback(ctx); err != nil {
t.Fatalf("release unrelated lock: %v", err)
}
select {
case <-blocked:
case <-time.After(10 * time.Second):
t.Fatal("unrelated waiter did not finish after its blocker released")
}
}