Skip to content

fix(VIOL-0034): cascade-skip instead of crashing when all version branches filtered#763

Merged
Muizzkolapo merged 3 commits into
mainfrom
fix/viol-0034-version-merge-cascade-skip
Jul 6, 2026
Merged

fix(VIOL-0034): cascade-skip instead of crashing when all version branches filtered#763
Muizzkolapo merged 3 commits into
mainfrom
fix/viol-0034-version-merge-cascade-skip

Conversation

@Muizzkolapo

Copy link
Copy Markdown
Owner

Summary

  • A version_consumption: merge action whose every version branch was guard-filtered crashed the pipeline with ConfigurationError: Version correlation failed (exit 1). All-branches-filtered is a legitimate low-density input shape, not a misconfiguration. Root cause: resolve_correlated_input (workflow/managers/output.py) raised unconditionally whenever correlation returned nothing, and the executor called it uncaught (_execute_action_run, executor.py:1145, outside the try/except).
  • resolve_correlated_input now distinguishes all-sources-empty (every version source has no target files) → raises the typed AllVersionsFilteredError — from a genuine correlation failure (some source had output) → still raises ConfigurationError.
  • The executor catches AllVersionsFilteredError at both action-run sites (sync + async) and resolves the action as SKIPPED (reason=all_versions_filtered, node-level disposition), mirroring the existing _handle_all_guard_filtered "produced nothing → SKIPPED" pattern — so the pipeline exits 0. Downstream consumers then cascade-skip via the normal mechanism.

Verification

  • New tests/unit/workflow/managers/test_resolve_correlated_input_all_filtered.py, RED→GREEN:
    • all sources empty → AllVersionsFilteredError
    • some source has output → ConfigurationError (genuine failure preserved)
    • executor _execute_action_run cascade-skips (SKIPPED, no raise) instead of propagating — pins the actual crash path, not just the isolated raise
  • Existing test_version_correlation_failure_raises_error comment corrected: its mock storage reports outputs, so it exercises the genuine-error ConfigurationError path; still green.
  • Blast radius: tests/unit/workflow/ + tests/core/graph/826 passed. ruff check/format clean; uv run mypy agent_actions → no issues (447 files).

Adds the ALL_VERSIONS_FILTERED reason and the AllVersionsFilteredError class
(data only) and pins the target behavior: resolve_correlated_input must raise
AllVersionsFilteredError when every version source is empty, and the executor
must cascade-skip (SKIPPED) instead of letting it propagate. The all-empty and
executor tests fail today; the detection + catch land next.
…nches filtered

resolve_correlated_input now distinguishes all-sources-empty (every version
branch produced no output — e.g. all guard-filtered) from a genuine correlation
failure. The former raises AllVersionsFilteredError, which the executor catches
at both action-run sites and resolves as a SKIPPED action (reason=
all_versions_filtered, node-level disposition) so the pipeline exits 0. A genuine
correlation failure (some source had output) still raises ConfigurationError.
@Muizzkolapo

Copy link
Copy Markdown
Owner Author

Code review

Found 1 issue:

  1. _handle_all_versions_filtered writes the node-level SKIPPED disposition through an unguarded storage_backend.set_disposition(...), bypassing the _write_skipped_disposition helper (executor.py:686) that every other skip path uses (e.g. _skip_action_upstream_failure). That helper wraps the identical call in try/except Exception ... logger.warning(...); here a storage write failure (e.g. a SQLite lock) would propagate out of _handle_all_versions_filtered and out of _execute_action_run, crashing the very path meant to cascade-skip gracefully. Route through _write_skipped_disposition (extended with an optional detail=) or wrap the call in the same try/except.

storage_backend = getattr(self.deps.action_runner, "storage_backend", None)
if storage_backend is not None:
storage_backend.set_disposition(
params.action_name,
NODE_LEVEL_RECORD_ID,
DISPOSITION_SKIPPED,
reason=ALL_VERSIONS_FILTERED,
detail=f"All version sources filtered: {avf.version_sources}",
)

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

The cascade-skip handler wrote the node-level skip disposition with an
inline, unguarded set_disposition call. set_disposition re-raises on a
storage error, so a DB/disk hiccup while recording the skip would
propagate out of the very path added to prevent a crash.

Route the write through _write_skipped_disposition, which already wraps
set_disposition in best-effort try/except with a warning (matching the
failed-disposition sibling), extended with an optional detail argument
truncated like reason. Removes the duplicated write.

Tests: a raising backend on the skip path (sync and async run sites)
still resolves SKIPPED without propagating; the node-level record id and
reason handed to set_disposition are pinned.
@Muizzkolapo Muizzkolapo merged commit 884cf30 into main Jul 6, 2026
5 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 6, 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