feat(system): fence terminal lifecycle generations

[nicodes]

Implements the agent/protocol layer for https://github.com/nicodes/ormos-be/issues/508.

Risk: high. This changes host PTY lifecycle, reconnect ordering, background retry workers, and relay protocol compatibility.

Summary

• add protocol v3 record ID and generation stream fences
• bind PTYs to authoritative lifecycle generations
• reset stale running rows before first tunnel publication and reconcile before accepting reconnect streams
• report exits with joinable retry workers until success, permanent 4xx, or shutdown
• expose TUI running, closing, exited, restart, close, and truthful empty states

Verification

• go test ./... -count=1
• go test -race ./... -count=1
• go vet ./...
• git diff --check

Deliberate gate proof: making initial reconnect reconciliation asynchronous caused TestInitialReconnectReconciliationBlocksStreamAcceptance to fail/hang; synchronous reconciliation was restored before the green runs.

[nicodes]

Initial independent high-risk review

Exact head reviewed: c512a98419214fdffb8650298f46f9389a1c1702
Base: main at 8270c5bdc926a300f5fb590b669ae6aa4a6017cd
Risk: high — PTY/process termination, lifecycle authorization and generation fencing, reconnect ordering, cancellation/retry workers, and a shared agent/backend protocol gate.
Review wave: first independent reviewer; no other reviewer output was read.

Scope and method

I inspected the complete pinned 15-file diff (871 additions, 108 deletions), then traced the changed paths through the surrounding stream dispatch, terminal session, reconnect, persistence HTTP, relay protocol, TUI, and tests. I compared the protocol rollout against the repository's known-good compatibility control in README.md:178-189 and the deployed backend main handshake implementation. I ran go test ./... (packages: ./..., default count, default timeout), go test -race ./internal/system ./relay (same defaults), go vet ./..., and git diff --check; all completed successfully. CI for this exact head was also green (ci, darwin, actionlint). I did not mutate the checkout or alter the pinned head.

Findings

BLOCKING — V3 agent strands against the currently deployed V2/legacy relay

Refs: relay/protocol.go:68-80, internal/system/system.go:801-849, internal/system/system.go:917-925; compatibility control README.md:178-189.

This head unconditionally advertises X-Ormos-Stream-Fence-Version: 3 and, before opening the tunnel, makes /system/terminal-sessions/reset a mandatory successful step. The backend main handshake currently accepts only versions 1 and 2 (the V3 support is in the separate, still-open backend PR #509), so an agent released from this head against the deployed backend receives 426 unsupported relay action protocol and never connects. Even if the handshake were accepted by an intermediate backend, an older backend without the reset route causes the agent to retry reset forever and never reach connectAndServe.

Reachable scenario/material impact: install/update this agent while the hosted backend is still on its current main/deployed version (the normal mixed-version rollout), start it with a valid pairing token, and it remains offline indefinitely; existing users lose tunnel, proxy, and terminal access. This violates the repository's explicit rollout order (backend compatibility first, then agent) and the issue acceptance criterion that mixed-version deployment fail safely rather than strand @latest. The PR does not gate publication on backend #509 or provide a compatibility fallback; the protocol version bump and mandatory reset are the changed behavior that creates the break.

Required acceptance criteria: either (1) land/deploy backend compatibility for V3 and the reset/terminal lifecycle routes before publishing this agent, with an enforced dependency/release gate, or (2) retain a mixed-version-compatible handshake/agent path that negotiates capabilities and does not require lifecycle endpoints against an older backend. Add an integration/compatibility test proving an old backend plus this agent remains usable for non-lifecycle operation and that the new lifecycle path is only enabled after V3/reset support is confirmed.

Disposition

No other evidenced reachable material blockers were found in the pinned diff. The PTY normalization, generation checks, reconnect reconciliation ordering, input-worker accounting/cancellation, sealing/protocol framing, and TUI stale-message handling were traced and covered by the changed tests; the local full and race test runs above passed. Residual risk remains on cross-repository deployment sequencing and real backend/browser interoperability, which this repository's tests cannot establish.

NEEDS ATTENTION

[nicodes]

Independent high-risk initial review

GitHub refused gh pr review --request-changes on this identity: Review Can not request changes on your own pull request (addPullRequestReview). Publishing the same packet here.

Risk: high. This head changes host PTY lifecycle, reconnect ordering, background retry workers, and relay protocol compatibility (v3 record/generation fences).

Pinned head reviewed: c512a98419214fdffb8650298f46f9389a1c1702 (matches git rev-parse HEAD and gh pr view headRefOid).

Method: Complete git diff main...HEAD of all 15 files (+871/-108) before reporting. Read the current system.go, terminal_sessions.go (including close/attachConnSize), streams.go, tui.go, tui_terminal.go, relay/protocol.go, relay/contract.go, and every test hunk in this PR. Control-compared terminal() lock scope to git show main:internal/system/terminal_sessions.go, and compared TUI create/close against enforcePolicy/closeTerminals lock discipline plus the backend reset/close contract (ResetSystemTerminalSessions marks every non-exited row exited; coordinateTerminalClose waits for this agent to kill the exact generation after a payloadless nudge). Did not read other review comments. Did not execute tests; CI at this head was already green (actionlint, ci, darwin).

---

BLOCKING

1. TUI can create and spawn a PTY before process-start reset; reset then exits that row and reconnect reconcile kills the shell

runTUI starts Run concurrently with the dashboard (internal/system/tui.go:65-66). Run refuses /system/connect until POST /system/terminal-sessions/reset succeeds, then sets sticky resetDone (internal/system/system.go:824-848). TUI create/attach does not consult resetDone (tui.go:293-300, 387-399; terminal_sessions.go:478-490).

Backend reset is not “unused leftovers only”: it lists every non-exited row for the system and reports each exited. A reachable sequence:

1. Agent starts. Reset is in flight, or retrying with the same 1s→30s backoff used for tunnel failures (system.go:831-844).
2. Operator presses + terminal. createTerminalSession persists state=running and AttachTerminal starts a host PTY (terminal_sessions.go:745-828 skips nothing for a just-created running row).
3. Reset then marks that same row exited.
4. The first successful connect runs reconcileTerminalSessionsSync before Accept (system.go:953-958). reconcileSnapshot closes any mapped PTY whose durable row is not running at the same generation (system.go:600-609).

The operator’s first shell dies, and the tab flips to exited. The window is one HTTP RTT on a healthy reset and seconds to minutes while reset 5xx/404 retries — the exact mixed-version startup this rollout uses.

This is new versus main, which had no reset and connected immediately.

Required fix: When RelayURL is set, do not persist or spawn a local PTY until resetDone is true. Cover it with a test that a create/attach attempted during a held reset never inserts d.terminals and never starts a process, and that /system/connect is not dialed until reset returns 2xx.

2. terminal() / reconcileSnapshot hold lifecycle and session locks across relay HTTP (and close)

	d.lifecycleMu.Lock()
	defer d.lifecycleMu.Unlock()
	d.terminalMu.Lock()
	defer d.terminalMu.Unlock()
	// ...
	if h.TerminalRecordID != "" && d.cfg.RelayURL != "" {
		ctx, cancel := context.WithTimeout(d.lifecycleContext(), terminalLocalActionTO)
		rows, err := d.fetchTerminalSessions(ctx)

terminalLocalActionTO is 10s. reconcileSnapshot also holds lifecycleMu for the list fetch and the subsequent s.close() loop (system.go:587-610). close() kills the process group before terminalMu, then blocks on that mutex to unregister and reportTerminalExit (terminal_sessions.go:1680-1708).

Reachable on any production agent (RelayURL set): a create/reattach that misses the in-memory map performs a relay round-trip while both mutexes are held. A slow, hung, or hostile paired relay freezes for up to 10s:

• every other create/reattach (lifecycleMu)
• event/reconnect reconcile, which coordinated close depends on (coordinateTerminalClose waits 4s for this agent to observe closing and report exit)
• enforcePolicy (terminal_sessions.go:839)
• close() map delete + exit report, so a session that already set closed=true stays in d.terminals and a later attach takes the reattach branch and returns terminal session is closed (900-904) instead of creating the new generation

Control: main’s terminal() held terminalMu only around map/PTY start and did no HTTP. enforcePolicy and closeTerminals snapshot under terminalMu and close outside it (streams_test.go:50-64 documents that holding terminalMu across close() deadlocks). proxyPortAllowed fetches with no session lock (system.go:411-421).

Required fix: Do not hold terminalMu or lifecycleMu across fetchTerminalSessions or s.close(). Validate the durable row, then re-check the map under the locks only for the insert/mismatch decision.

---

BLOCKING TEST GAP

3. Claimed close / reset-before-publish invariants have no failing test

This head adds TUI d → deleteTerminalSession and KindEvent → reconcileTerminalSessions() as the agent half of explicit close. Backend close marks the row closing, nudges, and waits for this process to kill that record+generation. Nothing in this PR proves:

• a KindEvent (or a durable closing/missing row) terminates the exact mapped PTY and reports that generation on /exit
• TUI/agent DELETE of a running tab is not accepted while the PTY remains in d.terminals
• /system/connect is not dialed until reset 2xx; a failing reset never reaches connectAndServe

TestInitialReconnectReconciliationBlocksStreamAcceptance does lock reconcile-before-Accept. TestLifecycleGenerationMismatchSchedulesExactStalePTYClose and the TUI restart test do not plant a live PTY under a closing row or a held reset. Issue 508 requires agent tests for close and agent-restart; those gates are absent, so the close path can regress to the pre-PR “delete the row, leave the shell” behavior without going red.

Required tests: one close/reconcile test that starts a real mapped session, injects a non-running or absent authoritative row, and asserts process teardown + exact-generation /exit; one reset-order test that a blocked/failing reset never hits /system/connect.

---

FOLLOW-UP

4. restartTerminalSession accepts incomplete or wrapped JSON that createTerminalSession rejects

system.go:726-741 returns any unmarshal with a non-empty ID, or unwraps {"session":...} with no State/Generation/SessionID check. Create in the same PR requires running + positive generation + session id (713-715). Current backend writes a flat terminalSessionInfo, so the wrapper path is dead and an id-only body becomes m.notice = "terminal created" then terminal is not running (tui.go:392-394, tui_terminal.go:88-90).

Rationale: decoder inconsistency next to a new privileged restart.
Acceptance: restart uses the same validity predicate as create; add a decoder test for {id}-only and wrapped bodies.

5. Generation mismatch schedules close and fails the attach instead of replacing

terminal_sessions.go:720-723 returns generation mismatch; existing session is closing and go s.close(). After finding 1, or any window where a mapped PTY outlives an exited durable row, TUI Enter→restart attaches generation N+1, hits the old PTY, and fails; the operator must retry after terminalKillGrace (2s).

Rationale: restart is specified to start one new shared PTY for the same tab.
Acceptance: attach of the authoritative running generation closes the stale PTY and then creates/returns the new session in the same call, with a test that plants gen 1 and attaches gen 2.

6. TUI close/restart presentation is incomplete

Exited Enter reuses terminalCreatedMsg / “terminal created” (tui.go:387-394, 540-546). Footer hints still say “enter open” and never mention d delete or restart (928-929). Unknown/empty State is attach-attempted then “terminal is not running”; non-running non-exited is reported as “closing” with no View suffix (549-551, 857-861).

Rationale: the PR claims truthful running/closing/exited/restart/close empty states.
Acceptance: View tests for (exited) / (closing); hints and notices that match the action actually taken.

7. Project delete still only hits HTTP

tui.go:587-589 deletes the project record and refreshes lists. Local PTYs for that project close only if a later KindEvent/reconnect reconcile runs. Issue 508 requires project delete to close live PTYs.

Rationale: leftover shells after project delete.
Acceptance: deleting a project (or observing its rows vanish) closes matching d.terminals entries in this process, with a test.

---

Investigated and not blocking: v3 advertisement and missing-record/generation refusal before handshake (TestTerminalLifecycleHeaderBoundaryPrecedesHandshake); exit-report generation + cancel after runCtx is set; url.PathEscape on new record-id paths; sticky resetDone after a later failed connect (intended once-per-process reset); event acceptance during initial reconcile (tested). Production Run assigns runCtx before spawning workers; a runCtx == nil Background retry is latent in tests, not a production Run leak.

Residual risk: I did not execute go test locally. I did not review browser/backend PRs, only the backend contract needed to judge this agent’s reset/close behavior.

NEEDS ATTENTION