diff --git a/CHANGELOG.md b/CHANGELOG.md index 0704c6a4..513ea8e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,16 @@ All notable changes to vouch are documented here. Format follows per-prompt block, the session banner, `vouch status` and the opt-in question all say so rather than calling it "this repo's" knowledge. +### Fixed +- vault sync no longer destroys a second vault edit made while the first + proposal is still pending. the pending-proposal dedup matched on page id + alone (its body-aware mode was never wired to the call site), so the + forward pass skipped the new edit as "unchanged" and the backward pass + then rewrote the mirror with KB-canonical content — the edit vanished + with no proposal, no warning, and no file. the dedup now compares the + edited body, so a genuinely different edit files its own proposal while + duplicate re-runs still coalesce. + ## [1.5.0] — 2026-07-20 ### Added diff --git a/src/vouch/vault_sync.py b/src/vouch/vault_sync.py index d7a7f027..8e1b27a4 100644 --- a/src/vouch/vault_sync.py +++ b/src/vouch/vault_sync.py @@ -270,15 +270,18 @@ def kb_to_vault(store: KBStore, vault_dir: Path) -> VaultSyncResult: def _has_pending_page_proposal( - store: KBStore, page_id: str, *, body: str | None = None + store: KBStore, page_id: str, *, edit_source_id: str | None = None ) -> bool: - """Return True if a pending proposal already targets ``page_id``. - - When ``body`` is supplied, only returns True if the pending proposal - also carries the same body — allowing a second different vault edit - to file a new proposal even while the first is still pending. - Prevents duplicate proposals when vault_to_kb runs multiple times - before the reviewer approves the first proposal for a given page edit. + """return True if a pending proposal already targets ``page_id``. + + when ``edit_source_id`` is supplied, only returns True if the pending + proposal also cites that vault-edit source. the source id is the + sha256 of the whole mirror file (put_source is content-addressed and + vault_to_kb records it in the proposal's sources), so it fingerprints + the complete edit — body and frontmatter alike. re-running sync on + the same edit coalesces on the pending proposal; any different edit, + including a frontmatter-only one, files its own proposal even while + the first is still pending. """ from .models import ProposalKind, ProposalStatus for proposal in store.list_proposals(ProposalStatus.PENDING): @@ -287,7 +290,9 @@ def _has_pending_page_proposal( payload = proposal.payload if not isinstance(payload, dict) or payload.get("id") != page_id: continue - if body is None or payload.get("body") == body: + if edit_source_id is None or edit_source_id in ( + payload.get("sources") or [] + ): return True return False @@ -354,15 +359,19 @@ def vault_to_kb( result.pages_skipped_unknown_id.append(rel) continue - # Fix 2 (#219): skip if a pending proposal already targets this page - # id. Without this guard, running vault_to_kb twice before the first + # fix 2 (#219): skip if a pending proposal already carries this same + # edit. without this guard, running vault_to_kb twice before the first # proposal is approved files duplicate proposals for the same edit, # cluttering the review queue and causing the second approve to fail - # with "page already exists". - if _has_pending_page_proposal(store, page_id): + # with "page already exists". the guard matches on the vault-edit + # source id — current_hash is the same sha256 of the whole file that + # put_source assigns below — so it stays scoped to duplicate runs: a + # different edit, body or frontmatter, still files its own proposal + # instead of being skipped and then destroyed by the backward pass. + if _has_pending_page_proposal(store, page_id, edit_source_id=current_hash): log.debug( - "vault sync: pending proposal already exists for page %r; " - "skipping to avoid duplicate", + "vault sync: pending proposal already carries this edit for " + "page %r; skipping to avoid duplicate", page_id, ) result.pages_skipped_unchanged.append(rel) diff --git a/tests/test_vault_sync.py b/tests/test_vault_sync.py index b765b08c..eebdd22d 100644 --- a/tests/test_vault_sync.py +++ b/tests/test_vault_sync.py @@ -428,6 +428,98 @@ def test_vault_to_kb_deduplicates_pending_proposals( ) +def test_vault_to_kb_second_different_edit_files_new_proposal( + store: KBStore, vault: Path, +) -> None: + """A second, *different* edit made while the first proposal is still + pending must file its own proposal. The dedup helper documents exactly + this ("allowing a second different vault edit to file a new proposal + even while the first is still pending"), but the call site never passed + ``body``, so the page-id-only match swallowed the second edit and + misreported it as unchanged.""" + kb_to_vault(store, vault) + mirror = vault / VAULT_DIR / "pages" / "alpha-page.md" + base = mirror.read_text(encoding="utf-8") + + mirror.write_text( + base.replace("Original body.", "Edit A."), encoding="utf-8" + ) + r1 = vault_to_kb(store, vault, actor="vault-sync") + assert "alpha-page" in r1.pages_proposed + + mirror.write_text( + base.replace("Original body.", "Edit B."), encoding="utf-8" + ) + r2 = vault_to_kb(store, vault, actor="vault-sync") + assert "alpha-page" in r2.pages_proposed, ( + "second different edit was misclassified as unchanged: " + f"{r2.pages_skipped_unchanged}" + ) + proposals = list((store.kb_dir / "proposed").glob("*.yaml")) + assert len(proposals) == 2 + + +def test_vault_to_kb_frontmatter_only_edit_files_new_proposal( + store: KBStore, vault: Path, +) -> None: + """A second edit that changes only frontmatter (body untouched) is + still a different edit and must file its own proposal — the dedup + fingerprint covers the whole file, not just the body.""" + kb_to_vault(store, vault) + mirror = vault / VAULT_DIR / "pages" / "alpha-page.md" + base = mirror.read_text(encoding="utf-8") + assert "title: Alpha page" in base + + mirror.write_text( + base.replace("title: Alpha page", "title: Alpha page (draft)"), + encoding="utf-8", + ) + r1 = vault_to_kb(store, vault, actor="vault-sync") + assert "alpha-page" in r1.pages_proposed + + mirror.write_text( + base.replace("title: Alpha page", "title: Alpha page (final)"), + encoding="utf-8", + ) + r2 = vault_to_kb(store, vault, actor="vault-sync") + assert "alpha-page" in r2.pages_proposed, ( + "frontmatter-only edit was misclassified as unchanged: " + f"{r2.pages_skipped_unchanged}" + ) + proposals = list((store.kb_dir / "proposed").glob("*.yaml")) + assert len(proposals) == 2 + + +def test_sync_vault_preserves_second_edit_while_first_pending( + store: KBStore, vault: Path, +) -> None: + """sync_vault's contract is that the forward pass captures user edits as + proposals *before* the backward pass overwrites the mirror. A second + different edit while the first proposal is pending must therefore end up + in a proposal — not be skipped forward and then destroyed backward.""" + sync_vault(store, vault) + mirror = vault / VAULT_DIR / "pages" / "alpha-page.md" + base = mirror.read_text(encoding="utf-8") + + mirror.write_text( + base.replace("Original body.", "Edit A."), encoding="utf-8" + ) + sync_vault(store, vault) + + mirror.write_text( + base.replace("Original body.", "Edit B."), encoding="utf-8" + ) + sync_vault(store, vault) + + proposals = list((store.kb_dir / "proposed").glob("*.yaml")) + assert any( + "Edit B." in p.read_text(encoding="utf-8") for p in proposals + ), ( + "edit B was silently destroyed: not captured in any proposal, and " + "the backward pass rewrote the mirror with KB-canonical content" + ) + + def test_vault_to_kb_warns_on_claim_stub_edit( store: KBStore, vault: Path, caplog: pytest.LogCaptureFixture, ) -> None: