feat(terminal): add an embedded terminal panel by spandan11106 · Pull Request #1089 · lgse/strata

spandan11106 · 2026-09-17T12:37:04Z

Description

Adds a window-scoped embedded terminal below the browser, toggled with F4. This is the first of the three steps sketched in #93: a provider-agnostic terminal foundation, with browser-to-terminal cwd synchronisation and the AI agent actions to follow as separate changes.

vte4 provides the terminal; no PTY or emulation code of our own.
The shell spawns lazily on first reveal, in the active local folder. Trash and non-native locations yield no working directory rather than a fabricated local path.
F4 hides and shows the panel without disturbing the session. The × in the panel header ends it, so the next reveal starts a fresh shell wherever the browser has moved to. Typing exit does the same.
While VTE holds focus it owns keyboard input, so Delete, Ctrl+C, Ctrl+V and the arrows cannot act on files behind the panel.
Background, foreground, cursor, selection and font follow the active theme and update live. A theme's own syntax colors fill the ANSI palette when it carries a real spread of hues; otherwise VTE's sixteen stay in place, because the accent-derived fallback collapses green, yellow, blue, magenta and cyan into one hue and makes git diff and ls output unreadable.
Ctrl+T and the external-terminal behavior are unchanged, and remain the fallback for locations the panel cannot open.
VTE is added to the packaging and build dependency lists, including the e2e base image, which needs one rebuild before the container suites will run.

Not included, deliberately: the terminal does not follow navigation. Injecting cd into a live shell needs a reliable signal that the shell is at a fresh, empty prompt, which VTE only provides from 0.80 — later than the version on the current base image. That work is tracked in #1103. A new session still starts in the folder being browsed, and closing the session re-targets the next one.

Visual evidence

Posted in a comment below: toggling with F4, resizing, the session surviving hide and show, and live theming.

How to test

Install VTE (vte4 on Arch, libvte-2.91-gtk4-dev on Debian/Ubuntu) and build.
Open a local folder and press F4. The panel opens with a shell already in that folder.
Run something (ls, pwd), drag the divider to resize, then press F4 twice. The same session comes back with its scrollback intact.
With the terminal focused, press Delete and Ctrl+A. Neither touches the file listing behind the panel.
Navigate to a different folder, click the × in the panel header, then press F4 again. A fresh shell starts in the folder you moved to.
With the panel open, change the theme in Settings, and change the text size with Ctrl + / Ctrl -.
Press Ctrl+T and confirm the external terminal still opens as before.

Expected result: the panel opens in the active folder, survives hiding and navigation, releases every key to the shell while focused, restarts in the current folder after being closed, and repaints with the rest of the interface when the theme or text size changes.

Related issue

Refs #93

This is the first of the three changes planned on that issue, so it does not close it. #93 stays open until the AI agent actions land on top of this terminal.

Add a window-scoped terminal panel below the browser, toggled with F4. The shell spawns on first reveal in the active local folder and survives hiding and navigation; the close control ends the session so the next reveal starts fresh wherever the browser has moved to. While VTE holds focus it owns keyboard input, so file shortcuts cannot act on the browser behind it. Colors, selection and font follow the active theme; a theme's own syntax colors fill the ANSI palette when it carries a real spread of hues, otherwise VTE's sixteen stay in place so diff and ls output remains readable. Ctrl+T and the external terminal behavior are unchanged. Refs lgse#93

spandan11106 · 2026-09-17T12:39:41Z

screenrecording-2026-09-17_18-04-43.mp4

guisilveira · 2026-09-17T14:49:23Z

Really nice work overall. This is very close to the direction we discussed in #93. The window-scoped VTE panel, F4 toggle, keyboard ownership, separation from the existing external terminal, lazy spawn, packaging updates, and keeping cwd synchronization out of this PR all look aligned with the proposed foundation.

I tested the current head locally and found two blocking issues that I think we should fix before merging, plus one missing test case:

1. Non-native locations can open the terminal in Strata's process cwd

terminal_directory() correctly returns None for Trash/non-native locations, but spawn_if_needed() still calls spawn_async() with no working directory.

