Merged
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (32 files)
|
jrf0110
force-pushed
the
540-wire-convoys-e2e
branch
from
c37cc96 to
c06babc
Compare
jrf0110
force-pushed
the
540-wire-convoys-e2e
branch
from
b7b53ca to
4e8ddf4
Compare
…odes, and DAG visualization Wire convoys end-to-end with feature branches, DAG dependency tracking, two merge modes, and full UI visualization. ## Convoy Feature Branches - Convoys create long-lived feature branches (convoy/<slug>/<id>/head) - Agent branches are siblings under the same prefix (convoy/<slug>/<id>/gt/<agent>/<bead>) - Sub-beads branch from and merge into the feature branch - Final landing MR merges feature branch to main when all beads complete - Always clone from rig default branch (feature branch may not exist on remote yet) ## DAG-Aware Scheduling - Beads with 'blocks' dependencies only dispatch when all blockers close - dispatchUnblockedBeads fires when a bead closes, cascading through DAG - Bead status transitions to in_progress before container start (not after) - Cycle detection (Kahn's algorithm) rejects circular depends_on graphs - Guard prevents reverting terminal statuses (closed/failed) to in_progress ## Convoy Merge Modes - review-then-land (default): refinery reviews each bead and merges into feature branch via direct merge; final PR/merge to main at convoy end - review-and-merge: each bead goes through full review + merge/PR cycle independently using the town's configured merge strategy - Mayor's gt_sling_batch tool accepts merge_mode parameter - Refinery system prompt includes convoy context for each mode ## Mayor Prompt — depends_on Emphasis - Dedicated section: 'depends_on — THE MOST IMPORTANT PART OF CONVOY PLANNING' - Default assumption shifted to 'most beads need depends_on' - Common patterns: foundation first, API before UI, code before tests - Bad/good examples with explicit dependency chains ## DAG Visualization - ConvoyTimeline renders beads in topological wave order (Kahn's algorithm) - Arrow indicators between dependency waves - Feature branch name shown in convoy header - BeadPanel drawer shows convoy membership, blockers, and dependents - Convoy section constrained to max-h-[40vh] with internal scroll - min-w-0 chain prevents horizontal page overflow from bead chips ## Data Model - convoy_metadata: feature_branch, merge_mode columns (with migrations) - getConvoyDependencyEdges, getConvoyForBead, getConvoyFeatureBranch, getConvoyMergeMode query helpers - tRPC schemas include dependency_edges, feature_branch, merge_mode ## Tests - 20 integration tests: feature branches, DAG edges, scheduling, progress tracking, merge modes, force close, edge cases - 15 unit tests: branch naming, git ref conflict prevention, cycle detection Refs: #204, #540
1. review-and-merge convoys now target the rig's default branch (not the convoy feature branch), so each bead lands independently 2. Standalone reviews use rig's default_branch instead of hardcoded 'main' 3. review-and-merge convoys auto-close immediately instead of queuing a redundant feature-branch landing step 4. Convoy progress only counts beads whose MR reviews are fully resolved, preventing premature ready_to_land while reviews are in flight 5. Cycle validation moved before any DB writes so invalid payloads don't leave partial convoy state behind 6. updateConvoyProgress guards against non-convoy tracks edges (MR beads tracking their source) that have no convoy_metadata row 7. BeadPanel fallback objects now carry rig_id from convoy data so cross-rig drawer navigation works correctly Refs: #204, #540
jrf0110
force-pushed
the
540-wire-convoys-e2e
branch
from
7ba8671 to
0ae89a5
Compare
…back - Remove non-null assertions in cycle detection (use ?? fallback instead) - Remove unused BeadDependencyRecord import - Remove stale eslint-disable directive in schemas.ts - closeOrphanedReviewBeads: only close 'open' MR beads (not 'in_progress' ones that are actively being polled by pollPendingPRs) - Add fallback slug 'convoy' when title sanitizes to empty string, preventing invalid git refs with empty path segments Refs: #204, #540
jrf0110
force-pushed
the
540-wire-convoys-e2e
branch
from
5d15385 to
51c687d
Compare
pandemicsyn
approved these changes
Mar 6, 2026
Comment on lines
+413
to
+415
| * Only affects beads with a pr_url (excluded by recoverStuckReviews) that | ||
| * are stale (>30 min) and whose agent is idle/dead/missing. | ||
| */ |
Contributor
There was a problem hiding this comment.
Not review feedback, just me trying to keep up. If your setup to require a human to merge the PR, does this mean the pr has to be merged within 30 minutes too ?
Contributor
Author
There was a problem hiding this comment.
Desired behavior is that an open PR keeps the review bead open until closed/merged. If that's not happening, I will fix in a follow-up!
| INNER JOIN ${beads} ON ${bead_dependencies.depends_on_bead_id} = ${beads.bead_id} | ||
| WHERE ${bead_dependencies.bead_id} = ? | ||
| AND ${bead_dependencies.dependency_type} = 'blocks' | ||
| AND ${beads.status} NOT IN ('closed', 'failed') |
Contributor
There was a problem hiding this comment.
Same deal as above, not review feedback.
So a blocker is unresolved if status is neither closed or failed. So failed is not a blocker? Robot says yes and that this is fine:
The important distinction is:
- closed: prerequisite satisfied
- failed: prerequisite unsatisfied, but terminal
For convoy progress, it can be reasonable to count failed as terminal.
| this.sql, | ||
| /* sql */ ` | ||
| UPDATE ${convoy_metadata} | ||
| SET ${convoy_metadata.columns.closed_beads} = ${convoy_metadata.columns.total_beads}, |
Contributor
There was a problem hiding this comment.
WARNING: Force-closing bypasses outstanding review beads
closeConvoy() only iterates the convoy's tracked source beads, but merge-request beads are attached one level lower (MR -> source bead). This manual closed_beads = total_beads update can therefore mark the convoy as landed while some review beads are still open/in_progress, and the alarm will keep processing those stale review entries afterward. Either close/fail the child review beads here too, or derive the final convoy state from the same "no pending MR children" rule used in updateConvoyProgress().
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
2 participants
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.
Summary
Video Walkthrough
Implements #540 — full convoy support from Mayor tools through the DO layer, HTTP/tRPC API, and dashboard UI. This is a large cross-cutting change touching 24 files across the worker, container, and frontend.
Mayor tools (container plugin)
gt_sling_batch— creates N beads + 1 convoy as a tracked group. Uses structured Zod array args instead of JSON-in-string to eliminate first-call parsing failures.gt_list_convoys/gt_convoy_status— progress checking tools for the Mayor.MayorGastownClientgainsslingBatch,listConvoys,getConvoyStatusmethods.gt_sling_batchovergt_slingfor multi-task work.TownDO (Durable Object)
slingConvoy— atomic batch creation of convoy bead + N issue beads + bead_dependencies links + polecat assignment + fire-and-forget dispatch.listConvoys/getConvoyStatus— query active convoys with per-bead breakdown includingrig_idandassignee_agent_name.closeConvoy— force-close a convoy and all its tracked beads, unhooking any assigned agents.beadOps.updateBeadStatusso all bead-close paths (review-queue, agentCompleted, schedulePendingWork) update convoy counters and auto-land.Parallel dispatch fixes (container)
git-manager.ts— serializescloneRepo/createWorktree/removeWorktreefor the same rig to preventindex.lockcollisions while allowing different rigs to proceed concurrently.process-manager.ts— serializesensureSDKServerto preventprocess.chdir/process.envraces when concurrent agents callcreateKilo().Bead lifecycle fixes
hookBeadno longer sets bead toin_progress— beads stayopenuntildispatchAgentconfirms the container process started.closeOrphanedReviewBeads— new alarm-driven cleanup for MR beads withpr_urlwhose assigned agent is idle/dead/missing (30-min timeout).initializeDatabase()so the alarm loop survives restarts/deploys.Dashboard UI
ConvoyTimelinecomponent rewritten to consume tRPClistConvoysdata (was broken — grouped byparent_bead_idinstead ofbead_dependencies).z-indexisolation on bead chips to prevent overlap with sticky navbar.Dead code cleanup
town-convoys.table.ts,town-convoy-beads.table.ts, orphanedconvoy_idfield fromCreateBeadBody.Verification
pnpm typecheck— all packages passvitest run mayor-tools.test.ts tools.test.ts— 21 tests pass (mayor tool tests + rig tool tests)vitest run client.test.ts— 3 new MayorGastownClient tests pass (9 pre-existing failures on main confirmed unrelated)Visual Changes
Convoy progress timeline added to Town Overview (below activity graph) and Rig Detail pages. Each convoy shows a progress bar, bead chips with status colors, assignee names, and a close button with confirmation. The section header is clickable to collapse/expand.
Reviewer Notes
dist-typesregeneration (build:types) has pre-existing errors from Hono type inference issues — these are not introduced by this PR.client.test.tshas 9 pre-existing failures onmaindue to missingtownIdin the test env fixture — confirmed by checking out main and running the same tests.updateConvoyProgressinbeads.tsis called on every bead close/fail — it does abead_dependencieslookup which is indexed, so the overhead is minimal for non-convoy beads (returns immediately when no rows found).Closes #540