feat(workspace): run and watch loops from chat (ent#458, folds ent#338)

[dolho]

Summary

A workspace user can start a loop from the chat it belongs to, watch it, and stop it — a collapsed strip above the composer that stays quiet until an agent in this chat is actually looping. Folds in ent#338, the per-run timeout that was never bounded by the agent's own ceiling.

No new backend surface, and that is the design

ent#458 scopes this to the platform-authenticated door (ent#78's auth-path invariant). A Workspace platform session already carries the operator's JWT — the same axios default header the rest of the app uses — and routers/loops.py gates on get_current_user / get_authorized_agent. So the panel calls the existing operator endpoints. No route, no model, no migration.

An external client holding a portal token never mounts it — hidden, not disabled: a disabled control advertises a capability that credential can never satisfy.

Live without a new transport

Loop events are already broadcast fleet-wide (#1106), and /workspace renders inside the same app shell whose root connects the WebSocket. They were already arriving; they were routed to the operator store, which filters to the agent on Agent Detail. So utils/websocket.js now routes each event to two stores:

Store	Filters to	Mounted by
stores/loops.js	the agent on Agent Detail	operator LoopsPanel
stores/portalLoops.js	the chat's participants	Workspace PortalLoops

The reportsStore + fleetReportsStore shape (#918); each a no-op when its surface isn't mounted. Separate stores because stores/loops.js is agent-at-a-time by construction (setAgent(name) replaces the list) while a room has several participants — one singleton would have each surface clearing the other's list on navigation (the skillsLibrary/skills split, ent#263).

A 12s backstop poll arms only while something is active, so an idle tab issues no traffic, and an unknown status counts as not active — the panel cannot sit claiming work is in flight forever (AC #4).

What it refuses to flatten

stop_reason carries six situations with six different next actions. portalLoopUtils.loopStatusLabel is the one place that won't collapse them — and the one that matters most: max_runs_reached reads as "Done", not "Stopped", on both the completed and the stopped row. Calling a loop that simply finished "Stopped" reads as a fault.

Headroom reports null for a guardrail that was never set — not 0%, not 100%. "No budget" and "budget untouched" are different facts, and a bar drawn at either extreme asserts the wrong one. An overshoot clamps to the end of the track, because the runtime lets the current run finish and cost can legitimately exceed its budget (#1155).

Guardrail defaults shown before Start mirror models.StartLoopRequest and are pinned against it (cross-language, cannot be imported). max_runs is deliberately excluded from that mirror: it is REQUIRED on the server, so the form's 10 is a suggestion — pinning it as a "default" would enshrine a fiction.

ent#338 — a bypass, not a display bug

task_execution_service reads the agent cap only when the caller passed no timeout_seconds, so an explicit timeout_per_run went straight to dispatch. A loop could run iterations longer than its owner's execution_timeout_seconds — multiplied by up to 100 runs. Now 400 with a structured agent_cap_seconds.

Refuse, not clamp (mirroring #929 for schedules): this feature puts the bounds on screen before Start, so a silent clamp would begin a loop with different bounds than the user was shown. It runs before #1156's deadline check, so that check can never quote a per-run timeout the caller may not have. It fails open on an unreadable cap — a resource ceiling, not a security gate, and the prior behaviour was no check at all.

Deferred, stated rather than dropped

AC #3 (loop history in the Activity tab) waits on ent#457, which builds that tab. The issue explicitly says history lives there and forbids a parallel surface, so the honest sequencing is to render into it once it exists. ent#457 is open and assigned.

Test Plan

• tests/unit/test_ent338_loop_timeout_cap.py — 5 tests: refusal above the cap with the structured detail, the boundary (equal is allowed), None never even reads the cap (None > int is the TypeError bug/design: schedule vs agent timeout precedence is silent + SIGKILL error message is ambiguous #929's docstring calls out), fail-open on a cap read error, and a source pin that the guard precedes the feat(loops): loop-level wall-clock deadline (max_duration_seconds) #1156 comparison.
• src/frontend/tests/unit/portalLoops.spec.js — 37 tests: the terminal vocabulary incl. max_runs_reached → Done, unknown status is not active, headroom null-vs-zero and overshoot clamp, the quiet strip, grouping, form pre-flight incl. the doom-loop 1 rejection and the ent#338 ceiling, payload omission of empty optionals, refusal-message parsing, the defaults↔models.py pin, and source assertions for the platform-only gate / always-available Stop / teaching empty state / semantic tokens only.
• Backend loop suites: 123 passed, 1 skipped.
• Full frontend unit suite: 1214 passed (57 files) — includes the raw-color and loading-gate ratchets.
• Raw-color ratchet: PortalLoops.vue uses semantic tokens only (action-primary, status-*); not in the offender list.
• Live against the running instance: timeout_per_run: 7000 vs a 3600s cap → 400 with agent_cap_seconds: 3600; 3600 at the boundary → 202; the real GET /loops payload fed through the pure rules → Done / 1/1 runs (100%) / cost null (no budget, correctly no bar) / strip null (quiet); POST /loops/{id}/stop → {"status":"stopping"}, confirming stop is cooperative and the row does not go terminal on the click.
• Not claimed: vite build fails locally on Rollup failed to resolve import "mermaid" — a declared dependency missing from this machine's node_modules, in AgentWorkspace.vue, which this branch does not touch. CI's npm ci has it.
• Manual: a browser pass on the strip's expand/collapse and the room variant with 2+ agent participants.

Related to Abilityai/trinity-enterprise#458
Related to Abilityai/trinity-enterprise#338

[dolho]

/review Report — self-review + a live pass on the running instance

Branch: feature/ent458-workspace-loops → dev @ abd7efc
Files Changed: 13 (+1308/−0)
Scope: CLEAN — ent#458 plus ent#338, which the issue explicitly folds into this wave
Plan Completion: AC #1 ✅ · #2 ✅ · #4 ✅ · #5 ✅ · #3 deferred to ent#457 by decision (that issue builds the Activity tab; ent#458 forbids a parallel surface)

---

Critical Findings — 2 found in my own diff, both fixed in abd7efc

[C1] The panel could render permanently empty (Confidence: 9/10)

visible derives from isPlatformSession, which derives from authStore.isAuthenticated and the portal token — both settled asynchronously:

isPlatformSession() {
  if (this.platformFallbackSuppressed) return false
  return !this.portalToken && !!useAuthStore().isAuthenticated
}

I guarded on it inside a watch keyed only on the participants:

watch(participants, (names) => {
  if (!visible.value) return      // ← at mount this is routinely false
  ...
}, { immediate: true })

So when auth confirmed after mount, the guard had already returned and nothing re-ran it — the strip would appear (its v-if is reactive) and never fetch, permanently, until the user switched chats. Same class as AdminEmailNudge's profileVerified (flagged on #2385 this week) and the ent#384 tile's key-appearance watch: a derived auth flag needs a watcher, not a read. Now watch([participants, visible]).

[C2] One mount point could wipe the other's list (Confidence: 8/10)

PortalConversation and PortalRoom mount the same component over one store singleton, and a room → 1:1 switch can mount the new panel before the old unmounts. My onUnmounted(() => store.clear()) would then blow away the list the new panel had just set — with no watcher left to re-run, since its participants hadn't changed. Unmount now clears only if the store still holds this instance's participants, and otherwise just stops the poll.

---

Clean Categories — checked, not assumed

• Enum completeness (4.12) — this is the category that needed real work, because loopStatusLabel is a consumer of a value set defined in Python. Grepped every stop_reason literal the producers write (loop_service.py, db/loops.py, cleanup_service.py): 9 distinct values, all covered — six in the stopped switch, interrupted on its own status branch, and error + max_consecutive_failures under failed. Unknown reasons fall through to a plain "Stopped" rather than rendering blank.
• Auth boundary — no new endpoint. The client-side gate is UX, not containment: the underlying routes are the operator's own and remain server-gated by get_current_user / get_authorized_agent, so hiding the strip cannot be load-bearing.
• Credential exposure — nothing secret reaches the panel.
• Error handling — Promise.allSettled per participant; a partial failure keeps what loaded and says the list may be incomplete (fix(ui): a failed background refresh keeps the data and says so (ent#253) #2382's rule) rather than blanking.
• Performance — one request per participant (typically 1–3), not per roster entry. The ent#2198 N+1 was per-agent across the whole roster on every thread refresh; this is a different unit, and a batched route would be new backend surface for a fleet this small. Stated in the flow doc.
• Docs — requirements §38.6/§38.7, a new feature flow, architecture pointer, index entry.

---

Live pass — against the running instance, not a fixture

The frontend container mounts the working tree and runs Vite dev, so localhost:8001 was serving this branch throughout.

It compiles where it will actually run: GET /src/components/portal/PortalLoops.vue → 200 with real transformed output (a compile error returns an error body); same for the store and the pure module. vite build inside the container with the real node_modules → exit 0. (My earlier "build fails on mermaid" was my host's incomplete node_modules, now settled — that claim is withdrawn.)

The form → payload → API path, using the branch's own startPayload:

form  -> {"message":"say the word ready","max_runs":2,"delay_seconds":0,"no_progress_threshold":3}
POST  -> {"loop_id":"loop_4QlERwFWrKWTRWMn","status":"queued", ...}

Empty optionals omitted so server defaults apply, exactly as specified.

A running loop, rendered through the real pure module:

PANEL STRIP : "1 loop running"
ACTIVE ROW  : Running
  runs : 1/5  20%
  cost : $0.0650086 / $1  (7%)
  time : 12s / 3600s

The null-vs-zero rule, proven by accident on real data — two rows in the same list, one with a time bar and one without, purely because one loop had a deadline set and the other didn't. That is the design claim ("no budget" ≠ "budget untouched") demonstrated rather than asserted.

Stop, end to end:

POST /stop            -> {"status":"stopping"}
immediately after     -> status still "running", stop_reason null   ← the row does NOT lie
after the iteration   -> stopped / user_stopped
                         reads "Stopped by you" | tone warn | Stop button gone

Step 2 is why the store deliberately does not optimistically mark a stopped loop terminal: an optimistic update would have shown a false "Stopped" for ~20 seconds while the agent was still working.

ent#338 live: timeout_per_run: 7000 against a 3600s cap → 400 with {"error":"loop_timeout_exceeds_agent_cap","agent_cap_seconds":3600,"requested_seconds":7000}; 3600 at the boundary → 202.

Suites: 39 frontend tests for this feature, 1216 passed (57 files) for the whole frontend suite with the repo present, 123 passed across the backend loop suites, 5 new backend tests.

---

Summary

• Critical: 2 found, 2 fixed (abd7efc) — 0 outstanding
• Informational: 0 worth raising
• Scope: clean
• Not covered: no browser automation available here, so expand/collapse interaction and the multi-agent room layout are unexercised — everything behind them (data, rules, request/response, compile) is verified. A human pass on those two is the remaining gap, and it's in the PR's test plan.

One environment note for whoever runs the suite: portalLoops.spec.js reads models.py to pin the defaults against the server, so it needs the repo, not just src/frontend. Running vitest inside the frontend container (which mounts only /app) fails those two tests with ENOENT: /backend/models.py. CI checks out the whole repo; verified green there.

[obasilakis]

Validated against the methodology checklist. The substance is in good shape — _reject_timeout_above_cap lands before the #1156 deadline comparison (and is pinned as such), the fail-open on an unreadable cap is argued rather than assumed, the dual-store WebSocket routing follows the reportsStore/fleetReportsStore precedent, and the second commit's fix — watching visible rather than reading it inside a participants-only watch — is the right shape for a derived auth flag. CI is green apart from one still-running pytest shard whose sibling seeds passed on both base and head. Security scans clean.

Three things before merge, one of which actually matters.

Blocking

The issue links are not closing keywords. The body says Related to Abilityai/trinity-enterprise#458 and Related to Abilityai/trinity-enterprise#338. .github/workflows/issue-status-on-merge.yml only promotes status-in-progress → status-in-dev on a closing keyword (Fixes / Closes / Resolves), so as written both issues strand in status-in-progress after this ships — the drift /groom keeps cleaning up.

Please rephrase to:

Fixes Abilityai/trinity-enterprise#458
Fixes Abilityai/trinity-enterprise#338

Worth noting for whoever merges: the reference is cross-tracker, so GitHub auto-closes nothing here regardless. Both issues need status-in-dev set by hand after merge, and a manual close at release. ent#458 in particular should stay open until ent#457 lands AC #3.

Docs polish

docs/memory/feature-flows.md — the index row is orphaned. It was appended at the very end of the file, after the closing See docs/TESTING_GUIDE.md for testing template and examples. line, rather than into the flows index table. It renders as a stray single-row table at the bottom of the page. Move it into the table proper.

docs/memory/feature-flows/workspace-loops.md has no ## Testing section. House style for flow docs is loose (dashboard-list-view.md runs five sections), but Testing is in every recent one, and the evidence already exists — it just did not make it out of the PR description and into the document. ## Security Considerations / ## Error Handling / ## Related Flows are also absent; those I would call optional given the precedent, but Testing should be there.

Non-blocking suggestions

• _reject_timeout_above_cap's except Exception: return is silent. Fail-open is the correct call for a resource ceiling, but a logger.warning would keep a repeatedly-failing cap read from being invisible — right now a persistently broken settings read looks identical to a fleet where nobody sets timeout_per_run.
• PortalLoops.vue::onStop assigns store.error directly from the component. Pinia permits it; an action would keep the mutation surface in one place.
• The unclaimed vite build failure on a missing mermaid in AgentWorkspace.vue is a local node_modules gap, not a branch defect — CI's build job passed. Fine as stated; no action.

Nothing here touches the design decisions, which read as deliberate throughout — particularly refusing rather than clamping the per-run timeout (the bounds are on screen before Start, so a silent clamp would begin a loop under different guardrails than the user was shown), and max_runs_reached reading as Done rather than Stopped.

[dolho]

Code review — re-review at d2fdbd2

The previous round's docs asks are done: the index row is filed in the flows table beside the other Workspace entries, and the flow doc has a ## Testing section with real counts.

This pass went at the panel and the store rather than the docs, and found a cluster with one root cause: the participants watcher.

High

1. PortalLoops.vue:252 — the watcher re-fires on every parent re-render, issuing a GET /loops per participant per keystroke.

participants is computed(() => props.participants), and both parents pass a freshly-created array on each render: PortalConversation.vue:134 passes the inline literal :participants="[agent.name]", and PortalRoom.vue:126 passes agentParticipants, a computed that re-derives from room.value (reassigned by the 3 s poll at PortalRoom.vue:520). A non-deep watch compares with Object.is, so a new array identity always triggers. store.setParticipants early-returns on unchanged content — but store.fetchLoops() on :257 is called unconditionally.

Concrete: a user types a 40-character message in a 1:1 Workspace chat. input is rendered in the parent template (v-model="input", :disabled="… !input.trim()"), so every keystroke re-renders it and fires 40 x GET /api/agents/{n}/loops?limit=20, each of which runs _build_status_response per loop and db.list_loop_runs per row. In a room it is an unconditional 3 s x N poll even with nothing running — which contradicts both the intended "an idle tab issues no traffic" and the 12 s backstop the PR describes.

Fix: watch a stable key (join the names into a string), or have the parents pass a stable array.

Medium

2. PortalLoops.vue:256 — the same re-fire resets form.agent, silently discarding the user's selection. In a room with [alpha, beta] a user opens the start form and picks beta; within 3 s the room poll replaces room.value, the watcher re-runs, and form.agent = names[0] puts it back to alpha. The select visibly snaps back — and a Start clicked in that window runs the loop on the wrong agent, since onStart reads form.agent. The form-field assignment belongs inside the "participants actually changed" branch.

3. PortalLoops.vue:275 — the unmount fallback store.stopPolling() kills the incoming panel's backstop. The comment above it establishes that the new panel can mount before the old unmounts — which is exactly the branch where ownedKey no longer matches store.participants, so stopPolling() runs. The store is a singleton and _ensurePolling() is only called from fetchLoops's finally, so if the new panel's initial fetch has already resolved and armed the 12 s timer, the outgoing panel clears it and nothing re-arms until the next fetch. Switching from a 1:1 to a room with a loop running leaves the room's panel on WS alone — so a dropped loop_completed, the exact failure the poll backstops, leaves the row stuck on "Running" indefinitely. That else-branch should be a no-op: the store is not this instance's any more.

4. PortalLoops.vue:263 — the auto-expand-when-running behaviour can never fire. onMounted reads store.hasActive, but the immediate watcher only starts the async fetchLoops(), so loops is empty at that moment. Nor can it be left over from a previous mount: the matching-key branch of onUnmounted calls store.clear(). So a user returning to a chat with a loop mid-flight gets the collapsed strip reading "N loops running" and no expansion — the documented "not asked to go looking for work that is in flight" does not happen. Needs a one-shot watch on store.hasActive rather than a read in onMounted.

Low

5. stores/portalLoops.js:148 — an event with no agent_name refetches everything, and every event refetches all participants. if (name && !participants.value.includes(name)) return short-circuits only when name is truthy, so any loop_run_completed / loop_completed payload missing agent_name refetches for every participant. The operator sibling (stores/loops.js:134-140) requires both agent_name and loop_id and does a targeted loadLoop(data.loop_id). A 100-run loop on any agent in the fleet emits 100 events; each costs N x GET /loops?limit=20 here (each expanding into per-loop run queries), whether or not the loop belongs to this chat. Mirror the operator store: require agent_name, prefer a single-loop reconcile.

6. portalLoopUtils.js:222 — startPayload silently drops two fields FORM_INITIAL carries. FORM_INITIAL spreads GUARDRAIL_DEFAULTS, which includes on_failure and max_consecutive_failures, but the optional map omits both, so neither is sent. Harmless today only because the omitted values happen to equal the server defaults in models.StartLoopRequest — but the SFC already shows max_consecutive_failures to the user as a promise about this loop (PortalLoops.vue:143), so the moment a control is added or a server default moves, the form will display one bound and start a loop under another. Either drop them from FORM_INITIAL or include them in the payload.

🤖 Generated with Claude Code

[obasilakis]

Re-reviewed at d2fdbd2. I read the three commits forward from the SHA my earlier review was submitted against (abd7efc), then read the current diff on its own merits — the backend guard, the store, the pure module, both mount points, and the two parents that supply the participants prop. CI is green at this head (23 successful, 3 skipped).

My earlier findings

"Related to" → "Fixes" (I marked this blocking) — withdrawn. The ask was wrong, and I should correct it rather than repeat it. I checked .github/workflows/issue-status-on-merge.yml properly this time. Its pattern is \b(?:fix(?:e[sd])?|close[sd]?|resolve[sd]?)\s+#(\d+), which requires whitespace immediately before the #, and it calls addLabels against context.repo.owner / context.repo.repo — always this repo. So Fixes Abilityai/trinity-enterprise#458 matches nothing and would change no label. Worse, the shape that does match is the bare Fixes #458, which would have stamped status-in-dev on the unrelated public issue abilityai/trinity#458. Leave the body as it is. Both enterprise issues need status-in-dev set by hand after merge either way, and ent#458 should stay open until ent#457 lands AC #3.

docs/memory/feature-flows.md index row — addressed. It now sits in the conversation-flows table beside the other Workspace entries, in that table's three-column shape, and the stray one-row table past the end of the file is gone.

workspace-loops.md missing ## Testing — addressed. The section is there with the real suite names and counts.

Non-blocking, still open (still non-blocking): _reject_timeout_above_cap's except Exception: return is still silent, and PortalLoops.vue:241 still assigns store.error from the component. Neither needs to hold the PR.

The review comment standing at the current head

The comment posted at d2fdbd2 raised six findings after the last commit, so none of them have been responded to in code. I verified all six against the current tree rather than taking them on trust. All six reproduce. Two of them I consider blocking, for reasons below; the other four I would take as follow-ups.

Blocking

1. The participants watcher fires on prop identity, not content, so the panel fetches continuously. PortalLoops.vue:198 is computed(() => props.participants) and :252 is a non-deep watch over it, which compares with Object.is. Both parents hand it a freshly allocated array on every render: PortalConversation.vue:134 passes the inline literal :participants="[agent.name]", and PortalRoom.vue:126 passes agentParticipants, a computed whose .filter().map() at PortalRoom.vue:266 allocates a new array each time room.value is reassigned. store.setParticipants does early-return on unchanged content, but store.fetchLoops() at :257 runs unconditionally.

The two concrete consequences: in a 1:1 chat, input is v-model-bound in the same component (PortalConversation.vue:245), so every keystroke re-renders the parent and issues one GET /api/agents/{n}/loops?limit=20 per participant — and each of those responses runs db.list_loop_runs per row inside _build_status_response, so a 40-character message costs 40 requests and up to 40 × 20 extra queries. In a room, PortalRoom.vue:520 reassigns room.value on every 3s poll tick, so the panel fetches every three seconds for the life of the chat whether or not anything is running.

That second case is what makes this blocking rather than a performance nit: it contradicts the contract the store documents for itself at stores/portalLoops.js:42-48 ("only while something is actually active, so an idle Workspace tab issues no traffic at all") and the AC #4 claim in the PR body, and it runs at 4× the rate of the 12s backstop it is supposed to be quieter than. Watching a stable key (the joined names) or having the parents pass a stable array fixes it.

2. The same re-fire resets form.agent, and in a room that can start a loop on the wrong agent. PortalLoops.vue:256 assigns form.agent = names[0] || null unconditionally inside the watch callback. In a room with participants [alpha, beta], a user opens the start form and selects beta; within three seconds the room poll re-fires the watcher and the select reverts to alpha. onStart at :229 reads form.agent, so a Start clicked in that window runs up to 100 LLM turns against alpha — billed to alpha's owner, on an agent the user did not choose. The form-field assignment belongs inside a "participants actually changed" branch, next to the setParticipants guard that already has one.

Non-blocking

3. Unbounded expanded list in a flex column that has no room for it. The expanded container at PortalLoops.vue:37 has no max-height and no overflow-y-auto, the row loop at :64 renders every loop returned, and fetchLoops requests limit: 20 per participant with no status filter — so it includes terminal history, not just active loops. In a three-agent room that is up to 60 rows, each with a three-column bar grid. The panel's root is also a plain div with no shrink-0, while its sibling message pane is flex-1 min-h-0 overflow-y-auto (PortalConversation.vue:75, PortalRoom.vue:70), so the expanded panel takes its height out of the conversation. The design-system contract asks for bounded viewports for unbounded data; a max-h-* plus overflow-y-auto on the expanded container would settle it.

4. startPayload silently drops two fields the UI displays as a promise. FORM_INITIAL (portalLoopUtils.js:43) spreads GUARDRAIL_DEFAULTS, which carries on_failure and max_consecutive_failures, but the optional map at :222-228 omits both, so neither is sent. This is harmless only while those values happen to equal the server defaults — and PortalLoops.vue:143 already shows max_consecutive_failures to the user as a statement about this loop. Either include them in the payload or drop them from FORM_INITIAL.

5. The same class again, on the deadline. max_duration_seconds is in FORM_INITIAL and in startPayload, but no control sets it, so it is always null. The copy at PortalLoops.vue:144 reads "No time limit unless you set one", which describes a control the form does not have. ent#458 AC #1 names duration among the guardrails visible before start; the default is arguably visible, but the sentence promises more than the form offers. Either add the input or reword to say there is no time limit.

6. WS handling refetches more than it needs to. stores/portalLoops.js:149 short-circuits only when agent_name is truthy, so any loop_run_completed / loop_completed payload without it refetches every participant. The operator sibling at stores/loops.js:134-140 requires both agent_name and loop_id and does a targeted single-loop reconcile. Since a 100-run loop anywhere in the fleet emits 100 events, mirroring the operator store's shape is worth doing.

7. Auto-expand cannot fire. PortalLoops.vue:263 reads store.hasActive in onMounted, but the immediate watcher has only started the async fetchLoops() by then, and the matching branch of onUnmounted calls store.clear(), so nothing survives from a previous mount. A one-shot watch on store.hasActive would deliver the documented behaviour.

8. The unmount fallback stops the incoming panel's poll. PortalLoops.vue:275's else runs precisely in the branch the comment above it describes — the new panel already owns the store — so stopPolling() clears a timer the incoming panel armed, and _ensurePolling is only reached from fetchLoops's finally. That else-branch should be a no-op.

9. test_ent336_s03_slot_timeout_floor.py:246 now has a stale docstring. test_loop_timeout_above_the_agent_cap_does_not_fire still explains that "neither routers/loops.py nor loop_service.py compares it to the cap", which this PR makes untrue. The assertion is still correct and still passes; it is the explanation that now documents a fixed bug as live behaviour — on the one test that exists to explain why S-03's upward arm was narrowed. Its sibling at :264 explicitly asks for it to be revisited alongside.

10. Worth a release note. As ent#338 itself flags, this is a behaviour change to a user-facing validator: an existing MCP run_agent_loop caller passing timeout_per_run above the agent cap starts receiving a 400 where it previously succeeded. The refuse-rather-than-clamp choice is right and well argued, but the change deserves a line at release time.

One question, not a finding

POST /api/agents/{name}/loops is gated on get_authorized_agent, which admits owner, admin, and anyone the agent is shared with. A Workspace platform session's roster includes shared agents, so a role: user collaborator granted chat-only access via /share can now start up to 100 turns billed to the owner's subscription, with no cost budget by default. I checked before writing this up: the gate is unchanged by this PR, and AgentDetail.vue's Loops tab is already pushed unconditionally rather than behind canShare, so the same person could already do this from the operator surface. The reach is the same; what changes is that this becomes the default surface for exactly that population. Given the "a dispatch spends money, so it is never unconditional" reasoning behind ent#329's owner-only toggle, is an owner-level control over who may run loops something you want filed as a follow-up, or is the sharing grant deliberately taken as sufficient here?

Verdict

Requesting changes on items 1 and 2 only. The backend half is in good shape — the guard lands before the #1156 deadline comparison and is pinned as such, the fail-open is argued rather than assumed, and it matches ent#338's own fix sketch. The store's fetch-token, partial-failure and cooperative-stop handling are right, and the refusal to flatten stop_reason is the correct call. The two blocking items are both in the participants watcher and should be small: watch a stable key, and move the form.agent assignment into the changed branch. Nothing else here needs to hold the merge.

[dolho]

Code review — re-review at 0f2e99a

The six findings from the last round are fixed: the watcher is keyed on content, the agent selection survives a re-fire, the unmount branch no longer stops the incoming panel's poll, auto-expand is a one-shot watch on hasActive, the WS filter requires agent_name, and the payload sends both guardrails.

Seven findings this round.

Medium

1. stores/portalLoops.js:159 — the event handler still refetches everything, once per run.

Requiring agent_name fixed the unattributed event; the cost per attributed one is unchanged. handleWebSocketEvent calls the full fetchLoops() on every loop_run_completed — one GET /loops?limit=20 per participant, and list_loops calls _build_status_response per row, which does an N+1 db.list_loop_runs(loop_id) and materialises every run with a 500-char response_preview. A single 100-run loop therefore triggers 100 refetches, each ~20 extra queries per participant, with a response carrying up to 20 loops × 100 runs.

The operator sibling (stores/loops.js:134) does a targeted loadLoop(data.loop_id) for exactly this reason. This store has neither that nor any coalescing.

2. stores/portalLoops.js:98 — a failed first load leaves the panel with no way out.

On a total fetch failure (anyOk === false) the store sets error but never sets hasLoaded. The template gates the teaching empty state on store.hasLoaded && !store.loops.length (PortalLoops.vue:44) and "Start another loop" on store.loops.length (:170), so both are hidden; _ensurePolling() then stops the poll because nothing is active. So: the backend is briefly down on first load, the user expands the strip, sees "Could not load loops.", and has no retry and no way to start a loop for the rest of the session — nothing re-runs until participantsKey or visible changes.

Low

3. PortalLoops.vue:269 — keyChanged is false on exactly the path the watcher exists for.

keyChanged = !previous || previous[0] !== key. The immediate run bails at if (!isVisible) return, so the second run — visible flipping true, which is the late-auth-confirm case this watcher was written for — has previous = [key, false] and the same key. form.agent is therefore never initialised: in a room the Agent <select> renders with nothing selected, and onStart silently falls back to participants.value[0], which need not be the agent the blank control implies. Set it whenever it is still null, not only on a key change.

This is the fix for finding #2 of the last round landing one branch short.

4. PortalLoops.vue:226 — the ent#338 client pre-flight is unreachable. validateStartForm(form) is called with no options, so agentTimeoutCap is always undefined, and both errors.timeout_per_run and the deadline-vs-per-run check (portalLoopUtils.js:216-224) can never fire — the only caller never supplies the cap, and the form exposes neither field. The PR says this "stops the user discovering their ceiling by rejection"; today it cannot. Related: errors.no_progress_threshold / .timeout_per_run / .max_duration_seconds have no rendering, so if any of those fields is added later, Start would refuse with no visible reason.

5. PortalLoops.vue:37 — the expanded panel is unbounded. No height cap or scroll region, while the store fetches limit: 20 per participant and the template renders all of them. A room with three agents is up to 60 rows in a flex-col h-full container, pushing the message area and — on short viewports — the composer out of view. The design-system contract requires bounded viewports for unbounded data, and the sibling panels above it (PortalAsks, PortalDeliverables) are shrink-0 with bounded content.

6. stores/portalLoops.js:96 — a permanent authorization gap reads as a transient one. A room may contain agent participants the platform user neither owns nor is shared on; GET /loops goes through get_authorized_agent, so those 403 forever. anyOk is true from the reachable ones, so the panel shows "Some agents could not be reached; this list may be incomplete." on every refresh for the life of the room, with nothing distinguishing an outage from a gap that will never close.

7. routers/loops.py:127 — ent#338 closes the create-time bypass and leaves the mirror half open. PUT /api/agents/{name}/timeout refuses a cap below any active schedule's timeout_seconds (agent_timeout_below_active_schedules, #929) and has no equivalent for running loops. An owner can lower the cap to 60s while a loop with timeout_per_run: 3600 is mid-flight, and the remaining iterations still run at 3600s — the same "iterations longer than the ceiling its owner set" the docstring argues against, arrived at from the other direction.

🤖 Generated with Claude Code

[obasilakis]

Approving at 0f2e99a3. Both blocking items are fixed, and I read the tree rather than the commit message for each.

1 — the participants watcher. participantsKey is the joined string and the watch source is [participantsKey, visible], so a re-rendered prop with identical contents no longer fires it. That closes the per-keystroke fetch in a 1:1 and the unconditional 3s fetch in a room, and restores the store's own "an idle Workspace tab issues no traffic at all".

2 — the agent selector. form.agent = names[0] now sits behind keyChanged, so a room's poll tick can't put the select back to the first participant under someone mid-selection.

You also took 4, 7 and 8 and half of 6, none of which I'd asked you to hold the PR for. The auto-expand fix in particular is the right shape: one-shot via a flag rather than the watcher's own stop handle, which with immediate: true would not be assigned yet when the callback runs — and hasActive can legitimately be true at mount, since the store is a singleton and a sibling panel may not have unmounted.

One thing I got wrong, and one small edge

Nothing to change for the merge, but both are worth having on the record.

if (keyChanged) is right for the re-fire it targets, but visible settles asynchronously — the C1 race the previous commit fixed. If the first watcher fire returns early on !isVisible and the second only flips visible, then keyChanged is false and form.agent is never seeded, so the select renders blank for the life of the chat. This is cosmetic, not a wrong-agent start — onStart falls back to participants.value[0], so a Start still runs on the agent the blank select would have shown. keyChanged || !form.agent covers it.

Three I would rather not see deferred, because they are text

Not merge blockers. But they are minutes, they carry no design question, and two of them are wrong today:

• test_ent336_s03_slot_timeout_floor.py:246. The docstring still explains that neither routers/loops.py nor loop_service.py compares a loop's timeout to the agent cap. This PR makes that untrue. The assertion is still correct and still passes; it is the explanation that now documents a fixed bug as live behaviour — on the one test that exists to say why S-03's upward arm was narrowed, and whose sibling at :264 explicitly asks for it to be revisited alongside. A stale comment on a canary rationale is the kind that stays wrong for a year.
• The start form's copy. It reads "No time limit unless you set one" and the form has no control that sets one — max_duration_seconds is in FORM_INITIAL and in startPayload and is always null. This PR's own standard is the reason I'm raising it rather than shrugging: the headroom rule is "null for a guardrail that was never set — not 0%, not 100%; no budget and budget untouched are different facts". A sentence describing a field that does not exist is the same class. The one-line reword stops the inaccuracy; the input is the fuller answer to AC #1, which names duration among the guardrails visible before Start.
• The release note. ent#338 changes a user-facing validator: an existing MCP run_agent_loop caller passing timeout_per_run above the agent cap now gets a 400 where it previously succeeded. Worth recording somewhere durable now rather than reconstructing it at freeze.

Fine as follow-ups

• 6, the half that remains. Requiring agent_name removed the bad case, and that was the important half — an unattributed event no longer fans out. What is left is that the handler still calls fetchLoops() for an event that names exactly one loop, so the cost is one request per participant per event, each expanding into a db.list_loop_runs per row it returns. Bounded (1–3 participants, only loops in this chat), but a 100-run loop still pays 100×N list calls where the operator sibling pays 100 single-loop reads.
• 3, the unbounded expanded list. limit: 20 per participant with no status filter includes terminal history, so a three-agent room reaches ~60 rows in a flex column whose sibling message pane is flex-1 min-h-0.
• The silent except Exception: return in _reject_timeout_above_cap, and onStop assigning store.error from the component. Both still open, both still non-blocking.

I have the remainder implemented and green locally if it's useful — say the word and I'll hand you the branch rather than push into yours.

CI

Green. The two red pytest shards (base seed 67890, head seed 99999) are 25-minute runner timeouts that produced no JUnit XML, not test failures — regression diff reports no new failures across the surviving seeds on both sides, with head at 12417 tests to base's 12412.

At merge

As established last round, the Related to links are correct as written and should stay — the workflow's pattern needs whitespace before the # and only labels this repo, so a cross-tracker Fixes matches nothing and the bare form would stamp the unrelated public issue #458. Both enterprise issues need status-in-dev set by hand, and ent#458 should stay open until ent#457 lands AC #3.