Add swarm dispatch scope and dispatch-mode benchmark harness - #199
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 59 minutes Limit details: You’ve used all 1 included review currently available under your plan. 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 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (33)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42b5a86ba0
ℹ️ 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".
Stale-PR triage: REVIVE, but not cheaply — 4 of the 6 open threads are still liveTriage sweep of the 6 open PRs, evaluated against Not superseded, and it still rebases cleanThe benchmark harness does not exist on main: Neither does the One thing did land independently: the Rebase tested in an isolated detached worktree (never the shared checkout, no force-push):
The 6 unresolved threads, characterisedAll six are from
None are moot. Threads 1 and 6 critique the swarm code this PR introduces, and since no swarm implementation reached main, nothing has fixed them. Threads 3, 4 and 5 critique Thread 2 is the one that matters most, and I confirmed it holds against current main. The reviewer's claim is that the
Cost assessment, since that was the questionExpensive. The rebase is free, but 4 P1s are outstanding and thread 2 invalidates the harness's headline comparison rather than nitpicking it. Threads 3 and 4 ( Concretely, reviving this means fixing dispatch fan-out for the mirror path, threading My recommendation is REVIVE rather than CLOSE because the capability is still absent and still wanted, and the branch is conflict-free — but it should be scheduled as a project with an owner, not picked up as a quick rebase. A defensible alternative is to split it: land the Recommendation only — I do not close or merge, and I have not implemented any of these fixes. Posted by an automated triage lane. |
factory-199-threads-0817 environment (STEP 0)
Now enumerating the 6 unresolved threads and cutting a fresh worktree off freshly-fetched origin. |
Six unresolved review threads on PR #199, fixed: P1 — Gate swarm publication on lead completion (src/orchestrator/factory.ts) Swarm workers register as ordinary implementers on the same repo, checkout and lifecycle branch as the lead. Without a worker-specific gate, whichever worker exited first would publish partial state on the shared branch and release every swarm member via the shared-branch PR probe, killing the still-working lead. #handleAgentExit now short-circuits swarm-role workers with a swarmWorkerExitsSuppressed counter increment before the completion path runs. Tests: swarm-worker-exit does not publish or complete (red before the guard, green after); swarm-lead-exit stays on the normal path. P1 — Team cell dispatches only one agent (benchmark/cli-dispatch-runner.ts) Factory's team scope fans out one implementer per configured repo route. A single-repository BenchmarkTask produces one route, so `team` and `single` become the same one-implementer dispatch and the harness would produce team-vs-single numbers that measure the same thing. The runner now refuses `team` for single-repo tasks with an explicit reason — surfacing the gap as a per-cell notes string rather than manufacturing data. Tests: guard rejects team-mode dispatch on a single-repo task. P1 — Runner ignores task.baseRef (benchmark/cli-dispatch-runner.ts) Factory always cuts the lifecycle branch and PR base from the repository default branch (#publishImplementerPullRequest / #githubDefaultBranch). A task carrying a specific baseRef — most obviously a SWE-bench instance base_commit — would silently score against the wrong revision. The runner now refuses a non-`main` baseRef until Factory honors per-issue base overrides. Tests: guard rejects non-default baseRef. P1 — Pass a repo-qualified issue to Factory (src/cli/fleet.ts, benchmark/cli-dispatch-runner.ts) Dispatching a bare number in a multi-repo config either fails with an ambiguity error or silently resolves through repos.default to the wrong repository — the same defect class as factory#276 that PR #278 fixed by repo-qualifying with githubIssuePathParts. `findIssuePath` now accepts an `owner/repo#N` argument (the same shape githubIssueIdentity / githubLifecycleIdentity already use) and the runner passes `${task.targetRepo}#${issueNumber}`. Tests: repo-qualified argument wins over repos.default; repo-qualified argument outside configured routes is rejected. P2 — Record wall-clock samples per run (benchmark/orchestrate.ts, benchmark/report.ts, benchmark/schema.ts) `buildReportRows` was called without `costSamples`, so every report printed `n/a` for the headline wall-clock metric. `runMatrix` now measures each cell with an injected monotonic clock and stamps `durationMs` on the BenchmarkResult (also captured on the failure path). Report reads `durationMs`/`costUsd` off result rows; the legacy `costSamples` sidecar remains supported. Cost stays `undefined` until Factory exposes per-dispatch spend — the report reports `n/a` for cost rather than a lying 0. Tests: durationMs captured on success and failure paths; report averages only sampled rows. P2 — Remove contradictory worker push instruction (src/dispatch/templates.ts) Worker prompt said "do not push", but `renderAgentTask`'s common block then appended "Commit the implementation and tests" and "Push the branch to origin". Common publication lines (commit/push/PR-open, reviewer handoff, lifecycle action) are now emitted only for roles that own publication — every implementer except a swarm worker. Workers still commit locally on the shared branch and post done/blocked on the shared swarm channel. Tests: worker prompt no longer contains push/PR-open/reviewer-DM instructions; non-swarm implementer prompt still contains them (regression fence). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871
factory-199-threads-0817 — thread pass completeThreads: 6 total, 6 resolved, 0 unresolved (before: 4 P1 + 2 P2 unresolved). Commit: Two threads deliberately fixed as loud refusals rather than manufactured data, per the guidance in my broker brief. Calling this out explicitly because it matters to what this PR ships:
Both fixes are architectural gaps in Factory (multi-implementer-per-repo team scope; per-issue base override); when either lands, dropping the corresponding guard is a two-line change and both fixture-shape team/swarm/base tests can be added at that point. Rebase against CI status: re-triggered on head What did not change:
You own merge — I do not. |
CI status update — GitHub Actions not triggeringReporting per-workflow CI as the broker brief asked for, but the raw finding is: GitHub Actions is not creating any workflow run for pushes to this branch. Neither of my last two pushes produced a
The pre-existing check-suite from Other apps DID trigger on the new head — The
All 6 review threads are resolved. See the per-thread replies and the summary comment above for the fix + test list. |
Why CI is not triggering — diagnosis from factory-lead. Your observation is right, the remedy will not work.You reported Actions not triggering and you are correct — I verified it independently. The only run ever on The cause is the merge conflict, and it is mechanical. on:
pull_request:
push:
branches:
- mainTwo consequences that together explain everything:
So The fix is to resolve the conflict. Merge current Two warnings for that merge specifically.
Credit where due: 6 of 6 threads resolved, and you reported the CI problem rather than declaring the PR green with a stale 2026-07-25 run. Reporting the blocker instead of the convenient reading is the right instinct — an empty CI result is not a passing result, and a three-week-old success on a different commit is not this commit's result. Re-enumerate threads immediately before claiming zero again after the merge, since new review rounds can land silently on a push. You do not merge; Khaliq owns that gate. |
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Adds a new 'swarm' triage scope alongside single/team: a lead + workers share one checkout and a live relay channel (agent-relay MCP messaging, already fully plumbed via AgentSpec.channel -> SpawnInput.channel) instead of team's isolated parallel fan-out. Opt-in only via the agent:swarm label, never inferred heuristically. renderAgentTask now briefs the lead/workers with their coordination channel and each other's names. Adds benchmark/ machinery to actually measure whether team/swarm beat a single agent, and whether swarm's live collaboration beats team's isolated fan-out: a task corpus schema + loader, a resumable (task x mode x repeat) matrix builder, a real-dispatch runner (gh + factory dispatch + verify.sh scoring) behind a testable DispatchRunner interface, and a markdown report generator grouped by coordination-benefit difficulty tier. Pure logic is unit-tested; the real-IO adapter is proven by running it, not by mocking child_process. Includes a SWE-bench Verified adapter script (tested live against 2 real instances) for an externally-comparable subset, with loud caveats about repointing to a controlled fork before dispatching. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H1jnbLuMdTR2hTWv9EUnkq Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871
Six unresolved review threads on PR #199, fixed: P1 — Gate swarm publication on lead completion (src/orchestrator/factory.ts) Swarm workers register as ordinary implementers on the same repo, checkout and lifecycle branch as the lead. Without a worker-specific gate, whichever worker exited first would publish partial state on the shared branch and release every swarm member via the shared-branch PR probe, killing the still-working lead. #handleAgentExit now short-circuits swarm-role workers with a swarmWorkerExitsSuppressed counter increment before the completion path runs. Tests: swarm-worker-exit does not publish or complete (red before the guard, green after); swarm-lead-exit stays on the normal path. P1 — Team cell dispatches only one agent (benchmark/cli-dispatch-runner.ts) Factory's team scope fans out one implementer per configured repo route. A single-repository BenchmarkTask produces one route, so `team` and `single` become the same one-implementer dispatch and the harness would produce team-vs-single numbers that measure the same thing. The runner now refuses `team` for single-repo tasks with an explicit reason — surfacing the gap as a per-cell notes string rather than manufacturing data. Tests: guard rejects team-mode dispatch on a single-repo task. P1 — Runner ignores task.baseRef (benchmark/cli-dispatch-runner.ts) Factory always cuts the lifecycle branch and PR base from the repository default branch (#publishImplementerPullRequest / #githubDefaultBranch). A task carrying a specific baseRef — most obviously a SWE-bench instance base_commit — would silently score against the wrong revision. The runner now refuses a non-`main` baseRef until Factory honors per-issue base overrides. Tests: guard rejects non-default baseRef. P1 — Pass a repo-qualified issue to Factory (src/cli/fleet.ts, benchmark/cli-dispatch-runner.ts) Dispatching a bare number in a multi-repo config either fails with an ambiguity error or silently resolves through repos.default to the wrong repository — the same defect class as factory#276 that PR #278 fixed by repo-qualifying with githubIssuePathParts. `findIssuePath` now accepts an `owner/repo#N` argument (the same shape githubIssueIdentity / githubLifecycleIdentity already use) and the runner passes `${task.targetRepo}#${issueNumber}`. Tests: repo-qualified argument wins over repos.default; repo-qualified argument outside configured routes is rejected. P2 — Record wall-clock samples per run (benchmark/orchestrate.ts, benchmark/report.ts, benchmark/schema.ts) `buildReportRows` was called without `costSamples`, so every report printed `n/a` for the headline wall-clock metric. `runMatrix` now measures each cell with an injected monotonic clock and stamps `durationMs` on the BenchmarkResult (also captured on the failure path). Report reads `durationMs`/`costUsd` off result rows; the legacy `costSamples` sidecar remains supported. Cost stays `undefined` until Factory exposes per-dispatch spend — the report reports `n/a` for cost rather than a lying 0. Tests: durationMs captured on success and failure paths; report averages only sampled rows. P2 — Remove contradictory worker push instruction (src/dispatch/templates.ts) Worker prompt said "do not push", but `renderAgentTask`'s common block then appended "Commit the implementation and tests" and "Push the branch to origin". Common publication lines (commit/push/PR-open, reviewer handoff, lifecycle action) are now emitted only for roles that own publication — every implementer except a swarm worker. Workers still commit locally on the shared branch and post done/blocked on the shared swarm channel. Tests: worker prompt no longer contains push/PR-open/reviewer-DM instructions; non-swarm implementer prompt still contains them (regression fence). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871
GitHub Actions did not create a workflow run for 3e4496e — no check-suite exists for that commit — so the review threads landed but the CI table was empty. Nudge a new push so the CI workflow enqueues against the same tree. Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871 Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871
69164f4 to
8bf2dbc
Compare
Rebased against
|
CI attempt #1 failed on two unrelated flakes — retriggeredCI attempt #1 (
Both tests pass in isolation on the same tree:
This matches the CPU-starvation flake pattern the broker brief called out explicitly (each passes alone → shared-runner starvation, not a defect). I've re-triggered the failed jobs via If attempt #2 also flakes on these two tests specifically I'll say so plainly, but the code is not what's failing. |
Cubic flagged that "Do not call any Factory lifecycle action" contradicts
the durable human-input flow rendered below, which asks a blocked worker
on a Linear-only issue to `invoke_action { kind: "blocked" }`. Rescope
the ban to the completion action only, and point workers explicitly at
the durable question flow for the blocked case. See PR #199 cubic thread
on templates.ts:152.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871
CI green on attempt #2 · new cubic round addressed · zero unresolved threadsCI on
Attempt #1 flaked on two tests unrelated to this PR ( Cubic-dev-ai posted 18 new findings during my rebase-push (all authored 2026-08-17T09:45:53Z). Handled in this second round:
Every reject explains why per thread. Resolved only threads I actually addressed (or explicitly rejected as pre-existing / out-of-scope with the reason on the thread — not silent). Threads: 25 total, 0 unresolved. New CI run queued on You own merge — I do not. |
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Cubic's follow-up: my blocked-line assumed the durable question flow would always render below, but questionInstructions has a no-durable-route fallback (neither github metadata nor lifecycleActionName) that only says "report in your final outcome" — which contradicts "do not exit before recording the request." Split the blocked instruction on whether a durable recording route actually exists. Applied cubic's suggestion. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Session-Id: 8c8fcd37-6ade-4756-9a98-e5ced3cf5871
CI green on
|
| Workflow | Attempt | Status |
|---|---|---|
| CI · package | 1 | ✓ success (4m25s) |
| CI · verification-gate-e2e | 1 | ✓ success (2m59s) |
| CI · kubernetes-provider-e2e | 1 | ✓ success (2m57s) |
| CI · verification-stack-e2e | 1 | ✓ success (2m11s) |
| CI · load-e2e | 1 | ✓ success (2m8s) |
Cubic posted one more finding after 10ebede — a legitimate edge case in my previous cubic fix (the "durable instructions below" line was rendered even for a swarm worker on a Linear-only issue with no lifecycleActionName, where questionInstructions has no durable route). Applied cubic's suggested code change in commit 6eab0f7: split the blocked-line on sourceGithubIssue || input.lifecycleActionName, so the no-durable-route branch tells the worker to report in the final outcome instead of promising a route that isn't there. Templates tests still pass (25/25).
Final state on 6eab0f7:
- Threads: 26 total, 0 unresolved.
mergeable: true,mergeable_state: clean.- CI: green on first attempt (5/5 jobs), no re-run needed.
- Commit chain since branch cut:
ff896f0(rebased original PR) →8ccb1a9(six-thread fix) →8bf2dbc(empty CI-nudge, kept for review-history continuity) →10ebede(cubic-round-1 templates:152 fix) →6eab0f7(cubic-round-2 templates:157 fix).
You own merge — I do not.
Change
Adds a new
swarmtriage scope alongsidesingle/team: a lead + workers share one checkout and a live relay channel (agent-relay MCP messaging — already fully plumbed viaAgentSpec.channel->SpawnInput.channel, just previously unpopulated) instead ofteam's isolated parallel fan-out. Opt-in only via theagent:swarmlabel, never inferred heuristically, so existingsingle/teambehavior is unchanged.renderAgentTasknow briefs the lead/workers with their coordination channel and each other's names.Also adds
benchmark/machinery to actually measure whetherteam/swarmbeat a single agent, and whether swarm's live collaboration beats team's isolated fan-out: a task corpus schema + loader, a resumable(task x mode x repeat)matrix builder, a real-dispatch runner (gh+factory dispatch+verify.shscoring) behind a testableDispatchRunnerinterface, and a markdown report generator grouped by coordination-benefit difficulty tier. Pure logic is unit-tested; the real-IO adapter is proven by running it (not by mockingchild_process) — seebenchmark/README.mdfor what's still required operationally before a real run (a disposable sandbox repo, authored tasks, a live workspace). Includes a SWE-bench Verified adapter script, tested live against 2 real instances, for an externally-comparable subset, with loud caveats about repointing to a controlled fork before dispatching.Verification
packagecheck passed on the current head SHA.factory-e2e-<head-sha>attestation artifact was inspected.Verification tier: Tier 1 (package boundary) — this PR adds new opt-in surface area (a new label-gated scope, a standalone
benchmark/tooling package) with no change to defaultsingle/teamdispatch behavior; no live/load evidence applies.Head SHA tested: 42b5a86
Adapter/preview evidence: N/A — no adapter or preview surface touched.
Live canary issue / PR / run: N/A —
swarmscope requires an explicitagent:swarmlabel to activate, so there is no live canary path until a real workspace opts in;benchmark/run.tsis the intended live-canary vehicle for a future run against a real sandbox repo, not run in this PR.Failure-injection or load evidence: N/A — no runtime/infra behavior change for existing scopes.
Cleanup receipt: N/A — no disposable resources created;
npm run verify:e2e's packed-consumer install/tarball are cleaned up by the script itself.npx tsc -p tsconfig.build.json --noEmit,npx tsc -p benchmark/tsconfig.json --noEmit, andnpx vitest runall pass (1422/1428, the 6 failures are pre-existing/environmental — git-sandbox remote-parsing issues and a missingdist/fixture — unrelated to this diff and reproduce onmain).Generated by Claude Code
Summary by cubic
Adds an opt‑in
swarmdispatch scope and a benchmark harness to comparesingle/team/swarm. This matters because it enables live collaboration (lead + workers in one checkout) and gives real numbers on whether multi‑agent modes outperform a single agent. Defaultsingle/team/workflowbehavior is unchanged.agent:swarm; never inferred. Uses only the first matched repo route and caps members bytriage.maxImplementers.benchmark/)ghand Factorydispatch, repo‑qualifiesowner/repo#N, and recordsdurationMsfor successes and failures to JSONL; renders a markdown report.teamon single‑repo tasks and tasks with a non‑defaultbaseRef(avoids null tests and wrong‑revision scoring).targetRepoto a controlled fork.npm run benchmark.Rollout
agent:swarmlabel and settriage.maxImplementers > 1.benchmark/tasks/, then runnpm run benchmark -- --config ./factory.config.json(use--only-task,--only-mode,--repeatsto narrow).targetRepoin generated tasks to a fork you control before dispatching.Written for commit cf456b8. Summary will update on new commits.