HOMELAB-1557: feat(subagents): attach socket + steer — swarm-attach CLI, manifest, human-intervention announcements #24

Merged

aaron merged 1 commit from plane/HOMELAB-1557-phase2 into main 2026-09-19 04:13:32 +00:00

Conversation 1 Commits 1 Files changed 19 +3244 -15

aaron commented 2026-09-19 03:48:16 +00:00

Owner

Copy link

What

Phase 2 of HOMELAB-1555 (ticket HOMELAB-1557 Phase 2, design Slices 3+4): the human interaction channel into running subagents.

Slice 3 — attach socket + manifest: src/core/attach/attach-server.ts — 0600 same-uid unix socket attach.sock beside the task store; newline-JSON RPC (list | steer | abort | result); subscribe reserved stub → Phase 3 error; malformed lines get an error reply and the connection survives; root-only (depth 1 binds nothing); stale-sweep of dead-pid sockets at startup; bind-generation counter closes superseded binds on session switch/shutdown (race found by tests). attach-manifest.ts — 0600 attach.json keyed rootSessionId+pid with kill(pid,0) liveness, debounced atomic writes (tmp+rename), removed on parent exit. Steer/abort/result delegate to existing AgentManager.steer()/abort(); steers to finished agents fail cleanly with curated status messages; queued aborts fail their store task.

Slice 4 — CLI + RPC + announcements: bin/swarm-attach.js (plain-JS CommonJS via nested bin/package.json — the package is type:module) with swarm-list/steer/abort symlink dispatch; manifest discovery across parents (live-pid choice, --swarm-root/--socket overrides); abort prompts without --yes. human-announce.ts — every successful human steer announced once (pi.appendEntry("subagents:human_intervention") + subagents:steered {source:"human"}); announcement failure never fails the steer; failed steers never announce. cross-extension-rpc.ts — subagents:rpc:list|steer, PROTOCOL_VERSION → 3, and caller provenance on rpc:spawn (caller:{extension,token} from subagents:ready/rpc:handshake, fail-closed).

Reviewer focus (implementer-flagged)

1. Socket/manifest 0600 + root-only bind-generation race in index.ts.
2. Steer/abort error semantics reuse vs. new state.
3. rpc:spawn provenance is a breaking contract change — external callers (pi-tasks) must handshake; judge fail-closed vs. deprecation window.
4. Announcement failure isolation; CLI arg parsing (-- escape, symlink dispatch).

Known flake (orchestrator-observed)

One timing-sensitive test failed once in 2 local runs (662/663 → 663/663 on rerun, tree clean). If CI/review sees a single transient failure, re-run before treating as blocker; worth identifying the timing assumption.

Verify

• tsc --noEmit clean
• vitest run: 68 files / 663 tests pass (84 new) — every design Slice 3+4 Automated Verification item is a named test
• Live acceptance (ticket): steer from second pane changes agent behavior within one turn; steer-to-finished fails cleanly; manifest removed on exit; 0600 enforced

Refs HOMELAB-1557 (Phase 2), epic HOMELAB-1555.

