Skip to content

feat(beads): add Stage 5 resilience layer for beads task-store backend - #35

Merged
trillium merged 5 commits into
mainfrom
fm/beads-migration-s5-resilience
Aug 2, 2026
Merged

feat(beads): add Stage 5 resilience layer for beads task-store backend#35
trillium merged 5 commits into
mainfrom
fm/beads-migration-s5-resilience

Conversation

@trillium

@trillium trillium commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Intent

Implement Stage 5 of the beads-authority migration: a resilience layer (read-side local mirror with stale-labeled fallback, DEGRADED-vs-MISSING bootstrap diagnostics, and a durable write-queue-and-reconcile for beads writes) so the beads task-store backend degrades gracefully instead of wedging firstmate during a Dolt/beads-store outage, per data/beads-authority-migration-scout/report.md section 5. Must land before Stage 6 flips any home's live fleet to the beads backend.

What Changed

  • Added bin/fm-beads-resilience-lib.sh, a new library providing a read-side local mirror of the beads store (with staleness labeling and freshest-mirror lookup) and a durable write-queue-and-reconcile path for beads writes made during a store outage.
  • Wired the resilience layer into fm-bootstrap.sh (write-queue reconcile sweep and DEGRADED-vs-MISSING diagnostics), fm-session-start.sh, fm-fleet-snapshot.sh, and fm-teardown.sh (bead close now enqueues for durable replay on any write failure, not just when the store is fully unreachable), and fm-bead-stamp.sh.
  • Updated docs/configuration.md, docs/scripts.md, and .agents/skills/bootstrap-diagnostics/SKILL.md to document the mirror/write-queue behavior and corrected an overstated claim about how often the mirror is refreshed; removed an unused public helper function.
  • Added test coverage across tests/fm-beads-resilience-lib.test.sh, tests/fm-bead-stamp.test.sh, tests/fm-bootstrap.test.sh, tests/fm-fleet-snapshot-view.test.sh, tests/fm-session-start.test.sh, and tests/fm-teardown.test.sh, including the previously-missing case for a failed bead close during a write-only store outage.

Risk Assessment

✅ Low: The fix commit (e26322c) correctly and completely addresses all four round-1 findings: close_linked_bead now queues on any close failure (not just store-unreachable) with a fail-safe idempotent-close reconciliation check (fm_beads_close_already_applied) that avoids infinite re-queuing of already-closed/absent beads while still re-queuing genuine conflicts; the dead fm_beads_mirror_any_fresh helper and its tests were removed; and the header comment / docs/configuration.md were corrected to stop overstating mirror-refresh coverage. Each change is backed by new, well-targeted unit and integration tests covering reachable-store failure, unreachable-store failure, already-closed, bead-absent, and genuine-conflict scenarios, with no behavioral regressions or new risks introduced.

Testing

test

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 4 issues found → auto-fixed ✅
  • ⚠️ bin/fm-teardown.sh:368 - close_linked_bead() only enqueues a failed task close for durable replay when task list --limit 1 also fails. If reads succeed but the write path is down (a plausible split state for a Dolt store, e.g. a read replica staying up while the write leader is unreachable - exactly the outage class Stage 5 targets), the failed close is treated as "already closed or rejected" and silently dropped rather than queued. This is inconsistent with fm-bead-stamp.sh's sibling logic, which queues unconditionally on any write failure regardless of the list-based reachability probe. Impact is partly mitigated by fm-ledger.sh's separate stale-bead sweep, but a confirmed-landed task's bead close can be lost with no queued retry.
  • ⚠️ bin/fm-beads-resilience-lib.sh:10 - The header comment (and the mirrored text in docs/configuration.md around line 70) lists "teardown's re-evaluate-queue" as one of the reads that opportunistically refreshes the local mirror. No call to fm_beads_mirror_write exists anywhere in bin/fm-teardown.sh - the post-teardown "re-evaluate the queue" step (AGENTS.md section 7) is the firstmate agent itself running task list --ready in a later turn, which never goes through this library. The claim overstates how often the mirror is actually refreshed, which could lead a future reader (or the bootstrap-diagnostics skill) to over-trust the DEGRADED: mirror's freshness.
  • ⚠️ tests/fm-teardown.test.sh:178 - The new store-unreachable -> enqueue branch added to close_linked_bead (bin/fm-teardown.sh) has no test. add_beads_task_mock always exits 0, so tests/fm-teardown.test.sh never exercises a failing task close or verifies anything lands in state/.beads-write-queue. Every other new resilience-layer behavior in this change (fm-bead-stamp.sh, fm-bootstrap.sh, fm-fleet-snapshot.sh, fm-session-start.sh) got dedicated new test coverage; this one branch was missed.
  • ℹ️ bin/fm-beads-resilience-lib.sh:136 - fm_beads_mirror_any_fresh is documented and unit-tested but has no real caller - bin/fm-bootstrap.sh uses fm_beads_mirror_freshest_iso directly, which does its own equivalent freshness scan. Dead public function.

🔧 Fix: {"summary": "fix beads close-queue write-only-outage gap, prune dead helper, correct docs"}
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • test
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Summary by CodeRabbit

  • New Features

    • Added resilient task-store operation during outages using local read mirrors and durable queued writes.
    • Displays degraded or stale data with freshness information when live task data is unavailable.
    • Automatically replays queued updates after the task store recovers.
    • Teardown and stamping operations now preserve failed updates for later reconciliation.
  • Documentation

    • Updated configuration, diagnostics, and operational guidance for degraded task-store scenarios.
  • Tests

    • Added coverage for mirrors, stale fallbacks, queued writes, replay, and recovery behavior.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds a shared beads resilience layer with local read mirrors, durable write queues, replay handling, and close reconciliation. Session, snapshot, teardown, stamping, and bootstrap flows now use degraded fallbacks and deferred writes.

Changes

Beads resilience

