Guard campaign relay providers with native admission - #1691
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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. Comment |
|
Multi-agent review roll call (CodeRabbit and Claude review automatically. Reviewers: post substantive findings only. Authors/agents: address every thread, push fixes to this branch, reply and resolve, then re-request review.) |
|
To use Codex here, create a Codex account and connect to github. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 492c253102
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _read_admission_handshake(descriptor: int) -> dict[str, Any]: | ||
| deadline = time.monotonic() + ADMISSION_HANDSHAKE_SECONDS |
There was a problem hiding this comment.
When launch_reserved_relay is called with a startup timeout below 10 seconds, this creates a fresh independent 10-second deadline, so a stalled admission wrapper can exceed the caller's absolute deadline. The subsequent spawn-error terminalization still uses the already-expired outer deadline and can raise relay_startup_timeout instead of returning the durable terminal RelayLaunch receipt. Pass the outer deadline through the process factory and clamp this wait to its remaining budget.
AGENTS.md reference: AGENTS.md:L406-L408
Useful? React with 👍 / 👎.
| observed_identity = identity(provider_pid) | ||
| if observed_identity is None: | ||
| # An unavailable identity is never authority to release another process's lease. | ||
| return |
There was a problem hiding this comment.
If a single identity probe transiently returns None while the provider is still alive, the detached monitor exits permanently without releasing or refreshing the lease. Once the finite TTL elapses, _cleanup reaps that lease as stale even though the provider continues running, allowing another heavy surface to acquire admission concurrently. Treat an unavailable identity as a bounded retry condition rather than terminating the refresher.
AGENTS.md reference: AGENTS.md:L459-L467
Useful? React with 👍 / 👎.
| _terminalize_relay( | ||
| root, | ||
| relay_id, | ||
| state="failed", | ||
| code=_admission_terminal_code(reasons), |
There was a problem hiding this comment.
When admission is denied before provider execution for a temporary gate such as heavy-lease-held, vitals-shed, or an unavailable pressure sensor, this terminalizes the already-claimed relay as failed. Because failed belongs to _TERMINAL_STATES, every later launch of the same deterministic relay returns that receipt without retrying, so a momentary host condition permanently prevents the required successor from starting. Keep pre-exec admission denials reserved and retryable rather than consuming the relay's sole attempt.
AGENTS.md reference: AGENTS.md:L422-L431
Useful? React with 👍 / 👎.
4444J99
added
the
lifecycle:blocked
1 participant
Summary
executeandlocal-worktreecapabilities, and a supported native transportRoot cause
The merged campaign relay treated an
ianva-*transport as sufficient even for remote-only lanes. Its provider launch also bypassed the authoritative machine-wide heavy gate; a lexical lease aroundPopenwould have released when the finite startup controller returned, before the long-lived provider exited.Validation
Exact head:
120f4816e4e22cc5d7af990b0f1443bc2aaba17eExact tree:
a70337d3f5c83f023e77b134da4b8f1fc21cdaaaPYTHONPATH=cli/src python3 -m pytest -q cli/tests/test_campaign_relay_native_admission.py— 9 passedos.execvpeadapterpressure-sensor-unavailable; remote CI owns those shards asynchronouslyReview provenance
Addresses the unresolved native-lane and host-admission findings on #1669:
No live provider, peer session, host configuration, credential, or signer state was inspected or changed.