HOMELAB-1558: feat(subagents): Phase 3 — full duplex attach (streaming subscribe, verbs, driver, interactive pane client) #26
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "plane/HOMELAB-1558-phase3"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Phase 3 of HOMELAB-1555 (ticket HOMELAB-1558): full duplex attach — the epic's headline feature.
6ef2223): activity bus tapped at the manager's passthrough sites (covers Agent-tool + scheduler + RPC spawns) + streamingsubscribeon the 0600 attach socket — bounded coalescing fan-out, backpressure dropstext_deltanever status,desyncframe → snapshot re-sync.ba325d7):prompt/answerverbs (answerreusesAgentManager.resume()),steer mode:followUp, advisorydriverindicator (agent|lead|human, last-steer-wins, announced per Phase 2 contract),subagents:rpc:watch/unwatchbus twin,PROTOCOL_VERSION 3→4advertised per-connection + in the manifest.c774e43): pi-tui interactive pane client (src/watch/pane.js, owner decision O1) — live scrollback, streaming line, input box; type-to-steer mid-run; Ctrl-C twice leaves and can never abort; pane close = unsubscribe never abort; reconnect re-subscribes with bounded snapshot; compaction boundary + monotonic usage. Degrades to the drift-tested tail renderer when the socket is absent (O2 fallback).1bd625d):docs/attach-protocol.mdreference (drift-tested against the frame module), field-allowlist security test, crash matrix (mid-fan-out / mid-flush / mid-teardown / dead-parent sweep), README.Owner decisions O1–O5 implemented (pi-tui client · interactive default · advisory announced ownership · one pane per agent · terminal answer fails clean).
⚠️ Explicit open point for review (owner-intent reading): O2 was implemented as "the interactive client is the default once a pane opens; tail is the socket-absent fallback." The stricter reading — background spawns with panes enabled default to the interactive client (i.e.,
watchPane: "interactive"as the default when opting in) — is NOT implemented:watchPanestill defaultsoff, andsplit|tabopen the tail renderer. If the owner intended the strict reading, it's a one-line default change; reviewer to adjudicate.Verify
tsc --noEmitcleanvitest runtwice: 76 files / 784 tests green both runsRefs HOMELAB-1558, epic HOMELAB-1555. Design:
interactive-subagent-sessions-phase3.md.Phase 3b — the verbs a pane needs, the honest "who is driving" label, and the in-process twin of `subscribe`. * agent-manager.ts: `answer(id, text)` — the delivery channel for a live child (pi's own `session.followUp`, which lands when the agent would otherwise stop). * attach/activity-frames.ts (new): the ONE activity→frame translation, shared by the socket and the bus twin so they cannot drift. * attach-server.ts: `steer` gains `mode: "steer"|"followUp"` (default = Phase-2 byte for byte) and an optional `client` label; `prompt`/`answer` verb with the curated failures and the extended announcement. A request in flight holds the transport pump back so a verb's OWN driver frames can never precede its reply. * attach-driver: `driver`/`streaming`/`pendingMessages` on `list`, the manifest, `rpc:list`, every snapshot and a `driver` frame; the label decays back to "agent" when the driven turn ends (manager status tap). * cross-extension-rpc.ts: `subagents:rpc:watch` / `unwatch` streaming the same frames on `subagents⌚<subId>`, each registration its own pseudo- connection; `rpc:list` carries the driver fields; `rpc:steer` sets `who:"human"` and announces `driver`/`pending`. `RpcHandle` gains unsubWatch/unsubUnwatch/unwatchAll, wired into disposeSession. * panes/pane-tracker.ts: `onResumed` clears the one-shot latches (fresh sentinel + fresh `watching` metadata on the next run) — the manager already clears `paneTerminalNotified` before flipping the status back to running (§D10). * core/index.ts: the lead's `steer_subagent` tool sets `who:"lead"`. * bin/swarm-attach.js: `subscribe <id> [--events …]` (persistent JSONL stream, exits on a terminal status, Ctrl-C = disconnect, never an abort) and `answer <id> <text>`. OWNER DECISIONS / DEVIATIONS (documented, not silent): - O5 is binding: `answer` on a TERMINAL agent fails clean with a curated status message. That narrows D4's target to a RUNNING child, where `resume()` is the wrong mechanism (it re-terminalizes a run that is still in flight), so the delivery uses `session.followUp()` instead. `answer` and `steer mode:"followUp"` therefore share one channel and differ only in the announced verb. D4's Slice-6 manual item ("answer an agent that finished 1 minute ago") is consequently NOT implementable and is superseded by O5. - D5's "attach-server-owned driver map" is hosted on the AgentManager instead: `list`, `attach.json` and `rpc:list` all need it, and the RPC layer must not depend on the attach server. It is still NOT an AgentRecord field. Tests: attach-verbs (14), attach-driver (6), rpc-watch (9), pane-tracker +3, swarm-cli +6. Full suite: 74 files / 746 tests green, tsc clean.Phase 3c — the pane becomes a client you can type into. * src/watch/{parse,render}.js (new, EXTRACTED from renderer.js): the Phase-1 parser and ANSI scrollback renderer, now shared verbatim by the tail renderer and the interactive client. renderer.js imports them; its behavior and its re-exported symbols are unchanged, so the Phase-1 parser drift test (tests/core/watch-renderer.test.ts) still guards BOTH programs. * src/watch/view-model.js (new): the "thin adapter" R-3.5 — frames → ConversationViewer-shaped {header, entries, streamingLine, footer}. The model is PLAIN TEXT by design (see the deviation note) and every mutation the pane performs on a frame lives in applyFrame(), so the reducer is unit-testable. * src/watch/pane-keys.js (new): the key decisions as a PURE function, because this is the only place a UI mistake can stop an agent — `x` twice (armed, 2 s) is the single abort path, Ctrl-C twice exits and can never abort, and single-letter commands only arm while the input box is empty. * src/watch/pane.js (new): the pane client. pi-tui based (OWNER DECISION O1), with the tail renderer as the documented fallback (O2): it discovers the attach socket, subscribes, renders live tokens/tools/usage/driver, and when the socket is absent or dies it tails the agent's .output through parse.js + render.js while retrying the socket (1s→2s→5s→10s). Disconnect is an unsubscribe and never an abort; a terminal status does NOT exit the client (the record stays answerable). * pane-tracker: `watchPane: "interactive"` opens a split pane running pane.js, advertises `interactive` in the pane metadata, re-opens an attach-mode pane on resume whose pane id was cleared, and clears `record.paneId` when the socket reports zero subscribers (D8.3/D13 — liveness rides the socket, no second herdr protocol). * attach server/registry: `onSubscriberChange` hook (the only notification a disconnect produces). * settings + suite-settings: `watchPane` gains `"interactive"`; off|split|tab keep their exact Phase-1 meanings. * bin/swarm-attach.js: `watch <agentId>` resolves the agent and execs the pane client in the current pane with inherited stdio. DEVIATION (documented, not silent): O1 makes the pane pi-tui based, so D7's RSS/dependency analysis is revised rather than merely overridden — a pi-tui pane is a node process carrying the pi-tui module graph (~45–60 MB), not the ~35 MB zero-dep renderer, and it is still 5–25x cheaper than a pi process. The model is emitted as plain text (the TUI must be able to MEASURE every line it draws; ANSI in the model breaks the width arithmetic) — the Phase-1 tail renderer keeps its ANSI output. Tests: watch-pane (19: view-model drift/compaction/desync/reconnect, the key guards, headless render, orphan exit, socket→tail fallback, arg parsing), pane-tracker +5, suite-settings-watchpane +1, swarm-cli +2. Full suite: 75 files / 773 tests green, tsc clean.Landing-worker review — @
1bd625d(SI-8) — DO NOT MERGEFresh-context independent review (reviewer subprocess, primary sources only: the diff, the repo at
1bd625d, the spec's D1–D13 + O1–O5, the Slice 5–8 Automated Verification checklists). Reviewer ≠ implementer. Nothing in the worktree was modified.Verdict: 2 BLOCKERS · 8 CONCERNS · 5 SUGGESTIONS.
All six frozen invariants hold (single-writer parent-brokered · Phase-2 compat · no pi-core changes · disconnect-never-aborts · 0600 socket+manifest · socket-path backpressure never blocks a producer). The blockers are not invariant breaks — they are a client-integration defect and a merge conflict.
BLOCKER 1 — the interactive pane never exits (pi-tui render loop never stopped); SIGTERM ignored; terminal left in raw mode
shutdown()(src/watch/pane.js:380-392) never callstui.stop()/terminal.stop()— neither appears anywhere in the repo. pi-tui'sProcessTerminal.start()doesprocess.stdin.setRawMode(true)+resume()+process.stdin.on("data", …)(@earendil-works/pi-tui/dist/terminal.js:113-183), and onlyterminal.stop()(:349-390) removes that listener, pauses stdin and restores raw mode.shutdown()merely setsprocess.exitCode, so the listen-mode stdin handle keeps the event loop alive and the process never exits. Every exit path funnels into the sameshutdown(q/Esc/Ctrl-C×2 →pane.js:460-465, 372;SIGINT/SIGTERM→pane.js:434-435), so the pane is immune to SIGTERM too, and bracketed paste / hidden cursor are never reset.Reproduced independently in a real pty at
1bd625d(unmodified repo file, cwdpi-swarm-suite, TUI confirmed up by[?2004hin the pane output):Counter-check proving the root cause and the fix: a
/tmpcopy with one addedtui.stop()inshutdown→AFTER_SIGTERM: EXITED(status=0)andAFTER_Q: EXITED(status=0). Repro harness:/tmp/pty-test4.py(python3 /tmp/pty-test4.py <pane.js> <cwd> {sigterm|q}).Why a green suite missed it: the Slice-7 proof "Ctrl-C twice exits the client and sends NO abort" (
tests/core/watch-pane.test.ts:182-196) calls the purehandleKeytwice — no client, no transport, no server. The test file itself states the pi-tui path is "covered by construction" (watch-pane.test.ts:20-23). So the Slice-7 automated-verification item Ctrl-C twice / parent death → exit 0 is not actually exercised on the shipped client.Secondary (same subsystem, include in the fix):
buildPaneCommand(src/core/panes/pane-tracker.ts:155-166) never passes--close-on-terminal, so D13's "finished panes do not accumulate" mitigation is unreachable for the shipped pane — the pane can neither exit itself nor be reaped by the parent, andswarm-attach watch'schild.on("close")wrapper (bin/swarm-attach.js:270) hangs with it.Fix scope (small): hold the TUI handle from
startTui()and calltui.stop()inshutdown()(before/aroundtransport.destroy()), then assert the real path — a pty or--headless-independent subprocess test that spawnspane.jswith a tty, sendsq/Ctrl-C×2, and asserts process exit +process.stdin.isRaw === false. Optionally wire--close-on-terminalinbuildPaneCommand.BLOCKER 2 — PR is no longer mergeable: conflict with
maininpi-swarm-suite/README.mdmainadvanced to12342f31(PR #25, Phase 4, merged 05:37–05:39Z) after this branch was cut. Local dry-run:git merge-tree --write-tree origin/main 1bd625d→CONFLICT (content): Merge conflict in pi-swarm-suite/README.md(all other changed files auto-merge). The PR reportedmergeable: truewhen opened and ismergeable: falsenow. Fix scope: merge/rebaseorigin/mainintoplane/HOMELAB-1558-phase3, resolve the README section (combine the Phase 3 and Phase 4 sections; keep both), push, let CI re-run — the current green CI (tasks 8753/8754/8755, head1bd625d: pr-title + pi-swarm-suite + pi-otel-telemetry allsuccess) was computed against the pre-Phase-4 base and must be re-earned on the merged SHA.CONCERNS (may merge; posted for the record)
enforceBoundshas no ceiling for non-droppable frames (subscriptions.ts:527,if (index < 0) break):status|usage|compaction|driver|desync|heartbeataccumulate past both caps O(turns). Invariant 6 is not violated (they are never dropped), but the D2.2 "rare by construction" memory claim is not enforced by the code. Consider coalescingusage/heartbeatto latest-per-agent.rewriteDesync(subscriptions.ts:537-539, 550) and mutates an item that is no longer in the queue → no new frame is emitted (probe: client tolddroppedFrames: 5when 125 were dropped) pluspendingBytesdrift.endEpisodeIfDrained(:579-587) is the only reset. Suggested fix: cleardesyncItemwhen the item is shifted off the queue indrain(:422-433). Reachable in the design's own SIGSTOP-then-resume manual scenario;view-model.jsignoresseq, so desync is the client's only gap signal (see S5).subscriptions.ts:570-573) names the connection's first subscription's agent, while the dropped frames may belong to another agent on that connection (up to 8 subs/conn) → a client watching B can be told about A's gap.x/a/qare consumed while the input box is empty (pane.js:459-465), so thepane-keys.js:14-15claim "typing a message that starts withxor containsqis never hijacked" is false: the firstxis eaten and arms, two quickxpresses abort the agent, and a message cannot begin withx/q/a. The test that "proves" safety (watch-pane.test.ts:218-224) feedsinputEmpty:false. Still the armed double-key path the invariant permits → concern, not blocker.cross-extension-rpc.ts:296-326): the RPC twin doesJSON.parse(line)+ synchronouspi.events.emit(...)from the agent's tap (node:events), so a slow synchronous listener stalls the run loop, and the registry drop path is unreachable (return true). In-process only.SteerOutcomereply gainedmode:"steer"(attach-server.ts, steer reply) which D4 did not sanction (D4 sanctioned the additive param only) — additive for tolerant clients; document it or drop it.closeOnTerminalis honoured for any subscriber of that agent, not just the pane that ownsrecord.paneId(attach-server.ts:376-386) — a third-party same-uid client can have the user's pane closed at terminalization.O2 adjudication (requested)
O2's row is labelled "Default pane mode" and reads "Interactive default — pane opens type-to-steer when attach socket is live;
.outputtail is the explicit fallback when it isn't" (spec:1427), under a heading that overrides designer recommendations. Read against its own label, the mandate is that the default mode of a watch pane is the interactive client, with tail only as the socket-absent fallback. The code implements the weaker deliberate reading:watchPanestill defaults"off"(src/core/settings.ts:324,src/suite-settings.ts:59) and"split"/"tab"— which open a pane while the socket is live — still run the Phase-1 tail renderer (pane-tracker.ts:30-38, 301-317); only the explicit"interactive"value reachespane.js. Owner intent check: the shipped client's own header asserts the strict reading —src/watch/pane.js:15-16"O2 — INTERACTIVE IS THE DEFAULT for a background spawn while the attach socket is live" — which the wiring contradicts, while the design's Slice-7 checklist (:1217) encodes the deliberate reading. So the spec and the owner decision disagree and the implementation follows the doc. Adjudicated as a CONCERN (not a blocker): no frozen invariant, and thepane.jstail-on-no-socket half of O2 is implemented. Aaron to confirm which reading is binding; if strict, it is a small wiring change (watchPanedefault / mode dispatch) plus its tests.Related: O5 supersedes D4's "answer = resume a retained finished session" — implementation deliberately refuses non-running agents (
attach-server.ts:676-702), so Slice 6/7's checklists still describe removed behavior; the design doc should be corrected.Crash-matrix honesty (Slice 8 checklist item)
crash matrix: kill -9 leaves no orphan writer…(attach-stream-limits.test.ts:232)mid-fan-out: the subscriber dies between a publish and a flush(:245)client.close(), then counts → 0,abortCalls == []mid-flush: the connection dies while the frame writer is draining(:263)mid-teardown: close() with a live subscriber…(:279)existsSyncon socket/manifest, post-close flush inert)a killed parent's artifacts are swept by the next boot(:295)process.kill(pid, 9)of a realnode -esleeper + real unlinks + fresh bind — but artifacts were written after a graceful closeSo "kill -9 at each of {mid-fan-out, mid-flush, mid-teardown}" is not exercised; two of three use a graceful close. Rename or add a real case.
SUGGESTIONS
S1 the
modefield on the steer reply (Concern 7). S2noticeis a documented wire event type with no server-side emitter — drop it from the protocol table or mark reserved. S3 bound/stripparams.clientbefore it reaches the manifest/roster/frames (attach-server.ts:716-724). S4view-model.jsnever checksseq(:185-193) — a cheap continuity check would restore D1's client-side drop detection. S5 several test names overstate their bodies ("every emitted frame's keys…"limits:121 requires 7 of 10 captured types;"a stalled reader drops oldest deltas"registry:190 never asserts "oldest";"the registry holds no manager surface at all"registry:307 checks one own key). Also: no test drives a real socket with a stalled reader — every backpressure test uses a fakeWriteLinecounter, so production'ssocket.writableLength < 1 MiB+'drain'resume path (attach-server.ts:419-440) is untested.Authorization (focus 3) — clean
agentIdis validated against the live roster (attach-server.ts:620-621;"*"refused bynormalizeAgentId,stream-frames.ts:118-124; same on the bus twincross-extension-rpc.ts:305-308).subIdis connection-scoped (attach-server.ts:658; subs live in a per-connection map,subscriptions.ts:243-256) → no cross-connection unsubscribe or cross-talk. Frame spoofing is impossible: the server generatesseq/agentIdper frame (subscriptions.ts:505-509) and client params are never echoed into frames;publishTogives real per-subscription isolation. The no-secret allowlist is enforced by construction (explicit object literals + a hand-copied allowlist inroster-json.ts:105-141). Residual (D12, not new): any same-uid client may subscribe to any of that root's agents.Abort-safety (focus 2) — clean apart from the hang
Enumerated every key path: no branch produces
abort/steer/prompt/answerexcept the armedx x(pane-keys.js:56-99,abortedlatch → one abort ever). Nothing sends a verb on socket close/reconnect (pane.js:331-337), watchdog reconnect (:422-429), ppid orphan, SIGINT/SIGTERM or teardown;swarm-attach's SIGINT isfinish(0)→socket.destroy(). Disconnect isregistry.detachonly and the registry holds no manager reference (attach-server.ts:443→subscriptions.ts:222-240); tests assertmanager.abortCalls == []on real socket close (attach-stream.test.ts:359-369).Fix worker: both blockers are mechanical and localized (pane.js
shutdown+ README merge). No invariant is broken, so nothing here invalidates the architecture — but the epic's headline feature does not currently exit, and a stale-base green CI is not a merge signal, so this PR does not pass the gate.Line-anchor corrections to the review above (verified against
1bd625d; the findings and verdict are unchanged):unknown agentvalidation:attach-server.ts:621-622(not:620-621)closeOnTerminalhonoured for any subscriber of the agent:attach-server.ts:372-379attach-server.ts:417, resume on drain:447"*"refused:stream-frames.ts:129-134(not:118-124)describe:attach-stream-limits.test.ts:231subscriptions.ts:527(if (index < 0) break),:537-539/:550(desync rewrite),pane.js:380/:388/:433,settings.ts:324,suite-settings.ts:59,pane-tracker.ts:155,watch-pane.test.ts:20-23,182-196,218-224Landing-worker review — round 2 @
ce0726c— MERGEFresh-context independent reviewer (separate
pi -p --no-sessionprocess, primary sources only: the8d026d6..ce0726cdelta, the repo atce0726c, the design's D1–D13 + O1–O5). Reviewer ≠ implementer. The worktree was not modified; scratch copies were deleted.Verdict: 0 BLOCKERS · 3 CONCERNS · 3 SUGGESTIONS. Both round-1 blockers are genuinely fixed; the three open items are pre-existing latent defects in the client, not regressions of this fix. Follow-ups filed as HOMELAB-1566.
Round-1 BLOCKER 1 — pane never exits — FIXED
Same repro shape as round 1 (real
pty.fork,node src/watch/pane.js, TUI confirmed up by[?2004hbefore the action):1bd625dce0726cqRUNNING, no raw teardownEXITED(code=0),[?2004l+[?25hEscRUNNINGEXITED(code=0)RUNNINGEXITED(code=0)SIGTERMRUNNINGEXITED(code=0)Positive control (proves the test can fail): a scratch copy of
src/watch/pane.jswith the singletui?.stop?.()line removed goes back totui_up=True RUNNING raw_teardown=Falsefor bothqandSIGTERM, and the 5 new pty tests go red (5 failed | 19 skipped). Reviewer added adversarial cases, all clean:SIGINT,qtyped in the first 100 ms, no--socketand no--file, stale socket file, parent pid killed (orphan frame → exit 0), and a signal-delay sweep (before the handler is installed at ~40–60 ms the default disposition applies with the TUI never started, so the terminal is never left raw).Why the round-1 report was fair: the old evidence was a pure-function
handleKeytest with no client, no transport, no server. That is now replaced with real-pty coverage (tests/core/watch-pane.test.ts:570-606), and--close-on-terminalis passed (core/panes/pane-tracker.ts:221), so D13 is reachable —swarm-attach watchdeliberately does not pass it.Round-1 BLOCKER 2 — base sync — FIXED
git merge-tree --write-tree origin/main ce0726c→0a40abaf, no conflict rows (round 1 wasCONFLICT). No markers in the resolution. No content dropped from either side:1bd625d→8d026d6keeps all of Phase 4's README additions (thepersistChildSessionsrow and the whole "Persisted child sessions" section), and the 9 lines where main differs are Phase 3's own supersessions (thewatchPanerow, the obsolete "subscribe is a reserved stub … ships in Phase 3" paragraph, therpc:list/rpc:steerparagraph rendered as the protocol-4 version). The only information lost in the whole merge is the history token "PROTOCOL_VERSIONbumped 2 → 3" — wording, not operational. Phase 4's code is present and green.O2 strict — FULLY implemented
paneClientKind(core/panes/pane-tracker.ts:68-72):interactive→client always;off→tail (unreachable,shouldOpenPanegates first);split|tab→client while the socket is live, Phase-1 renderer otherwise. Wired atpane-tracker.ts:366-367—this.opts.attachSocketLive?.()is read insideopenPane, per spawn — over the closure atcore/index.ts:646on the mutablelet attachServer(:541), assigned only in the post-bind.then(:720) and cleared at:782;listening: trueis a literal on the object built only afterawait listen()succeeds (attach-server.ts:476,787). So the gate is live, never cached, and can only be true when a socket exists.--close-on-terminalflows tosubscribe(pane.js:186,387) andswarm-attach watchdoes not pass it. Mid-run socket death still degrades inside the TUI (notice + transcript +tail (disconnected)all drawn in a real TUI frame).Adjacent defects the fix exposed: raw-stdout-under-TUI fixed except one path (S1); transcript-poll-in-TUI-mode fixed (
pane.js:486-497, verified inside a TUI frame); the pi-tui over-wide-line throw only partially fixed — see C1.Invariants 1–6: HOLD (verified, not assumed — single writer: no write/append/mkdir/unlink in
src/watch/*.js; no pi-core files touched,git diff --name-only 12342f3 ce0726cis 42 files all underpi-swarm-suite/; disconnect sends no verb; 0600 asserted by tests; backpressure untouched by the delta). Authorization and abort-safety: HOLD in the changed files ("*"refused,agentIdvalidated against the live roster,subIdconnection-scoped; the newif (stopped) return { consume: true }short-circuits after teardown and introduces no verb).CONCERNS
C1 —
fit()is not a display-width truncator, so pi-tui still throws and kills the pane.src/watch/view-model.js:225-229truncates bytext.length; pi-tui measuresvisibleWidth()(tab = 3 cols, East-Asian Wide = 2). Every model line goes through it (:287-292) and so does the notice (src/watch/pane.js:556). Reproduced by both the reviewer and me on the shipped file, real pty, 80×24: appending a mid-run line of"漢"*200→Rendered line … exceeds terminal width (159 > 80)→EXITED(code=1);"\t"*60+"end"→(183 > 80)→EXITED(code=1); a 200-char ASCII line staysRUNNING(control).nodeText()is a passthrough, so nothing normalises tabs upstream.fit()'s own doc comment (view-model.js:17) claims it truncates by visible length — it does not. Terminal is restored (this.stop()runs before the throw) and the agent is untouched, so this fails safe — but note that O2-strict makespane.jsthe default program forsplit/tabpanes whenever the socket is live, so this is now the default experience for transcripts containing tabs or wide glyphs. → HOMELAB-1566 (not a blocker: pre-existing in the client, no invariant, fail-safe, opt-in —watchPanestill defaultsoff).C2 — D13's close never fires for a subscriber that arrives after terminalization.
attach-server.ts:376-385(maybeCloseOnTerminal) is invoked only from the livestatusevent (:394);subscribe(:614-641) has no terminal-status gate and no post-reply re-check. Reviewer reproduced (scratch test, real server + real pane): terminalize before the pane can subscribe →subscriberCount("a1") > 0whileclosePanecalls stay[]. The happy path is green (terminalize after subscribe →closePane("wC:pD13"), and a non-asking subscriber is never closed).docs/attach-protocol.md:47-55overstates this. → HOMELAB-1566.C3 — the pty tests could silently skip in CI. They are
it.skipIf(!PYTHON3)(watch-pane.test.ts:502,570-606); with a PATH containing onlynodethe file reports19 passed | 5 skipped, exit 0 — a green suite with zero pane-exit coverage. Premise disproven: thek8srunner image installspython3(infra-core/core/manifests/apps/forgejo-runner/Dockerfile:12, image pinned by digest indeployment.yaml:30), so the 5 pty tests do execute in CI (reviewer ran them locally too: 52 passed, 5 executing). Downgraded to S2 (defence in depth), not a coverage gap.SUGGESTIONS
S1
pane.js:509writes raw stdout under the active pi-tui loop (orphan notice) — the one path the fix's ownsay()/repaintTui()discipline missed. S2 make the pty tests fail closed (dropskipIf, or assertpython3 --versionin CI). S3fit()degenerate branches::227returns the line untruncated whenwidth <= 0(guaranteed throw if a terminal reports 0 columns) and:228can emit a lone surrogate.Merge decision
Round-2 acceptance is met (blocker 1 reproduced-fixed against the round-1 repro shape, blocker 2 resolved, O2 strict complete); the open items fail safe, break no invariant, and are filed as HOMELAB-1566. CI is green on
ce0726c(run 13:pr-title+pi-swarm-suite+pi-otel-telemetry, allsuccess;tsc --noEmitclean, 79 files / 845 tests locally). Merging. NOTE: one PR-checklist item remains unchecked by design — the 30-min soak + live herdr end-to-end acceptance is manual, after the epic-end 0.9.1 upgrade.