Repository navigation
Runtime, database, calculations, and CI foundation - #2
Conversation
|
Stack audit resolution (latest head
|
ahernsean
left a comment
There was a problem hiding this comment.
Review — Phase 1 foundation
Reviewed against DESIGN.md and ran the suite locally (Python 3.13.12, locked deps). Findings below are the adverse ones only; the migration/checksum/integrity machinery and the durability pragmas look solid.
1. validate_interval is dead code, so its invariants are never enforced (driving_log/validation.py:19)
validate_interval is imported by nothing except tests/test_foundation.py. Every real write path goes through records._drive_values, which re-derives duration with round(seconds / 60) and never checks minute alignment. Consequences:
- The "drive timestamps must be minute-aligned" rule is not enforced anywhere in production. A CSV import carrying
2026-07-20T16:00:00.123456Zis accepted and stored verbatim. long_drive/crosses_midnightare implemented twice — here and inRecordService.warnings_for— and only the second copy runs.
Worth either wiring this into RecordService.create/update or deleting it so there is one implementation.
Related, in the same function: started_at_utc.astimezone().date() != ended_at_utc.astimezone().date() (validation.py:30) uses the system timezone, not the drive's. warnings_for correctly uses ZoneInfo(drive["timezone_name"]). On a host whose clock is UTC (which is how the CI containers run), this version would classify a 7:30 PM–8:30 PM EDT drive as crossing midnight. It is latent only because nothing calls it.
2. Pre-migration safety archive ignores the configured archive directory (driving_log/db.py:74-83)
create_archive(self, self.path.parent / "archives", self.path.parent / "archives" / f"...")Settings.archive_dir is independently configurable via DRIVING_LOG_ARCHIVE_DIR. If an operator sets it, the pre-upgrade backup — the single most important archive the system ever takes — is written to a directory that is not listed on /archives, not replicated by archive replicate, and not covered by retention. It should take the configured archive dir.
This becomes live for the first time with the schema v2 migration in #6, so it is worth fixing before that lands.
3. The four-job CI gate has never executed
There are zero check runs on this PR and on every other PR in the stack, so none of the gates described here have actually been verified on GitHub. Locally, pytest -m "not browser" gives 2 failed, 60 passed on a host without Tailscale:
FAILED tests/test_foundation.py::ApplicationTests::test_cli_doctor_json
FAILED tests/test_foundation.py::ApplicationTests::test_health_endpoints_and_doctor
E FileNotFoundError: [Errno 2] No such file or directory: 'tailscale'
The root cause arrives in #5 (doctor shells out to tailscale/systemctl with no fallback) and I've written it up there, but it means the gate defined in this PR cannot go green on a GitHub runner as written. Suggest running the workflow once on this branch before merging anything in the stack.
4. Minor — migration SQL is assembled by string interpolation (driving_log/db.py:161-178)
name, checksum and applied_at are quote-escaped by hand and spliced into an executescript body. The values are all internal, so this is not exploitable, but the DDL and the bookkeeping INSERT could be split so the latter can use bound parameters.
5. Minor — -wal/-shm keep umask permissions (driving_log/db.py:96-97)
os.chmod(self.path, 0o600) runs only when the database file is newly created, and only for the main file. The sidecar files are created by SQLite at 0644 & ~umask. Containment comes entirely from the 0700 state directory; the systemd units set UMask=0077, but a CLI invocation from an interactive shell will not.
Generated by Claude Code
|
Review resolution at
No merge performed. |
Phase 1 of the supervised driving log stack.
Implements the Python 3.13 FastAPI package, hash-locked dependencies, CLI, SQLite 3.34-compatible transactional schema migrations, production durability pragmas, integrity/schema checks, Apex solar and DST calculations, validation, health/doctor endpoints, structured logging, private seed ignores, and four-job CI.
Local gate: Ruff, format, strict mypy, dependency consistency, tests with coverage, and git diff check.