fix(hub): allow terminal re-registration after socket reconnect

[hqhq1025]

Summary

Fix "Terminal ID is already in use" error that occurs when web clients reconnect to a terminal session, particularly in PWA environments.

Problem

When a web socket reconnects (common in PWAs after network hiccups or tab backgrounding), the client retains the same terminal ID but receives a new socket ID. The TerminalRegistry.register() method rejected the registration because the old entry still existed in the registry — the disconnect cleanup hadn't completed or the socket never properly disconnected.

This was reported on a Windows runner + Ubuntu hub setup with the PWA web app.

Fix

Make register() handle re-registration gracefully:

• Same socket, same terminal ID → return existing entry (idempotent)
• Different socket, same terminal ID → clean up stale entry, then re-register

Terminal IDs are client-generated UUIDs (crypto.randomUUID()), so cross-client collisions are not a realistic concern. A different socket presenting the same terminal ID is reliably a reconnection, not a conflict.

Changes

Single file: hub/src/socket/terminalRegistry.ts — only the register() method.

Closes #345

[github-actions]

Findings

• [Major] Reconnects can still fail once the session is already at the terminal limit. The new stale-entry replacement in hub/src/socket/terminalRegistry.ts:27 is never reached if terminal:create rejects on hub/src/socket/handlers/terminal.ts:103, so the same terminalId can still get Too many terminals open for this session instead of reattaching.
  Suggested fix:

  const existing = terminalRegistry.get(terminalId)
  const isReconnect = existing?.sessionId === sessionId
  
  if (!isReconnect && terminalRegistry.countForSession(sessionId) >= maxTerminalsPerSession) {
      emitTerminalError(terminalId, `Too many terminals open for this session (max ${maxTerminalsPerSession}).`)
      return
  }
  
  const entry = terminalRegistry.register(terminalId, sessionId, socket.id, cliSocketId)

Summary

• Review mode: initial
• 1 issue found: the reconnect fix is incomplete because the per-session quota check still runs before stale-entry cleanup.

Testing

• Not run (automation): bun is not installed in this runner.
• Not found in repo/docs: a regression test covering reconnect with the same terminalId on a new socket while the session is already at the terminal limit.

HAPI Bot

[github-actions]

[MAJOR] This replacement path still misses one reconnect case: terminal:create rejects on countForSession() before register() is called (hub/src/socket/handlers/terminal.ts:103-114). If the session is already at HAPI_TERMINAL_MAX_TERMINALS, the stale entry is still counted and the reconnect fails with the max-terminals error, so the new cleanup never runs.

Suggested fix:

const existing = terminalRegistry.get(terminalId)
const isReconnect = existing?.sessionId === sessionId

if (!isReconnect && terminalRegistry.countForSession(sessionId) >= maxTerminalsPerSession) {
    emitTerminalError(terminalId, `Too many terminals open for this session (max ${maxTerminalsPerSession}).`)
    return
}

const entry = terminalRegistry.register(terminalId, sessionId, socket.id, cliSocketId)

[github-actions]

Findings

• [Major] Reused terminal IDs can evict an active terminal from another session. hub/src/socket/terminalRegistry.ts:28 now removes any existing entry for a different socket, but the reconnect guard in hub/src/socket/handlers/terminal.ts:98 only treats same-session reuse as valid. Because terminalId is still client-supplied (shared/src/socket.ts:6), a create for another session can silently steal the mapping instead of returning "already in use".

Summary
Review mode: follow-up after new commits
1 remaining issue found: cross-session terminal ID reuse now replaces the old registry entry instead of rejecting the conflict.

Testing
Not run (automation): review policy avoided executing PR code.
Not found in repo/docs: a regression test covering reuse of the same client-supplied terminalId from a different session.

HAPI Bot

[github-actions]

[MAJOR] Replacing any existing entry here is too broad. The new reconnect check in hub/src/socket/handlers/terminal.ts:98 only classifies same-session reuse as reconnect, but register() will still delete an entry from a different session. Since terminalId is client-supplied (shared/src/socket.ts:6), a conflicting create can silently evict the original terminal instead of returning "Terminal ID is already in use."

Suggested fix:

const existing = this.terminals.get(terminalId)
if (existing) {
    if (existing.socketId === socketId) {
        return existing
    }
    if (existing.sessionId !== sessionId) {
        return null
    }
    this.remove(terminalId)
}

[github-actions]

Findings

• No new diff-attributable issues found in the current head.

Summary
Review mode: follow-up after new commits
No remaining diff-attributable issues found after the latest fix. Residual risk: Not found in repo/docs: a regression test covering same-session terminal re-registration alongside the existing terminal handler cases in hub/src/socket/handlers/terminal.test.ts.

Testing
Not run (automation): review policy avoided executing PR code.
Not found in repo/docs: regression coverage for the reconnect / re-registration path.

HAPI Bot