fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-580: fail a ticket when its member reaches a terminal health state (re-brief of the rejected 3b2f395) #52

New Issue
Closed
opened 2026-08-15 07:09:42 +02:00 by ltms · 0 comments
ltms commented 2026-08-15 07:09:42 +02:00
Owner
Copy Link
Copy Source

Why this is a fresh unit, not a fix-up

Commit 3b2f395 on worker/cb577-f36fdc-18 (PR #49) attempted this and was rejected on review.
The shape is right; the commit is not. It was written by a terra worker that has since died to a
usage limit, so nothing was fixed in place.

Do not hand the old commit to an implementer as a starting point. It will keep the defect that
caused the rejection. Start from main. PR #49 is closed against this ticket.

What it must do

When FleetHealthMonitor observes a member in a terminal health state — GONE or NEVER_READY —
the tickets waiting on that member must be failed through CB-568's idempotent target-wide failure
operation (abandon), rather than left pending until something else notices.

The three defects that got 3b2f395 rejected

  1. The silent-default trap, for the sixth time. It added a 6-argument FleetHealthMonitor
    overload that defaulted failTarget to (_, _) -> { }. Every existing caller therefore kept the
    no-op, and the feature shipped turned off. The commit added zero tests, which is the tell: the
    overload exists so the old tests compile unchanged, so nothing covers the new path.

    Make failTarget a required constructor argument. Update the call sites. For tests, pass an
    explicit inert value that records calls — copy TestTurnTokens.inert and
    BridgeMcp.CapacitySource.none(). An overload that defaults the new collaborator is an automatic
    rejection at review.

  2. It fired every tick instead of on transition. failTarget.accept sat outside
    reportTransition, so a member that stays GONE had abandon called on every interval, for as
    long as it stayed in the roster. The brief asked for a bounded retry. Fire on the transition
    into the terminal state, and bound any retry explicitly.

  3. A null window and a duplicate import. It held an AtomicReference<MessageService> and had a
    duplicate AtomicReference import. A turn that failed before messagesRef.set(messages) was
    silently skipped. If the reference is genuinely needed to break a construction cycle, the empty
    case must be handled and logged, not silently dropped.

Acceptance criteria

  1. A member transitioning to GONE or NEVER_READY fails every ticket waiting on that target, via
    CB-568's existing idempotent target-wide failure operation. No second mechanism is added.
  2. failTarget is a required dependency. No defaulting overload exists.
  3. The failure fires on transition, not on every tick. A member that remains in a terminal state
    for N ticks produces one failure operation, not N.
  4. Any retry is explicitly bounded, and the bound is tested.
  5. The failed ticket names the real cause — GONE or NEVER_READY — not a generic error. This is
    CB-568's rule.
  6. No silently-skipped path: if a collaborator is not yet wired at the moment of the call, that is
    logged, not dropped.
  7. Tests cover the transition, the no-repeat property, the bound, and the terminal-state reason. A
    commit with no test for the new path is rejected.

Related

  • Rejected attempt: PR #49, commit 3b2f395.
  • Depends on CB-568's idempotent target-wide failure operation (already in main).
  • Part of M4 Unit 2 (docs/M4-Fleet-Health.md §12), criterion 12.
## Why this is a fresh unit, not a fix-up Commit `3b2f395` on `worker/cb577-f36fdc-18` (PR #49) attempted this and was **rejected on review**. The shape is right; the commit is not. It was written by a `terra` worker that has since died to a usage limit, so nothing was fixed in place. **Do not hand the old commit to an implementer as a starting point.** It will keep the defect that caused the rejection. Start from `main`. PR #49 is closed against this ticket. ## What it must do When `FleetHealthMonitor` observes a member in a terminal health state — `GONE` or `NEVER_READY` — the tickets waiting on that member must be failed through CB-568's idempotent target-wide failure operation (`abandon`), rather than left pending until something else notices. ## The three defects that got `3b2f395` rejected 1. **The silent-default trap, for the sixth time.** It added a 6-argument `FleetHealthMonitor` overload that defaulted `failTarget` to `(_, _) -> { }`. Every existing caller therefore kept the no-op, and the feature shipped turned off. The commit added **zero tests**, which is the tell: the overload exists so the old tests compile unchanged, so nothing covers the new path. Make `failTarget` a **required** constructor argument. Update the call sites. For tests, pass an explicit inert value that records calls — copy `TestTurnTokens.inert` and `BridgeMcp.CapacitySource.none()`. An overload that defaults the new collaborator is an automatic rejection at review. 2. **It fired every tick instead of on transition.** `failTarget.accept` sat *outside* `reportTransition`, so a member that stays `GONE` had `abandon` called on every interval, for as long as it stayed in the roster. The brief asked for a **bounded** retry. Fire on the transition into the terminal state, and bound any retry explicitly. 3. **A null window and a duplicate import.** It held an `AtomicReference<MessageService>` and had a duplicate `AtomicReference` import. A turn that failed before `messagesRef.set(messages)` was silently skipped. If the reference is genuinely needed to break a construction cycle, the empty case must be handled and logged, not silently dropped. ## Acceptance criteria 1. A member transitioning to `GONE` or `NEVER_READY` fails every ticket waiting on that target, via CB-568's existing idempotent target-wide failure operation. No second mechanism is added. 2. `failTarget` is a **required** dependency. No defaulting overload exists. 3. The failure fires **on transition**, not on every tick. A member that remains in a terminal state for N ticks produces one failure operation, not N. 4. Any retry is explicitly bounded, and the bound is tested. 5. The failed ticket names the real cause — `GONE` or `NEVER_READY` — not a generic error. This is CB-568's rule. 6. No silently-skipped path: if a collaborator is not yet wired at the moment of the call, that is logged, not dropped. 7. Tests cover the transition, the no-repeat property, the bound, and the terminal-state reason. A commit with no test for the new path is rejected. ## Related - Rejected attempt: PR #49, commit `3b2f395`. - Depends on CB-568's idempotent target-wide failure operation (already in `main`). - Part of M4 Unit 2 (`docs/M4-Fleet-Health.md` §12), criterion 12.
ltms closed this issue 2026-08-15 08:57:03 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#52
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?