fleetd #418: barrier the throw-path push-loop test on state decide() reads #419

Merged

ltms merged 1 commits from worker/418-588283-3 into main 2026-09-10 06:29:23 +02:00

Conversation 0 Commits 1 Files Changed 2

+45 -1

agent commented 2026-09-10 06:25:21 +02:00

Member

Copy Link

Copy Source

Problem

MessageServiceTest.anAskThatLeavesByThrowingStillClosesItsQuestion fails under heavy host load: expected: <INJECT> but was: <STOP>. That assertion runs before the interrupt, so it has nothing to do with the throwing path the test is named for.

MessageService.ask() does three things in order:

1. markAsyncQuestion(...) — flips poll() to Phase.ASKING
2. rendezvous.resolveQuestion(...)
3. pushLoop.onQuestionOpened(...) — the step that actually populates ReplyPushLoop.pendingQuestions, which decide() reads

The test's old barrier, awaitTicketPhaseOn(service, ticket, Phase.ASKING), only waits for step 1. Under load the asker thread can be descheduled between step 1 and step 3, so the barrier releases early and decide() correctly sees nothing pending yet -> STOP.

Fix (test-only, no production behaviour changed)

• Added ReplyPushLoop#pendingQuestionTurnIdsForTest(String lead) — a package-private test seam exposing the existing private pendingQuestionTurnIdsFor, modeled on MessageService#isCompletionStampedForTest (#399). Carries no production behaviour; nothing in ReplyPushLoop calls it.
• The test now captures the question's turnId from the Phase.ASKING TaskView (set in step 1, so it's already valid), then waits (awaitQuestionPendingOn, 3000ms deadline, same style as awaitTicketPhaseOn/awaitCompletionStamped) for that turnId to actually appear in the push loop's pending-question set before asserting decide(...) == INJECT. This barrier waits on the state decide() actually reads (step 3), not on the asserted condition itself (decide() == INJECT) — so it stays a real assertion, not a tautology.

Proof the new barrier is load-bearing (criterion 3)

Temporarily inserted Thread.sleep(300) immediately before pushLoop.onQuestionOpened(...) in MessageService.ask() (fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java:1005-1006), to simulate the descheduling window under load.

With the OLD barrier (awaitTicketPhaseOn(..., Phase.ASKING) only) + the injected delay -> RED:

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.221 s <<< FAILURE! -- in dev.ltms.fleet.msg.MessageServiceTest
[ERROR] dev.ltms.fleet.msg.MessageServiceTest.anAskThatLeavesByThrowingStillClosesItsQuestion -- Time elapsed: 0.069 s <<< FAILURE!
org.opentest4j.AssertionFailedError: the open question should be the one thing keeping this lead's schedule alive ==> expected: <INJECT> but was: <STOP>
	at dev.ltms.fleet.msg.MessageServiceTest.anAskThatLeavesByThrowingStillClosesItsQuestion(MessageServiceTest.java:1806)

This is exactly the reported failure signature.

With the NEW barrier (awaitQuestionPendingOn) + the same injected delay -> GREEN:

