Skip to content

docs(spec): add tmux window/rename suppression spec and research - #645

Closed
lavaman131 wants to merge 1 commit into
mainfrom
lavaman131/feature/lockdown-tmux
Closed

docs(spec): add tmux window/rename suppression spec and research#645
lavaman131 wants to merge 1 commit into
mainfrom
lavaman131/feature/lockdown-tmux

Conversation

@lavaman131

@lavaman131 lavaman131 commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds a Technical Design Document (RFC) and supporting research for suppressing destructive tmux actions (window close, rename, Ctrl+C/ESC) in Atomic agent sessions, along with the codebase survey and web reference docs that back it.

Key Changes

  • specs/2026-04-15-tmux-window-close-rename-suppression.md — Full RFC covering:

    • Disabling the tmux prefix entirely (set -g prefix None) on the isolated -L atomic server to eliminate all prefix-gated destructive bindings in one stroke
    • Silently suppressing mouse right-click context menus (MouseDown3Status, MouseDown3Pane) on agent windows
    • Blocking Ctrl+C and Escape in chat-session windows (always) and in workflow agent windows while a stage is active (ATOMIC_STAGE_ACTIVE env var)
    • Hiding the tmux status bar on the orchestrator window (window 0) since OpenTUI's Statusline already renders navigation hints there
    • Keeping q in the orchestrator as the sole exit path (unchanged)
    • ATOMIC_STAGE_ACTIVE lifecycle management (cleared at startup, set/cleared via finally around stage bodies)
  • research/docs/2026-04-15-tmux-window-close-rename-suppression.md — Codebase survey covering tmuxRun(), tmux.conf, session taxonomy, parseSessionName(), abort/exit plumbing, SDK idle/cancellation events (Claude/OpenCode/Copilot), and paused-state primitives (ticket update readme and mcp servers #3)

  • research/web/2026-04-15-tmux-keybind-suppression.md — Web reference covering tmux/psmux default binding tables, if-shell -F conditional semantics, display-message toast mechanics, and hook APIs

Context

This spec is a prerequisite for the paused-state objective (ticket #3): a clean tmux window must survive as a resume target for Claude's interactive/pane-capture path. Without window-close suppression, the paused-state feature cannot land safely.

Notes

  • No runtime code changes in this PR — spec and research only
  • All proposed binding changes are scoped to the -L atomic isolated server; the user's personal tmux is unaffected

@lavaman131 lavaman131 closed this Apr 15, 2026
@claude claude Bot changed the title docs(spec): add specs + research docs(spec): add tmux window/rename suppression spec and research Apr 15, 2026
@claude

claude Bot commented Apr 15, 2026

Copy link
Copy Markdown

Review — docs(spec): add specs + research

Docs-only PR adding a TDD plus two research docs for the tmux lockdown work. Overall this is a very thorough and well-structured package — the research is exhaustively cross-referenced to code (line numbers throughout), open questions are explicitly traced back to user resolutions, and the decision to prefer set -g prefix None over per-key unbind is well-justified. A few concerns worth raising before implementation.

Strengths

  • Exhaustive code cross-referencing. The research doc at research/docs/2026-04-15-tmux-window-close-rename-suppression.md:443-502 anchors every claim to file:line — I spot-checked tmux.ts:138-153 (tmuxRun), tmux.ts:437-451 (setSessionEnv/getSessionEnv), tmux.conf:65 (if-shell Escape pattern), and executor.ts:534,1112 (SharedRunnerState) and all align with the current code.
  • Alternatives table in spec §6 is genuinely comparative — rejects five options (per-key unbind, after-hooks reversal, key-table switch, process-level trap, OpenTUI-level block) with concrete reasons, not strawmen.
  • Socket isolation (§2.1, G5) correctly identified as giving "non-agent tmux unaffected" for free — this is the right architectural observation and explains why the threat model can be scoped to user-inside-tmux.
  • Divergence from original AC (silent vs. toast) is explicitly flagged in §9.1 with rationale, so reviewers aren't surprised.

Critical Concern

#{S:NAME} format variable is load-bearing but unverified on tmux itself, not just psmux.

The entire G3 gate (blocking Ctrl+C/ESC during active stages) depends on #{S:ATOMIC_STAGE_ACTIVE} reading a session environment variable inside if-shell -F (specs/…:196,368,374). The residual item §9.3 only calls out psmux verification, but I could not find #{S:NAME} documented anywhere in the web research (research/web/2026-04-15-tmux-keybind-suppression.md) or the tmux man page excerpt there. The standard tmux format for environment expansion is #{E:NAME} (introduced in tmux 3.2), and session-scoped env var access via formats is not a documented primitive I can locate.

If #{S:NAME} doesn't resolve, the predicate silently evaluates to empty → if-shell goes to false-branch → Ctrl+C always passes through during stages, defeating G3. Recommend verifying against the actual tmux source (format.c) before implementation, and documenting the exact tmux version that introduced support. Fallback options if unsupported:

  • Use #{E:NAME} if the env var propagates to the pane environment (needs a different write mechanism than set-environment).
  • Use if-shell without -F with a shell conditional (test -n \"$ATOMIC_STAGE_ACTIVE\") — works cross-shell if the env var is exported, but triggers /bin/sh vs PowerShell divergence the spec's design explicitly avoids.
  • Encode stage-active state in window_name suffix instead of env var (reachable via #{window_name}).

Other Concerns

  1. session-created hook timing (G9, §5.4.1). The three-hook belt-and-suspenders registers session-created via source-file at tmux.ts:228 — which runs after new-session. For the first session on a cold atomic server, the hook is registered after the session it would have fired for. client-attached should cover the practical path (user sees the UI only after attach), but the spec presents all three as redundant safety nets when one of them structurally cannot fire for the common case. Worth either removing session-created or documenting why it's kept.

  2. Line-number drift in §5.4.2. Spec says "after tmux.createSession(...) at executor.ts:311" but the actual session creation is at executor.ts:1090-1104 via OrchestratorPanel.create({ tmuxSession: … }). The research doc gets this right; the spec should too.

  3. Emergency escape hatch. With set -g prefix None, a user inside a corrupted tmux state has zero in-session recovery — no prefix + : for manual diagnosis, no detach. The spec accepts this, but in practice when this work ships and something goes sideways, maintainers will want an env-var-gated debug path (e.g., ATOMIC_TMUX_DEBUG=1 → set prefix2 to C-b). Consider adding a small hatch now rather than under duress later.

  4. Vestigial bindings in tmux.conf. Lines 33-40 (bind - split-window, bind | split-window, bind -r l/h/k/j resize-pane) become dead code under prefix = None. The spec §3.2 non-goals says they stay — but leaving dead config bindings that silently no-op is the kind of thing that confuses future readers. Either remove them in the same commit, or add a one-line comment # Inert under prefix=None — retained for ref only.

  5. Counter exception-safety (§5.3.2). The shared.activeStageCount++ / -- pattern is sound if wrapped in try/finally. The spec's pseudocode gets this right, but only in pseudo-code — the concrete implementation must preserve the increment-before-try invariant. Worth adding explicit test coverage for "throw between increment and try-block entry" (which today is impossible because the increment and try are adjacent, but trivially breakable if the wrap is refactored).

  6. Test plan specificity (§8.3). tests/sdk/runtime/tmux-keybind-suppression.test.ts is proposed but the "assert that the appropriate action occurred" guidance is thin. Testing "the key was silently dropped" requires either (a) observing no change in some downstream state after send-keys, or (b) hooking into tmux to verify the if-shell branch taken. Neither is straightforward. Suggest the spec enumerate the specific assertion mechanism (e.g., "after sending prefix & &, assert list-windows output is unchanged") before implementation starts, or the tests will drift into hand-waving.

  7. bind -n Escape send-keys Escape round-trip. The false-branch passes Escape back to the pane. With escape-time 0 this should be OK, but sending an actual escape char back into a tmux-managed pty is a subtle path — especially since many CLIs use Escape as a meta-key prefix for other sequences. Worth an explicit manual test line item in §8.4 for "Escape in vim-in-agent-window works normally between stages."

  8. Orchestrator Ctrl+C round-trip. The matrix §5.5 shows orchestrator Ctrl+C as passthrough→OpenTUI, achieved via bind -n C-c false-branch send-keys C-c. This creates a tmux → send-keys → same-pane → OpenTUI round trip. OpenTUI is configured with exitOnCtrlC: false (correctly noted in research §5), so it should be safe, but it's one more indirection than today's "OpenTUI sees Ctrl+C natively" path. Worth validating that the React useKeyboard still fires as expected with the extra tmux hop.

Style / Conventions

  • Matches the research/docs/YYYY-MM-DD-*.md and specs/YYYY-MM-DD-*.md conventions already established in the repo (dozens of sibling files).
  • Spec follows the same TDD structure as specs/2026-04-10-tmux-ux-improvements.md and specs/2026-04-02-enhanced-debug-logging-sdk-integration.md — sections, metadata table, and alternatives matrix are consistent.
  • Mermaid diagrams render; no broken refs found.

Test Coverage

N/A — docs-only. Spec test plan covers the future implementation; see concern #6 above for the specificity gap.

Security

N/A — docs-only, no attack surface changes. The spec's own §7.1 correctly scopes the threat model to accidental destruction, not malicious self-sabotage.


Recommendation: Merge after (a) verifying #{S:NAME} works on tmux (not just psmux) or spelling out the fallback inline, and (b) fixing the executor.ts:311 line reference in §5.4.2. Everything else can be tackled during implementation.

@lavaman131
lavaman131 deleted the lavaman131/feature/lockdown-tmux branch April 17, 2026 20:43
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