Skip to content

fix(gcp-to-aws): resume/recovery reliability — stale reuse, corrupt state, mid-flow read - #182

Open
herosjourney wants to merge 3 commits into
awslabs:mainfrom
herosjourney:fix/resume-recovery-reliability
Open

fix(gcp-to-aws): resume/recovery reliability — stale reuse, corrupt state, mid-flow read#182
herosjourney wants to merge 3 commits into
awslabs:mainfrom
herosjourney:fix/resume-recovery-reliability

Conversation

@herosjourney

Copy link
Copy Markdown
Contributor

Summary

The problem, in plain English: This plugin is built for multi-session use — six phases, state on disk, resume anywhere — but three of its recovery paths punish exactly the user who comes back. Reuse your preferences from a prior run and, if that file predates the current flow, Clarify dead-ends at a gate that forbids both fixing the file and proceeding. Get one truncated write to the state file and the documented remedy ("delete the file and restart the current phase") silently restarts the entire migration from Discover, because a missing state file initializes Phase 1 — even though every completed phase's artifacts are sitting intact in the same directory. And one instruction in the middle of Clarify tells the agent to read a file that does not exist yet at that point in the flow — or worse, on a re-run, reads a stale prior run's answer.

Changes:

  • Stale preferences reuse (clarify.md Step 0 Case 1): before offering "re-use and skip questions," check the file for schema currency (clarify_mode, prompt/design_consequence, source on extracted/default constraints, cpu_architecture when compute exists) and discovery match (AI profile present but no ai_constraints, database in inventory but no availability/db_size → stale). Compatible files get the existing reuse offer, unchanged. Incompatible files get a new offer: keep still-valid answers — carried forward onto the Assumption Sheet for confirmation, gaps filled by the wizard — or start fresh. Case 2 (drafts) has had exactly this legacy handling since the wizard landed; Case 1 never got it. Also: "start fresh" now renames the old file to preferences-superseded.json instead of deleting it (irreversible discard of all prior answers), and Step 5 cleans the backup up once a new complete file exists.
  • Corrupt .phase-status.json recovery (SKILL.md State Validation rule 2): replaced "Delete the file and restart the current phase" — self-contradictory, since with the file corrupt the current phase is unknowable, and per Workflow Execution a missing file initializes Discover. The remedy is now: infer completed phases from the artifacts on disk (inventory → discover, preferences.json → clarify, aws-design*.json → design, estimation-* → estimate, generation-*/MIGRATION_GUIDE.md → generate), confirm the inferred status with the user, rewrite the state file, resume. Explicitly noted as ground-truth reconstruction, not artifact-patching — the handoff-gate prohibition targets the latter.
  • Q7 mid-flow read (clarify-global.md): the database-tooling notes said "Read preferences.jsondesign_constraints.db_size.value" — during Clarify that file is only written at Step 5, so the instruction either fails or (on a re-run with a surviving prior file) silently reads last run's answer. Now points at the in-flight resolved Q13b value (Step 2 extraction / confirmed sheet / draft), with the existing threshold fallback unchanged.

Why

All three came out of the same audit as #181 (post-mortem of a real multi-session migration run whose workspace contains a pre-wizard preferences.json from June — exactly the file that would dead-end today's Case 1-A). #181 hardens the happy path's output; this PR hardens the paths users hit when something already went sideways: an old artifact, a corrupted write, a re-run. Recovery paths that destroy progress or dead-end are worse than no recovery paths, because users trust them.

Test plan

  • dprint check — clean
  • mise run lint:md — 0 errors across 804 files
  • mise run lint:frontmatter — OK (10 phase files)
  • Compatible-file path diff-verified unchanged: schema-current preferences.json still gets the original A/B reuse offer verbatim
  • Handoff gates untouched — no gate check added, removed, or weakened; recovery writes only .phase-status.json (which no gate protects) and never edits preferences.json content
  • Manual: point Clarify at a run directory with a pre-wizard preferences.json (no clarify_mode, no source fields) and confirm the keep-valid-answers offer appears instead of the plain reuse offer
  • Manual: truncate .phase-status.json in a run directory with design artifacts present and confirm the agent proposes resuming at estimate, not restarting Discover

Out of scope

  • clarify-ai-only.md Step 0 Case 1 reuse — its validation gate is minimal (file exists + migration_type == "ai-only"), so stale reuse there can't dead-end; can adopt the same compatibility check later for consistency
  • Parse validation for mid-pipeline artifacts (aws-design.json, estimation-*.json) — real gap, different mechanism (per-phase input gates), separate PR
  • schema-preferences.md catalog completeness (cpu_architecture, websocket type drift, agentic keys) — separate cleanup PR
  • heroku-to-aws / other skills

Type of Change

  • Bug fix

Team Folder

  • migrate/

…pt state, mid-flow read

- clarify.md Step 0 Case 1: compatibility-check preferences.json before
  offering reuse (schema currency + discovery match); incompatible files get
  a keep-valid-answers-and-fill-gaps path instead of dead-ending at
  GATE_FAIL; start-fresh renames to preferences-superseded.json instead of
  deleting; Step 5 cleans the backup up
- SKILL.md State Validation rule 2: corrupted .phase-status.json is now
  reconstructed from on-disk artifacts with user confirmation instead of
  'delete and restart' (which silently restarted from Discover)
