Group templates: provision multiple rooms from one document (CHOO-2657) - #407
Closed
abeldantas wants to merge 5 commits into
Closed
abeldantas wants to merge 5 commits into
abeldantas wants to merge 5 commits into
Conversation
abeldantas
force-pushed
the
work/group-templates
branch
from
September 8, 2026 23:25
c6f755f to
201182d
Compare
abeldantas
force-pushed
the
work/template-params
branch
from
September 8, 2026 23:25
e7f8895 to
2dba04f
Compare
abeldantas
changed the title
RoomSpec gains an `aliases` field (agent name → room-scoped alias) and provision() passes it to RoomCreateConfig so aliases are seeded at room creation. Export reads aliases back from the room_agents table and emits them keyed by agent name. CHOO-2657
Introduces a second document shape discriminated by a top-level `group:` key (vs `room:` for single-room). A group document declares a room group (name, description, optional color), a list of rooms (`rooms:`), and optional directed links between them (`links:`). Params are interpolated across the entire document before validation. `provision_group()` creates the group row first, then each room with `group_id` set, then resolves intra-document links by room name. Partial failure on a room is reported (not rolled back), matching the existing single-room precedent. The `/rooms/from-yaml` endpoint accepts either shape and returns `ProvisionResult` or `GroupProvisionResult` accordingly. CHOO-2657
Extend `interpolate()` to substitute placeholders in dict keys (not
just values), so alias maps like `{bot}: greeter` resolve when the
agent name comes from a param — the onboarder's actual need.
Adds 12 tests covering group parsing (shape, params across rooms,
alias key interpolation, validation errors), group provisioning
(two-room linked group, params across rooms, partial failure on bad
agent), single-room regression, and the /from-yaml endpoint handling
a group document.
CHOO-2657
…on duplication Catch duplicate room names at parse time — a set collision would let links silently point to the wrong room. Also collapses _provision_room_in_group into provision() via an optional group_id parameter, removing ~55 lines of duplication. CHOO-2657
abeldantas
force-pushed
the
work/group-templates
branch
from
September 10, 2026 08:22
201182d to
0fe8785
Compare
abeldantas
marked this pull request as ready for review
September 10, 2026 14:48
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
It introduces a concrete input-validation hole and missing operational logging around swallowed exceptions, plus test code issues (misleading naming and dead code) that should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds support for “group” YAML templates so a single document can provision a room group, multiple rooms, and directed links between those rooms, extending the existing single-room YAML provisioning flow.
Changes:
- Extend
RoomYamlService.parse()to support two document shapes:room:(single room) andgroup:+rooms:(group provisioning). - Add best-effort group provisioning (
provision_group) and room alias support flowing through parse → provision → export. - Update the
/rooms/from-yamlgateway endpoint and expand test coverage for group parsing/provisioning and endpoint behavior.
File summaries
| File | Description |
|---|---|
| core/tests/switch_core/test_rooms_yaml.py | Adds group-template parse/provision tests and endpoint regression coverage. |
| core/switch_core/rooms_yaml.py | Implements GroupSpec parsing, best-effort multi-room provisioning with links, and alias interpolation/export. |
| core/switch_core/gateway/rooms.py | Updates /rooms/from-yaml to accept group docs and return either single-room or group provision results. |
| core/switch_core/gateway/dependencies.py | Wires room_group_store into RoomYamlService construction. |
Review details
Suppressed comments (2)
core/tests/switch_core/test_rooms_yaml.py:1298
- This block builds and parses collision_template/spec2 but never uses it; it's dead code and also reinforces the (incorrect) collision framing for this test.
# FakeRoomService doesn't enforce unique names, so we need to pre-create
# the collision. Instead, make the second room reference an unknown agent.
core/switch_core/rooms_yaml.py:560
- provision_group catches link-attachment failures but doesn't log them, which can hide operational issues (e.g., DB errors) behind an "errors" entry.
except Exception as e:
errors.append(
{
"kind": "link",
"from": link.from_,
"to": link.to,
"error": str(e),
}
)
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
352
to
354
| if inputs and not declared: | ||
| raise ValueError("Inputs supplied but the template declares no params") | ||
|
|
Comment on lines
+525
to
+528
| except Exception as e: | ||
| errors.append( | ||
| {"room_index": i, "room_name": room_spec.name, "error": str(e)} | ||
| ) |
Comment on lines
+1272
to
+1274
| async def test_provision_group_partial_failure_collision(env): | ||
| """When the second room name collides, group + first room exist and the | ||
| error names the collision.""" |
* feat(rooms-yaml): add typed params block and {var} interpolation
Add ParamSpec model, resolve_params() and interpolate() helpers to
rooms_yaml.py. parse() now accepts an optional `inputs` dict and an
optional top-level `params:` sibling of `room:`. Declared placeholders
are substituted throughout the room tree before Pydantic validation;
whole-field placeholders preserve the typed value so enum/boolean params
fill non-string fields correctly. Undeclared {word} patterns are left
intact.
Also accepts an optional top-level `version:` key (default 0).
26 new tests covering resolve_params, interpolate, and parse-with-params
(defaults, overrides, missing required, undeclared input, unknown
placeholder passthrough, whole-field typed substitution, nested
docs/references interpolation, version key).
* feat(gateway): accept JSON body with inputs on /rooms/from-yaml
The endpoint now checks Content-Type: when application/json, it expects
{"yaml": "<template>", "inputs": {...}} and passes inputs through to
parse(). Raw YAML body (any other content type) still works for
defaults-only templates.
Adds integration tests: two-room instantiation with different owners,
missing-required-param rejection, and an endpoint test exercising the
JSON path through an ASGI transport.
* fix(rooms-yaml): address reviewer findings
1. Gateway: validate yaml value is a string before passing to
safe_load — non-string values (e.g. {"yaml": 123}) now return 400
instead of 500.
2. _coerce: catch OverflowError alongside ValueError/TypeError so
infinity values (1e309, .inf) get a clear error instead of
an unhandled exception.
3. Endpoint test: calls the real create_room_from_yaml function with
a mock Request instead of reimplementing the routing logic in a
separate Starlette app.
* feat(rooms-yaml): wire aliases through RoomSpec provision and export
RoomSpec gains an `aliases` field (agent name → room-scoped alias) and
provision() passes it to RoomCreateConfig so aliases are seeded at
room creation. Export reads aliases back from the room_agents table
and emits them keyed by agent name.
CHOO-2657
* feat(rooms-yaml): add group document shape and provision_group
Introduces a second document shape discriminated by a top-level `group:`
key (vs `room:` for single-room). A group document declares a room group
(name, description, optional color), a list of rooms (`rooms:`), and
optional directed links between them (`links:`). Params are interpolated
across the entire document before validation.
`provision_group()` creates the group row first, then each room with
`group_id` set, then resolves intra-document links by room name. Partial
failure on a room is reported (not rolled back), matching the existing
single-room precedent.
The `/rooms/from-yaml` endpoint accepts either shape and returns
`ProvisionResult` or `GroupProvisionResult` accordingly.
CHOO-2657
* feat(rooms-yaml): interpolate dict keys, tests for group templates
Extend `interpolate()` to substitute placeholders in dict keys (not
just values), so alias maps like `{bot}: greeter` resolve when the
agent name comes from a param — the onboarder's actual need.
Adds 12 tests covering group parsing (shape, params across rooms,
alias key interpolation, validation errors), group provisioning
(two-room linked group, params across rooms, partial failure on bad
agent), single-room regression, and the /from-yaml endpoint handling
a group document.
CHOO-2657
* fix(rooms-yaml): reject duplicate room names in group, remove provision duplication
Catch duplicate room names at parse time — a set collision would let
links silently point to the wrong room. Also collapses
_provision_room_in_group into provision() via an optional group_id
parameter, removing ~55 lines of duplication.
CHOO-2657
* feat(agent-ops): add create_room_from_yaml operation (CHOO-2666)
Thin agent-side caller of the existing RoomYamlService.parse + provision
path — the same engine the gateway's POST /rooms/from-yaml uses. Agents
can now provision rooms and groups from inline YAML templates via the MCP
tool surface, with params interpolation and the same authorization gate
as create_room (owner_id/is_admin).
Includes 5 behavioral tests (single room, parameterized, group with
links, missing input → nothing created, ownerless agent → refused) and
updates all three connector skill indexes.
* style(agent-ops): move imports to top level per code style rules
* feat(console): load agents from a remote host (CHOO-2560) (#364)
* fix(console): refuse to overwrite a colleague's agent credentials on the same server
The Add Agent path minted a new identity and wrote `.switch/agents/<name>.json`
without checking whether the slot already held credentials from a different
Console install on the same Switch server. This destroyed the displaced agent's
API token.
Add a same-endpoint guard in `runAddAgent` that reads the existing slot and
checks its `SWITCH_AGENT_ID` against the local DB — if the id is unknown
locally it belongs to a colleague and the create is refused with
`already-configured`. Mirror the guard in `writeNeutralAgentSettingsFs` as
defence in depth. The modal shows a toast directing the user to load the
existing agent instead.
CHOO-2560
* feat(console): add Load Existing Agents UI for remote hosts
Restore the ability to load agents on a remote host that were created by a
different Console install, host-scoped and re-runnable from the remote host
page.
Discovery merges two sources: server-assisted (GET /agents with repo_dir,
confirmed on-disk via SFTP) and a bounded $HOME scan, deduped by (dir, name)
with server-attributed entries winning to preserve owner attribution.
The Load Existing Agents section appears on every remote host page and
auto-expands when the host was just added (the post-add-host prompt). Each
row shows name, directory, owner, provider, and endpoint-mismatch warnings.
Agents without an inferred provider get an inline picker. Already-loaded
agents appear disabled. A manual "scan a directory" fallback covers agents
outside $HOME.
Also extends RemoteAgentSummary with knownAgentOptions so the gateway
client carries repo_dir through for server-assisted discovery, and removes
the overly aggressive same-endpoint guard from writeNeutralAgentSettingsFs
that blocked legitimate same-install agent replacements (the pre-mint check
in runAddAgent is the correct guard for that case).
CHOO-2560
* fix(console): address review — writer guard, manual scan, blockedReason, addressing verdict
1. Wire the same-endpoint defence in depth in writeNeutralAgentSettingsFs:
calls existingAgentIdInSlot and throws ExistingAgentCredentialsError when
the slot holds a different agent. Callers that already verified the
overwrite pass expectedAgentId to bypass the guard. runAddAgent passes
slotAgentId from its pre-mint DB check.
2. Manual directory scan results are now merged into local state instead of
discarded — a dir outside $HOME and the server's repo_dirs stays visible
after scanning.
3. Add blockedReason field to LoadableAgent: propagated from discovery,
shown in the UI as the disable reason per row. Replaces the separate
alreadyAgent/endpointMismatch checks in the selectableAgents filter.
4. Show per-row addressing verdict when ownerName is known: "session
access: yes · rooms: policy admits only its owner, ask <owner> to widen."
5. Test coverage for the already-configured guard: exercises the path where
sameEndpointAgentId returns a non-null id and addAgent returns
already-configured. Also tests the writer guard directly.
CHOO-2560
* style(console): address Copilot findings — imports, toggle label, memo deps
Move justAddedHost exports after imports per codebase convention, hide
select-all toggle when no agents are selectable, and fix useMemo deps for
serverId by reading MobX observables outside the memo.
* fix(console): prune hidden dirs in the $HOME scan; log scan failures; doc fix
Address remaining Copilot review findings on #364:
- The bounded $HOME scan now prunes every hidden directory except .switch
(plus node_modules) — dot-trees like .cargo/.npm hold hundreds of
thousands of entries and can never contain a surfaceable working dir.
The docstring previously claimed this pruning; now the code does it.
- findSwitchAgentDirsOnHost no longer swallows exec failures silently:
a failed scan is logged so it cannot masquerade as an empty host.
- existingAgentIdInSlot doc no longer implies read errors map to null;
they throw, and callers must let them propagate.
* feat(console): explicit scan, endpoint display, details, config delete + sidebar fix in Load existing agents
- fix: reload the agents store after Load so the sidebar shows the new
agents immediately (agentEvents is main-process-only; same idiom as the
Add Agent flow)
- discovery runs on 'Scan this host', not on section expand (no unasked SSH
into a colleague's box)
- every row can expand to details: description, agent id, endpoint, dir,
provider+source, owner, found-via
- endpoint mismatches show both sides (registered against X; this server is Y)
- per-row trash deletes the on-disk .switch/agents/<name>.json (confirm
modal; host-file only, server registration untouched; disabled for agents
already loaded here)
- owner/policy line says '(you)' instead of 'ask yourself to widen' when the
viewer owns the agent
* fix(console): bridge agent CRUD events to the renderer so views stop going stale
A locally removed agent stayed 'Already loaded in this Console' in the Load
existing agents section: agentEvents is a main-process-only bus, so no
renderer store or query reacts to agent create/update/delete — each call
site had to remember its own refetch, and the remove path forgot (the load
path had the mirror bug, patched by hand earlier).
Durable fix: agents:changed channel bridged from agentEvents at startup; a
global AgentCrudEvents reactor reloads the sidebar agents store and
invalidates every load-existing-agents discovery query on any agent CRUD.
* style(console): oxfmt pass on the new loading files
* fix(console): plain remove no longer deletes an agent's files or sidecar on its host
Removing a loaded agent local-only was wiping the on-disk credentials in the
working directory (.switch/agents/<name>.json, the settings.local.json env
block, launch profiles) and killing the sidecar — on a shared host those
belong to another install, so a colleague's agent was destroyed by a remove
that promised to be local. Attach guarantees it writes nothing; remove now
mirrors it.
DeleteAgentOptions gains a required removeProvisionedFiles flag: no caller
gets a default. The remove modal offers it as an explicit opt-in checkbox
naming the exact host:dir, warns that shared-host files may belong to
another install, and the misleading 'the folder stays on the filesystem'
copy now states plainly what a plain remove does. Server teardown keeps
cleaning its own provisioned agents; removing a location forgets it without
touching disk.
* style(console): prune comments that restate code or would rot
Removed a hardcoded line reference, process-history narration in the event
bridge doc, one-liners that restate the variable they sit on, and the manual
agentsStore.load() in the load mutation that the agents:changed bridge made
redundant (with its now-inaccurate comment).
* fix(console): manual directory scan now shows results in the agent list
The agent list is gated behind scanStarted, which only the "Scan this
host" button set. A manual directory scan found agents and stored them
but left scanStarted false, so the ternary never reached the list
branch. Flip scanStarted on manual-scan success so the results render
and auto-discovery kicks in alongside them.
* fix(console): show manual-scan results without triggering auto-discovery
The previous fix flipped scanStarted on manual-scan success, which
rendered the results but also kicked off the slow host-wide discovery
as a side effect. Instead, widen the rendering gate to also check
manualAgents — the placeholder hides once either scan path has results,
and auto-discovery stays gated on the explicit "Scan this host" button.
* fix(console): remove explicit scan gate — show agents on expand
The scanStarted gate required clicking "Scan this host" before results
would render, and manual directory scans never flipped it. Remove the
gate entirely: discovery runs when the section expands, results display
immediately, and the Rescan button handles refresh. Both auto-discovery
and manual scan results appear without an intermediate step.
* fix(console): reload location manager on agent CRUD so sidebar updates
The agent-crud event reactor reloaded agentsStore but not the location
manager, so a loaded agent appeared in "Your agents" but its location
never mounted — no sidebar row, no agent page navigation.
Call getLocationManagerStore().reload() alongside agentsStore.load().
Also drop locations whose last agent was removed inside _doLoad so
stale sidebar rows don't survive a delete.
* feat(console): split discovery — cheap auto-scan, deep walk behind Alt
The $HOME walk can be slow on large VMs. Split it from the cheap
server-assisted discovery: on section expand, only check the server's
registered agent directories (instant); the full home-directory walk
is a hidden alternate action revealed by holding Alt, with an honest
slow-scan label and tooltip. The manual directory scan stays as the
precision tool between the two.
* fix(console): guard _doLoad cleanup against in-flight onboarding
A concurrent CRUD event could trigger reload() while startAgentOnboarding
had already placed an unregistered placeholder in the locations map but
before the agent existed in the DB. The cleanup loop would delete it
mid-onboarding. Skip locations in pendingCreationIds.
* feat(console): reconcile sidecar sessions after loading a remote agent
After attachConfiguredAgents creates an agent row, start the remote session
reconciler so the sidecar's existing sessions appear in the sidebar
immediately — not just the agent identity.
Uses a dynamic import for remote-watcher to avoid pulling Electron's app
module into the test environment.
* feat(console): show owner attribution on loaded agents
When an agent is loaded from another install, persist the server-side
owner name (ownerName) in the agents table so the UI can distinguish it
from agents this Console created.
Visible on the Your Agents card ("loaded · by AbelD") and the agent
detail page titlebar. Null for agents created by this install, so
existing agents are unaffected.
Includes migration 0048 adding the owner_name column.
* fix(console): don't auto-scan host on Load section expand
Expanding the "Load existing agents" section was firing discovery
immediately, which is slow on VMs with many files. Put it behind a
"Discover agents" button so the user controls when the scan runs.
Addresses Louis's feedback on PR #364.
* fix(console): always show Deep Scan button, remove Alt-key gating
Abel tested the Load UI and flagged the Alt-key reveal as a UX trap —
users don't discover hidden modifiers. Remove the altHeld state and
keydown/keyup listener, show Deep Scan next to Rescan unconditionally,
drop the "Hold Alt" hint text and the tilde from the label, and set a
tooltip explaining the scan cost.
* fix(console): show 'Already loaded' label immediately after loading an agent
The discovery query refetches via SSH after a load, so the blocked-reason
label lagged behind the action by several seconds. Optimistically update
the react-query cache and manual-agents state with the loaded names
before the background refetch lands.
* docs(console): add screenshot of Load Existing Agents expanded view
* feat(console): improve Deep Scan UX with tooltip, progress bar, and cancel
Replace the native title attribute on Deep Scan buttons with the app's
Tooltip component for visibility. Show an estimated progress bar (30s)
instead of a spinner during deep scans so the user has a sense of
liveness and expected duration. Add a cancel button to abort a scan in
progress.
* perf(rooms): index rooms.group_id (#404)
group_id is an unindexed foreign key, so listing rooms by group and the
ON DELETE SET NULL on group removal both scan the rooms table. Adds the
index, matching ix_agents_parent_agent_id.
* fix(db): rejoin the two migration heads so the service can boot (#426)
Two migrations landed in parallel off the same parent, so the chain had two
heads. Startup runs `alembic upgrade head`, which is ambiguous with more than
one, and a deployment not already sitting on one of the two refuses to boot
with "Multiple head revisions are present".
`test_single_head` already asserts this and has been failing on main since the
second one merged: each pull request was green against its own branch, where
there was one head, and nothing re-ran the suite against the merge result.
The revision is empty. Neither head carries schema the other needs.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(plugins): sync opencode embedded skill with create_room_from_yaml
The connector's SKILL.md gained the new operation but the copy the app
embeds did not, and connector-assets.test.ts rightly refuses drift
between the two.
---------
Co-authored-by: Petr Bauch <petr.bauch@sandboxquantum.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
abeldantas
added a commit
that referenced
this pull request
Sep 16, 2026
Folds the core half of work/group-templates (#407) onto the template format as it stands after #405, so one YAML file can declare a room group, several rooms under it, and directed links between them. Before: a template made exactly one room. #407 added the group shape but sat on a base from before parse_template, builtins and kickoff landed, and could no longer merge. Now: parse_template tells the shapes apart by their top-level key. A `room:` document works as before, kickoff at the top level. A `group:` + `rooms:` (+ `links:`) document gives a GroupSpec; each room carries its own `kickoff:`, and a top-level one is refused with a message saying where it goes. Params and server builtins interpolate over the whole document, mapping keys included, so an alias map written `"{bot}": helper` resolves. Rooms accept `aliases:` and export them back. provision_group creates the group row, each room in order (filed under the group, its kickoff posted as the creator), then the links; a room that fails is reported in `errors` and the rest still go ahead. POST /rooms/from-yaml returns whichever result matches the document, entity params are checked across every room, and /rooms/template-schema describes both shapes. Agents get the same through a `create_room_from_yaml` operation, acting as their owner. The 45 Console files and two migrations in #407's diff were drift from an older base and are already on main; none of them are touched here. Claude-Session: https://claude.ai/code/session_01GpmTz1MFhce8ekJoPZgqBn
Collaborator
Author
|
Folded into #409: the core half (group document, provision_group, aliases, create_room_from_yaml) is there as commit 46e4c4d, merged onto main's parse_template and kickoff shape, plus the Console side (a Use page that creates agents and rooms from one document). The Console files in this diff were drift from an older base and are already on main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Right now one template file makes one room. We add a second document in the schema (a
group:key instead ofroom:) so a single YAML file can declare a room group, several rooms inside it, and directed links between them.