Skip to content

task-forge: enrich 33 underspecified beads tasks - #414

Open
ahrav wants to merge 7 commits into
mainfrom
task-forge/enrichment-sweep-2026-04-10
Open

task-forge: enrich 33 underspecified beads tasks#414
ahrav wants to merge 7 commits into
mainfrom
task-forge/enrichment-sweep-2026-04-10

Conversation

@ahrav

@ahrav ahrav commented Apr 10, 2026

Copy link
Copy Markdown
Owner

Summary

  • Enriched 33 underspecified beads tasks with implementation-ready descriptions via 34-agent fleet run
  • Each agent independently explored the codebase and produced descriptions with all 8 mandatory sections
  • Added run-task-forge-fleet.sh and runbooks/task-forge-enrich.md for repeatable enrichment sweeps

Enrichment Stats

Metric Value
Tasks enriched 33 of 34 (1 had null byte in output)
Total content added ~311K chars
Average per task ~9.2K chars
Agent codex / gpt-5.4
Image ahrav/gossip-rs-rust:latest
Fleet duration ~7 minutes

Sections Added Per Task

Every enriched task now has: Context, Current State, Desired State, Implementation Guidance (Files to Modify, Patterns to Follow, Utilities to Reuse, Blast Radius), Code References (with file:line citations), Related Work, Acceptance Criteria, Pointers.

New Infrastructure

  • run-task-forge-fleet.sh — Fleet orchestrator: identifies underspecified tasks, fans out 1 agent per task, polls Jetty, collects enrichment JSON from agent branches, applies via bd update, updates state ledger
  • runbooks/task-forge-enrich.md — Fully autonomous agent runbook: no human gates, Python-based manifest handling (avoids shell quoting issues with single quotes in descriptions), incremental JSON writes

Test plan

  • Dry run identifies correct underspecified tasks
  • 34 agents launched and completed (33 pushed branches)
  • Enrichments collected and applied via bd update
  • bd show <id> confirms enriched descriptions persisted
  • State ledger tracks enriched tasks for skip-on-rerun

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added fleet orchestrator for automated guide-sync workflows across documentation chapters.
    • Added review pattern enforcement system for detecting code quality issues.
    • Added design-doc audit workflow for documentation maintenance.
    • Added doc-rigor assessment workflow for Rust source file quality checks.
    • Added task-forge enrichment system for improving task specifications.
  • Documentation

    • Added architecture and research documentation for fleet orchestration system.
    • Added Jetty platform feature proposals and capabilities research.
  • Chores

    • Added new fleet-orchestrator crate with orchestration modules.
    • Added configuration files and skill definitions for review and audit workflows.
    • Added fleet state tracking and dependency graph systems.

@coderabbitai

coderabbitai Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

💤 Files selected but had no reviewable changes (1)
  • .beads/issues.jsonl
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 17052fb3-41bd-4a0e-82da-f2105c2877ee

📥 Commits

Reviewing files that changed from the base of the PR and between 85dabc9 and 7d37571.

📒 Files selected for processing (1)
  • .beads/issues.jsonl

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request introduces a comprehensive fleet orchestration system for managing multi-agent document synchronization and code audits. It adds a Rust crate (fleet-orchestrator) with modules for dependency graph management, affected-chapter computation, audit workflows, state persistence, Jetty API integration, git merging, and PR creation. The system also includes review-pattern enforcement via Bash hooks and rule management, three orchestration skill definitions, and wrapper scripts for guide-sync, design-doc audit, doc-rigor, and task-forge workflows.

Changes

Cohort / File(s) Summary
Review Pattern System
.claude/hooks/review-pattern-check.sh, .claude/review-rules.yaml, .claude/scripts/detect-patterns.sh, .claude/scripts/review-rule-lifecycle.sh, .claude/settings.json
New YAML-based review rule engine with stop-hook validation, pattern detection, and lifecycle analysis via Bash scripts. Enforces codified patterns against repository changes with configurable severity and detection hints.
Claude Skill Definitions
.claude/skills/detect-patterns/SKILL.md, .claude/skills/execute-review-findings/SKILL.md, .claude/skills/manage-rules/SKILL.md, .claude/skills/review-dispatch/SKILL.md, .claude/skills/review-pipeline/SKILL.md
New and updated skill workflows for analyzing findings, detecting patterns, managing rule lifecycle, and injecting active rules into review dispatch and pipeline agents.
Fleet Orchestrator Core Crate
crates/fleet-orchestrator/Cargo.toml, crates/fleet-orchestrator/src/lib.rs, crates/fleet-orchestrator/src/main.rs
New Rust crate with CLI binary supporting guide-sync and audit pipelines. Main orchestrator implements multi-phase workflows (graph construction, change detection, partitioning, agent launching, polling, merging, PR creation, state updates).
Fleet Orchestrator Modules
crates/fleet-orchestrator/src/affected.rs, crates/fleet-orchestrator/src/config.rs, crates/fleet-orchestrator/src/graph.rs, crates/fleet-orchestrator/src/jetty.rs, crates/fleet-orchestrator/src/partitioner.rs
Core orchestration modules: affected-chapter computation, configuration defaults, dependency graph parsing/persistence, async Jetty API client with trajectory polling, and work partitioning by tier.
Fleet Orchestrator Workflows
crates/fleet-orchestrator/src/audit.rs, crates/fleet-orchestrator/src/merge.rs, crates/fleet-orchestrator/src/pr.rs, crates/fleet-orchestrator/src/state.rs
Audit pipeline (candidate discovery/filtering/partitioning), git merge/worktree operations, PR creation with GitHub GraphQL branch cleanup, and persistent fleet state management with atomic lock-based updates.
Orchestration Wrapper Scripts
run-guide-sync-fleet.sh, run-audit-fleet.sh, run-doc-rigor-fleet.sh, run-task-forge-fleet.sh
Bash orchestration entry points for guide sync, design-doc audit, doc-rigor, and task-forge enrichment pipelines. Handle environment setup, candidate filtering, agent launching, polling, merging, PR creation, and state updates.
Fleet State & Configuration
.fleet-state.json, .last-audit-sha, fleet/dependency_graph.json, Cargo.toml
New fleet state ledger tracking design-doc audits and guide-sync progress, dependency graph mapping sources to chapters, base audit SHA tracking, and workspace member registration.
Audit Trajectory Files
audit-trajectories-*.txt, doc-rigor-trajectories-*.txt
Static audit metadata and trajectory tracking files recording batch mappings and design-doc/doc-rigor audit runs across system components.
Documentation & Research
docs/fleet-orchestrator/architecture.md, docs/fleet-orchestrator/deeper-research-fleet-primitives.md, docs/fleet-orchestrator/jetty-feature-proposals.md, docs/fleet-orchestrator/research-fleet-native-primitives.md
Comprehensive architecture overview, research agenda for fleet-native primitives in agent platforms, and prioritized Jetty feature proposals. Includes data-flow tables, phased control flow, and tier scaling diagrams.
Runbook & Documentation
runbooks/task-forge-enrich.md, CLAUDE.md
Task-forge enrichment agent runbook with output validation and enrichment schema, plus CLAUDE.md update documenting new review feedback loop and operator commands.

Sequence Diagram(s)

sequenceDiagram
    participant Orchestrator as Fleet Orchestrator<br/>(Rust)
    participant Jetty as Jetty API<br/>(Cloud)
    participant Agent as Chapter Agent<br/>(LLM)
    participant Git as Git Repo<br/>(Local Worktree)
    participant GitHub as GitHub API<br/>(PRs & Branches)
    
    Orchestrator->>Orchestrator: Phase 0: Build graph,<br/>detect changed sources<br/>compute affected chapters
    
    Orchestrator->>Orchestrator: Phase 1: Partition chapters<br/>by section & tier,<br/>create agent manifests
    
    Orchestrator->>Jetty: POST /v1/chat/completions<br/>launch wave (chapters)
    Jetty->>Agent: Execute runbook<br/>+ manifest + branch
    Agent->>Agent: Analyze & write docs
    Agent->>Git: Push to chapter branch
    
    Orchestrator->>Jetty: Poll /api/v1/db/trajectory<br/>until agents complete
    Jetty-->>Orchestrator: AgentStatus (Running/Completed/Failed)
    
    Orchestrator->>Git: Phase 2b (Tier-3):<br/>Launch coordinator agents<br/>for section integration
    Agent->>Agent: Merge chapters,<br/>validate cross-refs
    Agent->>Git: Push section branch
    
    Orchestrator->>Git: Phase 3: Create merge worktree,<br/>fetch section branches,<br/>merge into main
    Git->>Git: Resolve conflicts,<br/>run cargo checks
    
    Orchestrator->>GitHub: Push umbrella branch
    GitHub-->>Orchestrator: Branch created
    
    Orchestrator->>GitHub: Create PR with summary<br/>& verification notes
    GitHub-->>Orchestrator: PR `#NNN` created
    
    Orchestrator->>GitHub: GraphQL: updateRefs<br/>delete agent branches
    GitHub-->>Orchestrator: Branches deleted
    
    Orchestrator->>Orchestrator: Phase 4: Update .fleet-state.json<br/>mark chapters Current,<br/>update source SHAs,<br/>atomic write with lock
