Skip to content

feat(ward-migrate): compile-or-reject v0.1 identity invariants; bump coven-threads-core pin#464

Open
BunsDev wants to merge 2 commits into
mainfrom
feat/ward-migrate-invariant-fidelity
Open

feat(ward-migrate): compile-or-reject v0.1 identity invariants; bump coven-threads-core pin#464
BunsDev wants to merge 2 commits into
mainfrom
feat/ward-migrate-invariant-fidelity

Conversation

@BunsDev

@BunsDev BunsDev commented Jul 22, 2026

Copy link
Copy Markdown
Member

What

Closes the v0.1 invariant silent-drop gap in coven ward migrate and bumps the coven-threads-core pin to threads main. Implementation lane for threads-uqx.8 (Phase 5 approval semantics, spec §7).

1. Migration invariant fidelity (ward_migrate.rs)

Previously LegacyProtected deserialized only files, so v0.1 [protected].invariants vanished without a trace — exactly the silent-drop anti-pattern the retired-Ward corpus exists to prevent. Now:

  • All declarations compile (via coven_threads_core::IdentityInvariantSet::compile) → migration proceeds; per-declaration dispositions (typed fact + operator, no principal values) recorded in MigrationEntry.invariant_dispositions and rendered in the report; originals preserved verbatim in ward.toml.v01.bak. The generated Phase-2 ward.toml has no invariants surface, so they are never carried forward implicitly — the report says so.
  • Any declaration rejected → migration fails closed (Unmigratable, nonzero exit) with the compiler's explicit indexed reasons; the v0.1 file is untouched and no backup is written.
  • No invariants → nothing to preserve, no dispositions, no invariant chatter in the report.

2. Pin bump 7da030d6fa360b

The old rev was an unmerged feature-branch commit (pre-#15 squash). The new pin is coven-threads main, picking up the hardened core from threads PRs #15/#21/#22: tightened MaterializedDiff::try_new (rejects no-content and unchanged surfaces), approval/audit hardening, identity-invariant compiler. Daemon call sites verified compatible; full workspace green against the new pin.

Tests

New coverage mirrors the phase-5 retired-Ward corpus grammar:

  • five-shape fidelity: name/person/pronouns/purpose/coven, == and includes, single and double quotes — asserted disposition-by-disposition
  • explicit rejection per compiler lane: unsupported fact, missing operator, duplicate fact, empty value, missing mandatory name, missing mandatory person
  • fail-closed on rejection: no write, no backup, indexed reasons in the message
  • apply path: originals verbatim in backup only; generated toml still contains no invariants
  • no-invariants wards migrate cleanly with empty dispositions

Validation

  • cargo fmt --check
  • cargo clippy --workspace --all-targets --locked -- -D warnings
  • cargo test --workspace --locked ✓ (all 9 suites; 11 ward_migrate tests)

Beads: threads-uqx.8 (in_progress → evidence on merge). Claim: threads-uqx-8 held.

…coven-threads-core to threads main

Closes the silent-drop gap in ward migration (threads-uqx.8): v0.1
[protected].invariants were previously deserialized away without a
trace. Now every retired declaration is compiled through
coven-threads-core's IdentityInvariantSet compiler:

- all declarations compile -> migration proceeds; per-declaration
  dispositions (typed fact + operator) recorded in the report and the
  originals preserved in ward.toml.v01.bak (Phase-2 ward.toml has no
  invariants surface, so they are never carried forward silently)
- any declaration rejected -> migration fails closed (Unmigratable)
  with the compiler's explicit indexed reasons; nothing written
- no invariants -> nothing to preserve, no dispositions

Bumps the coven-threads-core pin 7da030d -> 6fa360b (threads main),
picking up the post-#15/#21/#22 hardened core (tightened
MaterializedDiff::try_new, approval/audit/identity-invariant
hardening). Full workspace fmt/clippy/test green against the new pin.

Tests mirror the phase-5 retired-Ward corpus grammar: five-shape
fidelity (name/person/pronouns/purpose/coven, ==/includes, both quote
styles), explicit rejection per compiler lane (unsupported fact,
missing operator, duplicate, empty value, missing mandatory
name/person), fail-closed no-write on rejection, and backup-only
preservation on apply.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 22, 2026 06:16

Copilot AI 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.

⚠️ Not ready to approve

The new dry-run report text can incorrectly claim invariants were preserved in the backup file even though dry-run does not write a backup, which makes the migration report misleading.

Pull request overview

This PR tightens coven ward migrate’s v0.1 → Phase-2 migration fidelity by explicitly compiling (or rejecting) retired v0.1 identity invariants via coven-threads-core, avoiding silent drops, and updates the coven-threads-core git pin to a newer reviewed revision.

Changes:

  • Parse and gate v0.1 [protected].invariants through IdentityInvariantSet::compile, recording per-declaration dispositions and failing migration closed on any rejection.
  • Extend the migration report output and tests to cover invariant compilation, rejection lanes, and “no invariants” behavior.
  • Bump coven-threads-core git rev and update Cargo.lock accordingly.
File summaries
File Description
crates/coven-cli/src/ward_migrate.rs Adds invariant compilation/rejection gate, report dispositions, and expanded migration tests.
crates/coven-cli/Cargo.toml Updates coven-threads-core git pin to 6fa360b.
Cargo.lock Locks the updated git dependency graph from the new pin.

