fix(store): a not-determined marker no longer overwrites a determination

CHANGELOG.md

@@ -10,6 +10,35 @@ flag changes and `doiget-mcp` tool spec changes will be called out explicitly he
 
 ## [Unreleased]
 
+### Fixed
+
+- **[store]** A default `metadata_only` re-write **downgraded a known
+  `oa_status` and `license` to their not-determined markers**, and said nothing
+  (#583).
+
+  `docs/STORE.md` §6 does let a re-fetch rewrite the `[doiget]` table, but the
+  permission is conditional: *"This is intentional, not silent: ... the operator
+  always learns the entry was downgraded and why."* Since #539, `metadata_only`
+  only runs the OA lookup when `include_oa_location` is set, so the ordinary call
+  shape produces `oa_status: None` and `license: "unknown"` — and those won,
+  with no `note:`, no `pdf.status` and no log row. The condition the permission
+  rests on was not met.
+
+  Both values are markers, not readings: `oa_status` is "omitted when not
+  determined" and `license` falls back to `"unknown"`. A paper that genuinely
+  stops being open access reports `Some("closed")`; a license that changes
+  reports the new string. So a call that carries the marker did not look, and
+  preferring the stored value is not a guess about which is newer.
+  `merge_metadata` now keeps the stored value in exactly that case, and
+  `"unknown"` has a name (`LICENSE_UNDETERMINED`) so the check does not hang off
+  a bare literal. [ADR-0056](docs/DECISIONS/0056-not-determined-is-not-an-answer.md),
+  and `docs/STORE.md` §6's note is amended to scope its claim to determinations.
+
+  The issue as filed said `oa_url` was overwritten with null. It is not —
+  `merge_opt!(url)` already protected it, which a probe confirmed before any of
+  this was written. Only the two `[doiget]` fields were affected, and the fix is
+  a merge rule rather than the store partition the issue proposed.
+
 ### Changed
 
 - **[ci]** One test was taking ten minutes and being run five times. The five

Cargo.toml

@@ -15,7 +15,7 @@ exclude = [
 ]
 
 [workspace.package]
-version       = "0.8.13-beta.22"
+version       = "0.8.13-beta.23"
 edition       = "2021"
 rust-version  = "1.86"
 license       = "MIT"
@@ -41,9 +41,9 @@ features-doc-link = "docs/SOURCES.md"
 
 [workspace.dependencies]
 # Core
-doiget-core = { path = "crates/doiget-core", version = "0.8.13-beta.22" }
-doiget-cli  = { path = "crates/doiget-cli", version = "0.8.13-beta.22" }
-doiget-mcp  = { path = "crates/doiget-mcp", version = "0.8.13-beta.22" }
+doiget-core = { path = "crates/doiget-core", version = "0.8.13-beta.23" }
+doiget-cli  = { path = "crates/doiget-cli", version = "0.8.13-beta.23" }
+doiget-mcp  = { path = "crates/doiget-mcp", version = "0.8.13-beta.23" }
 
 # Async runtime — features are deliberately limited (avoid `full`).
 tokio = { version = "1", default-features = false, features = [

crates/doiget-core/src/store/fs_store.rs

@@ -36,7 +36,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use fs2::FileExt;
 use tracing::warn;
 
-use super::metadata::{DoigetExtension, Metadata};
+use super::metadata::{DoigetExtension, Metadata, LICENSE_UNDETERMINED};
 use super::{EntryInfo, Store, StoreError};
 use crate::{Safekey, SCHEMA_VERSION};
 
@@ -532,11 +532,30 @@ fn merge_metadata(existing: Metadata, incoming: Metadata) -> Metadata {
         out.arxiv_categories = existing.arxiv_categories;
     }
 
-    // [doiget]: doiget owns this table; incoming wins (already in `out`).
-    // If incoming has no [doiget] but existing did, keep the existing one
-    // so a metadata-only re-write doesn't silently drop a fetch record.
-    if out.doiget.is_none() && existing.doiget.is_some() {
-        out.doiget = existing.doiget;
+    // [doiget]: doiget owns this table, so a re-write wins (STORE.md §6) --
+    // except for the two fields whose "absent" value is a marker rather than
+    // a reading. `oa_status` is omitted when not determined (#281) and
+    // `license` falls back to `LICENSE_UNDETERMINED`. A `metadata_only` call
+    // without `include_oa_location` never runs the OA lookup, so it carries
+    // exactly those markers; letting them win replaces an answer with the
+    // absence of one. STORE.md §6 permits a `[doiget]` downgrade because it
+    // is reported to the operator (#118) -- on this path nothing is, which is
+    // what ADR-0056 closes. Preserving cannot suppress real news: a paper that
+    // stops being open access reports `Some("closed")`, and a license that
+    // changes reports the new string.
+    match (existing.doiget, out.doiget.as_mut()) {
+        (Some(existing_d), Some(incoming_d)) => {
+            if incoming_d.oa_status.is_none() {
+                incoming_d.oa_status = existing_d.oa_status;
+            }
+            if incoming_d.license == LICENSE_UNDETERMINED {
+                incoming_d.license = existing_d.license;
+            }
+        }
+        // Incoming carries no [doiget] at all: keep the existing fetch record
+        // rather than dropping it.
+        (Some(existing_d), None) => out.doiget = Some(existing_d),
+        (None, _) => {}
     }
 
     // `other` (unknown tables / fields): union, prefer EXISTING on key
@@ -870,6 +889,54 @@ mod tests {
         FsStore::new(root).expect("FsStore::new")
     }
 
+    #[test]
+    fn a_default_rewrite_does_not_downgrade_a_known_oa_status_or_license() {
+        // Issue #583. `metadata_only` without `include_oa_location` never
+        // runs the OA lookup, so it carries `oa_status: None` and
+        // `license: "unknown"` -- the not-determined markers, not readings.
+        // Letting them win would replace an answer with the absence of one,
+        // and STORE.md §6 only permits a [doiget] downgrade that is reported.
+        let mut existing = sample_metadata();
+        existing.url = Some("https://example.org/paper.pdf".to_string());
+        let d = existing.doiget.as_mut().expect("sample has [doiget]");
+        d.oa_status = Some("gold".to_string());
+        d.license = "CC-BY-4.0".to_string();
+
+        let mut incoming = sample_metadata();
+        incoming.url = None;
+        let d = incoming.doiget.as_mut().expect("sample has [doiget]");
+        d.oa_status = None;
+        d.license = LICENSE_UNDETERMINED.to_string();
+
+        let out = merge_metadata(existing, incoming);
+        let d = out.doiget.expect("[doiget] survives");
+        assert_eq!(d.oa_status.as_deref(), Some("gold"));
+        assert_eq!(d.license, "CC-BY-4.0");
+        // `url` was already protected by `merge_opt!`; pinned so the two
+        // halves of #583 cannot drift apart.
+        assert_eq!(out.url.as_deref(), Some("https://example.org/paper.pdf"));
+    }
+
+    #[test]
+    fn a_rewrite_that_determined_a_new_oa_status_or_license_still_wins() {
+        // The other half: preserving must not suppress real news. A paper
+        // that stops being open access reports `Some("closed")`, not `None`.
+        let mut existing = sample_metadata();
+        let d = existing.doiget.as_mut().expect("sample has [doiget]");
+        d.oa_status = Some("gold".to_string());
+        d.license = "CC-BY-4.0".to_string();
+
+        let mut incoming = sample_metadata();
+        let d = incoming.doiget.as_mut().expect("sample has [doiget]");
+        d.oa_status = Some("closed".to_string());
+        d.license = "CC-BY-NC-4.0".to_string();
+
+        let out = merge_metadata(existing, incoming);
+        let d = out.doiget.expect("[doiget] survives");
+        assert_eq!(d.oa_status.as_deref(), Some("closed"));
+        assert_eq!(d.license, "CC-BY-NC-4.0");
+    }
+
     #[test]
     fn merge_metadata_preserves_existing_arxiv_categories() {
         // Issue #303 / review #318: a later metadata-only re-write that did

crates/doiget-core/src/store/metadata.rs

@@ -97,6 +97,14 @@ pub struct Metadata {
     pub other: std::collections::BTreeMap<String, toml::Value>,
 }
 
+/// The value `[doiget].license` carries when no license was determined.
+///
+/// It is a marker for "the lookup did not produce one", not a reading. A
+/// resolver that genuinely reports a license writes that license; nothing
+/// reports `"unknown"` as news. `merge_metadata` relies on that to tell an
+/// absent answer from a new one.
+pub const LICENSE_UNDETERMINED: &str = "unknown";
+
 /// doiget-specific extension table (`[doiget]`).
 ///
 /// Per `docs/STORE.md` §6, doiget owns this table outright and may

docs/DECISIONS/0056-not-determined-is-not-an-answer.md

@@ -0,0 +1,88 @@
+# 0056 - A not-determined marker never overwrites a determination
+
+- **Date:** 2026-08-31
+- **Status:** Accepted
+- **Supersedes:** -
+- **Amends:** [`docs/STORE.md`](../STORE.md) §6 — the re-fetch downgrade note
+- **Complements:** [0014](0014-docs-class-system.md) — the ADR a NORMATIVE doc change requires; [0055](0055-error-disposition.md), which drew the same distinction on the wire
+- **Source:** #583 (a default `metadata_only` re-write silently downgraded `[doiget].oa_status` and `.license`)
+
+## Context
+
+`docs/STORE.md` §6 lets doiget rewrite the `[doiget]` table on a re-fetch, and
+says why:
+
+> A doiget re-fetch of an entry that previously had a PDF but is now
+> metadata-only ... rewrites the `[doiget]` table (`source`, `size_bytes`, …)
+> in place. **This is intentional, not silent:** as of issue #118 the
+> blocked-PDF reason is surfaced to the caller ... so the operator always
+> learns the entry was downgraded and why.
+
+The permission is conditional on the report. Since #539 there is a path where
+the report does not exist: `metadata_only` takes `include_oa_location`, and when
+it is omitted — the ordinary call shape — the OA lookup never runs. The record
+built from that outcome carries `oa_status: None` and `license: "unknown"`, the
+merge let them win, and a caller got no `note:` line, no `pdf.status`, and no log
+row saying a known `gold` / `CC-BY-4.0` had been replaced with *not determined*.
+
+Measured before deciding, because the issue as filed claimed the wrong field:
+
+| field | existing | after a default re-write |
+|---|---|---|
+| `url` (where `oa_url` lands) | `Some("https://…/paper.pdf")` | `Some(…)` — preserved by `merge_opt!` |
+| `[doiget].oa_status` | `Some("gold")` | `None` |
+| `[doiget].license` | `"CC-BY-4.0"` | `"unknown"` |
+
+So the reserved top-level fields were never at risk. The defect is confined to
+the two `[doiget]` fields whose absent value is a **marker** rather than a
+reading — and both are documented as such: `oa_status` is "omitted when not
+determined (#281)", `license` is "an OA license string, or the literal
+`unknown`".
+
+## Decision
+
+**A marker for "no answer" does not overwrite an answer.**
+
+In `merge_metadata`, the `[doiget]` arm keeps the existing `oa_status` when the
+incoming one is `None`, and the existing `license` when the incoming one is
+`LICENSE_UNDETERMINED`. Everything else in the table still follows §6: doiget
+owns it and the re-write wins.
+
+This cannot suppress real news, which is the only reason it is safe:
+
+- a paper that stops being open access reports `oa_status: Some("closed")`
+- a license that changes reports the new string
+
+Neither reports the marker. `None` and `"unknown"` are only ever produced by a
+call that did not look, so preferring the stored value is not a guess about
+which is newer — it is the observation that only one of the two is a value.
+
+`"unknown"` gained a name (`LICENSE_UNDETERMINED`) so the merge does not depend
+on a string literal matching the others scattered through the orchestrator.
+
+## Consequences
+
+`docs/STORE.md` §6's note says a downgrade guard "is deferred (post-MVP) — it is
+a policy choice, not a correctness bug." That remains true of the case it was
+written about (#123: an entry loses its PDF because the OA host went
+off-allowlist) — there the downgrade is real, and it is reported. It is not true
+of a field the caller never asked about, so the note is amended to scope its
+claim to determinations rather than markers.
+
+Nothing BiblioFetch.jl reads changes shape. No field is added, removed or
+renamed, `schema_version` does not move, and the reserved top-level fields keep
+the `merge_opt!` behaviour they already had. This is a change to which of two
+`[doiget]` values doiget itself keeps.
+
+### Not done
+
+**Warning on every preserve.** The reserved-field arms `warn!` when they keep an
+existing value, because there it means two tools disagree and a human may need
+to look. Here it is the ordinary outcome of the ordinary call shape; a log line
+per default `metadata_only` would be noise, and the thing worth reporting —
+losing the value — no longer happens.
+
+**Extending this to `source` or `size_bytes`.** They have no marker value.
+`size_bytes: 0` is a legitimate reading for a metadata-only entry, and `source`
+always names the resolver that actually ran. Treating either as absence would
+invent exactly the call-history dependence #583 warned against.

docs/DECISIONS/INDEX.md

@@ -72,6 +72,7 @@ Status column reconciled 2026-05-17 against `CHANGELOG.md` slices (issue #150).
 | 0053 | A DOI resolver is addressing, not hosting | Accepted | `fix/533-resolver-hop-is-addressing` | #533 |
 | 0054 | An access refusal is a type, and it collapses to `NO_OA_AVAILABLE` | Accepted | `fix/538-typed-access-refusal` | #538 |
 | 0055 | A failure says what to do about it, in three states | Accepted | `feat/506-error-disposition` | #506 |
+| 0056 | A not-determined marker never overwrites a determination | Accepted | `fix/583-store-downgrade` | #583 |
 
 ## Conventions
 

docs/STORE.md

@@ -166,7 +166,19 @@ existing on-disk value WINS over whatever doiget carries (issue #123).
 > entry's recorded state changes. This is intentional, not silent: as of issue #118 the
 > blocked-PDF reason is surfaced to the caller (CLI `note:` line / MCP `pdf.status`),
 > so the operator always learns the entry was downgraded and why. A guard that refuses
-> to downgrade is deferred (post-MVP) — it is a policy choice, not a correctness bug.
+> to downgrade a **determination** is deferred (post-MVP) — it is a policy choice, not a
+> correctness bug.
+>
+> This permission covers determinations only. Two `[doiget]` fields carry a
+> not-determined *marker* rather than a reading — `oa_status` is omitted when not
+> determined, and `license` falls back to `"unknown"` — and a re-write that carries
+> the marker did not look, so it is not news. Per
+> [ADR-0056](DECISIONS/0056-not-determined-is-not-an-answer.md) the merge keeps the
+> stored value in that case. A paper that genuinely stops being open access reports
+> `oa_status = "closed"`, and a changed license reports the new string; both still
+> win. The distinction matters because the permission above is granted on the
+> condition that the downgrade is reported, and a field the caller never asked about
+> has nothing to report it against (#583).
 
 ## 7. TOML normalization
 

site/content/developer/store.md

@@ -172,7 +172,19 @@ existing on-disk value WINS over whatever doiget carries (issue #123).
 > entry's recorded state changes. This is intentional, not silent: as of issue #118 the
 > blocked-PDF reason is surfaced to the caller (CLI `note:` line / MCP `pdf.status`),
 > so the operator always learns the entry was downgraded and why. A guard that refuses
-> to downgrade is deferred (post-MVP) — it is a policy choice, not a correctness bug.
+> to downgrade a **determination** is deferred (post-MVP) — it is a policy choice, not a
+> correctness bug.
+>
+> This permission covers determinations only. Two `[doiget]` fields carry a
+> not-determined *marker* rather than a reading — `oa_status` is omitted when not
+> determined, and `license` falls back to `"unknown"` — and a re-write that carries
+> the marker did not look, so it is not news. Per
+> [ADR-0056](DECISIONS/0056-not-determined-is-not-an-answer.md) the merge keeps the
+> stored value in that case. A paper that genuinely stops being open access reports
+> `oa_status = "closed"`, and a changed license reports the new string; both still
+> win. The distinction matters because the permission above is granted on the
+> condition that the downgrade is reported, and a field the caller never asked about
+> has nothing to report it against (#583).
 
 ## 7. TOML normalization