fix(engine): the timeout reaper fails PENDING and STOPPING runs past their deadline, so no run stays unended for good by bmscomp · Pull Request #354 · bmscomp/kates

bmscomp · 2026-10-08T00:10:43Z

#353 is merged (44b880a), and main is merged in here (8446607), so this PR's diff is its own change: d082071, the fix, and c667a82, which #353 also made as 4665093 before it merged.

Why

Since #353, the daily sweep deletes only DONE and FAILED runs. Some runs never end, so no sweep ever takes them. Checked in the code:

PENDING. executeTest stores the run PENDING, and executeAsync stores it RUNNING with saveIfStatus(run, PENDING) once its tasks are submitted. The run stays PENDING when that write never happens:
- the Kates API stops or dies while creating the topic or submitting the tasks;
- an Error escapes before the write, and executeTest's catch (Throwable) only releases the permit;
- the event after the first save fails, and markStrandedAsFailed fails too.
@PreDestroy shutdown misses such a run too, because it walks activeHandles and handles are published only after the RUNNING write.
STOPPING. Nothing writes it now. Before fix(engine): cancel frees its slot; honest topic, trend, metrics data #203 (1.24.0), gRPC CancelTest called stopTest, which stored STOPPING. Before fix(engine): deleting a run stops it and gives back its slot #290 (1.25.0), the REST delete stored STOPPING, then deleted the row. A process that stopped in between left the run STOPPING.
Nothing ends either one. TestTimeoutReaper.reapStuckTests and TestOrchestrator.recoverOrphans read findByStatus(RUNNING) only. reconcileActiveRuns reads only this process's activeHandles.

Replicas. A run another replica is executing has its handles and its permit there, not here. recoverOrphans can't tell that run from an orphan: it fails every RUNNING run in the table. That's why the chart pins replicaCount: 1 and Recreate (values.schema.json). TestRun has no owner column and no heartbeat (updated_at), only created_at and version.

"No handle here" can't identify an orphan even locally: a PENDING run being submitted has a permit but no handles. So the two rules I was asked to consider are left out:

PENDING, no handle here and older than a startup grace;
STOPPING, no handle here and older than a short grace.

Both would fail another replica's live run, for example a submission still under way there. What's left is the rule that holds whichever process runs the run: its deadline. The deadline is stored with the run and bounds its whole life. A RUNNING run past it is already reaped, so failing a PENDING or STOPPING one past it ends nothing sooner than the reaper would anyway. The cost is time: a run left PENDING by a restart waits up to max-duration-ms plus the grace (2 h 05 min by default) before it ends. A stale run holds no permit after a restart, so the only effect of the wait is the status it shows.

Changes

TestTimeoutReaper

It reads RUNNING, PENDING and STOPPING runs, and holds each to the same deadline as before: the planned duration, capped by kates.engine.max-duration-ms, plus kates.engine.reaper-grace-ms, counted from creation.
It acts on the status it reads back with the whole run, and writes with a compare-and-set on that status.
Each task that is neither DONE nor FAILED is failed with an error that says why. A Trogdor task left STOPPING counts as unfinished too.
- Timeout: still running …: unchanged.
- Timeout: still pending …; its tasks were never stored as started, as when the Kates API stops while it starts them
- Timeout: still stopping …; only a Kates API before 1.25.0 stores a run STOPPING, and nothing ended this one
A run with no task gets one, <id>-submission, FAILED, with no phase, carrying the error. A PENDING run has none, and without it the API would show a FAILED run with no reason, and its JUnit export would pass.
Order. A PENDING run is settled only after its write lands. Its submission in this process can store it RUNNING in between, and settling first would hand back the permit of a run that goes on. Once FAILED is stored, the submission's own saveIfStatus(PENDING) fails, and it stops the tasks it started (the fix(engine): a run cancelled during its submission stays cancelled, and its tasks stop #321 path). RUNNING and STOPPING runs are still settled before the write.
No lifecycle event or completion metric, as for a reaped RUNNING run.

Comments. These said the reaper reads RUNNING only:

TestOrchestrator.markStrandedAsFailed and cancelTest, rewritten line for line so no CLI ref into TestOrchestrator.java moves;
TestCleanupScheduler.FINISHED;
TestOrchestratorCancelTest's class comment.

kates mcp

The reaper-deadline caveat covers PENDING and STOPPING runs and the <id>-submission task. It also says Kates API 1.25.0, the first with deadlines, holds only RUNNING runs to them. The id keeps its meaning: a FAILED run may have been reaped at its deadline.
Its ref to TestTimeoutReaper.java (39-47,50-136, unanchored) now points at 55-56,64-73,75-205, anchored on the statuses, the grace property, "-submission" and "Timeout: still ".
The audit-actor-recorded ref to TestCleanupScheduler.java moves from 99 to 97, because the javadoc above it is two lines shorter.

Docs

Book 11's status notes: the deadline applies to PENDING, RUNNING and STOPPING runs. A restart leaves a PENDING run PENDING until then, and a task-less run gets <id>-submission.
Book 02: a run whose tasks were still being started stays PENDING through a shutdown or crash, until its deadline.
Book 12: such runs no longer "never end"; the sweep takes them once the reaper has failed them.
The glossary's Test Run entry.
Book 12's sweep paragraph, from fix(engine): the API's daily run sweep deletes only finished runs, in batches, each through the run delete with an audit row #353, names DELETE /api/tests/{id} in code instead of linking it (c667a82). lychee reads the heading's trailing {id} as an attribute block and finds no #delete-apitestsid anchor, so Markdown Link Check failed here and fails on fix(engine): the API's daily run sweep deletes only finished runs, in batches, each through the run delete with an audit row #353 too.

Tests

TestTimeoutReaperTest (QuarkusTest, real database), 4 new tests, 7 in all:
- a PENDING run past its deadline ends FAILED with one <id>-submission task and the exact error;
- a PENDING run inside its deadline stays PENDING. It has no handle or permit here, like a run another replica is starting;
- a STOPPING run past its deadline ends FAILED. Its unfinished tasks get the STOPPING error, and its finished task and every row are kept;
- a STOPPING run inside its deadline stays STOPPING.
New TestTimeoutReaperSubmissionTest, in the style of TestOrchestratorCancelTest (mocked repository, a real orchestrator, a backend that records stops):
- a PENDING run past its deadline, whose submission is held inside backend.submit: the reaper stores it FAILED and frees the slot, and once released, the submission stops both its tasks and leaves the row as the reaper stored it;
- the submission stores the run RUNNING between the reaper's read and its write: the reaper's write doesn't land, nothing is stopped, the slot stays held, and endRun isn't called;
- a STOPPING run this process still runs: its tasks are stopped, its slot is freed, and it's stored FAILED.
Two deliberate breaks, each reverted afterwards:
- settling a PENDING run before the write fails the race test: the run keeps its slot ==> expected: <1> but was: <0>;
- reading RUNNING runs only again fails all 3 new in-memory tests and the 2 new past-deadline QuarkusTests.
Run locally:
- ./mvnw test -Dquarkus.grpc.codegen.skip=true -Dquarkus.http.test-port=0: 1,462 tests, 0 failures. After that run, the STOPPING error lost its last word ("after"); the two reaper test classes were run again and pass.
- spotless:check on JDK 25: clean.
- cd cli && go test -p 1 ./... -race -count=1, go vet ./..., golangci-lint run --new-from-rev=origin/main ./...: 0 issues. go test ./cmd -run TestMCPCaveat passes.
- scripts/check-cli-style.sh, scripts/check-cli-compat.sh, scripts/check-book-style.sh, gen-version-matrix.sh --check, gen-chart-table.sh --check, check-book-contrast.py.
- A scratch-copy HTML render of the book: no unresolved link, and check-book-fragments.py finds all 7,461 fragment links landing.
- python3 -m unittest in eval/mcp: OK.
Not run locally, because Docker is off: the *IT tests, the Kind tests, and the PDF render.

