Skip to content

Commit 501bad4

Browse files
benmfzenclaude
andcommitted
fix: an unreadable file must not read as an absent one
`Path.exists()` returns False when the OS denies access — macOS TCC, unix permissions, an unmounted share. Used as a guard before a read, it turns "I may not read this" into "this does not exist" and the caller falls back to defaults with nothing in the output to say so. Found in the field on 2026-07-20: a sibling deployment reported an empty review folder while nine finished videos sat in it, because the folder scan was guarded by `exists()` and the process had lost Documents access. The number 0 was indistinguishable from a real empty folder. Acting on it would have meant reporting "nothing to schedule" against a full queue. The same guard sits in front of this repo's own state: plan.json, the damping ledger, the performance store and config.json. An unreadable plan looks like no plan, so the scheduler books against built-in defaults; an unreadable store looks like no history, so the damped planner sees an empty record. Both give confident, wrong output. Adds postpeer_pilot/safe_read.py: a read either returns content, returns None because the file genuinely is absent, or raises Unreadable. Callers that want a default now choose it explicitly, so the fallback is a decision in the code rather than an accident of the filesystem. Applied to plan.active, plan.series_slots, plan.ledger_entries, plan.captions_by_post, perf.latest, perf meta-token lookup, config.load and config.api_key. Regression tests cover absent, present and chmod-000 paths, including that an unreadable plan or config raises instead of defaulting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e2c4114 commit 501bad4

5 files changed

Lines changed: 126 additions & 20 deletions

File tree

