Skip to content

fix: collaboration never started, role editor stole focus, auto-approved tools ran invisibly - #274

Merged
saucam merged 3 commits into
mainfrom
fix/collab-startup-and-role-input
Aug 4, 2026
Merged

fix: collaboration never started, role editor stole focus, auto-approved tools ran invisibly#274
saucam merged 3 commits into
mainfrom
fix/collab-startup-and-role-input

Conversation

@saucam

@saucam saucam commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes two of the three things reported against collaborative sessions. The third turned out to be intentional design — details at the bottom.

1. A collaboration was built but never started

session.create compiled the goal into the orchestrator's constitution, brought up the role-children, and returned. Nothing ever took a turn. The goal sat idle with an empty transcript until the owner typed into a session that already knew exactly what it was for — which is why the sidebar showed "2 sub-agents" while the centre pane stayed blank and no progress ever happened.

Confirmed against the live codeoid-review collaboration rather than inferred:

bed59627…  codeoid-review            [idle]  role=-       ← orchestrator
61f776e6…  codeoid-review:search     [idle]  role=worker
7922d5cf…  codeoid-review:architect  [idle]  role=worker

All three had a .meta.json and no .jsonl at all — not one message, not even a user prompt — with lastActivityAt equal to createdAt. Eighteen other sessions in the same directory had normal transcripts, so this wasn't a transcript-writing fault.

The orchestrator is now sent its goal as the opening user turn. Sending the goal text rather than a bare "begin" keeps the transcript self-describing on attach and on resume, instead of opening with a directive whose subject lives only in the constitution.

The send is deliberately fire-and-forget: a create that has already spawned children must not block on the first model call, and a failure leaves a usable (idle) collaboration plus a log line saying to send it a message, rather than failing the create. Children stay silent — bringing up a fleet of N still costs zero tokens.

2. The role editor stole focus on every keystroke

Typing in the collaborate role fields dropped the selection after each character, making continuous typing impossible.

The role rows rendered through <For>, which reconciles by item identity, while updateRole patched a row with { ...r, ...patch } — a brand-new object. So every character changed that row's identity, Solid disposed its DOM subtree and built a fresh one, and the <input> was remounted out from under the cursor.

Switched the roles list to <Index>, which keys by slot and hands the item in as an accessor, so the input element is stable across edits. This is exactly what <Index> is for: a fixed set of form rows whose contents change, as opposed to a keyed list that reorders.

3. Dynamic sub-agent spawning — already a deliberate decision

Not changed, because it isn't an oversight. ORCHESTRATOR_FLEET_TOOLS (fleet.ts:531) grants fleet_list, fleet_tasks, fleet_send, fleet_interrupt, fleet_panel — and documents the omission:

no fleet_spawn — §2 fixes a goal's role bindings for its whole life ("changing backends mid-goal would orphan live children"). The roster is declared at create time, and it is also what the tenant-wide live-children cap counts, so letting an orchestrator grow its own fleet ad hoc would route around a bound the owner set.

So the orchestrator delegates to, fans out across, and interrupts its declared roster, but cannot grow it. If that tradeoff should change, it's a design discussion (and a cap-accounting change), not a bug fix — happy to open an issue.

4. SDK-auto-approved tools ran invisibly

Followed up on the CLAUDE_SDK_CAN_USE_TOOL_SHADOWED warning in the daemon log:

canUseTool will not be invoked for: mcp__codeoid_memory__recall, recall_file, timeline, get_episode. Bare allowedTools entries auto-approve the whole tool before the callback is consulted.

It isn't just noise. canUseTool is ClaudeProvider's only tool_start emitter, so any tool listed by exact name in allowedTools executed with no tool_start at all — no tool_call message, nothing in the transcript, nothing in the UI, and nothing in the verbatim episode record that capturing tool calls exists to produce. That covers all four memory recall tools, plus the read-side fleet tools for a conductor.

Confirmed against the live instance rather than inferred — across 18 transcripts:

