Skip to content

fix(browser): normalize wait timeouts#655

Open
hiSandog wants to merge 1 commit intojackwener:mainfrom
hiSandog:fix-wait-timeout-normalization
Open

fix(browser): normalize wait timeouts#655
hiSandog wants to merge 1 commit intojackwener:mainfrom
hiSandog:fix-wait-timeout-normalization

Conversation

@hiSandog
Copy link
Copy Markdown

@hiSandog hiSandog commented Apr 1, 2026

Summary

  • normalize \ in both daemon and direct CDP paths so adapters can pass seconds while older millisecond-style callers keep working
  • switch \ from a fixed sleep to \ plus a selector wait, matching the newer intercept semantics
  • make \ use an explicit 10-second selector timeout and add regression coverage for the new wait helper and 36kr article flow

Testing

  • \

@jackwener/opencli@1.5.5 test
vitest run --project unit src/browser/wait.test.ts src/browser/dom-helpers.test.ts src/browser/cdp.test.ts

RUN v4.1.1 /Users/sandog/sandog/code/kaiyuan-space/opencli

Test Files 3 passed (3)
Tests 13 passed (13)
Start at 14:46:42
Duration 349ms (transform 82ms, setup 0ms, import 113ms, tests 259ms, environment 0ms)

  • \

@jackwener/opencli@1.5.5 test:adapter
vitest run --project adapter src/clis/36kr/article.test.ts src/clis/36kr/hot.test.ts

RUN v4.1.1 /Users/sandog/sandog/code/kaiyuan-space/opencli

Test Files 2 passed (2)
Tests 4 passed (4)
Start at 14:46:42
Duration 79ms (transform 36ms, setup 0ms, import 42ms, tests 10ms, environment 0ms)

  • \

@jackwener/opencli@1.5.5 typecheck
tsc --noEmit

Copy link
Copy Markdown
Contributor

@Astro-Han Astro-Han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 36kr wait improvement and the sinafinance fix both make sense. One thought on the approach:

WaitOptions.timeout is already defined as seconds, and only sinafinance/rolling-news.ts passes the wrong unit (10000 instead of 10). The simpler fix would be to just correct that one caller rather than introducing a unit-guessing function. Auto-detecting "is this seconds or milliseconds?" based on a >= 1000 threshold adds an implicit convention that could confuse future contributors — someone passing timeout: 1000 (1000 seconds) would silently get 1 second instead.

Enforcing one consistent unit (seconds, as already documented) and fixing the one wrong call site keeps the codebase simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants