feat(wta): PID-fallback pane scanner for hookless agent CLI panes#240
feat(wta): PID-fallback pane scanner for hookless agent CLI panes#240DDKinger wants to merge 1 commit into
Conversation
Adds Class C session detection for the wta-helper: a 3-second tick walks the helper's owner-tab pane shell-PID children (Win32 Toolhelp32, BFS depth <= 3) and matches exe basenames against a Phase A allow-list (copilot/copilot-cli/claude/gemini). Matches produce synthetic `pane:<guid>` rows in the session management view labelled `(detected) <cli>` so the user can Focus the pane. A two-tick confirmation gate filters one-shot invocations like `copilot --version`. Why: today the session management view only learns about shell-pane CLI sessions through PowerShell shell-integration hooks (Class B). Users without hooks installed -- the default for non-pwsh shells and any user who hasn't opted in -- see nothing, even though their CLI is clearly running in front of them. Class C fills that gap with no CLI cooperation. Scope (Phase A): - Per-tab, helper-local. No master-side aggregation; the session management view in tab N won't show PID-detected rows from tab M. Cross-tab visibility is a Phase B follow-up over ACP ext notifications. - Native exe matching only. `npx @anthropic-ai/claude-code` and friends appear as `node.exe` and are NOT detected -- Phase A.1 will add command-line inspection. - Not resumable. Synthetic rows have no real ACP session id, so Enter routes to Focus; Resume options are suppressed. - Not persisted. Helper restart drops synthetic rows; next scan re-derives. - Opt-out via `WTA_PID_SCAN=0` env var. Reducer invariants (helper-local): - Synthetic rows are REMOVED on Lost (no Ended tombstone in the picker). - Real hook-driven `SessionStarted` on the same pane removes the synthetic predecessor cleanly (no duplicate row). - Existing real rows on a pane make `PidScannerDetected` a no-op. - Same-pane PID/CLI change updates the synthetic row in place (no flicker). - `PidScanner*` events are blocked from reaching master via `unreachable!()` guards in `SessionHookParams::From` and `apply_event_locked` -- they are strictly helper-local. Tests: 15 new unit tests covering the pure `diff_snapshots` / `classify_exe` functions and the reducer arms (creation, deletion, no-op-on-real, in-place update, GUID lowercasing). All 147 existing `agent_sessions` / `session_registry` tests continue to pass. Trace target: `pid_pane_scanner` (`WTA_LOG=debug` for tick activity, `trace` for per-tick binding counts). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| hook Class B. The session management view in tab N won't show | ||
| PID-detected rows from tab M. | ||
| - **Opt-out**: `WTA_PID_SCAN=0` env var disables the scanner entirely. | ||
| - **Phase A limitation**: matches native exe basenames only |
| /// Provenance for this session — populated for historical rows from | ||
| /// the agent-pane origin index. See [`SessionOrigin`]. | ||
| pub origin: SessionOrigin, | ||
| /// True for rows synthesised by the PID-based pane scanner (Class C): |
| if let Some(prev) = self.sessions.get_mut(&prev_key) { | ||
| // Synthetic rows (PID-scanner Class C) must be | ||
| // REMOVED entirely when a real hook-driven | ||
| // session takes over the pane. Otherwise the |
| // | ||
| // Synthetic-owned panes are deliberately NOT in this set — we need | ||
| // the scanner to keep observing them so that `pid_scan_confirmed` | ||
| // gets refreshed each tick. Otherwise a synthetic pane would drop |
| //! * `pwsh -> cmd -> copilot.exe` (user wrapper): depth 2 | ||
| //! * `pwsh -> npm.cmd -> node.exe -> copilot.exe` (theoretical): depth 3 | ||
| //! | ||
| //! Going deeper risks crawling the entire Toolhelp32 snapshot for |
| exe: String, | ||
| } | ||
|
|
||
| /// RAII handle for a Toolhelp32 snapshot. Closing the handle in |
| // SAFETY: `CreateToolhelp32Snapshot` returns either a valid | ||
| // kernel object handle or INVALID_HANDLE_VALUE (-1 as | ||
| // isize). We check both. | ||
| let handle = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) }; |
|
|
||
| fn collect(&self) -> Vec<Entry> { | ||
| let mut out = Vec::new(); | ||
| // PROCESSENTRY32W requires the caller to set dwSize before |
| // PROCESSENTRY32W requires the caller to set dwSize before | ||
| // calling Process32FirstW — otherwise the API rejects the | ||
| // struct as invalid and returns false. | ||
| let mut pe: PROCESSENTRY32W = unsafe { std::mem::zeroed() }; |
| // calling Process32FirstW — otherwise the API rejects the | ||
| // struct as invalid and returns false. | ||
| let mut pe: PROCESSENTRY32W = unsafe { std::mem::zeroed() }; | ||
| pe.dwSize = std::mem::size_of::<PROCESSENTRY32W>() as u32; |
@check-spelling-bot Report
|
| Dictionary | Entries | Covers | Uniquely |
|---|---|---|---|
| cspell:csharp/csharp.txt | 32 | 2 | 2 |
| cspell:aws/aws.txt | 232 | 2 | 2 |
| cspell:fonts/fonts.txt | 536 | 1 | 1 |
Consider adding to the extra_dictionaries array (in the .github/actions/spelling/config.json file):
"cspell:csharp/csharp.txt",
"cspell:aws/aws.txt",
"cspell:fonts/fonts.txt",
To stop checking additional dictionaries, put (in the .github/actions/spelling/config.json file):
"check_extra_dictionaries": []Forbidden patterns 🙅 (1)
In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.
These forbidden patterns matched content:
Should probably be Otherwise,
(?<=\. )Otherwise\s
Errors and Warnings ❌ (2)
See the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.
| ❌ Errors and Warnings | Count |
|---|---|
| 54 | |
| ❌ forbidden-pattern | 2 |
See ❌ Event descriptions for more information.
✏️ Contributor please read this
By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.
If the listed items are:
- ... misspelled, then please correct them instead of using the command.
- ... names, please add them to
.github/actions/spelling/allow/names.txt. - ... APIs, you can add them to a file in
.github/actions/spelling/allow/. - ... just things you're using, please add them to an appropriate file in
.github/actions/spelling/expect/. - ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in
.github/actions/spelling/patterns/.
See the README.md in each directory for more information.
🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
There was a problem hiding this comment.
Pull request overview
This PR adds a helper-local “Class C” PID-based fallback scanner to surface hookless agent CLI panes (copilot/claude/gemini) in the WTA session management view by periodically walking each pane’s child process tree and creating/removing synthetic pane:<guid> rows.
Changes:
- Introduces a new
pid_pane_scannermodule (Toolhelp32 BFS depth ≤ 3 + allow-list exe matching) and associated diffing/tests. - Extends session reducer/state to support PID-scanner synthetic rows (
synthetic,synthetic_cli_pid,PidScanner*events) and ensures hook-driven sessions replace synthetic predecessors cleanly. - Wires periodic scan ticks into the helper App event loop and explicitly blocks
PidScanner*events from master-side registry/hook forwarding.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/wta/src/pid_pane_scanner.rs | New PID-based scanner implementation + snapshot diffing + unit tests |
| tools/wta/src/app.rs | Adds scan tick/result events and two-tick confirmation gate state + reducer dispatch |
| tools/wta/src/agent_sessions.rs | Adds synthetic-row fields + PidScanner* events + reducer behavior |
| tools/wta/src/main.rs | Spawns the periodic scan tick task (gated by env/identity) |
| tools/wta/src/session_registry.rs | Drops PidScanner* in master-side registry and guards hook forwarding |
| tools/wta/src/master/mod.rs | Treats PidScanner* as keyless (should not refresh from disk) |
| tools/wta/src/history_loader.rs | Initializes new AgentSession fields for historical rows |
| tools/wta/src/ui/agents_view.rs | Updates tests for new AgentSession fields |
| tools/wta/Cargo.toml | Enables Win32_System_Diagnostics_ToolHelp for Toolhelp32 APIs |
| AGENTS.md | Documents the new Class C PID-fallback behavior and limitations |
| fn new() -> Option<Self> { | ||
| // SAFETY: `CreateToolhelp32Snapshot` returns either a valid | ||
| // kernel object handle or INVALID_HANDLE_VALUE (-1 as | ||
| // isize). We check both. | ||
| let handle = unsafe { CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0) }; | ||
| if handle.is_null() || handle as isize == -1 { | ||
| tracing::debug!( | ||
| target: "pid_pane_scanner", | ||
| "CreateToolhelp32Snapshot failed (err={})", | ||
| std::io::Error::last_os_error(), | ||
| ); | ||
| return None; | ||
| } | ||
| Some(Self(handle)) | ||
| } |
| let pid_scan_enabled = | ||
| std::env::var("WTA_PID_SCAN").map(|v| v != "0").unwrap_or(true); | ||
| if pid_scan_enabled && pane_identity.is_some() { | ||
| let scan_tx = event_tx.clone(); |
| //! * **Owner-tab scoped.** Each helper scans only its own tab. The | ||
| //! session management view in tab N | ||
| //! tab N sees PID-detected rows from tab N; cross-tab visibility | ||
| //! would require master-side scanning, which is out of scope. |
| /// Title displayed in the session management view for PID-scanner synthetic rows. | ||
| /// Kept distinct | ||
| /// from real-row titles (which are derived from CLI output / cwd / | ||
| /// session) so users can tell at a glance that the row is a |
| pub async fn scan_tab( | ||
| shell_mgr: &ShellManager, | ||
| tab_id: &str, | ||
| skip_pane_if: impl Fn(&str) -> bool, | ||
| ) -> anyhow::Result<Vec<PaneCliBinding>> { | ||
| let panes_resp = shell_mgr.wt_list_panes(tab_id).await?; | ||
| let panes_arr = panes_resp | ||
| .get("panes") | ||
| .and_then(|v| v.as_array()) | ||
| .map(|a| a.as_slice()) | ||
| .unwrap_or(&[]); | ||
|
|
||
| let mut bindings = Vec::new(); | ||
| for pane in panes_arr { | ||
| let pane_guid_raw = match pane.get("session_id") { | ||
| Some(serde_json::Value::String(s)) => s.clone(), | ||
| // `list_panes` returns lowercase strings already, but | ||
| // numeric ids would never round-trip as GUIDs; skip them. | ||
| _ => continue, | ||
| }; | ||
| let pane_guid = pane_guid_raw.to_ascii_lowercase(); | ||
| if skip_pane_if(&pane_guid) { | ||
| continue; | ||
| } | ||
|
|
||
| let shell_pid = match pane.get("pid").and_then(|v| v.as_u64()) { | ||
| Some(p) if p > 0 && p <= u32::MAX as u64 => p as u32, | ||
| _ => continue, // pane has no live shell — nothing to walk. | ||
| }; | ||
|
|
||
| // Win32 process enumeration is purely synchronous. Push it | ||
| // onto the blocking pool so the runtime can keep servicing | ||
| // ACP / UI events while the snapshot completes. | ||
| let maybe_match = tokio::task::spawn_blocking(move || { | ||
| child_enum::matching_cli_under(shell_pid) | ||
| }) |
What
Adds Class C session detection for the wta-helper: a 3-second tick walks the helper's owner-tab pane shell-PID children (Win32
Toolhelp32, BFS depth ≤ 3) and matches exe basenames against a Phase A allow-list (copilot.exe/copilot-cli.exe/claude.exe/gemini.exe). Matches produce syntheticpane:<guid>rows in the session management view labelled(detected) <cli>so the user can Focus the pane. A two-tick confirmation gate filters one-shot invocations likecopilot --version.Why
Today the session management view only learns about shell-pane CLI sessions through PowerShell shell-integration hooks (Class B). Users without hooks installed — the default for non-pwsh shells and any user who hasn't opted in — see nothing, even though their CLI is clearly running in front of them. Class C fills that gap with no CLI cooperation.
Scope (Phase A)
npx @anthropic-ai/claude-codeand friends appear asnode.exeand are NOT detected — Phase A.1 will add command-line inspection.WTA_PID_SCAN=0env var disables the scanner entirely.Reducer invariants (helper-local)
PidScannerLost(noEndedtombstone in the picker).SessionStartedon the same pane removes the synthetic predecessor cleanly — no duplicate row.PidScannerDetecteda no-op — scanner is the fallback, never the authority.PidScanner*events are blocked from reaching master viaunreachable!()guards inSessionHookParams::Fromandapply_event_locked— they are strictly helper-local.Tests
pid_pane_scanner.rs+agent_sessions.rscovering:diff_snapshots(empty, new, lost, no-change, PID change, CLI change, multi-pane mixed),classify_exe(allow-list + node/npm rejection), reducer arms (creation, deletion, no-op-on-real, in-place update, GUID lowercasing).agent_sessions/session_registrytests continue to pass.Phase B (out of scope here)
Master-side aggregation via
intellterm.wta/session_added-style ext notification so the session management view in any tab/window sees the same set of PID-detected sessions. PlusWin32_System_ProcessSnapshotting/ cmdline inspection to identifynpx-wrapped CLIs.