Skip to content

fix: resolve 11 important bugs from deep code review#340

Merged
jackwener merged 1 commit intomainfrom
fix/important-bugs-round3
Mar 24, 2026
Merged

fix: resolve 11 important bugs from deep code review#340
jackwener merged 1 commit intomainfrom
fix/important-bugs-round3

Conversation

@jackwener
Copy link
Owner

Summary

  • I1: Log pre-navigation failures in debug mode instead of silently swallowing (execution.ts)
  • I2: Validate env var timeout values with NaN/negative guard (runtime.ts)
  • I4: Guard indexOf returning -1 for unknown strategies in cascade (cascade.ts)
  • I5: Fix shouldReplaceManifestEntry returning true for same-type entries — was causing unnecessary overwrites (build-manifest.ts)
  • I6: Prevent infinite loop in parseTsArgsBlock cursor advancement (build-manifest.ts)
  • I7: Skip redundant Page.enable CDP calls on repeated goto() (cdp.ts)
  • I8: Fix wait({time:0}) being treated as falsy due to if (options.time) check (page.ts)
  • I10: Warn when cookiesFile path doesn't exist before falling back to browser cookies (download/index.ts)
  • I11: Sanitize tab/newline chars in cookie name/value to prevent Netscape format corruption (download/index.ts)
  • I12: Use DEFAULT_DAEMON_PORT constant instead of hardcoded port in error message (mcp.ts)
  • I15: Log npm install failures in plugin lifecycle instead of swallowing silently (plugin.ts)

Test plan

  • TypeScript typecheck passes (tsc --noEmit)
  • All 223 unit tests pass (npm test)
  • Manual verification of edge cases (NaN env vars, time:0 wait, tab-in-cookie)

🤖 Generated with Claude Code

- I1: Log pre-navigation failures in debug mode instead of silently swallowing
- I2: Validate env var timeout values, fallback on NaN/negative
- I4: Guard against indexOf returning -1 for unknown strategies in cascade
- I5: Fix shouldReplaceManifestEntry returning true for same-type entries
- I6: Prevent infinite loop in parseTsArgsBlock cursor advancement
- I7: Skip redundant Page.enable calls in CDP goto
- I8: Fix wait({time:0}) being treated as falsy
- I10: Warn when cookiesFile path doesn't exist before fallback
- I11: Sanitize tab/newline chars in cookie name/value for Netscape format
- I12: Use DEFAULT_DAEMON_PORT constant instead of hardcoded port in error
- I15: Log npm install failures in plugin lifecycle instead of swallowing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jackwener jackwener merged commit d58e1a7 into main Mar 24, 2026
12 of 13 checks passed
@jackwener
Copy link
Owner Author

Reviewed and merged. I re-ran npm run typecheck and npm test locally; both passed. The only non-blocking note is that same-type duplicate manifest entries are still "first one wins" and silent, but that does not block this PR.

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.

1 participant