Skip to content

Commit 423ee59

Browse files
InterGenJLUclaude
andcommitted
fix(pkm): A31 — read-only commands tolerate a behind-schema DB (degraded column)
Found by real-install testing of pkm 0.2.0 on .227: A25 added the `degraded` column with an ALTER-TABLE migration, but migrations run only on a read-WRITE open. The read-only (immutable=1) open used by non-root readers — and by root via a read-only subcommand — returns before migrating, so on a system upgraded from a pre-A25 pkm every read command hitting list_installed crashed with `sqlite3.OperationalError: no such column: degraded` until some later root command happened to run the migration. The unit suite never caught it because it always builds a fresh, fully-migrated DB; only a real upgrade path exposes it — exactly what step-1 real-install testing is for. Fix: list_installed selects schema-tolerantly. Columns are introspected once at open (_table_columns -> self._installed_cols, on BOTH the read-only and the read-write paths), and a new _col() helper emits `NULL AS <col>` for any column absent on an un-migrated read-only DB instead of naming it directly — so reads degrade gracefully (degraded -> None = healthy) regardless of migration timing. Reuse this pattern for any future migration-added column read on the read path. tests/pkm/test_pkm_a31_schema_tolerant.py: old-schema DB (no degraded) opened read-only + list_installed no-crash, tier filter, and full-schema round-trip. Full pkm suite 323 passed; audit registry 30/30 (A31 added + verified). Folded into the unreleased pkm 0.2.0 (no version change — 0.2.0 has not shipped). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 parent a297936 commit 423ee59

3 files changed

Lines changed: 139 additions & 6 deletions

File tree

pkm/database.py

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,12 @@ def __init__(self, db_path=None, root="/", create_if_missing=True,
196196
f"file:{self.db_path}?immutable=1", uri=True
197197
)
198198
self.conn.execute("PRAGMA foreign_keys = ON")
199+
# PKM-A31: a read-only (immutable) open CANNOT run the ALTER-TABLE
200+
# migrations below — so a DB created by an older pkm can be missing
201+
# columns this pkm added (e.g. `degraded` from A25). Snapshot the
202+
# actual columns so read queries select schema-tolerantly (via
203+
# _col) instead of raising "no such column" on an un-migrated DB.
204+
self._installed_cols = self._table_columns("installed")
199205
return
200206

