test(notice): pin both detachments notifyFailure performs #634

Merged

coilyco-ops merged 1 commit from test/the-notice-detaches-twice-claude into main

2026-08-13 17:37:09 +00:00

coilyco-ops commented

2026-08-13 17:36:02 +00:00

Member

closes #627

Found by the other Angie seat, who built a duplicate of the threading fix, then asked what theirs covered that the landed one did not, and reported the one thing rather than opening a competing pull request.

The property

notifyFailure detaches twice and its comment names one:

noticeCtx, cancel := context.WithTimeout(
	context.WithoutCancel(ctx),
	failureNoticeTimeout,
)

WithoutCancel — the notice survives a turn that is already cancelled
WithTimeout — the notice gets its own bound rather than none

Neither was asserted anywhere. noticethreading_test.go pins the threading mark and one row about WithoutCancel carrying values, which is the mechanism of that defect rather than this guarantee.

The expiring turn is what the first protects and it is the common one. A turn that failed by running out of time arrives with a dead context, so without the detachment the notice is refused by its own deadline and the member gets silence.

Both mutations run

Test-only changes are the easiest place to write something that passes for the wrong reason, so I broke the code each test guards:

WithoutCancel removed -> "the notice was handed an already-cancelled context"
WithTimeout removed   -> "the notice ran unbounded, so a wedged send hangs the failure path"

Each caught by the test written for it, and nothing else fails — so neither passes incidentally.

The fake records the context it was handed rather than only whether the send returned nil, so the assertions are about the property and not about a mock.

Third test covers the case that makes the second non-decorative: a turn with an hour-long deadline must not lend it to the notice.

ward exec gate green. No production code touched.

closes #627 **Found by the other Angie seat**, who built a duplicate of the threading fix, then asked what theirs covered that the landed one did not, and reported the one thing rather than opening a competing pull request. ## The property `notifyFailure` detaches twice and its comment names one: ```go noticeCtx, cancel := context.WithTimeout( context.WithoutCancel(ctx), failureNoticeTimeout, ) ``` - **`WithoutCancel`** — the notice survives a turn that is already cancelled - **`WithTimeout`** — the notice gets its own bound rather than none Neither was asserted anywhere. `noticethreading_test.go` pins the threading mark and one row about `WithoutCancel` carrying *values*, which is the mechanism of that defect rather than this guarantee. **The expiring turn is what the first protects and it is the common one.** A turn that failed by running out of time arrives with a dead context, so without the detachment the notice is refused by its own deadline and the member gets silence. ## Both mutations run Test-only changes are the easiest place to write something that passes for the wrong reason, so I broke the code each test guards: ``` WithoutCancel removed -> "the notice was handed an already-cancelled context" WithTimeout removed -> "the notice ran unbounded, so a wedged send hangs the failure path" ``` Each caught by the test written for it, and nothing else fails — so neither passes incidentally. The fake records the context it was handed rather than only whether the send returned nil, so the assertions are about the property and not about a mock. Third test covers the case that makes the second non-decorative: a turn with an hour-long deadline must not lend it to the notice. `ward exec gate` green. No production code touched.

coilyco-ops added 1 commit

2026-08-13 17:36:02 +00:00

test(notice): pin both detachments notifyFailure performs

ci / image-build (pull_request) Successful in 22s

Details

ci / test (pull_request) Successful in 35s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

4387163284

notifyFailure detaches twice and its comment names one. WithoutCancel lets the
notice survive a turn that has already been cancelled, and WithTimeout gives it
its own bound rather than none. Neither was asserted anywhere.

The expiring turn is the case the first protects and it is the common one: a
turn that failed by running out of time reaches this with a dead context, so
without the detachment the notice would be refused by its own deadline and the
member would get silence.

The fake records the context it was handed rather than only whether the send
succeeded, so the assertions are about the property instead of about a mock
returning nil.

Both mutations were run. Removing WithoutCancel fails the cancelled-turn test
and removing WithTimeout fails the bound test, each by the test written for it,
so neither passes for a reason unrelated to what it claims.

Found by the other Angie seat, who built a duplicate of the threading fix and
then reported the one thing theirs covered that the landed one did not.

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-13 17:36:02 +00:00

coilyco-ops merged commit 284a7767a7 into main

2026-08-13 17:37:09 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-13 17:37:10 +00:00

Merge pull request 'test(notice): pin both detachments notifyFailure performs' (#634) from test/the-notice-detaches-twice-claude into main

coilyco-ops referenced this pull request

2026-08-13 17:39:02 +00:00

test(notice): pin the bound the failure notice keeps for itself #632

coilyco-ops referenced this pull request

2026-08-17 16:47:21 +00:00