Bound review fan-out: shared agent budget, live ignore_paths, evidence caps - #68
Merged
Merged
Conversation
Phase 6 (coverage loop) and Phase 6.7 (consistency-verify) run concurrently, but each brought its own limiter — a fresh Semaphore(8) per _run_parallel_review call, and none at all for the 12 verify_obligation agents — so ~16 harness subprocesses could be in flight at once and the node OOM-crashed reviewing a lockfile-regen PR (#65). One orchestrator-owned semaphore now gates every leaf agent invocation across all phases. It is sized min(max_concurrent_agents, max_concurrent_reviewers) so the deprecated reviewer knob still binds, and is env-configurable via PR_AF_MAX_CONCURRENT_AGENTS. The consistency-verify obligation cap moves from a literal to PR_AF_MAX_CONSISTENCY_OBLIGATIONS. Slots are only ever held around the leaf call itself — never while awaiting children — so low caps serialize instead of deadlocking. Also moves build_dimension_pack off the event loop (it forks grep subprocesses synchronously). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ignore_paths was dead config: defined with the right defaults (package-lock.json, yarn.lock, vendor/**, *.min.js, …), merged from API input, and never consulted anywhere. The 60k-line lockfile regen that triggered the crash in #65 flowed straight into every agent prompt, tripped depth escalation, and fed obligation extraction. Filter changed_files (and rebuild the diff from the kept patches, in the same format the GitHub client already uses for oversized PRs) before intake, so depth resolution, anatomy, meta-selectors, reviewers, and consistency-verify only ever see reviewable files. Behavior note: the defaults also ignore *.md, *.txt, and .github/** — those were previously reviewed only because the config was inert. Callers who want them reviewed can pass ignore_paths=[] per call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Evidence extraction forked one repo-wide grep per identifier mentioned in a finding body, with no cap and 10 findings extracting concurrently — these child processes stacked on top of the opencode subprocesses during the #65 crash (EAGAIN on thread spawn). Cap it at 8 identifiers per finding. _FILE_CACHE is process-lifetime and was bounded only by entry count: 2000 large files can pin multiple GB across reviews. Bound it by bytes (128MB), and serve oversized single files uncached. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 10, 2026
Workspaces under PR_AF_WORKDIR were created per reviewed PR and never removed, so the persistent volume grew without bound (#65). Reap lazily on workspace resolution: directories idle longer than the TTL (default 7 days, <= 0 disables) are deleted. The workspace being resolved for the current review is never touched, and idleness is measured off .git/FETCH_HEAD — which every review's fetch rewrites — so concurrently active workspaces are naturally skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
An unpinned 'agentfield>=0.1.84' means every image rebuild silently re-resolves the SDK — the Aug 4 redeploy jumped <=0.1.117 -> 0.1.120 as a side effect of a packaging-only commit (#65). Pin the exact version so SDK upgrades are deliberate, reviewable bumps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Note on the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #65.
Summary
The Aug 5 OOM crash was caused by the pipeline's peak-concurrency window having no shared limiter: Phase 6 (coverage loop) and Phase 6.7 (consistency-verify) run concurrently, the former with a fresh per-call
Semaphore(8)and the latter with none at all, so ~16 opencode subprocesses could be in flight while a 60k-line lockfile-regen diff inflated every prompt. This PR bounds every contributing layer on the pr-af side; the SDK-side buffering fix is Agent-Field/agentfield#903.Changes
1. Review-wide agent-concurrency budget (
feat(budget))min(max_concurrent_agents, max_concurrent_reviewers)so the deprecated reviewer knob still binds.PR_AF_MAX_CONCURRENT_AGENTS(default 8), honored by the webhook limits path and thereview()API.[:12]literal toPR_AF_MAX_CONSISTENCY_OBLIGATIONS.build_dimension_pack(which forks grep) moves off the event loop viaasyncio.to_thread.2.
ignore_pathswas dead config — now applied (fix(intake))**/package-lock.json,**/yarn.lock,vendor/**,*.min.js, … but was never consulted. Files matchingignore_pathsare now dropped before intake and the diff is rebuilt from the kept patches, so depth resolution, anatomy, meta-selectors, reviewers, and obligation extraction only ever see reviewable files. A lockfile-regen PR now resolves to aquickreview of nothing instead of a depth-escalated 16-agent fan-out.*.md,*.txt, and.github/**— previously reviewed only because the config was inert. Passignore_paths=[]per call to restore that.3. Evidence-extraction caps (
perf(evidence))grepchildren capped at 8 identifiers per finding (was unbounded)._FILE_CACHEis now byte-bounded (128 MB) rather than entry-count-only, and oversized single files are served uncached.4. Stale-workspace reaper (
feat(workspaces))PR_AF_WORKDIRwere never removed — one checkout per reviewed PR, forever, on the persistent volume. Workspaces idle pastPR_AF_WORKSPACE_TTL_DAYS(default 7,<= 0disables) are now reaped lazily on workspace resolution. The active workspace is never touched; idleness is measured off.git/FETCH_HEAD, which every review's fetch rewrites.5. Pin
agentfield==0.1.126in the Dockerfile (build(docker))>=0.1.84meant every rebuild silently re-resolved the SDK (the Aug 4 redeploy jumped ≤0.1.117 → 0.1.120 as a side effect of a packaging-only commit). Upgrades are now deliberate one-line bumps.Validation contract → tests
test_shared_budget_caps_leaf_agents_across_concurrent_phasesmax_concurrent_reviewersstill binds →test_deprecated_reviewer_knob_still_bindstest_consistency_obligation_cap_is_configurabletest_env_knobs_drive_budget_defaults,test_input_override_plumbs_through,test_webhook_limits_include_agent_budgetquick→test_lockfile_regen_is_filtered_before_intake**/name,dir/**, basename globs) →test_pattern_semanticstests/test_evidence_caps.pytests/test_workspace_reaper.pyRan locally: full pytest (103 passed) and
ruff check src/ scripts/clean, per-commit. Also verified end-to-end: a real review through a local control plane + this branch's Python node (opencode mocked via the e2e shim, live.ai()gates) — review succeeded under a budget of 3, the fixture'spackage-lock.jsonwas filtered at intake (1607 diff lines dropped), and measured peak concurrent opencode subprocesses was 2.Related
OPENCODE_MAX_CONCURRENTon the deployment.🤖 Generated with Claude Code