feat(agents) 1/5: a session has an origin and a name, and agents are declared - #181
Merged
Conversation
A task is an agent session. What separates the orchestrator's sessions from the ones a person spins up is not their type but their *provenance*, so sessions gain the two identity fields that make one row taxonomy possible (topology plan B3, spec §2.2): - `origin: SessionOrigin` — `Orchestrator` (auto-created by a dispatch, §4.1) or `User` (spun up from the UI). Fixed at creation, for life. - `name: Option<String>` — what the person who started it called it; `None` for a dispatched session, which the UI labels from its task. Origin is deliberately *not* ownership. Control (`HarnessControl`) says who may start a turn right now and moves on every take and hand-back; origin says who started the session and never moves. The layout says so too: origin sits in the pty handle's immutable `SessionMeta` beside the launch anchor, while control stays an atomic bit. A dispatched session an operator holds is `origin: orchestrator, control: user` — both true at once, and only control gates dispatch. `user_spawned` was the precursor of this field and is replaced by it rather than duplicated, so the two can never disagree. Set on both creation paths: the executor's `session_for` stamps `Orchestrator`, the operator's `open_unmanaged` stamps `User`. The SDK manager takes it from `OpenSession`, and auto-creation derives it from the turn's own provenance (`TurnOrigin::session_origin`). The binding registry carries a `SessionIdentity` beside its `WorkspaceContext` and keeps it across a rebind, so a resumed conversation does not come back unnamed. No wire change; `open_unmanaged_named` is the seam the picker's future name prompt lands on. Co-Authored-By: Claude <noreply@anthropic.com>
An agent is `harness × workspace` on a host, and until now nothing wrote
one down: `[fleet]` held the whole `Host → Harness → Workspace → Agent`
chain and was display-only, while the roster synthesized one entry per
machine. A laptop running claude *and* codex advertised a single worker
whose second CLI survived only as prose in its description — and prose is
not something a dispatch can target.
Declarations become the source of the roster:
- `AgentDeclaration` (`runtime/fleet/declaration.rs`): `agentId`, `hostId`,
`harness`, `workspace {path, type}`, optional `name`, `roles`, and a
`strategy` that *derives* concurrency (`checkout` ⇒ 1 session, serial).
`worktree` exists as a variant so a config naming it still parses, but
it is not selectable — nothing provisions a worktree yet.
- Persistence in `[fleet].agentDeclarations`, written through the same
`persist_setting` path the rest of the section already uses, with
create/update/remove and the queries the UI and the roster read back
(`config/agent_declarations.rs`).
- `spec_for` becomes `specs_for`: one `WorkerSpec` per declared agent
instead of one per daemon. A host that has declared nothing seeds one
agent per detected CLI, and the default one keeps the host's own id — so
an install that predates declarations advertises exactly the entry it
had, and nobody's roster goes empty.
- `HubWorker` gains `host_id` and strategy-derived `max_sessions`, and its
workspace becomes `{path, type}`; `control`/`handoff` are untouched, and
the advert is unchanged — the path still rides `metadata.workspace`, and
neither `hostId` nor `maxSessions` is emitted yet.
- Roles finally have a source: they are declared on the agent and reach
`metadata.roles` through the spec, where the mapping used to hard-code an
empty list. The remembered roster now persists them too.
- A dispatch's task is filed under the agent it named rather than the
first entry at that address, which several agents now share.
Model and data only: no wire-shape change, no UI.
Co-Authored-By: Claude <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
sanil-23 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR adds persisted agent declarations, structured workspace and roster metadata, multi-agent local host advertising, session provenance tracking, optional session names, and SDK and TUI tests. ChangesAgent declarations and configuration
Multi-agent roster integration
Session provenance and naming
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
This was referenced Aug 5, 2026
Merged
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tui/src/event_loop/cmd_dispatch/mod.rs (1)
216-242: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRegister every declared agent for a dynamically started host.
spawnreturns all localWorkerSpecvalues, but this handler takes only the first one. All sibling agents remain absent from the live roster until restart. The address-basedWorkerOp::Addalso cannot preserve each agent ID, role, workspace, or capacity.Add an agent-keyed runtime roster operation, then register every returned specification through it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tui/src/event_loop/cmd_dispatch/mod.rs` around lines 216 - 242, Update the dynamic host-start handling in the spawn branch to process every returned WorkerSpec instead of selecting only the first. Add and use an agent-keyed runtime roster operation that preserves each specification’s agent ID, role, workspace, and capacity, replacing the address-based WorkerOp::Add path so every declared agent is immediately registered in the live roster.
🧹 Nitpick comments (2)
src/tui/src/ui/harness_pane/tests/session.rs (1)
55-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the visible test helper.
Add a
///comment toharnesses. Line 55 changes it to apub(super)function. The Rust guidelines require documentation for every public function.Proposed fix
+/// Build local harnesses backed by `sessions` for harness-pane tests. pub(super) fn harnesses(sessions: PtyManager) -> LocalHarnesses {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tui/src/ui/harness_pane/tests/session.rs` at line 55, Add a concise Rust doc comment (`///`) immediately above the pub(super) test helper harnesses, describing its purpose and documenting the newly visible function without changing its implementation.Source: Coding guidelines
src/sdk/src/hub/tests/roster.rs (1)
607-694: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit
src/sdk/src/hub/tests/roster.rs. The file contains 537 non-comment, non-blank lines, exceeding the 500-line limit. Move related tests into separate modules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sdk/src/hub/tests/roster.rs` around lines 607 - 694, Split the oversized roster test file into focused modules, moving the related declaration/advert, capacity-default, and lane-resolution tests around worker_from_spec and lane_id into an appropriate separate test module. Preserve all test behavior, shared helpers, imports, and module visibility while keeping src/sdk/src/hub/tests/roster.rs under the 500 non-comment, non-blank line limit.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sdk/src/config/agent_declarations_tests.rs`:
- Around line 20-35: Add a JSON-file round-trip test alongside
a_declared_agent_round_trips_through_the_config_file, using a .json scratch path
and the same declare_agent/load_agent_declarations flow. Assert the declared
agent is written and reloaded with matching fields, including max_sessions(), to
cover the non-TOML JSON parsing branch.
In `@src/sdk/src/control_socket/server/hub_ops.rs`:
- Around line 120-122: Normalize workspace values in both projections by
replacing direct WorkspaceRef.path access with
worker.workspace_path().map(str::to_owned). Apply this in
src/sdk/src/control_socket/server/hub_ops.rs lines 120-122 for
FleetWorker.workspace and src/sdk/src/runtime/openhuman/worker_ops.rs lines
36-38 for WorkerInfo.workspace, preserving omission of blank or
whitespace-padded paths.
In `@src/sdk/src/runtime/fleet/declaration.rs`:
- Around line 118-122: Update max_sessions in
src/sdk/src/runtime/fleet/declaration.rs lines 118-122 so both Checkout and the
unsupported Worktree strategy return a capacity of one; retain
WORKTREE_MAX_SESSIONS only if it is no longer used elsewhere. Update the
assertion in src/sdk/src/runtime/fleet/declaration_tests.rs lines 7-14 to expect
one session for Worktree instead of parallel capacity.
In `@src/sdk/src/sessions/manager/turns.rs`:
- Around line 189-194: Update the session recreation flow around ensure_session
to resolve the registry plan first and, when resuming an existing binding, reuse
its persisted identity rather than deriving it from
request.origin.session_origin(). Keep TurnOrigin-based identity only for
sessions without a binding, preserving the resumed user session’s origin and
name after close; add a regression test covering close → frame turn → resumed
user origin and name.
In `@src/sdk/src/sessions/registry/types.rs`:
- Around line 48-52: Prevent bound session origins from being changed through
SessionIdentity updates: either make SessionIdentity::origin non-public or
update SessionRegistry::record_identity to retain the previously stored origin.
Add or use a name-only update API for changing the mutable display name without
replacing origin.
In `@src/tui/src/local_host/mod.rs`:
- Around line 353-370: Update the local-host dispatch flow so each selected
agent runs with its declared workspace instead of the host-wide
options.workspace: propagate the selected agent identity from shared-address
dispatch into executor creation and construct or select a PtySessionExecutor
using that declaration’s workspace. Alternatively, reject declarations with
differing workspaces before dispatch, but preserve the advertised workspace
contract. Add a dispatch test covering two declarations on one host with
different workspaces.
- Around line 343-351: Update the label-generation logic in the declaration
mapping around the single/multiple declaration handling so sibling declarations
sharing the same harness receive distinct labels by including their agent_id or
workspace; retain existing labels when harnesses are unique. Add a regression
test covering multiple local declarations with the same harness and assert that
their generated labels differ.
---
Outside diff comments:
In `@src/tui/src/event_loop/cmd_dispatch/mod.rs`:
- Around line 216-242: Update the dynamic host-start handling in the spawn
branch to process every returned WorkerSpec instead of selecting only the first.
Add and use an agent-keyed runtime roster operation that preserves each
specification’s agent ID, role, workspace, and capacity, replacing the
address-based WorkerOp::Add path so every declared agent is immediately
registered in the live roster.
---
Nitpick comments:
In `@src/sdk/src/hub/tests/roster.rs`:
- Around line 607-694: Split the oversized roster test file into focused
modules, moving the related declaration/advert, capacity-default, and
lane-resolution tests around worker_from_spec and lane_id into an appropriate
separate test module. Preserve all test behavior, shared helpers, imports, and
module visibility while keeping src/sdk/src/hub/tests/roster.rs under the 500
non-comment, non-blank line limit.
In `@src/tui/src/ui/harness_pane/tests/session.rs`:
- Line 55: Add a concise Rust doc comment (`///`) immediately above the
pub(super) test helper harnesses, describing its purpose and documenting the
newly visible function without changing its implementation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bcb719e2-6b7a-42ed-a3e3-e5920678ffa6
📒 Files selected for processing (75)
config.example.tomlsrc/sdk/src/config/README.mdsrc/sdk/src/config/agent_declarations.rssrc/sdk/src/config/agent_declarations_tests.rssrc/sdk/src/config/mod.rssrc/sdk/src/config/persist.rssrc/sdk/src/config/types/fleet.rssrc/sdk/src/config/types/mod.rssrc/sdk/src/control_socket/server/hub_ops.rssrc/sdk/src/hub/boot/mod.rssrc/sdk/src/hub/boot/types.rssrc/sdk/src/hub/handle/handoff.rssrc/sdk/src/hub/roster/mod.rssrc/sdk/src/hub/roster/types.rssrc/sdk/src/hub/socket/task_run.rssrc/sdk/src/hub/tests/handoff_advert.rssrc/sdk/src/hub/tests/roster.rssrc/sdk/src/runtime/fleet/README.mdsrc/sdk/src/runtime/fleet/declaration.rssrc/sdk/src/runtime/fleet/declaration_tests.rssrc/sdk/src/runtime/fleet/mod.rssrc/sdk/src/runtime/mod.rssrc/sdk/src/runtime/openhuman/tests.rssrc/sdk/src/runtime/openhuman/worker_ops.rssrc/sdk/src/sessions/manager/mod.rssrc/sdk/src/sessions/manager/turns.rssrc/sdk/src/sessions/manager/types.rssrc/sdk/src/sessions/mod.rssrc/sdk/src/sessions/ops/mod.rssrc/sdk/src/sessions/registry/behavior.rssrc/sdk/src/sessions/registry/mod.rssrc/sdk/src/sessions/registry/types.rssrc/sdk/src/sessions/tests/manager_tests/lifecycle.rssrc/sdk/src/sessions/tests/manager_tests/mod.rssrc/sdk/src/sessions/tests/ops_tests.rssrc/sdk/src/sessions/tests/registry_tests.rssrc/sdk/src/sessions/types.rssrc/tui/examples/pty_load.rssrc/tui/src/app_loop.rssrc/tui/src/event_loop/cmd_dispatch/mod.rssrc/tui/src/hub_relay/mod.rssrc/tui/src/local_host/mod.rssrc/tui/src/local_host/tests/declarations.rssrc/tui/src/local_host/tests/extras.rssrc/tui/src/local_host/tests/lifecycle.rssrc/tui/src/local_host/tests/mod.rssrc/tui/src/local_host/types.rssrc/tui/src/ui/app/changes/baseline_tests.rssrc/tui/src/ui/app/rail.rssrc/tui/src/ui/app/render/agents/rail/tests.rssrc/tui/src/ui/app/render/settings/status_line.rssrc/tui/src/ui/harness_pane/spawn.rssrc/tui/src/ui/harness_pane/tests/mod.rssrc/tui/src/ui/harness_pane/tests/origin.rssrc/tui/src/ui/harness_pane/tests/session.rssrc/tui/src/worker/app/tests/helpers/mod.rssrc/tui/src/worker/executor/run.rssrc/tui/src/worker/executor_tests/basic.rssrc/tui/src/worker/executor_tests/live.rssrc/tui/src/worker/executor_tests/sessions.rssrc/tui/src/worker/pty/handle/control.rssrc/tui/src/worker/pty/handle/lifecycle.rssrc/tui/src/worker/pty/handle/state.rssrc/tui/src/worker/pty/handle/types.rssrc/tui/src/worker/pty/manager/open.rssrc/tui/src/worker/pty/manager/session.rssrc/tui/src/worker/pty/mod.rssrc/tui/src/worker/pty/tests/control.rssrc/tui/src/worker/pty/tests/mod.rssrc/tui/src/worker/pty/types.rssrc/tui/tests/e2e_local_harness_pane.rssrc/tui/tests/e2e_screen_stream.rssrc/tui/tests/feature_harness_control.rssrc/tui/tests/feature_harness_handoff.rssrc/tui/tests/feature_paste/attached.rs
Comment on lines
+353
to
+370
| let workspace = declaration.workspace.clone(); | ||
| WorkerSpec { | ||
| id: declaration.agent_id.clone(), | ||
| host_id: host_id.to_string(), | ||
| address: host_id.to_string(), | ||
| name: label, | ||
| description: format!( | ||
| "{} on this machine · {}", | ||
| declaration.harness, workspace.path | ||
| ), | ||
| harness: declaration.harness.clone(), | ||
| // The one placement this process actually knows: the agent works | ||
| // in this directory. Declaring it is what gives the orchestrator | ||
| // a placed agent rather than a bare one it treats as having | ||
| // nowhere to work. | ||
| workspace: Some(workspace), | ||
| roles: declaration.roles.clone(), | ||
| max_sessions: declaration.max_sessions(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Run each declared agent in its declared workspace.
specs_for advertises declaration.workspace, but start_at creates one PtySessionExecutor from options.workspace for the entire host. The shared-address dispatch path selects a host address, not a declaration workspace. Agents declared for different repositories will therefore run in the host workspace while advertising another workspace.
Route the selected agent identity into executor selection, or reject declarations whose workspace differs from the host workspace. Add a dispatch test with two declarations on one host and different workspaces.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tui/src/local_host/mod.rs` around lines 353 - 370, Update the local-host
dispatch flow so each selected agent runs with its declared workspace instead of
the host-wide options.workspace: propagate the selected agent identity from
shared-address dispatch into executor creation and construct or select a
PtySessionExecutor using that declaration’s workspace. Alternatively, reject
declarations with differing workspaces before dispatch, but preserve the
advertised workspace contract. Add a dispatch test covering two declarations on
one host with different workspaces.
…tity Review fixes on the agent-declaration model. An agent may no longer declare a workspace its host does not run in. A host binds one address and serves every agent on it from the single executor it started in its own directory — a task frame carries no agentId to select on — so two agents declared for two repositories both ran in the host's while advertising two. Refused at start-up, by name, pointing at the [[hosts]] entry that does give a second directory; the advert now always names the directory the executor actually launches in. Per-agent workspaces need the selected agent's id on the wire, which is a protocol change and not this branch's. Siblings that share a harness are labelled by their agent id: "this device · claude" twice named neither. Worktree capacity drops to one session. Nothing provisions a worktree yet, so advertising four scheduled four concurrent sessions into one checkout — the collision the strategy exists to prevent, reached by declaring the fix. A session's origin is now immutable in fact, not only in prose: the field is private, record_identity establishes it once and preserves it afterwards, and rename() is the name-only path. A resumed binding's identity also wins over the turn that reopened it, so a closed user session recreated by a task frame keeps its owner and its name instead of coming back as an unnamed orchestrator one. Both hub worker projections read the workspace through workspace_path(), so a blank declaration is absent everywhere it is absent in the advert. JSON config persistence of declarations is now covered. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
sanil-23 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Collaborator
Author
|
@coderabbitai review |
|
Collaborator
Author
|
@coderabbitai review |
|
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
2 participants
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.
Stack 1/5 — base
main. Depends on: nothing.split/1-agent-modelmainsplit/2-tree-uisplit/1-agent-modelHost → Agent → Sessiontreesplit/3-vocabularysplit/2-tree-uisplit/4-wiresplit/3-vocabularyhosts[], per-agenthostId/maxSessions, resultsessionIdsplit/5-controlsplit/4-wireEach PR's diff shows only its own layer.
split/5-control's tree is byte-identical tobcc61e63, the merge commit on #180.This is the first of five stacked PRs that replace #180 (
feat/agent-rail, 230 filesacross 11 commits). CodeRabbit skips anything over 150 files and no human can review a
diff that size, so the branch was linearised onto
mainand cut at commit boundariesinto five layers that each build and test on their own. #180 stays open, unchanged,
as the reference for the whole change — please do not close it. The five branches
together are byte-identical to its merge commit
bcc61e63.What this layer does
The data model, with no UI and no wire-shape change.
feat(sessions): give a session an origin and a nameA task is an agent session; what separates the orchestrator's sessions from the ones a
person spins up is provenance, not type. So a session gains two identity fields:
origin: SessionOrigin—Orchestrator(auto-created by a dispatch) orUser(spunup from the UI). Fixed at creation, for life.
name: Option<String>— what the person who started it called it;Nonefor adispatched session, which the UI labels from its task.
Origin is deliberately not ownership.
HarnessControlsays who may start a turnright now and moves on every take and hand-back; origin says who started the session and
never moves. The layout says so too — origin sits in the pty handle's immutable
SessionMeta, control stays an atomic bit.user_spawnedwas the precursor oforiginand is replaced by it rather than duplicated, so the two can never disagree.
feat(fleet): declare agents and build the local roster from themAn agent is
harness × workspaceon a host, and until now nothing wrote one down.AgentDeclarationbecomes the source of the roster: persisted under[fleet].agentDeclarations,spec_forbecomesspecs_for(oneWorkerSpecperdeclared agent rather than one per daemon),
HubWorkergainshost_idandstrategy-derived
max_sessions, and roles finally have somewhere to persist to.What a reviewer should look at closely
originvscontrol. The invariant is that a dispatched session an operator holdsis
origin: Orchestrator, control: User— both true at once, and onlycontrolgatesdispatch. Anywhere
originis read to decide behaviour rather than labelling wouldbe a bug.
specs_for. A host that has declared nothing seeds one agentper detected CLI, and the default keeps the host's own id, so an install predating
declarations advertises exactly the entry it had. This is the path that stops an
existing user's roster going empty on upgrade.
strategyderives concurrency (checkout⇒ 1 session, serial).worktreeparsesbut is not selectable — nothing provisions a worktree yet.
metadata.workspace, and neitherhostIdnormaxSessionsis emitted. That lands instack 4/5.
Validation
Result on this branch:
fmt,clippy -D warnings,check, andbuildall clean.testreports 3810 passed, 5 failed, 14 ignored on macOS. All five failures arepre-existing on
mainand unrelated to this stack:daemon::providers::acp::tests::execution::a_new_acp_session_is_reported_before_the_task_completesdaemon::providers::tests::direct_runs_report_the_session_before_workspace_context(both are idle-watchdog timing tests; they pass in isolation)
worker::pty::tests::session::a_launch_root_preserves_trailing_whitespaceworker::pty::tests::session::a_session_snapshots_head_before_the_harness_can_commitworker::pty::tests::session::an_unborn_repository_records_its_root_without_a_launch_commit(the last three are the macOS
/varvs/private/varsymlink)Coverage was deliberately not run locally; CI owns that gate.
Summary by CodeRabbit