## What Phase 2 of HOMELAB-1555 (ticket HOMELAB-1557 Phase 2, design Slices 3+4): the human interaction channel into running subagents. **Slice 3 — attach socket + manifest:** `src/core/attach/attach-server.ts` — 0600 same-uid unix socket `attach.sock` beside the task store; newline-JSON RPC (`list | steer | abort | result`); `subscribe` reserved stub → Phase 3 error; malformed lines get an error reply and the connection survives; root-only (depth 1 binds nothing); stale-sweep of dead-pid sockets at startup; bind-generation counter closes superseded binds on session switch/shutdown (race found by tests). `attach-manifest.ts` — 0600 `attach.json` keyed rootSessionId+pid with kill(pid,0) liveness, debounced atomic writes (tmp+rename), removed on parent exit. Steer/abort/result delegate to existing `AgentManager.steer()/abort()`; steers to finished agents fail cleanly with curated status messages; queued aborts fail their store task. **Slice 4 — CLI + RPC + announcements:** `bin/swarm-attach.js` (plain-JS CommonJS via nested `bin/package.json` — the package is `type:module`) with `swarm-list/steer/abort` symlink dispatch; manifest discovery across parents (live-pid choice, `--swarm-root`/`--socket` overrides); abort prompts without `--yes`. `human-announce.ts` — every successful human steer announced once (`pi.appendEntry("subagents:human_intervention")` + `subagents:steered {source:"human"}`); announcement failure never fails the steer; failed steers never announce. `cross-extension-rpc.ts` — `subagents:rpc:list|steer`, `PROTOCOL_VERSION → 3`, and caller provenance on `rpc:spawn` (`caller:{extension,token}` from `subagents:ready`/`rpc:handshake`, fail-closed). ## Reviewer focus (implementer-flagged) 1. Socket/manifest 0600 + root-only bind-generation race in index.ts. 2. Steer/abort error semantics reuse vs. new state. 3. **rpc:spawn provenance is a breaking contract change** — external callers (pi-tasks) must handshake; judge fail-closed vs. deprecation window. 4. Announcement failure isolation; CLI arg parsing (`--` escape, symlink dispatch). ## Known flake (orchestrator-observed) One timing-sensitive test failed once in 2 local runs (662/663 → 663/663 on rerun, tree clean). If CI/review sees a single transient failure, re-run before treating as blocker; worth identifying the timing assumption. ## Verify - [x] `tsc --noEmit` clean - [x] `vitest run`: 68 files / **663 tests pass (84 new)** — every design Slice 3+4 Automated Verification item is a named test - [ ] Live acceptance (ticket): steer from second pane changes agent behavior within one turn; steer-to-finished fails cleanly; manifest removed on exit; 0600 enforced Refs HOMELAB-1557 (Phase 2), epic HOMELAB-1555.

aaron added 1 commit 2026-09-19 03:48:16 +00:00

HOMELAB-1557: feat(subagents): attach socket + steer — swarm-attach CLI, manifest, human-intervention announcements ... fb414dba32

Phase 2 Slices 3+4. The ROOT pi session now hosts a same-uid 0600 unix socket
beside the task store, so any local client (a second herdr pane, a shell, a
tail companion) can list, steer, abort, and read the result of live subagents —
while the parent process stays the only writer of child sessions. The socket is
a thin RPC over AgentManager's existing control surfaces; no pi-core changes.

What lands:
- attach/attach-manifest.ts: attach.json beside attach.sock, 0600 past umask,
  written atomically (tmp+rename). Carries rootSessionId+pid+socketPath+
  herdrPaneId+agents[{id,type,status,outputFile,paneId}] — paths and statuses
  only. kill(pid,0) is the liveness oracle; discovery scans
  <agentDir>/swarm/<enc>/<session>/attach.json and picks the newest LIVE one.
- attach/attach-server.ts: newline-JSON RPC (list | steer <id> <text> |
  abort <id> | result <id>; subscribe reserved with a curated Phase-3 error).
  Malformed lines get an error reply and the connection survives; unknown
  methods are refused with the supported list; a 1 MiB line cap bounds memory.
  Startup sweeps a dead-pid socket+manifest (never steals a LIVE parent's
  tree). Steers to finished/queued/unknown agents fail cleanly with the current
  status; queued abort delegates to abort() and fails the task
  cancelled-before-start in the durable store; result falls back to
  recoverStoredResult(). Root-only: depth != 0 binds nothing at all.
- attach/human-announce.ts: every SUCCESSFUL steer/abort writes one durable
  subagents:human_intervention entry (kind, agentId, agentType, text, at, via)
  and a subagents:steered event with source:"human". Announcement failures never
  fail the steer; failed steers never announce.
- bin/swarm-attach.js + swarm-list/steer/abort symlinks: plain CommonJS, zero
  deps, runnable from any pane. Discovery across manifests (newest live, --root
  pins one), 3 s connect / 10 s request timeouts, abort prompts unless --yes,
  non-zero exit on {ok:false}. bin/package.json pins CommonJS inside the ESM
  package (the inverse of src/watch/package.json).
