refactor: close out whole-codebase audit to 10/10

.github/workflows/ci.yml

@@ -38,21 +38,42 @@ jobs:
       - uses: actions/setup-node@v4
         with:
           node-version: '20'
+      - uses: dtolnay/rust-toolchain@stable
+      - name: Cargo cache
+        uses: actions/cache@v4
+        with:
+          path: |
+            ~/.cargo/registry
+            ~/.cargo/git
+            target
+          key: ${{ runner.os }}-cargo-${{ hashFiles('Cargo.lock') }}
+          restore-keys: ${{ runner.os }}-cargo-
       - name: Install deps
         run: bun install --frozen-lockfile
-      - name: Build
+      - name: Build daemon bundle
         run: bun run build
+      - name: Build Rust CLI
+        run: cargo build --release --manifest-path crates/cli/Cargo.toml
+      - name: Stage Rust binary into dist/
+        shell: bash
+        run: |
+          set -euo pipefail
+          if [ "$RUNNER_OS" = "Windows" ]; then
+            cp target/release/ghax.exe dist/ghax.exe
+          else
+            cp target/release/ghax dist/ghax
+          fi
       - name: Verify binaries (non-Windows)
         if: runner.os != 'Windows'
         run: |
-          set -e
+          set -eo pipefail
           ./dist/ghax --help | head -5
           test -f dist/ghax-daemon.mjs
       - name: Verify binaries (Windows)
         if: runner.os == 'Windows'
         shell: bash
         run: |
-          set -e
+          set -eo pipefail
           ./dist/ghax.exe --help | head -5
           test -f dist/ghax-daemon.mjs
       - name: Upload artifacts

ARCHITECTURE.md

@@ -159,7 +159,10 @@ instead of silently attaching to Edge.
 map lives on the daemon's active tab. `ghax click @e3` looks up `@e3`
 against that map and drives a Playwright locator.
 
-Refs survive until the next snapshot. If the DOM changed and you run
+Refs survive until the next snapshot — and only on the tab they were
+taken on. `tab <id>` and `new-window` clear the ref map when the active
+page changes, so a stale `@e3` from a previous tab can't silently
+resolve against the wrong DOM. If the DOM changed and you run
 `click @e3`, Playwright fails with a clear "no element" error — fix by
 re-snapshotting.
 

CHANGELOG.md