Loading
sequenceDiagram
    participant Orchestrator as Fleet Orchestrator<br/>(Rust)
    participant Jetty as Jetty API<br/>(Cloud)
    participant Auditor as Audit Agent<br/>(LLM)
    participant Git as Git Repo<br/>(Local Clone)
    participant GitHub as GitHub API<br/>(PRs)
    
    Orchestrator->>Orchestrator: Phase 0: Walk docs/ & diagrams/,<br/>collect blob SHAs via<br/>git ls-tree, filter<br/>unchanged files
    
    Orchestrator->>Orchestrator: Partition candidates<br/>across agents (capped)
    
    Orchestrator->>Jetty: POST /v1/chat/completions<br/>per agent with<br/>doc manifest
    Jetty->>Auditor: Audit scope source files,<br/>generate findings
    Auditor->>Git: Push to audit branch
    
    Orchestrator->>Jetty: Poll trajectories<br/>until all complete
    Jetty-->>Orchestrator: Status updates
    
    Orchestrator->>Git: Phase 3: Clone repo,<br/>create merge branch,<br/>fetch audit branches
    Git->>Git: Merge audit branches,<br/>run cargo fmt/check/doc
    
    Orchestrator->>GitHub: Push umbrella branch
    GitHub-->>Orchestrator: Branch created
    
    Orchestrator->>GitHub: Create audit PR<br/>with metrics & verification
    GitHub-->>Orchestrator: PR created
    
    Orchestrator->>GitHub: GraphQL: Delete<br/>agent branches
    
    Orchestrator->>Orchestrator: Phase 4: Update .fleet-state.json<br/>record blob SHAs<br/>and timestamps
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~105 minutes

Possibly related PRs

Poem

🐰 A fleet of agents hops through the guide,
Each chapter synced with orchestrator pride,
Git branches merge, states persist in time,
From chaos to order—a beautiful rhyme!
Jetty clouds spin, PRs bloom with grace,
Review patterns codified, patterns in place. 🌿

🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title uses an imperative verb ('enrich') and clearly describes the main change (enriching 33 underspecified beads tasks). It is 48 characters, under the 72-character limit, and directly relates to the core objective of this PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 75.00%.
Design Doc Freshness ✅ Passed PR adds Rust source files in new crates/fleet-orchestrator/ crate with 2,417 lines of comprehensive design documentation in docs/fleet-orchestrator/, satisfying the docs-updated pass condition.
Comment And Doc Hygiene ✅ Passed All new and modified .rs files and docs/*.md files maintain proper comment hygiene with no tracking IDs, milestone labels, PR references, review-response language, temporal narration, conversational tone, or history narration detected.
No Versioning Or Legacy Code ✅ Passed PR contains no versioning, deprecation, or backwards-compatibility patterns in production code; version field appears only in test demonstration data.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-forge/enrichment-sweep-2026-04-10

Comment @coderabbitai help to get the list of available commands and usage tips.

@ahrav
ahrav force-pushed the task-forge/enrichment-sweep-2026-04-10 branch from 2a7dc71 to 85dabc9 Compare April 10, 2026 04:11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2a7dc71aae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".


tracing::info!("Phase 4: updating fleet state");

let updates = audit::build_audit_state_update(&manifests, &current_blobs, &run_id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update audit state only for successfully processed manifests

This call records every manifest as processed even when its agent failed, timed out, or its branch never merged, because the update set is built from manifests rather than the successful subset from poll/merge results. In those failure scenarios, filter_unchanged will later treat those files as up to date (same blob SHA) and skip them indefinitely, so unaudited docs can be silently dropped from future runs until they change again.

Useful? React with 👍 / 👎.

Comment on lines +343 to +347
for (file_path, file_state) in &mut gs.source_files {
if file_state.dependent_chapters.contains(chapter_name)
&& let Some(sha) = current_blobs.get(file_path)
{
file_state.blob_sha = sha.clone();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not advance source blob state on partial chapter failure

Source-file blob SHAs are advanced as soon as any chapter tied to that source succeeds, even if sibling chapters for the same source were marked stale in the same run. Since change detection is driven by source blob deltas, this can suppress retries for failed chapters: once the source blob is updated to current, subsequent runs may see no changed source and never re-enqueue the stale chapter despite the retry comment above.

Useful? React with 👍 / 👎.

33 tasks enriched with implementation-ready descriptions via
34-agent fleet run. Each task now has all 8 mandatory sections:
Context, Current State, Desired State, Implementation Guidance,
Code References, Related Work, Acceptance Criteria, Pointers.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ahrav
ahrav force-pushed the task-forge/enrichment-sweep-2026-04-10 branch from 85dabc9 to 7d37571 Compare April 10, 2026 04:16
@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR enriches 33 underspecified beads tasks via an agent fleet and introduces run-task-forge-fleet.sh alongside the new fleet-orchestrator Rust crate (Jetty client, merge helpers, state persistence, affected-chapter computation). The Rust library is well-structured with solid tests, but the shell orchestrator has correctness issues that will prevent it from running successfully.

  • P0 — polling loop crashes on first completion: ((COMPLETED++)), ((FAILED++)), ((RUNNING++)), and ((COLLECTED++)) all use post-increment; under set -euo pipefail, incrementing from 0 returns 0 (falsy) and immediately kills the script.
  • P1 — failed enrichments permanently skipped: The state-ledger update loop marks every task as enriched: True regardless of whether bd update succeeded, so any task that errored is silently excluded from all future runs.
  • P1 — JSON manifest corrupted by sed escaping: The sed command escapes for sed replacement syntax but the output feeds into a bash ${var//...} substitution where those characters are not special — & or / in task data corrupts the JSON payload."

Confidence Score: 3/5

Not safe to merge: the shell orchestrator crashes on every non-dry-run due to the post-increment/set-e bug, and the state ledger permanently skips any task whose bd update call failed.

Two P0/P1 bugs make the primary user path non-functional: the polling loop exits immediately when the first agent completes, and the state ledger permanently hides failed enrichments. A third P1 (sed escaping) corrupts the JSON manifest for tasks with & or / in their data. The Rust library code is solid and does not block merge on its own.

run-task-forge-fleet.sh (lines 354-358, 406, 269-271, 523-534) requires fixes before this can be used in production.

Important Files Changed

Filename Overview
run-task-forge-fleet.sh New shell orchestrator with three critical bugs: post-increment operators crash the polling loop under set -e, failed bd update calls are still marked as enriched in the state ledger, and the JSON manifest injection uses misguided sed-escaping that corrupts special characters in task data.
crates/fleet-orchestrator/src/main.rs New 1293-line fleet-orchestrator binary; generate_run_id never checks the exit status of date, and substitute_runbook embeds the first manifest's context into all agents' system prompts.
crates/fleet-orchestrator/src/merge.rs New git-merge helpers; well-structured with good conflict-abort handling and tests; misleading shallow clone comment since --depth 1 is absent.
crates/fleet-orchestrator/src/state.rs Fleet state persistence with atomic write and exclusive file locking; comprehensive tests; no issues found.
crates/fleet-orchestrator/src/jetty.rs Async Jetty API client; wave-based launch with adaptive poll backoff; clean.
crates/fleet-orchestrator/src/affected.rs Affected-chapter computation from changed source files; correct early-cutoff filter and deterministic sort; well-tested.
.claude/hooks/review-pattern-check.sh New Stop hook scanning modified files against YAML review rules; hand-rolled YAML parser is fragile but functional for the current rule format.
runbooks/task-forge-enrich.md Agent runbook for autonomous task-forge enrichment; well-structured with Python-based manifest handling to avoid shell quoting issues.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[run-task-forge-fleet.sh] --> B[Parse .beads/issues.jsonl
Identify underspecified tasks]
    B --> C{DRY_RUN?}
    C -- yes --> D[Print plan, exit]
    C -- no --> E[Phase 1: Launch Jetty agents
one per task batch]
    E --> F[POST /v1/chat/completions
with task manifest in runbook]
    F --> G[Write trajectory IDs to
task-forge-trajectories.txt]
    G --> H[Phase 2: Poll until done]
    H -- BUG: COMPLETED++ kills script
when first agent completes --> I[Phase 3: Collect enrichments]
    I --> J[Phase 4: Apply enrichments
bd update id --description]
    J --> K[Update state ledger
BUG: marks ALL as enriched
even on bd failure]
    K --> L[Phase 5: Delete agent branches]
    subgraph fleet-orchestrator Rust crate
        M[main.rs: GuideSync and Audit pipelines]
        N[affected.rs: changed-file to chapter mapping]
        O[jetty.rs: HTTP client, wave launch, poll]
        P[merge.rs: worktree and git merge operations]
        Q[state.rs: atomic JSON state with file lock]
        M --> N
        M --> O
        M --> P
        M --> Q
    end
Loading

Comments Outside Diff (4)

  1. run-task-forge-fleet.sh, line 354-358 (link)

    P0 ((X++)) post-increment exits with code 1 under set -e, killing the polling loop

    set -euo pipefail is active. ((COMPLETED++)) uses post-increment: it evaluates to the old value and exits with bash status 1 when that old value is 0. On the very first poll that returns a completed trajectory — while COMPLETED is still 0 — the entire script dies before printing a single status line. The same failure happens for FAILED and RUNNING.

    Line 406 (((COLLECTED++))) has the identical bug: the first successfully collected enrichment terminates the collection loop.

    Fix: use the additive form which always evaluates to the new value:

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: run-task-forge-fleet.sh
    Line: 354-358
    
    Comment:
    **`((X++))` post-increment exits with code 1 under `set -e`, killing the polling loop**
    
    `set -euo pipefail` is active. `((COMPLETED++))` uses post-increment: it evaluates to the *old* value and exits with bash status 1 when that old value is 0. On the very first poll that returns a completed trajectory — while `COMPLETED` is still 0 — the entire script dies before printing a single status line. The same failure happens for `FAILED` and `RUNNING`.
    
    Line 406 (`((COLLECTED++))`) has the identical bug: the first successfully collected enrichment terminates the collection loop.
    
    Fix: use the additive form which always evaluates to the new value:
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. run-task-forge-fleet.sh, line 520-534 (link)

    P1 State ledger unconditionally marks failed enrichments as enriched

    The loop at line 523 iterates over all_enrichments and writes "enriched": True for every entry regardless of whether bd update succeeded. A task that timed out or received a non-zero exit code is still stamped as enriched, silently excluded from all future runs even though nothing was written. Track which IDs were successfully applied and only update the ledger for those.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: run-task-forge-fleet.sh
    Line: 520-534
    
    Comment:
    **State ledger unconditionally marks failed enrichments as enriched**
    
    The loop at line 523 iterates over `all_enrichments` and writes `"enriched": True` for every entry regardless of whether `bd update` succeeded. A task that timed out or received a non-zero exit code is still stamped as enriched, silently excluded from all future runs even though nothing was written. Track which IDs were successfully applied and only update the ledger for those.
    
    How can I resolve this? If you propose a fix, please make it concise.
  3. run-task-forge-fleet.sh, line 269-271 (link)

    P1 sed escaping targets sed replacement syntax, not bash ${var//...} substitution

    sed 's/[&/\]/\\&/g' escapes characters for a sed s replacement, but the result is fed into a bash ${agent_runbook//{{task_manifest}}/$escaped_manifest} substitution where &, /, and \ carry no special meaning. Any & in a task title or description becomes \& in the JSON payload — invalid JSON the agent will reject. Use jq --argjson manifest "$task_manifest" to inject the manifest into the payload instead of shell string substitution.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: run-task-forge-fleet.sh
    Line: 269-271
    
    Comment:
    **sed escaping targets `sed` replacement syntax, not bash `${var//...}` substitution**
    
    `sed 's/[&/\]/\\&/g'` escapes characters for a sed `s` replacement, but the result is fed into a bash `${agent_runbook//{{task_manifest}}/$escaped_manifest}` substitution where `&`, `/`, and `\` carry no special meaning. Any `&` in a task title or description becomes `\&` in the JSON payload — invalid JSON the agent will reject. Use `jq --argjson manifest "$task_manifest"` to inject the manifest into the payload instead of shell string substitution.
    
    How can I resolve this? If you propose a fix, please make it concise.
  4. run-task-forge-fleet.sh, line 383-384 (link)

    P2 TMPDIR variable name shadows the POSIX temp-dir environment variable

    TMPDIR=$(mktemp -d) overwrites the exported TMPDIR env var that Python, mktemp, and other tools use to discover where to place temporary files. Use a distinct name such as WORK_DIR and update the subsequent rm -rf "$TMPDIR" reference accordingly.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: run-task-forge-fleet.sh
    Line: 383-384
    
    Comment:
    **`TMPDIR` variable name shadows the POSIX temp-dir environment variable**
    
    `TMPDIR=$(mktemp -d)` overwrites the exported `TMPDIR` env var that Python, `mktemp`, and other tools use to discover where to place temporary files. Use a distinct name such as `WORK_DIR` and update the subsequent `rm -rf "$TMPDIR"` reference accordingly.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: run-task-forge-fleet.sh
Line: 354-358

Comment:
**`((X++))` post-increment exits with code 1 under `set -e`, killing the polling loop**

`set -euo pipefail` is active. `((COMPLETED++))` uses post-increment: it evaluates to the *old* value and exits with bash status 1 when that old value is 0. On the very first poll that returns a completed trajectory — while `COMPLETED` is still 0 — the entire script dies before printing a single status line. The same failure happens for `FAILED` and `RUNNING`.

Line 406 (`((COLLECTED++))`) has the identical bug: the first successfully collected enrichment terminates the collection loop.

Fix: use the additive form which always evaluates to the new value:

```suggestion
      completed) (( COMPLETED += 1 )) ;;
      failed|cancelled) (( FAILED += 1 )) ;;
      *) (( RUNNING += 1 )) ;;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: run-task-forge-fleet.sh
Line: 520-534

Comment:
**State ledger unconditionally marks failed enrichments as enriched**

The loop at line 523 iterates over `all_enrichments` and writes `"enriched": True` for every entry regardless of whether `bd update` succeeded. A task that timed out or received a non-zero exit code is still stamped as enriched, silently excluded from all future runs even though nothing was written. Track which IDs were successfully applied and only update the ledger for those.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: run-task-forge-fleet.sh
Line: 269-271

Comment:
**sed escaping targets `sed` replacement syntax, not bash `${var//...}` substitution**

`sed 's/[&/\]/\\&/g'` escapes characters for a sed `s` replacement, but the result is fed into a bash `${agent_runbook//{{task_manifest}}/$escaped_manifest}` substitution where `&`, `/`, and `\` carry no special meaning. Any `&` in a task title or description becomes `\&` in the JSON payload — invalid JSON the agent will reject. Use `jq --argjson manifest "$task_manifest"` to inject the manifest into the payload instead of shell string substitution.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: run-task-forge-fleet.sh
Line: 383-384

Comment:
**`TMPDIR` variable name shadows the POSIX temp-dir environment variable**

`TMPDIR=$(mktemp -d)` overwrites the exported `TMPDIR` env var that Python, `mktemp`, and other tools use to discover where to place temporary files. Use a distinct name such as `WORK_DIR` and update the subsequent `rm -rf "$TMPDIR"` reference accordingly.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/fleet-orchestrator/src/main.rs
Line: 180-188

Comment:
**`generate_run_id` silently produces an empty run ID when `date` fails**

The function checks that `Command::new("date")` doesn't error on spawn but never inspects `output.status.success()`. If `date -u` exits non-zero, `stdout` is empty and the run ID becomes `"guide-sync-"`, causing branch name collisions across runs. Add an exit-code check and bail with the stderr message.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/fleet-orchestrator/src/main.rs
Line: 427-433

Comment:
**Runbook substitution uses only `manifests.first()`, so all agents share one manifest's context**

`substitute_runbook` replaces `{{section_id}}`, `{{write_set}}`, `{{read_crates}}`, and `{{agent_id}}` using `manifests.first()`. Any runbook template that reads these fields for routing will receive the first agent's values for all agents. Consider building per-agent runbooks or explicitly documenting that these placeholders must not appear in the system prompt.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: crates/fleet-orchestrator/src/merge.rs
Line: 333-354

Comment:
**`create_clone_worktree` doc says "shallow clone" but doesn't pass `--depth 1`**

The comment reads "Shallow clone (single branch) to minimise disk and network usage", but the `git clone` invocation has no `--depth 1` flag — full history is transferred. Either add `--depth 1` or correct the comment to say "single-branch clone".

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "task-forge: enrich 33 underspecified bea..." | Re-trigger Greptile

Comment on lines +180 to +188
fn generate_run_id() -> anyhow::Result<String> {
let output = Command::new("date")
.args(["-u", "+%Y-%m-%d-%H%M"])
.output()
.context("failed to run `date` for run ID generation")?;

let stamp = String::from_utf8_lossy(&output.stdout).trim().to_owned();
Ok(format!("guide-sync-{stamp}"))
}

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.

P2 generate_run_id silently produces an empty run ID when date fails

The function checks that Command::new("date") doesn't error on spawn but never inspects output.status.success(). If date -u exits non-zero, stdout is empty and the run ID becomes "guide-sync-", causing branch name collisions across runs. Add an exit-code check and bail with the stderr message.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/fleet-orchestrator/src/main.rs
Line: 180-188

Comment:
**`generate_run_id` silently produces an empty run ID when `date` fails**

The function checks that `Command::new("date")` doesn't error on spawn but never inspects `output.status.success()`. If `date -u` exits non-zero, `stdout` is empty and the run ID becomes `"guide-sync-"`, causing branch name collisions across runs. Add an exit-code check and bail with the stderr message.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +427 to +433
let runbook = substitute_runbook(
&runbook_template,
manifests.first().expect("at least one manifest"),
&config,
&base_sha,
&run_id,
);

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.

P2 Runbook substitution uses only manifests.first(), so all agents share one manifest's context

substitute_runbook replaces {{section_id}}, {{write_set}}, {{read_crates}}, and {{agent_id}} using manifests.first(). Any runbook template that reads these fields for routing will receive the first agent's values for all agents. Consider building per-agent runbooks or explicitly documenting that these placeholders must not appear in the system prompt.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/fleet-orchestrator/src/main.rs
Line: 427-433

Comment:
**Runbook substitution uses only `manifests.first()`, so all agents share one manifest's context**

`substitute_runbook` replaces `{{section_id}}`, `{{write_set}}`, `{{read_crates}}`, and `{{agent_id}}` using `manifests.first()`. Any runbook template that reads these fields for routing will receive the first agent's values for all agents. Consider building per-agent runbooks or explicitly documenting that these placeholders must not appear in the system prompt.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +333 to +354
let output = Command::new("git")
.args([
"clone",
"--quiet",
"--no-checkout",
"--single-branch",
"--branch",
base_branch,
remote_url,
&clone_path.to_string_lossy(),
])
.output()
.context("spawn git clone for merge worktree")?;

if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
bail!(
"git clone failed (exit {:?}): {}",
output.status.code(),
stderr.trim_end()
);
}

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.

P2 create_clone_worktree doc says "shallow clone" but doesn't pass --depth 1

The comment reads "Shallow clone (single branch) to minimise disk and network usage", but the git clone invocation has no --depth 1 flag — full history is transferred. Either add --depth 1 or correct the comment to say "single-branch clone".

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/fleet-orchestrator/src/merge.rs
Line: 333-354

Comment:
**`create_clone_worktree` doc says "shallow clone" but doesn't pass `--depth 1`**

The comment reads "Shallow clone (single branch) to minimise disk and network usage", but the `git clone` invocation has no `--depth 1` flag — full history is transferred. Either add `--depth 1` or correct the comment to say "single-branch clone".

How can I resolve this? If you propose a fix, please make it concise.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new fleet orchestration library and associated tooling to automate documentation synchronization and design document audits. The changes include a Rust-based orchestrator, shell-based hooks for pattern checking, and lifecycle management scripts for review rules. I have provided feedback on improving the reliability of the base SHA resolution, the robustness of the Bash-based YAML parser, and the error handling in the merge logic. I also highlighted potential issues with command-line argument limits in the task enrichment script.

I am having trouble creating individual review comments. Click here to see my feedback.

crates/fleet-orchestrator/src/main.rs (287)

high

The get_base_sha() function resolves origin/main to determine the base for change detection, but the orchestrator does not perform a git fetch before this call. If the local repository's remote tracking branches are stale, the orchestrator will compute affected chapters based on an outdated codebase, potentially missing recent changes or attempting to sync against non-existent state. Consider adding a git fetch origin main call before resolving the base SHA.

    let _ = Command::new("git")
        .args(["fetch", "origin", &config.base_branch, "--quiet"])
        .output();
    let base_sha = get_base_sha()?;

.claude/hooks/review-pattern-check.sh (205-217)

medium

The current grep logic for matching patterns against added lines is fragile because it operates on the output of awk, which prefixes each line with line_num: + . If a grep_pattern in review-rules.yaml uses anchors like ^ (e.g., ^let _ =), it will fail to match. Additionally, the grep might accidentally match the line number itself if the pattern is numeric. Consider stripping the prefix before grepping or adjusting the regex to account for it.

run-task-forge-fleet.sh (491)

medium

Using bd update with a potentially very large description string passed as a command-line argument can hit OS-level ARG_MAX limits, especially when enriching dozens of tasks in a single run. The average enrichment is ~9.2K characters, which is safe for a few tasks, but could become an issue if the fleet size or task complexity grows. Consider using a temporary file for the description if bd supports reading from a file, or ensure the string is properly escaped for shell execution.

crates/fleet-orchestrator/src/merge.rs (196-205)

medium

The merge_section function uses a sequential merge strategy and aborts the entire merge if a conflict is detected. When implementing a more robust conflict resolution mechanism, ensure that diagnostic metadata (e.g., display_name) is treated as cosmetic and does not trigger conflicts; such duplicates should be silently deduplicated (first-wins). Avoid adding speculative complexity like path-keyed caching unless justified by an immediate batch caller. Additionally, if the function can return multiple errors, collect all potential errors first and prioritize the root-cause error.

References
  1. Diagnostic metadata (e.g., display_name) should be treated as cosmetic and not trigger conflicts; duplicates should be silently deduplicated.
  2. Avoid adding speculative complexity, such as path-keyed caching, if there isn't an immediate in-tree batch caller to justify it.
  3. When a function can return multiple errors, collect all potential errors first and prioritize the root-cause error.

.claude/hooks/review-pattern-check.sh (54-153)

medium

The custom YAML parser implemented in Bash is highly sensitive to indentation and formatting (e.g., it expects exactly 4 spaces for fields and specific markers for block scalars). This makes the hook brittle if review-rules.yaml is edited by tools that change indentation or use different YAML features (like flow style). Since python3 and PyYAML are already used in other scripts in this PR (e.g., review-rule-lifecycle.sh), consider using a Python helper to parse the rules reliably.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 3

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

🟠 Major comments (23)
.claude/skills/review-pipeline/SKILL.md-215-218 (1)

215-218: ⚠️ Potential issue | 🟠 Major

Unify confidence scale before logging findings.

Line 216 uses confidence >= 0.60, but earlier Phase 1 guidance uses a 0-100 scale (>= 60). If merged findings keep 0-100 confidence, this filter becomes effectively permissive and pollutes .claude/review-findings.jsonl.

💡 Proposed fix
-`.claude/review-findings.jsonl` (one JSON object per line, confidence >= 0.60).
+`.claude/review-findings.jsonl` (one JSON object per line, confidence >= 60).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/review-pipeline/SKILL.md around lines 215 - 218, The filter
"confidence >= 0.60" in the synthesis phase is inconsistent with Phase 1's 0–100
scale and will let low-confidence (e.g., 60/100) findings through; update the
logic that appends merged findings to `.claude/review-findings.jsonl` so
confidence is normalized before filtering (either convert 0–100 scores to
0.0–1.0 or convert the threshold to 60), then apply the threshold check (e.g.,
if confidence > = 0.60 when normalized or confidence >= 60 when using 0–100),
keep using the `/review-dispatch` schema and set `source: "review-pipeline"` on
each JSON object; locate the synthesis/append code that performs the "confidence
>= 0.60" check to implement this normalization and threshold change.
.claude/skills/review-dispatch/SKILL.md-437-448 (1)

437-448: ⚠️ Potential issue | 🟠 Major

Confidence threshold conflicts with the file’s 0-100 scoring model.

Lines 438-439 use confidence >= 0.60, but the ranker defines confidence as 0-100. This mismatch can over-log low-confidence findings and degrade rule quality.

💡 Proposed fix
-`.claude/review-findings.jsonl` (one JSON object per line). Only log findings
-with confidence >= 0.60.
+`.claude/review-findings.jsonl` (one JSON object per line). Only log findings
+with confidence >= 60.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/review-dispatch/SKILL.md around lines 437 - 448, The skill
currently filters findings using "confidence >= 0.60" while the ranker produces
confidence on a 0–100 scale, causing low-confidence items to be included; update
the logic that writes to .claude/review-findings.jsonl (the block that checks
confidence >= 0.60) to either (A) compare against 60 (i.e., confidence >= 60) to
match the 0–100 model, or (B) normalize the ranker score to 0–1 before applying
the 0.60 threshold; ensure the chosen approach is applied where findings are
appended and documented alongside id generation (rf-YYYYMMDD-NNN) and
category/subcategory assignment from the taxonomy.
.claude/scripts/detect-patterns.sh-94-105 (1)

94-105: ⚠️ Potential issue | 🟠 Major

Unresolved-only clusters must not be marked as codify candidates.

Lines 94–96 set fp_rate to 0 when no resolutions exist; combined with the filter at line 104 (.total_count >= 3 and .fp_rate <= 0.15), clusters with zero truth labels are incorrectly promoted to codify.

Add .resolved_count > 0 to the selection criteria:

Fix
       patterns_detected: [
-        .[] | select(.total_count >= 3 and .fp_rate <= 0.15)
+        .[] | select(.total_count >= 3 and .resolved_count > 0 and .fp_rate <= 0.15)
         | . + {recommendation: "codify"}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/scripts/detect-patterns.sh around lines 94 - 105, The selection is
promoting clusters with no truth labels because fp_rate is forced to 0 when
(.true_positive_count + .false_positive_count) == 0; update the
patterns_detected filter to require resolved examples by adding a check for
.resolved_count > 0 to the existing predicate (the block that currently uses .[]
| select(.total_count >= 3 and .fp_rate <= 0.15)), so only clusters with
.total_count >= 3, .fp_rate <= 0.15, and .resolved_count > 0 are marked with
{recommendation: "codify"}.
run-doc-rigor-fleet.sh-262-273 (1)

262-273: ⚠️ Potential issue | 🟠 Major

Require a launch ID before treating the agent as started.

A 2xx response with neither trajectory_id nor workflow_id currently gets written to TRAJ_FILE as an empty ID, and the poll/merge phases then operate on an invalid trajectory path.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-doc-rigor-fleet.sh` around lines 262 - 273, After attempting to extract
trajectory_id from workflow_id the script must not treat the agent as started
when both trajectory_id and workflow_id are empty; change the logic so that if
trajectory_id is still empty (even for 2xx http_code) you log the failure (use
the same err extraction from body: err=$(echo "$body" | jq -r '.error.message //
.error // .detail // empty')), print a FAIL line including agent_id and
http_code, and continue without appending to TRAJ_FILE; ensure you reference the
variables trajectory_id, workflow_id, http_code, body, agent_id and TRAJ_FILE
and avoid writing empty trajectory entries.
run-doc-rigor-fleet.sh-477-483 (1)

477-483: ⚠️ Potential issue | 🟠 Major

Only mark files processed after a successful merge.

These entries are written for every manifest file, including agents that failed or branches that never merged. On the next run, unchanged files from those failed partitions will be skipped even though no doc-rigor result was actually applied.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-doc-rigor-fleet.sh` around lines 477 - 483, The code currently writes
doc_rigor_state entries for every manifest in candidates["manifests"] (using
manifest["files"], current_blobs, processed_at=now, run_id) even if that
manifest/agent failed or the branch never merged; change this so those entries
are only set after a successful merge/application of doc-rigor for that
manifest. Concretely, move the doc_rigor_state update out of the unconditional
loop and instead perform the same per-file assignment (using doc_rigor_state[f]
with blob_sha from current_blobs, processed_at, run_id) inside the success path
where you detect a successful merge or after the code that applies the doc-rigor
result for that manifest/agent.
run-doc-rigor-fleet.sh-69-74 (1)

69-74: ⚠️ Potential issue | 🟠 Major

Fail fast if Jetty rejects the env sync.

This PATCH ignores the HTTP status entirely, so a 401/500 still looks successful and the run continues with missing or stale GitHub credentials in the collection environment.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-doc-rigor-fleet.sh` around lines 69 - 74, The PATCH command using curl
(the block with curl -sS -o /dev/null -X PATCH ...) currently ignores the HTTP
status; update it to fail fast by either adding --fail (or --fail-with-body) to
the curl options or by capturing the HTTP status with -w "%{http_code}" and
checking it, and if the status is not 2xx log an error including JETTY_HOST,
COLLECTION and the returned status and exit with a non‑zero code so the script
stops when Jetty rejects the env sync (use the existing variables JETTY_API_KEY
and token_payload in the error context).
run-doc-rigor-fleet.sh-373-382 (1)

373-382: ⚠️ Potential issue | 🟠 Major

Run the required clippy gate on merged Rust changes.

This flow can merge .rs documentation edits, but post-merge verification only runs fmt, check, and doc. The repo rule also requires cargo clippy --all-targets --all-features -- -D warnings before handing off a Rust change. As per coding guidelines, **/*.rs: "After modifying Rust code, ALWAYS run: cargo fmt --all && cargo check && cargo clippy --all-targets --all-features -- -D warnings && RUSTDOCFLAGS='-D warnings' cargo doc --no-deps --all-features".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-doc-rigor-fleet.sh` around lines 373 - 382, Post-merge verification omits
the required Clippy gate; update the verification block to run cargo clippy with
the required flags (cargo clippy --all-targets --all-features -- -D warnings)
and treat failures like the other checks by adding a PASS/WARN message and
appending a "- cargo clippy: FAILED" entry to VERIFY_NOTES so the script's
verification sequence (the block running cargo fmt, cargo check, RUSTDOCFLAGS...
cargo doc) includes clippy enforcement.
run-guide-sync-fleet.sh-28-33 (1)

28-33: ⚠️ Potential issue | 🟠 Major

Check the Jetty env sync response before starting the fleet.

The wrapper suppresses the PATCH response entirely, so auth or server errors go unnoticed and the orchestrator starts with stale credentials in the collection environment.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-guide-sync-fleet.sh` around lines 28 - 33, The patch request using curl
currently discards the response (uses -o /dev/null) so auth/server errors are
ignored; change the invocation that sends "$token_payload" to capture the HTTP
status and response body (remove -o /dev/null or write output to a variable),
check for non-2xx status (or use curl --fail) and log or exit with an error
before starting the fleet to avoid continuing with stale credentials; make sure
to reference the token_payload variable and the PATCH to
"https://flows-api.jetty.io/api/v1/collections/asdf22223/environment" when
adding the response handling and failure behavior.
docs/fleet-orchestrator/deeper-research-fleet-primitives.md-45-671 (1)

