Repository navigation
Run up to STRICT_MAX_WORKERS runs concurrently in the run service - #264
Conversation
RunService drained its queue with one worker thread, so a host that reported max_strict_workers=1 could never run two Strict reviews at once, and the reviewbot's per-backend concurrency caps could not take effect. - Settings.strict_max_workers (STRICT_MAX_WORKERS, 1-32, default 1) and StrictRuntimeConfig.max_workers start that many workers; the handshake reports the configured number. Strict API 1.3.0 -> 1.4.0. - RunQueue never hands out two runs with the same conflict key (resolved checkout + PR; issue tasks key on the checkout alone). Runs on one PR head share one PR-time worktree and cursor writes its tool-bridge config to a fixed path inside it. A busy key keeps its run queued in order without holding a worker. A malformed request cannot kill a worker. The checkout is resolved through RepositoryContextResolver (frozen path, REPO_PATHS, adapter manifest), the same resolver execution uses. - A PR run whose worktree cannot be materialized degrades to the live checkout at runtime, which the queue key cannot see. The cursor transport, the only one that writes its bridge config into the session cwd, now serializes bridge-carrying sessions in a shared checkout with an flock in that checkout's git dir (across services and the CLI too); a timeout returns a loud refusal + capability_gap. - The reaper is single-flight per service, and a reused worktree is touched under its materialize lock while the reaper re-reads mtime under its own lock, closing the gap between materialize releasing the tree and the run taking its shared hold. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: d39ff4ccde
ℹ️ 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".
| if (isinstance(self.max_workers, bool) | ||
| or not isinstance(self.max_workers, int) | ||
| or self.max_workers < 1): |
There was a problem hiding this comment.
When an SDK caller supplies max_workers > 32, this public configuration object accepts it, but StrictRuntime(config=...) immediately fails later when Settings.strict_max_workers enforces the 32-worker limit. This exposes an accepted-but-unusable SDK value and raises a raw Pydantic ValidationError instead of the SDK's stable InvalidRequestError; validate the same upper bound here.
Useful? React with 👍 / 👎.
Why
RunServicedrained its queue with one worker thread (MAX_STRICT_WORKERS = 1), so an embedded Strict host could never run two reviews at once. The reviewbot routing work (per-backend concurrency caps, e.g.cursor: 10) needs a host that can. This is PR 4a of that plan; the reviewbot side (PR 4b) consumes it.What
STRICT_MAX_WORKERS(Settings.strict_max_workers, 1–32, default 1, so behaviour is unchanged unless set) andStrictRuntimeConfig.max_workersstart N worker threads.capabilities()reports the configured number asmax_strict_workers. Strict API 1.3.0 → 1.4.0, so a consumer can gate on it.RunQueuenever hands out two runs with the same conflict key: the resolved checkout (viaRepositoryContextResolver: frozen path →REPO_PATHS→ adapter manifest) plus the PR (Nonefor issue tasks).UNKEYEDkey and cannot kill a worker..cursor/mcp.json).flockin that checkout's git dir for the session. This works across threads, processes, services and the CLI.capability_gap.touched under its materialize lock, and the reaper re-reads mtime under its own exclusive lock. This closes the gap betweenmaterializereleasing the tree and the run taking its shared hold._reap()is single-flight per service.Shared-state investigation (two concurrent run subprocesses of one host)
createdgate: one run, one executionrun_status.json/ reservationrun_idgenerationgit fetchinto the shared checkoutrefs/imx/<run_id>/{base,head}; no reader of the sharedFETCH_HEADis reachable from the run service (pr_rebaseis not a read-only kind)<repo>-<owner8>-pr<N>-<sha12>)materializeserialized byLOCK_EX, reuse verifiedgit worktree addinto distinct names.cursor/mcp.jsonin session cwdflock(shared live checkout)-coverrides / per-session scratch dir / run-dir compositionflocktouchon reuse + mtime re-check under the reaper's lock_reap()from several workersupdate-ref -draces onpacked-refs.lockonly delay a ref to the next sweeprefs/imxreapingRunLock(exit-3 lock-loser)<run_dir>/.lock~/.cursor,~/.codex,~/.zcode)strict_backend_concurrency=2)Verification
2018 passed, 17 skipped. Baseline onorigin/mainfd79058 was1992 passed, 17 skipped, so +26 new tests and no regressions. Run withPYTHONPATH=$PWD/src.test/test_run_service_concurrency.py):max_workers→ handshake path, and config boundscheck_spec_freshness --strict,check_doc_links,check_doc_citations: all OK. SPEC pages updated:app/run_service,config,contract,sdk,engine/worktrees,idempotency,providers/cursor.StrictRuntime,max_workers=2, cursor backend, a blob-less scratch clone of vllm-project/vllm-omni,review_depth=light):8274b38c7b7f) and #8284 (done in 782 s, REQUEST CHANGES with 2 comments, at32620f974304).bridge_trace.jsonlreferenced only its own PR worktree: 80/0 and 21/0 lines forpr8290/pr8284.suitepassed on both runs ford39ff4ccd: 36738016783 and 36738091058.For the reviewbot (PR 4b)
StrictRuntimewithStrictRuntimeConfig(max_workers=<cap for that backend>).capabilities().strict_api_version >= 1.4.0andmax_strict_workers == requested. Refuse to start otherwise; an older pinned copilot silently runs 1.max_strict_workersper host. The copilot queues any excess, and it serializes same-PR runs regardless.deploy/infermatrix-release.shato a commit containing this.🤖 Generated with Claude Code