fix(wait): settle --idle on held-open streams; report settled URL in target (#5)

* fix(wait): settle --idle on held-open streams (websocket/long-poll/SSE)

wait --idle counted requestWillBeSent up and loadingFinished/Failed down and
required inflight == 0. A websocket / long-poll / EventSource request fires
requestWillBeSent but never loadingFinished, so inflight never returned to
zero and --idle hung until --timeout on SPAs that hold such a stream open
(observed on Workday: a 20s timeout on an already-loaded page).

Settle when EITHER no requests are in flight for the window (the clean path,
unchanged for pages whose requests all complete) OR requests remain open but
the connection has been silent for ~2s (the stalled path). Response/data
events reset the silence clock, so an in-progress download is never mistaken
for a stalled stream — only a genuinely quiet still-open request settles.

Adds TestWaitIdleStalledStream (a held-open request settles via the stalled
path in ~2s instead of hanging); TestWaitIdle still guards the clean path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(wait,nav): report the settled URL in the envelope target

The envelope's target was captured before the action ran, so after a nav or a
--url / redirect wait it showed the pre-navigation URL — misleading (a wait
--url that matched could echo a target URL that didn't even contain the
matched substring).

Wait now reads the tab's location after the condition holds and returns it as
result.url; targetAction syncs target.url from any result carrying a url key
(nav already returned it). No extra round-trips.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(wait): collapse waitIdle to a single activity clock

Review follow-up (/simplify + /deslop) — no behavior change:

- waitIdle tracked two clocks (lastZero + lastActivity); lastZero is derivable
  (it only diverged from lastActivity in the pre-Enable in-flight-request edge,
  where a single clock is actually more correct — it respects active bytes
  instead of settling early). Collapse to one lastActivity clock and select the
  threshold by whether requests are in flight: `window` when idle, `idleStall`
  when a stream is held open. Drops a field, a branch, and one idle predicate.
- Dedup the "held-open stream / not-a-stalled-download" rationale that was
  stated three times (doc comment + two inline comments) down to once.

Live idle tests (clean + stalled paths) and the full race suite stay green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: sanketsudake

internal/chrome/cdp.go

@@ -1397,10 +1397,13 @@ func (c *CDP) Wait(ctx context.Context, id string, cond WaitCond) (map[string]an
 	default:
 		return nil, fmt.Errorf("wait needs one of --url, --visible, --gone, --text, --stable, --idle, --for")
 	}
-	if err := c.run(ctx, id, action); err != nil {
+	// Also read the URL the tab settled at, so the envelope's target reflects
+	// where a --url / redirect wait actually landed, not the pre-wait URL.
+	var loc string
+	if err := c.run(ctx, id, action, chromedp.Location(&loc)); err != nil {
 		return nil, err
 	}
-	return map[string]any{"waited": what}, nil
+	return map[string]any{"waited": what, "url": loc}, nil
 }
 
 // waitURL polls location.href until it contains substr (or the context ends).
@@ -1473,37 +1476,58 @@ func waitStable(window time.Duration) chromedp.Action {
 	})
 }
 
-// waitIdle returns once network activity has settled: no in-flight requests for
-// `window`. It tracks requestWillBeSent vs loadingFinished/Failed — for SPA
-// loads (Outlook, Workday) where "DOM loaded" ≠ "content rendered".
+// waitIdle returns once network activity has settled. It considers the page
+// settled when EITHER no requests are in flight for `window` (the clean path,
+// for pages whose requests all complete), OR requests remain open but the
+// connection has gone silent for `idleStall` (the stalled path). The stalled
+// path is what makes --idle usable on SPAs (Outlook, Workday) that hold a
+// websocket / long-poll / EventSource stream open indefinitely: such a request
+// fires requestWillBeSent but never loadingFinished, so inflight never returns
+// to zero and a strict "inflight == 0" wait would hang until --timeout. Progress
+// events (response/data), not just start/finish, keep the clock live, so an
+// in-progress download is never mistaken for a silent held-open stream.
 func waitIdle(window time.Duration) chromedp.Action {
+	// A still-open request is treated as idle after this much network silence.
+	// Longer than `window` so a normally-completing load always settles via the
+	// clean path first; short enough that a held-open stream doesn't wait out
+	// the whole --timeout.
+	const idleStall = 2 * time.Second
 	return chromedp.ActionFunc(func(ctx context.Context) error {
 		if err := network.Enable().Do(ctx); err != nil {
 			return err
 		}
 		var mu sync.Mutex
 		inflight := 0
-		lastZero := time.Now()
+		// lastActivity is the last time any request started, progressed, or
+		// ended — set only for network events (ListenTarget also delivers page/
+		// DOM events, which must not count as network activity).
+		lastActivity := time.Now()
 		chromedp.ListenTarget(ctx, func(ev interface{}) {
 			mu.Lock()
 			defer mu.Unlock()
 			switch ev.(type) {
 			case *network.EventRequestWillBeSent:
 				inflight++
+				lastActivity = time.Now()
 			case *network.EventLoadingFinished, *network.EventLoadingFailed:
 				if inflight > 0 {
 					inflight--
 				}
-				if inflight == 0 {
-					lastZero = time.Now()
-				}
+				lastActivity = time.Now()
+			case *network.EventResponseReceived, *network.EventDataReceived:
+				// bytes moving on an open request — keep it counted as active
+				lastActivity = time.Now()
 			}
 		})
 		t := time.NewTicker(100 * time.Millisecond)
 		defer t.Stop()
 		for {
 			mu.Lock()
-			idle := inflight == 0 && time.Since(lastZero) >= window
+			threshold := window
+			if inflight > 0 {
+				threshold = idleStall // still-open requests need the longer stall window
+			}
+			idle := time.Since(lastActivity) >= threshold
 			mu.Unlock()
 			if idle {
 				return nil

internal/chrome/wait_idle_test.go

@@ -55,3 +55,55 @@ addEventListener('load', () => setTimeout(() => {
 		t.Errorf("window.__done = %q after wait --idle, want true (idle returned too early, in %v)", got, time.Since(start))
 	}
 }
+
+// wait --idle must still settle when a request is held open indefinitely (a
+// websocket / long-poll / EventSource — the shape that made --idle hang on
+// Workday): inflight never returns to zero, so it settles via the stalled path
+// once the connection goes silent, rather than waiting out --timeout.
+func TestWaitIdleStalledStream(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping live-Chrome integration in -short mode")
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancel()
+
+	// Build the server BEFORE launching Chrome so its Close() defer runs LAST
+	// (LIFO), after b.Close() tears Chrome down. Otherwise srv.Close() blocks on
+	// the still-open /hang request, which only unblocks when Chrome disconnects
+	// — a teardown deadlock. /hang stays open until the request context is
+	// cancelled (Chrome dropping the connection at b.Close()).
+	mux := http.NewServeMux()
+	mux.HandleFunc("/hang", func(_ http.ResponseWriter, r *http.Request) {
+		<-r.Context().Done()
+	})
+	mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) {
+		fmt.Fprint(w, `<!doctype html><title>Stalled</title><body><script>
+addEventListener('load', () => setTimeout(() => { fetch('/hang'); }, 50));
+</script></body>`)
+	})
+	srv := httptest.NewServer(mux)
+	defer srv.Close()
+
+	b, err := launch(true, tmpProfile(t), 0)
+	if err != nil {
+		t.Skipf("cannot launch a managed headless Chrome here: %v", err)
+	}
+	defer b.Close()
+
+	id := firstTab(ctx, t, b)
+	if _, err := b.Navigate(ctx, id, srv.URL); err != nil {
+		t.Fatalf("Navigate: %v", err)
+	}
+
+	// Bound the wait well under the 30s+ a held-open request would otherwise
+	// cost: if the fix works it settles via the stalled path in ~2s.
+	wctx, wcancel := context.WithTimeout(ctx, 10*time.Second)
+	defer wcancel()
+	start := time.Now()
+	if _, err := b.Wait(wctx, id, WaitCond{Idle: true}); err != nil {
+		t.Fatalf("Wait --idle hung on a held-open request: %v", err)
+	}
+	if elapsed := time.Since(start); elapsed >= 8*time.Second {
+		t.Errorf("wait --idle took %v — it waited for the held-open request instead of settling on silence", elapsed)
+	}
+}

internal/cli/commands.go

@@ -753,6 +753,13 @@ func (a *App) targetAction(command string, fn func(ctx context.Context, b chrome
 			a.emitErr(command, code, msg, details)
 			return nil
 		}
+		// If the action settled at a new URL (nav, wait --url / redirect), report
+		// that in the envelope's target rather than the pre-action URL.
+		if m, ok := res.(map[string]any); ok {
+			if u, ok := m["url"].(string); ok && u != "" {
+				tgt.URL = u
+			}
+		}
 		// --wait-text folds "act, then confirm the write landed" into one call:
 		// after the action succeeds, block until the page contains the text.
 		if a.actWaitText != "" {

internal/cli/target_url_test.go

@@ -0,0 +1,56 @@
+package cli
+
+// Regression: the envelope's target must reflect the URL the tab settled at
+// after a nav / redirect wait, not the pre-action URL.
+
+import (
+	"context"
+	"testing"
+
+	"github.com/sanketsudake/chrome-cdp-cli/internal/chrome"
+	"github.com/sanketsudake/chrome-cdp-cli/internal/target"
+)
+
+// navWaitBrowser returns a settled URL from nav/wait so the CLI can echo the
+// post-action URL in the envelope's target.
+type navWaitBrowser struct {
+	fakeBrowser
+	settled string
+}
+
+func (n *navWaitBrowser) Navigate(_ context.Context, _, _ string) (map[string]any, error) {
+	return map[string]any{"url": n.settled}, nil
+}
+func (n *navWaitBrowser) Wait(_ context.Context, _ string, _ chrome.WaitCond) (map[string]any, error) {
+	return map[string]any{"waited": "url:after", "url": n.settled}, nil
+}
+
+func TestNavReportsSettledURL(t *testing.T) {
+	t.Parallel()
+	b := &navWaitBrowser{
+		fakeBrowser: fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "A", URL: "https://before/"}}},
+		settled:     "https://after/redirected",
+	}
+	env, _, code := run(t, b, "nav", "https://before/", "--target", "aa11", "--json")
+	if code != 0 {
+		t.Fatalf("exit = %d, want 0", code)
+	}
+	if got := env["target"].(map[string]any)["url"]; got != "https://after/redirected" {
+		t.Errorf("target.url = %v, want the settled URL (not the pre-nav URL)", got)
+	}
+}
+
+func TestWaitReportsSettledURL(t *testing.T) {
+	t.Parallel()
+	b := &navWaitBrowser{
+		fakeBrowser: fakeBrowser{tabs: []target.Info{{ID: "aa11", Title: "A", URL: "https://before/"}}},
+		settled:     "https://after/home",
+	}
+	env, _, code := run(t, b, "wait", "--url", "home", "--target", "aa11", "--json")
+	if code != 0 {
+		t.Fatalf("exit = %d, want 0", code)
+	}
+	if got := env["target"].(map[string]any)["url"]; got != "https://after/home" {
+		t.Errorf("target.url = %v, want the settled URL", got)
+	}
+}