Refine hook UX, terminal behavior, and commit graph badges#18
Merged
liam-russell merged 17 commits intomainfrom May 8, 2026
Merged
Refine hook UX, terminal behavior, and commit graph badges#18liam-russell merged 17 commits intomainfrom
liam-russell merged 17 commits intomainfrom
Conversation
- Hook editor defaults to after_worktree_create trigger with trigger_worktree as the intelligently-selected execution target - Form reorganised into logical sections: Identity → When/Where → Behavior → Dependencies → Script - Empty-hooks state restyled with dashed border and centred layout - keep_open_on_completion flag wired through to terminal session autoCloseOnExit: terminal tabs spawned by hooks auto-close when the process exits unless the hook has keep-open enabled - Blank terminal auto-spawn suppressed when a hook launch request is already pending for the same container - Context menu icons unified to lucide components (Repeat, FolderOpen, Copy, Play, Pencil, ShieldAlert) - Commit graph worktree indicator consolidated onto the branch ref badge with GitBranch + TreePine icons side-by-side; tag refs use Tag icon instead of 'tag:' text prefix - E2E panel-count assertion updated to >= 2 to reflect intentional suppression of the blank auto-spawn session when hook terminals are launched
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines the workspace hook and terminal UX by improving hook editor defaults/layout, unifying context-menu icon rendering, and adjusting terminal session lifecycle so hook-launched terminals can auto-close on exit unless configured to remain open. It also updates commit graph ref badges to use consistent iconography and updates an E2E assertion to match the new terminal auto-spawn behavior.
Changes:
- Reworked hook editor defaults (trigger/execution target) and reorganized hook modal UI/empty state.
- Added
autoCloseOnExitplumbing through terminal components and suppressed blank terminal auto-spawn when a hook launch is pending. - Unified context menu icons (lucide components) and refined commit graph ref/worktree/tag badges (GitBranch/TreePine/Tag).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/workspace/+page.svelte | Wires hook keep-open intent into terminal launch requests; replaces string menu icons with lucide components. |
| src/lib/components/WorkspaceHooksModal.svelte | Updates defaults and restructures hook editor sections + improved empty state. |
| src/lib/components/TerminalPanel.svelte | Adds autoCloseOnExit + callback hook for closing tabs after process exit. |
| src/lib/components/TerminalContainer.svelte | Suppresses blank auto-spawn when hook launch pending; stores per-session auto-close behavior and closes sessions automatically on exit. |
| src/lib/components/ContextMenu.svelte | Allows menu icons to be either strings or Svelte components and renders them consistently. |
| src/lib/components/CommitGraph.svelte | Consolidates worktree indicator into ref badge; uses Tag icon and strips tag: prefix for display. |
| e2e/specs/daily-workflow.spec.ts | Updates terminal panel-count assertion to reflect suppressed blank auto-spawn when hook terminals launch. |
Contributor
Author
|
@copilot apply changes based on the comments in this thread |
…d auto-close E2E test - Add keep_open_on_completion field to HookTerminalLaunchEvent Rust struct - Include hook.keep_open_on_completion when emitting hook-terminal-launch event - Add keepOpenOnCompletion: boolean to TypeScript HookTerminalLaunchEvent type - Update frontend handler to use event.keepOpenOnCompletion directly instead of looking up the value from operationHooks (which may be stale/empty) - Add E2E test asserting keepOpenOnCompletion:0 tabs auto-close and keepOpenOnCompletion:1 tabs remain visible after process exit Agent-Logs-Url: https://github.com/InterestingSoftware/SproutGit/sessions/7c4c4258-03ec-40aa-bce3-20dcdfe93741 Co-authored-by: liam-russell <17897133+liam-russell@users.noreply.github.com>
Contributor
Done in 370c67e. Both review comments are addressed:
|
Contributor
Author
|
@copilot tests are failing: 11 e2e/specs/daily-workflow.spec.ts:744:3 › Daily developer workflow › auto-closes terminal session when keepOpenOnCompletion is false
1 failed |
…; fix E2E test - compose_terminal_command now takes exit_after: bool; when true, appends `exit` (POSIX) or `\rexit` (PowerShell) after the hook script so the interactive shell exits upon completion, which triggers onTerminalClosed and the onAutoClosed → closeSession chain that removes the tab - Call site passes !hook.keep_open_on_completion as exit_after - E2E test: add insertHookDependency so keepOpen runs after autoClose, preventing the hookTerminalLaunchRequest reactive-update race - E2E test: drop the simultaneous-visibility assertion; wait for autoClose tab to appear then disappear, then wait for keepOpen tab to appear and verify it remains visible Agent-Logs-Url: https://github.com/InterestingSoftware/SproutGit/sessions/cc8b6b08-9fba-4a2b-9a71-fc5c36179d87 Co-authored-by: liam-russell <17897133+liam-russell@users.noreply.github.com>
Contributor
Fixed in f7330df. Root cause: Fix:
|
Contributor
Author
|
@copilot windows still fails: 10 e2e\specs\daily-workflow.spec.ts:660:3 › Daily developer workflow › runs multiple terminal hooks after create and opens named terminal sessions (1.5m)
2 failed |
Replaces the PTY-input `exit` approach with `-NonInteractive -Command` (PowerShell) / `-c` (POSIX) spawning when keep_open_on_completion=false. On Windows ConPTY the interactive shell keeps its output-buffer open after `exit` is received, so the background reader thread may not detect EOF for tens of seconds — far beyond the 20-second E2E poll window. Spawning the shell non-interactively lets the OS signal EOF immediately when the script process terminates, giving a reliable `terminal-closed` event. Changes: - compose_terminal_command: for exit_after=true, produce semicolon-joined format (PowerShell) / newline format (POSIX) suitable for -Command/-c; remove the appended `exit` command - spawn_terminal: add optional `command: Option<String>` parameter; when set, adds -NonInteractive -Command <cmd> or -c <cmd> to CommandBuilder - spawnTerminal (TypeScript): expose optional `command` parameter - TerminalPanel: when autoCloseOnExit=true and initialCommand is set, pass it to spawnTerminal as the non-interactive command; set sentInitialCommand to prevent the PTY-input $effect from double-submitting Agent-Logs-Url: https://github.com/InterestingSoftware/SproutGit/sessions/887bbe73-825c-49af-8214-6a2ebe8990e8
Copilot stopped work on behalf of
liam-russell due to an error
May 7, 2026 01:29
… process exit on Windows On Windows ConPTY, an interactive bash/PowerShell session does not exit after its hook script finishes. This caused the auto-close terminal E2E test to time out at the not.toBeVisible() assertion. Fix: when keep_open_on_completion=false, spawn the PTY in non-interactive mode via bash -c / powershell -NonInteractive -Command so the shell exits naturally when the script completes, reliably firing terminal-closed. - terminal.rs: add optional command param; use -c/-NonInteractive -Command - hooks.rs: add exit_after flag to compose_terminal_command; emit keep_open_on_completion in hook-terminal-launch event - sproutgit.ts: thread command param through spawnTerminal; add keepOpenOnCompletion to HookTerminalLaunchEvent type - TerminalPanel.svelte: non-interactive spawn when autoCloseOnExit=true
…rminals When two hooks with a dependency relationship both use terminal_tab execution mode, the Rust hook runner considers the first hook complete as soon as it emits hook-terminal-launch, then immediately starts the second hook. Both events arrive at the frontend in rapid succession, and Svelte 5 batches reactive updates — so a single-value hookTerminalLaunchRequest state was overwritten before the TerminalContainer effect could process it, causing the first session to be silently dropped. Fix: replace hookTerminalLaunchRequest (single value) with hookTerminalLaunchRequests (array queue). The TerminalContainer now accepts a launchRequests array prop and tracks processed IDs in a plain Set so every entry is handled exactly once, even when multiple requests arrive in the same reactive batch.
On Windows, the ConPTY master output pipe does not signal EOF when the child
process exits — it blocks indefinitely until the pseudo-console handle is
explicitly closed. This meant terminal-closed was never emitted for
auto-close hook sessions, leaving the terminal tab stuck open forever.
Fix: when spawn_terminal receives a command argument (non-interactive mode),
skip the PTY entirely and use std::process::Command with piped stdout/stderr.
A dedicated thread waits for child.wait(), then emits terminal-closed-{id}.
This is reliably cross-platform: Linux/macOS/Windows all signal process exit
correctly through a plain OS pipe.
The PTY path (command == None) is unchanged and still handles interactive
terminal sessions.
performVerifiedReload was using a hardcoded 15s budget for the waitForMainWindow call after window.location.reload(). On Windows CI the Tauri webview re-initialises from scratch after a reload and can take 20-40 s to reach document.readyState === 'complete'. This was marginal on slower runners and caused spurious beforeEach timeouts (90s test timeout exceeded) after c3453d1 increased the binary size. Use STARTUP_UI_TIMEOUT (45s) for the post-reload wait, consistent with every other window-ready call in reloadToHome. Before: 15_000 ms hardcoded After: STARTUP_UI_TIMEOUT (45_000 ms)
…e CWD handle On Windows, dropping the ConPTY master sends CTRL_CLOSE_EVENT to the attached shell process (typically PowerShell). PowerShell installs a custom console control handler and can take several seconds to run its exit cleanup before finally terminating. During that window the process holds an open handle to its current working directory (the worktree path), causing rmSync to fail with EBUSY when the next E2E test's beforeEach runs resetTestDirs. Fix: store the child process handle in PtySession and call child.kill() at the top of close_terminal, before dropping the master PTY. On Windows this calls TerminateProcess which is unconditional and immediate, ensuring the CWD handle is released before the caller returns.
reloadToHome had four sequential waits each with a STARTUP_UI_TIMEOUT
(45 s) budget:
A: waitForMainWindow (before ensureHome)
D: waitForMainWindow (before reload, inside performVerifiedReload)
F: waitForMainWindow (after reload, inside performVerifiedReload)
G: waitForHomeReady (after reload)
Calls D and F were redundant:
- D is duplicated work: ensureHome just verified the home screen is
visible, meaning the main window is responsive.
- F is subsumed by G: waitForHomeReady polls evaluate() in a loop with
the same isMissingMainWindowError catch, so it already waits for
document.readyState === 'complete' before checking for the import
button.
On a slow Windows CI runner, 4 × ~23 s = 92 s exceeded the 90 s
per-test timeout, causing spurious beforeEach failures on the
auto-close terminal test (and potentially other tests near the end of
the suite where cumulative startup variance accumulates).
Fix: inline the reload into reloadToHome and delete performVerifiedReload.
The new chain is: waitForMainWindow + ensureHome + reload + waitForHomeReady
= 3 waits maximum, giving comfortable headroom on slow runners.
Before: 4 × STARTUP_UI_TIMEOUT (45 s) = 180 s worst case
After: 3 × STARTUP_UI_TIMEOUT (45 s) = 135 s worst case
(in practice 1 s + 5 s + 20 s = ~26 s on a typical run)
On Windows, the keep-open hook terminal left a live PowerShell process with CWD set to the feature worktree directory. This held a directory handle that caused EBUSY in the next test's resetTestDirs() even after the 20-retry × 250ms budget. Two fixes: 1. Add close_all_terminals Tauri command that kills every PTY session in the TerminalManager at once. More reliable than per-panel close_terminal because it catches sessions whose ptyId the frontend may not have observed (spawnTerminal IPC race with page unmount). 2. Workspace onDestroy now calls closeAllTerminals() before stopWatchingWorktrees(). By the time waitForHomeReady returns and resetTestDirs() runs, all PTY children are dead and directory handles are released. Also fix beforeEach reset order in all 6 specs: resetConfigDb() → reloadToHome() → resetTestDirs() resetConfigDb() must run first because the home page fires listRecentWorkspaces() on mount. If the config DB is deleted after reloadToHome() triggers a webview reload, the IPC call races against run_config_migrations closing its rusqlite handle and connect_sqlite opening a fresh empty file — producing 'no such table: recent_workspaces' on macOS CI. The config DB lives in a separate run-scoped directory with nothing holding it open, so deleting it first is safe.
Two CI failures addressed: Windows — beforeEach 90s timeout on 'deletes merged bugfix worktree' reloadToHome() contained a window.location.reload() that took 20–45 s on slow Windows CI runners (WebView cold-start cost), consistently pushing beforeEach past the 90 s per-test timeout. The reload was not needed for isolation: SvelteKit route navigation already tears down and remounts all page components. The Tauri Playwright adapter cheatsheet also recommends UI-driven navigation over hard reloads as the suite default. Fix: move clearCachedWorkspaceHint() BEFORE ensureHome() so the home page onMount never sees the previous workspace hint, then drop the reload and waitForHomeReady entirely. Worst-case beforeEach budget is now two sequential 45 s waits (window + navigation) instead of three. Ubuntu — auto-close tab never appears (line 783, daily-workflow.spec.ts) The auto-close terminal tab exists for only ~300 ms (the shell runs echo + sleep 0.3 then exits, and keepOpenOnCompletion=false removes the tab on exit). On fast Ubuntu CI the tab can be created and destroyed inside the time createWorktreeViaUi() spends polling for the success toast, so waitFor() on the auto-close tab times out. Fix: wait for the keepOpen tab instead — its keepOpenOnCompletion=true hook depends on the auto-close hook, so seeing the keepOpen tab proves the auto-close hook ran and completed. The auto-close tab is then asserted absent, which is the important invariant under test.
Previous fix incorrectly assumed that 'keepOpen tab visible' proves
'autoClose tab gone'. It does not: in terminal_tab execution mode the
hook runner emits hook-terminal-launch and returns success IMMEDIATELY
without waiting for the spawned process to exit, so insertHookDependency
only sequences hook *dispatch order*, not *process completion*.
Real sequence:
1. autoClose hook dispatches -> launch event -> returns success
2. autoClose process runs (echo + sleep 0.3 ~= 300 ms)
3. keepOpen hook dispatches (dep satisfied by step 1) -> launch event
-> keepOpen tab appears
4. autoClose process exits -> terminal-closed event -> tab removed
Steps 2 and 3 run in parallel. Both tabs coexist for the duration of
the autoClose process plus IPC delivery. On macOS CI under load the
close pipeline (PTY exit -> wait-thread -> Tauri IPC -> frontend
listener -> reactive update -> DOM removal) can exceed expect()'s
implicit 5 s retry budget, causing the assertion to fail while the tab
is still visible.
Fix: wait for keepOpen tab to appear (both hooks dispatched), then poll
explicitly for the autoClose tab to disappear with DEFAULT_UI_TIMEOUT
(20 s). Long comment added in the test body explaining the race so
this isn't 'fixed' incorrectly a third time.
…c E2E sync
When a terminal session is spawned with a hook_id, the non-interactive
PTY wait-thread now records the exit timestamp (epoch ms) into a new
TerminalManager.closed_hook_terminals map. A new Tauri command,
is_hook_terminal_closed(hook_id), exposes this state.
This gives callers a deterministic, IPC-free synchronisation point for
'when did the hook process actually exit', upstream of the long chain
that drives terminal-tab disappearance:
PTY child exit
-> wait-thread
-> Tauri event
-> frontend listener
-> Svelte reactive update
-> DOM removal
The auto-close hook E2E test now polls is_hook_terminal_closed() rather
than the DOM, removing the multi-step async chain from its timeout
budget. This is the architectural fix for the recurring Ubuntu-CI
flake on the auto-close terminal-tab assertion.
Plumbed through:
- src-tauri/src/terminal.rs: new closed_hook_terminals map +
hook_id arg + is_hook_terminal_closed
- src-tauri/src/lib.rs: register new command
- src/lib/sproutgit.ts: spawnTerminal hookId + isHookTerminalClosed
- TerminalPanel.svelte: hookId prop forwarded to spawnTerminal
- TerminalContainer.svelte: hookId on Session + TerminalLaunchRequest
- workspace/+page.svelte: set hookId on launch request
- daily-workflow.spec.ts: sync via backend timestamp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release Notes
This release polishes the hook authoring experience and wires up terminal lifecycle behavior so hook-launched terminals clean up after themselves automatically.
after_worktree_createtrigger targeting the new worktree, so new hooks require fewer form interactions to get startedtag:text prefix