count
mcp.init tool listings 409
actual memory tool-call messages 0
Read tool calls (control) 594

The security boundary was never affected. These are reads tool-safety.ts already classifies as safe-to-run-unprompted; the audit record rides the PreToolUse hook, which still fires; and the send-class fleet verbs were never in allowedTools — only FLEET_TOOL_NAMES, the read set — so they still ride the owner's approval flow.

Fixed in the PreToolUse hook rather than by dropping the allowedTools entries, because those entries are also what makes an in-process MCP server's tools reachable at all (the design §3 gotcha documented on the fleet branch) — removing them risks making memory recall unreachable instead of merely invisible. The hook emits only for names in #autoApprovedTools, exactly the set that skips canUseTool, so an ordinary tool can never be double-emitted; and it reuses the SDK's own tool_use_id so tool_complete correlates exactly as it would have through the gate.

Two tests drive the real captured PreToolUse hook: one asserts a pre-approved tool now emits tool_start correlated on the SDK id, the other asserts Read still emits nothing there (no duplicates). The first fails against the previous commit.

5. Destroying a goal left its children in the sidebar

Reported as "destroying the top-level collab session should destroy all its children" and "children should be individually destroyable (which does not work?)".

Both already work on the daemon, which I verified rather than assumed:

  • Goal destroy cascades — #destroySession calls #teardownCollaborationChildren before destroying the goal, and an existing test pins that children are gone afterwards.
  • Individual child destroy works — probed against the real manager: session.destroy on a child returns response.ok and it leaves the list, siblings and goal untouched.

The web store was the problem. The destroy button calls removeSession(sessionId) with only the id it asked about, so the goal vanished while its children stayed listed — sessions the daemon had already destroyed. Clicking one attached to nothing, and the ghosts survived until a full refresh. That is almost certainly what read as "child destroy doesn't work".

removeSession now takes the goal's role-children with it, mirroring their per-goal lifetime. Fixed in the store rather than at the button so every removal path inherits the cascade, and focus now moves off any removed session instead of only the one named in the call — otherwise destroying a goal while a child was focused left focus pointing at a session that no longer existed.

Destroying a single child still removes only that child: it has no children of its own, so the same path is a no-op for its siblings and the goal.

Three store tests; two fail without the fix.

Verification

New test: collaboration auto-start > starts the orchestrator on its goal instead of leaving it idle — asserts the orchestrator's first runTurn carries the goal text, and that every child still has zero captured turns. It fails against pre-fix main by timing out waiting for a turn that never comes.

Also updated dispatch-host's collaboration helper, which assumed create left the orchestrator idle. It now waits for the kickoff turn to complete — checking numTurns, not just status, because the fire-and-forget send means an immediate status read still reads idle and would tick the dispatcher into a mid-turn orchestrator.

  • bun run typecheck clean (root + protocol + core); web tsc clean
  • bun run lint clean, 347 files
  • bun test2206 pass, 19 skip, 0 fail
  • web vitest390 pass across 40 files

Note

The focus fix has no automated test — asserting "the input element wasn't remounted" needs real DOM identity tracking across renders, which the existing jsdom component tests aren't set up for. Verified by reading the For/Index semantics against updateRole's immutable patch; worth a manual check when you next open the collaborate form.

🤖 Generated with Claude Code

…ling focus

Two defects found while investigating a codeoid-review collaboration that
showed its children in the sidebar and then did nothing.

1. A collaboration was built but never started.

session.create compiled the goal into the orchestrator's constitution,
brought up the role-children (deliberately silent, so a fleet of N costs
zero tokens), and returned. Nothing ever took a turn. The goal sat idle
with an empty transcript until the owner typed into a session that already
knew exactly what it was for.

Confirmed against a live instance: the parent and both children had a
.meta.json and no .jsonl at all -- not one message, not even a user prompt
-- with lastActivityAt equal to createdAt, while 18 other sessions had
normal transcripts.

