Skip to content

fix(client): preserve inline rename input across tab re-renders#81

Merged
Ark0N merged 1 commit into
Ark0N:masterfrom
aakhter:fix/inline-rename-guard
May 12, 2026
Merged

fix(client): preserve inline rename input across tab re-renders#81
Ark0N merged 1 commit into
Ark0N:masterfrom
aakhter:fix/inline-rename-guard

Conversation

@aakhter
Copy link
Copy Markdown
Contributor

@aakhter aakhter commented May 12, 2026

Summary

Right-click → rename on a session tab injects an <input> into the tab name. While the user is typing, any incoming SSE event that triggers renderSessionTabs() (a sibling session updating, a hook firing, a status change) destroys the input element mid-keystroke and the user loses what they were typing.

This patch adds a _inlineRenameActive flag that:

  • guards the two render paths (renderSessionTabs() and _fullRenderSessionTabs()) so they bail out early while a rename is in progress;
  • is set true when the inline input mounts (session-ui.js);
  • is cleared in finishRename(), which then explicitly calls renderSessionTabs() to restore the normal tab structure.

Also adds a re-entrance guard at the top of finishRename() so the blur event and the Enter keydown don't both fire it (was a latent double-call).

Drive-by: replaces tabName.innerHTML = '' with explicit child removal. The preceding tabName.textContent = '' already clears the element; this avoids an innerHTML write on a node that takes user-supplied content on the next line.

Follow-up to the inline-rename feature cherry-picked from #60.

Test plan

  • Open two sessions; in the second, generate continuous SSE traffic (start a build, run a long command).
  • Right-click the first tab → rename, start typing.
  • Verify the input is not destroyed by activity in the other session.
  • Verify Enter and blur both finalize the rename correctly (only once — no double-call).
  • Verify Esc still cancels (existing behavior, should be unaffected).

🤖 Generated with Claude Code

When the inline session-rename input is open, any incoming SSE event
that triggers renderSessionTabs() (a sibling session updating, a hook
firing, a status change) destroys the input element mid-keystroke and
the user loses what they were typing.

Add a _inlineRenameActive flag that:
- guards the two render paths (renderSessionTabs and
  _fullRenderSessionTabs) so they bail out early while a rename is
  in progress;
- is set true when the inline input mounts (session-ui.js);
- is cleared in finishRename, which then explicitly calls
  renderSessionTabs to restore the normal tab structure.

Also add a re-entrance guard at the top of finishRename so the blur
event and the Enter keydown do not both fire it (was a latent
double-call).

Drive-by: replace tabName.innerHTML = "" with explicit child removal.
The preceding textContent = "" already clears the element; this avoids
an innerHTML write on a node that takes user-supplied content on the
next line.

Follow-up to the inline-rename feature cherry-picked from Ark0N#60.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Ark0N
Copy link
Copy Markdown
Owner

Ark0N commented May 12, 2026

Thanks for the careful fix! Verified that all SSE-triggered renderSessionTabs() call sites (in panels-ui.js, subagent-windows.js, respawn-ui.js) and the lone direct _fullRenderSessionTabs() in settings-ui.js are all covered by the guard. The re-entrance guard on finishRename is a nice catch on the latent async double-fire.

@Ark0N Ark0N merged commit eade261 into Ark0N:master May 12, 2026
1 check passed
Ark0N added a commit that referenced this pull request May 12, 2026
…nd double-fire

Three follow-up fixes to the inline rename input introduced in #81:

1. IME composition guard. Pressing Enter to confirm a Chinese pinyin
   candidate (or any IME composition) was committing the half-composed
   text as the session name. Skip the keydown handler when isComposing
   is true or when keyCode is the legacy 229 sentinel that older
   Safari/Edge versions report on the Enter that triggers compositionend.

2. Ghost tab on mid-rename deletion. If a session was deleted via SSE
   while its tab was being renamed, the render-skip flag suppressed
   _renderSessionTabs() and the orphaned <input> stayed on screen until
   blur — at which point the rename PUT 404'd against the dead session.
   Replace the boolean _inlineRenameActive with a _activeRename
   {sessionId, cancel} object so _cleanupSessionData can abort an
   in-flight rename targeting the deleted session, and finishRename
   skips the API call when the session is gone.

3. Stuck-flag risk. Move the settle-once guard into a closure-local
   `settled` boolean so blur / Enter / Escape / external cancel all
   converge to a single idempotent path. Register _activeRename only
   after the input is fully wired so a throw earlier in setup can't
   strand state.

Adds test/inline-rename.test.ts with 7 Playwright tests that drive
startInlineRename via page.evaluate() against a stubbed session and
synthetic .tab-name node — no real PTY/tmux needed, runs in ~1.3s.

Also fixes test/mobile/helpers/server.ts which imported the WebServer
via a path one directory short of the repo root, breaking the entire
mobile test suite under the main vitest config.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants