From 994170e8206c71798b78ce6d11f4e44f9abd1a64 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 27 Jul 2026 02:08:31 +0200 Subject: [PATCH 1/2] fix(ci): gate version bump on green CI, publish releases atomically, add PTY session unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps identified after today's incidents (a broken version-bump.yml syntax error, and a false-positive major-version bump from the bot's own commit message): - version-bump.yml raced CI instead of waiting for it (both fired on the same push). A commit that failed to compile could still get bumped, tagged, and dispatched to release.yml, which would only then fail — after the tag was already public. Switched its trigger from `push: branches: [main]` to `workflow_run: workflows: ["CI"]`, gated on `conclusion == 'success'`, and pinned every git operation to workflow_run.head_sha instead of whatever main's tip happens to be. Added a concurrency group so overlapping runs queue instead of racing. - release.yml published each platform's installers to a live, public release as soon as that platform's job finished — if e.g. Windows and Linux succeeded but macOS notarization failed, users could already be downloading (and the updater pointing at) a release missing a platform entirely. Set releaseDraft: true and added a `publish` job that only flips the release public once every matrix leg succeeded (needs.release.result == 'success'); on any failure the draft is left as-is for inspection, never deleted, never partially published. - Added 4 Rust unit tests for the PTY session bookkeeping introduced in the stability-audit PR (kill_pty on an unknown id, kill_pty actually killing/removing a real session, insert_session reaping a replaced session, and the session_is_current/session_is_superseded generation checks). Extracted the logic those commands relied on into small testable functions (kill_pty_impl, insert_session, session_is_current/superseded) with no behavior change. Two real PTYs overlapping in one test process measured 80s+ on this Windows host (ConPTY/conhost teardown serializing) — the replace-session test keeps only one side real and uses a fake Child/MasterPty for the other, which also required adding anyhow as a direct dev-dependency (portable-pty's Child/MasterPty traits return anyhow::Error but don't re-export it publicly; it's already resolved transitively). cargo test was already wired into the existing CI job/matrix — no separate workflow needed. All 14 tests (10 pre-existing + 4 new) pass; cargo check --locked and npm run build both clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PioEvgnoP4fpxDrSAYTBdF --- .github/workflows/release.yml | 28 ++- .github/workflows/version-bump.yml | 51 ++++-- src-tauri/Cargo.lock | 1 + src-tauri/Cargo.toml | 8 + src-tauri/src/lib.rs | 266 ++++++++++++++++++++++++++--- 5 files changed, 313 insertions(+), 41 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 740a30b..09374b0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -88,12 +88,36 @@ jobs: tagName: ${{ github.ref_name }} releaseName: "AFKode ${{ github.ref_name }}" releaseBody: "See the commit history for changes. Installers below; the updater feeds from latest.json." - releaseDraft: false + # Stays a draft until every matrix leg has actually finished — see + # the `publish` job below. Without this, if e.g. only Windows and + # Linux finish before macOS notarization fails, users could already + # be downloading (and the updater already pointing at) a release + # that's missing the macOS build entirely. + releaseDraft: true prerelease: false args: ${{ matrix.args }} - winget: + # Only flips the release public once every matrix leg above succeeded — + # `needs.release.result` reflects the matrix job as a whole (success only + # if every leg succeeded), not just the leg that happens to finish last. + # If any leg failed, this job is skipped and the draft is left exactly as + # it is — assets from whichever platforms did succeed, still private — + # for a human to inspect rather than either quietly going out partial or + # getting deleted. + publish: needs: release + if: needs.release.result == 'success' + runs-on: ubuntu-latest + permissions: + contents: write + steps: + - name: Make the release public + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: gh release edit "${{ github.ref_name }}" --draft=false --repo ${{ github.repository }} + + winget: + needs: publish runs-on: windows-latest # Requires the package to exist in winget-pkgs (initial PR merged); # a failure here must not affect the release itself. diff --git a/.github/workflows/version-bump.yml b/.github/workflows/version-bump.yml index b44e1e1..1f821df 100644 --- a/.github/workflows/version-bump.yml +++ b/.github/workflows/version-bump.yml @@ -1,7 +1,14 @@ name: Version Bump -# Runs after every push to main and decides, from Conventional Commits since -# the last tag, whether this merge deserves a new version: fix: -> patch, +# Runs after the "CI" workflow finishes on main, and only proceeds if it +# succeeded — never bump/tag/release a commit that doesn't even compile. +# (This used to run directly on `push: branches: [main]`, racing CI instead +# of waiting for it: a commit that failed to compile could still get bumped, +# tagged, and dispatched to release.yml, which would then fail after the +# tag was already public.) +# +# Once gated on a green CI run, it decides from Conventional Commits since +# the last tag whether this deserves a new version: fix: -> patch, # feat: -> minor, a `!` after the type/scope or a `BREAKING CHANGE:` footer # -> major. Anything else (chore/docs/refactor/test/ci/style, or a push with # no new commits since the last tag) bumps nothing and the job exits early. @@ -19,29 +26,49 @@ name: Version Bump # release.yml via the API (`gh workflow run`), which release.yml has to # opt into with `workflow_dispatch:`. on: - push: - branches: [main] + workflow_run: + workflows: ["CI"] + types: [completed] permissions: contents: write actions: write +# The bump commit's own push re-triggers "CI" (it runs on every push to +# main), which in turn re-triggers this workflow — the guard below turns +# that into a fast, harmless no-op (see it below), but two overlapping runs +# racing to push/tag at once would still be possible without this: e.g. a +# human push and the bump commit's CI run completing within moments of each +# other. Queue them instead of letting them interleave. +concurrency: + group: version-bump-main + cancel-in-progress: false + jobs: bump: runs-on: ubuntu-latest - # Without this, the bump commit's own push to main would re-trigger this - # same workflow. (In practice GITHUB_TOKEN pushes already don't trigger - # further runs — see the note above — but this keeps the job correct - # even if that ever changes, e.g. a maintainer force-pushing a manual - # bump commit under their own account.) - # The whole value must be quoted: an unquoted YAML scalar containing + # Only proceed for a green CI run on main — never for a failed/cancelled + # run, and never for CI runs from a PR branch (those also trigger "CI"). + # The commit-message check is the same anti-recursion guard as before, + # now reading workflow_run's own head_commit instead of push's. + # The whole if: value must be quoted: an unquoted YAML scalar containing # ": " (a colon followed by a space, as in 'chore: bump version to') # gets parsed as a nested mapping key instead of plain text, which is - # an invalid workflow file GitHub Actions won't even schedule a job for. - if: "${{ !startsWith(github.event.head_commit.message, 'chore: bump version to') }}" + # an invalid workflow file GitHub Actions won't even schedule a job for + # (this exact bug broke this workflow's first real run). + if: >- + ${{ + github.event.workflow_run.conclusion == 'success' && + github.event.workflow_run.head_branch == 'main' && + !startsWith(github.event.workflow_run.head_commit.message, 'chore: bump version to') + }} steps: - uses: actions/checkout@v4 with: + # Pin to the exact commit CI just validated, not whatever main's + # tip happens to be when this job starts — those should be the + # same commit, but pinning removes any doubt/race. + ref: ${{ github.event.workflow_run.head_sha }} fetch-depth: 0 - name: Determine bump level from commits since the last tag diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 8aaf24d..7468820 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -12,6 +12,7 @@ checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" name = "afkode" version = "0.8.20" dependencies = [ + "anyhow", "base64 0.22.1", "core-foundation", "core-graphics 0.24.0", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index dcfc9d1..d14ae94 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -32,6 +32,14 @@ portable-pty = "0.9" tiny_http = "0.12" base64 = "0.22" +[dev-dependencies] +# portable-pty's Child/MasterPty traits return anyhow::Error, but portable_pty +# only imports it privately (no public re-export) — needed by name to +# implement those traits for the fakes in lib.rs's test module. Already +# resolved transitively via portable-pty itself; this just makes it directly +# nameable from test code. +anyhow = "1" + [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.59", features = [ "Win32_Foundation", diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 27f1a89..6710afb 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -112,6 +112,22 @@ struct PtySession { static SESSION_GEN: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); +/// True only if the session at `id` still exists and is this generation — +/// used while a reader thread is streaming output, to decide whether it's +/// still the owner of that id or should back off (see the reader thread in +/// `spawn_pty`). +fn session_is_current(sessions: &HashMap, id: &str, gen: u64) -> bool { + sessions.get(id).is_some_and(|s| s.gen == gen) +} + +/// True only if the session at `id` still exists but belongs to a *newer* +/// generation — a reused id (e.g. a webview reload) raced ahead of this +/// thread's own teardown. False (not true) if the entry is simply gone +/// already; that's a normal exit, not a supersession. +fn session_is_superseded(sessions: &HashMap, id: &str, gen: u64) -> bool { + sessions.get(id).is_some_and(|s| s.gen != gen) +} + /// `Child::wait()` blocks until the OS finishes tearing the process down — /// on Unix a killed/exited child stays a zombie in the process table until /// something calls `wait()` on it. Do that reaping on its own thread, never @@ -125,6 +141,30 @@ fn reap(mut child: Box) { }); } +/// Inserts `session` at `id`, reaping whatever session was already there. +/// Under normal operation nothing is there (the id is fresh) or the caller +/// already ran `kill_pty` first — but a reused id can race ahead of its own +/// teardown (e.g. a webview reload spawning a replacement before the old +/// session's reader thread notices), and `HashMap::insert` would otherwise +/// silently drop the previous `PtySession`, orphaning its child. +fn insert_session(state: &PtyState, id: String, session: PtySession) { + let previous = state.sessions.lock().unwrap().insert(id, session); + if let Some(mut prev) = previous { + let _ = prev.child.kill(); + reap(prev.child); + } +} + +/// The part of `kill_pty` that doesn't need a live `tauri::State` — kept +/// separate so it's callable from a unit test with a plain `PtyState`. +fn kill_pty_impl(state: &PtyState, id: &str) { + let session = state.sessions.lock().unwrap().remove(id); + if let Some(mut session) = session { + let _ = session.child.kill(); + reap(session.child); + } +} + #[derive(Default)] struct PtyState { sessions: Mutex>, @@ -859,7 +899,8 @@ async fn spawn_pty( let gen = SESSION_GEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let job = child.process_id().and_then(job_for_child); - let previous = state.sessions.lock().unwrap().insert( + insert_session( + &state, id.clone(), PtySession { master: pair.master, @@ -869,14 +910,6 @@ async fn spawn_pty( _job: job, }, ); - // The id was already occupied — a reused tab id (e.g. a webview reload) - // raced ahead of that old session's own teardown. Its reader thread will - // see the `gen` mismatch and back off instead of reaping it, so kill and - // reap it here or it's an orphaned, never-waited-on process. - if let Some(mut prev) = previous { - let _ = prev.child.kill(); - reap(prev.child); - } // Reader thread: pump PTY output to the frontend as UTF-8 chunks. // 32 KiB reads coalesce bursty TUI redraws into fewer IPC events. @@ -890,14 +923,8 @@ async fn spawn_pty( // currently owns that id — duplicated/interleaved lines while the // user types in the new session. let is_current = || { - app.try_state::().is_some_and(|state| { - state - .sessions - .lock() - .unwrap() - .get(&id) - .is_some_and(|s| s.gen == gen) - }) + app.try_state::() + .is_some_and(|state| session_is_current(&state.sessions.lock().unwrap(), &id, gen)) }; loop { match reader.read(&mut chunk) { @@ -955,7 +982,7 @@ async fn spawn_pty( let mut superseded = false; let session = app.try_state::().and_then(|state| { let mut sessions = state.sessions.lock().unwrap(); - if sessions.get(&id).is_some_and(|s| s.gen != gen) { + if session_is_superseded(&sessions, &id, gen) { superseded = true; return None; } @@ -1022,18 +1049,203 @@ async fn resize_pty( #[tauri::command] async fn kill_pty(state: State<'_, PtyState>, id: String) -> Result<(), String> { - let session = { - let mut sessions = state.sessions.lock().unwrap(); - sessions.remove(&id) - }; - if let Some(mut session) = session { - let _ = session.child.kill(); - // Reap off-thread — see `reap`'s doc comment. - reap(session.child); - } + kill_pty_impl(&state, &id); Ok(()) } +#[cfg(test)] +mod pty_session_tests { + use super::*; + + /// A trivial, near-instantly-exiting PTY child, real enough to exercise + /// `Child::kill`/`wait` for real without depending on any particular + /// shell being on PATH beyond what every Windows/macOS/Linux CI runner + /// already has. + fn spawn_trivial_session() -> PtySession { + let pty_system = native_pty_system(); + let pair = pty_system + .openpty(PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, + }) + .expect("openpty"); + #[cfg(windows)] + let cmd = { + let mut c = CommandBuilder::new("cmd.exe"); + c.args(["/c", "exit", "0"]); + c + }; + #[cfg(not(windows))] + let cmd = { + let mut c = CommandBuilder::new("sh"); + c.args(["-c", "exit 0"]); + c + }; + let child = pair.slave.spawn_command(cmd).expect("spawn"); + drop(pair.slave); + let writer = pair.master.take_writer().expect("writer"); + PtySession { + master: pair.master, + writer: std::sync::Arc::new(Mutex::new(writer)), + child, + gen: SESSION_GEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + _job: None, + } + } + + // A fake Child/MasterPty pair for tests that only care about session + // *bookkeeping* (which entry ends up under an id, whether the old one + // was reaped) rather than a real process. Two real PTYs overlapping in + // one test process was measured to serialize very slowly on Windows + // (ConPTY/conhost teardown, 80s+ for what should be a millisecond + // test) — so a "replace an existing session" test keeps only one side + // real and uses this for the other. + #[derive(Debug)] + struct FakeChild; + impl portable_pty::ChildKiller for FakeChild { + fn kill(&mut self) -> std::io::Result<()> { + Ok(()) + } + fn clone_killer(&self) -> Box { + Box::new(FakeChild) + } + } + impl Child for FakeChild { + fn try_wait(&mut self) -> std::io::Result> { + Ok(Some(portable_pty::ExitStatus::with_exit_code(0))) + } + fn wait(&mut self) -> std::io::Result { + Ok(portable_pty::ExitStatus::with_exit_code(0)) + } + fn process_id(&self) -> Option { + None + } + #[cfg(windows)] + fn as_raw_handle(&self) -> Option { + None + } + } + + #[derive(Debug)] + struct FakeMasterPty; + impl MasterPty for FakeMasterPty { + fn resize(&self, _size: PtySize) -> Result<(), anyhow::Error> { + Ok(()) + } + fn get_size(&self) -> Result { + Ok(PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, + }) + } + fn try_clone_reader(&self) -> Result, anyhow::Error> { + Ok(Box::new(std::io::empty())) + } + fn take_writer(&self) -> Result, anyhow::Error> { + Ok(Box::new(std::io::sink())) + } + #[cfg(unix)] + fn process_group_leader(&self) -> Option { + None + } + #[cfg(unix)] + fn as_raw_fd(&self) -> Option { + None + } + #[cfg(unix)] + fn tty_name(&self) -> Option { + None + } + } + + fn fake_session() -> PtySession { + PtySession { + master: Box::new(FakeMasterPty), + writer: std::sync::Arc::new(Mutex::new(Box::new(std::io::sink()) as Box)), + child: Box::new(FakeChild), + gen: SESSION_GEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed), + _job: None, + } + } + + #[test] + fn kill_pty_on_unknown_id_is_a_noop() { + let state = PtyState::default(); + // Must not panic, and must leave the (already-empty) map alone. + kill_pty_impl(&state, "does-not-exist"); + assert!(state.sessions.lock().unwrap().is_empty()); + } + + #[test] + fn kill_pty_removes_and_kills_the_matching_session() { + let state = PtyState::default(); + let session = spawn_trivial_session(); + let pid = session.child.process_id(); + state + .sessions + .lock() + .unwrap() + .insert("tab-1".into(), session); + + kill_pty_impl(&state, "tab-1"); + + assert!( + state.sessions.lock().unwrap().get("tab-1").is_none(), + "killed session must be removed from the map" + ); + assert!(pid.is_some(), "sanity: the trivial child had a real pid"); + } + + #[test] + fn inserting_over_a_reused_id_reaps_the_previous_child() { + let state = PtyState::default(); + // The soon-to-be-replaced session is a real spawned process, so + // `insert_session`'s kill()+reap() runs against a real child — the + // replacement is a fake, so only one real PTY is ever open at once. + let first = spawn_trivial_session(); + let first_gen = first.gen; + insert_session(&state, "reused-id".into(), first); + + let second = fake_session(); + let second_gen = second.gen; + assert_ne!(first_gen, second_gen); + + // Replacing "reused-id" must not panic and must leave the *new* + // session (not the old one) behind under that id. + insert_session(&state, "reused-id".into(), second); + + let sessions = state.sessions.lock().unwrap(); + let current = sessions.get("reused-id").expect("id still present"); + assert_eq!(current.gen, second_gen, "the newer session must win"); + } + + #[test] + fn session_generation_checks_distinguish_current_superseded_and_missing() { + let mut sessions: HashMap = HashMap::new(); + sessions.insert("tab-1".into(), spawn_trivial_session()); + let gen = sessions.get("tab-1").unwrap().gen; + + // Same generation: current, not superseded. + assert!(session_is_current(&sessions, "tab-1", gen)); + assert!(!session_is_superseded(&sessions, "tab-1", gen)); + + // A newer generation reused the id: no longer current, and flagged + // as superseded (a live reader thread should back off, not tear + // this down as if it were its own exit). + assert!(!session_is_current(&sessions, "tab-1", gen + 1)); + assert!(session_is_superseded(&sessions, "tab-1", gen + 1)); + + // The id was never (or no longer) present: neither current nor + // superseded — this is a plain, ordinary exit, not a race. + assert!(!session_is_current(&sessions, "missing", gen)); + assert!(!session_is_superseded(&sessions, "missing", gen)); + } +} + /// A pre-existing npm install with a custom `prefix` (nvm-windows, a manual /// `npm config set prefix`, ...) puts its global bin dir somewhere neither /// hardcoded fallback below covers. Shelling out to `npm` (a .cmd shim — From 8078d1374cfad2864fb9cc0913f0df94b0d9e613 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 27 Jul 2026 02:22:12 +0200 Subject: [PATCH 2/2] fix(ci): add libc dev-dep for the unix-cfg test mock, and drop real PTY spawning from tests entirely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI (ubuntu/macos) failed the previous commit: the #[cfg(unix)] mock methods referenced libc::pid_t, matching portable-pty's own trait signature, but libc — like anyhow before it — is only a transitive dependency, not directly nameable without its own Cargo.toml entry. While fixing that, re-ran the suite locally and hit real, host-dependent flakiness: a single real PTY session's Drop (closing its ConPTY/conhost handle) took anywhere from under a second to 60+ seconds on this machine, on different tests each run. That's the same class of "synchronous close can block for a while" risk `reap()` exists to keep off the async runtime for a session's child — but here it was the master/writer half, undeferred, blocking the test thread itself. Rather than fight that timing, dropped the real-PTY test helper entirely; all 4 pty_session_tests now use only the fake Child/MasterPty, making them fast (0.00s) and deterministic. Left a comment flagging the master/writer-drop-can-block-the-async-runtime pattern as a candidate for a future stability pass — out of scope for this PR. All 14 tests pass locally; cargo check --locked clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01PioEvgnoP4fpxDrSAYTBdF --- src-tauri/Cargo.lock | 1 + src-tauri/Cargo.toml | 4 +++ src-tauri/src/lib.rs | 66 ++++++++++---------------------------------- 3 files changed, 19 insertions(+), 52 deletions(-) diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 7468820..bae9b38 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -16,6 +16,7 @@ dependencies = [ "base64 0.22.1", "core-foundation", "core-graphics 0.24.0", + "libc", "portable-pty", "serde", "serde_json", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index d14ae94..87dc764 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -39,6 +39,10 @@ base64 = "0.22" # resolved transitively via portable-pty itself; this just makes it directly # nameable from test code. anyhow = "1" +# MasterPty::process_group_leader's #[cfg(unix)] signature returns +# Option — same situation as anyhow above, needed by name only +# to implement the trait for the unix-only cfg branch of the test fakes. +libc = "0.2" [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.59", features = [ diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 6710afb..5062a19 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1057,51 +1057,18 @@ async fn kill_pty(state: State<'_, PtyState>, id: String) -> Result<(), String> mod pty_session_tests { use super::*; - /// A trivial, near-instantly-exiting PTY child, real enough to exercise - /// `Child::kill`/`wait` for real without depending on any particular - /// shell being on PATH beyond what every Windows/macOS/Linux CI runner - /// already has. - fn spawn_trivial_session() -> PtySession { - let pty_system = native_pty_system(); - let pair = pty_system - .openpty(PtySize { - rows: 24, - cols: 80, - pixel_width: 0, - pixel_height: 0, - }) - .expect("openpty"); - #[cfg(windows)] - let cmd = { - let mut c = CommandBuilder::new("cmd.exe"); - c.args(["/c", "exit", "0"]); - c - }; - #[cfg(not(windows))] - let cmd = { - let mut c = CommandBuilder::new("sh"); - c.args(["-c", "exit 0"]); - c - }; - let child = pair.slave.spawn_command(cmd).expect("spawn"); - drop(pair.slave); - let writer = pair.master.take_writer().expect("writer"); - PtySession { - master: pair.master, - writer: std::sync::Arc::new(Mutex::new(writer)), - child, - gen: SESSION_GEN.fetch_add(1, std::sync::atomic::Ordering::Relaxed), - _job: None, - } - } - - // A fake Child/MasterPty pair for tests that only care about session + // A fake Child/MasterPty pair — these tests only care about session // *bookkeeping* (which entry ends up under an id, whether the old one - // was reaped) rather than a real process. Two real PTYs overlapping in - // one test process was measured to serialize very slowly on Windows - // (ConPTY/conhost teardown, 80s+ for what should be a millisecond - // test) — so a "replace an existing session" test keeps only one side - // real and uses this for the other. + // was reaped), not a real OS process. A real PTY was tried initially, + // but even a single one, on this host, made `Drop`-ing its `master` + // (a synchronous ConPTY/conhost teardown wait) take anywhere from under + // a second to 60+ seconds — the same slow-but-synchronous-drop pattern + // that `kill_pty_impl`'s real callers rely on `reap()`'s background + // thread to keep off the async runtime for the *child* half; the + // `master`/`writer` half's drop isn't similarly deferred (a candidate + // for the next stability pass, out of scope here). Too flaky and slow + // for a unit test either way — a fake sidesteps host-dependent PTY + // teardown timing entirely. #[derive(Debug)] struct FakeChild; impl portable_pty::ChildKiller for FakeChild { @@ -1183,8 +1150,7 @@ mod pty_session_tests { #[test] fn kill_pty_removes_and_kills_the_matching_session() { let state = PtyState::default(); - let session = spawn_trivial_session(); - let pid = session.child.process_id(); + let session = fake_session(); state .sessions .lock() @@ -1197,16 +1163,12 @@ mod pty_session_tests { state.sessions.lock().unwrap().get("tab-1").is_none(), "killed session must be removed from the map" ); - assert!(pid.is_some(), "sanity: the trivial child had a real pid"); } #[test] fn inserting_over_a_reused_id_reaps_the_previous_child() { let state = PtyState::default(); - // The soon-to-be-replaced session is a real spawned process, so - // `insert_session`'s kill()+reap() runs against a real child — the - // replacement is a fake, so only one real PTY is ever open at once. - let first = spawn_trivial_session(); + let first = fake_session(); let first_gen = first.gen; insert_session(&state, "reused-id".into(), first); @@ -1226,7 +1188,7 @@ mod pty_session_tests { #[test] fn session_generation_checks_distinguish_current_superseded_and_missing() { let mut sessions: HashMap = HashMap::new(); - sessions.insert("tab-1".into(), spawn_trivial_session()); + sessions.insert("tab-1".into(), fake_session()); let gen = sessions.get("tab-1").unwrap().gen; // Same generation: current, not superseded.