Repository navigation
Collect LRC liveness signals so the agent stops killing healthy commands - #14777
Conversation
The CLI subagent decides whether a long-running command is hung from a terminal grid snapshot alone. Commands that redirect output to a file, suppress it, or compute silently look identical to a hung process, so the subagent Ctrl-Cs them. This adds three tiers of client-collected evidence to every LRC snapshot: terminal output hash, process-tree CPU/state/count, and growth of the files the command writes to. A 1 Hz sampler runs only while an agent-monitored LRC is in flight, so `seconds_since_last_activity` stays wall-clock accurate regardless of the agent's poll cadence and ordinary terminal use pays nothing. The process and file tiers describe the local machine, so remote sessions report the grid tier and set `signals_unavailable` rather than reporting absence of evidence the client never had a way to gather. Co-Authored-By: Warp Agent <agent@warp.dev>
`warp_tui` re-exports `RequestCommandOutputResult`, so its tool call label tests construct the snapshot variant directly. Co-Authored-By: Warp Agent <agent@warp.dev>
Every scalar in the activity payload encodes with implicit presence, so the server can only distinguish "inspected and idle" from "not inspected" by whether the process submessage itself is present. A command is registered with the monitor when its first snapshot is built, and the sampler only visits commands that are already registered, so the first snapshot was reporting a present submessage whose counters were still at their initial zero. That is the signature of a process tree with nothing left running, produced for every healthy command on its very first poll — precisely the false reading this whole change exists to prevent. The process tier is now withheld until the sampler has actually observed it, and `signals_unavailable` covers that window as well as remote sessions. An all-zero tier that *was* observed is still reported, since an exited tree is a real answer. Also re-pins warp-proto-apis to 0a12f58b, which documents this presence contract in attachment.proto. Co-Authored-By: Warp Agent <agent@warp.dev>
Picks up the documented presence contract for the activity scalars and the narrowed meaning of `signals_unavailable`. Comment-only upstream, so the generated Rust API is unchanged. Co-Authored-By: Warp Agent <agent@warp.dev>
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![warp-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds client-side long-running-command activity signals by sampling terminal output, process-tree activity, and detected redirect targets, then plumbing those signals through the agent API result types.
Concerns
- Process churn detection only compares the number of live processes, so same-count PID replacement can still make active silent workloads look idle.
- Redirect target tails expand data collection from terminal-visible output to arbitrary file contents; size/mtime-style metadata is sufficient for liveness without sending redirected data.
- The tail reader can follow a redirect target that has been swapped to a symlink after sampling, undermining the no-symlink guarantee.
Security
- Redirected file tails can include private exports or logs the user intentionally kept out of terminal output.
File::openfollows symlinks, so the sampled regular-file check does not prevent reading a different local file at report time.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // Process churn is itself progress: a build spawning and reaping | ||
| // compilers may never accumulate much CPU in any single process. | ||
| let count_changed = self.process.live_process_count != previous_count; | ||
| saw_activity |= cpu_delta > 0 || io_delta > 0 || count_changed; |
There was a problem hiding this comment.
live_process_count misses churn where one PID exits and another starts between samples with the same count; because new PIDs contribute no baseline CPU delta, those active silent workloads can still look idle. Track PID-set changes rather than just count changes.
| path: file.path.to_string_lossy().into_owned(), | ||
| size_bytes: size, | ||
| size_delta_bytes: size as i64 - file.size_at_last_report as i64, | ||
| tail: read_tail(&file.path), |
There was a problem hiding this comment.
| fn read_tail(path: &Path) -> Option<String> { | ||
| use std::io::{Read, Seek, SeekFrom}; | ||
|
|
||
| let mut file = std::fs::File::open(path).ok()?; |
There was a problem hiding this comment.
File::open follows symlinks even though sampling used symlink_metadata, so a target swapped to a symlink after sampling can make the snapshot read an arbitrary local file. Recheck immediately before opening or use a no-follow open path.
The output and file activity tiers fired rarely in practice, and real work reflects in the process tier anyway. Track the proto change that removed them from LongRunningCommandActivity (warp-proto-apis 21dc6c9e46cb09b5724f134f881beda3d29cd68f): - seconds_since_last_activity is now driven only by process-tree activity (CPU delta, IO write delta, or pid-set churn). - signals_unavailable now only marks the first-snapshot case, before the sampler has observed the tree; it always accompanies an absent process submessage. - Remote sessions stop reporting activity entirely (activity absent on the wire, same as an older client). Previously they reported only the output tier, and with that tier gone an ever-growing seconds_since_last_activity would falsely suggest idleness. Delete the lrc_redirect module (it existed solely to find file-tier write targets), drop LrcFileActivity and the removed-field conversions, and rewrite the activity tests around the process tier, first-snapshot withholding, and remote monitor gating. Co-Authored-By: Warp Agent <agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds LRC process-activity sampling, carries the activity payload through agent result conversions, and adds unit coverage for activity accumulation and wire conversion.
Concerns
- Shell-executed long-running commands are still invisible to the sampler because the process tree excludes the shell itself; see the inline comment.
- This is a user-facing behavioral change, but the PR description does not include screenshots, a screen recording, or an equivalent end-to-end replay artifact showing the file-redirect, silent-compute, and genuine-hang cases working. Please attach that evidence before landing.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| /// siblings rather than descendants of the group leader, so the group is matched | ||
| /// per process rather than by walking down from the leader. | ||
| fn command_process_tree(system: &System, shell_pid: Pid, foreground_pgid: Option<u32>) -> Vec<Pid> { | ||
| let descendants = descendants_of(system, shell_pid); |
There was a problem hiding this comment.
while true; do :; done still report an empty process tree and can be cancelled as hung. Include the shell when it owns the foreground job, or otherwise account for shell-executed commands, before relying on descendants-only sampling.
There was a problem hiding this comment.
Good catch — confirmed and fixed.
The trace was as you describe: descendants_of excludes the shell pid, so a builtin-only command left command_process_tree empty, collect_process_sample still returned Some with per_pid: [], and take_report emitted a present process tier with live_process_count: 0 and signals_unavailable: false. That is the encoding for an exited tree, so the busy command was reported as the strongest available evidence that nothing was running.
command_process_tree now compares the shell's own process group against the pty's foreground group and includes the shell when the two match, which is exactly the case where a builtin or shell function is the foreground job. When an external job holds the terminal the groups differ and behavior is unchanged, so the genuine "tree exited" reading is preserved. Added the_shell_joins_the_tree_when_it_holds_the_terminal and the_shell_stays_out_of_the_tree_when_another_job_holds_the_terminal to cover both directions.
Windows still falls back to descendants-only, since foreground_pgid has no controlling-terminal group to read there.
…terminal Builtins and shell functions run in the shell process itself, so a descendants-only tree reported an observed-empty process tree for a busy command, which reads as an exited tree rather than as silent work. Co-Authored-By: Warp <agent@warp.dev>
| state.system.refresh_processes_specifics( | ||
| ProcessesToUpdate::All, | ||
| true, /* remove_dead_processes */ | ||
| ProcessRefreshKind::nothing().with_cpu().with_disk_usage(), | ||
| ); |
There was a problem hiding this comment.
This applies the expensive refresh kind (with_cpu().with_disk_usage()) to the entire process table at 1 Hz, when we only care about the command's tree. Could we split discovery from measurement?
// Phase A — discovery: enumerate the table, sample nothing.
state.system.refresh_processes_specifics(
ProcessesToUpdate::All,
true,
ProcessRefreshKind::nothing(),
);
let tree = command_process_tree(&state.system, shell_pid, foreground_pgid(shell));
// Phase B — measurement: expensive kind, only for the tree.
state.system.refresh_processes_specifics(
ProcessesToUpdate::Some(&tree),
true,
ProcessRefreshKind::nothing().with_cpu().with_disk_usage(),
);That makes the expensive work O(command tree) instead of O(all processes), and matches the existing pattern: SystemInfo only ever measures scoped pids, and all_processes_refresh_kind() deliberately samples nothing when enumerating the full table. The delta logic is already compatible since it uses cumulative accumulated_cpu_time().
This is also a Windows stability concern, not just perf: see the comment on SystemInfo::all_processes_refresh_kind (app/src/system/info.rs) — per-process CPU sampling across the full table fires an IPI per pid and can trip the DPC watchdog and bugcheck high-core-count machines. That's why the only existing full-table scan in the codebase runs once per process lifetime and samples nothing. Nothing gates this path off Windows today (only foreground_pgid/getpgid are unix-only). The two-phase split resolves that structurally, since CPU/disk are only ever sampled for tree members.
| use crate::terminal::model::terminal_model::ShellProcessInfo; | ||
|
|
||
| /// How often liveness signals are sampled while a monitored command is active. | ||
| pub const SAMPLE_INTERVAL: Duration = Duration::from_secs(1); |
There was a problem hiding this comment.
How'd we come to the 1s interval? We have an existing process sample that runs on the Warp process and it runs every 5s. I'd feel a bit better about following the existing precedent as to not regress anything performance-wise. Also, LRC snapshots don't come that frequently, so I think the extra time won't affect quality much.
| let shell = shell?; | ||
| let shell_pid = Pid::from_u32(shell.pid); | ||
|
|
||
| let mut state = self.state.lock(); |
There was a problem hiding this comment.
The monitor lock is held across the entire refresh + tree walk + getpgid syscalls here, and report() takes this same lock while the terminal model FairMutex is held. So worst case, a snapshot build stalls the terminal lock for a full tick's duration.
With the refresh scoped down this is probably cheap enough to be moot, but shrinking the lock scope (refresh into locals, then lock briefly to fold) — or a comment explaining why the bound is acceptable — would make the terminal-lock exposure explicit.
| let keep_sampling = !state.blocks.is_empty() || state.armed_actions > 0; | ||
| state.sampler_running = keep_sampling; |
There was a problem hiding this comment.
Nit: once keep_sampling goes false, state.system retains a full process-table snapshot for the lifetime of the executor. Worth clearing it here (e.g. state.system = System::new()) so we only hold that memory while actively monitoring.
Re-pins warp-proto-apis to f89d145, which merges proto main (web search cost) into the LRC activity proto branch. Co-Authored-By: Warp <agent@warp.dev>
danielpeng2
left a comment
danielpeng2
left a comment
There was a problem hiding this comment.
Sorry, I didn't mean to approve this yet. Can we make sure these changes are gated behind a feature flag?
Another thing we should do is make sure these are tested on all platforms (MacOS, Windows, Linux) before enabling in prod, since the process sampling code will work differently on different platforms. There should be folks on the team with Windows and Linux laptops that we can ask to sanity check.
Gated the change in 9f487ca |
This reverts commit a7034d6. Co-Authored-By: Warp <agent@warp.dev>
…hot (#353) ## Description Adds a `LongRunningShellCommandActivity` message to `LongRunningShellCommandSnapshot` (`activity`, field 6, additive and optional) in `apis/multi_agent/v1/attachment.proto`, carrying client-sampled liveness data for a long-running command: - **`since_last_activity`** — wall-clock `Duration` since the process tree last showed activity (CPU consumption, I/O writes, or process churn). Sampled client-side every ~5s independently of the agent's polling cadence, so it stays accurate even when agent reads are far apart. Resolution is the sampling interval. - **`process`** — `ProcessActivity` submessage with the CPU-time delta, a coarse aggregate tree `State` enum (`RUNNING`, `SLEEPING`, `DISK_WAIT`, `STOPPED`, `ZOMBIE`, `UNKNOWN`), live process count, and I/O-write-bytes delta. The `*_delta` fields accumulate over the whole window since the previous snapshot, not just the latest sample. Collection status is conveyed by message-level presence rather than flags: an absent `activity` means no reading was taken (unsupported client, remote session, or nothing sampled yet); an absent `process` means that tier was not collected; an all-zero `ProcessActivity` is a real reading (a quiet or exited tree). Scalar presence is deliberately not part of the contract because the Rust bindings are generated through a proto3 rewrite, where unset scalars are indistinguishable from defaults. ## Why we need this Make the changes in client and server work: client change: warpdotdev/warp#14777 server change: warpdotdev/warp-server#13955 ## Sensitive Fields The new messages contain no string fields (process state is a closed enum), so there is nothing to mark `(sensitive)`. - [x] I have ensured that all string fields that contain sensitive information are marked as `sensitive`. Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Warp Agent <agent@warp.dev> Co-authored-by: Oz <oz-agent@warp.dev>
Re-pin warp-proto-apis to 2bd5df2e3a73ada980d1e9c613ce369232e8c41d, the squash merge of warpdotdev/warp-proto-apis#353, now that it has landed on main. This also pulls in #369, which widens ModelTokenUsage.total_tokens and token_usage_by_category from uint32 to uint64. Adapt at the proto boundary rather than widening the persisted token counters: convert losslessly on the way out in persistence, and narrow saturatingly on the way in when accumulating conversation usage. Co-Authored-By: Warp <agent@warp.dev>
`sysinfo` is a `cfg(not(target_family = "wasm"))` dependency, but `lrc_activity` imported it unconditionally, breaking both wasm CI jobs. Move every use of the process table into `lrc_activity/sampler.rs`, which provides the real sampler off wasm and one that observes nothing on wasm. Pids cross that boundary as plain `u32`, so the monitor's accounting — and its tests — stay target-agnostic. Liveness signals are now also reported as unsupported on wasm, which has no local process tree to describe. Co-Authored-By: Warp <agent@warp.dev>
…nds (warpdotdev#14777) In some tbench traces with LRC, the CLI subagent judges a long-running command's liveness from the terminal grid alone, so a command that redirects to a file, suppresses output, or computes silently looks identical to a hang and gets Ctrl-C'd This PR adds an `LrcActivityMonitor` that samples the command's process activity every 5 seconds (CPU delta, I/O write delta, live-process count, aggregate state) and attaches it to LRC snapshots. On the wire the payload is `LongRunningShellCommandActivity`: `since_last_activity` as a `google.protobuf.Duration` plus a `process` tier whose `state` is an enum. All reading of the OS process table lives in `execute/lrc_activity/sampler.rs`, which is the only place `sysinfo` is used. Pids cross that boundary as plain `u32`, so the monitor's accounting stays target-agnostic and wasm — which has no local process table — gets a sampler that observes nothing. - **The foreground pgid is sampled by the monitor, not by `TerminalAttributesPoller`.** The existing fd poller is the obvious source, but it only runs during password-prompt detection: gated on notification settings, never started on the TUI surface, and stopped after its first positive reading — so it would essentially never be publishing during an LRC. The monitor calls `tcgetpgrp` on its own sampler instead, using the new `ShellProcessInfo` on `TerminalModel`. ([`foreground_pgid`](https://github.com/warpdotdev/warp/blob/f911ee51e5/app/src/ai/blocklist/action_model/execute/lrc_activity/sampler.rs#L212-L233), [`ShellProcessInfo`](https://github.com/warpdotdev/warp/blob/f911ee51e5/app/src/terminal/model/terminal_model.rs#L535-L551)) - **The activity payload is withheld until the tree has actually been sampled.** Blocks register lazily on their first `report()`, so the first snapshot is built before the sampler has run; emitting an all-zero-but-present reading there would claim "0 live processes, 0 CPU" about a healthy command at exactly the poll where cancellation is decided. `take_report` returns `None` until a real sample exists, so that snapshot simply carries no `activity` section — an all-zero tree observed *later* is still reported, since "the tree exited" is a real answer. (Per review on the proto PR, absence of the payload now carries this meaning; the earlier in-band `signals_unavailable` marker is gone.) ([`take_report`](https://github.com/warpdotdev/warp/blob/f911ee51e5/app/src/ai/blocklist/action_model/execute/lrc_activity.rs#L253-L276)) Re-pinning the proto past warpdotdev/warp-proto-apis#353 unavoidably picks up warpdotdev/warp-proto-apis#369, which widens `ModelTokenUsage.total_tokens` and the `token_usage_by_category` values from `uint32` to `uint64`. Master had not absorbed that yet, so the adaptation rides along here. It is handled **at the proto boundary only** — the persisted and displayed counters stay `u32`. Widening those would ripple through ~9 files including view rendering and a large number of tests, which belongs with warpdotdev#369's own migration rather than this PR. - Outbound (`crates/persistence/src/model.rs`): lossless `u64::from(...)`. `to_proto_combined` now sums in `u64`, which also removes a latent overflow in the three-way add. - Inbound (`app/src/ai/agent/conversation.rs`): a `narrow_token_count` helper saturating via `u32::try_from(..).unwrap_or(u32::MAX)`, and the accumulations use `saturating_add`. Saturating rather than `unwrap_or_default()` so an oversized count cannot surface as a plausible-looking small total. This matches the existing convention in `get_conversation_usage.rs`, which already narrows with `u32::try_from`. - [ ] No linked issue — came out of a staging trace investigation (`d2fc50d9`, `f6d8b53a`, `0af93eee`). - [ ] N/A — no UI surface changes, so no screenshots. - [ ] Not manually run via `./script/run` — end-to-end behavior needs the server half, which is gated off in prod (see Rollout). Unit coverage, all passing at the current tip (`f911ee51e5`): - `cargo test -p warp --lib lrc_activity` — 19 tests: activity detection via CPU/IO/pid churn, per-report reset, no report until first sample, exited-tree reporting, remote gating. - `cargo test -p ai action_result` — 12 tests: proto conversion incl. Duration round-trip and enum-state mapping. - `cargo test -p warp --lib conversation::` (64) and `cargo test -p persistence` (26) — cover the token-counter conversions above. Also clean: `cargo check -p warp --all-targets`, `cargo fmt --check`, `cargo clippy -p warp --all-targets -- -D warnings`, and the wasm gate `cargo clippy --locked --target wasm32-unknown-unknown -p warp --lib -- -D warnings`. End-to-end replay evals (file-redirect, silent compute, genuine hang) are still outstanding. Proto dependency is unblocked: warpdotdev/warp-proto-apis#353 has merged, and `Cargo.toml:352` now pins the resulting `main` commit `2bd5df2e3a73ada980d1e9c613ce369232e8c41d` (previously the PR-branch rev `65ff6b9`). Server counterpart: warpdotdev/warp-server#13955. Client-side sampling is gated on `FeatureFlag::LrcActivitySignal` (dogfood-on), and reported as unsupported on Windows (no controlling-terminal process group to narrow the tree with) and on wasm (no local process table). The wire payload is additive with every field optional, and the server half is behind `lrc_activity_signals` (off in prod). - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-NONE: user-visible behavior only changes when the server-side lrc_activity_signals flag is enabled, so the changelog entry belongs with that rollout rather than this client release. --> Co-Authored-By: Warp Agent <agent@warp.dev> --------- Co-authored-by: Warp Agent <agent@warp.dev> (cherry picked from commit 76cfd2c)
…arpdotdev#14777) Adapted port of upstream LRC activity monitoring for this ACP-only fork: local process-tree sampler (CPU/IO/pid churn, foreground-pgid narrowing) attached to long-running command snapshots, ShellProcessInfo on TerminalModel, LrcActivitySignal wired through the cargo-feature pipeline, and activity rendered into the ACP follow-up prompt Display boundary replacing upstream's proto wire path. Proto re-pin, server-API conversion files, token-counter widening, and warp_tui tests omitted (fork-absent surfaces).
## Description Re-enable local LRC process-activity sampling on Windows by dropping the `target_os = "windows"` exclusion from `lrc_activity_signals_supported()`. Wasm stays excluded; `FeatureFlag::LrcActivitySignal` still gates collection. Windows was turned off in #14777 because the shell-in-tree tests were Unix-only. The sampler already compiles there: `foreground_pgid` is always `None`, so the tree is descendants-only, matching the Unix missing-pgid fallback. Two-phase sysinfo refresh is unchanged (CPU/disk only on the command tree). No proto or warp-server changes. `LongRunningShellCommandActivity` and server ingestion already exist with no OS filter. ## Linked Issue - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. #15689 is labeled `factory:wilson` only. - [ ] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). Non-visual sampling change; visual proof was not requested. #15689 ## Testing - [ ] I have manually tested my changes locally with `./script/run`. Not a UI change. Unix: `cargo nextest run -p warp -E 'test(execute::lrc_activity::tests) or test(execute::shell_command::tests)'` — 20 passed. Format: `./script/format`. Clippy: `cargo clippy -p warp --all-targets --tests -- -D warnings`. @evelyn-with-warp confirmed the feature on a Windows machine, including expected debug-log output during testing. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-NONE <!-- warp:pr-description-artifacts start --> <!-- warp:pr-description-artifacts end --> --------- Co-authored-by: warp-agent-staging[bot] <240773466+warp-agent-staging[bot]@users.noreply.github.com>
## Description Re-enable local LRC process-activity sampling on Windows by dropping the `target_os = "windows"` exclusion from `lrc_activity_signals_supported()`. Wasm stays excluded; `FeatureFlag::LrcActivitySignal` still gates collection. Windows was turned off in warpdotdev#14777 because the shell-in-tree tests were Unix-only. The sampler already compiles there: `foreground_pgid` is always `None`, so the tree is descendants-only, matching the Unix missing-pgid fallback. Two-phase sysinfo refresh is unchanged (CPU/disk only on the command tree). No proto or warp-server changes. `LongRunningShellCommandActivity` and server ingestion already exist with no OS filter. ## Linked Issue - [ ] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. warpdotdev#15689 is labeled `factory:wilson` only. - [ ] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). Non-visual sampling change; visual proof was not requested. warpdotdev#15689 ## Testing - [ ] I have manually tested my changes locally with `./script/run`. Not a UI change. Unix: `cargo nextest run -p warp -E 'test(execute::lrc_activity::tests) or test(execute::shell_command::tests)'` — 20 passed. Format: `./script/format`. Clippy: `cargo clippy -p warp --all-targets --tests -- -D warnings`. @evelyn-with-warp confirmed the feature on a Windows machine, including expected debug-log output during testing. ## Agent Mode - [ ] Warp Agent Mode - This PR was created via Warp's AI Agent Mode CHANGELOG-NONE <!-- warp:pr-description-artifacts start --> <!-- warp:pr-description-artifacts end --> --------- Co-authored-by: warp-agent-staging[bot] <240773466+warp-agent-staging[bot]@users.noreply.github.com> (cherry picked from commit 09f0c2b)
…arpdotdev#14777) Adapted port of upstream LRC activity monitoring for this ACP-only fork: local process-tree sampler (CPU/IO/pid churn, foreground-pgid narrowing) attached to long-running command snapshots, ShellProcessInfo on TerminalModel, LrcActivitySignal wired through the cargo-feature pipeline, and activity rendered into the ACP follow-up prompt Display boundary replacing upstream's proto wire path. Proto re-pin, server-API conversion files, token-counter widening, and warp_tui tests omitted (fork-absent surfaces).
Description
In some tbench traces with LRC, the CLI subagent judges a long-running command's liveness from the terminal grid alone, so a command that redirects to a file, suppresses output, or computes silently looks identical to a hang and gets Ctrl-C'd
This PR adds an
LrcActivityMonitorthat samples the command's process activity every 5 seconds (CPU delta, I/O write delta, live-process count, aggregate state) and attaches it to LRC snapshots. On the wire the payload isLongRunningShellCommandActivity:since_last_activityas agoogle.protobuf.Durationplus aprocesstier whosestateis an enum.All reading of the OS process table lives in
execute/lrc_activity/sampler.rs, which is the only placesysinfois used. Pids cross that boundary as plainu32, so the monitor's accounting stays target-agnostic and wasm — which has no local process table — gets a sampler that observes nothing.Two design decisions to review
The foreground pgid is sampled by the monitor, not by
TerminalAttributesPoller. The existing fd poller is the obvious source, but it only runs during password-prompt detection: gated on notification settings, never started on the TUI surface, and stopped after its first positive reading — so it would essentially never be publishing during an LRC. The monitor callstcgetpgrpon its own sampler instead, using the newShellProcessInfoonTerminalModel. (foreground_pgid,ShellProcessInfo)The activity payload is withheld until the tree has actually been sampled. Blocks register lazily on their first
report(), so the first snapshot is built before the sampler has run; emitting an all-zero-but-present reading there would claim "0 live processes, 0 CPU" about a healthy command at exactly the poll where cancellation is decided.take_reportreturnsNoneuntil a real sample exists, so that snapshot simply carries noactivitysection — an all-zero tree observed later is still reported, since "the tree exited" is a real answer. (Per review on the proto PR, absence of the payload now carries this meaning; the earlier in-bandsignals_unavailablemarker is gone.) (take_report)Incidental:
uint32→uint64token countersRe-pinning the proto past warpdotdev/warp-proto-apis#353 unavoidably picks up warpdotdev/warp-proto-apis#369, which widens
ModelTokenUsage.total_tokensand thetoken_usage_by_categoryvalues fromuint32touint64. Master had not absorbed that yet, so the adaptation rides along here.It is handled at the proto boundary only — the persisted and displayed counters stay
u32. Widening those would ripple through ~9 files including view rendering and a large number of tests, which belongs with #369's own migration rather than this PR.crates/persistence/src/model.rs): losslessu64::from(...).to_proto_combinednow sums inu64, which also removes a latent overflow in the three-way add.app/src/ai/agent/conversation.rs): anarrow_token_counthelper saturating viau32::try_from(..).unwrap_or(u32::MAX), and the accumulations usesaturating_add. Saturating rather thanunwrap_or_default()so an oversized count cannot surface as a plausible-looking small total. This matches the existing convention inget_conversation_usage.rs, which already narrows withu32::try_from.Linked Issue
d2fc50d9,f6d8b53a,0af93eee).Testing
./script/run— end-to-end behavior needs the server half, which is gated off in prod (see Rollout).Unit coverage, all passing at the current tip (
f911ee51e5):cargo test -p warp --lib lrc_activity— 19 tests: activity detection via CPU/IO/pid churn, per-report reset, no report until first sample, exited-tree reporting, remote gating.cargo test -p ai action_result— 12 tests: proto conversion incl. Duration round-trip and enum-state mapping.cargo test -p warp --lib conversation::(64) andcargo test -p persistence(26) — cover the token-counter conversions above.Also clean:
cargo check -p warp --all-targets,cargo fmt --check,cargo clippy -p warp --all-targets -- -D warnings, and the wasm gatecargo clippy --locked --target wasm32-unknown-unknown -p warp --lib -- -D warnings.End-to-end replay evals (file-redirect, silent compute, genuine hang) are still outstanding.
Rollout
Proto dependency is unblocked: warpdotdev/warp-proto-apis#353 has merged, and
Cargo.toml:352now pins the resultingmaincommit2bd5df2e3a73ada980d1e9c613ce369232e8c41d(previously the PR-branch rev65ff6b9). Server counterpart: warpdotdev/warp-server#13955.Client-side sampling is gated on
FeatureFlag::LrcActivitySignal(dogfood-on), and reported as unsupported on Windows (no controlling-terminal process group to narrow the tree with) and on wasm (no local process table). The wire payload is additive with every field optional, and the server half is behindlrc_activity_signals(off in prod).Agent Mode
Co-Authored-By: Warp Agent agent@warp.dev