fix(dispatch): opruimlus vangt fouten per container af (follow-up #97) #98

Merged

janpeter merged 1 commit from fix/m41-reap-per-container into master

2026-10-02 21:39:26 +02:00

janpeter commented

2026-10-02 21:13:53 +02:00

Owner

Waarom

Follow-up op de niet-blokkerende opmerkingen uit de laatste review van scrum4me-docker#97.

WARNING: de catch in removeUnstarted omvatte de hele opruimlus. Faalde inspect of rm voor één kandidaat, bijvoorbeeld omdat hij tussen ps en inspect verdween, dan werden alle volgende weescontainers die ronde overgeslagen. Een hardnekkige fout kon zo de opruiming bij elke ronde blokkeren.
INFO: de test "never reaps a started, journalled or foreign container" maakte geen container van een ander slot aan, en ook geen poging die gelijktijdig wordt aangemaakt.

Wat

Elke kandidaat heeft nu een eigen try/catch. Een mislukte ps-listing beëindigt de ronde nog steeds stil (best effort); de volgende create of brokerstart probeert opnieuw.
Bescherming en filters zijn ongewijzigd: het gejournalde container-id, pogingen in creating, het eigen slot, status created en pid 0, en docker rm zonder -f.

Bewijs

Nieuwe test "keeps reaping the other candidates when one of them fails": de eerste kandidaat geeft een fout bij inspect en rm, de tweede is een echte wees. Eerst rood (de wees bleef staan), nu groen.
Nieuwe test "never reaps a container of another slot or of an attempt that is still being created":
- een kandidaat met slotlabel other-slot blijft staan;
- de container van een create die nog loopt, overleeft een tweede create met opruimen.
- Deze test legt bestaand gedrag vast en was dus meteen groen; er was geen bug.
Hernoemd: de bestaande test heet nu "never reaps a started or journalled container".
Suites: broker-bestanden 46/46. npm test geeft 1044 groen, plus de 2 bekende macOS-failures in transcript-retention. tsc -p tsconfig.dispatch.json is groen.

Story ST-1617 (Scrum4Me), follow-up op T-1956.

🤖 Generated with Claude Code

## Waarom Follow-up op de niet-blokkerende opmerkingen uit de laatste review van scrum4me-docker#97. - **WARNING:** de `catch` in `removeUnstarted` omvatte de hele opruimlus. Faalde `inspect` of `rm` voor één kandidaat, bijvoorbeeld omdat hij tussen `ps` en `inspect` verdween, dan werden alle volgende weescontainers die ronde overgeslagen. Een hardnekkige fout kon zo de opruiming bij elke ronde blokkeren. - **INFO:** de test "never reaps a started, journalled or foreign container" maakte geen container van een ander slot aan, en ook geen poging die gelijktijdig wordt aangemaakt. ## Wat - Elke kandidaat heeft nu een eigen `try/catch`. Een mislukte `ps`-listing beëindigt de ronde nog steeds stil (best effort); de volgende create of brokerstart probeert opnieuw. - Bescherming en filters zijn ongewijzigd: het gejournalde container-id, pogingen in `creating`, het eigen slot, status `created` en pid 0, en `docker rm` zonder `-f`. ## Bewijs - **Nieuwe test** "keeps reaping the other candidates when one of them fails": de eerste kandidaat geeft een fout bij `inspect` en `rm`, de tweede is een echte wees. Eerst rood (de wees bleef staan), nu groen. - **Nieuwe test** "never reaps a container of another slot or of an attempt that is still being created": - een kandidaat met slotlabel `other-slot` blijft staan; - de container van een create die nog loopt, overleeft een tweede create met opruimen. - Deze test legt bestaand gedrag vast en was dus meteen groen; er was geen bug. - **Hernoemd:** de bestaande test heet nu "never reaps a started or journalled container". - **Suites:** broker-bestanden 46/46. `npm test` geeft 1044 groen, plus de 2 bekende macOS-failures in `transcript-retention`. `tsc -p tsconfig.dispatch.json` is groen. Story ST-1617 (Scrum4Me), follow-up op T-1956. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

janpeter added 1 commit

2026-10-02 21:13:53 +02:00

fix(dispatch): opruimlus vangt fouten per container af (follow-up review #97 )

CI / Compose config (pull_request) Successful in 10s

Details

CI / Build-arg coverage (pull_request) Successful in 5s

Details

CI / Docker build (pull_request) Successful in 2m1s

Details

c5da72ca7b

Een kandidaat die tussen ps en inspect verdween, of die Docker niet wil
verwijderen, sloeg via de catch rond de hele lus alle volgende
weescontainers van die ronde over. Elke kandidaat staat nu in een eigen
try/catch; een mislukte listing stopt de ronde nog steeds stil. Tests
voor twee kandidaten waarvan de eerste faalt, en expliciet voor de
bescherming van een ander slot en van een poging die nog wordt
aangemaakt; de testnaam over 'foreign' is rechtgezet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

s4m-codex-reviewer reviewed

2026-10-02 21:17:00 +02:00

s4m-codex-reviewer left a comment

COMMENT

Geen blokkerende codebevindingen gevonden. De foutafhandeling is correct van de hele opruimronde naar individuele kandidaten verplaatst; bestaande journal-, slot- en in-flight-bescherming en plain docker rm blijven behouden. Dit past bij de productstandaarden voor kleine wijzigingen en hergebruik van bestaande patronen. De comments documenteren de reden voldoende; aanvullende productdocumentatie is voor deze interne bugfix niet nodig.

geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

Findings

minor — tests/dispatch-broker.test.ts:171: De vaste wachttijd van 50 ms bewijst niet dat de eerste create de after-hook heeft bereikt. Bij trage IO kan de tweede opruimronde al plaatsvinden voordat de container bestaat, waardoor de test slaagt zonder het bedoelde scenario te toetsen. Synchroniseer expliciet op het bereiken van de create-hook.
minor — tests/dispatch-broker.test.ts:158: De regressietest dekt een inspect-fout, maar geen rm-fout na een geslaagde inspect. De brokenId-fixture gooit altijd tijdens inspect, zodat de rm-fouttak in deze test nooit wordt bereikt. Voeg een afzonderlijk scenario toe waarin rm faalt en een volgende kandidaat toch wordt verwijderd.

Verificatie

De opgegeven commit c5da72ca7b is opgehaald en de diff komt overeen met de invoer. git diff --check slaagt. Tests konden niet starten: uitvoering vanuit /tmp is geblokkeerd en de native rolldown-module kan daar niet worden geladen. De dispatch-typecheck faalt onder meer door ontbrekende vendor/scrum4me-shared-bronnen in deze checkout; dit is niet als door deze diff veroorzaakte fout vastgesteld. Daardoor ontbreken groene test- en typecheckresultaten en kies ik conform de safe-default COMMENT.

# COMMENT Geen blokkerende codebevindingen gevonden. De foutafhandeling is correct van de hele opruimronde naar individuele kandidaten verplaatst; bestaande journal-, slot- en in-flight-bescherming en plain `docker rm` blijven behouden. Dit past bij de productstandaarden voor kleine wijzigingen en hergebruik van bestaande patronen. De comments documenteren de reden voldoende; aanvullende productdocumentatie is voor deze interne bugfix niet nodig. geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. ## Findings - **minor** — __tests__/dispatch-broker.test.ts:171: De vaste wachttijd van 50 ms bewijst niet dat de eerste create de after-hook heeft bereikt. Bij trage IO kan de tweede opruimronde al plaatsvinden voordat de container bestaat, waardoor de test slaagt zonder het bedoelde scenario te toetsen. Synchroniseer expliciet op het bereiken van de create-hook. - **minor** — __tests__/dispatch-broker.test.ts:158: De regressietest dekt een inspect-fout, maar geen rm-fout na een geslaagde inspect. De brokenId-fixture gooit altijd tijdens inspect, zodat de rm-fouttak in deze test nooit wordt bereikt. Voeg een afzonderlijk scenario toe waarin rm faalt en een volgende kandidaat toch wordt verwijderd. ## Verificatie De opgegeven commit c5da72ca7bf0a8b06589c19ef8d0b039338b5612 is opgehaald en de diff komt overeen met de invoer. `git diff --check` slaagt. Tests konden niet starten: uitvoering vanuit `/tmp` is geblokkeerd en de native rolldown-module kan daar niet worden geladen. De dispatch-typecheck faalt onder meer door ontbrekende `vendor/scrum4me-shared`-bronnen in deze checkout; dit is niet als door deze diff veroorzaakte fout vastgesteld. Daardoor ontbreken groene test- en typecheckresultaten en kies ik conform de safe-default COMMENT.

janpeter merged commit 03aeac37c4 into master

2026-10-02 21:39:26 +02:00

janpeter referenced this pull request from a commit

2026-10-02 21:39:28 +02:00

Merge pull request 'fix(dispatch): opruimlus vangt fouten per container af (follow-up #97)' (#98)