Revert idle "claude foreground" filter — back to legacy 5s notify
Reverts in one combined commit: -9931a92(inline pane_id + watch list into bash script) -6772b8d(pivot per-distro → per-pane via TILETOPIA_PANE_ID env) -f51033a(original per-distro idle filter) End-to-end probe never worked correctly against the real running app even after fixing the wsl.exe-drops-positional-args bug. Probe script ran fine in isolation but kept returning false-negative when called through tiletopia's wsl.exe spawn. Rather than keep iterating, back out cleanly — pane behaviour is now the original "go idle after 5s of silence regardless of what's running." memory.md session log notes the lessons for a future retry: don't ship per-distro again (CLAUDE.md explicitly says multi-claude-per-distro is the primary use case); prove the probe end-to-end before wiring into the idle effect (a "Test probe" button in MCP panel would have caught this in minutes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9931a92c5f
commit
50fbd0e531
7 changed files with 27 additions and 486 deletions
|
|
@ -10,7 +10,6 @@ use crate::creds;
|
|||
use crate::hosts::{self, SshHost, SshHostView};
|
||||
use crate::mcp::{self, McpMirror, McpServerHandle, McpState, PendingActions, RunningServer};
|
||||
use crate::mcp_policy::McpPolicy;
|
||||
use crate::probe::ProbeCache;
|
||||
use crate::pty::{list_wsl_distros, PaneId, PtyManager, SpawnSpec};
|
||||
|
||||
const WORKSPACE_FILE: &str = "workspace.json";
|
||||
|
|
@ -303,31 +302,3 @@ pub async fn mcp_policy_save(app: AppHandle, policy: McpPolicy) -> Result<(), St
|
|||
pub async fn mcp_hard_deny_labels() -> Result<Vec<&'static str>, String> {
|
||||
Ok(crate::mcp_policy::hard_deny_rules().to_vec())
|
||||
}
|
||||
|
||||
// ---- idle-detection filter -------------------------------------------------
|
||||
|
||||
/// Probe whether any of the built-in watched processes (currently
|
||||
/// `["claude"]`) is running in the given WSL distro. Result is cached
|
||||
/// per-distro for ~3s — see {@link ProbeCache}. Fail-safe: any probe error
|
||||
/// resolves to `true` so the caller suppresses the idle indicator (the
|
||||
/// agreed trade-off; the previous "always notify" bug was worse than the
|
||||
/// occasional over-suppression).
|
||||
///
|
||||
/// Frontend only calls this for WSL panes. PowerShell + SSH skip the probe
|
||||
/// and fall back to the legacy always-notify behaviour. Empty distro names
|
||||
/// resolve to `true` (no info → fail-safe).
|
||||
#[tauri::command]
|
||||
pub async fn is_watch_process_running(
|
||||
cache: tauri::State<'_, Arc<ProbeCache>>,
|
||||
distro: String,
|
||||
pane_id: PaneId,
|
||||
) -> Result<bool, String> {
|
||||
// Probe shells out — keep it off the async runtime's thread.
|
||||
let cache_arc: Arc<ProbeCache> = (*cache).clone();
|
||||
let running = tokio::task::spawn_blocking(move || {
|
||||
cache_arc.is_watch_process_running(&distro, pane_id)
|
||||
})
|
||||
.await
|
||||
.map_err(|e| format!("probe join failed: {e}"))?;
|
||||
Ok(running)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,13 +5,11 @@ mod creds;
|
|||
mod hosts;
|
||||
mod mcp;
|
||||
mod mcp_policy;
|
||||
mod probe;
|
||||
mod pty;
|
||||
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::mcp::{McpServerHandle, McpState, PendingActions};
|
||||
use crate::probe::ProbeCache;
|
||||
use crate::pty::PtyManager;
|
||||
|
||||
pub fn run() {
|
||||
|
|
@ -42,9 +40,6 @@ pub fn run() {
|
|||
// Pending action registry — separate managed state so mcp_action_reply can
|
||||
// grab it without needing to lock McpState or reach into TileService.
|
||||
let pending_actions: Arc<PendingActions> = Arc::new(PendingActions::default());
|
||||
// Idle-filter probe cache: shared across all is_watch_process_running
|
||||
// calls so a per-distro answer is reused for a few seconds. See probe.rs.
|
||||
let probe_cache: Arc<ProbeCache> = Arc::new(ProbeCache::new());
|
||||
|
||||
tauri::Builder::default()
|
||||
.plugin(tauri_plugin_clipboard_manager::init())
|
||||
|
|
@ -53,7 +48,6 @@ pub fn run() {
|
|||
.manage(mcp_state)
|
||||
.manage(McpServerHandle::default())
|
||||
.manage(pending_actions)
|
||||
.manage(probe_cache)
|
||||
.invoke_handler(tauri::generate_handler![
|
||||
commands::list_distros,
|
||||
commands::spawn_pane,
|
||||
|
|
@ -76,7 +70,6 @@ pub fn run() {
|
|||
commands::mcp_policy_load,
|
||||
commands::mcp_policy_save,
|
||||
commands::mcp_hard_deny_labels,
|
||||
commands::is_watch_process_running,
|
||||
])
|
||||
.run(tauri::generate_context!())
|
||||
.expect("error while running tauri application");
|
||||
|
|
|
|||
|
|
@ -1,230 +0,0 @@
|
|||
//! "Is a watched process running in THIS pane?" probe for the idle filter.
|
||||
//!
|
||||
//! Background: tiletopia's idle indicator fires when a pane goes 5s without
|
||||
//! PTY output. When the user is reading a long `claude` response the pane
|
||||
//! is silent but nothing actionable is happening — the indicator becomes
|
||||
//! noise. This module lets the frontend ask "is `claude` running in pane N?"
|
||||
//! before flagging idle, and suppresses if so.
|
||||
//!
|
||||
//! ## Per-pane granularity (revised v2 design)
|
||||
//!
|
||||
//! v1 of this module was per-distro: one `pgrep` in the distro answered for
|
||||
//! all panes. That was wrong for tiletopia's primary use case — running
|
||||
//! multiple claude sessions across panes in the same distro is THE point of
|
||||
//! the app, and per-distro suppression silenced every pane the moment one
|
||||
//! ran claude. Revised: per-pane via env-var marker.
|
||||
//!
|
||||
//! How it works:
|
||||
//!
|
||||
//! 1. `pty.rs` tags every WSL spawn with `TILETOPIA_PANE_ID=<id>` propagated
|
||||
//! into the distro via `WSLENV`. The user's shell inherits it; every
|
||||
//! descendant process inherits from the shell. So `claude` running in
|
||||
//! pane N has `TILETOPIA_PANE_ID=N` in `/proc/<claude_pid>/environ`.
|
||||
//! 2. This probe runs `pgrep -x <name>` for each watched process, then for
|
||||
//! each PID it returns reads `/proc/<pid>/environ` (null-separated) and
|
||||
//! checks for an exact `TILETOPIA_PANE_ID=<target>` entry.
|
||||
//! 3. Cache keyed by `(distro, pane_id)`; ~3s TTL.
|
||||
//!
|
||||
//! PowerShell + SSH panes still skip the probe (frontend short-circuits).
|
||||
//! No `/proc` on the remote side for SSH, no parallel concept on Windows.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use parking_lot::Mutex;
|
||||
|
||||
/// Built-in list of process names whose presence in a pane suppresses idle.
|
||||
///
|
||||
/// [[user-watch-list]] TODO: surface this as a user-editable list
|
||||
/// (workspace config field or dedicated `watch.json`). For now the constant
|
||||
/// covers the only real-world use case (Anthropic's `claude` CLI taking its
|
||||
/// time on a long response). Adding entries to the constant is the only
|
||||
/// knob today.
|
||||
pub const DEFAULT_WATCH_PROCESSES: &[&str] = &["claude"];
|
||||
|
||||
/// How long a probe result is reused before we re-shell. Sized against the
|
||||
/// frontend's 1s idle-tick interval — 3s means ~one `wsl.exe` call per
|
||||
/// (distro, pane) per 3 ticks while reacting to "claude finished" within a
|
||||
/// few seconds. Too short = wsl.exe spam; too long = stale answer once
|
||||
/// claude actually exits.
|
||||
const CACHE_TTL: Duration = Duration::from_secs(3);
|
||||
|
||||
/// Cache entry: timestamp the probe ran + whether any watched process was
|
||||
/// found in this specific pane.
|
||||
#[derive(Clone, Copy)]
|
||||
struct CacheEntry {
|
||||
at: Instant,
|
||||
running: bool,
|
||||
}
|
||||
|
||||
/// Probe cache keyed by `(distro, pane_id)` so panes in the same distro
|
||||
/// running different processes get independent answers.
|
||||
pub struct ProbeCache {
|
||||
cache: Mutex<HashMap<(String, u64), CacheEntry>>,
|
||||
}
|
||||
|
||||
impl ProbeCache {
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
cache: Mutex::new(HashMap::new()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns true iff one of the watched processes is running in pane
|
||||
/// `pane_id` of `distro`. Cached for {@link CACHE_TTL}. On probe failure
|
||||
/// returns `false` — **fail-safe is to NOT suppress**. The v1 fail-safe
|
||||
/// of "suppress on error" was wrong: a transient probe failure shouldn't
|
||||
/// silence the idle indicator. Better to occasionally over-notify than
|
||||
/// permanently silence.
|
||||
pub fn is_watch_process_running(&self, distro: &str, pane_id: u64) -> bool {
|
||||
let key = (distro.to_string(), pane_id);
|
||||
|
||||
// Fast path: fresh cached answer.
|
||||
{
|
||||
let guard = self.cache.lock();
|
||||
if let Some(entry) = guard.get(&key) {
|
||||
if entry.at.elapsed() < CACHE_TTL {
|
||||
return entry.running;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Slow path: re-probe. Drop the lock before shelling out so other
|
||||
// probes aren't blocked.
|
||||
let running = probe_pane(distro, pane_id, DEFAULT_WATCH_PROCESSES);
|
||||
tracing::debug!(
|
||||
target: "tiletopia_lib::probe",
|
||||
distro = %distro,
|
||||
pane_id,
|
||||
running,
|
||||
"probed"
|
||||
);
|
||||
|
||||
let mut guard = self.cache.lock();
|
||||
guard.insert(
|
||||
key,
|
||||
CacheEntry {
|
||||
at: Instant::now(),
|
||||
running,
|
||||
},
|
||||
);
|
||||
running
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for ProbeCache {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
/// Build a single-line bash script with the target pane_id and watch-list
|
||||
/// **interpolated directly** into the script text. The earlier design
|
||||
/// passed these as positional args (`bash -c "..." _ <id> <names...>`)
|
||||
/// but `wsl.exe`'s arg-passing layer silently dropped everything after the
|
||||
/// `-c` script string, so `$1`/`$@` were always empty inside bash.
|
||||
/// Interpolating from Rust sidesteps the whole arg-passing path.
|
||||
///
|
||||
/// Both inputs are safe to inline: `pane_id` is a `u64` (no metachars) and
|
||||
/// `watched` is a compile-time const list. If future code wires user-supplied
|
||||
/// process names through here, validate/escape them first.
|
||||
///
|
||||
/// Returns exit 0 if any watched process running in this specific pane has
|
||||
/// the matching `TILETOPIA_PANE_ID` env marker; exit 1 otherwise.
|
||||
fn build_probe_script(pane_id: u64, watched: &[&str]) -> String {
|
||||
let watch_list = watched
|
||||
.iter()
|
||||
.map(|n| format!("\"{n}\""))
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
format!(
|
||||
r#"target_id={pane_id}; for name in {watch_list}; do for pid in $(pgrep -x "$name" 2>/dev/null); do [ -z "$pid" ] && continue; if [ -r "/proc/$pid/environ" ]; then if tr '\0' '\n' < "/proc/$pid/environ" 2>/dev/null | grep -qxF "TILETOPIA_PANE_ID=$target_id"; then exit 0; fi; fi; done; done; exit 1"#
|
||||
)
|
||||
}
|
||||
|
||||
fn probe_pane(distro: &str, pane_id: u64, watched: &[&str]) -> bool {
|
||||
if !cfg!(windows) {
|
||||
// Non-Windows builds don't ship the app; pretend no watched process
|
||||
// so developer test runs see the idle indicator working.
|
||||
return false;
|
||||
}
|
||||
if distro.is_empty() {
|
||||
tracing::debug!("probe: empty distro name; treating as not-running");
|
||||
return false;
|
||||
}
|
||||
|
||||
let script = build_probe_script(pane_id, watched);
|
||||
let args: Vec<String> = vec![
|
||||
"-d".to_string(),
|
||||
distro.to_string(),
|
||||
"--".to_string(),
|
||||
"bash".to_string(),
|
||||
"-c".to_string(),
|
||||
script,
|
||||
];
|
||||
|
||||
let out = match crate::pty::quiet_command_pub("wsl.exe")
|
||||
.args(&args)
|
||||
.stdout(std::process::Stdio::null())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.output()
|
||||
{
|
||||
Ok(o) => o,
|
||||
Err(e) => {
|
||||
tracing::info!(
|
||||
"probe: wsl.exe spawn for distro={distro:?} pane={pane_id} failed: {e}"
|
||||
);
|
||||
return false;
|
||||
}
|
||||
};
|
||||
|
||||
match out.status.code() {
|
||||
Some(0) => true,
|
||||
Some(1) => false,
|
||||
Some(other) => {
|
||||
tracing::debug!(
|
||||
"probe: distro={distro:?} pane={pane_id} bash exit={other} — treating as not-running"
|
||||
);
|
||||
false
|
||||
}
|
||||
None => {
|
||||
tracing::debug!(
|
||||
"probe: distro={distro:?} pane={pane_id} killed by signal — treating as not-running"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::build_probe_script;
|
||||
|
||||
/// Regression: positional args (`bash -c "..." _ <id> <names>`) were
|
||||
/// silently dropped by wsl.exe's arg-passing layer, so `$1`/`$@` were
|
||||
/// always empty inside bash. The script must inline pane_id + watch
|
||||
/// names directly. Lock that in.
|
||||
#[test]
|
||||
fn build_probe_script_inlines_pane_id_and_watch_list() {
|
||||
let s = build_probe_script(42, &["claude"]);
|
||||
assert!(s.contains("target_id=42"), "pane_id not inlined: {s}");
|
||||
assert!(s.contains("\"claude\""), "watch name not inlined: {s}");
|
||||
// No "$1" or "$@" — those are the trap.
|
||||
assert!(!s.contains("$1"), "script still references $1: {s}");
|
||||
assert!(
|
||||
!s.contains("\"$@\""),
|
||||
"script still references $@: {s}"
|
||||
);
|
||||
// Sanity: matches the exact env marker shape pty.rs sets.
|
||||
assert!(
|
||||
s.contains("TILETOPIA_PANE_ID=$target_id"),
|
||||
"marker lookup malformed: {s}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_probe_script_multiple_watch_names() {
|
||||
let s = build_probe_script(7, &["claude", "vim", "less"]);
|
||||
assert!(s.contains("\"claude\" \"vim\" \"less\""), "watch list bad: {s}");
|
||||
}
|
||||
}
|
||||
|
|
@ -154,32 +154,7 @@ impl PtyManager {
|
|||
_ => None,
|
||||
};
|
||||
|
||||
// Reserve the pane id BEFORE spawning so we can tag the shell's
|
||||
// env with it — see TILETOPIA_PANE_ID below. We still insert into
|
||||
// the panes map further down, after the reader thread is wired.
|
||||
let id = self.next_id.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
let (mut cmd, spawn_err) = build_command(&spec)?;
|
||||
// WSL panes get a TILETOPIA_PANE_ID env marker so the idle-filter
|
||||
// probe (probe.rs) can tell which descendant processes belong to
|
||||
// which pane — inheritance does the work: the shell inherits from
|
||||
// wsl.exe via WSLENV, and every child (e.g. claude) inherits from
|
||||
// the shell, so checking `/proc/<pid>/environ` for the marker
|
||||
// answers "is this process running in pane N?" exactly.
|
||||
if matches!(spec, SpawnSpec::Wsl { .. }) {
|
||||
cmd.env("TILETOPIA_PANE_ID", id.to_string());
|
||||
// WSLENV controls which Windows-side env vars are forwarded into
|
||||
// the distro. Append our marker rather than clobbering — users
|
||||
// may have their own WSLENV set up. `/u` = always pass through
|
||||
// as a Unix-style env var.
|
||||
let existing = std::env::var("WSLENV").unwrap_or_default();
|
||||
let combined = if existing.is_empty() {
|
||||
"TILETOPIA_PANE_ID/u".to_string()
|
||||
} else {
|
||||
format!("{existing}:TILETOPIA_PANE_ID/u")
|
||||
};
|
||||
cmd.env("WSLENV", combined);
|
||||
}
|
||||
let (cmd, spawn_err) = build_command(&spec)?;
|
||||
let child = pair.slave.spawn_command(cmd).context(spawn_err)?;
|
||||
|
||||
// We need to keep the master alive (drop = close the PTY), but we
|
||||
|
|
@ -195,6 +170,8 @@ impl PtyManager {
|
|||
let writer: SharedWriter = Arc::new(Mutex::new(writer_raw));
|
||||
let ring: Arc<Mutex<PaneRing>> = Arc::new(Mutex::new(PaneRing::new()));
|
||||
|
||||
let id = self.next_id.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
self.panes.lock().insert(
|
||||
id,
|
||||
PaneHandle {
|
||||
|
|
@ -480,13 +457,6 @@ fn looks_like_password_prompt(buf: &[u8]) -> bool {
|
|||
|
||||
/// Run a process without flashing a console window on Windows.
|
||||
fn quiet_command(program: &str) -> std::process::Command {
|
||||
quiet_command_pub(program)
|
||||
}
|
||||
|
||||
/// Public variant for cross-module callers (currently {@link crate::probe}).
|
||||
/// Same behaviour as the in-module `quiet_command`; the wrapper exists so
|
||||
/// other modules don't each re-implement the CREATE_NO_WINDOW dance.
|
||||
pub fn quiet_command_pub(program: &str) -> std::process::Command {
|
||||
let mut c = std::process::Command::new(program);
|
||||
#[cfg(windows)]
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue