Files
mvalentsev 32636ddeaa fix(mcp): report an absent integrity verdict instead of a clean one (#2290)
sqlite_integrity_errors answers [] when chroma.sqlite3 is not there, and the
MCP startup gate published that as checked: true, ok: true. A clean bill of
health and an unopened database were the same answer, and /statusz built its
health flag from it.

os.path.exists cannot separate the two: it is genericpath.exists, which calls
os.stat inside a try whose except folds every OSError and ValueError into
False, and os.stat follows symlinks. ENOENT from lstat is now the only answer
accepted as proof of absence; everything else falls through to the open
attempt, where SQLite reports it. An absent database gets the not-applicable
shape #1931 introduced, checked: false / ok: null plus a reason.

ENOENT does not say which path component was missing, so it proves something
about the path and not about the palace: a palace directory that is itself a
dangling symlink, and an unmounted mount point, both report no verdict rather
than a failed probe. That is the answer develop gives too, without develop's
claim that the check passed.

Every state that is not proven absent reaches the probe. Measured against
develop, six palace states out of nine answered checked: true, ok: true: an
absent database, a palace directory that does not exist, an intact database
under a mode-000 directory, a dangling symlink, a symlink loop, and a path
whose parent is a file. The last four now report PRAGMA quick_check failed and
so trip the existing -32002 refusal, as do an over-long name and an embedded
NUL. A database file at mode 000 already reported ok: false, because
os.path.exists is true there and the probe ran.

The absence question is asked once per call. Reaching the probe through
sqlite_integrity_errors would ask it again, and a database unlinked between
the two would answer [] the second time, which is the clean verdict this
change exists to withhold. The quick_check body moved to _quick_check_errors,
which has no absence gate, and both callers use it.

That probe answers rather than raises. A palace directory named with a byte
that is not valid UTF-8 reaches it, and sqlite_read_uri fails on the name
before SQLite is opened. Measured on 3.9, 3.11, 3.12, 3.13 and 3.14: the raise
happens up to 3.12, and 3.13 percent-encodes the byte instead, so the database
opens and the verdict is a real one. UnicodeEncodeError is a ValueError
subclass, which the absence gate's docstring says callers never see, and
palace.py's post-mine validator and cli.py's repair preflight both call this
unguarded, so on the 3.9 and 3.11 that CI covers it is a traceback out of
mempalace mine and mempalace repair on a database that is sitting right there.
It is reported like every other unreadable path now. Its test asserts the
answer on every version and the reporting only where the raise is reachable.

The connection is wrapped in closing(), as at the other two quick_check sites
in this module: sqlite3's context manager ends the transaction and leaves the
handle open, so the probe leaked one descriptor per call until the cyclic
collector ran.

/statusz reads an absent verdict as healthy rather than unhealthy. develop
already answered ok: true for a palace with no database, through the clean
verdict this removes, so the change is what keeps the new ok: null from
turning a fresh install red; the visible flip is the non-chroma backends,
unhealthy since the #1931 fix. The default in integrity.get("ok", False) keeps
a payload with no ok key failing closed.

SqliteIntegrityStatus carries its errors as a tuple. With a list field the
generated __hash__ raised TypeError on every call and a caller could append to
a verdict it had been handed, so frozen=True described nothing.

Windows raises ENOENT where POSIX raises ENOTDIR for a file used as a
directory component, so there the palace path is proven absent and reported as
no verdict rather than as a failed probe. Both answers are safe; the
difference belongs to the platform, not to the gate. The ENOTDIR test is
scoped to POSIX for that reason, as is the one for a name POSIX allows and
Windows does not.

The refresh clears the no-verdict reason on every one of its five exits,
including the two that return before any probe runs, so a server pointed at a
new palace cannot keep naming the old one's database.

The payload still reads those globals one at a time without the lock, and the
comments there no longer claim otherwise: a refresh landing between two reads
can publish ok: true for a palace with no database, and snapshotting into
locals moves that window rather than closing it. Holding the refresh lock
across the payload would close it, at the cost of serialising every status
read behind an O(database size) probe; publishing the verdict as one value
closes it without that, and is the change worth making.
2026-08-22 23:32:58 +05:00
..