- clarify-global.md Q7: use the in-flight Q13b value, not preferences.json,
  which doesn't exist mid-Clarify (or is a stale prior run's on re-runs)

@leon1418 leon1418 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🤖 AI review 🤖]

Scope reviewed: All 3 changed files read in full; mainline context for clarify.md (Step 0 Case 1/2, Step 5), clarify-global.md (Q7 tooling notes), SKILL.md (State Validation §); schema-preferences.md constraint catalog; handoff-gates.md prohibition language; sibling PRs #180#186 diffs for interaction/conflict.

Summary: Three targeted fixes to recovery paths that previously either dead-ended users or destroyed progress. Each fix is well-motivated by the same multi-session audit that produced #181. The design is sound: stale-preferences get a graceful upgrade path instead of a gate dead-end, corrupt state files are reconstructed from durable artifact evidence instead of nuclear-restart, and the mid-Clarify Q7 read is redirected to the correct in-flight source.

Findings: 1 optional suggestion (inline). No blocking issues.

Interactions with siblings:

  • #181 (extraction-consistency gates): #181 adds new handoff-gate checks 7/8 that reference chosen_by: "user" and metadata.questions_asked — this interacts with #182's "keep-valid-answers" path (Case 1-A with compat fail). If a carried-forward value retains its original chosen_by: "extracted" and Q13b doesn't appear in questions_asked, the new gate 7 won't fire (correctly). No conflict, but merge order matters: #182 first is clean; #181 first adds gates that already accommodate the seeded-wizard path.
  • #183 (journey orientation + answer recap): No file overlap. Independent.
  • #185 (decision gate, SKILL.md): Modifies the Phase Routing Table in SKILL.md but NOT the State Validation section. No textual conflict.
  • #186 (decision report, stacked on #185): Touches SKILL.md header only. No conflict.
  • #180 (heroku parity): Different skill directory entirely. No interaction.
  • #184 (generate last-mile): Different files. No interaction.

Merge order recommendation: #182 is independent of all siblings and can merge in any order. If merging #181 first, no semantic conflict arises — the new extraction-consistency gates are compatible with the seeded-wizard flow.

Validation gaps (cannot verify without a running agent):

  • Manual test cases (pre-wizard preferences.json triggering the compat-fail path; truncated .phase-status.json with design artifacts present) — author marked these as pending manual verification in the test plan, which is appropriate.
  • The artifact→phase inference heuristic hasn't been tested against edge cases like partial writes (e.g., estimation-infra.json present but estimation-ai.json missing).

CI/Merge state: MERGEABLE, REVIEW_REQUIRED, no status checks configured. Linting assertions in PR description (dprint, md lint, frontmatter) are self-reported clean.

2. **Invalid JSON**: If `.phase-status.json` fails to parse, do NOT delete it and do NOT restart from Discover — the phase artifacts on disk are the durable record of progress. Reconstruct instead:
1. Enumerate `$MIGRATION_DIR` and infer completed phases from artifacts: any of `gcp-resource-inventory.json` / `billing-profile.json` / `ai-workload-profile.json` → discover completed; `preferences.json` → clarify completed; `aws-design.json` / `aws-design-ai.json` / `aws-design-billing.json` → design completed; `estimation-*.json` → estimate completed; `generation-*.json` or `MIGRATION_GUIDE.md` → generate completed.
2. Present the inferred status to the user: "Your state file was corrupted, but I can see [phases] completed from the artifacts on disk. Resume at [next phase]? (Y/N)"
3. On Y: rewrite `.phase-status.json` with the inferred phases marked `"completed"`, the next phase `"pending"`, `current_phase` set to it, and a fresh `last_updated`. Continue normally. On N: ask which phase to resume from and write that instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🤖 AI review 🤖]

Optional: The artifact→phase inference maps estimation-*.json to estimate-completed. In a partial-write scenario (e.g., agent crashed mid-estimate: estimation-infra.json exists but estimation-ai.json does not), this would still infer estimate as completed and propose resuming at generate — potentially skipping unfinished estimate work.

Consider noting that the user-confirmation step (2.ii) is the safety net here: "Resume at generate?" gives the user the chance to say N and pick estimate. But if you wanted to tighten it, you could cross-check against preferences.json to see whether AI artifacts should exist (i.e., ai_constraints section present → expect estimation-ai.json) and downgrade the inference to "estimate may be partial" when expected artifacts are absent.

This is non-blocking because the user confirmation dialog already covers the case — just a robustness consideration for a future hardening pass.

herosjourney and others added 2 commits August 13, 2026 07:15
… AI writes

When reconstructing .phase-status.json, do not mark estimate completed if
AI artifacts are expected (ai_constraints / ai profile / aws-design-ai)
but estimation-ai.json is missing. Note that Y/N confirmation remains the
safety net for other partial-write ambiguity.

Co-authored-by: Cursor <cursoragent@cursor.com>
@herosjourney

Copy link
Copy Markdown
Contributor Author

Addressed the optional partial-write note on corrupt-state reconstruction:

  • Tightened inference: if AI estimate output is expected (preferences.json has ai_constraints, or ai-workload-profile.json / aws-design-ai.json is present) but estimation-ai.json is missing while another estimation-*.json exists, treat estimate as incomplete and propose resume at estimate — not generate.
  • Confirmation safety net: step 2.ii now explicitly notes that Y/N remains the backstop for residual ambiguity the heuristic misses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants