feat(vbrief): add pre-cutover detection and backward compatibility guard#353
Conversation
…ard (#334) - Add Pre-Cutover Detection Guard section to deft-directive-setup, deft-directive-build, and deft-directive-sync skills -- detects old-model artifacts (SPECIFICATION.md/PROJECT.md without deprecation sentinel, vbrief/ without lifecycle folders) and redirects to task migrate:vbrief with actionable messages - Add model state reporting to deft-directive-sync Phase 7 summary (pre-v0.20 legacy / v0.20+ OK / v0.20+ with warnings) - Add post-migration placeholder integrity check to scripts/vbrief_validate.py -- warns when SPECIFICATION.md or PROJECT.md exist but lack <!-- deft:deprecated-redirect --> sentinel - Add greenfield path verification documenting lifecycle folder creation in deft-directive-setup - All error messages include specific fix commands (task migrate:vbrief, task project:render, task scope:activate) - Add CHANGELOG entry under [Unreleased] - 39 tests in tests/cli/test_precutover_guard.py covering placeholder integrity, skill guard content, actionable messages, anti-patterns, model state reporting, and greenfield path
Greptile SummaryThis PR adds a Pre-Cutover Detection Guard to three skills ( Confidence Score: 5/5Safe to merge — all findings are P2 style/polish items with no functional impact The feature logic is sound, the three skills are consistent, and the validator behaves correctly. The 39 tests accurately exercise the acceptance criteria. Only minor polish issues remain: a duplicate CHANGELOG entry (pre-existing squash artefact, now visible because the file was touched), a slightly imprecise warning message for empty deprecated files, and a silent OSError inconsistency — none affect runtime behavior or correctness. CHANGELOG.md — duplicate entry on lines 18-19 should be cleaned up before merge Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([Skill invoked:\nsetup / build / sync]) --> B{Pre-Cutover\nDetection Guard}
B --> C{SPECIFICATION.md or\nPROJECT.md exists\nwithout sentinel?}
B --> D{vbrief/specification.vbrief.json\nexists but no lifecycle\nfolders?}
C -->|Yes| E[Display actionable message\n+ task migrate:vbrief]
D -->|Yes| E
C -->|No| F{All checks\npass?}
D -->|No| F
F -->|setup / build| G[Stop — redirect to\nmigration first]
F -->|sync| H[Skip Phases 1-6\nProceed to Phase 7]
E --> G
E --> H
H --> I[Phase 7 Summary\nDocument Model:\npre-v0.20 legacy]
F -->|No pre-cutover| J[Continue normally]
J --> K[Phase 7 Summary\nDocument Model:\nv0.20+ OK or v0.20+ with warnings]
subgraph vbrief_validate.py
V1([task check]) --> V2[validate_all]
V2 --> V3[validate_deprecated_placeholders]
V3 --> V4{SPECIFICATION.md or\nPROJECT.md exists\nwithout sentinel?}
V4 -->|Yes| V5[WARN: non-redirect content\nexit 0]
V4 -->|No| V6[OK: exit 0]
end
Prompt To Fix All With AIThis is a comment left during a code review.
Path: CHANGELOG.md
Line: 18-19
Comment:
**Duplicate CHANGELOG entry with corrupted trailing text**
The `### Changed` section contains two entries for the same PR (#314, #319 deft-setup+interview rename). Line 18 additionally has the PR title appended twice after "assertions" (a squash-merge artefact), producing malformed text. Per AGENTS.md Pre-Commit File Review rule 2, modified files must be re-read and checked for accidental double entries before committing.
Line 18 should be removed entirely (or de-corrupted), keeping only the clean copy at line 19.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: scripts/vbrief_validate.py
Line: 537-543
Comment:
**Misleading warning message for empty deprecated files**
When a deprecated file is empty (zero bytes), `DEPRECATED_REDIRECT_SENTINEL not in content` is `True`, so the validator emits `"SPECIFICATION.md contains non-redirect content"`. An empty file contains no content — redirect or otherwise — making the message technically inaccurate. The test `test_empty_deprecated_file_warns` intentionally exercises this path and asserts the current wording, so both the message and the test assertion would need updating together.
Consider a formulation that's accurate for both cases:
```python
warnings.append(
f"{filename} is missing the deprecation redirect sentinel -- "
"this file is deprecated; use scope vBRIEFs "
"in vbrief/ instead"
)
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: scripts/vbrief_validate.py
Line: 533-535
Comment:
**Silent OSError differs from the rest of the validator's error handling**
`load_vbrief()` appends an error string to the errors list on `OSError`, making read failures visible. Here, an `OSError` (e.g. permission denied on an existing `SPECIFICATION.md`) is silently `continue`d — the warning is never emitted, producing a false-negative. For consistency, consider either adding the path to warnings or errors:
```python
except OSError as exc:
warnings.append(f"{filename}: cannot read: {exc}")
continue
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "fix: address Greptile review findings (b..." | Re-trigger Greptile |
- Fix CHANGELOG test count: 42 -> 39 (semantic accuracy per AGENTS.md rule) - Add Document Model item (#7) to deft-directive-sync Phase 7 numbered list - Clarify sync guard wording: 'stop' -> 'skip Phases 1-6, proceed to Phase 7' to avoid conflict with Phase 7 obligation
Summary
Add pre-cutover detection and backward compatibility guard for the Phase 2 vBRIEF Architecture Cutover. Skills detect old-model artifacts and redirect to migration with actionable messages. Post-migration placeholder integrity is validated by task check.
Closes #334
Changes
Old-model detection (3 skills)
Actionable error messages
Post-migration placeholder integrity (vbrief_validate.py)
Sync model state reporting
Greenfield path verification
Tests
Checklist