fix(core): use effective step timeout for stalled tasks - #664
Merged
Conversation
🦋 Changeset detectedLatest commit: cb52bb5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
View your CI Pipeline Execution ↗ for commit cb52bb5
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
jumski
force-pushed
the
09-02-issue_621_effective_step_timeout
branch
from
September 2, 2026 00:40
bb4ae5b to
2131023
Compare
jumski
force-pushed
the
09-02-issue_621_effective_step_timeout
branch
2 times, most recently
from
September 2, 2026 00:58
7a9e910 to
256879e
Compare
Contributor
🔍 Preview Deployment: Website✅ Deployment successful! 🔗 Preview URL: https://pr-664.pgflow.pages.dev 📝 Details:
_Last updated: _ |
jumski
changed the base branch from
09-01-issue_645_failed_run_terminalization
to
graphite-base/664
September 2, 2026 01:11
jumski
force-pushed
the
graphite-base/664
branch
from
September 2, 2026 01:11
5bf15c7 to
7903661
Compare
jumski
force-pushed
the
09-02-issue_621_effective_step_timeout
branch
from
September 2, 2026 01:11
256879e to
ec55330
Compare
jumski
force-pushed
the
09-02-issue_621_effective_step_timeout
branch
from
September 2, 2026 01:11
ec55330 to
db903fb
Compare
Stalled recovery used only the flow timeout, so step overrides could leave tasks started long after their PGMQ messages became visible. Use the same step-over-flow timeout selection as start_tasks while preserving the 30-second recovery buffer, requeue limits, permanent-stall behavior, and the #645 parent-state guards. The PGMQ-only two-second visibility margin remains unchanged.
jumski
force-pushed
the
09-02-issue_621_effective_step_timeout
branch
from
September 2, 2026 01:32
db903fb to
cb52bb5
Compare
Contributor
Author
Contributor
🚀 Production Deployment: Website✅ Successfully deployed to production! 🔗 Production URL: https://pgflow.dev 📝 Details:
Deployed at: 2026-09-02T07:22:06+02:00 |
This was referenced Sep 2, 2026
jumski
added a commit
that referenced
this pull request
Sep 4, 2026
## Summary `start_tasks()` invoked `pgflow.set_vt_batch()` inside an unreferenced `SELECT` CTE, so PostgreSQL could skip the call and leave a claimed task with only the short initial PGMQ read visibility. A message then became visible while its `step_tasks` row stayed `started`, and workers repeatedly read it as a task that never ran. - The final statement now consumes a guard over the renamed `visibility_reset` CTE, making the visibility update structurally required. When `set_vt_batch()` updates fewer messages than were claimed, the statement fails with `start_tasks(): visibility updated N of M claimed messages`, rolling back the task transitions and attempt increments and returning nothing. - Effective delay stays `coalesce(step.opt_timeout, flow.opt_timeout) + 2`, matching the #664 stalled-recovery threshold. A repeatedly visible message whose task is already `started` claims nothing and consumes no attempt — a benign duplicate, per #651/#652 direction. - Consolidates the three unreleased Core migrations on `main` (`terminalize_skipped_tasks` #638/#649, `failed_run_terminalization` #645/#663, `effective_step_timeout` #621/#664) plus the development temp migration into one Atlas-generated `task_lifecycle_hardening` migration against the `0.15.0` baseline (#666). Both migration-only data repairs are preserved in order: active tasks under skipped steps become `skipped` **before** remaining active tasks under failed runs become `cancelled`. - Adds a `0.15.0` upgrade fixture (`supabase/upgrade_fixture/` + `scripts/run-upgrade-fixture`): starts from the `0.15.0` baseline, applies the consolidated migration, and asserts both historical repairs plus final runtime visibility behavior. Wired into the `core` `test:pgtap` nx target so CI executes it. - New pgTAP coverage for all seven #656 test bullets: effective-timeout visibility (shorter / longer / null fallback), rollback on total and partial visibility failure, benign duplicate batches, and mixed-batch atomicity. Visibility-delay checks use `clock_timestamp()` to match `set_vt_batch()`, and repeated duplicates recheck that the attempt count stays unchanged. - Removes the unsupported sqruff 0.39.0 `--force` option so the required `pnpm nx fix-sql core` migration step runs normally. - One `@pgflow/core` patch changeset. ## Checks - `pnpm nx verify-migrations core` — pass (incl. schemas-synced) - `pnpm nx gen-types core` + `verify-gen-types core --skip-nx-cache` — pass - `pnpm nx fix-sql core` — pass (37 files, nothing to fix) - `pnpm nx test:pgtap core --skip-nx-cache` — pass (288 files, 1352 tests; focused start_tasks suites 26/26; `upgrade fixture: PASS`) - `pnpm nx affected --target=prepush --base=origin/main --head=HEAD` — 28/28 successful - `atlas migrate validate` — pass; `atlas.sum` re-hashed after the manual repair edits Closes #656
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.

Summary
Fix stalled-task recovery to use the effective step timeout instead of always using the flow timeout.
This PR stacks directly on #663 (
09-01-issue_645_failed_run_terminalization) and preserves its run, step-state, and task eligibility guards.Root cause
start_tasks()sets PGMQ visibility from the effective timeout:requeue_stalled_tasks()used onlyflows.opt_timeout. A short step timeout could therefore make the PGMQ message visible while its task row remainedstarteduntil the longer flow timeout and recovery buffer expired.Behavior
Recovery now requires:
The comparison stays strict. A null step timeout inherits the non-null flow timeout.
The PGMQ-only two-second margin is not added to recovery. Adding it again would change the existing 30-second recovery grace to 32 seconds. The 15-second cron cadence can add up to roughly 15 seconds after eligibility.
The change preserves:
run.status = 'started',step_state.status = 'started', andtask.status = 'started';permanently_stalled_at is nullandFOR UPDATE OF task SKIP LOCKEDbehavior;set_vt_batch(..., 0);Tests
Added
effective_step_timeout.test.sqlwith deterministic timestamps inside one transaction:Before the source fix, the focused test failed 5 of 10 assertions for the expected reason. The short step override returned 0 and stayed
startedat 36 seconds. The long step override requeued at 36 seconds, so its later 91-second call returned 0.After the source fix:
Migration and release note
Atlas generated
20260901203454_pgflow_temp_effective_step_timeout.sql. It replacespgflow.requeue_stalled_tasks(), includes the cumulative #645 guards, and performs no backfill.Added a separate patch changeset for
@pgflow/core. The fixed release group expands the patch at release time.Checks
pnpm nx verify-migrations core --skip-nx-cache— passpnpm nx gen-types core --skip-nx-cache— pass; no generated type diffpnpm nx verify-gen-types core --skip-nx-cache— passpnpm nx test:pgtap core --skip-nx-cache— pass; 285 files, 1326 testspnpm nx test core --skip-nx-cache— passpnpm nx lint core --skip-nx-cache— pass; 0 errors and 2 existing type-test warningspnpm nx build core --skip-nx-cache— passpnpm changeset status— pass; patch fixed group detectedgit diff --check— passpnpm nx fix-sql corehit the known Sqruff CLI mismatch:error: unexpected argument '--force' found. The direct repository fallback,sqruff --config=.sqruff fix --parsing-errors pkgs/core/schemas/, processed 37 files and found nothing to fix. Direct Sqruff lint also passed.Two fresh independent Sol xhigh review rounds returned
APPROVEDwith no required findings.Out of scope
start_tasks()visibility CTE;The stack still contains temporary migrations, so the main-targeted temporary-migration check can fail until the settled release sequence consolidates them. This PR does not consolidate the parent migration.
Fixes #621