Skip to content

fix(desktop): read tmux format output with a separator no tmux version escapes - #8720

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/desktop-tmux-format-separator
Oct 7, 2026
Merged

waleedlatif1 merged 1 commit into
stagingfrom
fix/desktop-tmux-format-separator

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Sim joins the fields of tmux -F output with a 0x1f byte and splits on it. tmux 3.4 and 3.5 print control characters in format output as octal escapes, so the separator arrived as the four characters \037 and no line split.
  • Effect on those versions:
    • Sim never found the tmux client running in a terminal, so it treated that terminal as a plain shell, and the agent's runs were refused as busy.
    • The panes operation came back empty.
  • The separator is now the printable |~sim~|, which no tmux version escapes. A field that happens to contain it changes the line's field count, so that line is dropped rather than misread.

Why not accept both forms

  • tmux 3.3 and later also escape backslashes (\ prints as \\).
  • So "split on the byte or on \037" could cut a field whose text holds a real \037.
  • Telling the cases apart would mean decoding each version's escaping. A printable separator doesn't depend on escaping at all.

Versions checked (real binaries)

tmux 0x1f separator |~sim~| separator
2.9a raw verbatim
3.3a raw verbatim
3.4 (Ubuntu 24.04) escaped as \037 verbatim
3.5a escaped as \037 verbatim
3.6a raw verbatim
3.7c (current Homebrew) raw verbatim

On real tmux 3.4, listPanes returned nothing with staging's code and the session's panes with this change.

Autopsy

  • The unit tests run against a fake tmux that printed format output unescaped, so this kind of bug could never fail them.
  • The fake now escapes its output the way 3.4 and 3.5 do: backslashes doubled, control characters as octal escapes.
  • The new attachment test goes through that fake. It fails with the old separator and passes with this one.
  • It was found by a new Electron E2E against real tmux 3.4. That E2E follows in a separate PR.

Type of Change

  • Bug fix

Testing

  • Unit tests, red first with the old separator.
  • Real tmux 2.9a, 3.3a, 3.4, 3.5a, 3.6a and 3.7c.
  • Electron E2E with real tmux 3.4.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…n escapes

tmux 3.4 and 3.5 print control characters in -F output as octal escapes, so
the 0x1f field separator arrived as the text \037 and no line split: Sim never
found the tmux client in a terminal, treated it as a plain shell, and the
panes operation came back empty. The separator is now printable text that no
tmux escapes; a field that happened to contain it changes the line's field
count, so that line is dropped rather than misread.

The fake tmux in the unit tests now escapes its output the way 3.4 and 3.5 do,
so this cannot pass unnoticed again.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@vercel

vercel Bot commented Oct 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 7, 2026 3:05am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Changes how the desktop app parses tmux terminal output.

The PR appears safe to merge.

What we checked:

  • Separator collisions cannot shift fields: parseFormatLines rejects records with extra fields before callers read their values.

Summary

Replaces the control-character separator with printable |~sim~| so tmux output remains readable across versions.

  • Both attachment lookup and pane listing use the same separator.
  • Adds an attachment regression test whose fake escapes control characters.
  • waleedlatif1 explicitly accepts dropping records whose fields contain the separator, rather than reading those records incorrectly.

No actionable issues found. Tests were not run during this review.

Reviews (1) · Last reviewed commit: "fix(desktop): read tmux format output wi..." · Reviewed by Greptile

This branch was previously deployed

1 inactive deployment
Preview — b752e116 Deployed Oct 7, 2026 by vercel[bot]
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