Skip to content

feat(gastown): add bead dependency editing#1231

Merged
jrf0110 merged 12 commits intomainfrom
gt/toast/121c908a
Mar 18, 2026
Merged

feat(gastown): add bead dependency editing#1231
jrf0110 merged 12 commits intomainfrom
gt/toast/121c908a

Conversation

@jrf0110
Copy link
Copy Markdown
Contributor

@jrf0110 jrf0110 commented Mar 18, 2026

Summary

Implements bead dependency management (issue #1100) across the full stack:

  • Core functions (beads.ts): addBeadDependency() with self-reference validation and DFS cycle detection for 'blocks' edges (ON CONFLICT DO NOTHING for idempotent adds); removeBeadDependency() that protects 'tracks' edges from deletion.
  • TownDO methods: addBeadDependency() and removeBeadDependency() with initialization, bead event logging, and alarm arming to dispatch newly unblocked beads.
  • HTTP endpoints (bead-dependencies.handler.ts): POST and DELETE routes under the existing rig auth middleware, with rigId ownership validation matching the pattern in handleDeleteBead.
  • Mayor tool client: addBeadDependency() and removeBeadDependency() on MayorGastownClient.
  • Mayor tools: gt_bead_add_dependency and gt_bead_remove_dependency, plus optional depends_on array on gt_sling.
  • Mayor system prompt: Documented new tools and updated gt_sling description.

Two bugs fixed during review:

  1. removeBeadDependency alarm logic: was calling getNewlyUnblockedBeads(dependsOnBeadId) (wrong — that function is for the bead-close path). Fixed to call hasUnresolvedBlockers(beadId) directly, since beadId is the bead whose blocking situation just changed.
  2. Missing rigId validation in both HTTP handlers: added getBeadAsync + bead.rig_id !== params.rigId check, consistent with handleDeleteBead.

Closes #1100

Verification

  • pnpm typecheck — passed
  • pnpm prettier --check — passed
  • pnpm eslint on changed files — passed

Visual Changes

N/A

Reviewer Notes

The cycle detection DFS is correct: the adjacency map encodes bead_id → depends_on_bead_id (A depends on B = edge A→B). Starting from dependsOnBeadId and traversing its own depends_on edges — if we reach beadId, the proposed new edge would close a cycle. Self-reference is caught separately before the DFS.

The gt_bead_add_dependency tool intentionally exposes only 'blocks' and 'parent-child' dependency types (not 'tracks'), since tracks edges are system-managed by the convoy machinery.

jrf0110 added 8 commits March 18, 2026 18:06
…tions

Add two new exported functions to beads.ts for managing bead dependency
edges after creation. Includes self-reference validation, cycle detection
via DFS for 'blocks' dependencies, and protection against removing
system-managed 'tracks' edges.

Also adds 'dependency_added' and 'dependency_removed' to the BeadEventType
enum for audit trail support.

Closes #1100 (partial — layer 1/7)
…thods

Wire the core dependency functions into TownDO with proper initialization,
event logging, and alarm arming for unblocked bead dispatch.

Closes #1100 (partial — layer 2/7)
Add bead-dependencies.handler.ts with POST and DELETE handlers for
adding and removing bead dependencies. Register routes in gastown.worker.ts
under user auth middleware.

POST /api/towns/:townId/rigs/:rigId/beads/:beadId/dependencies
DELETE /api/towns/:townId/rigs/:rigId/beads/:beadId/dependencies/:dependsOnBeadId

Closes #1100 (partial — layer 3/7)
…ependency

Wire the mayor tool client to the new bead dependency HTTP endpoints.

Closes #1100 (partial — layer 4/7)
…tools and gt_sling depends_on

Add two new mayor tools for managing bead dependencies after creation:
- gt_bead_add_dependency: add a blocks/parent-child dependency edge
- gt_bead_remove_dependency: remove a dependency edge (auto-dispatches if unblocked)

Also add optional depends_on parameter to gt_sling for declaring
dependencies at sling time.

Closes #1100 (partial — layers 5-6/7)
Add gt_bead_add_dependency and gt_bead_remove_dependency to the Surgical
Editing section of the mayor's system prompt. Update gt_sling description
to mention the new optional depends_on parameter.

Closes #1100 (partial — layer 7/7)
- Use literal union type instead of z.infer<typeof DependencyType> to
  satisfy consistent-type-imports rule
- Replace non-null assertion with explicit undefined check in DFS loop
- Fix prettier formatting in gastown.worker.ts
…ency handlers

- removeBeadDependency now correctly checks if beadId itself is unblocked
  (using hasUnresolvedBlockers) rather than calling getNewlyUnblockedBeads
  with dependsOnBeadId, which was designed for the bead-close path
- handleAddBeadDependency and handleRemoveBeadDependency now validate that
  beadId belongs to the rigId in the URL path, matching handleDeleteBead
@jrf0110 jrf0110 enabled auto-merge (squash) March 18, 2026 19:06
Comment thread cloudflare-gastown/container/plugin/mayor-tools.ts Outdated
Comment thread cloudflare-gastown/src/handlers/bead-dependencies.handler.ts Outdated
Comment thread cloudflare-gastown/src/handlers/bead-dependencies.handler.ts
Comment thread cloudflare-gastown/src/handlers/bead-dependencies.handler.ts
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Mar 18, 2026

Code Review Summary

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
cloudflare-gastown/src/dos/town/beads.ts 1107 removeBeadFromConvoy() updates the counters but never re-runs the landing logic, so removing the last incomplete bead can leave a convoy stuck open forever.

Fix these issues in Kilo Cloud

Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

WARNING

File Line Issue
cloudflare-gastown/src/dos/Town.do.ts 874 removeBeadDependency() only re-arms the alarm, so recently hooked beads can stay stuck behind the 2-minute dispatch cooldown after becoming unblocked.
Files Reviewed (2 files)
  • cloudflare-gastown/src/dos/Town.do.ts
  • cloudflare-gastown/src/dos/town/beads.ts - 1 issue

Reviewed by gpt-5.4-20260305 · 673,080 tokens

- Fix Comment 1: Add dependsOn parameter to slingBead() so dependency rows
  are inserted atomically before dispatch alarm is armed, preventing a new
  bead from being dispatched before its blockers are registered.

- Fix Comment 2: Restrict AddDependencyBody to EditableDependencyType
  (blocks, parent-child) so callers cannot create system-managed 'tracks'
  edges via the public API.

- Fix Comment 3: Validate that depends_on_bead_id belongs to the same rig
  as the primary bead in handleAddBeadDependency, preventing cross-rig
  dependency injection.

- Fix Comment 4: Same cross-rig validation for dependsOnBeadId in
  handleRemoveBeadDependency.
Comment thread cloudflare-gastown/src/dos/Town.do.ts Outdated
jrf0110 and others added 2 commits March 18, 2026 19:48
…heck

Mirror the slingConvoy() pattern: only call dispatchAgent() when the new
bead has no unresolved blockers. Previously, slingBead() with depends_on
would insert the dependency rows atomically but then immediately dispatch
the agent unconditionally, allowing blocked beads to start work before
their prerequisites close.
Add convoy_id support to gt_sling and gt_bead_update mayor tools so the
mayor can sling new beads directly into a convoy and move existing beads
in/out of convoys. Introduces addBeadToConvoy/removeBeadFromConvoy
helpers that manage the tracks dependency, metadata, and convoy counters
atomically.
Comment thread cloudflare-gastown/src/dos/Town.do.ts
Comment thread cloudflare-gastown/src/dos/town/beads.ts
Comment thread cloudflare-gastown/src/dos/town/beads.ts Outdated
- Validate convoy_id before creating the bead in slingBead() to prevent
  orphan rows when the convoy doesn't exist or isn't type=convoy.
- addBeadToConvoy now recounts closed_beads (via shared
  recountConvoyClosedBeads helper) so adding an already-closed bead is
  reflected correctly, and clears the ready_to_land flag when adding an
  open bead to a convoy that was already marked complete.
- removeBeadFromConvoy uses recountConvoyClosedBeads instead of a naive
  decrement, matching updateConvoyProgress's MR-aware counting logic.
`,
[convoyId]
);
recountConvoyClosedBeads(sql, convoyId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WARNING: Removing the last incomplete bead can leave the convoy stuck open

recountConvoyClosedBeads() fixes the counters, but this path never re-runs the landing logic. If a convoy is 1/2 complete and the remaining open bead is removed, the counts become 1/1 here, yet ready_to_land is never set (and review-and-merge convoys never auto-close), so the convoy stays open forever.

@jrf0110 jrf0110 merged commit 5db843c into main Mar 18, 2026
17 of 18 checks passed
@jrf0110 jrf0110 deleted the gt/toast/121c908a branch March 18, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Mayor cannot edit bead dependencies — missing at every layer

2 participants