fix(crew-state): let a declared wait outrank a cancelled run record - #166
Closed
quinnbot-ai wants to merge 7 commits into
Closed
quinnbot-ai wants to merge 7 commits into
quinnbot-ai wants to merge 7 commits into
Conversation
…enguid#2707) * fix(bearings): always show decision options and a close/drop control Freeform-only Captain's Call cards hid the option buttons the board was designed around, and there was no way to drop a stale hold without inventing an answer. Require selectable options, keep freeform as a supplement, and route the reserved __drop__ answer through decline so the hold leaves Captain's Call. * no-mistakes(review): Fix drop closure and decision-only option validation * no-mistakes(review): Preserve answerability for non-decision cards * no-mistakes(document): Clarify decision drop documentation
Signature-only PRs can hide skipped review, test, or document steps. Fail unless no-mistakes >= 1.46.0 attests those three steps completed.
…#2728) * feat(captain-hold): collapse the decisions concept into tasks held for the captain A decision is no longer a separate type: it is an ordinary backlog task held for the captain, identified by its task id. bin/fm-captain-hold.sh owns the surviving behaviors - guarded hold creation, the recorded-answer close (answer/answers with a release mode for captain-gated work), the source bindings, and the investigation completion gate - and bin/fm-decision-hold.sh becomes a one-release compatibility shim over it. The fleet snapshot now parses hold-until and computes captain_actionable as queued + captain-held + unblocked + due, independent of row kind, plus a presentation-only deferred_marker for prose-deferred rows. Bearings renders every due captain-held task in Captain's Call, date-deferred holds as dated Charted Next gates, suppresses prose-deferred rows from default views with an omitted disclosure, and excludes from Recently Landed anything that closed while still held for the captain. Legacy compatibility: pre-collapse <origin>-decision-<key> rows are already plain task ids and keep working; short keys in recorded metadata, concrete origin bindings, chat --resolve-key fallbacks, and old resolution records all resolve in place. * no-mistakes(review): Fix captain answer replay and body preservation * no-mistakes(review): Fix captain hold idempotency and legacy replay * no-mistakes(review): Validate card close modes and compatibility routing * no-mistakes(review): Enforce release replay mode matching * no-mistakes(review): Prevent duplicate decision cards and released replay mismatches * no-mistakes(review): Preserve answer columns and legacy resolve replays * no-mistakes(document): Document strict replay and legacy compatibility * no-mistakes(lint): Quote done literals to satisfy ShellCheck * no-mistakes: apply CI fixes * fix(rebase): keep collapsed captain hold board semantics
…id#2733) * fix(watch): announce recovery once per generation and keep successors supervising A lost Pi/OpenCode handling handshake re-announced the same recovery generation on every cycle and spent the successor's first ~55s blind, so a real crew event could be ignored and then dropped. Record the announcement in the durable marker, confirm the handshake before the follow-up without swallowing failure, and enter the poll loop immediately. * no-mistakes(review): Tighten recovery event timing regression * no-mistakes(document): Document recovery-loop supervision guarantees
A run stopped through the supported no-mistakes abort keeps its branch, head, and PR, and the crew that aborted it declares the external wait it is idling on and then exits, so no agent is expected in that endpoint. The run-step is authoritative, so that terminal record reported `failed` forever, the shared absorb classification read it as neither working nor paused, and every supervision cadence re-escalated the deliberately agent-free endpoint as a possible wedge. A declared wait may now outrank a TERMINAL CANCELLED record, and only under three conditions that are each load-bearing: - the record is `cancelled`, never `failed`, so an ordinary terminal failure keeps surfacing exactly as before whatever the log says; - the status log's last line is a declared `paused:` external wait, so the wait is explicit rather than inferred from an idle endpoint; - that declaration was appended after the cancelled run started, so a `paused:` line left over from before this run cannot mask its cancellation. The ordering anchor is the run's start time from the top-level runs listing: no cancellation instant is available anywhere in `axi status` or `axi logs`. Everything fails closed - an unreadable status mtime or an unavailable run start keeps the terminal record authoritative.
Conflict was in bin/fm-crew-state.sh and its test: main independently gave the supported abort's terminal record its own `cancelled` state (distinct from a pipeline failure) for reconciler attribution, while this branch kept mapping it to `failed` and added the declared-wait override on top. Resolved by keeping main's `cancelled` state at all three cancelled sites and layering this branch's RUN_CANCELLED flag onto it, so the declared-wait override still fires and only for a cancelled record. The tests that pinned the unqualified outcome now pin `cancelled` instead of `failed`; the failed-run safety case is unchanged.
Owner
Author
|
Closing as invalid delivery: this branch was cut from kunchenguid/main but targets quinnbot-ai/main, so it carries 4 upstream commits and 54 changed files that are not its own work - and its copy of bin/fm-merge-local.sh predates PR #167, so merging would silently revert that fix. The branch and its commits are deliberately PRESERVED; the accepted crew-state fix will be reapplied on a clean branch cut from upstream main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
A run stopped through the supported
no-mistakes axi abortkeeps its branch, head, and PR.The crew that aborted it declares the external wait it is idling on and exits, so no agent is expected in that endpoint.
bin/fm-crew-state.shgives the run-step precedence and maps that terminal record tofailed, so the crew's own laterpaused:declaration never got a chance to become current state.crew_absorb_classthen readfailedas neither working nor paused, so the wake surfaced on every cadence, forever - the reproduced lane escalated as a possible wedge 13 times.Correction to the reported mechanism
The report said
fm-crew-state.shemitscancelledand thatcancelledfalls throughcrew_absorb_classtonone.It does not emit
cancelled: both theoutcomeandstatuscases map a cancelled run toRUN_STATE=failedwith detailrun cancelled, and it isfailedthat falls through tonone.The reproduction confirmed the emitted line was
state: failed - source: run-step - run cancelled.Net effect is as reported, but it matters for the fix:
cancelledandfailedwere already conflated into one emitted state, so telling them apart was the first thing the fix had to restore.The change
A declared wait may outrank a TERMINAL CANCELLED record, under three conditions that are each load-bearing:
cancelled, neverfailed- an ordinary terminal failure keeps surfacing exactly as before, whatever the status log says;paused:external wait, so the wait is explicit rather than inferred from an idle endpoint;paused:line left over from before this run cannot mask its cancellation.crew_absorb_classneeded no change: it already mapspausedto paused.The ordering anchor, and its exact bound
No cancellation instant is available anywhere: neither
no-mistakes axi statusnoraxi logscarries a timestamp.The only timestamp the CLI exposes for a run is the start time in the top-level runs listing, verified to be the start (that run's first step log was written at the listed minute, its last step log nine hours later).
Condition 3 is therefore bound to the run's start.
It excludes every wait declared before this run existed, and it admits a wait declared while the run was still active.
That bound is recorded with its evidence in
docs/verification/supervision.md.Everything fails closed: an unreadable status mtime or an unavailable run start keeps the terminal record authoritative.
Verification
The base is broken
Three cases fail on unfixed code, each with the exact defect signature:
cancelled run + pause declared after it -> paused- gotstate: failed - source: run-step - run cancelledcoarse cancelled record + later declared wait -> paused- same linedeclared wait after a supported abort classified as 'none', not pausedThe three negative cases pass on base (base always answers
failed); their value is proven by mutation below.Also reproduced end to end against the real aborted run: before the fix that lane read
state: failed - source: run-step - run cancelled, after it readsstate: paused - source: status-log - ... - declared after the cancelled run.That check was a read-only run of the helper; nothing in the live lane was written or modified.
Mutations
Nine single-site mutations, each caught:
failedtooa declared wait never masks a terminal failed runa pause declared before the run cannot mask its cancellationan undeclared wait after a cancellation still surfacesfailedinstead ofpausedoutcome: cancelledsitestatus: cancelledsitean unreadable status mtime refuses cleanly(shell error on stderr)M1, M2, M3, M5, M7 and M9 each produce a distinct, single failure.
M4 and M8 produce the same failure set: both break the override entirely, one at the emit and one at the ordering input.
That is honest coverage, not a hidden gap - the observable contract is identical for both - so no test was added to separate them.
M9 was the reason
test_unreadable_status_mtime_refuses_cleanlyexists at all: without it the numeric guard was decoration, since dropping it still failed closed and only differed by shell noise on stderr.Nothing else in the change survives being broken.
Suites
tests/fm-crew-state.test.sh(new case group(l)), plusfm-watch-triage,fm-classify-decision-key,fm-teardown, andfm-busy-stateall pass.bin/fm-lint.sh,shellcheck -x, andbin/fm-doc-audience-check.share clean.