mirror of
https://github.com/jhd3197/ServerKit.git
synced 2026-10-30 00:27:11 +09:00
Juan DenisandClaude Opus 5
89481b15c0
Plan 75 §G1 — the door and the ratchet. The 33-file migration is NOT
done; see below.
run_checked(cmd, *, privileged=False, timeout=60, input=None). Around
backend/app's raw subprocess calls sit 220 copies of capture_output/text,
56 hand-rolled TimeoutExpired handlers, 19 FileNotFoundError handlers and
1,164 literal {'success': False, 'error': ...} dicts. Each is a place an
exec failure becomes a false fact.
It keeps the distinction those handlers lose: returncode is None when the
command never ran. Exit 1 means the command answered "no"; no exit code
at all means nobody answered. A caller rendering "not installed" can
finally tell them apart — plan 74's outage in one field. Unprivileged
calls get the same argv[0] resolution as privileged ones, because the
sbin outage is not something that only happens when you ask for root.
The ratchet is tests/raw_subprocess_census.py + RAW_SUBPROCESS_CEILING,
wired into the existing "Audit Raw Subprocess Patterns" CI job and
enforced by test_raw_subprocess_ratchet.py. The grep steps already there
check eight hand-listed files for three known-bad patterns, so a new file
was invisible to the audit entirely; the census counts every call site
under backend/app. It is AST-based because a grep for subprocess.run also
matches comments, docstrings and `except subprocess.SubprocessError`, and
a miscounting ratchet reads green either way. test_the_ceiling_is_not_stale
fails if the ceiling drifts more than 10 above reality — a stale ceiling
silently re-authorises everything a migration removed.
First migration: cron_service, 13 raw calls -> 3. Its `crontab -l` read
and Popen-with-stdin `crontab -` install were written out five times each
and are now two helpers over run_checked. Census 267 -> 258.
The other 32 files are untouched: docker_service (49), database_service
(24), git_service (18), security_service (14), python_service (13) are
the bulk. The door plus the ratchet are what make that incremental
instead of one unreviewable diff.
test_utils_system.py loads system.py under its own importlib spec, so
@patch('app.utils.system.<name>') binds to whichever module object is in
sys.modules at patch time — not necessarily the one run_checked closes
over. shutil/subprocess are singletons so the file's existing patches are
safe; these are the first tests to patch a module-level function of
system.py, and they use patch.object(_module, ...) instead.
BASELINE_COUNT 3383 -> 4081. Most of that is pre-existing drift (the
ratchet only fails on a drop, so an un-bumped floor never complains), not
tests added here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>