Layer / File(s) Summary
Resilience library and queue mechanics
bin/fm-beads-resilience-lib.sh, docs/scripts.md, AGENTS.md, tests/fm-beads-resilience-lib.test.sh
Adds timestamped mirrors, freshness helpers, locked FIFO queues, replay, and idempotent close reconciliation.
Deferred beads writes
bin/fm-bead-stamp.sh, bin/fm-teardown.sh, tests/fm-bead-stamp.test.sh, tests/fm-teardown.test.sh
Queues failed stamp and close operations while preserving fail-open behavior.
Mirror-backed reads
bin/fm-session-start.sh, bin/fm-fleet-snapshot.sh, tests/fm-session-start.test.sh, tests/fm-fleet-snapshot-view.test.sh, .github/workflows/ci.yml
Uses fresh mirrors when live reads fail and reports stale-read metadata.
Bootstrap outage handling
bin/fm-bootstrap.sh, AGENTS.md, .agents/skills/bootstrap-diagnostics/SKILL.md, docs/configuration.md
Adds degraded and missing diagnostics, queue reconciliation, and detect-only handling for the new mutating sweep.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SessionStart
  participant fm_beads_resilience_lib
  participant BeadsStore
  participant LocalMirror
  participant WriteQueue

  SessionStart->>fm_beads_resilience_lib: Read beads backlog
  fm_beads_resilience_lib->>BeadsStore: Query in-flight and queued tasks
  BeadsStore-->>fm_beads_resilience_lib: Live results or failure
  fm_beads_resilience_lib->>LocalMirror: Store successful read results
  fm_beads_resilience_lib->>LocalMirror: Read fresh mirror after failure
  SessionStart-->>SessionStart: Render current or stale sections
  SessionStart->>fm_beads_resilience_lib: Reconcile queued writes
  fm_beads_resilience_lib->>WriteQueue: Drain queued operations
  fm_beads_resilience_lib->>BeadsStore: Replay task writes
  BeadsStore-->>fm_beads_resilience_lib: Success or failure
  fm_beads_resilience_lib->>WriteQueue: Remove successes and retain failures
Loading

Possibly related PRs

Suggested reviewers: kunchenguid

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 48.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: adding the Stage 5 beads task-store resilience layer.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fm/beads-migration-s5-resilience

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@trillium
trillium force-pushed the fm/beads-migration-s5-resilience branch from 822537d to 99a3955 Compare August 2, 2026 07:48
…ckend

Adds a read-side local mirror and a durable write-queue so a Dolt/beads-
store outage degrades gracefully instead of wedging firstmate, per the
beads-authority-migration design (data/beads-authority-migration-scout/
report.md section 5). Must land before any home flips its live fleet to
the beads backend (Stage 6).

- bin/fm-beads-resilience-lib.sh: new library owning the mirror
  (write/read/age/freshness/timestamp) and the durable write queue
  (enqueue/count/reconcile), reusing fm-wake-lib.sh's append-only-log and
  locking patterns.
- fm-session-start.sh and fm-fleet-snapshot.sh: a successful beads read
  opportunistically refreshes its mirror; a failed read falls back to a
  fresh mirror with explicit stale labeling, never presented as current.
- fm-bootstrap.sh: MISSING: escalates only when both the live store and
  every mirror are unusable; otherwise reports DEGRADED: naming the
  mirror's timestamp. Bootstrap's mutating sweep also reconciles the
  write queue against a recovered store, no new polling loop.
- fm-bead-stamp.sh and fm-teardown.sh: a beads write that fails because
  the store is unreachable is queued for replay instead of only warned
  and lost; the fail-open posture on other gaps (no CLI, bead not found)
  is unchanged.
- docs/configuration.md, AGENTS.md, and the bootstrap-diagnostics skill
  document the new state files, the DEGRADED: diagnostic, and the
  mechanics summary, with docs/configuration.md as the single owner.

Tests: new tests/fm-beads-resilience-lib.test.sh and
tests/fm-bead-stamp.test.sh unit-test the library and the write-queue-
on-outage path directly; tests/fm-session-start.test.sh and
tests/fm-fleet-snapshot-view.test.sh cover mirror-refresh-on-read and
stale-mirror-fallback-labeling through both caller shapes;
tests/fm-bootstrap.test.sh covers the full MISSING:/DEGRADED: matrix
and write-queue reconciliation on bootstrap.
@trillium
trillium force-pushed the fm/beads-migration-s5-resilience branch from 9e8c830 to 190aa20 Compare August 2, 2026 08:46

@coderabbitai coderabbitai 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.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
bin/fm-session-start.sh (1)

300-320: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

The mixed-section fallback prints mirror data with no stale label.

Consider this state: the in-flight live read fails and the in-flight mirror is fresh, so inflight_ok=1 and out_inflight now holds mirror content. The queued live read fails and no fresh queued mirror exists, so queued_ok=0. The condition at line 300 is false, and control reaches the else branch.

Line 315 then prints $out_inflight raw. That content came from the mirror, and $inflight_stale_since is never used on this path. The digest presents stale mirror data as if it were a current read, under the generic line beads task listing failed; falling back to title-line rendering.

The library header at lines 25-27 of bin/fm-beads-resilience-lib.sh states that a mirror file is never presented as current and that every caller falling back to one must label the output as a stale mirror. docs/configuration.md line 73 repeats that rule.

Label each section independently, so the stale prefix follows the data rather than the all-or-nothing branch.

🐛 Proposed restructure
+  print_beads_section() { # <label> <ok> <stale-since> <output>
+    if [ "$2" -eq 1 ] && [ -n "$3" ]; then
+      printf '(stale mirror, beads store unreachable since %s) %s, as of last successful read:\n' "$3" "$1"
+    elif [ "$2" -eq 1 ]; then
+      printf '## %s\n' "$1"
+    else
+      printf '## %s (read failed)\n' "$1"
+    fi
+    printf '%s\n' "$4"
+  }
+
   if [ "$inflight_ok" -eq 1 ] && [ "$queued_ok" -eq 1 ]; then
-    if [ -n "$inflight_stale_since" ]; then
-      printf '(stale mirror, beads store unreachable since %s) In flight, as of last successful read:\n' "$inflight_stale_since"
-    else
-      printf '## In flight\n'
-    fi
-    printf '%s\n' "$out_inflight"
-    if [ -n "$queued_stale_since" ]; then
-      printf '(stale mirror, beads store unreachable since %s) Queued, as of last successful read:\n' "$queued_stale_since"
-    else
-      printf '## Queued\n'
-    fi
-    printf '%s\n' "$out_queued"
+    print_beads_section "In flight" "$inflight_ok" "$inflight_stale_since" "$out_inflight"
+    print_beads_section "Queued" "$queued_ok" "$queued_stale_since" "$out_queued"
   else
     printf 'beads task listing failed; falling back to title-line rendering.\n'
