Stop deleted workflows from coming back after a stale edit#2489
Open
BradGroux wants to merge 1 commit into
Open
Stop deleted workflows from coming back after a stale edit#2489BradGroux wants to merge 1 commit into
BradGroux wants to merge 1 commit into
Conversation
Co-authored-by: Brad Groux <bradgroux@hotmail.com> Signed-off-by: Brad Groux <bradgroux@hotmail.com>
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.
Fixes #2390.
What users saw
A deleted workflow could return after someone edited a stale copy. A later
delete removed the workflow row again, but the replacement definition could
remain queryable on the relay.
The failure sequence was:
E2.
rejected the missing row.
Why deletion missed the current definition
Canonical workflow deletion follows this path:
kind:5→validate_standard_deletion_event→handle_standard_deletion_event→handle_a_tag_deletionFor
30620:<owner>:<workflow UUID>, the workflow-specific branch deleted theworkflowscontrol-plane row and returned before the generic coordinatetombstone path ran. The current kind:30620 definition therefore survived.
What changed
Canonical workflow deletion now uses one database transaction that:
kind:30620 replacement writes;
owner;
owner, and
dtag;The shared lock closes the writer/deleter race. A concurrent replacement either
finishes before deletion and is tombstoned, or begins after deletion commits.
The path is idempotent, repairs orphaned definitions when the row is already
gone, invalidates workflow caches after either side changes, reports row and
event deletion counts, and reuses the authoritative event-kind constants.
Safety and scope
The existing validator still proves that the signer owns the coordinate, or is
the owning human for an agent. The database mutation independently scopes both
the row and events by community and owner.
This PR intentionally:
dtag used by current workflow writers;Those remain separate policy, migration, and UX decisions.
Regression proof
The database lifecycle regression covers create/delete, recreation, multiple
simultaneously-live matching definitions, wrong-owner no-op, orphan cleanup,
and repeated idempotent deletion.
The concurrency regression holds an uncommitted replacement under the shared
advisory lock, proves deletion waits, commits the writer, and proves deletion
removes both the row and replacement.
The signed relay end-to-end regression publishes E1, deletes and queries zero
definitions, publishes E2, deletes again, and again queries zero definitions.
Verification
Source tree
c44ddf5b16156767169836b1256a59893465c0ba:just testjust ciOne full integration attempt hit the unchanged
steer_folds_into_active_turn_without_cancellingtiming test inbuzz-agent.The PR does not touch
buzz-agent; the test then passed 10/10 in isolation, andthe subsequent full
just testrun passed.GitHub rejected the first push because the repository-configured author email
is protected. The current head
a350356fe9cc0c13e12bba3ac86fd4a17a1fef51is a metadata-only rewrite toBrad's GitHub noreply identity. It has the exact tree above and an empty diff
from the fully verified commit. Both focused PostgreSQL regressions were rerun
successfully at the current head, and the required
Co-authored-byandSigned-off-bytrailers remain unchanged.The end-to-end proof used a local relay with PostgreSQL, Redis, and S3-compatible
storage. It validates the signed relay protocol path; it is not production
deployment evidence.