In VTE, a missing working directory means the child inherits the application's cwd, so the embedded terminal ends up opening in an unrelated local directory.

I reproduced this from Trash: pressing F4 successfully opened the terminal, and pwd showed a local directory instead of the terminal being unavailable for that location.

I think None here should mean "cannot start an embedded terminal for this location" rather than falling through to VTE's cwd behavior.

The same consideration applies to a native path that cannot be represented in the format VTE expects. Silently inheriting Strata's cwd would be misleading there too.

How to reproduce

Launch the PR branch normally.
Open Trash in Strata.
Press F4.
Run:

pwd

The embedded terminal opens successfully and reports a local directory, even though the current Strata location has no native local path.

I recorded a video showing this reproduction and attached it below.

bug-1.mp4

2. `TerminalPanel` currently keeps itself alive through strong Rc cycles

There appear to be two self-owned callback cycles:

PanelState
→ VTE terminal
→ child_exited callback
→ TerminalPanel
→ Rc<PanelState>

and:

PanelState
→ widget tree
→ close button
→ clicked callback
→ TerminalPanel
→ Rc<PanelState>

I confirmed this with a small regression test using Rc::downgrade().

Before dropping the external TerminalPanel:

strong refs before drop: 3

After drop(panel), the weak reference could still be upgraded:

LEAK: PanelState is still alive after TerminalPanel was dropped;
remaining strong refs: 3

The upgrade() itself temporarily adds one strong ref, so this means two strong references were still keeping the state alive after the owning TerminalPanel was dropped, which lines up with the two callbacks above.

Using weak captures for callbacks owned by the panel/widget tree should break those cycles. I'd also keep this regression test afterward so the window-scoped lifecycle remains protected.

How to reproduce

I temporarily added this regression test to src/ui/terminal_panel/tests.rs:

#[test]
fn terminal_panel_releases_state_after_drop() {
    crate::test_support::gtk_test(
        "ui::terminal_panel::tests::terminal_panel_releases_state_after_drop",
        || {
            let preferences = super::ThemeManager::shared();

            let panel = super::TerminalPanel::new(
                &preferences,
                std::rc::Rc::new(|| None),
            );

            let weak_state = std::rc::Rc::downgrade(&panel.state);

            eprintln!(
                "strong refs before drop: {}",
                std::rc::Rc::strong_count(&panel.state)
            );

            drop(panel);

            match weak_state.upgrade() {
                None => {
                    eprintln!("PASS: PanelState was released");
                }
                Some(state) => {
                    panic!(
                        "LEAK: PanelState is still alive after TerminalPanel was dropped; \
                         remaining strong refs: {}",
                        std::rc::Rc::strong_count(&state)
                    );
                }
            }
        },
    );
}

Then run:

./scripts/test-headless.py ui::terminal_panel::tests::terminal_panel_releases_state_after_drop

On the current PR head, it fails with:

strong refs before drop: 3

LEAK: PanelState is still alive after TerminalPanel was dropped;
remaining strong refs: 3

I've attached the full log of the failing test output below.

  ./scripts/test-headless.py ui::terminal_panel::tests::terminal_panel_releases_state_after_drop