45-671: ⚠️ Potential issue | 🟠 Major

Remove the research-ledger IDs from the doc body.

Markers like P3.4.F4, P1.2.F6, and P3.1.F1-F6 make the document depend on an external notes system. Inline the evidence directly, or move the lookup table into this file so the prose stands on its own. As per coding guidelines, {**/*.rs,docs/**/*.md,diagrams/**/*.md}: "Do NOT include tracking IDs" and "Comments and docs must stand alone."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/deeper-research-fleet-primitives.md` around lines 45
- 671, The document contains research-led tracking IDs (e.g. P3.4.F4, P1.2.F6,
P3.1.F1-F6) embedded throughout the Proposal sections; remove these IDs and
either inline the short evidence text directly where each ID is cited or
consolidate a self-contained evidence appendix inside this file (so the prose no
longer depends on external lookup). Search for tokens matching the pattern
"P[0-9]+\.[0-9]+\.F[0-9]+" (and ranges like P3.1.F1-F6) across the Proposal
headings (Proposal 1..11) and replace each ID with the corresponding summarized
evidence sentence or move all ID→evidence mappings into a new local "Research
Evidence" table in this document, updating in-text citations to reference that
local table instead of external IDs.
run-doc-rigor-fleet.sh-177-178 (1)

177-178: ⚠️ Potential issue | 🟠 Major

Guard the zero-work path before dividing by AGENT_COUNT.

Line 177 computes FILES_PER before the TO_PROCESS == 0 early exit. When there is nothing to process, AGENT_COUNT is also 0, so the script exits on division-by-zero instead of printing the intended "Nothing to process" message.

Suggested fix
- FILES_PER=$(( TO_PROCESS / AGENT_COUNT ))
- echo "Partition: ${AGENT_COUNT} agents (~${FILES_PER} files each)"
- echo "================================================"
- echo "$CANDIDATES" | jq -r '.manifests[] | "  \(.agent_id): \(.files | length) files"'
- echo ""
-
 if [[ "$TO_PROCESS" -eq 0 ]]; then
   echo "Nothing to process. All files are up to date."
   exit 0
 fi
+
+FILES_PER=$(( TO_PROCESS / AGENT_COUNT ))
+echo "Partition: ${AGENT_COUNT} agents (~${FILES_PER} files each)"
+echo "================================================"
+echo "$CANDIDATES" | jq -r '.manifests[] | "  \(.agent_id): \(.files | length) files"'
+echo ""
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-doc-rigor-fleet.sh` around lines 177 - 178, Move the zero-work guard so
the script checks TO_PROCESS == 0 (and prints "Nothing to process") before
performing the division to compute FILES_PER; specifically, ensure you don't
evaluate FILES_PER = $(( TO_PROCESS / AGENT_COUNT )) or reference AGENT_COUNT
when TO_PROCESS is zero. Update the logic around the FILES_PER calculation and
the existing early-exit that prints "Nothing to process" so the division only
runs when TO_PROCESS > 0 (and AGENT_COUNT > 0).
run-task-forge-fleet.sh-129-153 (1)

