Skip to content

Take the run lock for agac retry --filter - #1710

Closed
Muizzkolapo wants to merge 3 commits into
locks/one-run-per-workflowfrom
issue-1698-filter-run-lock
Closed

Muizzkolapo wants to merge 3 commits into
locks/one-run-per-workflowfrom
issue-1698-filter-run-lock

Conversation

@Muizzkolapo

@Muizzkolapo Muizzkolapo commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

Closes #1698. Stack D, 1 of 2, on your #1695 → #1696 (it needs hold_run_lock). Next: #1699 fix.

Problem

agac retry --filter goes to RetryFilterCommand before RetryCommand.execute, so it never took the run lock that #1696 adds. It rewrites:

  • dispositions;
  • checkpoint rows;
  • stored target rows;
  • the status file;
  • the retry manifest.

During a run or retry of the same workflow, the run's next status save could drop its change, or the retry could put its snapshot back over the filter.

Change

  • RetryFilterCommand.execute holds hold_run_lock for the whole command, taken before the store is opened, as RetryCommand does. The body moves to _filter.
  • A --dry-run writes nothing and still reads past the lock (decision taken as both Refuse a second run of a workflow while one is in progress #1696 reviewers recommended: lock it).
  • docs/reference/cli/retry.md: the sentence saying --filter does not take the lock is corrected. The CLI manifest is updated.

Verification

  • tests/integration/test_a_second_run_of_a_workflow_is_refused.py gains two tests:
    • test_a_filter_is_refused_while_another_holds_the_workflow: fails without the change, passes with it;
    • test_a_filter_dry_run_is_not_refused.
  • The file runs 8 passed. Filter tests across the suite: 17 passed.
  • ruff, ruff format and mypy are clean.

Muizzkolapo and others added 3 commits November 8, 2026 01:08
LockDefaults.SIMPLE_LOCK_TIMEOUT_SECONDS had no reader. Every portalocker
lock in the code is in the run tracker, and each one uses the atomic
timeout. The internal defaults page described a shared/read lock that
does not exist. Remove the constant, the tests that pinned its value, and
its docs row.
Nothing above SQLite stopped a second `agac run` of a workflow. Two runs
started half a second apart both did all the work, and in batch mode both
submitted a batch job, which doubles the spend with a real vendor.

`agac run` and `agac retry` now hold an advisory, non-blocking, exclusive
file lock beside the workflow's database for the whole command. They take
it before the workflow loads, because `--fresh` clears the store inside
the load; a later lock would wipe the first run's rows before refusing.
A second run exits 1 and names the holder's pid. The operating system
releases the lock when the holder exits or is killed, so no stale lock
can remain and no bypass flag is needed. Read-only commands, `retry
--dry-run` and `retry --filter` do not take the lock.
--filter goes to RetryFilterCommand before RetryCommand.execute, so it
never took the run lock #1696 adds. It rewrites dispositions,
checkpoint rows, stored target rows, the status file and the retry
manifest. During a run or retry of the same workflow, the run's next
status save could drop its change, or the retry could put its snapshot
back over the filter.

RetryFilterCommand.execute now holds hold_run_lock for the whole
command, taken before the store is opened, as RetryCommand does. A
dry run writes nothing and still reads past the lock.

Closes #1698

Signed-off-by: Muizz Lateef <Lateefmuizz@gmail.com>
@Muizzkolapo
Muizzkolapo force-pushed the locks/one-run-per-workflow branch from de8d2bd to 78faece Compare November 8, 2026 14:44
@Muizzkolapo
Muizzkolapo deleted the branch locks/one-run-per-workflow November 8, 2026 16:02
@Muizzkolapo Muizzkolapo closed this Nov 8, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Nov 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant