Invite an agent into the room: one checkbox, read-only or full - #194
Merged
Conversation
The explicit lane beside the wake lane's automatic one: a human invites a
second agent — another claude-code, a codex, a different model — into a
spec's room to chat, critique, draft specs, or pitch in on code. One
choice at invite time, read only or full; the two modes map exactly onto
the credential tiers sail already ships and the harness enforces them.
An invite is a run. POST /v1/specs/{id}/invite (CLI: sail spec invite
<id> --agent <a> [--model m] [--full]) launches a fresh session seeded
with the spec body and the room tail (delivery ledger seeded exactly like
wake), principal <agent>/invite-<runId>, under the normal watcher and
guardrail ceiling. Two new run roles carry the contract end to end:
- invite (read only): the room lane's machinery verbatim under a new
role — viewer credential at the boundary, the harness tool-cut command
regardless of mispassed flags, no repo reservation, worktree-digest
guard on stop, stop-gate git protocol skipped. The gate lets it run
alongside anything, its own spec's live build included (the same-spec
backstop stays for automatic wakes). Offered only where the harness
can enforce it: supportsReadOnlyInvite() at the AgentCli seam, with
the refusal reason declared there so GET /v1/agents reports the same
words the launch gate refuses with (claude-code today; codex is a 400
naming full access as the lane).
- invite-full: the member credential a dispatched agent holds, nothing
more — post to the room, draft the spec body, spec create (born draft
by API default, promoted by a human), change code under the harness's
own full-permission mode. Bought with two structural payments: the
repo reservation (reserved like a build, one writer per repo always;
a held reservation refuses with the dispatch conflict vocabulary) and
a mandatory pre-launch snapshot labeled invite-<runId>, published as
snapshot_created with the spec id so it renders in the room; a failed
snapshot aborts the launch before anything runs.
Neither mode claims the spec, checks out a branch, or triggers the
review pipeline: both roles join nonTriggeringLane, the run-row
fallback, the lifecycle reactor's exclusion, and the synced-stop role
stamping — the review loop stays anchored to dispatch. Both join
SESSION_ROLES so the stop/status/reaper lanes cover dead invites.
Inviting requires the same tier as dispatching (DispatchPolicy
verbatim), so agent principals cannot invite — structurally, like the
wake lane's human-authors-only rule. Schema: runs_v6 rebuild widening
the role CHECK to both invite roles.
Review findings 01a00c6a-3072-7000 (HIGH) and 01a00c6a-3072-7001 (MEDIUM): the invite lane passed the caller's model string unvalidated into the bash -l -c agent command (shell injection past the read-only tool cut) and accepted agents whose binary is absent from the container, reserving repos and snapshotting for a session that dies on launch. startInvite now routes the model through Spec.validatedModel (422 on shell-unsafe values, same vocabulary as spec writes) and refuses with AGENT_NOT_CONFIGURED when command -v cannot find the agent's binary in the container — both before any reservation or snapshot.
The read-only lane's tool cut was additive-vulnerable: a workspace or user .claude/settings.json could merge extra Bash allow-rules into the session, and a workspace .mcp.json could launch MCP server processes past the cut. Every room invocation (wakes and read-only invites share the seam) now pins --setting-sources "" and --strict-mcp-config, so the sail-owned --settings file is the only configuration in the session. The world-readable box credential resolves to the box FDE identity — a higher tier than the viewer credential the lane holds — and Read could lift it. The sail-owned settings now deny tool-level reads of /var/lib/sail/run/box.credential; spec-CLI auth reads at the OS level and is untouched. Verified on a live container: an ambient SessionStart hook fires without the flag and not with it, while --settings hooks still fire under --setting-sources "".
…uild Two review findings on the invite lane: - startInvite skipped requireTrustedRoster, the every-lane check dispatch runs before any container work. A node whose handle is absent from the synced FDE roster but still matches a spec's assignee could reserve, snapshot, and launch an invited agent. Gate it exactly like dispatch, before reserve or snapshot. - SESSION_ROLES backed both invite reaping and singular project-level session selection. Because a read-only invite runs alongside a live build and is newer, a project-level status, log, or stop selected the invite instead of the build. Split PROJECT_SESSION_ROLES (build, adhoc, room) for latestForProjectOnNode/runningForProjectOnNode; keep SESSION_ROLES (invites included) for the reaper lanes. Invites stay addressable and stoppable by their own run id via findById + sessionRole().
Resolves the review's recurring HIGH on the read-only invite lane by first establishing what the boundary actually is, empirically, rather than by assertion. The prior code (and the disputed review comments) claimed the lane's "only file access is the Read tool" and that "every unlisted command is denied" — both false: cat/head/tail/grep are harness-permitted read commands. What is actually true, verified against claude 2.1.228 running the exact roomInvocation: (1) writes and arbitrary interpreters are refused; (2) recognized read commands are auto-approved only WITHIN the working directory — a read of any path outside ~/workspace is refused. The room/invite lane launches in ~/workspace, and every container secret (box.credential, ~/.ssh, ~/.sail/api-token, ~/.claude/.credentials.json, ~/.git-credentials, sail.db) lives outside it, so all are unreadable by default. So the boundary IS harness-enforced; the finding overstated the exposure and the agent's dispute had the right conclusion for the wrong reason. On top of that primary cwd-scope, add explicit Read-deny rules for the two highest-value credentials the user asked to harden — ~/.ssh (the Sail CLI identity) and ~/.git-credentials — beside the existing box.credential deny, so the protection does not rest solely on the cwd heuristic. Verified the glob deny blocks the SSH key at the permission-engine level. Corrects every stale/false isolation claim in AgentCli and ClaudeCodeHookConfig to state the cwd-scope as primary and the denies as belt-and-suspenders; the residual (a secret committed inside the workspace, a kernel escape) stays with the sidecar follow-up. Tightens three ProjectProvisionerTest assertions that matched the git-credential path by substring (the new deny rule now mentions it) to match the actual credential write. mvn clean verify: 2774 tests, all JaCoCo gates.
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
1 participant
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.
Spec: sail-room-invite. Mast counterpart: standardapplied/mast PR on the same branch (
agent/sail-room-invite).A room is a channel; the missing verb was inviting an agent — a second claude-code, a codex, a different model — to chat, critique, draft specs, or pitch in on code. One choice at invite time: read only or full. No new permission tiers — the two modes map onto the two credential tiers sail already ships, and the harness's own permission modes do the enforcement. Wake stays the automatic lane, unchanged; invite is the explicit one.
Design
An invite is a run.
POST /v1/specs/{id}/invite/sail spec invite <id> --agent <a> [--model m] [--full]launches a fresh session (never a resume — the point is a new participant) seeded with the spec body and room tail, ledger seeded exactly like wake, principal<agent>/invite-<runId>, one bounded turn under the normal watcher + guardrail ceiling. Two new run roles carry the contract end to end:invite(read only) — the shipped room-lane machinery verbatim under a new role: viewer credential decided at the API boundary from the run row, the harness tool-cut command inAgentSessionregardless of mispassed flags, no repo reservation, worktree-digest guard on stop, stop-gate git protocol skipped.DispatchGatelets it run alongside anything, its own spec's live build included — which is why it cannot reuse roleroom: the same-spec conflict is the wake lane's atomic backstop and stays intact for wakes. Mode support is declared at theAgentCliseam (supportsReadOnlyInvite()+ the refusal reason), soGET /v1/agentsreports the same words the launch gate refuses with; codex read-only is a 400 naming full access as its lane.invite-full— exactly the member credential a dispatched agent holds: post to the room, draft the spec body,spec create(born draft by API default; a human promotes), change code under full permissions. Bought with two structural payments: the repo reservation (reserved like a build through the sameBEGIN IMMEDIATEtransaction; a held reservation refuses with the dispatch conflict vocabulary, not a queue) and a mandatory pre-launch snapshot labeledinvite-<runId>, published assnapshot_createdwith the spec id so it renders in the room. Snapshot failure aborts the launch and releases the reservation — the payment for YOLO is not optional.Review-pipeline exclusion is structural: both roles join
nonTriggeringLane, the pipeline's run-row fallback, the lifecycle reactor's filter, and the synced-stop role stamping. The review loop stays anchored to dispatch; a full invite's pushed commits surface in the room and the next build's review sees them. Both roles joinSESSION_ROLESso stop/status/reaper cover dead invites. Inviting requires the same tier as dispatching (DispatchPolicyverbatim), so agent principals cannot invite — structural, like the wake lane's human-authors-only rule.One deliberate deviation from the spec text: the full lane's order is reserve → snapshot → launch (the acceptance bullet reads snapshot → reserve → launch). Reservation-first preserves the load-bearing "no mutation before the reservation" dispatch invariant, means the snapshot captures a quiesced tree rather than a concurrent build mid-write, and a refused invite takes no stray multi-minute dir-backend snapshot. Flagged in the spec room.
Schema:
runs_v6rebuild widening the role CHECK (append-only tail, FK-suspension handled by the runner, child-table survival covered by the existing rebuild tests).Not done, deliberately: no in-container
spec invite(agent-to-agent invites are a non-goal;LocalApiRouterkeeps its no-dispatch-routes contract); no queueing on conflict; no wake-contract changes.Tests
InviteLaunchTest(13 cases: room-contract wiring, runs-alongside-own-build, reserve→snapshot→launch order, refusal vocabulary + no stray snapshot, snapshot-failure abort + release, codex 400 / codex full, unknown agent, agent-lane refusal, locality,SailOperationsdelegation +GET /v1/agentsmodes),InvitePrincipalAccessTest(viewer vs member tiers at the local socket; full invitespec createborn draft and attributed),InvitePromptTest, plus additions toDispatchGateTest,RunStoreTest,SchemaManagerTest,AgentCliTest,AgentSessionTest(harness-restricted even when full-permissions is mispassed),SailStopGateTest(real bash: invite skips the git protocol, invite-full keeps it),ReviewPipelineControllerTest,SpecLifecycleReactorTest,RoomWakeReactorTest(read-only invites get the commit guard; full invites don't),SyncTransitionEventsTest,ApiRouterTest.mvn clean verifygreen — 2771 tests, all JaCoCo gates including api.* 100%.