…new DELETE /api/tests The kates chart's cleanup CronJob, which values-prod.yaml and values-staging.yaml enable, never deleted a run: - it asked GET /api/tests for status COMPLETED, which the API refuses with a 400; - it sent no API key, so got a 401 with auth on (the default); - it paired the runs' id and createdAt in an order the JSON never has; - it computed its cutoff with date options busybox lacks; - it read only the newest 200 runs; - it carried the API pod's labels, so the chart's NetworkPolicy blocked it from the API. The API now does the picking: DELETE /api/tests?createdBefore=<instant> deletes the finished runs (DONE and FAILED by default, ?status= to narrow) created before then. It deletes oldest first, at most limit (1000) a call, each through the same delete as DELETE /api/tests/{id} and with an audit row, and answers matched, deleted and remaining. ?dryRun=true only counts. Anything else is a 400, a malformed dryRun included, so a typo cannot turn a count into a delete. The CronJob sends the key from the apiKey Secret and computes its cutoff with date -d @seconds, which busybox reads. It calls until no run matches, at most 100 calls, and fails on anything but a 200 with the API's answer; backoffLimit is 2. cleanup.statuses (default DONE and FAILED) picks the runs. Chart 0.10.10. kates test prune --older-than 30d does the same by hand. It counts first, asks unless --yes, and with --dry-run only counts. prune is no longer an alias of kates test cleanup, which deletes orphans still RUNNING. Caveat refs into TestResource and TestRunRepository are re-pointed.

@RolesAllowed

…ention #350 made every REST endpoint name its scope (deny-unannotated- endpoints), so DELETE /api/tests takes @RolesAllowed(admin), as the single and bulk deletes do; ScopesTest now refuses an agent's prune too, and book 11 lists pruning under admin. TestResource's only conflict was its imports: both kept. The caveat refs take main's copy of each cli/cmd/mcp file, re-pointed once through a line map from main's TestResource and TestRunRepository to this branch's.

…uses it is about to change The sentence named the runs the sweep leaves alone, PENDING and RUNNING, which the follow-up that makes the sweep delete only DONE and FAILED runs changes. Saying it deletes old finished runs stays true either way, so that PR needn't touch charts/ and bump the chart again.

… batches, each through the run delete with an audit row TestCleanupScheduler, the Kates API's own retention sweep (kates.cleanup.retention-days, 90 days): - read every run with findAll(), results included, each day; - deleted any run that was not RUNNING or PENDING, so STOPPING runs too, whose tasks may still produce; - removed rows with repository.delete, so it neither settled a run's tasks nor wrote an audit row; - swallowed a createdAt it could not parse; - held one transaction around it all. The delete loop of DELETE /api/tests moves into RunRetention: - count, then at most limit ids oldest first; - each through orchestrator.deleteTest with an audit row; - count again. The endpoint answers byte for byte as before. The sweep uses that loop: - DONE and FAILED runs older than the retention only, in passes of 1000, until none match, a pass deletes nothing, or 1000 passes; - audit details "retention sweep: created before <cutoff> (kates.cleanup.retention-days=<n>)"; - deletes count as they happen, so a pass that throws still reports what it deleted. The error is logged and ends that sweep. PENDING, RUNNING and STOPPING runs stay, whatever their age.

…to fix/sweep-safe-audited The one conflict, book 11's DELETE /api/tests section, keeps both sides: #352's sentence on the admin scope, and this branch's description of the new sweep in place of the old one.

…ly sweep, and refs follow the moved lines The audit-no-actor caveat and the did_kates_cause_this prompt told agents that only the REST test endpoints write audit rows. DELETE /api/tests (#352) and the API's daily retention sweep now write them too. Caveat refs into TestResource are re-pointed through a line map from #352's head, with an anchored ref to RunRetention's audit row.

@Audited

…ention #351 holds every write to @Audited or @NotAudited, so DELETE /api/tests is @Audited(DELETE, test). Each run it deletes still gets its own row, which now names who pruned it; a call that deletes none, or is refused, gets the response filter's row, and a dry run marks its trail done and leaves none. AuditActorTest checks an agent's refused prune; TestResourcePruneTest uses list's actor argument; book 11 says who the rows name. The caveat refs take main's copy of each cli/cmd/mcp file, re-pointed once from main's TestResource and TestRunRepository.

…ix/sweep-safe-audited #351 is on main, so the sweep's audit rows name system:scheduler: RunRetention's prune takes an Actor, the sweep passes Actor.SCHEDULER, and the endpoint passes none, so its rows keep naming the caller and mark its audit trail done. TestCleanupSchedulerTest checks the rows by actor and the mocks follow the new argument. Conflicts: - TestResource: both injections kept (RunRetention, AuditTrail); a dry run marks its trail done, then counts through RunRetention. - Book 11: #352's sentence on who the rows name, and this branch's description of the sweep, now naming system:scheduler, as books 12 and kates/docs do. - The cli/cmd/mcp files take #352's copy, with #351's audit texts, re-pointed once to this branch's Java. audit-no-actor is retired on main, so this branch's edit to it goes; audit-actor-recorded names the sweep instead, with anchored refs to where it records.

#352's squash has the same tree as its last head, 31785d1, which this branch already had (7e6da8c), so every conflict resolves to this branch's side: TestResource, TestRunRepository, the test application.properties, kates mcp's runs and scenario tools, books 10 and 11, and kates/docs/api-reference.md. Book 12 merged without a conflict but with #352's cleanup paragraphs appended a second time, the last one with the old sweep's "not RUNNING or PENDING"; it keeps this branch's text too. The merge's tree is 7e6da8c's.

…their deadline, so no run stays unended for good Since #353 the daily sweep deletes only DONE and FAILED runs, and nothing ended some runs at all. A run stays PENDING when its submission never stores it RUNNING, as when the Kates API stops while it creates the run's topic, and STOPPING when a Kates API before 1.25.0, which stored that status on the way to ending a run, stopped before it did. No later version stores it. The reaper and orphan recovery read RUNNING runs only, and the reconciler only this process's handles, so such a run stayed as it was, counted toward nothing, and no sweep removed it. The reaper now holds PENDING and STOPPING runs to the deadline it holds RUNNING ones to: the planned duration, capped by kates.engine.max-duration-ms, plus kates.engine.reaper-grace-ms, counted from creation. Each unfinished task gets an error that starts "Timeout: still pending" or "Timeout: still stopping" and says why. A run with no task, as a PENDING run has none, gets one, <id>-submission, that carries the error, so the API and a JUnit export say why it failed. The deadline is the only rule that can't fail a live run on another replica. Which process runs a run is not stored: handles and permits are in memory, and a PENDING run has no handle even in its own process, since handles are published after the RUNNING write. recoverOrphans doesn't tell runs apart either; it fails every RUNNING run, which is why the chart pins one replica. A "no handle here, older than a startup grace" rule would fail another replica's submission still under way, so it was left out. The deadline is stored with the run and bounds its whole life, so failing a PENDING or STOPPING run past it ends nothing sooner than the RUNNING reaper would. A PENDING run is settled only after its conditional write lands. Its submission in this process may store it RUNNING in between, and settling first would hand back the permit of a run that goes on. RUNNING and STOPPING runs are still settled first. The reaper-deadline caveat in kates mcp says this, and its ref to TestTimeoutReaper is re-pointed and anchored. The audit-actor ref to TestCleanupScheduler follows its javadoc, now two lines shorter. Books 02, 11, 12 and the glossary describe the new rule.

… code, not as a link Markdown Link Check failed on 12-deployment.md:80 with "Cannot find fragment" for 11-api-reference.md#delete-apitestsid. lychee reads the trailing {id} of "#### DELETE /api/tests/{id}" as a Pandoc attribute block, so it gives that heading no such anchor, although Quarto renders one. The link came with #353's sweep paragraph, and #353 fails the same check. Other chapters name such endpoints in code instead.

# Conflicts: # cli/cmd/mcp_tools_runs.go # docs/book/12-deployment.md # kates/src/main/java/com/bmscomp/kates/service/TestCleanupScheduler.java

bmscomp added 12 commits October 8, 2026 00:35

Merge fix/chart-cleanup-retention (#352, with main and #350 in it) in…

f7ada0a

…to fix/sweep-safe-audited The one conflict, book 11's DELETE /api/tests section, keeps both sides: #352's sentence on the admin scope, and this branch's description of the new sweep in place of the old one.

Merge remote-tracking branch 'origin/main' into fix/stale-runs-failed

8446607

# Conflicts: # cli/cmd/mcp_tools_runs.go # docs/book/12-deployment.md # kates/src/main/java/com/bmscomp/kates/service/TestCleanupScheduler.java

bmscomp merged commit 6856db7 into main Oct 8, 2026
64 of 66 checks passed

Navigation Menu

Repository navigation

fix(engine): the timeout reaper fails PENDING and STOPPING runs past their deadline, so no run stays unended for good - #354

fix(engine): the timeout reaper fails PENDING and STOPPING runs past their deadline, so no run stays unended for good#354
bmscomp merged 12 commits into
mainfrom
fix/stale-runs-failed

bmscomp commented Oct 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bmscomp commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Changes

Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bmscomp commented Oct 8, 2026 •

edited

Loading