From d03bb0ed08d79327502a149e805ddc6dec52813c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 05:48:54 +0000 Subject: [PATCH] Correct the stop timeout comments on the archive writers The comments on the engine's stop and on the timeout test gave false reasons for leaving archive writers open when the stop budget runs out. Closing them would wait for any write in progress, and a worker still running would then open new writers that nothing closes, so closing gains nothing over a kill. Model: opus-5-5 --- internal/delivery/engine.go | 6 +++--- internal/delivery/engine_lifecycle_test.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/delivery/engine.go b/internal/delivery/engine.go index a9d06f2..4786f08 100644 --- a/internal/delivery/engine.go +++ b/internal/delivery/engine.go @@ -368,9 +368,9 @@ func (e *Engine) start() { // writer for long by then: the archive sweeper stops before the // engine, and deleting a webhook only closes one. If the pool did // not drain in time, the writers are left open, as a kill would -// leave them: a worker still running may be mid-write, and -// closing its writer would wait on that write and then fail the -// next delivery the worker archives. +// leave them. Closing them would wait for any write in progress, +// and a worker still running would then open new writers that +// nothing closes, so it gains nothing over a kill. func (e *Engine) stop(ctx context.Context) error { e.log.Info("delivery engine stopping") diff --git a/internal/delivery/engine_lifecycle_test.go b/internal/delivery/engine_lifecycle_test.go index ae1afa5..9fadf9d 100644 --- a/internal/delivery/engine_lifecycle_test.go +++ b/internal/delivery/engine_lifecycle_test.go @@ -327,10 +327,10 @@ func TestEngine_StopHookClosesArchives(t *testing.T) { } // TestEngine_StopHookTimeoutLeavesArchivesOpen covers a stop whose -// budget runs out while a worker is still running. That worker may -// be in the middle of an archive write, so the archive writers are -// left open, as a kill would leave them, rather than closed -// underneath it. +// budget runs out while a worker is still running. The archive +// writers are left open, as a kill would leave them: closing them +// would wait for any write in progress, and that worker would then +// open new writers that nothing closes. func TestEngine_StopHookTimeoutLeavesArchivesOpen(t *testing.T) { t.Parallel()