[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.550 s -- in dev.ltms.fleet.msg.MessageServiceTest
[INFO] BUILD SUCCESS

(0.55s elapsed is consistent with the 300ms sleep plus the barrier's own poll.)

Restore confirmed: the temporary Thread.sleep was removed from MessageService.java afterward. git diff -- fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java is empty — production source is untouched, and the only changes in this PR are the test seam in ReplyPushLoop.java and the barrier fix in MessageServiceTest.java.

Other phase-as-barrier uses checked (criterion 4)

Only the two other awaitTicketPhaseOn(..., Phase.ASKING) call sites exist in the file (CB-582 nudge tests):

• anAsyncTicketThatPausesOnAQuestionNudgesTheLeadWithNoPriorPollCall (~line 1728): Phase.ASKING is used only to capture the TaskView/turnId; the actual push-loop assertions are gated by a subsequent awaitNudge(...), which waits for a real agent.prompt call — an even stronger barrier on published push-loop state. Not exposed to this race.
• answeringAQuestionStopsFurtherNudgesAboutIt (~line 1757): same pattern — Phase.ASKING only for turnId capture, followed by awaitNudge(...) before any push-loop-dependent assertion. Not exposed.

Other awaitTicketPhaseOn(..., Phase.DONE) sites either don't depend on data published after DONE (they only read TaskView.reply(), set at the same step), or already call awaitCompletionStamped afterward (the #399 fix) where the completion-stamp ordering matters. No other phase-as-barrier gap found.

Build

mvn clean install (fleetd/): BUILD SUCCESS, real unpiped output:

[INFO] Tests run: 1502, Failures: 0, Errors: 0, Skipped: 0

Files changed

• fleetd/src/main/java/dev/ltms/fleet/msg/ReplyPushLoop.java — added pendingQuestionTurnIdsForTest test seam (production behaviour unchanged)
• fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java — new awaitQuestionPendingOn helper; anAskThatLeavesByThrowingStillClosesItsQuestion now barriers on push-loop state instead of Phase.ASKING

## Problem `MessageServiceTest.anAskThatLeavesByThrowingStillClosesItsQuestion` fails under heavy host load: `expected: <INJECT> but was: <STOP>`. That assertion runs before the interrupt, so it has nothing to do with the throwing path the test is named for. `MessageService.ask()` does three things in order: 1. `markAsyncQuestion(...)` — flips `poll()` to `Phase.ASKING` 2. `rendezvous.resolveQuestion(...)` 3. `pushLoop.onQuestionOpened(...)` — the step that actually populates `ReplyPushLoop.pendingQuestions`, which `decide()` reads The test's old barrier, `awaitTicketPhaseOn(service, ticket, Phase.ASKING)`, only waits for step 1. Under load the asker thread can be descheduled between step 1 and step 3, so the barrier releases early and `decide()` correctly sees nothing pending yet -> `STOP`. ## Fix (test-only, no production behaviour changed) - Added `ReplyPushLoop#pendingQuestionTurnIdsForTest(String lead)` — a package-private test seam exposing the existing private `pendingQuestionTurnIdsFor`, modeled on `MessageService#isCompletionStampedForTest` (#399). Carries no production behaviour; nothing in `ReplyPushLoop` calls it. - The test now captures the question's `turnId` from the `Phase.ASKING` `TaskView` (set in step 1, so it's already valid), then waits (`awaitQuestionPendingOn`, 3000ms deadline, same style as `awaitTicketPhaseOn`/`awaitCompletionStamped`) for that `turnId` to actually appear in the push loop's pending-question set before asserting `decide(...) == INJECT`. This barrier waits on the state `decide()` actually reads (step 3), not on the asserted condition itself (`decide() == INJECT`) — so it stays a real assertion, not a tautology. ## Proof the new barrier is load-bearing (criterion 3) Temporarily inserted `Thread.sleep(300)` immediately before `pushLoop.onQuestionOpened(...)` in `MessageService.ask()` (fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java:1005-1006), to simulate the descheduling window under load. **With the OLD barrier** (`awaitTicketPhaseOn(..., Phase.ASKING)` only) + the injected delay -> RED: ``` [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.221 s <<< FAILURE! -- in dev.ltms.fleet.msg.MessageServiceTest [ERROR] dev.ltms.fleet.msg.MessageServiceTest.anAskThatLeavesByThrowingStillClosesItsQuestion -- Time elapsed: 0.069 s <<< FAILURE! org.opentest4j.AssertionFailedError: the open question should be the one thing keeping this lead's schedule alive ==> expected: <INJECT> but was: <STOP> at dev.ltms.fleet.msg.MessageServiceTest.anAskThatLeavesByThrowingStillClosesItsQuestion(MessageServiceTest.java:1806) ``` This is exactly the reported failure signature. **With the NEW barrier** (`awaitQuestionPendingOn`) + the same injected delay -> GREEN: ``` [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.550 s -- in dev.ltms.fleet.msg.MessageServiceTest [INFO] BUILD SUCCESS ``` (0.55s elapsed is consistent with the 300ms sleep plus the barrier's own poll.) **Restore confirmed**: the temporary `Thread.sleep` was removed from `MessageService.java` afterward. `git diff -- fleetd/src/main/java/dev/ltms/fleet/msg/MessageService.java` is empty — production source is untouched, and the only changes in this PR are the test seam in `ReplyPushLoop.java` and the barrier fix in `MessageServiceTest.java`. ## Other phase-as-barrier uses checked (criterion 4) Only the two other `awaitTicketPhaseOn(..., Phase.ASKING)` call sites exist in the file (CB-582 nudge tests): - `anAsyncTicketThatPausesOnAQuestionNudgesTheLeadWithNoPriorPollCall` (~line 1728): `Phase.ASKING` is used only to capture the `TaskView`/`turnId`; the actual push-loop assertions are gated by a subsequent `awaitNudge(...)`, which waits for a real `agent.prompt` call — an even stronger barrier on published push-loop state. Not exposed to this race. - `answeringAQuestionStopsFurtherNudgesAboutIt` (~line 1757): same pattern — `Phase.ASKING` only for `turnId` capture, followed by `awaitNudge(...)` before any push-loop-dependent assertion. Not exposed. Other `awaitTicketPhaseOn(..., Phase.DONE)` sites either don't depend on data published after `DONE` (they only read `TaskView.reply()`, set at the same step), or already call `awaitCompletionStamped` afterward (the #399 fix) where the completion-stamp ordering matters. No other phase-as-barrier gap found. ## Build `mvn clean install` (fleetd/): `BUILD SUCCESS`, real unpiped output: ``` [INFO] Tests run: 1502, Failures: 0, Errors: 0, Skipped: 0 ``` ## Files changed - `fleetd/src/main/java/dev/ltms/fleet/msg/ReplyPushLoop.java` — added `pendingQuestionTurnIdsForTest` test seam (production behaviour unchanged) - `fleetd/src/test/java/dev/ltms/fleet/msg/MessageServiceTest.java` — new `awaitQuestionPendingOn` helper; `anAskThatLeavesByThrowingStillClosesItsQuestion` now barriers on push-loop state instead of `Phase.ASKING`

agent added 1 commit 2026-09-10 06:25:22 +02:00

fleetd #418: barrier the throw-path push-loop test on state decide() reads ... ab0cc71aa4

anAskThatLeavesByThrowingStillClosesItsQuestion barriered on Phase.ASKING,
which markAsyncQuestion sets in ask()'s FIRST step. The assertion right
after it depends on ask()'s THIRD step (pushLoop.onQuestionOpened), which
is what actually populates ReplyPushLoop's pendingQuestions map. Under
load the asker thread can be descheduled between those two steps, so the
barrier released before decide() had anything to see, and it correctly
returned STOP instead of the expected INJECT.

Add ReplyPushLoop#pendingQuestionTurnIdsForTest, a package-private test
seam (modeled on MessageService#isCompletionStampedForTest) exposing the
private pendingQuestionTurnIdsFor. The test now waits for its own turnId
to appear there before asserting on decide() — not for decide() itself to
return INJECT, which would make the barrier assert nothing.

Checked every other awaitTicketPhaseOn(..., Phase.ASKING) in the file
(two, in the CB-582 nudge tests): both are followed by a real awaitNudge()
that waits for an actual agent.prompt push-loop call before any assertion
depends on push-loop state, so they are not exposed to this race.

No production behaviour changed.

ltms merged commit 2d09c8b027 into main 2026-09-10 06:29:23 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

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

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#419