Repository navigation
fix(engine): the API's daily run sweep deletes only finished runs, in batches, each through the run delete with an audit row - #353
Merged
Conversation
…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.
…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.
…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.
bmscomp
added a commit
that referenced
this pull request
Oct 8, 2026
… 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.
…eading
Markdown Link Check failed on #delete-apitestsid. lychee's parser reads
a trailing {id} on a heading as an attribute block, so to it
"DELETE /api/tests/{id}" is the heading "DELETE /api/tests" and has no
such anchor, while Quarto reads {id} as text. No anchor satisfies both
checkers, so the two mentions are plain code.
…/sweep-safe-audited
bmscomp
added a commit
that referenced
this pull request
Oct 8, 2026
…their deadline, so no run stays unended for good (#354) * fix(chart): the test-run cleanup CronJob deletes old runs, through a 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. * fix(chart): the README describes the API's own sweep without the statuses 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. * fix(engine): the API's daily run sweep deletes only finished runs, in 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. * fix(cli): kates mcp's audit texts name the prune endpoint and the daily 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. * fix(engine): the timeout reaper fails PENDING and STOPPING runs past 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. * docs(book): book 12's sweep paragraph names DELETE /api/tests/{id} in 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#352 has merged (e0490ab), and main is merged in here (06f075f), so this PR's diff against main is its own work. 4665093 also stops linking book 12 to the
DELETE /api/tests/{id}heading. The link checker reads that heading's trailing{id}as an attribute block, so the anchor doesn't exist for it.Why
TestCleanupScheduleris the Kates API's own retention sweep: daily,kates.cleanup.retention-days, 90 by default. #352 found it unsafe:findAll(), results included, once a day;repository.delete, which neither settles a run's tasks nor writes an audit row;createdAtit could not parse;Changes
RunRetention(new,service/): the delete loop of #352'sDELETE /api/tests, now shared.count(statuses, before), andprune(statuses, before, limit, auditDetails), which returnsPass(matched, deleted, remaining).prunecounts, takes at mostlimitids oldest first, deletes each throughorchestrator.deleteTest(id), and counts again.TestResource.pruneTestscalls it. Its answers, 400s, audit details and log line are unchanged, andTestResourcePruneTestpasses unmodified.TestCleanupScheduler: same identity, property and 24 h schedule, and no more@Transactionalaround the whole sweep.retention sweep: created before <cutoff> (kates.cleanup.retention-days=<n>).deleteTestfinds a run gone and writes no row.system:scheduler(Actor.SCHEDULER, from feat(api): every change names who made it in the audit log, over REST and gRPC #351).RunRetention.prunetakes anActor. The endpoint passes none, so its rows keep naming the caller and close the request's audit trail.kates mcp: #351'saudit-actor-recordedcaveat now says the daily sweep leaves asystem:schedulerrow for each run it deletes, with anchored refs to where it records. (This branch's earlier edit toaudit-no-actorwent with that caveat when #351 retired it.) Refs intoTestResourceare re-pointed through a line map from #352's head, with a new anchored ref toRunRetention's audit row.Docs: books 10, 11 and 12, and
kates/docs/api-reference.md, describe the new sweep. Book 12 notes that each replica also sweeps when it starts. The chart README's sentence about the sweep was reworded in #352 (01a26b0) to stay true either way, so this PR touches nothing undercharts/and needs no chart bump.Merge: #352's latest head (with main and #350) is merged in. The one conflict, book 11's
DELETE /api/testssection, keeps both: #352's sentence on theadminscope, and this branch's description of the sweep.Not done, filed separately: with STOPPING runs no longer swept, nothing ends a stale PENDING or STOPPING run. The reaper and orphan recovery read RUNNING only, and the reconciler reads only this process's handles. That needs a rule that can't fail a live run on another replica. It's filed as its own task ("Fail stale PENDING and STOPPING test runs").
Tests
TestCleanupSchedulerTest(5 tests).sweep(2002-01-01)deletes exactly the five DONE and FAILED ones over several passes, each with one audit row;RunRetentionmocked:TestResourcePruneTest(23) is unmodified and passes.TestResourceDeleteTestsets the new field.application.propertiescomment ("archive runs") that this branch didn't make worse../mvnw test -Dquarkus.grpc.codegen.skip=true -Dquarkus.http.test-port=0: 1,449 tests, 0 failures, before feat(api): every change names who made it in the audit log, over REST and gRPC #351. After the merge, a full run of 1,455 tests failed only 3 ofTestCleanupSchedulerTest's mocked cases, which still expected the old audit call. Those were fixed, and the class (6 tests) passes with the prune, delete, audit and scope tests.spotless:checkon JDK 25: clean.cd cli && go test -p 1 ./... -race -count=1andgolangci-lint run --new-from-rev=origin/main ./...: 0 issues.scripts/check-cli-style.sh,scripts/check-cli-compat.sh,scripts/check-book-style.shandgen-version-matrix.sh --check.python3 -m unittestineval/mcp: OK.*ITtests and the Kind tests.