Skip to content

test(desktop): stop the agent's terminal commands in the real app, with real shells and real tmux - #8733

Merged
waleedlatif1 merged 4 commits into
stagingfrom
test/desktop-terminal-cancel-e2e
Oct 7, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
test/desktop-terminal-cancel-e2e

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Terminal cancel ships without a feature flag, so this tests it in the real app, with real shells and a real tmux server. It asserts what is running and which panes exist, never which calls were made.

Builds on #8705, #8720, #8722 and #8725, all on staging.

  • Stop: ends an agent command in a plain shell, and a tagged tmux run, closing only its pane. The user's own command, and the pane the user split beside the agent's, keep running.
  • Sign-out: through the web app's logout, and through the session cookie going away (another account signing in). Every agent command stops, and the user's tmux panes keep running.
  • Switching Terminal off: the same.
  • A tmux run whose chat was put away: sign-out stops it, though no live terminal holds it any more.
  • Launch recovery:
    • after an interrupted sign-out, the previous process's run is stopped;
    • after a crash mid-wait (the call never got its result), it is stopped;
    • a run handed back as running survives a quit and the relaunch, and its pane can still be read;
    • a finished run is only forgotten;
    • a pane that took a recorded id after a tmux restart is never touched.
  • tmux without pane options: using real tmux 2.9a's refusal text, a run goes ahead untracked, and Stop and sign-out leave it alone.

How it is built

  • Agent and user commands ignore SIGHUP, SIGINT and SIGTERM, so only Sim's own stop (which escalates to SIGKILL) can end them, and a stray stop would show.
  • Each launch gets its own tmux server (TMUX_TMPDIR).
  • Every process name carries a per-run suffix, so concurrent runs on one machine never see each other's processes.
  • The fixture Sim and its helpers move to e2e/executor-sim.ts, shared with the background executor suite.
  • The macOS Desktop E2E job installs tmux, so the tmux scenarios run in CI. Without tmux they skip, with the reason given.
  • Each scenario's status and duration go to TERMINAL_CANCEL_REPORT_PATH (test-results/terminal-cancel-report.json in CI, uploaded on failure). A failed scenario attaches its calls, its panes, the app's output and the process list.
  • By design, sign-out and Terminal off close every shell. So a user's command in a plain-shell tab is checked against Stop, and a user's command in a tmux pane is checked against every path.

Type of Change

  • Test coverage

Testing

  • EC2 (Linux, real tmux 3.4): 12/12. Each of 14 guards, removed in turn, turns its scenario red:
    • Stop (plain shell and tmux);
    • user commands and panes left alone;
    • the sign-out, cookie and Terminal-off paths;
    • untracked runs;
    • the launch and sign-out sweeps;
    • the tag check on recorded runs;
    • dropping finished records;
    • keeping collectable runs;
    • marking runs handed back.
  • Cold runs: 120/120 across 10 fresh Playwright invocations on EC2, on this branch rebased onto staging.
  • Mac (no tmux installed): the 4 plain-shell scenarios pass and the 8 tmux scenarios skip.
  • The background executor suite after the fixture move: 11/11 on the Mac.

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)

…th real shells and real tmux

An Electron suite for terminal cancel, which ships without a feature flag:
Stop, sign-out (by the web app's logout and by the session cookie going away),
switching Terminal off, the launch after an interrupted sign-out or a crash,
and untracked runs on tmux without pane options. It asserts what is running
and which panes exist, never which calls were made.

- Agent and user commands ignore SIGHUP, SIGINT and SIGTERM, so only Sim's own
  stop can end them and a stray stop would show; each launch gets its own tmux
  server, and every process name carries a per-run suffix.
- The fixture Sim and its helpers move to e2e/executor-sim.ts, shared with the
  background executor suite.
- The macOS E2E job installs tmux so the tmux scenarios run in CI; elsewhere
  they skip when tmux is missing.
@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 8:26am UTC

Request Review

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@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 4 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] Adds end-to-end tests for terminal command cancellation.

The PR appears safe to merge; no new blocking issue was found.

What we checked:

  • Earlier results survive worker replacement: Each check reads the saved results before adding its own outcome. The configured run uses one worker, so replacement workers preserve earlier checks without overlapping writes.

Summary

Adds real Electron tests for stopping agent terminal commands without closing user-owned tmux panes. Shares the executor fixture and installs tmux in CI.

  • The latest change saves each scenario immediately and keeps earlier worker results, including retries.
  • All five supplied previous threads are unnumbered. Their fixes are present: shared randomness, removed debug prints, saved reports, cookie-loss user-pane checks, and results preserved across worker replacement.
  • No new actionable issues were found.

Reviews (3) · Last reviewed commit: "test(desktop): add each terminal-cancel ..." · Reviewed by Greptile

Comment thread apps/desktop/e2e/terminal-cancel.spec.ts Outdated
Comment thread apps/desktop/e2e/terminal-cancel.spec.ts Outdated
Comment thread apps/desktop/e2e/terminal-cancel.spec.ts
Comment thread apps/desktop/e2e/terminal-cancel.spec.ts Outdated
… them, write a suite report, and keep the user pane through a session ended elsewhere
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@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 4 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

Comment thread apps/desktop/e2e/terminal-cancel.spec.ts Outdated
…ishes, so a retry keeps the original failure
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@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 4 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

@waleedlatif1
waleedlatif1 merged commit 102019b into staging Oct 7, 2026
36 checks passed
@waleedlatif1
waleedlatif1 deleted the test/desktop-terminal-cancel-e2e branch October 7, 2026 16:32

This branch was previously deployed

1 inactive deployment
Preview — 53aab9b7 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