- cross-extension-rpc.ts: subagents:rpc:list + subagents:rpc:steer (same
  serializer, semantics, and announcement as the socket) and PROTOCOL_VERSION
  2 -> 3. subagents:rpc:spawn is now caller-gated: caller:{extension,token}
  with the token from the subagents:ready broadcast {contractVersion,
  spawnToken} or subagents:rpc:handshake. Fail-closed — a missing/blank caller
  or a mismatched token refuses the spawn (provenance gate, documented as not a
  process-isolation boundary).
- core/index.ts: root-only session_start wiring after attachTaskStore, closed
  in disposeSession and on session_before_switch. A bind-generation counter
  closes a bind that a switch/shutdown raced (listen() is async; the socket
  file appears before the server object does).
- README: attach socket + CLI section.

Invariants held: socket + manifest 0600 same-uid; single writer (the socket is
an advisory second input — every intervention is announced into the parent
transcript, so the orchestrator sees it); root-only; fail-open everywhere else.

Tests: 84 new across 6 files (manifest modes/atomicity/liveness/discovery,
per-method round-trips, clean failures, queued abort against a REAL
AgentManager+TaskStore, subscribe stub, unknown method, malformed-line
survival, oversize line, stale sweep, manifest lifecycle, root-only, CLI
subprocess round-trips incl. the abort prompt, discovery, announcement once +
failure-tolerant + absent-on-failure, JSONL visibility, RPC list/steer/version
3/spawn caller auth, index wiring + teardown). Full suite 663 green,
typecheck clean.

aaron commented 2026-09-19 04:13:27 +00:00

Author

Owner

Copy link

Landing-worker review — verdict: CONCERNS (no blockers) → merging

Fresh-context review (pi -p --no-session --model deepseek/deepseek-flash, primary sources only: PR diff vs origin/main merge-base, the design artifact, the invariants, the implementer focus list). Reviewer reproduced claims against real code; landing worker independently re-verified the findings below marked [verified].

Findings

Concerns