129-153: ⚠️ Potential issue | 🟠 Major

Track all eight mandatory sections when selecting tasks.

The selector only looks for four sections and only treats a task as underspecified when two of those are missing. Tasks that still lack Current State, Desired State, Related Work, or Pointers can be skipped forever even though they still violate the task contract.

Based on learnings: "Every beads task must be self-contained with: Context, Current State, Desired State, Implementation Guidance, Code References, Related Work, Acceptance Criteria, and Pointers."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-task-forge-fleet.sh` around lines 129 - 153, The MANDATORY_SECTIONS list
and underspecified logic only check four sections and mark a task underspecified
only if two are missing; update MANDATORY_SECTIONS to include all eight required
headers (Context, Current State, Desired State, Implementation Guidance, Code
References, Related Work, Acceptance Criteria, Pointers) with sensible
case-insensitive marker variants, then change the underspecified condition in
the tasks loop (where tid/desc are read and count_missing_sections is called) to
treat a task as underspecified if any required section is missing or the
description is empty (i.e., if len(missing) > 0 or len(desc) == 0) so
count_missing_sections and underspecified/well_specified classification enforce
the full beads task contract.
runbooks/task-forge-enrich.md-93-95 (1)

93-95: ⚠️ Potential issue | 🟠 Major

Keep the incremental-write contract consistent.

Step 2 tells agents to persist each task immediately, but Step 3 then tells them to wait until all tasks are researched and shows a single final write. Agents that follow the later example can lose every completed enrichment on sandbox failure, which defeats the resilience requirement you set earlier.

Suggested doc fix
-## Step 3 — Write Enrichment JSON
-
-After researching ALL tasks, write the output. Use Python to avoid any
-shell quoting issues:
+## Step 3 — Persist Enrichment JSON Incrementally
+
+Persist `task-forge-enrichments.json` after each task is completed. Load the
+existing file if present, append/update the current task's enrichment, and
+rewrite the file atomically so partial progress survives sandbox failures.
 
 ```python
 import json
+from pathlib import Path
 
-# Build from your research findings
-enrichments = []
-for task_data in manifest:
-    enrichments.append({
-        "id": task_data["id"],
-        "title": task_data["title"],
-        "enriched_description": "...",  # Full markdown from Step 2
-        "sections_added": [
-            "Context", "Current State", "Desired State",
-            "Implementation Guidance", "Code References",
-            "Related Work", "Acceptance Criteria", "Pointers"
-        ],
-        "scope": {
-            "files_affected": 0,
-            "modules_crossed": 0,
-            "touches_hot_path": False,
-            "has_unsafe": False,
-            "allocation_tier": "WARM"
-        },
-        "recommended_skills": []
-    })
+path = Path("task-forge-enrichments.json")
+existing = []
+if path.exists():
+    existing = json.loads(path.read_text())
 
-with open("task-forge-enrichments.json", "w") as f:
-    json.dump(enrichments, f, indent=2, ensure_ascii=False)
+current = {
+    "id": task_data["id"],
+    "title": task_data["title"],
+    "enriched_description": "...",
+    "sections_added": [
+        "Context", "Current State", "Desired State",
+        "Implementation Guidance", "Code References",
+        "Related Work", "Acceptance Criteria", "Pointers"
+    ],
+    "scope": {
+        "files_affected": 0,
+        "modules_crossed": 0,
+        "touches_hot_path": False,
+        "has_unsafe": False,
+        "allocation_tier": "WARM"
+    },
+    "recommended_skills": []
+}
+
+existing = [e for e in existing if e["id"] != current["id"]]
+existing.append(current)
+path.write_text(json.dumps(existing, indent=2, ensure_ascii=False))
 
-print(f"Wrote {len(enrichments)} enrichments")
+print(f"Wrote {len(existing)} enrichments")

Also applies to: 176-210

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@runbooks/task-forge-enrich.md` around lines 93 - 95, The doc shows
conflicting persistence guidance—Step 2 mandates incremental per-task writes but
Step 3 demonstrates a single final write; change the example to perform
incremental, idempotent saves per task by loading the existing JSON (path =
Path("task-forge-enrichments.json")), filtering out any entry with the same id
(current = {...} built from task_data), appending the new current enrichment,
and writing back via path.write_text(json.dumps(existing, ...)); update the
print to report len(existing) so each task is persisted immediately and replaces
prior entries with the same id (apply same fix in the other occurrence at lines
~176-210).
run-task-forge-fleet.sh-401-404 (1)

401-404: ⚠️ Potential issue | 🟠 Major

Don’t mark collected tasks as enriched until the schema is valid and bd update succeeds.

Right now anything that parses as JSON is accepted, and the state ledger marks every collected task as enriched: true even when bd update failed or timed out. That persists partial/bad descriptions and suppresses retries on the next run.

Also applies to: 482-544

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-task-forge-fleet.sh` around lines 401 - 404, The script currently accepts
any JSON and sets tasks as enriched immediately after creating
"${ENRICHMENTS_DIR}/${aid}.json"; change the flow so you only mark tasks
enriched after (1) validating the enrichment JSON against the schema (not just
json.tool) and (2) successfully running `bd update` for that file. Concretely:
after producing "${ENRICHMENTS_DIR}/${aid}.json" and checking basic JSON, run a
schema validator (or a validation step) against that file, then invoke `bd
update "${ENRICHMENTS_DIR}/${aid}.json"` and test its exit code/timeout; only if
the validator and `bd update` both succeed should you set `enriched: true` in
the state ledger (the code that currently sets enriched based on `task_count`
must be moved to run after successful `bd update`), otherwise leave tasks
unmarked so they will be retried on the next run.
run-task-forge-fleet.sh-71-73 (1)

71-73: ⚠️ Potential issue | 🟠 Major

Move the merge/PR flow into a temporary clone instead of the caller’s checkout.

This script force-checks out main, creates a branch, commits, and pushes from the user's working tree. If the checkout is dirty, git checkout -f can discard tracked changes, and any mid-run failure leaves the repo on a synthetic branch. run-audit-fleet.sh already uses a temp clone to avoid exactly this class of damage.

Also applies to: 567-579, 614-617

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-task-forge-fleet.sh` around lines 71 - 73, The script currently runs git
checkout "${BASE_BRANCH}" and git checkout -f "${BASE_BRANCH}" directly in the
caller's repo (and similar logic at the other sections), which can discard local
changes and leave the user's working tree on a synthetic branch; update the
logic so all merge/PR branch creation, commits, pushes and any forced checkouts
occur inside a temporary clone: create a temp directory, git clone
--no-hardlinks or git clone --depth=1 of the current repo into it, perform the
git checkout "${BASE_BRANCH}", create the feature branch, commit, push, and run
merges inside that clone (use the same BASE_BRANCH, branch names and git remote
operations currently referenced), then cleanly remove the temp clone and return
without touching the caller’s working tree; remove any use of git checkout -f on
the caller repo and ensure error handling cleans up the temp clone on failure.
docs/fleet-orchestrator/research-fleet-native-primitives.md-25-33 (1)

25-33: ⚠️ Potential issue | 🟠 Major

Replace workstation-specific absolute paths with repo-relative references.

Hard-coding /Users/ahrav/Projects/... makes this unusable for every other reader and breaks the claim that the document is self-contained context. Use repo names plus paths relative to each repo root instead.

Also applies to: 699-799

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/research-fleet-native-primitives.md` around lines 25
- 33, The table currently hard-codes workstation-specific absolute paths;
replace those absolute paths with repo-relative references using the repo names
(gossip-rs, mise, mlcbakery, spot, gossip-rs-learning-guide) and paths relative
to each repo root (e.g., ./, ./subdir) so the document is portable; update the
table entries for the listed repos and any other occurrences of the same
absolute paths elsewhere in the document (the later section that repeats these
repo paths) to use the same repo-relative format.
run-audit-fleet.sh-405-430 (1)

405-430: ⚠️ Potential issue | 🟠 Major

Write .fleet-state.json atomically, or concurrent sweeps can clobber each other.

Both Python blocks do a plain read-modify-write of .fleet-state.json with no lock and no temp-file rename. That can lose updates from another fleet namespace or leave truncated JSON if two runs overlap. The Rust helper in crates/fleet-orchestrator/src/state.rs:129-160 already implements the safer lock + atomic-write pattern for this exact file.

Also applies to: 505-549

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-audit-fleet.sh` around lines 405 - 430, The current Python blocks (the
one that sets state_file/state_file write via the state_file, all_state, st
variables and the final "with open(state_file,'w') as f: json.dump(...)"
pattern) do a plain read-modify-write on `.fleet-state.json` and must be
replaced with an atomic write + lock pattern (as implemented in
crates/fleet-orchestrator/src/state.rs:129-160). Change both Python blocks
(around the regions that build all_state/st and call with open(state_file,'w')
as f ...) to: acquire an exclusive lock on the state file (e.g., using
fcntl.flock or a small portalocker helper), read the current JSON after
acquiring the lock into all_state, write the new JSON to a temporary file in the
same directory (e.g., state_file + ".tmp"), flush and fsync the temp file, then
atomically rename the temp file over state_file and release the lock; ensure
errors leave the original file intact and that blob updates still use
blobs.get(...) and the same keys so behavior is unchanged.
run-audit-fleet.sh-210-223 (1)

210-223: ⚠️ Potential issue | 🟠 Major

Guard the empty-work path before dividing by AGENT_COUNT.

The partition summary on line 216 executes $(( TO_PROCESS / AGENT_COUNT )) before the TO_PROCESS == 0 check on line 221. When $CANDIDATES has no manifests, both AGENT_COUNT (from .manifests | length) and TO_PROCESS (from .to_process) are 0, causing a division-by-zero error that exits the script before reaching the "Nothing to process" message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-audit-fleet.sh` around lines 210 - 223, The script currently computes and
prints a partition summary using $(( TO_PROCESS / AGENT_COUNT )) before checking
for the empty-work case, which can trigger a division-by-zero when AGENT_COUNT
is 0; fix this by guarding the empty/zero-work path before any arithmetic on
AGENT_COUNT and TO_PROCESS (e.g., move the if [[ "$TO_PROCESS" -eq 0 ]] check to
precede the partition summary or add a protective conditional that only computes
the per-agent division when AGENT_COUNT>0), ensuring references to AGENT_COUNT,
TO_PROCESS and CANDIDATES are validated before performing the division or
printing the summary.
crates/fleet-orchestrator/src/main.rs-892-899 (1)

892-899: ⚠️ Potential issue | 🟠 Major

Use update_state() for this final write to avoid lost updates.

This is a read-modify-write split across two independent lock acquisitions. A concurrent run can update another namespace after read_state() returns and then get overwritten by write_state(). state::update_state() already exists to make this mutation atomic.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/main.rs` around lines 892 - 899, The current
read-modify-write using read_state/read_state_path, get_guide_sync_state,
update_for_successful_agents, set_guide_sync_state and write_state is vulnerable
to lost updates; replace that sequence with a single call to state::update_state
(or update_state) that takes a closure which loads the current state, obtains
the guide sync state (get_guide_sync_state), applies
update_for_successful_agents to the guide state, sets it back with
set_guide_sync_state, and returns the modified state so the mutation is
performed under one atomic/state lock instead of separate read_state/write_state
calls.
crates/fleet-orchestrator/src/audit.rs-262-277 (1)

262-277: ⚠️ Potential issue | 🟠 Major

Guard agents_cap == 0 before the round-robin split.

When agents_cap is zero, num_agents also becomes zero and buckets[i % num_agents] panics. Since this is a public helper and the cap comes from config, it should reject zero explicitly instead of crashing mid-run.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/audit.rs` around lines 262 - 277, The function
partition_audit currently computes num_agents = agents_cap.min(files.len()) and
uses buckets[i % num_agents], which panics when agents_cap == 0; add an explicit
guard at the start of partition_audit to handle agents_cap == 0 before computing
num_agents (e.g., return Vec::new() or otherwise propagate an error), so the
code never reaches the round-robin loop that uses buckets[i % num_agents];
update references to agents_cap, num_agents, and buckets in partition_audit
accordingly.
crates/fleet-orchestrator/src/partitioner.rs-52-92 (1)

52-92: ⚠️ Potential issue | 🟠 Major

Fix the read_crates paths to be repo-relative.

These entries omit the crates/ prefix (gossip-coordination/src/, scanner-engine/src/, etc.). If agents consume read_crates as repo-relative directories, every non-empty mapping here points at a path that does not exist, so the enrichment/search step loses the code context it is supposed to read.

Based on learnings: key scope mappings in this repo use crates/... prefixes such as crates/gossip-coordination/src/, crates/scanner-engine/src/, and crates/gossip-contracts/src/identity/.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/partitioner.rs` around lines 52 - 92, The
returned crate paths from function section_crates are not repo-relative (they
lack the leading "crates/" prefix), so callers expecting repo-relative
directories (read_crates) will fail to find files; update every path string in
section_crates (e.g., "gossip-contracts/src/identity/",
"gossip-coordination/src/", "scanner-engine/src/", etc.) to include the
"crates/" prefix (e.g., "crates/gossip-contracts/src/identity/") so all
non-empty match arms return correct repo-relative directories.
crates/fleet-orchestrator/src/jetty.rs-383-399 (1)

383-399: ⚠️ Potential issue | 🟠 Major

Treat non-2xx poll responses as terminal failures.

poll_status() never checks the HTTP status code. A 401/404/500 here becomes either a parse error or Unknown, and poll_all_until_done() keeps waiting until the overall timeout because that trajectory never transitions to a terminal state. Permanent API failures should fail fast instead of burning the full poll window.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/jetty.rs` around lines 383 - 399, In
poll_status, currently the HTTP response body is parsed without checking the
response code, so non-2xx responses (401/404/500) get treated as parse errors or
Unknown and cause poll_all_until_done to wait until timeout; update poll_status
(in jetty.rs) to check the HTTP status on resp (e.g.,
resp.status().is_success()), and on non-success return an anyhow::Error (include
status code and trajectory_id in the message) so callers fail fast; ensure you
still deserialize and map body.status to AgentStatus only when the response is
successful.
crates/fleet-orchestrator/src/merge.rs-228-246 (1)

228-246: ⚠️ Potential issue | 🟠 Major

Use the requested base branch here instead of hardcoding origin/main.

The worktree is created from a caller-supplied base_branch, but the consolidated branch always starts from origin/main. Any run against a non-main target will merge onto the wrong base and can produce a PR that does not apply cleanly to the requested branch.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/merge.rs` around lines 228 - 246, The
consolidated branch is being created from a hardcoded "origin/main" in
merge_consolidated; change the function signature of merge_consolidated to
accept a caller-supplied base_branch (e.g. &str or &String) and use that value
when running git_ok checkout (replace the "origin/main" argument with the
provided base_branch), update any call sites to pass the caller's base branch,
and keep branch_name creation and the rest of merge logic unchanged so the
consolidated branch originates from the correct base branch.
crates/fleet-orchestrator/src/main.rs-1196-1204 (1)

1196-1204: ⚠️ Potential issue | 🟠 Major

Only mark audit files processed after a successful merged result.

Both paths write build_audit_state_update(&manifests, ...), so launch failures, poll timeouts, merge conflicts, and zero-branch runs still advance the audit ledger. On the next run filter_unchanged() will skip those files even though their audit never landed.

Also applies to: 1280-1286

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/main.rs` around lines 1196 - 1204, The code
updates the audit ledger unconditionally by calling
audit::build_audit_state_update and writing state (read_state, get_audit_state,
set_audit_state, write_state) even when runs fail/timeout/produce merge
conflicts or zero-branch results; change this so the build_audit_state_update +
state read/modify/write sequence only runs after a confirmed successful merged
audit result (the branch that represents a completed/merged audit), not in the
"no drift found" / failure paths; locate both occurrences (the block using
audit::build_audit_state_update and the duplicate at the other site around where
filter_unchanged is relevant) and move or gate the state update behind the
success/merged-result condition so only truly landed audits advance the ledger.
🟡 Minor comments (6)
docs/fleet-orchestrator/jetty-feature-proposals.md-25-33 (1)

25-33: ⚠️ Potential issue | 🟡 Minor

Add language tags to the unlabeled fenced blocks.

These examples are being flagged by markdownlint for missing info strings. Adding a language tag here will clear the lint noise and improve rendering.

Also applies to: 81-89, 386-395, 431-444

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/jetty-feature-proposals.md` around lines 25 - 33, Add
language info strings to the unlabeled fenced code blocks (the triple-backtick
blocks) so markdownlint stops flagging them; specifically update the block
containing the "CURRENT:" / "WITH ARTIFACT STORE:" example (the fenced block
that includes those labels) and the other unlabeled fenced blocks referenced in
the comment (the blocks around the examples starting with "CURRENT:" at the
other locations). Use a suitable language tag like ```text or ```txt for these
ASCII diagram blocks to avoid changing rendering.
docs/fleet-orchestrator/jetty-feature-proposals.md-249-252 (1)

249-252: ⚠️ Potential issue | 🟡 Minor

Drop the F10 shorthand here.

F10 is an external tracking label; readers cannot resolve it from this document alone. Spell out the failure mode directly in the prose. As per coding guidelines, {**/*.rs,docs/**/*.md,diagrams/**/*.md}: "Do NOT include tracking IDs" and "Comments and docs must stand alone."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/jetty-feature-proposals.md` around lines 249 - 252,
Replace the inline tracking label by removing "F10" from the phrase "Agentic
drift (the number one failure mode from research, F10)" and instead spell out
the failure mode for standalone clarity; e.g., change it to "Agentic drift — the
tendency for autonomous agents to pursue unintended or misaligned goals and
strategies as they optimize, which is the top failure mode identified in our
research." Ensure the sentence in the bullet list that currently contains
"Agentic drift (the number one failure mode from research, F10)" is updated
accordingly.
docs/fleet-orchestrator/deeper-research-fleet-primitives.md-65-69 (1)

65-69: ⚠️ Potential issue | 🟡 Minor

Add language info strings to these fenced blocks.

markdownlint is already flagging these fences as unlabeled, which keeps the doc noisy in CI and makes the examples harder to scan in rendered output.

Also applies to: 222-225, 315-321, 362-370, 584-587, 619-637

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/deeper-research-fleet-primitives.md` around lines 65
- 69, The fenced code blocks containing the agent/read/write examples (for
example the block with "Agent A  --write--> 
/fleet/{run_id}/artifacts/sec-04-a1/" etc.) are unlabeled; update each
triple-backtick fence to include a language info string such as "text" or
"console" (e.g., change ``` to ```text) to satisfy markdownlint and improve
rendering; apply the same fix to the other unlabeled fences mentioned (around
lines 222-225, 315-321, 362-370, 584-587, 619-637) so all examples use a
language info string consistently.
docs/fleet-orchestrator/research-fleet-native-primitives.md-263-269 (1)

263-269: ⚠️ Potential issue | 🟡 Minor

Fix the malformed Spot table row.

The header declares three columns (Component | Source File | Purpose), but Line 269 has four cells, so Markdown renderers will drop one of them.

Suggested fix
-| Auth | Clerk JWT | `spot/src/middleware.ts` | Forwarded as Bearer token to Mise |
+| Auth | `spot/src/middleware.ts` | Clerk JWT forwarded as Bearer token to Mise |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/research-fleet-native-primitives.md` around lines 263
- 269, The table row for "Auth | Clerk JWT | `spot/src/middleware.ts` |
Forwarded as Bearer token to Mise" has four cells but the header defines three;
update that row so it has exactly three pipe-separated cells matching the
columns "Component", "Source File", "Purpose" (e.g., make the Component "Auth
(Clerk JWT)", Source File "`spot/src/middleware.ts`", Purpose "Forwarded as
Bearer token to Mise") — locate the row mentioning Auth/Clerk JWT and
`spot/src/middleware.ts` and adjust the pipes accordingly.
docs/fleet-orchestrator/architecture.md-1-1 (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Add a [[scopes]] entry in docs/scope-map.toml for this new design doc.

Per coding guidelines, new design docs in docs/ must have a corresponding [[scopes]] entry to enable design-doc scope checks. The entry for docs/fleet-orchestrator/architecture.md is currently missing from docs/scope-map.toml. Add a mapping that covers the source directories this doc describes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/fleet-orchestrator/architecture.md` at line 1, Add a new [[scopes]]
entry to docs/scope-map.toml for the design doc
docs/fleet-orchestrator/architecture.md: create a [[scopes]] block referencing
the doc path (docs/fleet-orchestrator/architecture.md) and list the relevant
source directories the design covers (e.g., fleet-orchestrator source dirs under
src/ or pkg/); ensure the entry follows the existing scope-map.toml schema (use
the same keys used elsewhere such as doc, paths or dirs) so the design-doc scope
checks will include this new file.
crates/fleet-orchestrator/src/audit.rs-111-129 (1)

111-129: ⚠️ Potential issue | 🟡 Minor

Only skip findings/ directories, not arbitrary filenames containing findings.

path.contains("findings") also drops files like docs/findings-overview.md, even though the docstring says only findings/ subdirectories should be excluded. Matching path components here avoids silently shrinking the audit set.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/audit.rs` around lines 111 - 129, The current
walk_md_files uses path.contains("findings") which incorrectly skips any
filename with that substring; modify the logic in walk_md_files to treat path as
a filesystem path and check path components for a component equal to "findings"
(e.g., use std::path::Path::new(&path).components() or iterating
Path::components/ancestors) and continue only when a component exactly matches
"findings" so that files like "findings-overview.md" are not excluded; keep the
rest of the loop and the CandidateFile push behavior unchanged.
🧹 Nitpick comments (3)
.claude/skills/execute-review-findings/SKILL.md (1)

418-424: Define ID-based update semantics for review-findings.jsonl.

This section should explicitly require matching by finding id and rewriting the record, otherwise duplicate JSONL entries can skew downstream pattern metrics.

💡 Suggested wording tweak
 1. For each finding that was executed, update its `resolution` field:
+   - Match existing records by `id` and update in place (rewrite file atomically);
+     do not append a second record for the same finding.
    - `status`: "fixed", "wontfix", or "false_positive"
    - `action`: brief description of what was done
    - `was_true_positive`: true if the finding was a real issue, false if not
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/execute-review-findings/SKILL.md around lines 418 - 424,
Update the `.claude/skills/execute-review-findings/SKILL.md` section that
describes updating `.claude/review-findings.jsonl` to require ID-based rewrite
semantics: explicitly state that updates must match on the finding `id` and
replace the existing JSONL record (rather than appending a new entry) when
setting `resolution` (`status`, `action`, `was_true_positive`), to prevent
duplicate entries and preserve downstream metrics; mention the `id` key and the
`resolution` field as the matching/replacement points and instruct implementers
to rewrite the line for that `id` instead of adding a new JSONL line.
.claude/skills/detect-patterns/SKILL.md (1)

140-142: Consider clarifying FP rate calculation for unresolved findings.

The note says "unresolved findings default to FP rate 0" which is technically correct (0/0 = 0 via denominator check). However, this means patterns with all unresolved findings will appear as "low FP" candidates. The suggestion to "review and resolve existing findings first" is appropriate, but consider also noting that the FP rate reflects only resolved findings (TP + FP denominator).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/skills/detect-patterns/SKILL.md around lines 140 - 142, Update the
"All patterns noisy" note to explicitly state that the FP rate is calculated
only over resolved findings (FP / (TP + FP)) and that unresolved findings are
excluded (hence unresolved-only patterns evaluate as FP rate 0), so such
patterns may falsely appear as "low FP" candidates; modify the text around the
phrase "unresolved findings default to FP rate 0" to mention the resolved-only
denominator and add a brief recommendation to resolve findings or filter by
resolved count when assessing pattern FP rates.
.claude/hooks/review-pattern-check.sh (1)

191-197: Prefix match may over-match without trailing slash.

The scope check [[ "$mod_file" == "$scope_dir"* ]] will match src/foo.rs for scope src/, but also matches srcfoo.rs for scope src (no trailing slash). Consider normalizing scope dirs to include trailing slashes, or use a path-component-aware check.

♻️ Optional fix to ensure directory boundary matching
         for scope_dir in "${scope_arr[@]}"; do
             [[ -z "$scope_dir" ]] && continue
-            if [[ "$mod_file" == "$scope_dir"* ]]; then
+            # Ensure scope_dir ends with / for proper prefix matching
+            local normalized_scope="${scope_dir%/}/"
+            if [[ "$mod_file" == "$normalized_scope"* ]]; then
                 in_scope=1
                 break
             fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/hooks/review-pattern-check.sh around lines 191 - 197, The prefix
check can over-match (e.g., scope "src" matching "srcfoo.rs"); to fix, normalize
each scope_dir to have a trailing slash before the comparison (or otherwise
perform a path-component-aware check) so you only match directory boundaries:
ensure you transform scope_dir (from scope_arr) into a canonical form like
scope_dir_with_slash (e.g., append '/' if missing) and then test [[ "$mod_file"
== "${scope_dir_with_slash}"* ]]; update the loop using the existing variables
scope_arr, scope_dir, mod_file and set in_scope as before when a match is found.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cdac8716-65ee-4e32-8f13-ff8e08ff6740

📥 Commits

Reviewing files that changed from the base of the PR and between 3b16c9c and 85dabc9.

⛔ Files ignored due to path filters (1)
  • .fleet-state.json.lock is excluded by !**/*.lock
📒 Files selected for processing (48)
  • .beads/issues.jsonl
  • .claude/hooks/review-pattern-check.sh
  • .claude/review-findings.jsonl
  • .claude/review-rules.yaml
  • .claude/scripts/detect-patterns.sh
  • .claude/scripts/review-rule-lifecycle.sh
  • .claude/settings.json
  • .claude/skills/detect-patterns/SKILL.md
  • .claude/skills/execute-review-findings/SKILL.md
  • .claude/skills/manage-rules/SKILL.md
  • .claude/skills/review-dispatch/SKILL.md
  • .claude/skills/review-pipeline/SKILL.md
  • .fleet-state.json
  • .last-audit-sha
  • CLAUDE.md
  • Cargo.toml
  • audit-trajectories-2026-04-03.txt
  • audit-trajectories-2026-04-04.txt
  • audit-trajectories-audit-2026-04-04-2051.txt
  • audit-trajectories-audit-2026-04-04-2122.txt
  • audit-trajectories-audit-2026-04-04-2158.txt
  • audit-trajectories-audit-2026-04-04.txt
  • crates/fleet-orchestrator/Cargo.toml
  • crates/fleet-orchestrator/src/affected.rs
  • crates/fleet-orchestrator/src/audit.rs
  • crates/fleet-orchestrator/src/config.rs
  • crates/fleet-orchestrator/src/graph.rs
  • crates/fleet-orchestrator/src/jetty.rs
  • crates/fleet-orchestrator/src/lib.rs
  • crates/fleet-orchestrator/src/main.rs
  • crates/fleet-orchestrator/src/merge.rs
  • crates/fleet-orchestrator/src/partitioner.rs
  • crates/fleet-orchestrator/src/pr.rs
  • crates/fleet-orchestrator/src/state.rs
  • doc-rigor-trajectories-doc-rigor-2026-04-04-2051.txt
  • doc-rigor-trajectories-doc-rigor-2026-04-04-2122.txt
  • doc-rigor-trajectories-doc-rigor-2026-04-04-2158.txt
  • doc-rigor-trajectories-doc-rigor-2026-04-04.txt
  • docs/fleet-orchestrator/architecture.md
  • docs/fleet-orchestrator/deeper-research-fleet-primitives.md
  • docs/fleet-orchestrator/jetty-feature-proposals.md
  • docs/fleet-orchestrator/research-fleet-native-primitives.md
  • fleet/dependency_graph.json
  • run-audit-fleet.sh
  • run-doc-rigor-fleet.sh
  • run-guide-sync-fleet.sh
  • run-task-forge-fleet.sh
  • runbooks/task-forge-enrich.md

Comment on lines +424 to +433
// Prepare runbook for the entire batch. Individual agent context is
// embedded in the user message rather than per-agent system prompts,
// keeping the launch_wave API simple.
let runbook = substitute_runbook(
&runbook_template,
manifests.first().expect("at least one manifest"),
&config,
&base_sha,
&run_id,
);

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.

⚠️ Potential issue | 🔴 Critical

Render the guide-sync runbook per agent, not once from the first manifest.

substitute_runbook() injects manifest-specific fields like agent_id, section_id, write_set, and read_crates, but this code renders it once from manifests.first() and reuses that prompt for every launch. That gives later agents the first agent's scope/permissions, which can send them to the wrong chapters or wrong source directories.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/fleet-orchestrator/src/main.rs` around lines 424 - 433, The runbook is
currently rendered once using manifests.first() and reused for every agent,
causing all agents to inherit the first manifest's agent-specific fields;
instead, call substitute_runbook per agent using each manifest so each agent
gets its own personalized runbook. Locate where runbook is assigned (the
variable runbook using substitute_runbook with runbook_template and
manifests.first()) and move that substitute_runbook call into the loop/logic
that iterates or constructs each agent/manifest, passing the current manifest
(not manifests.first()) and assigning the result to a per-agent runbook variable
that you pass into the launch/agent creation path (keep references to
runbook_template, substitute_runbook, manifests, and the per-agent runbook
consistent). Ensure no shared runbook is reused across agents.

Comment thread run-audit-fleet.sh
Comment on lines +325 to +347
while [[ $ELAPSED -lt $POLL_TIMEOUT ]]; do
COMPLETED=0; RUNNING=0; FAILED=0
while IFS=' ' read -r aid tid wid; do
st=$(curl -s -H "Authorization: Bearer ${JETTY_API_KEY}" \
"${JETTY_HOST}/api/v1/db/trajectory/${COLLECTION}/design-doc-audit/${tid}" \
| jq -r '.status // "unknown"')
case "$st" in
completed) ((COMPLETED++)) ;;
failed|cancelled) ((FAILED++)) ;;
*) ((RUNNING++)) ;;
esac
done < "$TRAJ_FILE"

echo " [${ELAPSED}s] completed=${COMPLETED} running=${RUNNING} failed=${FAILED} / ${TOTAL}"

if [[ $((COMPLETED + FAILED)) -eq $TOTAL ]]; then
echo ""
echo "All agents finished. (completed=${COMPLETED} failed=${FAILED})"
break
fi

sleep $POLL_INTERVAL
((ELAPSED += POLL_INTERVAL))

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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Offending counter sites:"
rg -n '\(\(([A-Z_]+)\+\+\)\)' run-audit-fleet.sh

echo
echo "Demonstrating Bash behavior under set -e:"
bash -lc 'set -e; x=0; ((x++)); echo "unreachable"' || echo "post-increment exits under set -e"
bash -lc 'set -e; x=0; ((++x)); echo "pre-increment is safe"'

Repository: ahrav/Gossip-rs

Length of output: 532


Use pre-increment operators for all counters to avoid premature script exit under set -e.

Post-increment operators like ((COMPLETED++)) return exit status 1 when the variable is 0 (first increment), causing the script to exit immediately under set -e. This affects polling (lines 332–334) and merge operations (line 388). Switch to pre-increment syntax ((++COMPLETED)), ((++FAILED)), ((++RUNNING)), and ((++MERGED)).

Suggested fix
-      completed) ((COMPLETED++)) ;;
-      failed|cancelled) ((FAILED++)) ;;
-      *) ((RUNNING++)) ;;
+      completed) ((++COMPLETED)) ;;
+      failed|cancelled) ((++FAILED)) ;;
+      *) ((++RUNNING)) ;;

-    ((MERGED++))
+    ((++MERGED))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-audit-fleet.sh` around lines 325 - 347, The post-increment operations in
the polling loop and merge logic (variables COMPLETED, RUNNING, FAILED and
MERGED) can return a non-zero exit status when used under set -e; change all
uses of ((COMPLETED++)), ((FAILED++)), ((RUNNING++)) and ((MERGED++)) to the
pre-increment form ((++COMPLETED)), ((++FAILED)), ((++RUNNING)) and ((++MERGED))
so the arithmetic expansion returns zero and the script won’t exit prematurely;
update every occurrence (polling loop that reads TRAJ_FILE and the merge/count
section) accordingly.

Comment thread run-doc-rigor-fleet.sh
Comment on lines +60 to +63
# ── Ensure local repo is on the base branch ────────────────────────
git checkout "${BASE_BRANCH}" --quiet 2>/dev/null || \
git checkout -f "${BASE_BRANCH}" --quiet 2>/dev/null || true

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.

⚠️ Potential issue | 🔴 Critical

Don't force-checkout over a dirty worktree.

The git checkout -f fallback can silently discard local edits when the branch switch fails. This should stop and ask for a clean checkout instead of rewriting the user's working tree.

Also applies to: 496-497

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@run-doc-rigor-fleet.sh` around lines 60 - 63, The current fallback using "git
checkout -f \"${BASE_BRANCH}\"" can discard local changes; replace that forced
checkout behavior with a safety check: run "git status --porcelain" (or use "git
diff --quiet" and "git diff --cached --quiet") to detect an unclean worktree
before attempting to checkout and, if dirty, print a clear error instructing the
user to stash/commit/clean their changes and exit non‑zero instead of performing
a forced checkout; update the checkout logic around the references to
BASE_BRANCH and the two checkout attempts so the script aborts when the repo is
dirty and only performs the non‑destructive "git checkout \"${BASE_BRANCH}\"" if
the worktree is clean.

wave_delay_sec: 2,
state_file: ".fleet-state.json".to_owned(),
dep_graph_file: "fleet/dependency_graph.json".to_owned(),
runbook_file: "runbooks/guide-sync-partitioned.md".to_owned(),

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.

CRITICAL: Default runbook paths point at files that are not in this branch

FleetConfig::default() now hard-codes runbooks/guide-sync-partitioned.md and runbooks/guide-sync-coordinator.md, and AuditConfig::default() does the same for runbooks/design-doc-audit-partitioned.md, but none of those files exist in this repository. In a clean checkout both pipelines fail on the first read_to_string() before any agents launch.

// Prepare runbook for the entire batch. Individual agent context is
// embedded in the user message rather than per-agent system prompts,
// keeping the launch_wave API simple.
let runbook = substitute_runbook(

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: Every chapter agent gets the first manifest's system prompt

substitute_runbook() bakes agent_id, section_id, write_set, and read_crates into the runbook, but this call does it once with manifests.first() and then reuses that prompt for the whole wave. The per-agent user message only carries the section name and chapter count, so later agents never receive their own write scope or read scope.

c.config.host, c.config.collection, c.config.task_name, trajectory_id
);

let resp = c.http.get(&url).timeout(POLL_HTTP_TIMEOUT).send().await?;

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: Permanent poll failures fall back to "still running" until timeout

reqwest does not error on HTTP 4xx/5xx here. If Jetty returns a lookup or auth error, poll_status() either parses Unknown or bubbles a decode error, and poll_all_until_done() leaves the prior Running state in place until the global timeout. Check resp.status().is_success() before deserializing so hard failures fail fast.

let base_sha = get_base_sha()?;
tracing::info!(%base_sha, "resolved origin/main");

let current_blobs = audit::get_doc_blobs(&base_sha)?;

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: Audit state is recorded from the pre-merge doc snapshot

current_blobs is captured from base_sha before any audit edits land, and the same map is later fed into build_audit_state_update(). Any doc or diagram actually changed by the sweep gets its old blob SHA written back to state, so filter_unchanged() will re-queue it on the next run once the PR has been merged.

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.

1 participant