Fleet placement requester: spawn an agent on any available node (#411)#414
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
khaliqgant
added
the
no-agent-relay-review
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pear already serves spawn:<cli> as a fleet node but never asks the relay placement engine to place an agent for itself. Add the requester path: - BrokerManager.placeAgent(): dispatch a spawn onto an eligible fleet node via messaging.placement.spawn, using a dedicated agent-scoped pear-requester-<projectId> identity (placement invoke requires agent scope; workspace-key alone is read-only). Maps the four RelayPlacementError codes to clear messages; any-node placement fails fast so "no eligible node" is an instant message, never a silent hang. - BrokerManager.listNodes(): fleet-node roster for the picker UI (name/live/load/capabilities), flagging this machine's own node. - IPC broker:place-agent (structured outcome, no thrown error for the expected placement failures) + broker:list-nodes; preload + PearAPI + mock parity. A placed remote agent is reachable over relay chat; its raw terminal is owned by the target node's broker and has no relay transport yet — the result carries `local` so the UI can present a chat-first surface for remote landings (acceptance #2b, tracked as an upstream relay gap). Unit coverage for the error-message + node-summary mapping; existing broker suite green (105). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rs (#411) Add tests/placement/rung1-cross-node.mts — a repeatable Rung-1 gate that stands up two isolated brokers, enrolls B as a pear fleet node, and places a REAL claude agent on B, proving: placement lands on B, B owns the PTY, the requester broker does NOT (the empirical #2b proof that a placed agent's raw terminal has no relay transport), chat reachability, and the full error matrix (no-eligible / capability_mismatch / node-death) — each a clear message, never a hang. Mandatory failsafe: the script refuses a no-host placement whenever any foreign spawn:claude node (e.g. the live `pear` node) is in the target workspace, degrading to targeted placement on B (which can only resolve to B) or hard-aborting under PLACEMENT_E2E_REQUIRE_HERMETIC=1 — so a real spawn can never land on someone else's broker. Move the pure placement helpers (BrokerPlacementError, buildPlacementMessage, placementRequesterName, toBrokerNodeSummary) into placement.ts so the Node gate script can import them without pulling in electron; broker.ts imports and re-exports them. Unit test retargeted; 12/12 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#411) Add a "Run on" selector to the Spawn Agent dialog: - "This Mac" (default) → the proven direct local spawn path, unchanged. - "Any available node (may include this Mac)" and any live remote node → the placement engine via placeProjectAgent → broker.placeAgent. A local landing integrates exactly like a direct spawn (terminal attached + tracked). A remote landing has no local PTY — its raw terminal has no relay transport yet (acceptance #2b) — so the dialog shows a chat-first notice ("reachable via chat; terminal view for remote nodes isn't available yet") and never opens a broken terminal pane. Placement failures surface the main-process per-code message verbatim. Rung-1 #2a: prove relay chat REACHABILITY deterministically (placed agent in the relay roster + relay accepts/routes a DM to it) instead of depending on a fresh claude composing an autonomous reply from behind its trust prompt — the full LLM round-trip is kept as a logged bonus. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Idempotent additive fleet-node enrollment for real Mac minis over BatchMode SSH (enroll-mini.sh) + a from-Mac targeted placement proof (place-from-mac.mts). The enroll script is additive-only: unique instance pear-fleet-<host>, non-default port, dedicated state dir, workspace-key join via env (never argv), before/after capture, documented state-dir-scoped teardown. MANDATORY FAILSAFE #2 (project-root reaper guard): agent-relay node up runs killOrphanedBrokerProcesses(projectRoot), which reaps every broker whose CWD is projectRoot; findProjectRoot() walks up for markers incl .agentworkforce/relay, so a markerless dir under $HOME resolves projectRoot=$HOME and kills every $HOME-rooted broker (real cross-tenant incident on a busy mini -> relay#1328). Fix: pin AGENT_RELAY_PROJECT to the unique per-instance dir + marker, and a hard preflight that refuses to start node up unless the effective project root is strictly inside our sandbox (never $HOME/an ancestor/an existing broker's CWD). Proven on finn-mini: 6/7 (placement lands on mini, mini owns PTY, live broker 404s, chat #2a roster+accepted delivery + BONUS full LLM round-trip via PTY). Fix validated end-to-end on this Mac: guard refuses $HOME; a real pinned enroll leaves the live pear broker untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ffort roster de-reg (#411) The authoritative no-leak proof is the mini broker reporting agent_not_found for the placed agent (its worker/PTY is gone). Workspace-roster de-registration via relay.agents.delete is best-effort: eventually-consistent and the backend can refuse on an FK constraint even though the agent is stopped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fa86dba to
6baf646
Compare
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6baf64683c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const outcome = await pear.broker.placeAgent(project.id, { | ||
| cli, | ||
| ...(node && node.trim() ? { node: node.trim() } : {}), | ||
| ...(customName?.trim() ? { name: customName.trim() } : {}), | ||
| ...(customModel?.trim() ? { model: customModel.trim() } : {}) |
There was a problem hiding this comment.
When a remote or any-node placement is selected, the validated root is never included in this request, nor is a repo constraint supplied. The fleet spawn handler consequently defaults to the target node's own advertised options.cwd, and listNodes exposes every capable node in the shared workspace. With multiple project roots or a node serving another checkout, the agent can therefore start in a different repository despite the dialog's “Spawn into” selection, risking work against the wrong codebase.
Useful? React with 👍 / 👎.
| } else { | ||
| // Remote landing — no local PTY. Reachable over relay chat; do NOT open a | ||
| // (would-be broken) terminal tab. The caller surfaces a chat-first message. | ||
| useUIStore.getState().openTab({ kind: 'agents', projectId: project.id }) |
There was a problem hiding this comment.
When result.local is false, this branch neither adds the placed agent to any renderer state nor creates/selects a direct-message conversation; it only opens the agents tab. ComposeBar builds its recipient choices exclusively from useAgentStore().agents, while the remote worker is absent from the requester's local broker snapshot, so after the notice the user cannot select or message the supposedly chat-reachable agent through Pear.
Useful? React with 👍 / 👎.
| private async getPlacementRelay(session: BrokerSession): Promise<AgentRelay> { | ||
| if (session.placementRelay) return session.placementRelay |
There was a problem hiding this comment.
If two listNodes/placeAgent calls arrive before the first cache fill—such as the doubled roster effect under the existing React StrictMode or calls from multiple windows—both pass this check and invoke registerOrRotate for the same identity. The second rotation invalidates the first token, so one caller can immediately issue its roster or placement request with a revoked token and fail intermittently. Cache a keyed in-flight promise and clear it safely with the session, as required by the repository's duplicate-event lifecycle guidance.
Useful? React with 👍 / 👎.
1 participant
Closes #411 (requester side). Likely closes #126 into this — lead's call (this is the generalized form of "spawn/connect to a broker on any machine": placement is the connection/UI layer, generalized from a specific host to any eligible node).
What this adds
Pear was already a fleet node (serves
spawn:<cli>) but never asked the relay placement engine to place an agent for itself. This wires the requester side:BrokerManager.placeAgent()— dispatches a spawn onto an eligible fleet node viamessaging.placement.spawn, using a dedicated agent-scopedpear-requester-<projectId>identity (placement invoke needs agent scope; workspace-key alone is read-only). Maps all fourRelayPlacementErrorcodes to clear messages; any-node placement fails fast so "no eligible node" is an instant message, never a silent hang.BrokerManager.listNodes()— fleet-node roster for the picker (name/live/load/caps,isSelf).broker:place-agent(structured outcome — expected placement failures return data, don't throw) +broker:list-nodes; preload +PearAPI+ mock parity.No
execUrl/box path is used or added. Helpers live in an electron-freeplacement.tsso the E2E harness can import them.Acceptance → evidence (Rung-1 gate,
tests/placement/rung1-cross-node.mts, 9/9)Real claude, two isolated brokers sharing one workspace, agent placed on node B:
ack.node==B,handlerNodeIdset, invocation completedterminal.output/terminal.screen). Filed AgentWorkforce/relay#1327. Remote agents get the chat-first surface until it lands.placement_ttl_expired"No node advertises claude right now." (documented; no silent hang)capability_mismatch(targeted node lacking cap) verifiedCleanup verified after every run; the live dev broker (instance
pear, port 3889) was never touched.Isolation finding (for the record)
The hermetic local-workspace variant (cloud-auth-invisible brokers self-provisioning a local workspace) was attempted and does not hold: a spawned broker resolves cloud auth from outside
$HOMEand joins the operator's default workspace, where the livepearnode is eligible. A mandatory failsafe was added — the gate refuses a no-host placement whenever any foreignspawn:claudenode is present (so a real spawn can never land on a live broker) and degrades to safe targeted placement on B. The no-host least-loaded selection is therefore proven deterministically only in a dedicated workspace; the cross-node mechanics are proven via targeted placement here. (Also: a bare v10 broker advertisesspawn:*, so even a clean two-broker workspace makes no-host nondeterministic between requester and target.)Product note
"Any available node" includes the user's own Mac (usually least-loaded → often resolves local); the option is labelled accordingly.
Rung-2 — real Mac minis over SSH (
tests/placement/rung2/)Enrolls each mini as an additive, isolated fleet node into the live operator workspace (the workspace the running Pear app's broker
pearis in), then places a real agent from this Mac targeted at that mini and proves it lands + is reachable. Deliverables:enroll-mini.sh— idempotent, additive-only enrollment run overssh -o BatchMode=yes: enumerates existing brokers first and never touches them; unique instancepear-fleet-<host>, non-default port (AGENT_RELAY_BROKER_PORT), dedicated--state-dir; workspace-key join via env, never argv (kept out ofps); before/after state capture; documented, state-dir-scoped teardown (node down --state-dir …, never--all).place-from-mac.mts— requester exactly asBrokerManager.placeAgent(workspace key pulled from the livepearbroker/api/session, agent-scopedpear-requester), placing targeted at the mini node.Both minis — PROVEN (all core proofs green on finn AND sf)
Enrolled
pear-fleet-finn(10.6.3 headless) andpear-fleet-sf(10.6.2 headless), each on a non-default port with a dedicated state dir, joined the operator workspace, advertisingspawn:{claude,codex,gemini,opencode}. Existing brokers/agents on both minis left untouched (finn: ~50 codex agents + 4 brokers on :3889–3892; sf:sandbox-swap:3889 +sf-mini:3890).node:pear-fleet-<host>,spawn:codex)spawn:codexin operator wsack.node== the mini)pearbroker does not own the PTY (#2b, cross-machine)agent_not_foundagent_not_foundThe #2a round-trip is stronger than Rung-1: both minis' codex is authenticated, so the placed agent actually surfaced the probe token in its PTY (not just roster+accept). #2b is reconfirmed cross-machine on both: the mini owns the terminal, the requester's live
pearbroker 404s it. After release, each mini's broker reportsagent_not_foundfor the placed agent (no live leak). Workspace-roster de-registration (relay.agents.delete) is best-effort: sf's row deleted cleanly; finn's row hit a backend FK constraint (Failed query: delete from "agents") and lingered as a stopped roster record — a backend bookkeeping quirk, not a running-agent leak or a placement defect.Version-skew findings
~/.agentworkforce/relay/bin/agent-relay = 10.6.3— matches the requester, no skew. (/opt/homebrew/bin/agent-relay = 10.6.0is a node-script that only runs in a login shell; mise shims are dead — no global version set. Documented in the script's binary-detection.)= 10.6.2(minor skew vs 10.6.3).sf-mini — incident + mandatory failsafe #2 (this is why the enroll script is what it is)
The first sf enroll attempt killed sf's live production brokers. Root cause (filed upstream: AgentWorkforce/relay#1328):
agent-relay node uprunskillOrphanedBrokerProcesses(projectRoot), which terminates every broker whose CWD ==projectRoot;findProjectRoot()walks up for markers including.agentworkforce/relay, so a markerless workdir under$HOMEresolvesprojectRoot=$HOMEand reaps every$HOME-rooted broker. Unique instance/port/state-dir do not isolate this — it keys off project root.Fix (shipped in
enroll-mini.sh), two layers:AGENT_RELAY_PROJECTto the unique per-instance dir (findProjectRoot honors it first) + drop a marker so the walk-up stops there even if the env is stripped →projectRootcan never be$HOME.node upunless the effectiveprojectRootis strictly inside the dedicated sandbox — never$HOME, never an ancestor of$HOME, never any existing broker's CWD.Fix validated on this Mac (live
pearbroker present throughout): guard PASSes on a pinned root; guard REFUSES a markerless$HOMEsubdir (reproduces the incident); a real pinned enroll (throwaway instance, port 39160) produced zero "Killing orphaned" lines, came up isolated, and left the livepearbroker (:3889) alive + responsive; clean teardown after.Supervised sf re-attempt with the fixed script — additive, verified by strict bracketing. With sf's brokers restored, the sf enroll was bracketed BEFORE/AFTER on process inventory:
sandbox-swap@3889 (pid 2992) +sf-mini@3890 (pid 3015); 7 relay procs.pear-fleet-sfcame up RUNNING + CONNECTED.sandbox-swap@3889 (pid 2992) +sf-mini@3890 (pid 3015) still alive, pluspear-fleet-sf@39151 (pid 6953). PIDs{2992,3015}→{2992,3015,6953}.pear-fleet-sf+ its broker process — nothing removed, nothing else changed. The fix holds on the exact machine the incident occurred on.Rung-3
Rung-3 (cloud sandbox) deferred to
AgentWorkforce/cloud#2683— pear side is ready: the enroll script is surface-agnostic and any sandbox that can run it + reach relaycast becomes a placement target.🤖 Generated with Claude Code