Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe preflight now places DNS resolution and HTTP probing under one 8-second lifetime. DNS timeout produces an advisory, while shutdown and interruption propagate. Tests cover stalled and late DNS results, shared-deadline behavior, and cleanup. Documentation describes the shared budget. ChangesTarget URL preflight lifetime
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant probeTargetUrl
participant dnsLookup
participant fetchImpl
probeTargetUrl->>dnsLookup: Resolve hostname within shared deadline
dnsLookup-->>probeTargetUrl: Return DNS result
probeTargetUrl->>fetchImpl: Start HTTP probe within remaining budget
fetchImpl-->>probeTargetUrl: Return HTTP response
Merge Risk: ⚪ Minimal · up to
|
|
🏆 Season 4 CLI Improvement Bonus: entry recorded. Thanks, @lxcario! To be counted, this PR needs the following by Oct 29, 11:59 PM PT:
Rewards go to merged PRs judged a genuine improvement, and up to three bonus-eligible PRs count per person. Untick the box to withdraw the entry. This comment refreshes when you edit the PR. Full rules. |
What does this PR do?
Target URL preflight currently awaits DNS before arming its timeout or observing shutdown. A stalled resolver can therefore hold a run command indefinitely before the trigger request, and Ctrl-C cannot settle the probe while DNS is pending.
Start critical-operation tracking and one shared 8-second DNS/HTTP deadline before the lookup. Stop waiting promptly on shutdown or timeout, clear the deadline timer on every exit, and remove the DNS abort listener. Late resolver results cannot start HTTP; late rejections remain handled.
DNS timeouts retain the existing advisory policy. Interruption propagates the original reason without dispatching a run. Existing DNS refusal/proxy handling, resolved-address checks, redirect behavior, literal-IP handling, and
--skip-preflightremain covered.Related issue
Closes #502
Season 4 CLI Improvement Bonus
Discord username: resquwue
Type of change
Checklist
mainbranch.npm run lintandnpm run format:checkpass.npm run typecheckpasses.npm run test:coverage, with all four coverage measures above 80%.DOCUMENTATION.md.Notes for reviewers
--version, and--helppass.npm run test:e2e -- --testTimeout=15000: 101 passed, 10 skipped, across eight passing and two skipped files. With the default 5-second limit, the agent-install test that launches all eight targets times out on this Windows machine; the identical isolated test also times out with the original production source (7.23 s). No repository timeout setting or unrelated test was changed.--skip-preflightremains available.Summary by CodeRabbit