TUI: guided session as default product, boardroom as inspect overlay (#191)#433
Merged
Conversation
Reposition guided 5-screen flow as the Session View (primary product) and the boardroom App as an Inspect Overlay layered above it. Spec covers naming rename (TuiMode boardroom -> session, Screen advanced -> inspect), Ctrl+I entry/exit pair, copy audit across tui-app/ screen-manager/running-view, and a new docs/tui/information-architecture.md. No structural refactor.
15 tasks: rename PageKind/Screen/TuiMode/AdvancedModeWrapper/handler names, rebind Ctrl+A -> Ctrl+I, add Esc exit on inspect overlay, wire [INSPECT] status chip, audit copy in tui-app/screen-manager/main/ refresh-context/running-view, publish docs/tui/information-architecture.md. PagesStore is in-memory so no migration. Server /api/boardroom/* routes explicitly out of scope.
Sweep flagged three doc-comment leftovers from the inspect-overlay rename: - running-view.tsx:486 — "advanced/boardroom mode" → "the inspect overlay" - use-session-persistence.ts:34 — "advanced mode" → "inspect overlay" - panel-hints.ts:16 — "advanced-mode toggle" → "inspect-overlay toggle" Comment-only; no behavior change.
…-held Multiple grove worktrees on the same host previously failed with "Port 4515 is bound by a process that this grove project did not spawn" because each worktree's spawnService threw on conflict. Now: when the configured port (PORT for server, MCP_PORT for mcp) is held by a process whose pid does not match this grove's pidfile, spawnService picks an OS-assigned ephemeral port via pickFreePort() and updates the matching env var so siblings (e.g., MCP child reading GROVE_SERVER_PORT) target the rebound server. The pre-existing adopt-on-identity-match path is unchanged. Includes a logged warning so operators can see the fallback.
Ctrl+I in terminals is byte 0x09 — identical to Tab. The unit tests
passed because they synthesized {ctrl: true, name: "i"} directly,
bypassing terminal byte-decoding. In a real tmux/xterm session,
Ctrl+I cycled the boardroom panel focus instead of opening the
inspect overlay.
Switched to Ctrl+G (0x07, distinct from any printable / Tab / Enter
/ Backspace). Verified end-to-end via tmux capture-pane: Ctrl+G
opens overlay (breadcrumb shows "Running > Inspect"); Esc and
Ctrl+G both return to RunningView.
Updated: keymap binding, InspectModeWrapper exit handler,
INSPECT_HINTS, footer chip, help overlay, IA doc, JSDoc, and the
3 corresponding TDD tests.
Round 1 adversarial-review flagged two HIGH issues: 1. Server+MCP spawn parallel; MCP could read stale env.PORT before server's port-fallback wrote the new value, leaving MCP pointing at the foreign-held default port. Fix: pre-resolve every service port sequentially before the parallel spawn (preResolvePort helper), so process.env mutations are settled before any child reads serviceEnv. 2. PagesRouter unmounts RunningView when inspect is pushed; on Esc/ Ctrl+G return RunningView remounts fresh, losing local React state. The IA doc claimed bit-for-bit preservation — false. Fix: corrected IA doc and InspectModeWrapper JSDoc to describe the actual remount behavior and which state survives.
#191 round 2) Round 2 review flagged: 1. spawnService still mutated env mid-parallel-spawn (TOCTOU). Removed the in-spawn foreign-port fallback entirely; preResolvePort owns that decision before any spawn promise starts. spawnService now throws on late TOCTOU collision rather than mutating env behind MCP's back. 2. Adopted MCP could keep stale GROVE_SERVER_PORT after server fallback shifted the port. Added serverPortShifted detection in startServices; when set, MCP spawn runs with forceFreshSpawn so the adopt path is skipped and MCP rebinds to a free MCP_PORT, picking up the freshly resolved server port via env.
Round 3 review flagged: 1. forceFreshSpawn picked a free MCP port without killing the pidfile-recorded stale MCP, orphaning it on the default port. Clients hitting localhost:4015 would still hit the stale bridge. Fix: verify pidfile identity in the force-fresh path; SIGTERM (then SIGKILL after 3s) the stale child and reuse its port. Foreign listeners still trigger the pick-free-port path. 2. Enter on a feed item called openDetail which was wired to onEnterInspect — an accidental inspect-entry path violating the 'Ctrl+G only' contract. Fix: openDetail wired to a no-op; Enter on feed items is a no-op until a real detail route exists. Test now asserts Enter does NOT call enterInspect.
…191) Consolidates fixes for rounds 4-9 of the adversarial review loop: * Round 4: pidfile schema gains per-child {port, serverPort}; classifyForceFreshOwner preserves live sibling owners. * Round 5: classifier fails closed on transient pidfile read errors; stopServices rewrite preserves per-child metadata; in-flight SAME_PROCESS_IN_FLIGHT lock prevents double-spawn for same groveDir. * Round 6: pidfile not rewritten on pure-adoption; mixed adoption+ spawn merges prior entries to preserve metadata. * Round 7: pure-borrower stopServices leaves pidfile untouched. * Round 8: Esc dropped from InspectModeWrapper to defer to App's modal cascade; Ctrl+G is the unambiguous exit. * Round 9: per-child {parentPid} in pidfile schema; classifier consults child.parentPid first to handle mixed-owner shutdown. Round 10 surfaced one remaining HIGH finding (preserved sibling MCP not carried forward into the rewritten pidfile). Loop hit the 10- round cap; finding logged for follow-up.
…rking-sketch # Conflicts: # src/tui/screens/screen-manager.tsx
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.
Summary
Reframes the TUI's guided 5-screen flow as the primary Session View and the boardroom
Appas an on-demand Inspect Overlay. Pure terminology + UX polish; no structural refactor ofApp/ panels.ScreenManager) is the only landing surface.Ctrl+IfromRunningView; exits viaCtrl+IorEsc.Ctrl+Bretained for one release as undocumented back-compat.[INSPECT]chip while the overlay is active.TuiModevalueboardroom→session,Screenvalueadvanced→inspect,PageKindvalueadvanced→inspect,AdvancedModeWrapper→InspectModeWrapper,handleToggleAdvanced→handleEnterInspect,onToggleAdvanced→onEnterInspect,ADVANCED_HINTS/advanced-hints.ts→INSPECT_HINTS/inspect-hints.ts,ScreenContextvalueboardroom→inspect.docs/tui/information-architecture.md./api/boardroom/*are intentionally untouched (server contract).Closes #191.
Spec:
docs/superpowers/specs/2026-05-14-tui-guided-default-design.mdPlan:
docs/superpowers/plans/2026-05-14-tui-guided-default.mdTest Plan
bun test src/tui/— 1343 pass, 1 skip, 1 pre-existingchokidarenv failure unrelated to this branchboardroom/advancedstrings in user-facing copy (server API path comments excluded)Ctrl+Ienters /Ctrl+Ano-ops;Esc,Ctrl+I, andCtrl+Bexit;[INSPECT]chip renders only when overlay is activegrove up, open a session, pressCtrl+I, confirm overlay + chip; pressEscorCtrl+I, confirm return; press?in both views, confirm new help lineCtrl+Adoes nothing inRunningView