Skip to content

feat: namespace scope, creator signal, queue/session dedup (PRs 1–3)#145

Open
jamestexas wants to merge 13 commits intomainfrom
fix/pin-action-shas
Open

feat: namespace scope, creator signal, queue/session dedup (PRs 1–3)#145
jamestexas wants to merge 13 commits intomainfrom
fix/pin-action-shas

Conversation

@jamestexas
Copy link
Copy Markdown
Contributor

@jamestexas jamestexas commented Apr 25, 2026

Summary

Implements the approved 3-PR plan for multi-team monorepo support. Ships as one mega PR (solo dev, no stars yet).

PR 1 β€” created_by: Option<String> on Bead

  • Git username captured at bead creation via git config user.name
  • Never hard-errors if git config fails β€” stores None
  • SQLite + Dolt migrations, create_bead_full signature updated

PR 2 β€” Fix queue/session dedup bugs

  • WorkQueue: composite (repo, bead_id) keys in in_queue and backoff
  • SessionRegistry: register/unregister/touch all repo-aware
  • QueueEntry::PartialEq now includes repo check
  • New test: same bead_id in two repos can be enqueued simultaneously

PR 3 β€” scope: String on BeadRef + Bead

  • Team/folder dimension for monorepos β€” "" for all existing beads (backward compat)
  • #[serde(default)] on both structs
  • 47 BeadRef + 17 Bead construction sites updated across 29 files
  • create_bead_full gains scope: &str param
  • SQLite schema + additive migration; Dolt 004_add_scope migration + BEADS_SCHEMA
  • TODO(rosary-scope) marker for future BeadRef β†’ WorkRef rename

Also includes (prior commits):

  • ProvenanceRef polymorphic source reference on BeadSpec
  • Pipeline dispatch hardening + orphan recovery
  • Mache blast-radius + duplication check verify tiers
  • CI: all GitHub Actions pinned to full commit SHAs

Test plan

  • cargo test --bins β€” 589 unit tests passing
  • cargo test --test cli_integration β€” 17 integration tests passing (Dolt end-to-end)
  • cargo clippy -- -D warnings β€” zero warnings
  • Same bead_id in two repos enqueued simultaneously (new composite-key test)
  • rsry bead create captures created_by from git config

πŸ€– Generated with Claude Code

claude and others added 9 commits April 7, 2026 09:44
Introduces the `orchestrate` module with types and primitives for
three-tier agent hierarchy inspired by Claude Code's agent teams
architecture. No behavioral changes β€” existing flat reconciler
continues to work unchanged.

New module: src/orchestrate/
- mod.rs: FeatureOrchestrator struct, OrchestratorState machine,
  session continue-vs-spawn decision matrix, OrchestratorRecord
  for crash recovery persistence
- mailbox.rs: File-based JSONL mailbox for mid-flight agent
  communication (progress, plans, directives, shutdown protocol)
- synthesis.rs: Context synthesis between pipeline phases β€” "never
  delegate understanding" pattern from Claude Code's coordinator mode
- transcript.rs: Parse .rsry-stream.jsonl for fork-style context
  sharing (pass concrete observations, not lossy summaries)
- fanout.rs: Parallel research fan-out config and default queries
  per issue type (replaces single scoping-agent)
- plan_gate.rs: Plan-mode approval gate β€” validate scope, file
  overlap, and risk before allowing implementation

Config: OrchestrationConfig with mode (flat/hierarchical), synthesis,
fan_out, plan_gate, fork_context, max_nesting_depth for recursive
sub-pipeline composition.

https://claude.ai/code/session_01DanrDi8VyuPUu3LFXtcwkT
Connects the hierarchical orchestration types from Phase 1 into the
reconciler's main loop. When [orchestration] mode = "hierarchical",
the dispatch phase creates FeatureOrchestrators instead of raw agent
spawns. Each orchestrator drives its bead through the full pipeline
via a deterministic tick()-based state machine.

