spec-kitty / spec-kitty/spec-kitty
Dossier: fail-closed WP schema validation flips non-task WP-glob files to non-present (latent false DIVERGENCE)
- Dominant language
- Python
- Stars
- 1.6k
- Forks
- 165
- Avg merge
- 14h 52m
- Merged PRs (30d)
- 303
Description
Discovered by the landing-pass adversarial squad on **PR #2898** (`fix(dossier): reconciler squad-review source follow-ups (#2883 items 1-4,7)`). Non-blocking for #2898 — **nil live blast radius today** — but a latent false-`DIVERGENCE` class the dossier reconciler exists to prevent, so tracking it here as a sibling of #2883.
## Symptom
`_is_wp_artifact` (`src/specify_cli/dossier/indexer.py`) classifies **any** `.md` file whose stem matches `^WP\d{2,}` (case-insensitive) — **anywhere** in the mission tree, not just `tasks/WP##.md` — as a task work-package, and hashes it via `WPMetadata`'s static projection. `WPMetadata` is `extra="forbid"`.
PR #2898 item 3 changed `_hash_wp_projection`'s `ValidationError` handling from *raw-byte fallback* to *fail-closed → mark non-present*. Correct for a genuine task WP with churny runtime frontmatter (the AS-4 case). But it also fires for a legitimate **non-task auxiliary file** that merely shares the `WP##` name prefix and carries its own (valid, but non-`WPMetadata`) frontmatter — flipping it from `is_present=True` (stable raw-byte hash) to `is_present=False` (empty hash, error reason).
If a snapshot recorded such a file **present** (pre-PR code) and a later `reconcile` re-indexes source with the new code, the path becomes `in_recorded=True, in_source=False` → **`DivergenceKind.MISSING_IN_SOURCE` with zero underlying file change**.
## Live reproduction (verified on the PR branch)
Real file already on `main`:
`kitty-specs/quality-devex-hardening-3-2-01KRJGKH/glossary-fragments/WP07.md`
(frontmatter: `wp`, `mission`, `authored_by`, `authored_at`, `requirement_ref` — a real glossary-fragment schema, not a task WP).
```python
from pathlib import Path
from specify_cli.dossier.indexer import Indexer
from specify_cli.dossier.manifest import ManifestRegistry
d = Indexer(ManifestRegistry()).index_feature(
Path("kitty-specs/quality-devex-hardening-3-2-01KRJGKH"), "software-dev")
# glossary-fragments/WP07.md:
# BEFORE #2898 (main): is_present=True, hash=01b90436c3e8...
# AFTER #2898: is_present=False, hash='', error_reason="wp frontmatter invalid: 6 validation errors for WPMetadata..."
```
## Why it did not block #2898 (blast radius = nil today)
Verified across the repo:
- The **only** two non-`tasks/` `WP*.md` files with real frontmatter are `glossary-fragments/WP07.md` and `refactor-stable-gate-substrate-01KWK3FY/WP01-activity-log.md`.
- **Neither mission has a recorded dossier snapshot** (`.kittify/dossiers/*/snapshot-latest.json`).
- **No** `snapshot-latest.json` in the repo records a WP-glob-matching non-task `.md` file as present (the `occurrences/WP0*.yaml` hits are `.yaml`, which `_is_wp_artifact` does not match).
So there is no recorded snapshot that can currently produce the false divergence. It becomes live the moment a mission with such a file gets a snapshot on one code version and is reconciled on the other. Bounded by spec A-003 (no live hosted customers).
## Root cause
The `WP##`-prefix glob is a filename heuristic; the fail-closed change silently assumes every `WP\d+*.md` in a mission tree is a `WPMetadata`-shaped task file. That assumption is false on `main` today. The pre-PR raw-byte fallback happened to be *correct* for these auxiliary files (static frontmatter → nothing runtime-mutable to churn); item 3 removed that safety for the wrong set.
## Recommended fix (dossier-mission owner's call)
A structural fix at the artifact-classification boundary, not another `try/except` branch. Options:
- **(a)** Scope the `WPMetadata` validation / fail-closed path to files actually classified as task WPs (under `tasks/`, or manifest-declared). 1961 of 1963 `WP*.md` live under `tasks/`; the 2 exceptions are exactly the false positives, so this is low-risk and keeps #2898's own `test_schema_invalid_frontmatter_wp_fails_closed` green (its WP is built under `tasks/`).
- **(b)** Narrow `_WP_FILENAME_RE` / `_is_wp_artifact` at source so auxiliary files hash as generic artifacts (raw-byte), never entering the WP-projection path. Changes their `artifact_class`; consider against the manifest.
Best resolved coherently alongside the remaining #2883 items (5/6) on this same seam.
## Also fold when this is picked up (squad MINORs)
1. **`ArtifactRef` `is_present → non-empty content_hash_sha256` is an unenforced invariant** (architect lens). #2898's unified `snapshot.present_projection` now silently *depends* on it (present artifacts with an empty hash are dropped, not surfaced as divergence). Today it's enforced only by `indexer.py` construction discipline — `ArtifactRef.content_hash_sha256` (`models.py`) has no `min_length` and no cross-field validator. Add a `model_validator` (or at least a targeted regression test) so a future producer can't silently reintroduce the drop.
2. **Test-witness gap** (debugger lens). `tests/dossier/test_indexer.py::test_schema_invalid_frontmatter_wp_fails_closed` witnesses only the single-field regex-format `ValidationError` (`work_package_id: NOT-A-WP`), not the `extra="forbid"` blast-radius shape (foreign-field mapping) that actually exists on `main`. Add a case using the real `glossary-fragments/WP07.md`-shaped frontmatter.
Refs #2883, #2180. Surfaced during PR #2898 landing.
Contributor guide
Assessment
This issue has not been assessed yet.