Skip to content

fix: remove activePtyPath short-circuit that blocked pty respawn#29

Merged
jmaxdev merged 1 commit intoTrixtyAI:mainfrom
matiaspalmac:fix/terminal-strictmode-race
Apr 19, 2026
Merged

fix: remove activePtyPath short-circuit that blocked pty respawn#29
jmaxdev merged 1 commit intoTrixtyAI:mainfrom
matiaspalmac:fix/terminal-strictmode-race

Conversation

@matiaspalmac
Copy link
Copy Markdown
Contributor

Changes

Removes the activePtyPath.current short-circuit in Terminal.tsx. Under React Strict Mode (default in dev), the ref was set at the top of the PTY effect before any async work ran. If the Strict Mode synthetic unmount fired before spawn_pty completed, the remount saw activePtyPath.current === targetPath and skipped setupPty entirely, leaving the backend PTY unspawned and the terminal blank.

The optimization was trying to avoid redundant respawns when deps fire with the same path, but:

  • React already bails out of effects when primitive deps are unchanged (Object.is), so the additional guard rarely helps.
  • kill_pty + spawn_pty is cheap — dropping the old state and spawning a fresh shell at the same path is fine for the cases that would slip through.
  • The guard caused a real, reproducible bug in dev.

Removed the activePtyPath ref and the two lines that read/wrote it. Everything else (listener lifecycle, fit() before spawn, passing rows/cols to spawn_pty) is unchanged.

Issues

Copilot AI review requested due to automatic review settings April 19, 2026 03:14
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a dev-only React Strict Mode race where the Terminal PTY session could fail to spawn on first open (issue #28) by removing an effect-level short-circuit that prevented setupPty() from running after a Strict Mode remount.

Changes:

  • Removed activePtyPath ref used to track the “last spawned” path.
  • Removed the early-return guard that skipped PTY setup when targetPath matched the ref.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jmaxdev jmaxdev force-pushed the fix/terminal-strictmode-race branch from 9a5e6b6 to aa9762d Compare April 19, 2026 03:47
@jmaxdev jmaxdev self-requested a review April 19, 2026 03:57
@jmaxdev jmaxdev merged commit d4b13d7 into TrixtyAI:main Apr 19, 2026
4 checks passed
@matiaspalmac matiaspalmac deleted the fix/terminal-strictmode-race branch April 19, 2026 04:05
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.

[Bug]: Terminal stays empty and does not open at folder path on first attempt

3 participants