-    printf '%s\n' "$out_inflight"
-    printf '%s\n' "$out_queued"
+    print_beads_section "In flight" "$inflight_ok" "$inflight_stale_since" "$out_inflight"
+    print_beads_section "Queued" "$queued_ok" "$queued_stale_since" "$out_queued"
     if [ -f "$path" ]; then
       print_backlog_manual_compact "$path" "fallback"
     fi
   fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-session-start.sh` around lines 300 - 320, Update the fallback branch
around the main `inflight_ok`/`queued_ok` condition to render the in-flight and
queued sections independently, using each section’s stale marker
(`inflight_stale_since` or `queued_stale_since`) whenever its output came from a
mirror. Ensure mirror content is never printed raw or presented as current,
while preserving title-line rendering for sections that lack usable task-list
data.
🧹 Nitpick comments (5)
bin/fm-beads-resilience-lib.sh (2)

110-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

fm_beads_mirror_write does not need the wake lib.

Line 113 already falls back to echo $$ when fm_current_pid is absent. The only use of the wake lib here is that PID helper. Sourcing it on every mirror write adds a source cost and creates $STATE through the lib's own mkdir -p, which the header at lines 66-72 describes as a side effect worth avoiding. Consider dropping line 110 and relying on $$.

♻️ Proposed simplification
 fm_beads_mirror_write() { # <view> <raw-output>
   local view=$1 raw=$2 path tmp
-  fm_beads_require_wake_lib
   path=$(fm_beads_mirror_view_path "$view") || return 1
   mkdir -p "$(dirname "$path")" 2>/dev/null || true
-  tmp="${path}.tmp.$(fm_current_pid 2>/dev/null || echo $$)"
+  tmp="${path}.tmp.$$"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-beads-resilience-lib.sh` around lines 110 - 113, Remove the
fm_beads_require_wake_lib call from fm_beads_mirror_write and retain the
existing fm_current_pid fallback to $$ when constructing tmp, so mirror writes
no longer source the wake library or trigger its side effects.

152-167: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

fm_beads_mirror_freshest_iso re-reads each mirror three times.

For each view the function calls fm_beads_mirror_fresh, then fm_beads_mirror_age_seconds, then fm_beads_mirror_timestamp_iso for the winner. Each of those calls fm_beads_mirror_read, which runs two jq processes. With three views this is up to 14 jq invocations on the bootstrap detect path. One fm_beads_mirror_age_seconds call per view is enough, because the freshness test is just age <= max_age.

♻️ Proposed single-pass version
 fm_beads_mirror_freshest_iso() { # [<max-age-seconds>] -> ISO-8601 timestamp of
   # whichever known view has the freshest fresh mirror; returns 1 if none fresh.
-  local max_age=${1:-$FM_BEADS_MIRROR_MAX_AGE} view best_age best_view age
+  local max_age=${1:-$FM_BEADS_MIRROR_MAX_AGE} view best_age best_written age
   best_age=
-  best_view=
+  best_written=
   for view in $FM_BEADS_MIRROR_VIEWS; do
-    fm_beads_mirror_fresh "$view" "$max_age" || continue
     age=$(fm_beads_mirror_age_seconds "$view") || continue
+    [ "$age" -le "$max_age" ] || continue
     if [ -z "$best_age" ] || [ "$age" -lt "$best_age" ]; then
       best_age=$age
-      best_view=$view
+      best_written=$FM_BEADS_MIRROR_WRITTEN_AT
     fi
   done
-  [ -n "$best_view" ] || return 1
-  fm_beads_mirror_timestamp_iso "$best_view"
+  [ -n "$best_written" ] || return 1
+  fm_beads_epoch_to_iso "$best_written"
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-beads-resilience-lib.sh` around lines 152 - 167, Update
fm_beads_mirror_freshest_iso to perform a single fm_beads_mirror_age_seconds
read per view, compare the returned age directly against max_age, and track the
freshest eligible view without calling fm_beads_mirror_fresh. Preserve the
existing no-fresh-view return status and convert the selected best_view to ISO
via fm_beads_mirror_timestamp_iso.
bin/fm-bead-stamp.sh (2)

50-63: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The three enqueue calls are duplicated between the outage path and the per-write path.

Lines 39-41 and lines 52, 57, and 62 pass identical arguments. If the reason string or the state names change, both copies must change together. Extract one helper per operation, or one helper that takes the operation name.

♻️ Proposed extraction
+queue_dispatch_writes() {
+  fm_beads_write_enqueue "$BEADS_ID" "dispatch=sent" set-state "$BEADS_ID" dispatch=sent --reason "dispatched: agent=$AGENT" || true
+  fm_beads_write_enqueue "$BEADS_ID" "lifecycle=sent" set-state "$BEADS_ID" lifecycle=sent --reason "dispatched: agent=$AGENT" || true
+  fm_beads_write_enqueue "$BEADS_ID" "assign $AGENT" assign "$BEADS_ID" "$AGENT" || true
+}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-bead-stamp.sh` around lines 50 - 63, Deduplicate the retry enqueue
commands used by the outage path and the per-write failure handling in the bead
stamping flow. Extract shared helpers for the dispatch state update, lifecycle
state update, and assignment operation (or a single parameterized helper), then
update both paths to call the shared logic so each operation’s command and
reason arguments have one source of truth.

37-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Queued writes for a nonexistent bead retry on every bootstrap.

The live path checks task show "$BEADS_ID" at line 45 before writing, so a bead that does not exist is skipped. This outage path cannot make that check, which is correct. The consequence is that if $BEADS_ID names a bead that never existed, three queued writes fail replay on every later bootstrap, and each one prints a BEADS_WRITE_QUEUE: replay failed line. Only close has the reconciliation exception in fm_beads_write_queue_reconcile.

Consider a bounded attempt count or an age cap on queue entries so a permanently inapplicable write leaves the queue with one clear report instead of repeating without end. The queued_at field the enqueue already records supports an age cap directly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-bead-stamp.sh` around lines 37 - 43, Add bounded retry or age-cap
handling for the three outage-path entries created by fm_beads_write_enqueue,
using their existing queued_at timestamps so writes for nonexistent beads do not
replay indefinitely. Update fm_beads_write_queue_reconcile to discard or mark
permanently inapplicable entries after the limit and emit one clear report,
while preserving normal retries for eligible queued writes and the existing
close reconciliation exception.
tests/fm-beads-resilience-lib.test.sh (1)

181-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the two untested queue paths.

The suite covers unreachable, partial, full recovery, and the three close-idempotency cases. Two paths in fm_beads_write_queue_reconcile have no coverage:

  • The merge branch at bin/fm-beads-resilience-lib.sh lines 246-248, reached when a write is enqueued while a drain is in flight. Append a line to the queue file directly between the drain and the merge, then assert both the re-queued failure and the new write survive.
  • A malformed queue record, which currently re-queues forever. Append a non-JSON line and assert the intended handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-beads-resilience-lib.test.sh` around lines 181 - 216, Add tests in
fm-beads-resilience-lib.test.sh covering the missing
fm_beads_write_queue_reconcile paths: during an in-flight drain, append a queue
record between drain and merge and verify both the re-queued failure and newly
appended write remain; also append a malformed non-JSON queue line and assert
the intended handling, including its queue-retention or removal behavior and
reconciliation result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.agents/skills/bootstrap-diagnostics/SKILL.md:
- Around line 60-64: Update the BEADS_WRITE_QUEUE diagnostics section to
document the exact “reconciliation clean, queue empty” message emitted by the
resilience library, identifying it as informational and requiring no action.
Fold it into the existing successful reconciliation entry if appropriate, while
preserving the documented handling of the other queue messages.

In `@AGENTS.md`:
- Line 114: Update the description of .beads-write-queue and
.beads-write-queue.lock to cover any beads write that could not be applied,
including isolated store rejections rather than only store outages; preserve the
existing replay and manual-edit warnings.

In `@bin/fm-beads-resilience-lib.sh`:
- Around line 225-241: Handle malformed queue records separately in the
reconciliation loop around the argv parsing and task invocation: when the JSON
record cannot be parsed or yields no valid argv, log a distinct
BEADS_WRITE_QUEUE message and drop it or move it to a quarantine file instead of
appending it to remaining or counting it as a replay failure. Also update argv
extraction to preserve embedded newlines by emitting NUL-separated values from
jq and reading them with read -r -d '' so each queued argument remains a single
argument.
- Around line 193-199: Update bin/fm-beads-resilience-lib.sh lines 193-199 in
fm_beads_close_already_applied to capture task show’s exit status and return 1
when the lookup fails; only a successful lookup showing an absent bead or
status=closed should return success. Update
tests/fm-beads-resilience-lib.test.sh lines 165-179 so the bd-gone fixture emits
the real task CLI unknown-id signal and add a fourth failure-reason case
verifying the queued close remains queued.
- Around line 244-254: Update the queue merge logic around fm_lock_acquire_wait
to compute the merge temporary path once, then preserve remaining on any cat or
mv failure. Only remove $remaining after the merged file has been successfully
moved into $queue; otherwise retain it and report the failed merge, ensuring
re-queued writes are not lost.

In `@bin/fm-bootstrap.sh`:
- Around line 911-921: Complete the truncated DEGRADED beads messages in
bin/fm-bootstrap.sh around the task CLI and task list checks: end the CLI
message with the condition that the CLI is installed, and the store message with
the condition that the store is reachable. Update the corresponding quoted
formats in .agents/skills/bootstrap-diagnostics/SKILL.md to exactly match the
corrected emitted strings.

In `@bin/fm-fleet-snapshot.sh`:
- Around line 473-478: Guard the stale_since_json assignment in the
fm_beads_mirror_fresh fleet branch so a failed fm_beads_mirror_timestamp_iso
call cannot emit an empty string. Preserve stale=true, but set stale_since_json
to a valid timestamp when available or the existing JSON null representation on
failure.

In `@bin/fm-session-start.sh`:
- Around line 278-298: Update the task list handling in the inflight and queued
flows to keep stderr diagnostics separate from the listing captured in
out_inflight and out_queued. Remove the 2>&1 merging from both task list
invocations, while preserving rc_inflight/rc_queued status handling and ensuring
fm_beads_mirror_write receives stdout-only content.

In `@bin/fm-teardown.sh`:
- Around line 352-371: Update close_linked_bead so the missing-task-CLI branch
calls fm_beads_write_enqueue with the same bead ID, close payload, and reason
used by the failed task close path before returning successfully. Preserve the
warning and fail-open behavior, ensuring the confirmed close remains queued for
later reconciliation when task is unavailable.

In `@docs/configuration.md`:
- Line 59: Update the stale listing-fallback statement near the beads resilience
documentation to describe per-section fallback behavior: use a fresh per-view
mirror when a beads read fails, and use the markdown title-line fallback only
when that section has no usable mirror. Keep the wording consistent with the
authoritative behavior documented in the “Beads resilience layer” section and
remove the contradictory whole-list fallback claim.

In `@tests/fm-bootstrap.test.sh`:
- Around line 302-307: Update write_beads_mirror_fixture to create the mirror
through the authoritative fm_beads_mirror_write helper instead of constructing
.beads-mirror-ready.json with jq. Preserve the existing fixture output, and
modify only the written_at field afterward when age_seconds is used to create a
stale mirror.
- Around line 333-369: Update both DEGRADED assertions in the missing-CLI and
unreachable-store test cases to require the full “using local mirror from
<ISO-8601 timestamp>” detail, not only the diagnostic prefix. Use a regex or
equivalent assertion that validates the timestamp format while preserving the
existing scenario-specific message checks.

---

Outside diff comments:
In `@bin/fm-session-start.sh`:
- Around line 300-320: Update the fallback branch around the main
`inflight_ok`/`queued_ok` condition to render the in-flight and queued sections
independently, using each section’s stale marker (`inflight_stale_since` or
`queued_stale_since`) whenever its output came from a mirror. Ensure mirror
content is never printed raw or presented as current, while preserving
title-line rendering for sections that lack usable task-list data.

---

Nitpick comments:
In `@bin/fm-bead-stamp.sh`:
- Around line 50-63: Deduplicate the retry enqueue commands used by the outage
path and the per-write failure handling in the bead stamping flow. Extract
shared helpers for the dispatch state update, lifecycle state update, and
assignment operation (or a single parameterized helper), then update both paths
to call the shared logic so each operation’s command and reason arguments have
one source of truth.
- Around line 37-43: Add bounded retry or age-cap handling for the three
outage-path entries created by fm_beads_write_enqueue, using their existing
queued_at timestamps so writes for nonexistent beads do not replay indefinitely.
Update fm_beads_write_queue_reconcile to discard or mark permanently
inapplicable entries after the limit and emit one clear report, while preserving
normal retries for eligible queued writes and the existing close reconciliation
exception.

In `@bin/fm-beads-resilience-lib.sh`:
- Around line 110-113: Remove the fm_beads_require_wake_lib call from
fm_beads_mirror_write and retain the existing fm_current_pid fallback to $$ when
constructing tmp, so mirror writes no longer source the wake library or trigger
its side effects.
- Around line 152-167: Update fm_beads_mirror_freshest_iso to perform a single
fm_beads_mirror_age_seconds read per view, compare the returned age directly
against max_age, and track the freshest eligible view without calling
fm_beads_mirror_fresh. Preserve the existing no-fresh-view return status and
convert the selected best_view to ISO via fm_beads_mirror_timestamp_iso.

In `@tests/fm-beads-resilience-lib.test.sh`:
- Around line 181-216: Add tests in fm-beads-resilience-lib.test.sh covering the
missing fm_beads_write_queue_reconcile paths: during an in-flight drain, append
a queue record between drain and merge and verify both the re-queued failure and
newly appended write remain; also append a malformed non-JSON queue line and
assert the intended handling, including its queue-retention or removal behavior
and reconciliation result.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 273435ca-b596-481a-9739-6bdac3094a47

📥 Commits

Reviewing files that changed from the base of the PR and between 469eaf5 and 8b4f7d1.

📒 Files selected for processing (19)
  • .agents/skills/bootstrap-diagnostics/SKILL.md
  • .github/workflows/ci.yml
  • AGENTS.md
  • bin/fm-bead-stamp.sh
  • bin/fm-beads-resilience-lib.sh
  • bin/fm-bootstrap.sh
  • bin/fm-fleet-snapshot.sh
  • bin/fm-session-start.sh
  • bin/fm-teardown.sh
  • docs/configuration.md
  • docs/scripts.md
  • tests/fm-backend.test.sh
  • tests/fm-bead-stamp.test.sh
  • tests/fm-beads-resilience-lib.test.sh
  • tests/fm-bootstrap.test.sh
  • tests/fm-fleet-snapshot-view.test.sh
  • tests/fm-gotmp.test.sh
  • tests/fm-session-start.test.sh
  • tests/fm-teardown.test.sh

Comment on lines +60 to +64
- `BEADS_WRITE_QUEUE: reconciled queued write for <id> (<description>)` - a beads write that failed during an earlier outage (dispatch stamp, assignment, or a task-close) replayed cleanly against the recovered store on this bootstrap's mutating sweep; no action needed, it is reported only so the recovery is visible.
`(bead already closed)` on a queued close means the bead was already closed - by the outage's original close attempt landing after all, or by someone else - and firstmate reconciled instead of retrying it forever.
- `BEADS_WRITE_QUEUE: task CLI not found, N write(s) remain queued` / `BEADS_WRITE_QUEUE: store still unreachable, N write(s) remain queued` - the beads store outage is still ongoing; treat the same as an unresolved `DEGRADED:`/`MISSING:` beads line above rather than a new problem, and expect the queue to keep draining on later bootstraps once the store recovers.
- `BEADS_WRITE_QUEUE: replay failed for <id> (<description>); re-queued` - a queued write hit a genuine replay failure (not just an unreachable store) and was left queued for the next attempt.
Investigate if the same id keeps failing to replay across multiple bootstraps rather than treating a one-off as actionable.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

One emitted line has no entry: reconciliation clean, queue empty.

bin/fm-beads-resilience-lib.sh line 257 emits BEADS_WRITE_QUEUE: reconciliation clean, queue empty. Line 5 of this file lists BEADS_WRITE_QUEUE as an actionable trigger, so an agent that sees this line looks here and finds no matching entry. The four other formats match the library exactly.

Add a short entry stating that the line is informational and needs no action, or fold it into the line 60 entry.

🧰 Tools
🪛 SkillSpector (2.4.4)

[warning] 25: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.

Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.

(Excessive Agency (EA2))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.agents/skills/bootstrap-diagnostics/SKILL.md around lines 60 - 64, Update
the BEADS_WRITE_QUEUE diagnostics section to document the exact “reconciliation
clean, queue empty” message emitted by the resilience library, identifying it as
informational and requiring no action. Fold it into the existing successful
reconciliation entry if appropriate, while preserving the documented handling of
the other queue messages.

Comment thread AGENTS.md
x-poll.error x-poll.claim-error generated X-mode relay and offer-claim diagnostic dedupe markers
.wake-queue durable queued wakes: epoch<TAB>seq<TAB>kind<TAB>key<TAB>payload
.beads-mirror-<view>.json generated read-side mirror of the beads task store, refreshed opportunistically by ordinary reads (session start, fleet snapshot); never a second authority (`docs/configuration.md` "Backlog backend"; `bin/fm-beads-resilience-lib.sh`)
.beads-write-queue .beads-write-queue.lock durable pending-writes log and its lock for a beads write attempted during a store outage, replayed by bootstrap once the store recovers; never touch by hand

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Describe all queued write failures.

Line 114 states that the queue contains writes attempted during a store outage. A reachable store can also reject one write, and that write is queued for replay. Replace this condition with wording such as “a beads write that could not be applied.”

As per coding guidelines, use authoritative scripts, skills, and documentation as the owners of schemas and procedures; do not reimplement their behavior independently.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@AGENTS.md` at line 114, Update the description of .beads-write-queue and
.beads-write-queue.lock to cover any beads write that could not be applied,
including isolated store rejections rather than only store outages; preserve the
existing replay and manual-edit warnings.

Source: Coding guidelines

Comment on lines +193 to +199
fm_beads_close_already_applied() { # <task-id> - true if the bead is absent or already closed
local id=$1 out status
out=$(task show "$id" --json 2>/dev/null)
[ -n "$out" ] || return 0
status=$(printf '%s' "$out" | jq -r '.status // empty' 2>/dev/null)
[ "$status" = closed ]
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

A failed task show is treated as an absent bead, and the test fixture encodes that assumption. fm_beads_close_already_applied discards the exit status of task show <id> --json and reports "already applied" whenever the output is empty. A transient per-bead error, a permission error, or an unrecognized --json flag then causes fm_beads_write_queue_reconcile to drop a queued close from the durable queue and report it as reconciled. The confirmed-landed close is lost.

  • bin/fm-beads-resilience-lib.sh#L193-L199: capture the exit status of task show and return 1 when the lookup fails, so the queued close stays queued; treat only a successful lookup reporting an absent bead or status=closed as applied.
  • tests/fm-beads-resilience-lib.test.sh#L165-L179: change the bd-gone case to emit the signal the real task CLI produces for an unknown id, and add a fourth case where show fails for another reason and the queued close must remain queued.
📍 Affects 2 files
  • bin/fm-beads-resilience-lib.sh#L193-L199 (this comment)
  • tests/fm-beads-resilience-lib.test.sh#L165-L179
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-beads-resilience-lib.sh` around lines 193 - 199, Update
bin/fm-beads-resilience-lib.sh lines 193-199 in fm_beads_close_already_applied
to capture task show’s exit status and return 1 when the lookup fails; only a
successful lookup showing an absent bead or status=closed should return success.
Update tests/fm-beads-resilience-lib.test.sh lines 165-179 so the bd-gone
fixture emits the real task CLI unknown-id signal and add a fourth
failure-reason case verifying the queued close remains queued.

Comment on lines +225 to +241
while IFS= read -r line; do
[ -n "$line" ] || continue
id=$(printf '%s' "$line" | jq -r '.task_id // "?"' 2>/dev/null)
desc=$(printf '%s' "$line" | jq -r '.description // ""' 2>/dev/null)
local -a argv=()
while IFS= read -r arg; do argv+=("$arg"); done < <(printf '%s' "$line" | jq -r '.argv[]? // empty' 2>/dev/null)
if [ "${#argv[@]}" -gt 0 ] && task "${argv[@]}" >/dev/null 2>&1; then
replayed=$((replayed + 1))
echo "BEADS_WRITE_QUEUE: reconciled queued write for $id ($desc)"
elif [ "${argv[0]:-}" = close ] && fm_beads_close_already_applied "$id"; then
replayed=$((replayed + 1))
echo "BEADS_WRITE_QUEUE: reconciled queued write for $id ($desc) (bead already closed)"
else
failed=$((failed + 1))
printf '%s\n' "$line" >>"$remaining"
echo "BEADS_WRITE_QUEUE: replay failed for $id ($desc); re-queued"
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

A malformed queue line is re-queued forever and keeps reconcile red.

If jq -r '.argv[]?' yields nothing for a line, argv stays empty. Line 231 then fails its length test, line 234 sees ${argv[0]:-} as empty, and the line goes to the else branch. The line is written back to $remaining and counted as a failure. The same thing happens on every later bootstrap. One corrupt or truncated record therefore blocks the queue from ever reporting clean, and it emits replay failed for ? () on each session start.

Handle an unparsable record separately: drop it with a distinct BEADS_WRITE_QUEUE: line so an operator can see it, or move it to a quarantine file instead of the live queue.

Also note that line 230 splits argv on newlines. A queued argument that contains a newline replays as several arguments. The current call sites pass single-line reasons, so this is latent, but jq -j with NUL separators and read -r -d '' removes the assumption.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-beads-resilience-lib.sh` around lines 225 - 241, Handle malformed
queue records separately in the reconciliation loop around the argv parsing and
task invocation: when the JSON record cannot be parsed or yields no valid argv,
log a distinct BEADS_WRITE_QUEUE message and drop it or move it to a quarantine
file instead of appending it to remaining or counting it as a replay failure.
Also update argv extraction to preserve embedded newlines by emitting
NUL-separated values from jq and reading them with read -r -d '' so each queued
argument remains a single argument.

Comment on lines +244 to +254
fm_lock_acquire_wait "$FM_BEADS_WRITE_QUEUE_LOCK"
if [ -s "$remaining" ]; then
if [ -e "$queue" ]; then
cat "$remaining" "$queue" >"$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)" &&
mv "$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)" "$queue"
else
mv "$remaining" "$queue"
fi
fi
rm -f "$remaining" "$drained" 2>/dev/null
fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

A failed merge loses the re-queued writes.

Lines 247-248 build $queue.merge.<pid> and then move it over $queue. If cat or mv fails, the && chain stops, the merge temp file stays behind, and line 253 deletes $remaining. The writes that failed replay then exist only in the leaked merge file, and the live queue keeps just the entries enqueued during the drain. The failed writes are dropped without any report.

Guard the merge and keep $remaining until the merge lands.

Line 247 and line 248 also evaluate $(fm_current_pid 2>/dev/null || echo $$) twice for what must be the same path. Compute it once.

🐛 Proposed fix
   fm_lock_acquire_wait "$FM_BEADS_WRITE_QUEUE_LOCK"
   if [ -s "$remaining" ]; then
     if [ -e "$queue" ]; then
-      cat "$remaining" "$queue" >"$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)" &&
-        mv "$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)" "$queue"
+      local merge="$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)"
+      if cat "$remaining" "$queue" >"$merge" 2>/dev/null && mv "$merge" "$queue" 2>/dev/null; then
+        rm -f "$remaining" 2>/dev/null
+      else
+        rm -f "$merge" 2>/dev/null
+        echo "BEADS_WRITE_QUEUE: could not merge re-queued writes back; they remain in $remaining"
+        fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
+        rm -f "$drained" 2>/dev/null
+        return 1
+      fi
     else
-      mv "$remaining" "$queue"
+      mv "$remaining" "$queue" 2>/dev/null || {
+        echo "BEADS_WRITE_QUEUE: could not restore re-queued writes; they remain in $remaining"
+        fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
+        rm -f "$drained" 2>/dev/null
+        return 1
+      }
     fi
   fi
   rm -f "$remaining" "$drained" 2>/dev/null
   fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fm_lock_acquire_wait "$FM_BEADS_WRITE_QUEUE_LOCK"
if [ -s "$remaining" ]; then
if [ -e "$queue" ]; then
cat "$remaining" "$queue" >"$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)" &&
mv "$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)" "$queue"
else
mv "$remaining" "$queue"
fi
fi
rm -f "$remaining" "$drained" 2>/dev/null
fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
fm_lock_acquire_wait "$FM_BEADS_WRITE_QUEUE_LOCK"
if [ -s "$remaining" ]; then
if [ -e "$queue" ]; then
local merge="$queue.merge.$(fm_current_pid 2>/dev/null || echo $$)"
if cat "$remaining" "$queue" >"$merge" 2>/dev/null && mv "$merge" "$queue" 2>/dev/null; then
rm -f "$remaining" 2>/dev/null
else
rm -f "$merge" 2>/dev/null
echo "BEADS_WRITE_QUEUE: could not merge re-queued writes back; they remain in $remaining"
fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
rm -f "$drained" 2>/dev/null
return 1
fi
else
mv "$remaining" "$queue" 2>/dev/null || {
echo "BEADS_WRITE_QUEUE: could not restore re-queued writes; they remain in $remaining"
fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
rm -f "$drained" 2>/dev/null
return 1
}
fi
fi
rm -f "$remaining" "$drained" 2>/dev/null
fm_lock_release "$FM_BEADS_WRITE_QUEUE_LOCK"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-beads-resilience-lib.sh` around lines 244 - 254, Update the queue
merge logic around fm_lock_acquire_wait to compute the merge temporary path
once, then preserve remaining on any cat or mv failure. Only remove $remaining
after the merged file has been successfully moved into $queue; otherwise retain
it and report the failed merge, ensuring re-queued writes are not lost.

Comment thread bin/fm-session-start.sh
Comment on lines 278 to +298
out_inflight=$(task list --label "$label" --status in_progress,blocked --limit "$BACKLOG_LIMIT" 2>&1)
rc_inflight=$?
if [ "$rc_inflight" -eq 0 ]; then
fm_beads_mirror_write inflight "$out_inflight" 2>/dev/null || true
inflight_ok=1
elif fm_beads_mirror_fresh inflight; then
out_inflight=$(fm_beads_mirror_read inflight)
inflight_stale_since=$(fm_beads_mirror_timestamp_iso inflight)
inflight_ok=1
fi

out_queued=$(task list --label "$label" --ready --limit "$BACKLOG_LIMIT" 2>&1)
rc_queued=$?
if [ "$rc_inflight" -eq 0 ] && [ "$rc_queued" -eq 0 ]; then
printf '## In flight\n%s\n' "$out_inflight"
printf '## Queued\n%s\n' "$out_queued"
if [ "$rc_queued" -eq 0 ]; then
fm_beads_mirror_write ready "$out_queued" 2>/dev/null || true
queued_ok=1
elif fm_beads_mirror_fresh ready; then
out_queued=$(fm_beads_mirror_read ready)
queued_stale_since=$(fm_beads_mirror_timestamp_iso ready)
queued_ok=1
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

2>&1 puts task diagnostics into the mirror.

Lines 278 and 289 merge stderr into the captured output. Line 281 and line 292 then write that combined text to the mirror on success. If task list succeeds but writes a warning or a deprecation notice to stderr, that text becomes part of the cached listing. During a later outage the digest replays the warning as backlog content, with a stale-mirror label attached to it.

Capture stderr separately, or mirror only the stdout portion.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-session-start.sh` around lines 278 - 298, Update the task list
handling in the inflight and queued flows to keep stderr diagnostics separate
from the listing captured in out_inflight and out_queued. Remove the 2>&1
merging from both task list invocations, while preserving rc_inflight/rc_queued
status handling and ensuring fm_beads_mirror_write receives stdout-only content.

Comment thread bin/fm-teardown.sh
Comment on lines +352 to +371
# Fail-open by design, matching fm-bead-stamp.sh: a missing task CLI warns on
# stderr and never blocks or fails an already-confirmed teardown. Any close
# failure - store unreachable, a transient write-path outage while reads still
# succeed, or a genuine conflict - is queued via fm-beads-resilience-lib.sh for
# replay once the store recovers (beads-authority-migration Stage 5 resilience
# layer, report.md section 5), so the confirmed-landed close is never silently
# dropped; fm_beads_write_queue_reconcile treats a bead already reported closed
# on replay as reconciled rather than retrying it forever.
close_linked_bead() {
local beads_id=$1 id=$2
local beads_id=$1 id=$2 reason
reason="landed: firstmate task $id teardown confirmed work landed"
command -v task >/dev/null 2>&1 || {
echo "warning: task CLI not found on PATH, could not close bead $beads_id for $id" >&2
return 0
}
task close "$beads_id" --reason "landed: firstmate task $id teardown confirmed work landed" >/dev/null 2>&1 \
|| echo "warning: could not close bead $beads_id for $id (already closed or unreachable)" >&2
if task close "$beads_id" --reason "$reason" >/dev/null 2>&1; then
return 0
fi
echo "warning: could not close bead $beads_id for $id, queuing for retry" >&2
fm_beads_write_enqueue "$beads_id" "close: $id" close "$beads_id" --reason "$reason" || true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

A missing task CLI still drops the close silently.

Lines 363-366 return 0 without enqueueing. The comment at lines 352-359 states that any close failure is queued "so the confirmed-landed close is never silently dropped". A missing CLI is a close failure, and it is not queued, so the comment overstates the guarantee.

Queueing is possible in this case. fm_beads_write_enqueue needs only jq, not the task CLI. fm_beads_write_queue_reconcile already has a dedicated branch for task CLI not found, N write(s) remain queued, which implies the design expects writes to sit in the queue while the CLI is absent.

Enqueue on the missing-CLI path as well, or narrow the comment to say that a missing CLI drops the close.

🐛 Proposed fix
   command -v task >/dev/null 2>&1 || {
     echo "warning: task CLI not found on PATH, could not close bead $beads_id for $id, queuing for retry" >&2
+    fm_beads_write_enqueue "$beads_id" "close: $id" close "$beads_id" --reason "$reason" || true
     return 0
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bin/fm-teardown.sh` around lines 352 - 371, Update close_linked_bead so the
missing-task-CLI branch calls fm_beads_write_enqueue with the same bead ID,
close payload, and reason used by the failed task close path before returning
successfully. Preserve the warning and fail-open behavior, ensuring the
confirmed close remains queued for later reconciliation when task is
unavailable.

Comment thread docs/configuration.md
If either read fails, the whole listing falls back to the title-line rendering of `data/backlog.md` rather than printing a partial digest.
Beads requires the `task` CLI on `PATH` and access to the active beads store.
Bootstrap validates the beads backend and reports a `MISSING:` line if the CLI is absent or the store is unreachable.
Bootstrap validates the beads backend and reports a `MISSING:` line if the CLI is absent or the store is unreachable and no fresh local mirror covers the gap, or a `DEGRADED:` line naming the mirror's timestamp when one does; see "Beads resilience layer" below.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Line 57 now contradicts the new resilience section.

Line 57 still states that if either beads read fails, the whole listing falls back to the title-line rendering of data/backlog.md rather than printing a partial digest. bin/fm-session-start.sh lines 283-298 changed that. A failed read now falls back to the per-view mirror when the mirror is fresh, and only a section with no usable mirror reaches the markdown fallback. Update line 57 so it agrees with lines 72-73.

The coding guidelines require rewriting or pruning stale material rather than leaving it, and require documentation to stay consistent with its authoritative owner.

As per coding guidelines: "Keep shared agent guidance concise, avoid duplicating codebase facts, and prefer authoritative references; rewrite or prune stale material rather than appending indefinitely."

Also applies to: 69-77

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/configuration.md` at line 59, Update the stale listing-fallback
statement near the beads resilience documentation to describe per-section
fallback behavior: use a fresh per-view mirror when a beads read fails, and use
the markdown title-line fallback only when that section has no usable mirror.
Keep the wording consistent with the authoritative behavior documented in the
“Beads resilience layer” section and remove the contradictory whole-list
fallback claim.

Source: Coding guidelines

Comment on lines +302 to +307
write_beads_mirror_fixture() {
local home=$1 age_seconds=$2 written_at
mkdir -p "$home/state"
written_at=$(($(date +%s) - age_seconds))
jq -n --argjson written_at "$written_at" --arg output 'ready-task-1' \
'{written_at: $written_at, output: $output}' > "$home/state/.beads-mirror-ready.json"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Use the resilience library to create mirror fixtures.

write_beads_mirror_fixture recreates the .beads-mirror-ready.json schema. A schema change in fm_beads_mirror_write can make this test fixture invalid without updating it.

Create the mirror with fm_beads_mirror_write. After that, change only written_at when the test needs a stale mirror.

As per coding guidelines, “Use authoritative scripts, skills, and documentation as the owners of schemas and procedures; do not reimplement their behavior independently.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-bootstrap.test.sh` around lines 302 - 307, Update
write_beads_mirror_fixture to create the mirror through the authoritative
fm_beads_mirror_write helper instead of constructing .beads-mirror-ready.json
with jq. Preserve the existing fixture output, and modify only the written_at
field afterward when age_seconds is used to create a stale mirror.

Source: Coding guidelines

Comment on lines +333 to +369
printf '%s\n' "$out" | grep -Fq 'DEGRADED: task CLI not found (beads store; install:' \
|| fail "missing task CLI with a fresh mirror did not report DEGRADED: $out"
printf '%s\n' "$out" | grep -Fq 'MISSING: task CLI' \
&& fail "missing task CLI with a fresh mirror still reported MISSING: $out"

# 3. task CLI present but store unreachable, no mirror -> hard MISSING:
case_dir="$TMP_ROOT/beads-unreachable-no-mirror"
home="$case_dir/home"
mkdir -p "$home/config"
printf '%s\n' beads > "$home/config/backlog-backend"
fakebin=$(make_fake_toolchain "$case_dir")
cat > "$fakebin/task" <<'SH'
#!/usr/bin/env bash
exit 7
SH
chmod +x "$fakebin/task"
out=$(PATH="$fakebin:$BASE_PATH" FM_HOME="$home" FM_ROOT_OVERRIDE="$home" \
FM_FAKE_TREEHOUSE_LEASE_HELP=1 FM_BOOTSTRAP_DETECT_ONLY=1 "$ROOT/bin/fm-bootstrap.sh")
printf '%s\n' "$out" | grep -Fq 'MISSING: task store is unreachable or broken (beads backend configured, cannot run '\''task list'\''), and no usable local mirror' \
|| fail "unreachable store with no mirror did not report MISSING: $out"

# 4. task CLI present but store unreachable, fresh mirror -> DEGRADED:
case_dir="$TMP_ROOT/beads-unreachable-fresh-mirror"
home="$case_dir/home"
mkdir -p "$home/config"
printf '%s\n' beads > "$home/config/backlog-backend"
write_beads_mirror_fixture "$home" 60
fakebin=$(make_fake_toolchain "$case_dir")
cat > "$fakebin/task" <<'SH'
#!/usr/bin/env bash
exit 7
SH
chmod +x "$fakebin/task"
out=$(PATH="$fakebin:$BASE_PATH" FM_HOME="$home" FM_ROOT_OVERRIDE="$home" \
FM_FAKE_TREEHOUSE_LEASE_HELP=1 FM_BOOTSTRAP_DETECT_ONLY=1 "$ROOT/bin/fm-bootstrap.sh")
printf '%s\n' "$out" | grep -Fq "DEGRADED: task store is unreachable or broken (beads backend configured, cannot run 'task list'); using local mirror from" \
|| fail "unreachable store with a fresh mirror did not report DEGRADED: $out"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the mirror timestamp in both DEGRADED: cases.

Lines 333 and 368 only assert the diagnostic prefix. A regression can omit the required mirror timestamp and still pass both cases.

Assert the using local mirror from <ISO-8601 timestamp> portion for the missing-CLI and unreachable-store paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/fm-bootstrap.test.sh` around lines 333 - 369, Update both DEGRADED
assertions in the missing-CLI and unreachable-store test cases to require the
full “using local mirror from <ISO-8601 timestamp>” detail, not only the
diagnostic prefix. Use a regex or equivalent assertion that validates the
timestamp format while preserving the existing scenario-specific message checks.

@trillium
trillium merged commit fc82bfa into main Aug 2, 2026
12 checks passed
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