The orchestrator is now sent its goal as the opening user turn. Sending
the goal text rather than a bare "begin" keeps the transcript
self-describing on attach and on resume. The send is fire-and-forget: a
create that already spawned children must not fail on the first model
call, and a failure leaves a usable idle collaboration with a log line
saying so. Children stay silent as before.

2. The collaborate role editor lost focus on every keystroke.

The role rows rendered through <For>, which reconciles by item identity,
while updateRole patched a row with { ...r, ...patch } -- a new object.
Every character retyped the row's identity, so Solid disposed that row's
DOM and built a fresh one, remounting the <input> under the cursor. Typing
a role name one character at a time was the only way through it.

Switched the roles list to <Index>, which keys by slot and hands the item
in as an accessor, so the input element is stable across edits. This is
what <Index> is for: a fixed set of form rows whose CONTENTS change, not a
keyed list that reorders.

Also adjusts dispatch-host's collaboration helper, which assumed create
left the orchestrator idle. It now waits for the kickoff turn to complete
-- checking numTurns, not just status, because the fire-and-forget send
means an immediate status read still reads idle.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

… invisibly

The daemon log carried CLAUDE_SDK_CAN_USE_TOOL_SHADOWED on every query:

  canUseTool will not be invoked for: mcp__codeoid_memory__recall,
  recall_file, timeline, get_episode. Bare allowedTools entries auto-approve
  the whole tool before the callback is consulted.

It is not just noise. canUseTool is ClaudeProvider's ONLY tool_start
emitter, so any tool listed by exact name in allowedTools ran with no
tool_start at all: no tool_call message, nothing in the transcript,
nothing in the UI, and nothing in the verbatim episode record that
capturing tool calls exists to produce. That covers all four memory recall
tools and, for a conductor, the read-side fleet tools.

Confirmed against a live instance rather than inferred: across 18
transcripts there are 409 mcp.init tool listings and ZERO memory tool
calls, while Read appears 594 times.

The security boundary was never affected. These are reads that
tool-safety.ts already classifies as safe-to-run-unprompted, the audit
record rides the PreToolUse hook (which still fires), and the send-class
fleet verbs were never in allowedTools -- only FLEET_TOOL_NAMES, the read
set -- so they still ride the owner's approval flow.

Fixed in the PreToolUse hook rather than by dropping the allowedTools
entries: those entries are also what makes an in-process MCP server's
tools reachable at all (the design §3 gotcha noted on the fleet branch),
so removing them risks making memory recall unreachable instead of merely
invisible. The hook emits only for names in #autoApprovedTools -- exactly
the set that skips canUseTool -- so an ordinary tool can never be emitted
twice, and it reuses the SDK's own tool_use_id so tool_complete correlates
exactly as it would have through the gate.

Tests: two cases in provider-claude, driving the real captured PreToolUse
hook. The positive one fails against the previous commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@saucam saucam changed the title fix: start a collaboration on its goal, and stop the role editor stealing focus fix: collaboration never started, role editor stole focus, auto-approved tools ran invisibly Aug 3, 2026
…oal is destroyed

Destroying a collaboration goal already tears its role-children down on the
daemon -- #destroySession calls #teardownCollaborationChildren before
destroying the goal itself, and an existing test pins that children are
gone afterwards. Individually destroying a single child works too; both
were verified against the real manager.

The web store did not mirror it. The destroy button calls
removeSession(sessionId) with just the id it asked about, so the goal
vanished while its children stayed listed -- sessions the daemon had
already destroyed. Clicking one attached to nothing, and the ghosts
survived until a full refresh.

removeSession now takes the goal's role-children with it, matching the
daemon's per-goal lifetime. Fixed in the store rather than at the destroy
button so every removal path mirrors the cascade, and focus is moved off
any removed session rather than only off the one named in the call --
otherwise destroying a goal while a child was focused left focus pointing
at a session that no longer existed.

Destroying a single child still removes only that child: it has no
children of its own, so the same code path is a no-op for siblings and the
goal.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@saucam
saucam merged commit 7176536 into main Aug 4, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants