test(e2e): workflow example 15 — disaster recovery, rollback a failed apply (trusted profile) #802

Closed
freemo wants to merge 1 commits from test/e2e-wf15-disaster-recovery into master
Owner

Summary

E2E test for Workflow Example 15 — disaster recovery: rollback a failed apply using the trusted profile. Tests plan rollback, plan correct (revert mode), correction diff, re-execution after correction, and the full recovery lifecycle.

Closes #761

ISSUES CLOSED: #761

Changes

  • Decision ID extraction (Major fix): Replaced generic Extract First Id From Text with dedicated Extract Decision Id From Tree keyword that uses a targeted regex ("decision_id"\s*:\s*"<ULID>") to avoid extracting plan_id from JSON tree output. This ensures plan explain and plan correct receive the correct decision ID.
  • Checkpoint ID extraction (Major fix): Replaced broken first-ULID extraction from plan artifacts with dedicated Extract Checkpoint Id From Status keyword that targets last_checkpoint_id / checkpoint_id fields in plan status --format json output. Removed the dangerous fallback that silently used plan_id as checkpoint. When no checkpoint is available, the test gracefully skips with a clear message via Skip If.
  • AC#9 final status assertion (Major fix): Removed tautological terms ('phase', 'errored', 'cancelled') from the terminal state assertion. After successful recovery, only 'applied' or 'completed' are accepted as valid terminal states.
  • Return code assertions on recovery steps (Major fix): Added Should Be Equal As Integers ${r_X.rc} 0 assertions to all critical recovery commands including plan diff (step 14), which previously lacked an rc check. The initial plan apply and plan artifacts still use expected_rc=None without rc checks since those may legitimately fail.
  • ROOT CAUSE annotation enforcement (Major fix — AC#4): Added a secondary plan tree --format plain request specifically for the ROOT CAUSE display annotation check. Also searches JSON tree and status output for root_cause/root cause references. Logs WARN (rather than failing) when not found, since LLM behavior is non-deterministic and may not annotate root cause.
  • Checkpoint skip with explicit documentation (Major fix — AC#6–AC#9): Replaced terse Skip If with thoroughly documented skip explaining when checkpoint absence is expected (LLM behavior) vs when it indicates a problem. Added guidance to investigate persistent skips.
  • Errored state verification (Major fix — Issue #3): Added dedicated assertion after initial apply/status to verify the plan entered an errored/failed state. Logs WARN when state is not errored (LLM may produce a successful plan), continuing with recovery steps regardless.
  • Correction ID extraction (Major fix — Issue #5): Replaced generic Extract First Id From Text on combined stdout+stderr with dedicated Extract Correction Id From Output keyword that targets the "correction_id" field specifically. Falls back to first ULID from stdout only (not stderr) to avoid extracting stray IDs.
  • Dead ELSE branch removed (Major fix — Issue #6): Removed unreachable IF/ELSE guard on correction ID extraction (rc==0 already guaranteed by prior assertion). Correction ID is now always extracted directly.
  • AC#3 status assertion tightened: Removed generic 'execute' term that matches field labels in every status output. Kept only specific state values: 'processing', 'errored', 'applied'.
  • AC#5 explain assertion improved (Issue #7, #20): Removed overly generic terms ('decision', 'question') that would match any explain output. Replaced with specific forensic content terms: 'rationale', 'snapshot', 'alternative', 'chosen', 'confidence', 'context'.
  • AC#7 correct assertion improved (Issue #7): Removed generic 'correct' (matches command name) and 'status'. Replaced with more specific terms: 'correction_id', 'corrected', 'applied', 'revert'.
  • AC#8 diff assertion strengthened (Issue #7): Replaced generic 'file' and 'change' with diff-specific markers: '---', '+++', 'original', 'modified'.
  • AC#6 rollback assertion tightened (Issue #12): Removed generic 'complete' from the disjunction. Kept 'rollback', 'restored', 'revert', 'checkpoint'.
  • plan correct — removed undocumented --plan flag (Issue #8): The spec signature for plan correct does not include a --plan flag. Removed to align with documented spec workflow.
  • Missing plan diff investigation step added (Issue #9): Added plan diff ${plan_id} --format plain step between tree (step 9) and explain (step 10), matching the spec WF15 workflow which shows investigating the diff before explain.
  • plan apply instead of lifecycle-apply (Issue #10): Changed both initial and final apply commands from plan lifecycle-apply to plan apply with --yes flag to match the spec-documented command.
  • Phase advancement verification (Issue #13): Added plan status check after strategize (step 5b) to verify phase transition before executing the execute phase.
  • Consistent explicit timeouts (Issue #14): Added explicit timeout=120s to steps 8, 9, 10, and 17 which previously relied on the default.
  • ULID regex consolidation comment (Issue #15): Added comment in Extract Plan Id keyword noting that M1 defines a similar keyword and consolidation is deferred to a separate ticket.
  • plan diff --correction spec alignment (Issue #16): Fixed plan diff invocation when correction_id is available to use plan diff --correction ${correction_id} without also passing plan_id, matching the spec's either/or notation.
  • Extract Plan Id fallback regex tightened (Issue #17): Replaced overly broad [\w-]+ regex with ULID/UUID-specific patterns: [0-9A-HJ-KM-NP-TV-Z]{26} and UUID 8-4-4-4-12 format.
  • Explicit expected_rc on steps 5 and 6 (Issue #19): Added explicit expected_rc=${0} to strategize and execute steps for consistency with other steps.
  • ULID regex corrected: Fixed Crockford Base32 regex from [0-9A-HJ-NP-Z] to [0-9A-HJ-KM-NP-TV-Z] which correctly excludes I, L, O, U.
  • Consistent Traceback/INTERNAL checks: Added Should Not Contain ... Traceback alongside all INTERNAL checks across all steps.
  • Git subprocess timeouts: Added timeout=60s on_timeout=kill to all three Run Process calls (git add, git commit, git rev-parse).
  • Test-case-level timeout: Added [Timeout] 30 minutes to prevent unbounded execution.
  • Keyword deduplication: Extract Plan Id now delegates to Extract First Id From Text for ULID/UUID scanning, keeping only its field-extraction fallback as additional logic.
  • Step renumbering: Sequential 1–17 (plus 5b for phase verification and 9b for investigation diff).

Manual Verification

Prerequisites

  • OPENAI_API_KEY or ANTHROPIC_API_KEY environment variable set

Commands

nox -e e2e_tests

What to Look For

  • WF15 test passes (or SKIPs gracefully if no checkpoint available)
  • Decision IDs correctly extracted from plan tree JSON via targeted decision_id regex
  • Checkpoint IDs extracted from plan status --format json via targeted field regex
  • Recovery commands (rollback, correct, re-execute, apply) assert rc == 0
  • plan diff also asserts rc == 0
  • Final status only accepts applied or completed — not errored or generic terms
  • No Traceback or INTERNAL in any command's output
  • Test skips gracefully when no LLM keys are present

Deferred Items

  • Inline Python fixtures refactor (nit #18): Moving inline fixture strings to .py files loaded at runtime is a substantial refactor outside the scope of this ticket.
  • Extract Plan Id consolidation (nit #15): Both M1 and WF15 define local Extract Plan Id keywords with different signatures. Consolidating into common_e2e.resource is deferred to a separate ticket to avoid touching unrelated test files.
## Summary E2E test for Workflow Example 15 — disaster recovery: rollback a failed apply using the trusted profile. Tests plan rollback, plan correct (revert mode), correction diff, re-execution after correction, and the full recovery lifecycle. Closes #761 ISSUES CLOSED: #761 ## Changes - **Decision ID extraction (Major fix):** Replaced generic `Extract First Id From Text` with dedicated `Extract Decision Id From Tree` keyword that uses a targeted regex (`"decision_id"\s*:\s*"<ULID>"`) to avoid extracting `plan_id` from JSON tree output. This ensures `plan explain` and `plan correct` receive the correct decision ID. - **Checkpoint ID extraction (Major fix):** Replaced broken first-ULID extraction from `plan artifacts` with dedicated `Extract Checkpoint Id From Status` keyword that targets `last_checkpoint_id` / `checkpoint_id` fields in `plan status --format json` output. Removed the dangerous fallback that silently used `plan_id` as checkpoint. When no checkpoint is available, the test gracefully skips with a clear message via `Skip If`. - **AC#9 final status assertion (Major fix):** Removed tautological terms (`'phase'`, `'errored'`, `'cancelled'`) from the terminal state assertion. After successful recovery, only `'applied'` or `'completed'` are accepted as valid terminal states. - **Return code assertions on recovery steps (Major fix):** Added `Should Be Equal As Integers ${r_X.rc} 0` assertions to all critical recovery commands including `plan diff` (step 14), which previously lacked an rc check. The initial `plan apply` and `plan artifacts` still use `expected_rc=None` without rc checks since those may legitimately fail. - **ROOT CAUSE annotation enforcement (Major fix — AC#4):** Added a secondary `plan tree --format plain` request specifically for the ROOT CAUSE display annotation check. Also searches JSON tree and status output for `root_cause`/`root cause` references. Logs WARN (rather than failing) when not found, since LLM behavior is non-deterministic and may not annotate root cause. - **Checkpoint skip with explicit documentation (Major fix — AC#6–AC#9):** Replaced terse `Skip If` with thoroughly documented skip explaining when checkpoint absence is expected (LLM behavior) vs when it indicates a problem. Added guidance to investigate persistent skips. - **Errored state verification (Major fix — Issue #3):** Added dedicated assertion after initial apply/status to verify the plan entered an errored/failed state. Logs WARN when state is not errored (LLM may produce a successful plan), continuing with recovery steps regardless. - **Correction ID extraction (Major fix — Issue #5):** Replaced generic `Extract First Id From Text` on combined stdout+stderr with dedicated `Extract Correction Id From Output` keyword that targets the `"correction_id"` field specifically. Falls back to first ULID from stdout only (not stderr) to avoid extracting stray IDs. - **Dead ELSE branch removed (Major fix — Issue #6):** Removed unreachable IF/ELSE guard on correction ID extraction (rc==0 already guaranteed by prior assertion). Correction ID is now always extracted directly. - **AC#3 status assertion tightened:** Removed generic `'execute'` term that matches field labels in every status output. Kept only specific state values: `'processing'`, `'errored'`, `'applied'`. - **AC#5 explain assertion improved (Issue #7, #20):** Removed overly generic terms (`'decision'`, `'question'`) that would match any explain output. Replaced with specific forensic content terms: `'rationale'`, `'snapshot'`, `'alternative'`, `'chosen'`, `'confidence'`, `'context'`. - **AC#7 correct assertion improved (Issue #7):** Removed generic `'correct'` (matches command name) and `'status'`. Replaced with more specific terms: `'correction_id'`, `'corrected'`, `'applied'`, `'revert'`. - **AC#8 diff assertion strengthened (Issue #7):** Replaced generic `'file'` and `'change'` with diff-specific markers: `'---'`, `'+++'`, `'original'`, `'modified'`. - **AC#6 rollback assertion tightened (Issue #12):** Removed generic `'complete'` from the disjunction. Kept `'rollback'`, `'restored'`, `'revert'`, `'checkpoint'`. - **`plan correct` — removed undocumented `--plan` flag (Issue #8):** The spec signature for `plan correct` does not include a `--plan` flag. Removed to align with documented spec workflow. - **Missing `plan diff` investigation step added (Issue #9):** Added `plan diff ${plan_id} --format plain` step between tree (step 9) and explain (step 10), matching the spec WF15 workflow which shows investigating the diff before explain. - **`plan apply` instead of `lifecycle-apply` (Issue #10):** Changed both initial and final apply commands from `plan lifecycle-apply` to `plan apply` with `--yes` flag to match the spec-documented command. - **Phase advancement verification (Issue #13):** Added `plan status` check after strategize (step 5b) to verify phase transition before executing the execute phase. - **Consistent explicit timeouts (Issue #14):** Added explicit `timeout=120s` to steps 8, 9, 10, and 17 which previously relied on the default. - **ULID regex consolidation comment (Issue #15):** Added comment in `Extract Plan Id` keyword noting that M1 defines a similar keyword and consolidation is deferred to a separate ticket. - **`plan diff --correction` spec alignment (Issue #16):** Fixed `plan diff` invocation when correction_id is available to use `plan diff --correction ${correction_id}` without also passing `plan_id`, matching the spec's either/or notation. - **`Extract Plan Id` fallback regex tightened (Issue #17):** Replaced overly broad `[\w-]+` regex with ULID/UUID-specific patterns: `[0-9A-HJ-KM-NP-TV-Z]{26}` and UUID 8-4-4-4-12 format. - **Explicit `expected_rc` on steps 5 and 6 (Issue #19):** Added explicit `expected_rc=${0}` to strategize and execute steps for consistency with other steps. - **ULID regex corrected:** Fixed Crockford Base32 regex from `[0-9A-HJ-NP-Z]` to `[0-9A-HJ-KM-NP-TV-Z]` which correctly excludes I, L, O, U. - **Consistent Traceback/INTERNAL checks:** Added `Should Not Contain ... Traceback` alongside all `INTERNAL` checks across all steps. - **Git subprocess timeouts:** Added `timeout=60s on_timeout=kill` to all three `Run Process` calls (`git add`, `git commit`, `git rev-parse`). - **Test-case-level timeout:** Added `[Timeout] 30 minutes` to prevent unbounded execution. - **Keyword deduplication:** `Extract Plan Id` now delegates to `Extract First Id From Text` for ULID/UUID scanning, keeping only its field-extraction fallback as additional logic. - **Step renumbering:** Sequential 1–17 (plus 5b for phase verification and 9b for investigation diff). ## Manual Verification ### Prerequisites - `OPENAI_API_KEY` or `ANTHROPIC_API_KEY` environment variable set ### Commands ```bash nox -e e2e_tests ``` ### What to Look For - WF15 test passes (or SKIPs gracefully if no checkpoint available) - Decision IDs correctly extracted from `plan tree` JSON via targeted `decision_id` regex - Checkpoint IDs extracted from `plan status --format json` via targeted field regex - Recovery commands (rollback, correct, re-execute, apply) assert `rc == 0` - `plan diff` also asserts `rc == 0` - Final status only accepts `applied` or `completed` — not `errored` or generic terms - No `Traceback` or `INTERNAL` in any command's output - Test skips gracefully when no LLM keys are present ## Deferred Items - **Inline Python fixtures refactor (nit #18):** Moving inline fixture strings to `.py` files loaded at runtime is a substantial refactor outside the scope of this ticket. - **`Extract Plan Id` consolidation (nit #15):** Both M1 and WF15 define local `Extract Plan Id` keywords with different signatures. Consolidating into `common_e2e.resource` is deferred to a separate ticket to avoid touching unrelated test files.
freemo added the
Type
Testing
label 2026-03-13 01:16:26 +00:00
freemo added this to the v3.2.0 milestone 2026-03-13 01:16:26 +00:00
freemo force-pushed test/e2e-wf15-disaster-recovery from a0377ba8b1 to 803fde3e97 2026-03-13 16:24:29 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 803fde3e97 to 8ef5722458 2026-03-13 16:31:36 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 8ef5722458 to 93827a0e26 2026-03-13 16:42:18 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 93827a0e26 to 0e17b080d4 2026-03-13 17:00:52 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 0e17b080d4 to 62dd9d81fd 2026-03-13 17:28:38 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 62dd9d81fd to 47436aca0a 2026-03-13 17:46:51 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 47436aca0a to a0377ba8b1 2026-03-13 18:13:02 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from a0377ba8b1 to 0111d02d65 2026-03-13 18:14:21 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 0111d02d65 to 9884fbb4a5 2026-03-13 18:25:18 +00:00 Compare
freemo force-pushed test/e2e-wf15-disaster-recovery from 9884fbb4a5 to a3ccdaba41 2026-03-13 18:36:45 +00:00 Compare
freemo added the
State
In Review
label 2026-03-13 21:16:36 +00:00
freemo force-pushed test/e2e-wf15-disaster-recovery from a3ccdaba41 to cc07e67799 2026-03-13 23:19:50 +00:00 Compare
freemo added the
Priority
Medium
label 2026-03-14 04:10:14 +00:00
Author
Owner

PM Review — Day 34

Status: Mergeable, 0 reviews, M3 (v3.2.0)
Author: @freemo

E2E test for WF15 (disaster recovery, rollback a failed apply). Most comprehensive test in the M3 batch — 15 sequential steps covering forensic inspection (--show-context --show-reasoning), rollback, correction (revert mode), diff --correction, re-execute, and lifecycle-apply.

See #799 for common review notes.

Action Items

Who Action Deadline
@hurui200320 Peer review Day 36
## PM Review — Day 34 **Status**: Mergeable, 0 reviews, M3 (v3.2.0) **Author**: @freemo E2E test for WF15 (disaster recovery, rollback a failed apply). Most comprehensive test in the M3 batch — 15 sequential steps covering forensic inspection (`--show-context --show-reasoning`), rollback, correction (revert mode), `diff --correction`, re-execute, and lifecycle-apply. **See #799 for common review notes.** ### Action Items | Who | Action | Deadline | |-----|--------|----------| | @hurui200320 | **Peer review** | Day 36 |
freemo added a new dependency 2026-03-16 02:42:16 +00:00
freemo added a new dependency 2026-03-16 02:42:16 +00:00
Author
Owner

PM Status — Day 36 (2026-03-16)

Day 34 review assignment deadline check. This PR has been in review for 2+ days with 0 reviewer activity.

Reminder: Assigned reviewer — please post your review by Day 37 EOD or flag any blockers. These E2E test PRs are foundational for milestone acceptance gates and cannot remain unreviewed indefinitely.

If you are unable to review by the deadline, please comment so the review can be reassigned.

## PM Status — Day 36 (2026-03-16) Day 34 review assignment deadline check. This PR has been in review for 2+ days with 0 reviewer activity. **Reminder**: Assigned reviewer — please post your review by **Day 37 EOD** or flag any blockers. These E2E test PRs are foundational for milestone acceptance gates and cannot remain unreviewed indefinitely. If you are unable to review by the deadline, please comment so the review can be reassigned.
hamza.khyari was assigned by freemo 2026-03-16 22:21:09 +00:00
hurui200320 was assigned by freemo 2026-03-16 22:21:15 +00:00
hamza.khyari was unassigned by freemo 2026-03-16 22:21:21 +00:00
Author
Owner

@hurui200320 I am going to have you take over this PR, it is mostly completed but is waiting on #628 and #966 One is yours and one is Brent's. Please be sure to get this PR and the two blocking PRs I listed in asap, thanks.

@hurui200320 I am going to have you take over this PR, it is mostly completed but is waiting on https://git.cleverthis.com/cleveragents/cleveragents-core/issues/628 and https://git.cleverthis.com/cleveragents/cleveragents-core/issues/966 One is yours and one is Brent's. Please be sure to get this PR and the two blocking PRs I listed in asap, thanks.
Author
Owner

PM Status — Day 37 — Rebase Required

This PR has merge conflicts and cannot be merged in its current state. 42% of all open PRs (21 of 50) have conflicts — this is a project-wide issue that must be resolved.

@freemo — Please rebase this PR onto master by Day 39 EOD (2026-03-19). If you cannot rebase by then, please post a comment explaining the blocker.


PM rebase request — Day 37

## PM Status — Day 37 — Rebase Required This PR has **merge conflicts** and cannot be merged in its current state. 42% of all open PRs (21 of 50) have conflicts — this is a project-wide issue that must be resolved. @freemo — Please rebase this PR onto `master` by **Day 39 EOD (2026-03-19)**. If you cannot rebase by then, please post a comment explaining the blocker. --- *PM rebase request — Day 37*
hurui200320 force-pushed test/e2e-wf15-disaster-recovery from cc07e67799 to 700aef534c 2026-03-18 08:45:57 +00:00 Compare
freemo requested review from CoreRasurae 2026-03-22 16:40:05 +00:00
freemo requested review from brent.edwards 2026-03-22 16:40:05 +00:00
hurui200320 force-pushed test/e2e-wf15-disaster-recovery from 700aef534c to d0bf51d77d 2026-03-23 05:54:29 +00:00 Compare
hurui200320 force-pushed test/e2e-wf15-disaster-recovery from d0bf51d77d to 12c0e78854 2026-03-23 07:03:52 +00:00 Compare
hurui200320 force-pushed test/e2e-wf15-disaster-recovery from 12c0e78854 to 08878c03a1 2026-03-23 13:22:08 +00:00 Compare
hurui200320 force-pushed test/e2e-wf15-disaster-recovery from 08878c03a1 to 5d279081b8 2026-03-24 05:40:51 +00:00 Compare
hurui200320 force-pushed test/e2e-wf15-disaster-recovery from 5d279081b8 to 0d3371a274 2026-03-27 09:59:43 +00:00 Compare
hurui200320 was unassigned by freemo 2026-04-02 06:15:23 +00:00
freemo self-assigned this 2026-04-02 06:15:23 +00:00
freemo force-pushed test/e2e-wf15-disaster-recovery from 0d3371a274 to 91bcffe1b5 2026-04-02 07:00:21 +00:00 Compare
Author
Owner

🤖 Backlog Groomer (groomer-1): Closing as duplicate of #761.

Issue #761 (test(e2e): workflow example 15 — disaster recovery, rollback a failed apply) is the canonical version with full labels (MoSCoW/Must have, Priority/Critical, State/In Review, Type/Testing) and milestone v3.2.0. This issue is an exact title duplicate.

🤖 **Backlog Groomer (groomer-1):** Closing as duplicate of #761. Issue #761 (`test(e2e): workflow example 15 — disaster recovery, rollback a failed apply`) is the canonical version with full labels (`MoSCoW/Must have`, `Priority/Critical`, `State/In Review`, `Type/Testing`) and milestone `v3.2.0`. This issue is an exact title duplicate.
freemo closed this pull request 2026-04-02 17:33:07 +00:00
Some required checks failed
CI / lint (pull_request) Failing after 5s
Required
Details
CI / typecheck (pull_request) Failing after 5s
Required
Details
CI / coverage (pull_request) Has been skipped
Required
Details
CI / security (pull_request) Failing after 5s
Required
Details
CI / quality (pull_request) Failing after 5s
Required
Details
CI / unit_tests (pull_request) Failing after 5s
Required
Details
CI / docker (pull_request) Has been skipped
Required
Details
CI / integration_tests (pull_request) Failing after 5s
Required
Details
CI / e2e_tests (pull_request) Failing after 5s
CI / build (pull_request) Failing after 4s
Required
Details
CI / helm (pull_request) Failing after 4s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Has been skipped

Pull request closed

Sign in to join this conversation.
2 Participants
Notifications
Due Date
No due date set.
Reference: cleveragents/cleveragents-core#802