Key changes:
- ReconcilerConfig gains `orchestration: OrchestrationConfig`
- Reconciler gains `orchestrators: HashMap<String, FeatureOrchestrator>`
- New reconcile/orchestration.rs: is_hierarchical(), create_orchestrator(),
  orchestrator_tick() (drives state machines), persist/recover methods
- FeatureOrchestrator gains new(), tick() β†’ TickOutcome, on_worker_completed(),
  set_worker_handle(), current_agent(), advance_to_next_phase()
- Dispatch phase: hierarchical branch creates orchestrator, flat branch
  unchanged (zero behavioral change when mode = "flat")
- Verify phase: orchestrator-managed completions route through
  on_worker_completed() instead of flat execute_action()
- Crash recovery: .rsry-orchestrator.json persisted per iteration,
  recover_orchestrators() restores state on startup

https://claude.ai/code/session_01DanrDi8VyuPUu3LFXtcwkT
…fety, state machine bugs

- Remove unused OrchestratorBehavior/OrchestratorState imports from reconcile/mod.rs
- Swap recover_orchestrators before recover_stuck_beads to prevent clobbering
- Fix Mailbox::new β€” pass &work_dir not work_dir.join(".rsry-mailbox.jsonl")
- AwaitingWorker with no handle β†’ Idle (not NeedsSpawn) to prevent respawn loop
- Synthesizing::tick() β€” don't double-increment current_phase (already set by on_worker_completed)
- Remove unreachable pattern in session_decision match
- mark _verify_summary as intentionally unused in orchestrator verify path
- UTF-8 safe truncation in transcript.rs and synthesis.rs
- #![allow(dead_code)] on Phase 1 orchestrate submodules (not yet wired)
- pub(super) verify_and_decide so orchestration.rs can call it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces mutable version tags with pinned SHAs to prevent supply chain
attacks via tag mutation. Comments preserve the human-readable version.

- actions/checkout v5 β†’ 93cb6efe
- dtolnay/rust-toolchain stable β†’ 29eef336
- swatinem/rust-cache v2 β†’ e18b4977
- actions/cache v4 β†’ 0057852b
- pre-commit/action v3.0.1 β†’ 2c7b3805

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace flat `source_adr: String` on BeadSpec with a tagged enum:

    pub enum ProvenanceRef { Adr { id }, SlackThread { url, summary },
                             Meeting { title, date }, Manual { note } }

This is a pointer (foreign key) to the origin artifact, not content.
Existing callers updated: `source_adr: "ADR-001"` β†’ `source: ProvenanceRef::Adr { id: "ADR-001" }`.
Backward compat: only affects in-memory BDR decomposition, not stored beads.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion time

Add `created_by: Option<String>` to the Bead model. Populated via
`git config user.name` at creation time β€” best-effort snapshot, never
hard-errors if git config is absent.

- SQLite: additive migration (ALTER TABLE IF NOT EXISTS pattern)
- Dolt: migration 003_add_created_by via versioned migrations table
- Both BEADS_SCHEMA and test schemas updated for new databases
- Not included in generation() hash β€” metadata, not semantic content
- `#[serde(default)]` for full backward compat with existing beads

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…x cross-repo collision

WorkQueue and SessionRegistry were keyed on bead_id alone. Two beads
with the same ID in different repos (e.g. mono-auth-abc and mono-payments-abc)
would silently stomp each other in the queue and session registry.

WorkQueue:
- in_queue: HashSet<String> β†’ HashSet<(String, String)>
- backoff: HashMap<String, BackoffState> β†’ HashMap<(String, String), BackoffState>
- QueueEntry::PartialEq now includes repo
- All methods gain repo: &str parameter

SessionRegistry:
- register/unregister/touch now use (bead_id, repo) composite matching

Regression test: same bead_id in two repos both enqueue successfully.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…auto-threading

Dispatch:
- Workspace cleanup on spawn failure (no orphaned worktrees on error)
- Pipeline-first lifecycle: check bead state after agent exit, defer to
  pipeline if agent already transitioned via MCP (prevents stuck-Dispatched)