mise config files in /home/guilherme are not trusted. Trust them? Yes
mise config files in /home/guilherme/Projects/strata are not trusted. Trust them? Yes
mise WARN  Remote versions cannot be fetched for astral-sh/uv: HTTP timed out after 3.00s for https://api.github.com/repos/astral-sh/uv/releases?per_page=100 (change with `fetch_remote_versions_timeout` or env `MISE_FETCH_REMOTE_VERSIONS_TIMEOUT`).: error sending request: operation timed out
mise WARN  Remote versions cannot be fetched for openai/codex: HTTP timed out after 3.00s for https://api.github.com/repos/openai/codex/releases?per_page=100 (change with `fetch_remote_versions_timeout` or env `MISE_FETCH_REMOTE_VERSIONS_TIMEOUT`).: error sending request: operation timed out
mise WARN  Remote versions cannot be fetched for anomalyco/opencode: error decoding response body for url (https://api.github.com/repos/anomalyco/opencode/releases?per_page=100): request or response body error: operation timed out
mise WARN  Failed to resolve tool version list for codex: [/home/guilherme/.config/mise/config.toml] codex@latest: unable to fetch versions for codex: HTTP timed out after 3.00s for https://api.github.com/repos/openai/codex/releases?per_page=100 (change with `fetch_remote_versions_timeout` or env `MISE_FETCH_REMOTE_VERSIONS_TIMEOUT`).: error sending request: operation timed out
mise WARN  Failed to resolve tool version list for uv: [/home/guilherme/.config/mise/config.toml] uv@latest: unable to fetch versions for uv: HTTP timed out after 3.00s for https://api.github.com/repos/astral-sh/uv/releases?per_page=100 (change with `fetch_remote_versions_timeout` or env `MISE_FETCH_REMOTE_VERSIONS_TIMEOUT`).: error sending request: operation timed out
mise WARN  Failed to resolve tool version list for opencode: [/home/guilherme/.config/mise/config.toml] opencode@latest: unable to fetch versions for opencode: error decoding response body for url (https://api.github.com/repos/anomalyco/opencode/releases?per_page=100): request or response body error: operation timed out
rust@1.98.1 info: syncing channel updates for 1.98.1-x86_64-unknown-linux-gnu
rust@1.98.1 info: latest update on 2026-09-03 for version 1.98.1 (48a229cea 2026-09-01)
rust@1.98.1 info: component clippy is up to date
rust@1.98.1 info: component rust-analyzer is up to date
rust@1.98.1 info: component rust-docs is up to date
rust@1.98.1 info: component rustfmt is up to date
rust@1.98.1 info: checking for self-update (current version: 1.29.1)
✓ installed 1 tool in 1.6s: rust@1.98.1
mise WARN  trusted_config_paths in non-global config /home/guilherme/.config/mise/config.toml is ignored for security reasons
mise WARN  yes in non-global config /home/guilherme/.config/mise/config.toml is ignored for security reasons
    Blocking waiting for file lock on build directory
   Compiling strata v0.18.0 (/home/guilherme/Projects/strata)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 27.43s
     Running unittests src/main.rs (target/debug/deps/strata-cf5e540b095310a5)

running 1 test

running 1 test
strong refs before drop: 3

thread 'ui::terminal_panel::tests::terminal_panel_releases_state_after_drop' (137007) panicked at src/ui/terminal_panel/tests.rs:78:21:
LEAK: PanelState is still alive after TerminalPanel was dropped; remaining strong refs: 3
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/std/src/panicking.rs:679:5
   1: core::panicking::panic_fmt
             at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/core/src/panicking.rs:80:14
   2: strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}
             at ./src/ui/terminal_panel/tests.rs:78:21
   3: strata::test_support::gtk_test_with_env::<&str, &std::ffi::os_str::OsStr, core::iter::sources::empty::Empty<(&str, &std::ffi::os_str::OsStr)>, strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}>
             at ./src/test_support.rs:55:9
   4: strata::test_support::gtk_test::<strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}>
             at ./src/test_support.rs:35:5
   5: strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop
             at ./src/ui/terminal_panel/tests.rs:54:5
   6: strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}
             at ./src/ui/terminal_panel/tests.rs:53:46
   7: <strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0} as core::ops::function::FnOnce<()>>::call_once
             at /home/guilherme/.rustup/toolchains/1.98.1-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   8: <fn() -> core::result::Result<(), alloc::string::String> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test ui::terminal_panel::tests::terminal_panel_releases_state_after_drop ... FAILED

failures:

failures:
    ui::terminal_panel::tests::terminal_panel_releases_state_after_drop

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 1944 filtered out; finished in 0.14s

test ui::terminal_panel::tests::terminal_panel_releases_state_after_drop ... FAILED

failures:

---- ui::terminal_panel::tests::terminal_panel_releases_state_after_drop stdout ----