1. src/core/attach/attach-server.ts:252-259 — the socket is group/world-accessible between listen() and chmodSync: the OS creates it 0777 & ~umask, then it is chmod'd 0600. [verified] live probe: umask 002 → 775 at the listen callback, 600 after chmod. Under umask 022 the file is 0755 (no write bit → connect() denied), so the window only matters for group-writable umasks; steady state satisfies invariant #1. Design explicitly chose "chmod after bind, past umask", so this is a design-level trade-off, not an implementation deviation. Cheap hardening: const prev = process.umask(0o077); await listen(...); process.umask(prev); (net.Server.listen has no mode option).
2. tests/core/attach-wiring.test.ts:84-96 — the orchestrator-observed flake, root-caused: the cold await import("../../src/core/index.js") sits inside the first test body (~4.4 s standalone, ~4.1 s alongside its sibling attach test files) against vitest's default 5 s testTimeout, so CPU load pushes it over. [verified] reproduced locally: full-suite run with a concurrent tsc → this test + watch-renderer.test.ts:286 failed; clean full-suite rerun → 68 files / 663 pass. Fix: hoist the import to module scope/beforeAll.
3. bin/swarm-attach.js:81-82 — flags after steer text are silently absorbed into the message (steer a1 "do X" --json sends "do X --json"), and USAGE claims flags "may appear anywhere" while the code comment says the opposite. -- escape itself is correct (traced).
4. src/core/agent-manager.ts:929-939 — {ok:true} is not proof of delivery: record.session.steer(msg).catch(() => {}) swallows a rejected delivery while the CLI prints "lands after the current tool call". Pre-existing manager semantics (no new state), so the ack wording over-claims rather than the code being wrong.
5. src/core/attach/* — the socket sees only the root manager's own children. team-registry.allRecords() aggregates across in-process managers, so depth-≥2 (teamlead-spawned) agents are absent from list and unsteerable via steer. Not a regression (design says "thin RPC over AgentManager"), but undocumented; lands on HOMELAB-1559 teams.
6. Docs drift: docs/architecture.md:252 still says the RPC surface is "kept byte-identical" and lists only subagents:rpc:ping|spawn|stop. The PR adds handshake/list/steer, bumps PROTOCOL_VERSION 2→3, changes the subagents:ready payload to {contractVersion, spawnToken}, and makes rpc:spawn fail-closed. [verified] Only README.md was updated.

Suggestions (non-blocking; follow-up eligible)

• attach-server.ts:414-418 — close() can hang: a connection accepted after the socket.destroy() loop is never destroyed, so server.close(cb) may not call back and the socket/manifest survive teardown. server.closeAllConnections?.() fixes it.
• bin/swarm-attach.js:314-322 — the abort confirm is skipped when the list reply is not ok:true (destructive op proceeds unconfirmed); fail closed instead.
• attach-server.ts:180 — a second pi on the same session tree can unlink a live socket in the window between listen() and the first manifest write ("socket without a manifest" branch).
• No bin map in pi-swarm-suite/package.json and nothing on PATH, yet README/USAGE show bare swarm-list / swarm-steer.
• attach-server.ts:235 — buffer.length compares UTF-16 code units against a byte cap (ATTACH_MAX_REQUEST_BYTES).
• tests/core/attach-server.test.ts:332 — expect(JSON.parse(readdirSync(...) ? "{}" : "{}")).toEqual({}) is a tautology [verified]; the .tmp filter on the next line is the real assertion.
• attach-manifest.ts:22,24 — two separate node:path imports.
• bin/swarm-attach.js:158-169 duplicates roster-json.ts:111-121 formatAge with no drift test, unlike the renderer precedent.

Focus-area verdicts

1. 0600 + root-only bind generation — OK apart from the umask window above. Generation counter is sound (index.ts:612-630: in-flight bind self-closes when teardownDone || generation !== attachGeneration); depth ≠ 0 returns before any fs/socket op (attach-server.ts:209); sweep refuses to steal a live-owner tree. The attachStarting-blocks-rebind path is unreachable because pi re-instantiates the extension per session.
2. Steer/abort semantics — OK. Reuses manager.steer()/abort(); queued abort fails its task in the real store (tested against real AgentManager + TaskStore). Caveat = concern 4.
3. rpc:spawn provenance — OK to ship fail-closed; no deprecation window required (see below).
4. Announcement isolation — OK both directions: announceHumanIntervention wraps each surface and never throws; announce is called only after a true steer()/abort(); a throwing appendEntry still yields {ok:true} with the roster event landing (tested).
5. CLI parsing — OK except concern 3. -- escape correct; symlink dispatch on basename(argv[1]) works via real subprocess tests; discovery keeps only pidAlive manifests (EPERM = alive), newest updatedAt wins, newest-dead loses, --root/--swarm-root/--socket verified; abort confirm [y/N] gated on y|yes unless --yes.

Compatibility judgment (rpc:spawn fail-closed)

No deprecation window needed before merge. Machine-wide grep found no live caller of subagents:rpc:spawn: the in-repo agent_swarm spawns via deps.manager.spawn directly, and the only client found (pi-swarm-view RpcClient) is a disabled extension. An un-upgraded external caller (pi-tasks) would receive unauthorized caller: subagents:rpc:spawn requires caller provenance { extension, token } (protocol 3) — loud and actionable. Operational note for future consumers: the spawn token is minted per extension instance and pi re-instantiates extensions per session, so a caller caching the token across /new//resume must re-handshake (subagents:rpc:handshake {caller:{extension}}).

Gate evidence

• CI (Forgejo Actions, ci.yaml): run 4875 → success. Per job: pr-title ✅ (#8717), pi-swarm-suite ✅ (#8718), pi-otel-telemetry ✅ (#8719). Green first try — no flake in CI.
• Local (landing worker, reproduced on fb414db): npx tsc --noEmit → exit 0; npx vitest run → 68 files / 663 tests pass. The one earlier failure set was load-induced (see concern 2) and did not repeat.
• Manual/live acceptance (steer from a second pane, manifest removal on exit, 0600 enforcement in a live session) remains a ticket-level item and is unchanged by this merge.

Merging now (plain merge, branch retained — same convention as #22/#23). Concerns 1, 2, 5 and 6 are worth a small follow-up ticket (umask hardening + import hoist + teams-scope doc note + architecture.md contract refresh).

## Landing-worker review — verdict: CONCERNS (no blockers) → merging Fresh-context review (`pi -p --no-session --model deepseek/deepseek-flash`, primary sources only: PR diff vs `origin/main` merge-base, the design artifact, the invariants, the implementer focus list). Reviewer reproduced claims against real code; landing worker independently re-verified the findings below marked **[verified]**. ### Findings **Concerns** 1. `src/core/attach/attach-server.ts:252-259` — the socket is group/world-accessible between `listen()` and `chmodSync`: the OS creates it 0777 & ~umask, then it is chmod'd 0600. **[verified]** live probe: `umask 002 → 775` at the listen callback, `600` after chmod. Under `umask 022` the file is 0755 (no write bit → `connect()` denied), so the window only matters for group-writable umasks; steady state satisfies invariant #1. Design explicitly chose "chmod after bind, past umask", so this is a design-level trade-off, not an implementation deviation. Cheap hardening: `const prev = process.umask(0o077); await listen(...); process.umask(prev);` (`net.Server.listen` has no `mode` option). 2. `tests/core/attach-wiring.test.ts:84-96` — the orchestrator-observed flake, root-caused: the cold `await import("../../src/core/index.js")` sits inside the first test body (~4.4 s standalone, ~4.1 s alongside its sibling attach test files) against vitest's default 5 s `testTimeout`, so CPU load pushes it over. **[verified]** reproduced locally: full-suite run with a concurrent `tsc` → this test + `watch-renderer.test.ts:286` failed; clean full-suite rerun → 68 files / 663 pass. Fix: hoist the import to module scope/`beforeAll`. 3. `bin/swarm-attach.js:81-82` — flags after steer text are silently absorbed into the message (`steer a1 "do X" --json` sends `"do X --json"`), and `USAGE` claims flags "may appear anywhere" while the code comment says the opposite. `--` escape itself is correct (traced). 4. `src/core/agent-manager.ts:929-939` — `{ok:true}` is not proof of delivery: `record.session.steer(msg).catch(() => {})` swallows a rejected delivery while the CLI prints "lands after the current tool call". Pre-existing manager semantics (no new state), so the ack wording over-claims rather than the code being wrong. 5. `src/core/attach/*` — the socket sees only the **root manager's own** children. `team-registry.allRecords()` aggregates across in-process managers, so depth-≥2 (teamlead-spawned) agents are absent from `list` and unsteerable via `steer`. Not a regression (design says "thin RPC over AgentManager"), but undocumented; lands on HOMELAB-1559 teams. 6. Docs drift: `docs/architecture.md:252` still says the RPC surface is "kept byte-identical" and lists only `subagents:rpc:ping|spawn|stop`. The PR adds handshake/list/steer, bumps `PROTOCOL_VERSION` 2→3, changes the `subagents:ready` payload to `{contractVersion, spawnToken}`, and makes `rpc:spawn` fail-closed. **[verified]** Only `README.md` was updated. **Suggestions** (non-blocking; follow-up eligible) - `attach-server.ts:414-418` — `close()` can hang: a connection accepted after the `socket.destroy()` loop is never destroyed, so `server.close(cb)` may not call back and the socket/manifest survive teardown. `server.closeAllConnections?.()` fixes it. - `bin/swarm-attach.js:314-322` — the abort confirm is skipped when the `list` reply is not `ok:true` (destructive op proceeds unconfirmed); fail closed instead. - `attach-server.ts:180` — a second pi on the same session tree can unlink a live socket in the window between `listen()` and the first manifest write ("socket without a manifest" branch). - No `bin` map in `pi-swarm-suite/package.json` and nothing on PATH, yet README/`USAGE` show bare `swarm-list` / `swarm-steer`. - `attach-server.ts:235` — `buffer.length` compares UTF-16 code units against a byte cap (`ATTACH_MAX_REQUEST_BYTES`). - `tests/core/attach-server.test.ts:332` — `expect(JSON.parse(readdirSync(...) ? "{}" : "{}")).toEqual({})` is a tautology **[verified]**; the `.tmp` filter on the next line is the real assertion. - `attach-manifest.ts:22,24` — two separate `node:path` imports. - `bin/swarm-attach.js:158-169` duplicates `roster-json.ts:111-121` `formatAge` with no drift test, unlike the renderer precedent. ### Focus-area verdicts 1. **0600 + root-only bind generation** — OK apart from the umask window above. Generation counter is sound (`index.ts:612-630`: in-flight bind self-closes when `teardownDone || generation !== attachGeneration`); depth ≠ 0 returns before any fs/socket op (`attach-server.ts:209`); sweep refuses to steal a live-owner tree. The `attachStarting`-blocks-rebind path is unreachable because pi re-instantiates the extension per session. 2. **Steer/abort semantics** — OK. Reuses `manager.steer()`/`abort()`; queued abort fails its task in the real store (tested against real `AgentManager` + `TaskStore`). Caveat = concern 4. 3. **`rpc:spawn` provenance** — OK to ship fail-closed; **no deprecation window required** (see below). 4. **Announcement isolation** — OK both directions: `announceHumanIntervention` wraps each surface and never throws; announce is called only after a true `steer()/abort()`; a throwing `appendEntry` still yields `{ok:true}` with the roster event landing (tested). 5. **CLI parsing** — OK except concern 3. `--` escape correct; symlink dispatch on `basename(argv[1])` works via real subprocess tests; discovery keeps only `pidAlive` manifests (EPERM = alive), newest `updatedAt` wins, newest-*dead* loses, `--root`/`--swarm-root`/`--socket` verified; abort confirm `[y/N]` gated on `y|yes` unless `--yes`. ### Compatibility judgment (`rpc:spawn` fail-closed) **No deprecation window needed before merge.** Machine-wide grep found no live caller of `subagents:rpc:spawn`: the in-repo `agent_swarm` spawns via `deps.manager.spawn` directly, and the only client found (`pi-swarm-view` `RpcClient`) is a *disabled* extension. An un-upgraded external caller (pi-tasks) would receive `unauthorized caller: subagents:rpc:spawn requires caller provenance { extension, token } (protocol 3)` — loud and actionable. Operational note for future consumers: the spawn token is minted **per extension instance and pi re-instantiates extensions per session**, so a caller caching the token across `/new`/`/resume` must re-handshake (`subagents:rpc:handshake {caller:{extension}}`). ### Gate evidence - **CI (Forgejo Actions, `ci.yaml`)**: run 4875 → **success**. Per job: `pr-title` ✅ (#8717), `pi-swarm-suite` ✅ (#8718), `pi-otel-telemetry` ✅ (#8719). Green first try — no flake in CI. - **Local (landing worker, reproduced on `fb414db`)**: `npx tsc --noEmit` → exit 0; `npx vitest run` → **68 files / 663 tests pass**. The one earlier failure set was load-induced (see concern 2) and did not repeat. - **Manual/live acceptance** (steer from a second pane, manifest removal on exit, 0600 enforcement in a live session) remains a ticket-level item and is unchanged by this merge. Merging now (plain merge, branch retained — same convention as #22/#23). Concerns 1, 2, 5 and 6 are worth a small follow-up ticket (umask hardening + import hoist + teams-scope doc note + `architecture.md` contract refresh).

aaron merged commit f883bb6af4 into main 2026-09-19 04:13:32 +00:00

aaron referenced this pull request from a commit 2026-09-19 04:13:33 +00:00

Merge pull request 'HOMELAB-1557: feat(subagents): attach socket + steer — swarm-attach CLI, manifest, human-intervention announcements' (#24) from plane/HOMELAB-1557-phase2 into main

aaron referenced this pull request 2026-09-19 04:22:16 +00:00

HOMELAB-1558: feat(subagents): persisted child sessions (flag-gated, single-writer enforced) #25

aaron referenced this pull request from a commit 2026-09-19 04:33:40 +00:00

HOMELAB-1558: fix(subagents): unconditional single-writer stop + marker hardening

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/pi-extensions!24

No description provided.