201207
if not create_if_missing and not self.db_path.exists():
@@ -214,6 +220,27 @@ def __init__(self, db_path=None, root="/", create_if_missing=True,
214220
self._migrate_supersedes_columns()
215221
self._migrate_q9_columns()
216222
self._migrate_a25_degraded_column()
223+
# PKM-A31: snapshot columns AFTER migrations (read-write path is fully
224+
# migrated, so this is the complete set). Keeps the read-tolerant _col
225+
# helper correct on both open paths.
226+
self._installed_cols = self._table_columns("installed")
227+
228+
def _table_columns(self, table):
229+
"""Set of column names present in `table`. Used so read queries tolerate
230+
a behind-schema DB (a read-only open can't run the ALTER-TABLE
231+
migrations, so an older DB may lack newer columns). PKM-A31."""
232+
try:
233+
return {row[1] for row in
234+
self.conn.execute(f"PRAGMA table_info({table})")}
235+
except sqlite3.OperationalError:
236+
return set()
237+
238+
def _col(self, name):
239+
"""A SELECT term for an `installed` column that yields NULL when the
240+
column is absent on a behind-schema (un-migrated, read-only) DB, instead
241+
of raising 'no such column'. PKM-A31."""
242+
return name if name in getattr(self, "_installed_cols", ()) \
243+
else f"NULL AS {name}"
217244

218245
def _migrate_supersedes_columns(self):
219246
"""Idempotent migration: add superseded_by + superseded_at columns
@@ -328,18 +355,20 @@ def list_installed(self, tier=None):
328355
can disambiguate same-version-different-release upgrades without
329356
an extra get_installed roundtrip per package.
330357
"""
358+
# PKM-A31: build the column list schema-tolerantly so a read-only open
359+
# of a DB created by an older pkm (missing e.g. `degraded`) yields NULL
360+
# for the absent column rather than crashing every read command.
361+
sel = ", ".join(self._col(c) for c in (
362+
"name", "version", "release", "tier", "description",
363+
"install_reason", "degraded"))
331364
if tier:
332365
rows = self.conn.execute(
333-
"SELECT name, version, release, tier, description, "
334-
"install_reason, degraded "
335-
"FROM installed WHERE tier = ? ORDER BY name",
366+
f"SELECT {sel} FROM installed WHERE tier = ? ORDER BY name",
336367
(tier,)
337368
).fetchall()
338369
else:
339370
rows = self.conn.execute(
340-
"SELECT name, version, release, tier, description, "
341-
"install_reason, degraded "
342-
"FROM installed ORDER BY name"
371+
f"SELECT {sel} FROM installed ORDER BY name"
343372
).fetchall()
344373
return [
345374
{"name": r[0], "version": r[1], "release": r[2], "tier": r[3],

tests/pkm/audit/findings.yaml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,3 +436,18 @@ findings:
436436
regression_test: "tests/pkm/test_pkm_a29_a30_guidance_cosmetics.py"
437437
evidence: "cmd_info header version omits release + hardcoded 50-char rule; 'Required by' lines show bare reverse-dependent version (misreadable); verify --all PROBLEM line + history line overflow narrow terminals."
438438
fix: "cmd_info shows the full version-release identity (_vr_str) and sizes the '=' rule to the title; get_reverse_depends now carries release so 'Required by' shows version-release (no fabricated default). verify/history line lengths are bounded by existing per-list caps (... and N more) — acceptable, not changed."
439+
440+
# ---------------- FOUND BY REAL-INSTALL TESTING (post-audit) --------------
441+
- id: PKM-A31
442+
title: "Read-only commands crash on a behind-schema DB: list_installed queries `degraded` but read-only opens skip the ALTER-TABLE migration"
443+
module: database.py
444+
locations: ["pkm/database.py:190-199", "pkm/database.py:339-349", "pkm/database.py:252-263"]
445+
lens: correctness
446+
severity: high
447+
tier: trust
448+
status: verified
449+
owner: maintainer
450+
batch: 5
451+
regression_test: "tests/pkm/test_pkm_a31_schema_tolerant.py"
452+
evidence: "Found 2026-06-16 testing pkm 0.2.0 on .227. A25 added the `degraded` column with _migrate_a25_degraded_column (ALTER TABLE), but migrations run ONLY on a read-write open; the read-only (immutable=1) open used by non-root readers — and by root via a read-only subcommand — returns before migrating. On a system upgraded from a pre-A25 pkm, every read command hitting list_installed (list/the update-summary/etc.) crashed with `sqlite3.OperationalError: no such column: degraded` until some later root command happened to run the migration. The pkm unit suite never caught it because it always builds a fresh, fully-migrated DB — only a real upgrade path exposes it."
453+
fix: "list_installed selects schema-tolerantly via a new _col() helper: columns are introspected once at open (_table_columns -> self._installed_cols, on BOTH open paths), and a column absent on an un-migrated read-only DB is emitted as `NULL AS <col>` instead of being named directly — so reads degrade gracefully (degraded -> None = healthy) regardless of migration timing. Pattern to reuse for any future migration-added column read on the read-only path. test_pkm_a31_schema_tolerant.py: old-schema DB (no degraded) read-only open + list_installed no-crash + tier filter + full-schema round-trip."
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
#!/usr/bin/env python3
2+
# SPDX-License-Identifier: GPL-3.0-or-later
3+
"""PKM-A31 regression: read-only commands tolerate a behind-schema DB.
4+
5+
Found by real-install testing (2026-06-16, .227): pkm 0.2.0 added the `degraded`
6+
column (A25) with an ALTER-TABLE migration, but migrations only run on a
7+
read-WRITE open. A read-only command (`pkm list`, non-root — or root via a
8+
read-only subcommand) opens the DB immutable and CANNOT migrate it, so on a
9+
system upgraded from a pre-A25 pkm `list_installed` crashed with
10+
`sqlite3.OperationalError: no such column: degraded` until some later root
11+
command happened to run the migration. That is the ugly-crash class the audit
12+
targets.
13+
14+
Fix: list_installed (and the read path generally) selects schema-tolerantly via
15+
_col() — a column absent on an un-migrated DB yields NULL instead of raising.
16+
"""
17+
18+
import sqlite3
19+
import tempfile
20+
import unittest
21+
from pathlib import Path
22+
23+
from pkm.database import PackageDB
24+
25+
26+
class SchemaTolerantReadTest(unittest.TestCase):
27+
def _old_schema_db(self, path):
28+
"""A minimal `installed` table as an OLDER pkm would have created it —
29+
WITHOUT the `degraded` column (and without other newer columns)."""
30+
c = sqlite3.connect(str(path))
31+
c.execute(
32+
"CREATE TABLE installed ("
33+
"id INTEGER PRIMARY KEY, name TEXT, version TEXT, "
34+
"release INTEGER DEFAULT 1, tier TEXT, description TEXT, "
35+
"install_reason TEXT DEFAULT 'manual')"
36+
)
37+
c.execute(
38+
"INSERT INTO installed (name, version, release, tier, description) "
39+
"VALUES ('foo', '1.0', 2, 'core', 'a package')"
40+
)
41+
c.commit()
42+
c.close()
43+
44+
def test_list_installed_tolerates_missing_degraded(self):
45+
with tempfile.TemporaryDirectory() as td:
46+
dbp = Path(td) / "pkm.db"
47+
self._old_schema_db(dbp)
48+
# Read-only open: cannot run the migration → `degraded` stays absent.
49+
db = PackageDB(dbp, read_only=True)
50+
try:
51+
rows = db.list_installed() # must NOT raise
52+
finally:
53+
db.close()
54+
self.assertEqual(len(rows), 1)
55+
self.assertEqual(rows[0]["name"], "foo")
56+
self.assertEqual(rows[0]["release"], 2)
57+
self.assertIsNone(rows[0]["degraded"]) # absent → NULL → healthy
58+
59+
def test_tier_filter_also_tolerates_missing_degraded(self):
60+
with tempfile.TemporaryDirectory() as td:
61+
dbp = Path(td) / "pkm.db"
62+
self._old_schema_db(dbp)
63+
db = PackageDB(dbp, read_only=True)
64+
try:
65+
rows = db.list_installed(tier="core")
66+
finally:
67+
db.close()
68+
self.assertEqual(len(rows), 1)
69+
self.assertIsNone(rows[0]["degraded"])
70+
71+
def test_full_schema_db_still_reports_degraded(self):
72+
# A normal (read-write, freshly created) DB has the full schema +
73+
# migrations, so `degraded` is a real column and round-trips.
74+
with tempfile.TemporaryDirectory() as td:
75+
dbp = Path(td) / "pkm.db"
76+
db = PackageDB(dbp, create_if_missing=True,
77+
root=str(Path(td) / "root"))
78+
try:
79+
db.add_installed("bar", "2.0", release=1, tier="core")
80+
db.mark_degraded("bar", "hook-x failed")
81+
rows = db.list_installed()
82+
finally:
83+
db.close()
84+
row = next(r for r in rows if r["name"] == "bar")
85+
self.assertEqual(row["degraded"], "hook-x failed")
86+
87+
88+
if __name__ == "__main__":
89+
unittest.main()

0 commit comments

Comments
 (0)