thread 'ui::terminal_panel::tests::terminal_panel_releases_state_after_drop' (137005) panicked at src/test_support.rs:81:5:
ui::terminal_panel::tests::terminal_panel_releases_state_after_drop failed
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/std/src/panicking.rs:679:5
   1: core::panicking::panic_fmt
             at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/core/src/panicking.rs:80:14
   2: strata::test_support::gtk_test_with_env::<&str, &std::ffi::os_str::OsStr, core::iter::sources::empty::Empty<(&str, &std::ffi::os_str::OsStr)>, strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}>
             at ./src/test_support.rs:81:5
   3: strata::test_support::gtk_test::<strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}>
             at ./src/test_support.rs:35:5
   4: strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop
             at ./src/ui/terminal_panel/tests.rs:54:5
   5: strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0}
             at ./src/ui/terminal_panel/tests.rs:53:46
   6: <strata::ui::terminal_panel::tests::terminal_panel_releases_state_after_drop::{closure#0} as core::ops::function::FnOnce<()>>::call_once
             at /home/guilherme/.rustup/toolchains/1.98.1-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   7: <fn() -> core::result::Result<(), alloc::string::String> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/48a229ceaefd4985c50990b14116b6d856af0985/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    ui::terminal_panel::tests::terminal_panel_releases_state_after_drop

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 1944 filtered out; finished in 0.23s

error: test failed, to rerun pass `--bin strata`

3. I'd like one real E2E test for the terminal before this is ready

The GTK tests added here are useful, especially for F4 and keyboard ownership, but the E2E changes currently only add F4 support to the harness/environment.

Since this introduces VTE as a native dependency and a real subprocess/PTY boundary, I think we should have at least one end-to-end scenario that:

open Strata in a known temp directory
→ press F4
→ run a command in the real VTE terminal
→ verify that command observes the expected cwd

Ideally it could also hide/show the panel and verify that the same session remains usable afterward.

That would cover the actual integration boundary instead of only the surrounding GTK state.

How to reproduce / verify the current gap

Looking at the PR's tests/e2e changes, the harness gains support for sending F4 and the E2E image gains the VTE dependency, but there is no new scenario that actually interacts with the terminal.

A useful regression test would:

Start Strata in a known temporary directory.
Press F4.
Type a command into the real VTE terminal, for example writing pwd to a temporary file.
Read that file from the test harness.
Assert that it matches the directory Strata was browsing.
Optionally hide/show with F4 and verify that the same session remains usable.

I also looked into the async spawn lifecycle because child == None while spawn_async() is pending looked potentially racy. I tried reproducing duplicate/orphan shells with rapid F4 toggles and immediate F4 → close cycles using an instrumented $SHELL, but I couldn't reproduce incorrect behavior, so I wouldn't block this PR on that without stronger evidence.

Other than the issues above, the implementation looks very solid and well aligned with the architecture we discussed. I don't think this needs a redesign. The blockers look fairly localized.

VTE inherits the application's working directory when it is spawned without one, so Trash and other locations with no native path opened a shell in whatever directory Strata was running from. A directory that cannot be represented as UTF-8 fell through the same path. The panel now treats a missing directory as "no session here": it reveals itself with a line saying the terminal is only available for local folders and spawns nothing. Ctrl+T keeps reporting its own error for those locations. The panel also held itself alive. Its child-exited handler, its close button and its spawn completion callback all captured the panel, so the state outlived the window that owned it. All three now hold a weak reference, with a regression test over the drop. Adds end-to-end coverage across the real PTY boundary: a shell runs in the browsed directory and survives hiding the panel, and a location without a local path starts no shell at all. Refs lgse#93

spandan11106 · 2026-09-17T15:28:22Z

Thanks for the thorough pass, and for the reproductions — both blockers were real and both are fixed.

1. Non-native locations could open a terminal in Strata's cwd

Confirmed and fixed. spawn_if_needed() passed directory.as_deref() straight to spawn_async(), so None meant "inherit", not "refuse". The comment I had left there even described the fallthrough as acceptable for an unrepresentable path, which was wrong for the same reason.

None now means the panel cannot start a session. A location without a usable local path — Trash, a remote without a native path, or a path that is not valid UTF-8 — reveals the panel with a line saying the embedded terminal is only available for local folders, and no child is spawned. Ctrl+T still reports its own error for those locations, unchanged.

2. Strong Rc cycles kept `PanelState` alive

Confirmed. Your test failed here exactly as you described. Two callbacks owned the panel: the child-exited handler and the close button's clicked handler. The spawn completion callback was a third, which your test could not reach because it only fires once a shell has actually started.

All three now capture Rc::downgrade(&state) and rebuild the panel on upgrade. I have kept the regression test, lightly reworded, as the_panel_releases_its_state_when_dropped.

3. An end-to-end scenario for the terminal

Added tests/e2e/scenarios/test_embedded_terminal.py, with the flow you described:

Open Strata in the fixture directory, press F4, run pwd > cwd.txt in the real VTE, read the file from the harness, and assert it matches the browsed directory.
Then echo $$ before and after hiding and showing the panel, and assert the pid is unchanged — the same session survives the toggle.

A second scenario covers the first blocker across the real PTY boundary: in Trash, pressing F4 and typing a command writes no file, because no shell exists to run it. That fails on the previous head, where the inherited cwd made it succeed.

Note that the E2E base image had to be rebuilt for this branch, since the Dockerfile gained the VTE runtime and development packages. The published base will need republishing before CI can use it.

On the async spawn lifecycle

Agreed, and thanks for trying to break it. child == None while spawn_async() is pending is now slightly more than a race question, because the completion callback holds only a weak reference: if the panel is gone by the time the spawn finishes, the callback does nothing rather than resurrecting state. I could not reproduce duplicate or orphan shells either.

The panel resolved its folder the way Ctrl+T does, which in Columns mode prefers the pointer-hovered column. Opening it after clicking into a subfolder could therefore start the shell in the parent the pointer happened to rest on rather than the folder the window says it is showing. It now follows the active location. Refs lgse#93

…inal # Conflicts: # src/ui/theme.rs

This was referenced Sep 17, 2026

feat(terminal): follow browser navigation in the embedded terminal spandan11106/strata#1

Closed

feat(terminal): follow browser navigation in the embedded terminal #1093

Closed

spandan11106 mentioned this pull request Sep 17, 2026

feat(terminal): add Open with AI agent actions #1094

Draft

spandan11106 marked this pull request as ready for review September 17, 2026 20:20

spandan11106 requested a review from l0gicgate September 17, 2026 20:21

guisilveira and others added 3 commits September 18, 2026 09:39

fix(terminal): harden embedded session lifecycle

b49c89f

Merge remote-tracking branch 'origin/main' into feat/93-embedded-term…

c04cf09

…inal # Conflicts: # src/ui/theme.rs

test(terminal): extend the theme fixture for per-theme syntax colors

6232424

Navigation Menu

Uh oh!

feat(terminal): add an embedded terminal panel - #1089

feat(terminal): add an embedded terminal panel#1089
spandan11106 wants to merge 6 commits into
lgse:mainfrom
spandan11106:feat/93-embedded-terminal

spandan11106 commented Sep 17, 2026 •

edited

Loading

Uh oh!

spandan11106 commented Sep 17, 2026

Uh oh!

guisilveira commented Sep 17, 2026

Uh oh!

spandan11106 commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

spandan11106 commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Visual evidence

How to test

Related issue

Uh oh!

spandan11106 commented Sep 17, 2026

Uh oh!

guisilveira commented Sep 17, 2026

1. Non-native locations can open the terminal in Strata's process cwd

How to reproduce

2. TerminalPanel currently keeps itself alive through strong Rc cycles

How to reproduce

3. I'd like one real E2E test for the terminal before this is ready

How to reproduce / verify the current gap

Uh oh!

spandan11106 commented Sep 17, 2026

1. Non-native locations could open a terminal in Strata's cwd

2. Strong Rc cycles kept PanelState alive

3. An end-to-end scenario for the terminal

On the async spawn lifecycle

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

spandan11106 commented Sep 17, 2026 •

edited

Loading

2. `TerminalPanel` currently keeps itself alive through strong Rc cycles

2. Strong Rc cycles kept `PanelState` alive