[antonwolfy]

Summary

#701: skill shorten and skill novelty analyze end in a traceback when --cache-dir is not a usable directory. On main, skill shorten --artifact text:rows.md --atoms atoms.json --model o --max-usd 0.05 --cache-dir cache, where cache is an existing regular file, builds the model client, pays for the first retention adjudication and then dies with FileExistsError: [Errno 17] File exists: '.../cache' from RetentionOracle._store (path.parent.mkdir(...)), before any verdict is kept; with no cap it is the same. analyze shares the oracle and fails the same way.

The fix takes the issue's first option: validate --cache-dir up front and exit 2 with an InputError that names the flag and the path, before any client is built. The second option (catch OSError in _store and warn once) does not stop a run from paying for adjudications it cannot keep, so it is not taken.

--cache-dir	main	now
an existing regular file	FileExistsError traceback after the first paid call	exit 2, Error: --cache-dir <path> is not a directory; pass a directory the retention verdicts can be stored in, no call made
a path below a regular file	same	exit 2, --cache-dir <path> has <file> above it, which is not a directory; ...
a dangling symlink, or a path below one	OSError traceback on the first store (mkdir over the link)	exit 2, --cache-dir <path> is not a directory; ... (or has <link> above it ...)
an unwritable directory, or a missing path below one	OSError traceback on the first store (mkdir/write_text unguarded)	exit 2, --cache-dir <path>: <dir> is not writable; ...
a path below a directory without search permission	PermissionError traceback from the check itself on Python before 3.14	exit 2, --cache-dir <path>: cannot inspect <dir> (...); ... (on 3.14, exists() reads it as missing and the unwritable-directory refusal applies)
an existing directory, or one that does not exist yet	works	unchanged (created on the first store)

Closes #701

The same cause in the repo commands' clone cache is tracked in #765.

Changes

• usable_cache_dir in dokimi/dokimi/commands/novelty_run.py: the one check, returning the Path (or None when the flag is unset) or raising InputError; an OSError from inspecting the path (an ancestor without search permission) becomes an InputError naming the flag and path.
• cmd_novelty_analyze (commands/novelty.py) calls it before the run estimate and any client; score_candidates (commands/novelty_atoms_file.py, reached by skill shorten) calls it before building the oracle's client.
• New tests/test_novelty_cache_dir_usable.py: the helper against a regular file, a child of a file, a dangling symlink and a child of one, a directory, a missing child of a directory, and an unwritable directory (skipped as root); skill shorten (with and without --max-usd) and skill novelty analyze driven with a regular file as --cache-dir assert exit 2, the path in the message and that no client was built.
• Changelog fragment (Fixed).

Risk

A --cache-dir that was always going to crash on the first store is now refused before the run starts, with exit 2 instead of a traceback. A directory that exists and is writable, and one that does not exist yet, behave as before. The check reads the filesystem once (exists, is_dir, os.access); a disk that fills mid-run still raises from RetentionOracle._store, unchanged.

Testing

On a Linux review host, at 8295fb3 (on main 7137604):

• make ci-local: 31 passed, 2 failed, 14 skipped; the two failures are Install gitleaks (the download returned exit 22) and Scan commits (exit 127, gitleaks not installed), so no commit scan ran here; the skips are steps that need docker, micromamba, semgrep, the fuzz expression, the deploy and matrix jobs, and the pinned Claude Code install. 20451 tests passed, 19 skipped.
• Unreadable ancestor on every Python: test_usable_cache_dir_names_the_flag_when_inspecting_the_path_raises makes is_dir() raise PermissionError and checks the InputError names --cache-dir, the path and the inspected directory; with the except OSError narrowed to FileNotFoundError it fails (together with the chmod-based test), so the refusal branch is covered also where Python 3.14 or root hides the error.
• Unsearchable ancestor: usable_cache_dir on a path below a directory of mode 0600 raised PermissionError before the fix and raises InputError now; with the try/except OSError removed, test_usable_cache_dir_refuses_a_path_below_a_directory_it_cannot_search fails (8 other tests pass).
• Dangling symlink: usable_cache_dir on cache -> missing-target (and cache/child) now raises InputError; with the walk's not probe.is_symlink() condition removed, test_usable_cache_dir_refuses_a_dangling_symlink_and_a_child_of_one fails.
• Repro: with the new test file on main's code (the helper name given main's value, a plain Path(raw)), six of the seven tests fail on their assertions (the one that accepts a directory and a missing child passes, as it should); here all seven pass.
• Mutants, one break per copy, each fails the test named: the walk-up to the nearest existing parent removed (test_usable_cache_dir_accepts_unset_a_directory_and_a_missing_child, ..._refuses_a_child_of_a_regular_file, ..._refuses_an_unwritable_directory_and_a_child_of_one); the os.access check removed (..._refuses_an_unwritable_directory_and_a_child_of_one); the is_dir check removed (..._refuses_a_regular_file_naming_the_flag_and_path, ..._refuses_a_child_of_a_regular_file, both shorten cases and the analyze test); analyze not calling the helper (test_analyze_refuses_a_cache_dir_that_is_a_file_before_any_call); score_candidates not calling it (both test_shorten_refuses_... cases).
• Merged together with fix/partial-status-guards on main 7137604: pytest -n auto 20449 passed, 19 skipped, mypy clean, ruff check clean.
• Not run: the matrix legs on Python 3.11, 3.12 and 3.14, and CI's merge commit.

Revert

A plain revert restores the traceback on an unusable --cache-dir. Nothing is stored.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated (or N/A) — see the docs-drift checklist in CONTRIBUTING.md
• Self-reviewed per "Before opening a PR" in AGENTS.md
• No ad-hoc run artifacts committed
• New files carry the SPDX license header (or are exempt in .github/.licenserc.yaml)

[exai-bot]

✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

CI at 5b438d7: 25 passing, 3 skipped.

Review details

• Commit reviewed: 5b438d7 against merge base 8295fb3
• Coverage: 2 files in 1 groups
• Engine: OpenCodeReview v1.13.5-exai.4.8f73eada, us.openai.gpt-6.1-sol
• Usage: 57k tokens (33k from cache), 15s
• Tool calls: 7 (5 file_read, 1 code_search, 1 local_check_results)
• small change set: 2 files

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.72%. Comparing base (7137604) to head (5b438d7).
⚠️ Report is 1 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #766   +/-   ##
=======================================
  Coverage   95.72%   95.72%           
=======================================
  Files         415      415           
  Lines       55363    55384   +21     
  Branches     9516     9520    +4     
=======================================
+ Hits        52994    53015   +21     
  Misses       1466     1466           
  Partials      903      903           

Files with missing lines	Coverage Δ
dokimi/dokimi/commands/novelty.py	99.74% <100.00%> (+<0.01%)	⬆️
dokimi/dokimi/commands/novelty_atoms_file.py	100.00% <100.00%> (ø)
dokimi/dokimi/commands/novelty_run.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vchamarthi]

Pushed 1 commit to fix/cache-dir-unusable (8295fb3..5b438d7).

• Existing --cache-dir of mode 0600 is pinned as refused by a new test.
• Changelog names FileExistsError only for a regular file and an OSError traceback for the other cases.
  Run: the full suite with main merged in, 20497 passed.