From 2ce19e7075fb8bd8f95f3917e729322ea11262e4 Mon Sep 17 00:00:00 2001 From: Jean-Pierre Bergamin Date: Tue, 11 Aug 2026 14:51:16 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat(iter-142):=20session=20hygiene=20?= =?UTF-8?q?=E2=80=94=20daemon=20stop=20pid=20honesty,=20disk=20GC,=20eval?= =?UTF-8?q?=20ASI=20fix,=20wait=20sleep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-causes the daemon-stop false-negative (dogfooding session 63, 3/3 reproducible) to launch-record.json being a single global file clobbered by concurrent launches on different ports; scopes it per port and makes the proxy-registry fallback path kill the real Firefox pid instead of the daemon's own. Adds immediate (age-independent) reclamation of dead-owner temp profiles, GC for stale throttle-state files, and GC for the legacy port-less spawn lock. Fixes eval's async-IIFE wrapper to detect ASI-separated statement boundaries (not just `;`) and to auto-return a trailing bare expression instead of silently dropping it as undefined. Adds `wait --sleep-ms` (with a `--time` legacy alias) as a plain-delay form that skips the Firefox connection entirely. Defers Theme C (auto-consent honesty + BBC CMP coverage), Theme D (full-page screenshot header dedup), and the console-locale item to iteration-144 — see that plan's "why these were deferred" section. Co-Authored-By: Claude Opus 5 --- crates/ff-rdp-cli/src/cli/args.rs | 32 +- crates/ff-rdp-cli/src/commands/eval.rs | 323 ++++++++++++++++-- crates/ff-rdp-cli/src/commands/launch.rs | 15 + crates/ff-rdp-cli/src/commands/wait.rs | 67 +++- crates/ff-rdp-cli/src/daemon/client.rs | 65 +++- crates/ff-rdp-cli/src/daemon/registry.rs | 99 ++++++ .../ff-rdp-cli/src/daemon/throttle_state.rs | 130 +++++++ crates/ff-rdp-cli/src/daemon_record.rs | 150 ++++++-- crates/ff-rdp-cli/src/dispatch.rs | 2 + crates/ff-rdp-cli/src/script/runner.rs | 4 + crates/ff-rdp-cli/src/util/profile_dir.rs | 75 +++- crates/ff-rdp-cli/tests/e2e/eval.rs | 46 +++ crates/ff-rdp-cli/tests/e2e/wait.rs | 93 +++++ .../live/live_142_daemon_stop_pid_honesty.rs | 139 ++++++++ .../tests/live/live_142_disk_growth.rs | 243 +++++++++++++ .../tests/live/live_142_eval_asi_await.rs | 69 ++++ crates/ff-rdp-cli/tests/live/main.rs | 3 + .../iteration-142-session-hygiene.md | 60 +++- .../iteration-144-session-hygiene-followup.md | 114 +++++++ 19 files changed, 1627 insertions(+), 102 deletions(-) create mode 100644 crates/ff-rdp-cli/tests/live/live_142_daemon_stop_pid_honesty.rs create mode 100644 crates/ff-rdp-cli/tests/live/live_142_disk_growth.rs create mode 100644 crates/ff-rdp-cli/tests/live/live_142_eval_asi_await.rs create mode 100644 kb/iterations/iteration-144-session-hygiene-followup.md diff --git a/crates/ff-rdp-cli/src/cli/args.rs b/crates/ff-rdp-cli/src/cli/args.rs index 740ed7a..e01bd36 100644 --- a/crates/ff-rdp-cli/src/cli/args.rs +++ b/crates/ff-rdp-cli/src/cli/args.rs @@ -742,16 +742,24 @@ other. On success, the output gains {\"match_count\": N, \"chosen_index\": N}. Output: {\"results\": {\"typed\": true, \"tag\": \"INPUT\", \"value\": \"...\"}, \"total\": 1, \"meta\": {...}}")] Type(TypeArgs), - /// Wait for a condition to become true (polls every 100ms). - /// Exactly one of --selector, --text, --eval, or --ref must be specified. - #[command(long_about = "Wait for a condition to become true (polls every 100ms). + /// Wait for a condition to become true (polls every 100ms), or sleep for a fixed duration. + /// Exactly one of --selector, --text, --eval, --ref, or --sleep-ms must be specified. + #[command( + long_about = "Wait for a condition to become true (polls every 100ms), or sleep for a fixed duration. -Exactly one of --selector, --text, --eval, or --ref must be specified. +Exactly one of --selector, --text, --eval, --ref, or --sleep-ms must be specified. Use --ref to wait for an element identified by its ARIA-tree ref ID (daemon mode only). Equivalent to --selector but uses a stable ref handle. -Output: {\"results\": {\"matched\": true, \"elapsed_ms\": N, \"condition\": \"selector|text|eval\"}, \"total\": 1, \"meta\": {...}}")] +Use --sleep-ms for a plain delay with no condition and no Firefox +connection at all — e.g. `ff-rdp wait --sleep-ms 2000`. Prefer a real +condition (--selector/--text/--eval/--ref) whenever one exists; a fixed +sleep is always a guess about how long something takes. --timeout-ms does +not apply to --sleep-ms, which always runs for exactly its own duration. + +Output: {\"results\": {\"matched\": true, \"elapsed_ms\": N, \"condition\": \"selector|text|eval|sleep\"}, \"total\": 1, \"meta\": {...}}" + )] Wait(WaitArgs), /// List cookies via the Firefox StorageActor (includes httpOnly, secure, sameSite, etc.) #[command( @@ -1695,8 +1703,22 @@ pub struct WaitArgs { /// ARIA-tree ref ID from a previous dom/snapshot call (daemon mode only, e.g. 'e3') #[arg(long = "ref", value_name = "REF_ID", group = "condition")] pub ref_id: Option, + /// Plain sleep for this many milliseconds — no condition, no Firefox + /// connection, just a delay. For when you need to pace commands rather + /// than wait for a specific page state (use --selector/--text/--eval/--ref + /// instead whenever a real condition exists — a fixed sleep is always a + /// guess). The legacy spelling `--time` is also accepted as a hidden + /// alias (iter-142: this was the flag dogfooders reached for first). + #[arg( + long = "sleep-ms", + alias = "time", + value_name = "MS", + group = "condition" + )] + pub sleep_ms: Option, /// Timeout in milliseconds before giving up (canonical flag — use this one). /// The legacy spelling `--wait-timeout` is also accepted as a hidden alias. + /// Not used by --sleep-ms, which always runs for exactly its own duration. #[arg(long = "timeout-ms", alias = "wait-timeout", default_value_t = 5000)] pub wait_timeout: u64, } diff --git a/crates/ff-rdp-cli/src/commands/eval.rs b/crates/ff-rdp-cli/src/commands/eval.rs index f15e07c..a04fb37 100644 --- a/crates/ff-rdp-cli/src/commands/eval.rs +++ b/crates/ff-rdp-cli/src/commands/eval.rs @@ -104,6 +104,17 @@ pub(crate) fn load_script( /// `evaluateJSAsync` already knows how to await. `eval_path` stays /// `"page-await"` either way; from the caller's perspective only the /// previously-broken await scripts start working, nothing else changes. +/// +/// iter-142 Theme E fixed two follow-on defects in the same wrap: (1) the +/// single-vs-multi-statement heuristic (used to decide whether the wrap +/// synthesizes a `return`) only recognized `;` as a statement separator, so +/// an ASI-separated (newline-only) multi-statement script like +/// `await Promise.resolve(1)\n42` was misclassified as one expression and +/// wrapped into invalid JS — a syntax error reported past the end of the +/// user's input; (2) even when correctly classified as multi-statement, the +/// wrap never returned anything, so a trailing bare expression silently +/// became `{"type":"undefined"}` instead of its real value. See +/// [`top_level_statement_boundaries`] and [`wrap_top_level_await`]. pub(crate) fn build_script(user_script: &str, stringify: bool, _isolate: bool) -> String { // The stringify helper: if the value is already a string, return it as-is; // otherwise JSON.stringify it. This prevents double-encoding when the JS @@ -148,13 +159,15 @@ const STATEMENT_LEADING_KEYWORDS: &[&str] = &[ /// Best-effort (not a JS parser) check for whether `script` is a single /// expression, safe to wrap as `return (