Skip to content

feat(chat): read-only subagent sessions surface (#187) - #241

Merged
dzianisv merged 4 commits into
mainfrom
feat/subagent-sessions-187
Nov 13, 2026
Merged

dzianisv merged 4 commits into
mainfrom
feat/subagent-sessions-187

Conversation

@dzianisv

Copy link
Copy Markdown
Owner

Summary

Read-only subagent surface for #187, per the Stage 2 design (design, review PASS conditional on the recorded test exception, comment 6857247200). A collapsible Subagents section on the parent detail screen lists child sessions with a live status badge. No release in this task.

  • AC1 — SubagentSection mounted in app/session/[id].tsx above the message list, outside the FlatList. One row per child (newest first): title (fallback Untitled subagent), agent · provider/model only when the server provides them, relative started time, state pill. loadChildren(parentID) fetches the global pool via the existing client.session.list() and filters with the new pure selectChildren(all, parentID).
  • AC2 — Status is derived, never stored: deriveChildStatus(child, sessionStatus, sessionErrors) — Failed (recorded session.error) > Running (busy/retry) > Done (time.idle present, or an idle status entry observed live this app session) > Idle. Parent summary badge via summarizeChildren (counts + worst-state dot). No polling; SSE only.
  • AC3 — The session.created leak is closed in events.ts: roots prepend as before; children upsert into childrenByParent[parentID] only if that parent's key already exists, otherwise ignored (next loadChildren picks them up). Root list never receives a child.
  • AC4 — New sessionErrors: Record<sessionID, string> in the events store, set on session.error for any session id, cleared on next busy. Failed rows show the server message (2 lines max). Existing erroredSessions set and all notification logic untouched.
  • AC5 — Graceful degradation: a server returning no parentID matches yields the No subagents yet empty state, not an error; rows without agent/model render without that line. A section load failure shows its own error + Retry and never blocks the transcript.
  • AC6 — No regression (see gates below). loadSessions, normalizeSessions, transcript render path untouched.

Out of scope (per design): tap-to-inspect, nested navigation, prompting/aborting/managing children, permission handling, creating subagents, editing/deleting children, child notifications, iOS/tablet layout.

OQ1 verification (blocking gate, done before any UI code)

Verified against the server source, Qiiks/opencode tag v1.18.18-orw.1 (commit 7731d43f):

  • (a) The /api/session pool returns children carrying parentID. packages/opencode/src/session/session.ts: Session.Info has parentID: optional(SessionID) (:231) plus agent/model; fromRow maps parentID: row.parent_id ?? undefined (:85); listByProject (:987) and listGlobal (:560) apply the isNull(parent_id) roots filter only when roots=true — a call without it returns children too.
  • (b) The session.created SSE payload carries parentID. packages/schema/src/v1/session.ts:571-579: Created: define({ type: "session.created", schema: { sessionID, info: SessionInfo } }), and SessionInfo.parentID: optional(SessionID) (:550). Publish site session.ts:537: events.publish(SessionV1.Event.Created, { sessionID: result.id, info: result }) with the full Info including parentID. The bridge forwards the full event data to SSE clients (event-v2-bridge.ts: payload: { id, type, properties: event.data }).

Related findings from the same read:

  • OQ3 answered: the server does not populate time.idle — the Time schema (session.ts:203-208, v1 SessionInfo.time) has only created/updated/compacting/archived. Cold-loaded children therefore read Idle until a live busy → idle transition is observed, exactly as the design accepts.
  • OQ4 checked: sessions.ts handleEvent session.updated is map-only (replaces a matching id, never adds) — no leak path, no guard needed.
  • R6: the server does have a session.deleted event (v1 session.ts:588, published at session.ts:624); the mobile store has no handler for it, so a server-side-deleted child stays in childrenByParent until the next focus-load. Accepted for this read-only slice, per design.
  • Server children endpoint exists: the server also ships a dedicated children endpoint (SessionHttpApi.children → session.children(parentID), session.ts:598-606). This slice implements the design as written (list-all + filter, no SDK change); swapping the fetch behind selectChildren is a follow-up, no UI/store change needed.

Files changed (16, +933/−14)

  • src/lib/session-children.ts (new) — selectChildren, deriveChildStatus, summarizeChildren
  • src/lib/session-children.test.ts (new) — 15 AC-mapped unit tests, no mocks
  • src/components/chat/SubagentSection.tsx (new) — collapsible section + rows
  • src/stores/sessions.ts — childrenByParent, childrenLoading, childrenError, loadChildren(parentID)
  • src/stores/events.ts — sessionErrors record; session.created routing by parentID
  • src/lib/sdk.ts + src/lib/v2-shape.ts — Session/toSession carry agent, model {providerID, modelID}, time.idle (one mapping point; fields omitted, not undefined-valued, when the server sends none, so the mapped shape is unchanged for sessions without them)
  • app/session/[id].tsx + src/components/chat/index.ts — mount (one line + barrel export)
  • src/lib/i18n/{en,ru,zh-Hans}.json — subagents.* keys (title, running, done, idle, failed, none, loadError, retry, summary)
  • src/lib/session-list.integration.test.ts — 4 new tests on the existing integration surface (subagents-seeded mock over real HTTP)
  • tests/fixtures/mock-opencode-server.ts — --seed-subagents (parent + done/idle children)
  • .maestro/flows/subagents.yaml (new, NEWER/non-blocking) + scripts/run-e2e-flows.sh — flow + 4101 mock startup

Not touched, per brief: ToolCallCard, SessionInfo, root list, notifications, sdk-v1.ts.

Local test evidence (exact head SHA eff8c468)

  • npm run check:versions — exit 0 (Version metadata aligned: 0.4.17 (44))
  • npm run typecheck — exit 0
  • npm test (full suite) — 414 pass / 0 fail (baseline at 38316ae5: 395 pass / 0 fail; delta +19: 15 new unit tests in session-children.test.ts + 4 new integration tests; 0 removed; test files 50 → 51)
  • Mock seed verified over real HTTP: /experimental/session returns the parent (no parentID) + 2 children (parentID: seed-parent, one with agent/model/time.idle, one without); roots:true load path excludes children (integration test).
  • One failure found and fixed during the run: the existing v2-shape test does deepStrictEqual on toSession output, so the mapping now omits absent agent/model/idle keys instead of emitting undefined values. No existing test was edited.

Deviations and notes

  • App code implemented mechanically in this lane (no Google AI Studio access from here), not generated via AI Studio as usual. Recorded honestly; the gates above are the evidence.
  • Test scope follows the recorded exception exactly: the only new test file is session-children.test.ts (three helpers); the session.created routing is covered via the existing integration/e2e surface (integration tests + Maestro flow), not a unit test. The design §2 row suggesting edits to sse.test.ts/v2-shape.test.ts was not applied — that would be net test growth beyond the recorded exception. (Design §2's test pointer should also read §3, not §5 — a design-doc fix for Stage 5.5, not this PR.)
  • formatTime/statusColor duplicated, not exported: SessionInfo.formatTime and ToolCallCard.statusColor are module-private and both files are out of scope for this slice, so the component duplicates the relative-time format (same chat.sessionInfo.time.* keys) and the four pill hex values, each with a comment saying so.
  • Summary wording: the all-done case renders 3 subagents · 3 Done (count + pill label) rather than the design's 3 subagents · all done, to stay within the design's nine i18n keys.
  • Maestro flow not run locally (no emulator on this VM); it is NEWER/non-blocking and runs in CI on this PR. Its known limit is documented in the flow header: the live session.created routing branch has no automated cover (the mock emits no session.created broadcast and no UI creates a child) — verified by inspection; live-pair proof stays a release-time phone check.
  • Push note: this branch was pushed via the Git Database API because the available gh token lacks workflow scope and a plain git push refuses any ref touching .github/workflows/*. No workflow file is touched by this PR — the 4101 mock startup lives in run-e2e-flows.sh instead. The pushed tree was verified byte-identical to the local tree (7153ddb8…, incl. the 100755 mode on run-e2e-flows.sh).

Adds a collapsible Subagents section to the parent detail screen listing
child sessions with a live status badge, per the Stage 2 design. Children
come from the existing global session list filtered client-side on parentID;
status is derived from the SSE sessionStatus map + a new sessionErrors
record; the session.created leak (children prepended to the root list) is
closed by routing on parentID. No release in this task.

OQ1 verified against Qiiks/opencode v1.18.18-orw.1 server source: the
/api/session pool returns children carrying parentID (roots filter applies
only when roots=true), and the session.created SSE payload carries
parentID (v1 SessionInfo). OQ3 answered: the server does NOT populate
time.idle, so cold-loaded children read Idle until observed live.

Gates on the exact head SHA: check:versions exit 0, typecheck exit 0,
full suite 414 pass / 0 fail (baseline 395 + 19: 15 new unit tests for the
three pure helpers, 4 new integration tests). App code implemented
mechanically in this lane (no Google AI Studio access); see PR body.
@dzianisv

Copy link
Copy Markdown
Owner Author

PASS — Stage 4 review of PR #241 (issue #187), head eff8c468e5dc01a0ea8b3682e95ed616a842fcde.

Test-count delta: baseline at 38316ae5 395 pass / 0 fail → at head 414 pass / 0 fail (+19: 15 new unit tests in src/lib/session-children.test.ts + 4 new integration tests in src/lib/session-list.integration.test.ts; 0 removed; test files 50 → 51 in the npm test scope — the one new file is session-children.test.ts). Verified independently, not taken from the PR body: full npm test run in the real checkout at the exact head (ℹ tests 414, pass 414, fail 0), and CI on merge commit 27fee36 (head merged into cad292ac) ran 424 pass / 0 fail — the extra 10 are PR #238's auto-scroll tests from main. PR files verified against GitHub: 16 files, +933/−14, identical to the local diff.

CI this verdict is delivered against (checked ~00:15 WITA Nov 14): Typecheck and unit tests PASS (job 131993211037), test PASS (job 131993209742), build PASS 14m59s (job 131993210198), Secret scan (gitleaks) PASS; build-fdroid/release skipping (expected on a PR). Still pending, no failures: Unsigned iOS Simulator build, activation-e2e. The subagents Maestro flow runs inside activation-e2e as a NEWER non-blocking flow (scripts/run-e2e-flows.sh:139-146), so its result does not gate this verdict — Stage 5 to confirm it when the job lands.

Exception compliance (issue #187, comment 6857247200): all 15 new unit tests cover only the three helpers — selectChildren (AC1, AC3), deriveChildStatus (AC2, AC4), summarizeChildren (AC2) — verified by reading the file; no mocks anywhere in it (plain object factories only). No mock-based tests in the diff; the integration tests use the pre-existing real-HTTP mock-server fixture.

Validated findings: none. AC-by-AC evidence (diff actually read against the design + PRD):

  • AC1 — app/session/[id].tsx:752 mounts <SubagentSection> above the message list, outside the FlatList. src/lib/session-children.ts:20-25 selectChildren filters parentID === parent.id, newest-first by time.updated; rows show title (fallback Untitled subagent), agent · provider/model only when provided, relative started time (SubagentSection.tsx formatTime logic verified identical to SessionInfo.formatTime).
  • AC2 — deriveChildStatus (src/lib/session-children.ts:38-49) priority Failed > Running > Done > Idle matches design §3 exactly. Done-rule invariant holds: sessionStatus entries are created only by the SSE status handler (src/stores/events.ts:298) and the reconnect resync (events.ts:139-167), which only ever clears a busy flag for an id already in the map — a cold-loaded child can never read Done it cannot prove. summarizeChildren (session-children.ts:58-76) counts + worst-state dot; no polling (load on focus + SSE only).
  • AC3 — events.ts:379-397 routes session.created on parentID: roots prepend exactly as before, children upsert into childrenByParent only if the parent key already exists (deduped, prepended = newest first), otherwise ignored. sessions.ts:510-519 handleEvent session.updated is map-only, never adds — no second leak path (OQ4 confirmed). Load-time roots exclusion proven by the new integration test.
  • AC4 — sessionErrors set on session.error for any session id, cleared on next busy (events.ts:285-296, 410-414); erroredSessions, notifications and the viewed-session banner are untouched. Failed rows show the server message, 2 lines max.
  • AC5 — no parentID matches → No subagents yet empty state, not an error; rows without agent/model omit that line (proven by seed-child-idle in the integration test + Maestro flow); load failure shows its own error + Retry and never blocks the transcript.
  • AC6 — loadSessions, normalizeSessions and the transcript render path are untouched by the diff; typecheck + full suite green in CI and locally.

Filed notes (not fails, no implementer action required before merge):

  • N1 — Live branches without automated cover (recorded residual): the live session.created routing branch (events.ts:383-396) and the live Failed/Running transitions have no automated test — the mock emits no session.created broadcast and no UI creates a child, so the Maestro flow documents this limit in its own header. Verified by inspection; live-pair proof stays a release-time phone check, as Stage 3 recorded.
  • N2 — Design §2 test row not applied: sse.test.ts/v2-shape.test.ts not extended. The new toSession mapping (agent/model/idle) IS covered — the integration test asserts agent/model/time.idle over real HTTP. What stays uncovered is pre-existing, unmodified normaliser behaviour (session.execution.failed → session.error + idle, v2-shape.ts:597-606); sse.test.ts has no failed-case coverage at all. Pre-existing gap, not introduced here; follow-up, not this PR.
  • N3 — Summary wording: all-done renders 3 subagents · 3 Done, not the design §3 example 3 subagents · all done — the design's nine i18n keys (§2) cannot express "all done" without a 10th key, so the implementation is consistent with the key set. Suggest fixing the design §3 wording in Stage 5.5, alongside the already-recorded §2 pointer fix (§2 should read §3, not §5).
  • N4 — Collapsed load failure is silent: error + Retry render only when expanded, so a failed load while collapsed shows no signal. Design §5 does not require a collapsed error indicator; a red dot on the collapsed header would be a follow-up.
  • N5 — PR is BEHIND main (still MERGEABLE): fix(chat): auto-scroll transcript to newest message (#155) #238 (cad292ac) touched app/session/[id].tsx (auto-scroll import/effect/handleScroll) but none of this PR's mount lines — no textual or semantic conflict (the mount sits outside the FlatList; auto-scroll keys off messageData only). Update the branch before merge; after the update the suite should read 424 pass / 0 fail (405 on main + 19 here — already proven by CI on merge commit 27fee36).
  • N6 — Duplicated formatTime/pill colours are intentional (helpers are module-private, files out of scope); formatTime verified logic-identical to SessionInfo.formatTime. Acceptable for this slice.
  • N7 — Mechanical implementation without Google AI Studio (recorded deviation): judged on its own merits — code is clean and matches the design. No finding.
  • N8 — loadChildren refetches on every focus with no staleness check, and childrenLoading is a single global flag — matches the design's focus-load pattern. Cosmetic.

@dzianisv

Copy link
Copy Markdown
Owner Author

Triage: Unsigned iOS Simulator build on head cc0a3e5 (Stage 5, recorded by the lead).

Run 43459869318 (iOS CI, pull_request, head cc0a3e5): conclusion failure. The job itself (132004118830) has conclusion cancelled with zero steps recorded — no step of it ever executed, so no code from this PR ran in it. This change could not have caused it.

Same workflow is green on main: iOS CI run 43455305640 on cad292a (success) and run 43459958942 on fb7344d (success). The job is not a required check (required: Typecheck and unit tests, test, build — all pass on cc0a3e5). Per the workflow rule, this cancelled non-required job does not block the merge. If it cancels again on the next head, the same triage applies.

@dzianisv
dzianisv merged commit b5e9363 into main Nov 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant