fix(gcp-to-aws): resume/recovery reliability — stale reuse, corrupt state, mid-flow read - #182
fix(gcp-to-aws): resume/recovery reliability — stale reuse, corrupt state, mid-flow read#182herosjourney wants to merge 3 commits into
Conversation
…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
left a comment
There was a problem hiding this comment.
[🤖 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"andmetadata.questions_asked— this interacts with #182's "keep-valid-answers" path (Case 1-A with compat fail). If a carried-forward value retains its originalchosen_by: "extracted"and Q13b doesn't appear inquestions_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.jsontriggering the compat-fail path; truncated.phase-status.jsonwith 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.jsonpresent butestimation-ai.jsonmissing).
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. |
There was a problem hiding this comment.
[🤖 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.
… 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>
|
Addressed the optional partial-write note on corrupt-state reconstruction:
|
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:
clarify.mdStep 0 Case 1): before offering "re-use and skip questions," check the file for schema currency (clarify_mode,prompt/design_consequence,sourceon extracted/default constraints,cpu_architecturewhen compute exists) and discovery match (AI profile present but noai_constraints, database in inventory but noavailability/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 topreferences-superseded.jsoninstead of deleting it (irreversible discard of all prior answers), and Step 5 cleans the backup up once a new complete file exists..phase-status.jsonrecovery (SKILL.mdState 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.clarify-global.md): the database-tooling notes said "Readpreferences.json→design_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.jsonfrom 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— cleanmise run lint:md— 0 errors across 804 filesmise run lint:frontmatter— OK (10 phase files).phase-status.json(which no gate protects) and never editspreferences.jsoncontentclarify_mode, nosourcefields) and confirm the keep-valid-answers offer appears instead of the plain reuse offer.phase-status.jsonin a run directory with design artifacts present and confirm the agent proposes resuming at estimate, not restarting DiscoverOut of scope
clarify-ai-only.mdStep 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 consistencyaws-design.json,estimation-*.json) — real gap, different mechanism (per-phase input gates), separate PRcpu_architecture,websockettype drift, agentic keys) — separate cleanup PRType of Change
Team Folder
migrate/