Skip to content

reliability: reconstruct DNS single-flight on protected main - #205

Draft
seonghobae wants to merge 36 commits into
mainfrom
reliability/reconstruct-dns-singleflight-main-7faf
Draft

reliability: reconstruct DNS single-flight on protected main#205
seonghobae wants to merge 36 commits into
mainfrom
reliability/reconstruct-dns-singleflight-main-7faf

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Purpose

Carry issue #108's live-only DNS single-flight reliability/security boundary onto the exact current protected main without transferring stale predecessor evidence. This is the current-main replacement for #156.

Exact identity

The integrated tree preserves current protected-main release workflow, release runbook, release evidence contract test, and release changelog entry while retaining the validated DNS [Unreleased] entry.

Runtime contract

One canonical (hostname, port) may share only one live raw DNS lookup. Each caller retains its own finite deadline and independently reapplies address scope, local-development policy, deduplication and max_resolved_addresses. Completed results are not cached. Worker-start failures release the resolver slot and remove the live flight without swallowing KeyboardInterrupt/SystemExit; public ordinary failures remain behind the generic denial boundary.

Evidence discipline

Historical #156 checks, reviews, mergeability, and base identity do not transfer to this head. Keep Draft until hosted exact-head CI/security/SAST complete, all current-head findings are addressed, actual immutable-pinned Dependency Review executes successfully rather than being skipped, and any active ruleset-required Strix evidence is substantive rather than wrapper-only/backend-unavailable. Do not self-approve, bypass, Admin-merge, or treat queued/skipped/status/model/predecessor evidence as passing.

Progresses #108. Supersedes #156 after this replacement is observable and #156 is closed as historical predecessor.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 50 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c0f73e2-0e38-483e-a886-fb4f71d15f94

📥 Commits

Reviewing files that changed from the base of the PR and between 7faf7a3 and 3933278.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • docs/research/dns-resolution-resource-bounds.md
  • src/egressweave/validation.py
  • tests/test_dns_resolution_singleflight.py
  • tests/test_dns_resolution_singleflight_documentation.py
  • tests/test_dns_singleflight_docstrings.py
  • tests/test_dns_timeout.py
  • tests/test_dns_worker_start_failure.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@seonghobae
seonghobae marked this pull request as ready for review August 12, 2026 23:18
@seonghobae
seonghobae marked this pull request as draft August 13, 2026 00:18

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Current-head review finding

MAJOR — A short-deadline flight owner can prematurely fail longer-deadline callers

_resolve_all_global_addresses() registers the shared authority flight before acquiring _DNS_RESOLUTION_SLOTS, then uses only the first caller's dns_timeout_seconds for that slot acquisition. If the first caller has a short deadline and the global resolver pool is temporarily full, it stores a shared error and completes the flight. A second caller that already joined the same authority with a longer remaining deadline is woken and denied immediately, even though its own deadline still permits waiting for capacity and performing the lookup.

That contradicts this PR's documented contract that each caller retains its own finite deadline. The path is fail-closed, so it does not widen egress authority, but it creates cross-caller availability coupling and makes the result depend on which caller wins ownership.

Fix direction: make pre-worker capacity exhaustion a retryable flight outcome for non-owner waiters, or otherwise decouple shared-flight creation from the first caller's slot-acquisition budget. The owner whose deadline expires must still fail. A waiter with remaining time must be able to re-enter the registry, become the next owner, acquire a later-available slot, and complete one DNS lookup. Preserve one live resolver per authority, the finite global slot ceiling, no completed-result cache, generic errors, and per-caller address-policy validation.

Regression: orchestrate a short-deadline owner and a longer-deadline joiner while a controlled slot provider rejects the first acquisition and permits the next. Assert the short caller receives the generic denial, the long caller succeeds, exactly one getaddrinfo call runs, the slot is balanced, and the flight registry is empty. Add the async-equivalent case or prove the shared synchronous core covers it under the existing async deadline wrapper.

Copy link
Copy Markdown
Contributor Author

@opencode-agent fix

Address the current-head review finding on 39332783589c8571f15d77ea60170d918fb82461 test-first. Preserve one live DNS resolver per exact authority, the finite global slot ceiling, generic non-leaking denials, no completed-result cache, and caller-specific address validation. Prove that a short-deadline initial owner timing out before worker start cannot poison a longer-deadline waiter that still has time to acquire capacity and perform the single shared lookup. Run the focused concurrency suite, full Python 3.10–3.14 CI, exact 100% owned-production statement/branch coverage, Ruff, compileall, package verification, and update the existing research/changelog wording only where needed. Do not merge or weaken gates.

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