Review details

  • Files reviewed: 2/3 changed files
  • Comments generated: 2
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +313 to +316
message: format!(
"would migrate Ward v0.1{}",
invariant_summary_suffix(&invariant_dispositions)
),
Comment on lines +235 to +240
let message = format!(
"v0.1 [protected].invariants rejected explicitly ({} of {} declarations): {}",
rejection_reasons.len(),
legacy_invariants.len(),
rejection_reasons.join("; "),
);
…riant remnants

Review advisories from the fresh-context pass on b39dd22:

- A-1: a ward.toml that parses as Phase-2 (WardConfig tolerates unknown
  fields) but still carries a v0.1 [protected].invariants remnant was
  blessed AlreadyMigrated, silently ignoring the declarations. Now such
  hybrids return Unmigratable with the remnant count; the file is left
  untouched for the principal to fix.
- N-1: set-level rejection message now reads 'N rejection(s) across M
  declaration(s)' instead of the misleading 'N of M declarations'.
- N-3: Cargo.lock regenerated via cargo update --precise; the remaining
  tempfile getrandom 0.4.2->0.3.4 edge re-resolution is within
  tempfile's declared '>=0.3.0, <0.5' range and CI builds --locked.
- N-5: stray indentation normalized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 22, 2026 08:37

Copilot AI 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.

✅ Ready to approve

The functional changes and accompanying tests look solid, with only minor doc/error-message clarity nits noted.

Note: this review does not count toward required approvals for merging.

Review details

Comments suppressed due to low confidence (1)

crates/coven-cli/src/ward_migrate.rs:42

  • MigrationEntry::invariant_dispositions is documented as "per-declaration", but when compilation fails the current implementation records compiler errors (which can be set-level and may not be 1:1 with declarations). Tighten the doc comment to reflect the actual semantics so the report contract is clear.
    /// Per-declaration disposition for v0.1 `[protected].invariants`:
    /// each retired declaration either compiled deterministically into a
    /// typed identity invariant or was rejected explicitly. Never silent.
    pub invariant_dispositions: Vec<InvariantDisposition>,
  • Files reviewed: 2/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

Comment on lines +164 to +166
message: format!(
"ward.toml parses as Phase-2 but retains a v0.1 [protected].invariants remnant ({remnants} declaration(s)); Phase-2 has no invariants surface, so these would be silently inert — remove the remnant or restore the v0.1 file and re-run migration"
),
@BunsDev

BunsDev commented Jul 22, 2026

Copy link
Copy Markdown
Member Author

Fresh-context review cycle (informs, does not simulate, the human gates)

A fresh-context adversarial review was run against head b39dd22, fixes applied in c28a732, and the delta re-verified. Verdict: SIGN-OFF RECOMMENDED at c28a732 — zero blocking.

Verified by the reviewer (independent runs)

  • Diff = PR claims; silent-drop confirmed present at main, closed here
  • Fail-closed ordering proven: rejection early-return precedes dry-run/backup/write; no write, no backup, generated_toml: None on rejection
  • 12/12 ward_migrate tests; full cargo test -p coven-cli --locked exit 0
  • Pin bump: 7da030d is a dead-end branch rev (not an ancestor of threads main); 6fa360b is main. All core behavior deltas tighten (MaterializedDiff::try_new rejects no-op diffs; AutoRegression veto wire key required; ApplyAudit strictly validated). Daemon production code constructs MaterializedDiff nowhere (both api.rs sites are cfg(test)); wire-side consumption means previously persisted no-op diffs now fail closed on reload — the tightening's intent. WARD_AUDIT_SCHEMA_SQL unchanged — no store migration.
  • No principal values in report output: dispositions render typed fact+operator only; values live only in ward.toml.v01.bak
  • Exit contract: Unmigratable → has_errors → nonzero

Advisories → dispositions

Item Severity Disposition
A-1 hybrid Phase-2 ward with v0.1 invariants remnant blessed as AlreadyMigrated (silent-ignore lane) medium Fixed in c28a732: v01_invariant_remnants probe → Unmigratable with remnant count, file untouched; new hybrid test. Reviewer confirmed the toml::Value re-parse cannot diverge from WardConfig::from_toml_str (strictly more permissive), so no bypass
N-1 misleading "N of M declarations" on set-level rejections nit Fixed: "N rejection(s) across M declaration(s)"
N-3 lock also flips tempfile→getrandom edge 0.4.2→0.3.4 nit Explained, no change: tempfile 3.27.0 declares getrandom >=0.3.0,<0.5; any re-resolution re-picks within range; CI builds --locked. Verified against the local registry manifest
N-5 stray indent nit Fixed (fmt normalized)
A-2 non-invariant v0.1 tables (approval_tiers, audit, harness_blocks) drop with backup-only preservation, no report line low Deferred: backup preserves everything; Phase-2 has no such surfaces; candidate report-line extension noted for the uqx.6 integration lane
A-3 dispositions are stdout-ephemeral (no durable machine-readable record) low Deferred: recovery = deterministic recompile from ward.toml.v01.bak; flagged to uqx.6 to re-source from the backup
N-2 core's unsupported identity fact {:?} echoes left-of-operator text (reversed declaration would echo principal-adjacent text) nit Filed upstream: coven-threads bead threads-ype
N-4 --familiar <nonexistent> → empty report, exit 0 nit Deferred (pre-existing, outside lane)
N-6 restore-failure swallows on write rollback nit Deferred (pre-existing; daemon load fails closed on a bad file)

Reviewer's bottom line: "Every migrate lane now terminates in an explicit disposition or an explicit rejection — the never-silent property holds end-to-end. Clean to hand to Nova (uqx.9) and Val (uqx.10)."

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