feat(plan): harden persistence with schema_version + loaded-object validation (#18)

[OriNachum]

What

Hardens the plan engine's persistence contract to match the frame engine (the #5 peer). Plans are durable artifacts, but plan_store.load() previously trusted whatever JSON it read — no version check, no validation of loaded enum values.

Closes #18.

Changes

• PLAN_SCHEMA_VERSION = 1 added to devague/plan.py; Plan carries a schema_version field, written on save (devague/plan_store.py).
• Fail-closed load. plan_store.load() raises IncompatiblePlanSchemaError when a plan declares a schema_version newer than this devague supports → surfaced as a clean DevagueError (exit 1) with an upgrade hint, no traceback.
• Backward compatible. A pre-0.7.0 plan with no schema_version key loads silently as the current schema (int(d.get("schema_version", PLAN_SCHEMA_VERSION)) in from_dict).
• Loaded-object validation. Task.origin / Task.status and PlanRisk.kind are validated at construction via __post_init__; a hand-edited/corrupted plan surfaces an actionable "malformed plan" error instead of a traceback.
• resolve_plan (devague/cli/_plans.py) now distinguishes invalid slug / newer schema / missing / malformed — each with its own remediation, mirroring frame resolve.

Deliberately out of scope (the issue's "consider … ids" item)

Task/dep/cover id cross-reference validation at load is not added: coverage and acyclic-dependency checks already run against the live frame in plan converge; validating at load would duplicate that logic and could reject valid in-progress plans. The frame engine likewise doesn't validate id cross-refs at load. Per-issue non-goals (no model redesign, no TDD entities, no orchestration) respected.

Tests & verification

• Mirrors the frame suites: schema round-trip, newer-schema rejection, legacy compat, malformed-value rejection, and CLI clean-error tests (tests/test_plan.py, tests/test_plan_store.py, tests/test_cli_plan.py).
• Full suite: 222 passed; coverage 97% total (changed modules 98–100%). flake8 / black / isort / markdownlint all clean.
• Manual E2E confirmed: new plan writes "schema_version": 1; tampering to 99 → upgrade-hint error (exit 1); bad origin → malformed error (exit 1); removing the key → loads fine.

Docs & version

• docs/spec-contract.md "The plan peer" documents the plan schema/version + validation behavior.

• Version bumped 0.6.1 → 0.7.0 with a CHANGELOG entry.

• devague (Claude)

[qodo-code-review]

Review Summary by Qodo

Harden plan persistence with schema versioning and loaded-object validation

✨ Enhancement

Walkthroughs

Description

• Add schema_version field to Plan with validation on load/save
• Implement fail-closed schema compatibility checking with upgrade hints
• Validate loaded Task and PlanRisk enum values via __post_init__
• Enhance resolve_plan to distinguish schema, malformed, missing errors
• Bump version 0.6.1 → 0.7.0 with comprehensive test coverage

Diagram

flowchart LR
  A["Plan Model"] -->|"add schema_version field"| B["Plan with Version"]
  B -->|"write on save"| C["plan_store.save"]
  C -->|"JSON with schema_version"| D["Persisted Plan"]
  D -->|"check on load"| E["plan_store.load"]
  E -->|"version > current?"| F["IncompatiblePlanSchemaError"]
  E -->|"validate enums"| G["Task/PlanRisk __post_init__"]
  G -->|"invalid value?"| H["ValueError"]
  F -->|"caught by"| I["resolve_plan"]
  H -->|"caught by"| I
  I -->|"clean DevagueError"| J["User-friendly exit 1"]

Loading

File Changes

1. devague/plan.py ✨ Enhancement +19/-1

Add schema_version and enum validation to Plan model

devague/plan.py

---

2. devague/plan_store.py ✨ Enhancement +10/-1

Implement schema version checking and IncompatiblePlanSchemaError

devague/plan_store.py

---

3. devague/cli/_plans.py ✨ Enhancement +16/-1

Distinguish schema, malformed, and missing plan errors

devague/cli/_plans.py

---

View more (6)

4. tests/test_plan.py 🧪 Tests +44/-1

Add schema version and enum validation test cases

tests/test_plan.py

---

5. tests/test_plan_store.py 🧪 Tests +30/-1

Add schema version persistence and compatibility tests

tests/test_plan_store.py

---

6. tests/test_cli_plan.py 🧪 Tests +23/-0

Add CLI tests for schema and malformed plan errors

tests/test_cli_plan.py

---

7. CHANGELOG.md 📝 Documentation +11/-0

Document 0.7.0 plan persistence hardening changes

CHANGELOG.md

---

8. docs/spec-contract.md 📝 Documentation +11/-0

Document plan schema version and validation contract

docs/spec-contract.md

---

9. pyproject.toml ⚙️ Configuration changes +1/-1

Bump version from 0.6.1 to 0.7.0

pyproject.toml

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Plan slug mismatch allowed ✓ Resolved 🐞 Bug ☼ Reliability

Description

plan_store.load() reads .devague/plans/<requested>.json but never verifies that the loaded plan.slug
equals the requested slug; if the file is tampered/corrupt, subsequent plan_store.save() will
write/update CURRENT_PLAN under the embedded slug, causing unexpected plan switching or overwriting
another plan.

Code

devague/plan_store.py[R49-56]

Evidence

load() selects the file by the requested slug but returns the embedded plan without checking
equality, while save() later uses the embedded plan.slug for the filename and current-plan
pointer—so a mismatch can redirect persistence to a different slug.

devague/plan_store.py[34-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`plan_store.load(slug)` loads the JSON file selected by the *requested* `slug`, but it only checks that the persisted `plan.slug` is *valid*, not that it *matches* the requested slug. Because `plan_store.save(plan)` uses `plan.slug` to choose the output filename and to update `.devague/current_plan`, a tampered file can redirect writes to a different plan and/or overwrite another plan.

### Issue Context
This PR is explicitly hardening plan persistence; failing to enforce `plan.slug == slug` leaves an integrity hole that can lead to data loss.

### Fix Focus Areas
- devague/plan_store.py[34-57]

### Suggested fix
1. After `plan = from_dict(...)` and slug validation, add:
  - `if plan.slug != slug: raise ValueError(f"plan slug mismatch: expected {slug!r}, found {plan.slug!r}")`
2. Add a unit test that writes a plan file under `demo.json` whose JSON contains `{"slug": "other", ...}` and asserts `plan_store.load("demo")` raises `ValueError` (and that the CLI surfaces it as a malformed plan, if desired).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Schema version coerced loosely ✓ Resolved 🐞 Bug ≡ Correctness

Description

devague.plan.from_dict() coerces schema_version with int(...), which silently accepts booleans and
truncates floats (e.g., 1.9→1), potentially treating a malformed/non-integer schema_version as
supported and undermining the intended fail-closed schema contract.

Code

devague/plan.py[R189-190]

Evidence

from_dict() is the reconstruction path used by plan_store.load(); it currently coerces
schema_version via int(), and load() relies on the resulting value for its compatibility gate.

devague/plan.py[166-197]
devague/plan_store.py[45-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`from_dict()` currently uses `schema_version=int(...)`, which performs permissive coercion (bool→int, float truncation). This can cause malformed persisted data to be silently interpreted as a different schema version than was written.

### Issue Context
The persistence contract is explicitly “schema_version is an integer” and load is supposed to fail closed on unsupported/newer schemas. Permissive coercion increases the chance of false acceptance for corrupted/hand-edited files.

### Fix Focus Areas
- devague/plan.py[170-197]
- devague/plan_store.py[45-57]

### Suggested fix
1. Replace `int(d.get(...))` with strict parsing, e.g.:
  - If missing: default to `PLAN_SCHEMA_VERSION` (back-compat).
  - If present:
    - Accept only `int` (but reject `bool` explicitly), or optionally accept digit-only strings.
    - Reject floats/None/objects with a clear `ValueError` message.
2. Add a test covering a malformed type, e.g. `{"schema_version": 1.9}` or `true`, expecting a `ValueError` (“malformed plan”).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[OriNachum]

Re: the second Qodo finding, "Schema version coerced loosely" (no inline thread for it) — also fixed in 14183e6. schema_version is now parsed strictly via a shared frame.parse_schema_version(d, default) helper: a missing key still defaults to the current version (back-compat), but a present value must be a real int. bool and non-int types (1.9, true, "1", null) are rejected with a clear ValueError instead of being silently coerced — plain int(1.9) truncated to 1 and int(True) yielded 1, letting a malformed version load as current. Parametrized tests added on both test_frame.py and test_plan.py; applied to both persistence twins.

• devague (Claude)