Process contract: a bounded drain that exits 15, and a four-state health endpoint - #273
Merged
Conversation
turbolytics
marked this pull request as ready for review
September 13, 2026 14:58
turbolytics
marked this pull request as draft
September 13, 2026 19:17
…llback outlives it
…try ladder outlives it
…failures and retries
…ecords the process contract
…lways runs and reports, and the review's claims are corrected
turbolytics
force-pushed
the
feat/process-contract
branch
from
September 13, 2026 20:32
43cbda2 to
7b03ff8
Compare
turbolytics
marked this pull request as ready for review
September 13, 2026 20:32
turbolytics
added a commit
that referenced
this pull request
Sep 13, 2026
…ates CHANGELOG.md: main now keeps an Unreleased section. serve's entries join its Added list, and serve's limits sit under Known limits, rather than under a v1.3.0 heading of their own. The version is named when it is tagged. internal/errs/errs_test.go: both sides appended a test at the end of the file. Both are kept. internal/validate/validate.go merged without a conflict and with a bug: #273's checkDrainDeadline ran on every file, so a serve file reported pipeline.drain_deadline: pass for a pipeline it does not have. A check that could not apply must not read as a pass. Each file now gets its own checks: serve rules for a serve file, the drain deadline for a pipeline. TestValidateServe_AValidServeFilePasses now asserts no pipeline check reports on a serve file; it failed on the merged code and passes now. docs/coverage/matrix.md merged as text into a page that no longer matched its status files, and test_the_committed_page_is_current failed. make coverage-page regenerated it: 39 features, cli.serve covered at unit and release, 0 gaps. go vet ./..., go test -short -race ./... and pytest tests/tooling pass on the merge.
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
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.
Closes #161. Rebased on #279, which fixed the offset commit after a failed batch that the review of this PR found on main.
Design:
docs/superpowers/specs/2026-09-12-process-contract-design.mdPlan:
docs/superpowers/plans/2026-09-12-process-contract.mdWhat changes
pipeline.drain_deadline_seconds, default 30, bounds the whole shutdown after SIGTERM. Onecore.DrainBudgetis shared by the batch in flight, the drain of what is still buffered, both state syncs and every manager's final poll. When it passes, the process exits 15 withsystem.lifecycle.drain_incomplete. Retryable: nothing unwritten was committed, and the next start replays it./healthzreportsstarting,healthy,degradedorfailedwith areason.failedanswers 503 and the rest answer 200. The oldokandstuckbodies becomehealthyandfailed, and their HTTP codes do not change. Retries are tracked per sink as role/type.lifecycle.drain.boundedlosestracked_by: #161and is enforced.manager.drain.boundedis added and enforced.Decisions made during implementation
internal/config.validatecompares them and must not link DuckDB.system.lifecycle.internalis added besidedrain_incomplete, because the registry requires a catch-all in every domain.context.WithoutCancel. The DuckDB driver ignores the context, so this changes nothing there; it guards a transaction that honours it, and a test with one covers it.Adversarial review
An adversarial review of the first version found three critical and three important defects. Two of the critical ones were the offset bug on main, fixed in #279. The rest are fixed here: the flush aborted by a cancel, the manager's final poll skipped or its failure ignored, the health wiring untested, the retry key shared between sinks of one type, the validate branches untested, one failure counted three times, and a rollback "fix" that DuckDB never needed described as one.
Verified
go test -short -race ./...,go vet, gofmt,pytest tests/tooling: clean.lifecycle.drain.boundedin 5 seconds./healthzhealthy with idle-tick resets; SIGTERM exits 0 with 300 of 300 rows and offset 299.drain_deadline_seconds: 2: validate warned beforehand; SIGTERM exited 15 after 2 seconds; 0 rows written.Coverage status files follow from this PR's CI report artifacts.