Repository navigation
feat(workflows): #716 chain policy, bounded retry, capability guard, durable terminal effects - #727
Conversation
…ntent PRE migrations 01.00.38_01: nullable jobs/jobs_archive required_worker_schema + the physical entry-to-running claim trigger (WF001). 01.00.41_01: nullable run_epoch for guarded effect identity. 01.00.41_02: the unique wf_effect_key intent arbiter. All three forward-only PRE files, inert for legacy NULL rows; the manifest gains exactly these three entries.
…rable terminal effects One execution-policy resolver (_policy.py) with ExecutionPolicy/RetrySpec/ TerminalEffect/ExhaustionEvent/FiniteReservationCohort lowering into chain starts, forks, emits, outbox consumers and row stamps; the claim-owned full-fenced attempt disposition (api/_attempt.py) owning every ordinary re-pend/reclaim/cancel-fence verdict; the guarded cohort/capacity-owner projection at boot and dispatch; exactly-one durable effect intent committed with the winning terminal under the root lock; the bounded non-consuming advertised retry (durable metadata counter, budget-free) with its typed exhaustion; direct-driver guarded refusal and the worker boot capability stamp (level 2). Legacy Snooze/RetryAfter semantics unchanged; the ledger claim arbiter stays four-expression at this slice.
Focused real-PG modules for the policy resolver/ladder rows, the claim-fenced dispositions, the capability guard (three-key migration inventory at this slice), the fleet policy surface, and the terminal-effect intent/replay/ cancel-boundary semantics; the two-process chain deadline/curve/capacity-owner roll and the real-kill effect system cases; the claim-identity threading in the retained routing/loop/snooze/ledger pins; the negative type-probe corpus entry; and the guide/api-reference/upgrade docs for the declared surface.
D1: the upgrade runbook names the complete required PRE set in order (01.00.38_01, 01.00.41_01, 01.00.41_02) with the create-failure mode and the worker boot gate; the guide and API reference name the capability PRE too. D2: the user-facing effect timing states the probed invariant — the effect runs after its own node's failed terminal; the run's root stays nonterminal while the intent is undelivered or the effect job is unfinished, and the root's failed terminal is derived by the run's maintenance only after the effect settles; a cancel-first ordering terminalises the root cancelled and the committed intent still runs. D3: the internal gate name, the stale 'future boundary' claim, and the internal SQL constant name are gone from user docs. Dead duplicate runner lookup removed; the sequential worker-replacement system test's docstring is accurate.
…cumented contract
RULING_716_D2 (B): the documentation was wrong; the production sweep order
is acceptable. The user docs now state the probed contract: the effect runs
after its own node's failed terminal and may execute after the run root is
already failed or cancelled; root maintenance derives status from graph node
rows and may derive failed while the intent is undelivered and before an
effect job exists, so a terminal root does not prove the effect settled; the
stable effect key guarantees exactly one intent and one effect-job
materialization; execution/external effects are at-least-once. The ruling's
required probe is a real test on real PostgreSQL: wf_join_rederive before
wf_outbox_drain with no preliminary drain, asserting and printing all five
stages. The E1 residual ('column default' -> hard-coded transient bind) is
corrected in the author report.
…st (final-review finding 1) The late-priority phase's insert-clock/claim-clock inference (created_at as commit visibility; started_at across overlapping dispatch snapshots) was flaky under legal scheduling shapes. The phase is now deterministic: the priority chain's source runs on its own control actor/queue cohort with a control worker (no default-queue dispatch in flight), the emitted priority-9 row's commit is observed as 'pending' on the test's independent connection, the control worker is reaped, and the default workers phase in — the first claiming one job per round, so its own earliest attempt IS its first eligible claim (a durable single-worker record, never a cross-worker clock inference). Both clock-based assertion blocks are removed. The opposite-direction control seeds a higher-priority backlog and asserts the chain row is NOT the first claim. The fixture's backlog actor now succeeds (payload+JobContext signature — it had never succeeded) with a 1s workload so both worker processes overlap on the backlog.
…leet-test determinism (DeepSeek4)
The late-priority primary test's every count/attempt query is scoped to
THIS run's exact rows: the 12 inserted backlog ids (one INSERT..RETURNING)
and the high row's id captured at the commit-visibility handshake; the
first-work assertion binds the worker's earliest attempt to that exact
high-row id, so a stale priority-9 row from another flow can never satisfy
it. The inverted control scopes its own queries the same way and deletes
its rows/attempts/ledger deterministically in finally (cleanup failures
print loudly, never masking the verdict). The fleet test's two claimants
are phased: the second dispatch starts only after the winner's gate holds
the slot (entered), making 'not-claimable' deterministic; the
concurrent-dispatch admission race's other legal outcome ('denied') stays
the documented fallback, not this test's coin flip.
…s actual path (DeepSeek5) The deterministic post-entered path performs zero mark_snoozed denials (both reserved rows attempt 1, snooze_count 0), so the leak message, the stale denial/floor comment, and the completion timeout message now describe the actual path: the headroom fence refused the second dispatch; the post-claim mark_snoozed denial stays the documented fallback, covered by test_wf_capacity_enforcement_pins.py. Comments and assertion messages only; no assertion logic or behavior change.
… and the exact first-work handshake (final2 findings 1-2) Finding 1: the compact two-cohort control on two real worker processes (the diagnostic worker entry: the same app-import capability through the production worker_main_async wrapper, so the boot's INFO-level capacity diagnostics surface in the worker's own log sink). Cohort A saturates at its strict slot (its second row stays unclaimed while the first holds the slot — the headroom fence, observed on the test's own connection inside A's 20s workload window); cohort B's rows claim and complete inside A's execution span (durable attempts + ledger-window containment); both actor_config rows assert the operator's non-overwrite (2) and the code's deployed literal (1); the operator-owned mismatch diagnostic (actor-config-capacity-override naming cohort A) fires on both real boots, and the matching code-owned cohort logs nothing. cron_registry=[] in the diagnostic entry keeps the march apps' cron flows out of the control's schema. Finding 2: the late-priority phase's worker B starts only after an independent connection observes worker A's claim of the EXACT high-row id (status running + locked_by_worker = A's registered row) — no legal schedule can put B's dispatch ahead of A's first claim. The exact-id first-work assertion is retained.
…rable attempt-window cohort proof (DeepSeek6 findings 1-3) Finding 1: the cohort control's evidence is now durable and causally ordered, from job_attempts on the exact rows (an attempt row materializes at its attempt's end; started_at is the claim instant copied from the job row): A1 is exactly one gate-held attempt; every B attempt sits strictly INSIDE A1's attempt window (B claimed after A1's claim — the phased spawn — and finished before A1's release-gated completion) and B's attempts belong to the second real process; A2's first attempt starts only AFTER A1's attempt finished. The claim handshake reads the row's durable claim state (running + locked_by_worker = A's registered id). The comment's untestable 'exact attempts' claim is replaced by exactly these provable window assertions. Finding 2: both fixed sleeps (the 20s cohort-A body, the 10s priority body) are replaced by the test-set release gate — a schema-scoped e2e_release_gate row the TEST stamps (open_at) once its own observation is complete (the exact-row claim witnessed, the second worker genuinely joined, the fence's refusal seen). The body polls the gate through its own WorkerSettings-local connection (the cron bridge's DB-local idiom); the hold is occupancy, deliberately NOT a HITL park — a held outcome releases the slot, while a body mid-await keeps the worker's slot, the row's lock and the cohort's reservation genuinely held (the heartbeat renews the leases). A never-opened gate fails the attempt loudly (bounded 240s guard). Both tests seed their gate closed and clear it in finally (release-on-failure keeps reap graceful); the inverted control seeds and clears the same gate. Finding 3: the report's stale file counts corrected (63 files +10,168/−450 at 6dad083; the round-6 correction 4 files +350/−6), and the '8s body' comment removed with the sleep it described. Counterfactuals (both run, both RED, both reverted): a broken fence (reservation slots 1→2 + worker concurrency 2) fails the live fence assert — A2 claimed while A1's window is open; a delayed-A schedule (B completes before A's source even runs) fails the durable containment — B's attempts sit outside A1's window.
…ort describe the real reap order (DeepSeek7 §8) Wording-only (no behavior, no assertion change): reap is a hard kill (proc.kill(), _harness.py:390-391 — no grace phase). The priority test's finally reaps FIRST, so its open/clear is row hygiene (no gate-held body survives a failure path), not a pre-reap release; the cohort control's finally opens and clears BEFORE its reap loop (the release is issued while the workers are alive); the inverted control's cleanup also runs after its own reap. The report's round-7 sentence now states both shapes exactly.
…t states the real reap order (final3 Kimi finding 1)
Comment-only (no behavior, no assertion change): the inverted-priority
control's arrange comment still promised 'the cleanup below releases it
so reap stays graceful' — the finally hard-reaps first (proc.kill(), no
grace phase) and the open/clear afterwards is row hygiene. Full sweep of
the test file, _wf_app.py, _wf_harness.py and _wf_diag_entry.py for
grace/graceful/SIGTERM/terminate claims about reaping: every remaining
mention either negates grace about reap ('hard kill — no grace phase')
or describes the engine's own SIGTERM-drain termination budget — no
false reap-grace claim remains.
…tatement's moved keyed predicate The PR moved and rewrote NODE_REPEND_SQL_TEMPLATE from the ladder into workflows/api/_attempt.py as a params-CTE statement; the repended UPDATE's write predicate is now WHERE j.id = params.job_id (with the status/locked_by_worker/attempt/claim_epoch/ledger-claim/cancel fences), so the old scoping substring 'WHERE id = $1' no longer appears and the bounded-write audit's tripwire fired in CI (six failed jobs, one deterministic signature). The re-review confirms the statement is still keyed to ONE claimed row — the registry substring follows it, consistent with the sibling NODE_DEADLINE_REPEND_SQL params-based entry. Non-weakening proven on a clean git archive of 70bcfd1: base as-is passes (4 passed), and base with THIS registry change fails (base's runner_ladder template lacks the head's predicate) — the new substring pins the head's keyed predicate, it cannot pass a widened statement. Consistency sweep of the whole _EXEMPT registry against the current tree: 86 entries, every scoping substring matches, 0 stale keys, 0 now-LIMITed statements.
…1 refactor deleted _run_tx1's fenced-out path lost its else arm in 7cbeac1 (the _Tx1Outcome refactor's hunk): a finalize fenced out WITHOUT a ledger id (the Exit path's _finalize_exit call site — api/_runner.py:1484 — never passes one) stopped marking its running wf_step_ledger row 'fenced', leaving the record to the terminal-flow phantom reap and breaking the probe's contract (the ledger says 'fenced', tx2 never runs). Restores the deleted arm verbatim (same statement, same five bind arguments, the arbiter tuple flow/step/attempt/map_index, the status-only fence a no-claim-instant caller keeps) under the head's _Tx1Outcome shape; the by-ID arm's accepted claim-instant guard is untouched. wsql.ledger_fence_attempt regains its production call site; the probe (base-passing, head-failing on all three CI Pythons) is the red-first control.
…removed and its docstring states the real executed path DeepSeek's fence review (§7): _finalize_exit's ledger_id/ledger_claimed_at parameters are supplied by NO caller — the sole call site (api/_runner.py:1484) passes only claim_epoch, and no test or doc calls the method otherwise — so the executed ledger terminal is the by-attempt wsql.ledger_terminal write (the node's arbiter tuple), the same behavior base had, while the docstring claimed the EXACT claim identity (row id + instant). Dead declaration plus false documentation. AST sweep across src/ and tests/: exactly one _finalize_exit call site (no ledger_id/ledger_claimed_at), zero test references; docs/examples grep clean. Per the standard's dead-declaration prohibition the two parameters are removed (finalize_node's own defaults are unchanged — behavior identical), and the docstring now states the real guarantee: the ledger terminal is the by-attempt write keyed by the node's arbiter tuple; the JOB row's terminal stays the claim CAS (terminal_mark binds the claim's worker/attempt/epoch); a fenced-out claim writes nothing and its ledger row reads 'fenced' by the same tuple. No caller is invented; threading the claim id into the Exit path would be a behavior change and is left to the contract's owner.
|
Repairs since the draft was opened, after CI and independent review found them:
Process note, stated plainly: the first pushed version carried two real defects because local gate runs were void — the venv editable Attribution of the remaining local reds (separate investigation, proven on frozen base and head trees): the IPv6 test is environmental (warm PG publishes Reviews on this HEAD: DeepSeek CLEAN ( |
Fixes #716. Slice 1 of the workflow adoption landing plan (
SPEC_LANDING_WORKFLOWS.md), rebased ontomainat70bcfd1d7.What this slice ships
Migrations (
01.00.38_01_pre_wf_worker_capability,01.00.41_01_pre_wf_run_epoch,01.00.41_02_pre_wf_terminal_effect_intents) plus theirreleased_migrations.sha256entries. The claim and its arbiter remain four-expression at this merge point; the five-expression claim epoch arrives with slice 3.Product
api/_attempt.py);TransactionalStepUnsupportedErrorrefusal.Docs describe the behavior that actually ships. The effect contract was corrected after a specification ruling: an effect runs after its own node's
failedterminal and may execute after the run root is alreadyfailedorcancelled; a terminal root does not prove the effect settled.Tests
Red-first: six new unit modules and both system controls fail on clean
origin/mainfor documented absence, plus six behavior pins with exact causes. One control passes on base and is labelled a preserved-behavior pin rather than claimed red-first.tests/test_wf_*.py -n 21072 passed with only the recorded pre-existingBrokenPool.acquirefake-shape warningactor-config-capacity-overridefires forwf-policy-cap-aandactor-config-capacity-code-changestays silentVerification
Ruff clean (1706 formatted); tree-pinned Pyright 1.1.414 0 errors, 0 warnings; ty 0.0.85 49/49 MUST_ERROR markers on both checkers.
Independent review
2f5b788d…(REVISE, 3 doc defects + 1 report cause),82a6ab2c…(REVISE, D2 docs),241382ab…(CLEAN),4e292765…(REVISE, test isolation false-green),dc25f760…(REVISE, stale comments),18c08b35…(REVISE, snapshot evidence),923bd88d…(CLEAN)eb043ef5…(REVISE, flaky test),2792daab…(REVISE, missing fleet controls),3740b728…(REVISE, leftover reap-grace claim),792a0a36…(CLEAN)Every finding since the first red team was in tests, docs, or the claims about them; the product code has been stable since round 1.
Known limits
test_wf_demo_legs.py::test_leg2_the_kill_and_resume_drillis an independent, pre-existing load-sensitive kill/reclaim flake, reproduced on base and excluded here by three separate reviews.run_epocheffect roots fail at terminal after the body rather than by a typed pre-body refusal; fleet WF001 is not yet mapped to a typed worker-loop refusal. Both are documented, not hidden.