@@ -6,7 +6,80 @@ Format inspired by [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Breaking
+- Two error-surface tightenings fell out of the `evalInTarget` helper
+  consolidation. Both are behavior improvements but worth flagging for
+  anyone with scripted error handling:
+  - `ext storage` used to return `{ ok: true }` when the underlying JS
+    expression threw. It now throws a `DaemonError` (exit code 4) with
+    the exception details, so a failed `chrome.storage.*.set` no longer
+    silently looks successful.
+  - `ext message` used to return `null` when the cross-extension
+    `chrome.runtime.sendMessage` threw outside its inner try/catch. It
+    now throws a `DaemonError` as well.
+
+### Added
+- `console --since <epoch-ms>` and `network --since <epoch-ms>` filter
+  buffer entries server-side, so callers (notably `qa` and `canary`)
+  don't have to ship hundreds of irrelevant entries across the HTTP
+  RPC just to discard them locally. `console` also accepts `errors:
+  true` via RPC opts so the daemon drops non-error levels before
+  serialising; the Rust CLI uses this on the hot path.
+
+### Fixed
+- **Invariant enforcement**: `ctx.refs` is now cleared when the active
+  tab changes (via `tab <id>` or `new-window`). Previously the
+  "refs survive only until next snapshot" rule held within a tab but
+  broke across tab switches — `@e3` from tab A could silently resolve
+  against tab B's DOM. A new smoke check (`refs cleared on tab
+  switch`) asserts the invariant.
+
 ### Changed
+- Daemon DRY pass: three new helpers collapse repeated shapes.
+  `evalInTarget()` centralises nine `Runtime.evaluate` sites with
+  consistent `exceptionDetails` handling (which also fixes a latent
+  bug in `ext.storage` where a thrown expression was silently
+  returned as `{ok: true}`). `getSwTarget()` owns the five-step
+  find-sw / pool.get / Runtime.enable dance across five extension
+  verbs. `withCdpSession()` owns the session open + try/finally +
+  detach lifecycle across five gesture/profile sites. The three
+  `ext.{panel,popup,options}.eval` handlers now register in a loop
+  since they differ only by URL filter + label.
+- Rust CLI DRY pass: new `time_util` module consolidates three
+  copies of the ISO-8601 / days-to-ymd logic (the `ship` copy was
+  using a slower year-loop algorithm than the other two); new
+  `qa_common` module shares the `console_errors_since` /
+  `failed_requests_since` filters between `qa` and `canary`, with
+  the filtering now happening daemon-side via the new `since:` opt.
+- `dispatch.rs` swaps a hand-rolled percent-encoder for the
+  `urlencoding` crate; `qa.rs` swaps a hand-rolled URL resolver for
+  `url::Url::join`. Adds `url` + `urlencoding` as direct deps (both
+  are tiny; `url` was already in the tree transitively via
+  `reqwest`).
+- `require_daemon` (Rust) now trusts `/health` as the liveness
+  signal and only falls back to the `kill(pid, 0)` syscall when
+  `/health` fails — every CLI invocation shaves a syscall.
+- `snapshot.ts` caches `getComputedStyle()` results per element for
+  the duration of one walk. The cursor-interactive pass used to
+  force-recalc styles O(n · depth) times on SPA-sized trees; now
+  O(n) via a scoped `WeakMap`.
+
+- Daemon: `pageTargetId()` caches the target id on a `WeakMap<Page>`.
+  Playwright target ids are stable for a page's lifetime, but reading
+  one costs a full `CDPSession.newCDPSession` + `Target.getTargetInfo`
+  + detach round-trip. Every command that walks tabs (`activePage`,
+  `tabs`, `find`, `status`, `tab`) used to pay that per page per call.
+  With the cache, the hot path is O(1).
+- Daemon: `tabs` and `find` handlers now fan out per-page
+  `pageTargetId` + `page.title()` in parallel with `Promise.all` instead
+  of a serial await loop. With N tabs open this drops N round-trips to 1.
+- `snapshot.ts`: the aria-tree disambiguation pass used to call
+  `parseLine()` twice per line (once to count role+name duplicates, once
+  to emit). Parsed once into a reused array — meaningful on large SPAs.
+- `dispatch.rs`: removed the dead `stub()` helper and
+  `EXIT_PHASE_PENDING` constant left over from the Rust-port phases,
+  and refreshed the stale "Phase 1 + 2" module doc.
+
 - `attach.rs` simplification (post-/simplify pass): collapsed the
   two-function `spawn_daemon` + `spawn_daemon_with_retry` recursion-with-
   flag into a single `for attempt in 0..2` loop inside `spawn_daemon`.

CLAUDE.md

@@ -32,10 +32,13 @@ the tool in the past.
    daemon pointing at the same state file. For parallel agents, use
    `GHAX_STATE_FILE=/tmp/ghax-<agent>.json` — each gets its own daemon.
 
-3. **Refs survive only until the next snapshot.** `ghax click @e3` looks
-   up `@e3` against the daemon's *last* snapshot ref map. If the DOM
-   changed, re-snapshot first. Never cache ref IDs in code that outlives
-   a single action.
+3. **Refs survive only until the next snapshot — and only on the tab
+   they were taken on.** `ghax click @e3` looks up `@e3` against the
+   daemon's *last* snapshot ref map. If the DOM changed, re-snapshot
+   first. The `tab` and `new-window` handlers clear the ref map when
+   the active page changes, so a stale ref from tab A can't resolve
+   against tab B; a smoke check asserts this. Never cache ref IDs in
+   code that outlives a single action.
 
 4. **Daemon restart required after editing `src/daemon.ts`.** The daemon
    bundle is loaded once at attach time. Changes to `src/daemon.ts` don't

README.md

@@ -218,8 +218,8 @@ chain < steps.json
 record start [name] | stop | status
 replay <file>
 gif <recording> [out.gif] [--delay ms] [--scale px] [--keep-frames]
-console [--errors] [--last N] [--dedup] [--source-maps]
-network [--pattern re] [--status 4xx|500|400-499] [--last N] [--har <path>]
+console [--errors] [--last N] [--since <epoch-ms>] [--dedup] [--source-maps]
+network [--pattern re] [--status 4xx|500|400-499] [--last N] [--since <epoch-ms>] [--har <path>]
 cookies
 ext list
 ext targets <ext-id>

crates/cli/Cargo.toml

@@ -37,3 +37,10 @@ ctrlc = { version = "3", features = ["termination"] }
 rustyline = "15"
 # atty: TTY detection for shell-mode prompt and scripted-mode failure propagation.
 atty = "0.2"
+# url: standard URL parsing — used by qa.rs to resolve crawl-mode hrefs
+# against a base. Already in the tree transitively via reqwest, so pulling
+# it in directly adds no wall-clock cost to builds.
+url = "2"
+# urlencoding: tiny percent-encoding crate for the one place dispatch.rs
+# needs it (ext inspect). Replaces a hand-rolled byte table.
+urlencoding = "2"

crates/cli/src/canary.rs

@@ -18,14 +18,15 @@
 
 use crate::args::Parsed;
 use crate::dispatch::{EXIT_CDP_ERROR, EXIT_OK, EXIT_USAGE};
+use crate::qa_common;
 use crate::rpc;
 use crate::state;
 use anyhow::Result;
 use serde::Serialize;
 use serde_json::{json, Value};
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
-use std::time::{Duration, SystemTime, UNIX_EPOCH};
+use std::time::Duration;
 
 // ── JSON report types ──────────────────────────────────────────────────────
 
@@ -56,38 +57,7 @@ struct CanaryReport {
 
 // ── Helpers ────────────────────────────────────────────────────────────────
 
-fn now_ms() -> u64 {
-    SystemTime::now()
-        .duration_since(UNIX_EPOCH)
-        .unwrap_or(Duration::ZERO)
-        .as_millis() as u64
-}
-
-fn iso_now() -> String {
-    let ms = now_ms();
-    let secs = ms / 1000;
-    let millis = ms % 1000;
-    let s = secs % 60;
-    let m = (secs / 60) % 60;
-    let h = (secs / 3600) % 24;
-    let days = secs / 86400;
-    let (year, month, day) = days_to_ymd(days);
-    format!("{year:04}-{month:02}-{day:02}T{h:02}:{m:02}:{s:02}.{millis:03}Z")
-}
-
-fn days_to_ymd(days: u64) -> (u64, u64, u64) {
-    let z = days + 719468;
-    let era = z / 146097;
-    let doe = z - era * 146097;
-    let yoe = (doe - doe / 1460 + doe / 36524 - doe / 146096) / 365;
-    let y = yoe + era * 400;
-    let doy = doe - (365 * yoe + yoe / 4 - yoe / 100);
-    let mp = (5 * doy + 2) / 153;
-    let d = doy - (153 * mp + 2) / 5 + 1;
-    let m = if mp < 10 { mp + 3 } else { mp - 9 };
-    let y = if m <= 2 { y + 1 } else { y };
-    (y, m, d)
-}
+use crate::time_util::{iso_now, now_ms};
 
 /// Extract hostname from a URL for the log filename.
 /// Returns `"unknown"` on parse failure.
@@ -178,33 +148,8 @@ pub fn cmd_canary(parsed: &Parsed) -> Result<i32> {
                     notes = Some(vec![format!("redirected to {final_url}")]);
                 }
 
-                // Console errors since cycle start.
-                let console_log = rpc::call(port, "console", json!([]), json!({ "last": 500 }))
-                    .unwrap_or(Value::Array(vec![]));
-                console_errors = console_log
-                    .as_array()
-                    .unwrap_or(&vec![])
-                    .iter()
-                    .filter(|e| {
-                        e.get("level").and_then(|v| v.as_str()).unwrap_or("") == "error"
-                            && e.get("timestamp").and_then(|v| v.as_u64()).unwrap_or(0) >= cycle_start
-                    })
-                    .count();
-
-                // Failed network requests since cycle start.
-                let net_log = rpc::call(port, "network", json!([]), json!({ "last": 500 }))
-                    .unwrap_or(Value::Array(vec![]));
-                failed_requests = net_log
-                    .as_array()
-                    .unwrap_or(&vec![])
-                    .iter()
-                    .filter(|e| {
-                        let ts = e.get("timestamp").and_then(|v| v.as_u64()).unwrap_or(0);
-                        let status = e.get("status").and_then(|v| v.as_u64()).unwrap_or(0);
-                        ts >= cycle_start && status >= 400
-                    })
-                    .count();
-
+                console_errors = qa_common::console_errors_since(port, cycle_start, 500).len();
+                failed_requests = qa_common::failed_requests_since(port, cycle_start, 500).len();
             }
         }
         // ok = nav succeeded AND no console/net errors.

crates/cli/src/dispatch.rs

@@ -1,9 +1,4 @@
-//! Verb dispatch table. Mirrors the `switch (verb)` block in `src/cli.ts`.
-//!
-//! Phase 1 + 2 scope: every trivial verb plus medium verbs (attach, detach,
-//! restart, status, qa, canary, review, ship, pair, diff-state, chain, replay,
-//! gif). Phase 3 verbs that need SSE or REPL (shell, console --follow,
-//! network --follow, ext sw logs --follow) still stub out to the Bun CLI.
+//! Verb dispatch table — every CLI verb routes through here.
 
 use crate::args::{self, Parsed};
 use crate::output;
@@ -17,10 +12,6 @@ pub const EXIT_OK: i32 = 0;
 pub const EXIT_USAGE: i32 = 1;
 pub const EXIT_NOT_ATTACHED: i32 = 2;
 pub const EXIT_CDP_ERROR: i32 = 4;
-// Phase 3 wired up the last stubs, but keep this and `stub()` around as the
-// escape hatch for any future verb that lands without a Rust port yet.
-#[allow(dead_code)]
-pub const EXIT_PHASE_PENDING: i32 = 64;
 
 pub fn run(verb: &str, rest: &[String]) -> i32 {
     let cfg = state::resolve_config();
@@ -47,7 +38,6 @@ pub fn run(verb: &str, rest: &[String]) -> i32 {
 }
 
 fn dispatch_inner(cfg: &Config, verb: &str, rest: &[String]) -> Result<i32> {
-    // Phase 2 medium verbs — wired into per-module commands.
     match verb {
         "attach" => return attach::cmd_attach(&args::parse(rest), cfg),
         "detach" => return attach::cmd_detach(cfg),
@@ -62,7 +52,6 @@ fn dispatch_inner(cfg: &Config, verb: &str, rest: &[String]) -> Result<i32> {
         "canary" => return canary::cmd_canary(&args::parse(rest)),
         "review" => return review::cmd_review(&args::parse(rest)),
         "ship" => return ship::cmd_ship(&args::parse(rest)),
-        // Phase 3B — shell REPL.
         "shell" => return crate::shell::cmd_shell(),
         _ => {}
     }
@@ -80,7 +69,6 @@ fn dispatch_inner(cfg: &Config, verb: &str, rest: &[String]) -> Result<i32> {
             simple(cfg, verb, parsed)
         }
 
-        // The "ev" verb (JS execution against the page) — daemon RPC name matches.
         "eval" => {
             let parsed = args::parse(rest);
             simple(cfg, "eval", parsed)
@@ -152,14 +140,6 @@ fn simple_no_args(cfg: &Config, cmd: &str, parsed: Parsed) -> Result<i32> {
     Ok(EXIT_OK)
 }
 
-#[allow(dead_code)]
-fn stub(verb: &str, phase: &str) -> i32 {
-    eprintln!(
-        "ghax: `{verb}` not yet ported to the Rust CLI ({phase}). Use the Bun CLI for now (set GHAX_BIN=./dist/ghax)."
-    );
-    EXIT_PHASE_PENDING
-}
-
 fn dispatch_ext(cfg: &Config, rest: &[String]) -> Result<i32> {
     let Some(sub) = rest.first() else {
         eprintln!("Usage: ghax ext <list|targets|reload|sw|panel|popup|options|storage|message> [...]");
@@ -223,8 +203,7 @@ fn dispatch_ext_sw(cfg: &Config, rest: &[String]) -> Result<i32> {
             let parsed = args::parse(tail);
             if matches!(parsed.flags.get("follow"), Some(Value::Bool(true))) {
                 let port = state::require_daemon(cfg)?;
-                // URL-encode the ext-id to match the TS `encodeURIComponent` call.
-                let encoded_id = url_encode(ext_id);
+                let encoded_id = urlencoding::encode(ext_id);
                 return crate::sse::stream(port, &format!("/sse/ext-sw-logs/{encoded_id}"));
             }
             let port = state::require_daemon(cfg)?;
@@ -276,24 +255,6 @@ fn dispatch_gesture(cfg: &Config, rest: &[String]) -> Result<i32> {
     simple(cfg, cmd, parsed)
 }
 
-/// Percent-encode a string the same way JS `encodeURIComponent` does.
-///
-/// Only unreserved characters (A-Z a-z 0-9 - _ . ~) are left as-is;
-/// everything else is `%XX`-encoded. This matches the daemon's expectation for
-/// the ext-sw-logs SSE path where the ext-id may contain colons or underscores.
-fn url_encode(s: &str) -> String {
-    let mut out = String::with_capacity(s.len());
-    for byte in s.bytes() {
-        match byte {
-            b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
-                out.push(byte as char);
-            }
-            b => out.push_str(&format!("%{b:02X}")),
-        }
-    }
-    out
-}
-
 fn dispatch_record(cfg: &Config, rest: &[String]) -> Result<i32> {
     let Some(sub) = rest.first() else {
         eprintln!("Usage: ghax record <start|stop|status> [name]");