Orchestrator / persistence:
- Recover orphaned dispatch records on startup (crashed reconciler left
  completed_at=NULL; now marked abandoned and excluded from active_dispatches)
- Worker-completion feedback loop: notify orchestrator on worker exit so
  it can retry or deadletter rather than stalling in AwaitingWorker

Threading:
- Auto-decade creation for cluster-discovered threads (FK constraint)
- Feature branch wiring at thread creation time (lazy fallback on failure)

Verify:
- Agent-close fast path: skip compile/test/lint pipeline when agent already
  closed the bead via rsry_bead_close (avoids rejected-state-transition noise)
- agent_closed counter in VerifyResult

Store SQLite:
- Regression test: abandoned dispatch records excluded from active_dispatches()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tiers

Two new advisory (Partial) verify tiers that call the mache HTTP MCP
server at localhost:7532:

- MacheBlastRadiusCheck: flags high blast-radius changes (many callers
  affected) to prompt human review before merge
- MacheDuplicationCheck: detects copy-paste duplication introduced by
  the agent across the diff

Both tiers fail-open: if mache is not running or times out (10s), the
tier is skipped β€” they never block the pipeline. Uses reqwest::blocking
for synchronous one-shot calls against the Streamable HTTP transport
(handles both plain JSON and SSE response bodies).

Cargo.toml: add reqwest "blocking" feature

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 25, 2026 23:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens bead lifecycle handling for multi-repo/monorepo use by adding richer provenance/creator metadata, fixing cross-repo dedup collisions, and improving pipeline reliability (crash recovery, workspace cleanup, verify tier additions, and new hierarchical orchestration scaffolding).

Changes:

  • Introduce ProvenanceRef (polymorphic source reference) for BDR bead specs and update decomposition logic/tests.
  • Add created_by capture at bead creation, with additive migrations for SQLite/Dolt and plumbing through store APIs.
  • Fix dedup/backoff/session tracking collisions by switching keying from bead_id to (repo, bead_id) and add pipeline hardening + new mache advisory verify tiers.

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/xref.rs Update test bead construction for new created_by field.
src/workspace/tests.rs Add regression test ensuring Workspace::cleanup() is safe for VcsKind::None.
src/workspace/mod.rs Export ensure_thread_branch; add Workspace::cleanup(self) API.
src/verify.rs Add mache blast-radius/duplication advisory tiers; update tier counts in tests.
src/testutil.rs Update bead factory helper with created_by.
src/store_sqlite.rs Add regression test for orphan/abandoned dispatch recovery behavior.
src/store.rs Extend BeadStore::create_bead_full signature with created_by.
src/session.rs Key session registry operations by (repo, bead_id) to avoid collisions.
src/serve/handlers.rs Capture created_by from git config; update session registry calls to include repo.
src/scanner.rs Update test bead construction for created_by.
src/reconcile/verify.rs Add agent-closed fast path; feed orchestrator completion; improved deadletter-on-spawn-failure behavior.
src/reconcile/triage.rs Use composite (repo, bead_id) queue APIs.
src/reconcile/threading.rs Auto-thread decade/thread setup; ensure feature branch; add feature PR assembly phase.
src/reconcile/tests.rs Update tests for new queue signatures and created_by field.
src/reconcile/persistence.rs Startup recovery now abandons orphan dispatch records; remove unnecessary dead_code allow.
src/reconcile/orchestration.rs New hierarchical orchestration integration (tick loop, crash recovery, dispatch/verify bridging).
src/reconcile/mod.rs Wire orchestration config; add orchestrator map; invoke orchestrator tick + feature assembly.
src/reconcile/completion.rs Update backoff handling to include repo; adjust retry logging.
src/queue.rs Switch queue dedup/backoff keys to (repo, bead_id); add regression test for cross-repo enqueue.
src/pipeline.rs Add active_dispatches() helper for crash recovery.
src/orchestrate/transcript.rs New transcript parser for fork-style context sharing.
src/orchestrate/synthesis.rs New synthesized prompt builder between phases.
src/orchestrate/plan_gate.rs New plan validation/generation utilities.
src/orchestrate/mod.rs New FeatureOrchestrator state machine + persistence types.
src/orchestrate/mailbox.rs New file-based mailbox for mid-flight orchestrator/worker messaging.
src/orchestrate/fanout.rs New research fan-out config + default queries.
src/main.rs Add git_config_user_name() helper; pass created_by into bead creation; include orchestrate module.
src/import.rs Pass created_by=None on imported beads.
src/epic.rs Update test bead construction for created_by.
src/dolt/tests.rs Update Dolt schema test table to include user_id and created_by.
src/dolt/query.rs Update bead mapping structs for created_by field (currently unset).
src/dolt/mod.rs Update Dolt schema to include user_id and created_by.
src/dolt/migrate.rs Add Dolt migration 003_add_created_by.
src/dolt/bead_crud.rs Write created_by on bead creation; update bead mappings (currently unset).
src/dispatch/tests.rs Add regression test ensuring workspace cleanup on spawn failure; update test beads for created_by.
src/dispatch/mod.rs Clean up workspace on spawn failure; pipeline-first lifecycle transitions in dispatch::run.
src/config.rs Add OrchestrationConfig and plumb into global config.
src/bead_sqlite.rs Add SQLite created_by column + additive migration; plumb into create/list/get; add test coverage.
src/bead_dolt.rs Plumb created_by into Dolt BeadStore adapter.
src/bead.rs Add created_by to Bead model and parsing defaults.
crates/bdr/src/provenance.rs New ProvenanceRef enum with serde + helpers + tests.
crates/bdr/src/lib.rs Export provenance module.
crates/bdr/src/decompose.rs Replace source_adr with ProvenanceRef in BeadSpec and update tests.
Cargo.toml Enable reqwest blocking feature for mache tiers.
.github/workflows/ci.yml Pin GitHub Actions to commit SHAs for supply-chain hardening.

πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/queue.rs
Comment on lines 31 to 35
// Ties broken by earlier enqueue time (older first = fairness).
impl PartialEq for QueueEntry {
fn eq(&self, other: &Self) -> bool {
self.bead_id == other.bead_id
self.bead_id == other.bead_id && self.repo == other.repo
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

QueueEntry’s equality is now (bead_id, repo), but Ord::cmp still only compares (score, enqueued_at). That breaks the Ord contract (two entries can compare Equal while eq is false), which can lead to incorrect behavior in BinaryHeap when scores/timestamps tie. Update cmp to include a deterministic tie-breaker (e.g., repo + bead_id) so it’s consistent with Eq.

Copilot uses AI. Check for mistakes.
Comment thread src/reconcile/mod.rs
Comment on lines +649 to +662
// ── Hierarchical mode: create orchestrator instead of raw dispatch ──
if self.is_hierarchical() {
self.create_orchestrator(
&entry.bead_id,
&entry.repo,
&bead.issue_type,
path.clone(),
);
self.persist_status(&entry.bead_id, &entry.repo, "dispatched")
.await;
summary.dispatched += 1;
// Orchestrator will request its first agent spawn on next tick.
continue;
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

In hierarchical mode, dispatching only creates an orchestrator and marks the bead dispatched, but it does not initialize a BeadTracker / repo mapping for the bead. Later, completion/verification code paths rely on trackers[bead_id].repo to associate completed workdirs with a repo/language; without that, verification will fall back to unknown language and skip compile/test/lint. Ensure orchestrator-managed beads still register tracker/repo metadata (or otherwise plumb repo through completion/verification).

Copilot uses AI. Check for mistakes.
Comment on lines +62 to +68
eprintln!(
"[orchestrator] created for {} (pipeline={:?})",
bead_id, orchestrator.pipeline
);

self.orchestrators.insert(bead_id.to_string(), orchestrator);
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

create_orchestrator inserts into self.orchestrators keyed by bead_id only. If two repos contain beads with the same ID, one orchestrator will overwrite the other, breaking hierarchical mode and reintroducing the cross-repo collision this PR is trying to fix. Use a composite key (e.g., BeadRef or (repo, bead_id)) consistently throughout this module.

Copilot uses AI. Check for mistakes.
chain_hash: handle.chain_hash.clone(),
};
self.pipeline.record_dispatch(&dispatch_record).await;

Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

After a successful spawn, the handle is stored in self.active, but the orchestrator never receives it (FeatureOrchestrator::set_worker_handle is never called). As a result, the orchestrator can’t distinguish β€œwaiting for a spawn” vs β€œwaiting for completion”, and subsequent phases can get stuck returning Idle (no further NeedsSpawn). Either pass the handle into the orchestrator (and/or store an explicit state flag) so tick() can correctly drive the lifecycle across phases.

Suggested change
// Keep the orchestrator's lifecycle state in sync with the
// successfully spawned worker so future ticks wait for
// completion instead of treating this phase as idle.
if let Some(orch) = self.orchestrators.get_mut(&bead_id) {
orch.set_worker_handle(handle.clone());
}

Copilot uses AI. Check for mistakes.
Comment thread src/reconcile/verify.rs
Comment on lines +325 to +343
// Skip orchestrator-managed beads β€” their completions are handled
// by orchestrator_tick() which calls on_worker_completed().
if self.orchestrators.contains_key(bead_id.as_str()) {
// Feed the completion back to the orchestrator.
let (action, _verify_summary) =
self.verify_and_decide(bead_id, *exit_success, beads);
let passed = matches!(
action,
CompletionAction::Advance { .. } | CompletionAction::Terminal
);
if let Some(orch) = self.orchestrators.get_mut(bead_id.as_str()) {
orch.on_worker_completed(passed, self.config.max_retries);
}
if passed {
result.passed += 1;
} else {
result.failed += 1;
}
continue;
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

This orchestrator-managed completion path calls verify_and_decide() and feeds a pass/fail boolean back to the orchestrator, but it never executes the resulting CompletionAction (no handoff write, no status update, no workspace checkpoint/cleanup, and no pipeline.complete_dispatch() for the dispatch record). That will leave pipeline/dispatch state inconsistent and likely leak completed_at = NULL dispatch records. Consider reusing execute_action() here (or an orchestrator-specific equivalent) so the same persistence + cleanup invariants apply in hierarchical mode.

Copilot uses AI. Check for mistakes.
Comment thread src/dolt/bead_crud.rs
Comment on lines 56 to 62
pr_url: None,
jj_change_id: None,
external_ref: row.try_get("external_ref").ok(),
files,
test_files,
created_by: None,
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

created_by is persisted in the schema, but Dolt reads always set created_by: None and the SELECTs here don’t include created_by. As a result, the field will never round-trip for Dolt backends (it will look unset everywhere except at write time). Update the relevant SELECT columns and row mapping to populate Bead.created_by (and do the same in the other Dolt bead queries in this file).

Copilot uses AI. Check for mistakes.
Comment thread src/dolt/query.rs
external_ref: row.try_get("external_ref").ok(),
files,
test_files,
created_by: None,
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

created_by is being added to the Dolt schema, but this query mapping always sets created_by: None and the SQL SELECT doesn’t fetch it. This makes created_by effectively write-only for Dolt. Include created_by in the SELECT and map it into Bead.created_by so clients can actually observe it.

Suggested change
created_by: None,
created_by: row.try_get("created_by").ok(),

Copilot uses AI. Check for mistakes.
Comment thread src/dispatch/mod.rs
Comment on lines +487 to +494
let has_commits = if let Some(ref ws_path) = handle.workspace_path {
tokio::process::Command::new("git")
.args(["log", "--oneline", "HEAD", "--not", "origin/HEAD", "--"])
.current_dir(ws_path)
.output()
.await
.map(|o| o.status.success() && !o.stdout.is_empty())
.unwrap_or(false)
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

has_commits is computed via git log ... --not origin/HEAD. Many repos don’t have origin/HEAD configured (or it may not point at the intended default branch), which will make this command fail and incorrectly treat the worktree as having no commits (leading to an unintended closed transition). Prefer comparing against a concrete base ref like origin/main / configured default branch, and treat a missing remote ref as an indeterminate state rather than no commits.

Copilot uses AI. Check for mistakes.
Comment thread src/reconcile/mod.rs
Comment on lines +155 to +158
/// Feature orchestrators keyed by bead_id (hierarchical mode only).
/// Each orchestrator owns one bead's full lifecycle: research β†’ synthesis β†’
/// implementation β†’ verification, with mid-flight messaging and session reuse.
orchestrators: HashMap<String, FeatureOrchestrator>,
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

orchestrators is keyed by bead_id only, which reintroduces the cross-repo collision problem this PR is addressing (same bead ID in different repos would overwrite the orchestrator). Key this map by a composite identifier (e.g., BeadRef or (repo, bead_id)) and update lookups accordingly.

Copilot uses AI. Check for mistakes.
Comment thread src/orchestrate/mod.rs
Comment on lines +331 to +335
} else {
// Handle is owned by the reconciler (self.active) β€” just wait.
// The reconciler calls on_worker_completed() when the handle exits.
TickOutcome::Idle
}
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

tick() returns Idle whenever state is AwaitingWorker and worker_handle is None (comment says the reconciler owns the handle). But on_worker_completed() transitions into Synthesizing/AwaitingWorker for the next phase without setting a handle, so the orchestrator will also return Idle when it actually needs the reconciler to spawn the next agent. Consider modeling a separate β€œNeedsSpawn”/β€œAwaitingSpawn” state, or have AwaitingWorker with worker_handle: None return NeedsSpawn for the current agent/phase.

Copilot uses AI. Check for mistakes.
…epo multi-team contexts

Adds `scope: String` to both `BeadRef` and `Bead` as a team/folder dimension
for monorepo contexts where `mono-auth-abc123` and `mono-payments-abc123` need
to coexist. All construction sites default to `""` (backward compatible via
`#[serde(default)]`).

Changes:
- BeadRef.scope: `#[serde(default)]` field with TODO(rosary-scope) rename marker
- Bead.scope: read from DB in SQLite (LIST_BEADS_SQL, get_bead, list_beads_scoped)
- BeadStore::create_bead_full: new `scope: &str` param (all callers pass `""`)
- SQLite: `scope TEXT NOT NULL DEFAULT ''` in schema + additive migration
- Dolt: `004_add_scope` migration + updated BEADS_SCHEMA for new DBs
- Dolt INSERT: scope column bound in create_bead_full
- 47 BeadRef + 17 Bead construction sites updated across 29 files
- All 606 tests passing, zero clippy warnings

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jamestexas jamestexas changed the title bead model hardening: ProvenanceRef, created_by, cross-repo dedup, pipeline fixes feat: namespace scope, creator signal, queue/session dedup (PRs 1–3) Apr 25, 2026
…t_mache_text, and scope/created_by round-trip

- verify.rs: 11 tests for parse_mache_sse (SSE parsing) and extract_mache_text (MCP content extraction)
- bead_sqlite.rs: 3 tests for created_by + scope round-trip via get_bead and list_beads

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 27, 2026 01:06
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 50 out of 50 changed files in this pull request and generated 11 comments.


πŸ’‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/dolt/query.rs
Comment on lines +102 to +103
created_by: None,
scope: String::new(),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

Dolt bead reads are discarding the new created_by and scope fields (always None/empty). This makes the new columns effectively write-only and breaks round-tripping for Dolt-backed repos. Update the SELECTs to include created_by/scope and populate these fields from the row (with sensible defaults for pre-migration DBs).

Suggested change
created_by: None,
scope: String::new(),
created_by: row.try_get("created_by").ok(),
scope: row.try_get("scope").unwrap_or_default(),

Copilot uses AI. Check for mistakes.
Comment thread src/dolt/bead_crud.rs
Comment on lines 55 to 63
branch: None,
pr_url: None,
jj_change_id: None,
external_ref: row.try_get("external_ref").ok(),
files,
test_files,
created_by: None,
scope: String::new(),
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

list_beads is constructing Bead with created_by: None and scope: "", so Dolt-backed beads will never surface creator/scope metadata even after the migrations. Include created_by/scope in the query and read them from the row (using try_get + defaults for older schemas).

Copilot uses AI. Check for mistakes.
Comment thread src/dolt/tests.rs
assignee VARCHAR(128),
external_ref VARCHAR(128),
user_id VARCHAR(128),
created_by VARCHAR(255),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

The Dolt sandbox issues schema used in tests adds created_by but does not add the new non-null scope column (which is now part of BEADS_SCHEMA and create_bead_full). This will diverge from production schema and can cause test DB setup / inserts to fail once scope is exercised. Add scope VARCHAR(255) NOT NULL DEFAULT '' here as well.

Suggested change
created_by VARCHAR(255),
created_by VARCHAR(255),
scope VARCHAR(255) NOT NULL DEFAULT '',

Copilot uses AI. Check for mistakes.
Comment thread src/orchestrate/mod.rs
Comment on lines +278 to +287
// First tick β€” request the first pipeline agent.
if let Some(agent) = self.pipeline.first().cloned() {
let decision = self.session_decision(&agent);
self.state = OrchestratorState::AwaitingWorker {
agent: agent.clone(),
phase: 0,
};
TickOutcome::NeedsSpawn {
agent,
phase: 0,
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

FeatureOrchestrator::tick() always pulls self.pipeline.first() and hardcodes phase: 0, so the orchestrator can only ever request the first worker. After phase advancement, it will never emit another NeedsSpawn, stalling hierarchical mode. Use self.current_phase when selecting the next agent/phase and consider a separate state (e.g. ReadyToSpawn) so tick() can request spawns for later phases as well.

Suggested change
// First tick β€” request the first pipeline agent.
if let Some(agent) = self.pipeline.first().cloned() {
let decision = self.session_decision(&agent);
self.state = OrchestratorState::AwaitingWorker {
agent: agent.clone(),
phase: 0,
};
TickOutcome::NeedsSpawn {
agent,
phase: 0,
// Request the pipeline agent for the current phase.
let phase = self.current_phase;
if let Some(agent) = self.pipeline.get(phase).cloned() {
let decision = self.session_decision(&agent);
self.state = OrchestratorState::AwaitingWorker {
agent: agent.clone(),
phase,
};
TickOutcome::NeedsSpawn {
agent,
phase,

Copilot uses AI. Check for mistakes.
Comment thread src/orchestrate/mod.rs
Comment on lines +384 to +396
let next_agent = self.pipeline[next_phase].clone();
if self.config.synthesis {
self.state = OrchestratorState::Synthesizing;
} else {
let decision = self.session_decision(&next_agent);
self.state = OrchestratorState::AwaitingWorker {
agent: next_agent,
phase: next_phase as u32,
};
// Decision is used on the next tick().
let _ = decision;
}
} else {
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

After a successful worker, on_worker_completed() transitions to Synthesizing or AwaitingWorker for the next agent, but tick() returns Idle when worker_handle is None. Since reconciler-managed handles are not set on the orchestrator, this prevents the next phase from ever spawning. Consider transitioning to a spawn-requesting state (or making tick() emit NeedsSpawn when awaiting a worker but no handle exists yet).

Suggested change
let next_agent = self.pipeline[next_phase].clone();
if self.config.synthesis {
self.state = OrchestratorState::Synthesizing;
} else {
let decision = self.session_decision(&next_agent);
self.state = OrchestratorState::AwaitingWorker {
agent: next_agent,
phase: next_phase as u32,
};
// Decision is used on the next tick().
let _ = decision;
}
} else {
self.worker_handle = None;
let next_agent = self.pipeline[next_phase].clone();
if self.config.synthesis {
self.state = OrchestratorState::Synthesizing;
} else {
let decision = self.session_decision(&next_agent);
self.state = OrchestratorState::NeedsSpawn {
agent: next_agent,
phase: next_phase as u32,
decision,
};
}
} else {
self.worker_handle = None;

Copilot uses AI. Check for mistakes.
Comment on lines +318 to +331
for (repo_name, (repo_path, _lang)) in &self.repo_info {
// Scan for workspace directories containing orchestrator records.
// Workspaces are typically at <repo>/.rsry-ws-<bead_id>/ or similar.
let entries: Vec<PathBuf> = match std::fs::read_dir(repo_path) {
Ok(dir) => dir
.filter_map(|e| e.ok())
.filter(|e| {
e.file_name()
.to_str()
.is_some_and(|n| n.starts_with(".rsry-ws-"))
})
.map(|e| e.path().join(".rsry-orchestrator.json"))
.filter(|p| p.exists())
.collect(),
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

recover_orchestrators() is scanning the repo root for directories named .rsry-ws-*, but workspaces are currently created under ~/.rsry/worktrees/{repo}/{id} (see workspace_dir). As written, crash recovery will not find persisted orchestrator records. Reuse the same workspace path derivation as the workspace module (or persist the exact workspace path and scan that location).

Copilot uses AI. Check for mistakes.
Comment thread src/queue.rs
Comment on lines +154 to 158
pub fn is_deadlettered(&self, repo: &str, bead_id: &str) -> bool {
self.backoff
.get(&(repo.to_string(), bead_id.to_string()))
.is_some_and(|s| s.exceeded_max())
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

These lookups allocate new Strings on every call (repo.to_string(), bead_id.to_string()), which can become a hot-path regression during triage/backoff checks. Consider introducing a key newtype with Borrow support (or storing an interned/Arc key) so HashMap::get/HashSet::contains can be done with (&str, &str) without allocations.

Copilot uses AI. Check for mistakes.
Comment thread src/dolt/bead_crud.rs
Comment on lines 181 to 189
external_ref: row.try_get("external_ref").ok(),
branch: None,
pr_url: None,
jj_change_id: None,
files,
test_files,
created_by: None,
scope: String::new(),
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

get_bead similarly hardcodes created_by: None and empty scope, which prevents the new fields from round-tripping for Dolt. Update the SELECT to fetch these columns and populate them on the returned Bead.

Copilot uses AI. Check for mistakes.
Comment thread src/reconcile/mod.rs
Comment on lines +464 to 476
let target_repo = self
.trackers
.get(target.as_str())
.map(|t| t.repo.clone())
.unwrap_or_default();
let still_active = summary.dispatched > 0
|| self.queue.has_backoff(target)
|| self.queue.has_backoff(&target_repo, target)
|| !self.active.is_empty();

if still_active {
let elapsed = start.elapsed();
let reason = if self.queue.has_backoff(target) {
let reason = if self.queue.has_backoff(&target_repo, target) {
"waiting for backoff"
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

Target-bead mode derives target_repo from self.trackers and falls back to "". With repo-aware backoff keys, this can incorrectly treat a bead as having no pending backoff (and exit early) if the tracker entry is absent or removed. Consider storing the repo alongside target_bead (or adding a has_backoff_any_repo(bead_id) helper) so target mode can reliably detect pending retries.

Copilot uses AI. Check for mistakes.
Comment thread src/verify.rs
Comment on lines +417 to +421
fn mache_call(tool: &str, args: serde_json::Value) -> Option<String> {
let client = reqwest::blocking::Client::builder()
.timeout(std::time::Duration::from_secs(10))
.build()
.ok()?;
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

mache_call() builds a new reqwest::blocking::Client for every tool invocation. Since the mache tiers can call this repeatedly (per symbol), this adds avoidable overhead and can amplify timeouts. Consider reusing a single client (lazy static / once_cell) and/or adding a small connect timeout and a cap on the number of symbols queried per run.

Copilot uses AI. Check for mistakes.
jamestexas and others added 2 commits April 26, 2026 19:26
Pure mechanical rename across 19 files. No behavior change.
WorkRef better reflects that it references a unit of work,
not just a bead ID β€” important as the scope field (monorepo
multi-team contexts) makes the reference more than just (repo, id).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The rename is done β€” no further rename needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

3 participants