Skip to content

fix: #649 review round 1 -- add's store reads and exit codes, GitHub tags, a real MSRV job - #650

Merged
sotashimozono merged 1 commit into
nextfrom
fix/649-review-1
Sep 29, 2026
Merged

sotashimozono merged 1 commit into
nextfrom
fix/649-review-1

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

Round 1 of 3 of the review of the promotion PR #649. Bumps the version to 0.9.0-beta.28.

Fixed

  • doiget add and unreadable store entries (silent-failure review, HIGH). A store.read error, such as corrupt TOML or a lock timeout, was treated as "no entry". The entry was then re-resolved and overwritten, and its tags, collections and annotation were lost. add now stops with UnreadableEntry, exit 4. add --from-dir and missing now print a warning for such an entry instead of dropping it silently.
  • add exit codes were the reverse of docs/ERRORS.md §4: a failed resolve exited 4 and a store failure exited 1. A failed resolve now exits the way fetch would for the same error code (cli_exit_code), and a store failure exits 4.
  • GitHub tags. A new valid_tag check follows git's ref-name rules and also refuses % and #. It applies to tags taken from the URL and to the API's tag_name, so a tag cannot escape the repos/{owner}/{repo} path.
  • MSRV. The declared MSRV was 1.86, but rmcp 3.x requires 1.88. The msrv job never caught this: it installed 1.86, but rust-toolchain.toml (stable) overrode it. The job log shows the override. The declared MSRV is now 1.88, and both MSRV jobs pin it with RUSTUP_TOOLCHAIN. docs/PUBLIC_API.md §7 allows raising the MSRV in a minor release with a CHANGELOG entry; 0.9.0 is a minor release.

Changed

  • PdfLegStatus::{PreprintFallback, PreprintDoiFallback}.found_by is now preprint::FoundBy instead of String, with a new FoundBy::Unpaywall. The wire values are unchanged.
  • The preprint_allowlist doc comment now names each finder's own gate (bioRxiv, INSPIRE and ADS each have one).

Tests added

  • A failed arXiv fallback, whether the host returns 404 or a body that is not a PDF, keeps blocked, suggested_arxiv_id and the block's message, and writes nothing.
  • add: running it with no arguments is misuse (exit 2); an unreadable entry exits 4 and leaves the entry unchanged; an unresolvable DOI exits 1 (NOT_FOUND).
  • coverage: a 404 from Crossref gives error[NOT_FOUND], exit 1, and empty stdout.

Not taken

  • One shared "logged GET" helper for preprint, pubmed and software. The three helpers differ on purpose: a 404 means "not found" for GitHub, "no record" for PubMed (classified from the body), and "this finder found nothing" for the preprint finders. PubMed already reports a 429 as RATE_LIMITED; only the default 60 s wait is GitHub-specific. Merging them would put all three sources' 404 rules behind one flag, which is a riskier change than this round should make.

🤖 Generated with Claude Code

…tags, a real MSRV job

- doiget add stops on an unreadable store entry instead of writing over
  it; --from-dir and missing report one rather than dropping it silently.
- add's exit codes follow docs/ERRORS.md §4 (they were swapped).
- A GitHub tag with .., %, ?, # or whitespace is refused, from the URL
  and from the latest-release answer.
- PdfLegStatus found_by is the FoundBy enum (with Unpaywall), not a String.
- Declared MSRV 1.86 -> 1.88: rmcp 3.x already needed it, and the msrv
  job never saw it because rust-toolchain.toml overrode its toolchain.
  Both MSRV jobs now pin with RUSTUP_TOOLCHAIN.
- preprint_allowlist's doc names each finder's own gate.
- Tests: failed arXiv fallback keeps the block (404 / not a PDF), add's
  misuse / unreadable-entry / unresolved exits, coverage's error path.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Signed-off-by: sotashimozono <[email protected]>
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.96491% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/doiget-cli/src/commands/add.rs 44.44% 5 Missing ⚠️
crates/doiget-cli/src/commands/missing.rs 60.00% 2 Missing ⚠️
crates/doiget-cli/src/commands/fetch.rs 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@sotashimozono
sotashimozono merged commit a9314c5 into next Sep 29, 2026
49 of 50 checks passed
github-actions Bot added a commit that referenced this pull request Sep 29, 2026
fix: #649 review round 1 -- add's store reads and exit codes, GitHub tags, a real MSRV job a9314c5
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.

1 participant