postpeer_pilot/config.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
scheduled.jsonl local ledger of what this tool scheduled (series caps)
1313
"""
1414
import json
15+
from . import safe_read
1516
import os
1617
from pathlib import Path
1718

@@ -51,17 +52,17 @@
5152
def load() -> dict:
5253
cfg = dict(DEFAULTS)
5354
f = HOME / "config.json"
54-
if f.exists():
55-
cfg.update(json.loads(f.read_text()))
55+
raw = safe_read.read_text_if_present(f)
56+
if raw is not None:
57+
cfg.update(json.loads(raw))
5658
return cfg
5759

5860

5961
def api_key() -> str:
6062
if os.environ.get("POSTPEER_API_KEY"):
6163
return os.environ["POSTPEER_API_KEY"]
6264
env = HOME / ".env"
63-
if env.exists():
64-
for line in env.read_text().splitlines():
65+
for line in safe_read.read_lines_if_present(env):
6566
if line.startswith("POSTPEER_API_KEY="):
6667
return line.split("=", 1)[1].strip()
6768
raise RuntimeError(f"POSTPEER_API_KEY not set (env var or {env})")

postpeer_pilot/perf.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from datetime import datetime, timezone
2121
from pathlib import Path
2222

23-
from . import config
23+
from . import config, safe_read
2424

2525
STORE = config.HOME / "performance.jsonl"
2626

@@ -42,10 +42,8 @@ def _append(rows: list):
4242

4343
def latest() -> list:
4444
"""Latest snapshot per (source, title): [{'title', 'views', 'source'}]."""
45-
if not STORE.exists():
46-
return []
4745
best = {}
48-
for ln in STORE.read_text().splitlines():
46+
for ln in safe_read.read_lines_if_present(STORE):
4947
try:
5048
r = json.loads(ln)
5149
best[(r.get("source"), r["title"])] = r # append-only: later line wins
@@ -128,8 +126,7 @@ def pull_meta(limit: int = 50) -> int:
128126
return 0
129127
token = None
130128
env = Path(m.get("env", "")).expanduser()
131-
if env.exists():
132-
for ln in env.read_text().splitlines():
129+
for ln in safe_read.read_lines_if_present(env):
133130
if ln.startswith("META_PAGE_TOKEN="):
134131
token = ln.split("=", 1)[1].strip()
135132
if not token:

postpeer_pilot/plan.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from collections import defaultdict
88
from datetime import date, datetime, timedelta
99

10-
from . import api, config
10+
from . import api, config, safe_read
1111

1212
PLAN_FILE = config.HOME / "plan.json"
1313
LEDGER = config.HOME / "scheduled.jsonl"
@@ -19,8 +19,9 @@ def active() -> dict:
1919
plan = {"per_day_by_wd": {int(k): int(v) for k, v in cfg["per_day_by_wd"].items()},
2020
"slot_hours": list(cfg["slot_hours"]),
2121
"series_day_cap": int(cfg["series_day_cap"]), "source": "config defaults"}
22-
if PLAN_FILE.exists():
23-
p = json.loads(PLAN_FILE.read_text())
22+
raw = safe_read.read_text_if_present(PLAN_FILE)
23+
if raw is not None:
24+
p = json.loads(raw)
2425
plan.update({"per_day_by_wd": {int(k): int(v) for k, v in p["per_day_by_wd"].items()},
2526
"slot_hours": list(p.get("slot_hours", plan["slot_hours"])),
2627
"source": "plan.json"})
@@ -49,8 +50,7 @@ def _series_by_day(exclude_slots: set | None = None) -> dict:
4950
handed out AND ledger-recorded in the same run must not count twice toward the cap."""
5051
out = defaultdict(list)
5152
exclude_slots = exclude_slots or set()
52-
if LEDGER.exists():
53-
for ln in LEDGER.read_text().splitlines():
53+
for ln in safe_read.read_lines_if_present(LEDGER):
5454
try:
5555
r = json.loads(ln)
5656
if r.get("series") and r["when"] not in exclude_slots:
@@ -70,10 +70,8 @@ def record(when: str, video: str, series: str | None, post_id: str = "",
7070

7171

7272
def ledger_entries() -> list:
73-
if not LEDGER.exists():
74-
return []
7573
out = []
76-
for ln in LEDGER.read_text().splitlines():
74+
for ln in safe_read.read_lines_if_present(LEDGER):
7775
try:
7876
out.append(json.loads(ln))
7977
except json.JSONDecodeError:
@@ -86,8 +84,7 @@ def ledger_by_post_id() -> dict:
8684
performance matching: a published post whose id is in here needs no fuzzy text
8785
match to know its canonical caption."""
8886
out = {}
89-
if LEDGER.exists():
90-
for ln in LEDGER.read_text().splitlines():
87+
for ln in safe_read.read_lines_if_present(LEDGER):
9188
try:
9289
r = json.loads(ln)
9390
if r.get("post_id"):

postpeer_pilot/safe_read.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
"""Absent and unreadable are not the same thing.
2+
3+
`Path.exists()` returns **False** when the OS refuses access — macOS TCC, unix
4+
permissions, an unmounted network share. Used as a guard before reading a file,
5+
it silently turns "I am not allowed to read this" into "this does not exist",
6+
and the caller falls back to defaults without anyone noticing.
7+
8+
For a publishing tool that failure is not cosmetic. A plan file that cannot be
9+
read looks like *no plan*, so the scheduler quietly books against built-in
10+
defaults; a store that cannot be read looks like *no history*, so the damped
11+
planner sees an empty performance record. Both produce confident, wrong output.
12+
13+
The rule this module enforces: a read either returns content, returns ``None``
14+
because the file genuinely is not there, or raises. It never degrades quietly.
15+
"""
16+
from __future__ import annotations
17+
18+
from pathlib import Path
19+
20+
21+
class Unreadable(OSError):
22+
"""The path exists (or its status is unknowable) but could not be read."""
23+
24+
25+
def read_text_if_present(path: Path) -> str | None:
26+
"""Return file contents, or ``None`` if the file genuinely does not exist.
27+
28+
Raises :class:`Unreadable` for every other failure — permission denied,
29+
unreadable mount, a directory where a file was expected. Callers that want
30+
a default must choose it explicitly after catching, so the fallback is a
31+
decision in the code rather than an accident of the filesystem.
32+
"""
33+
try:
34+
return Path(path).read_text()
35+
except FileNotFoundError:
36+
return None
37+
except OSError as exc: # PermissionError, NotADirectoryError, EIO …
38+
raise Unreadable(f"{path} exists but could not be read: {exc}") from exc
39+
40+
41+
def read_lines_if_present(path: Path) -> list[str]:
42+
"""``read_text_if_present`` split into lines; ``[]`` when the file is absent."""
43+
raw = read_text_if_present(path)
44+
return raw.splitlines() if raw is not None else []

tests/test_safe_read.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"""Unreadable must never look like absent.
2+
3+
Regression for the failure found on 2026-07-20: `queue_status` reported an empty
4+
review folder while nine finished videos sat in it. The guard was
5+
`if PATH.exists()` — and `exists()` returns False when the OS denies access, so a
6+
permission problem was indistinguishable from "nothing there". Anything that
7+
schedules against such a reading books work it cannot see.
8+
"""
9+
import json
10+
import os
11+
import stat
12+
13+
import pytest
14+
15+
from postpeer_pilot import config, plan, safe_read
16+
17+
18+
def test_absent_file_reads_as_none(tmp_path):
19+
assert safe_read.read_text_if_present(tmp_path / "nope.json") is None
20+
assert safe_read.read_lines_if_present(tmp_path / "nope.jsonl") == []
21+
22+
23+
def test_present_file_reads_through(tmp_path):
24+
f = tmp_path / "there.txt"
25+
f.write_text("a\nb\n")
26+
assert safe_read.read_text_if_present(f) == "a\nb\n"
27+
assert safe_read.read_lines_if_present(f) == ["a", "b"]
28+
29+
30+
@pytest.mark.skipif(os.geteuid() == 0, reason="root bypasses file permissions")
31+
def test_unreadable_file_raises_instead_of_defaulting(tmp_path):
32+
f = tmp_path / "locked.json"
33+
f.write_text("{}")
34+
f.chmod(0)
35+
try:
36+
with pytest.raises(safe_read.Unreadable):
37+
safe_read.read_text_if_present(f)
38+
finally:
39+
f.chmod(stat.S_IRUSR | stat.S_IWUSR)
40+
41+
42+
@pytest.mark.skipif(os.geteuid() == 0, reason="root bypasses file permissions")
43+
def test_unreadable_plan_does_not_silently_fall_back(tmp_path, monkeypatch):
44+
"""The important one: an unreadable plan must NOT masquerade as 'no plan',
45+
because the scheduler would then quietly book against built-in defaults."""
46+
pf = tmp_path / "plan.json"
47+
pf.write_text(json.dumps({"per_day_by_wd": {str(i): 1 for i in range(7)}}))
48+
pf.chmod(0)
49+
monkeypatch.setattr(plan, "PLAN_FILE", pf)
50+
try:
51+
with pytest.raises(safe_read.Unreadable):
52+
plan.active()
53+
finally:
54+
pf.chmod(stat.S_IRUSR | stat.S_IWUSR)
55+
56+
57+
@pytest.mark.skipif(os.geteuid() == 0, reason="root bypasses file permissions")
58+
def test_unreadable_config_does_not_silently_fall_back(tmp_path, monkeypatch):
59+
f = tmp_path / "config.json"
60+
f.write_text("{}")
61+
f.chmod(0)
62+
monkeypatch.setattr(config, "HOME", tmp_path)
63+
try:
64+
with pytest.raises(safe_read.Unreadable):
65+
config.load()
66+
finally:
67+
f.chmod(stat.S_IRUSR | stat.S_IWUSR)

0 commit comments

Comments
 (0)