v1.0 tag-cut: ADR-035 + STO-02 (application_id, user_version) + STO-04 e2e integrity (Day 2)#20
Conversation
Background — `docs/arch-analysis-2026-05-22-1924/` is a from-scratch
architecture review whose §8 surfaced five open questions that a follow-on
SME roundtable reframed as a single missing-feedback-loop pattern.
ADR-035 is the level-5 (rules) intervention that pattern requires.
This commit lands both the ADR and the first wave of code changes that
ADR-035 commits the project to:
- **ADR-035** (`docs/clarion/adr/ADR-035-operational-tuning-discipline.md`)
Accepted on 2026-05-23. Extends ADR-021 §4. Every operational constant
in Clarion source MUST declare four things — stated basis, override
surface, retune trigger, coupling — in a code comment immediately
adjacent to the constant. Plus file-LOC budgets (1500 LOC trigger
declares a split-trigger condition) and crate-boundary budgets.
- **STO-02 application_id (closes gap-register STO-02)**
`crates/clarion-storage/src/pragma.rs` sets `application_id =
0x434C524E` ("CLRN") lazily on first open of a fresh file; refuses
to open any database with a different non-zero `application_id`.
New `StorageError::ForeignDatabase` variant carries the offending
value into the error envelope.
- **STO-02 user_version forward-incompatibility refusal**
`crates/clarion-storage/src/schema.rs` declares `CURRENT_SCHEMA_VERSION
= 1` with a compile-time assertion that it matches the highest
`MIGRATIONS[]` entry. `verify_user_version()` refuses to operate on a
database whose `user_version` is strictly greater. `Writer::spawn`
invokes `verify_user_version()` directly so a forward-incompat file
is rejected before the writer-actor opens for business. New
`StorageError::FutureUserVersion` carries `found` and `current` into
the envelope.
- **HTTP error envelope for the new variants**
`crates/clarion-cli/src/http_read.rs::classify_read_error` returns
500 `STORAGE_ERROR` with a header-rejection message for
`ForeignDatabase` / `FutureUserVersion`. The HTTP API does not open
its own writer, but the reader pool can encounter the same file
header mismatches and the response code should be distinct.
- **`PyrightRunState` for shared restart budget**
`plugins/python/src/clarion_plugin_python/pyright_session.py` adds
a `PyrightRunState` dataclass shared across session recycles. Without
it, the 3-restart cap reset at every 25-file recycle, letting a
crash-looping pyright silently consume `ceil(N/25) * 3` restarts
instead of the documented 3 for an entire run. `server.py`
constructs one `PyrightRunState` per `ServerState` and passes it
into every new `PyrightSession`.
- **`analyze_failure_modes.rs` integration test**
Closes the test-discipline interlock that ADR-035's
systems-thinker analysis named: exercises the `RunOutcome::HardFailed`
branch in `analyze.rs::run_with_options` where the writer-actor
rejects an `InsertEdge` mid-run, distinguishing the failure surface
from `SoftFailed` (which carries the full stats blob).
- **`schema_apply.rs` discipline tests**
Cover the new application_id + user_version surface: foreign-DB
refusal, future-DB refusal, version-bump from zero, idempotent
re-open.
Verification: `cargo build`, `cargo clippy -D warnings`, `cargo nextest`
(509 passed), `cargo doc -D warnings`, `cargo deny`, `ruff`,
`ruff format --check`, `mypy --strict`, `pytest` (143 passed).
Closes gap-register STO-02 (`clarion-f2a984fd6d`).
Forward-references gap-register STO-04 (e2e integrity_check)
and the v1.1 backlog of constants needing declared bases.
Walking-skeleton e2e now closes gap-register STO-04: every run must exit cleanly against `PRAGMA integrity_check`. A v1.0-shipped binary cannot leave a corrupt `.clarion/clarion.db` on the operator's disk without a CI signal. The backup procedure half of STO-04 (operator-facing guidance on shutdown → wal_checkpoint(TRUNCATE) → file copy) shipped in PR 3 (`docs/clarion/1.0/operations.md`); this commit lands the regression test that catches a corruption regression. Closes gap-register STO-04 (`clarion-ee22d1d72c`).
There was a problem hiding this comment.
Pull request overview
This PR strengthens “Day 2” operational safety around SQLite persistence and the Python plugin’s Pyright lifecycle, and tightens CI/e2e coverage to catch corruption and failure-mode regressions early.
Changes:
- Add SQLite file identity + forward-compat guards (
application_id,user_version) and propagate newStorageErrorvariants through writer open, migrations, and HTTP error classification. - Fix Python plugin Pyright restart-budget semantics by introducing a run-wide
PyrightRunStateshared across session recycles, with new pytest coverage. - Add e2e
PRAGMA integrity_checkassertion and introduce/extend integration tests for writer-open discipline and analyze failure-mode promotion; add accompanying architecture-analysis documentation artifacts.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/sprint_1_walking_skeleton.sh | Adds final PRAGMA integrity_check assertion to catch DB corruption in CI. |
| plugins/python/tests/test_server.py | Updates fakes for new PyrightSession ctor kwargs; adds tests ensuring restart/disabled budgets survive recycle boundaries. |
| plugins/python/src/clarion_plugin_python/server.py | Stores run-wide Pyright state in ServerState and passes it into PyrightSession. |
| plugins/python/src/clarion_plugin_python/pyright_session.py | Introduces PyrightRunState and moves restart/disabled tracking to shared run state. |
| crates/clarion-storage/tests/schema_apply.rs | Adds open-time contract tests for foreign application_id, future user_version, and stamping legacy DBs. |
| crates/clarion-storage/src/writer.rs | Ensures writer refuses forward-incompatible DBs by calling schema::verify_user_version() at spawn. |
| crates/clarion-storage/src/schema.rs | Introduces CURRENT_SCHEMA_VERSION, compile-time migration-version check, verify_user_version, and applies user_version after migrations. |
| crates/clarion-storage/src/pragma.rs | Adds Clarion application_id constant and enforces/stamps it on writer open. |
| crates/clarion-storage/src/error.rs | Adds StorageError::{ForeignDatabase, FutureUserVersion} with stable, subcoded error messages. |
| crates/clarion-cli/tests/analyze_failure_modes.rs | Adds integration test ensuring writer-actor mid-run failure promotes to HardFailed and writes minimal FailRun stats. |
| crates/clarion-cli/src/http_read.rs | Adds HTTP error classification arms for new storage errors; minor formatting changes. |
| docs/arch-analysis-2026-05-22-1924/temp/validation-final-report.md | Adds validator report artifact for the final report. |
| docs/arch-analysis-2026-05-22-1924/temp/validation-catalog.md | Adds validator report artifact for the subsystem catalog. |
| docs/arch-analysis-2026-05-22-1924/temp/task-discovery.md | Adds discovery task spec used to generate the analysis docs. |
| docs/arch-analysis-2026-05-22-1924/temp/task-catalog-template.md | Adds subsystem catalog contract/template used in the analysis. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-python-plugin.md | Adds generated subsystem catalog entry for the Python plugin. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-clarion-storage.md | Adds generated subsystem catalog entry for clarion-storage. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-clarion-scanner.md | Adds generated subsystem catalog entry for clarion-scanner. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-clarion-plugin-fixture.md | Adds generated subsystem catalog entry for the fixture plugin. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-clarion-mcp.md | Adds generated subsystem catalog entry for clarion-mcp. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-clarion-core.md | Adds generated subsystem catalog entry for clarion-core. |
| docs/arch-analysis-2026-05-22-1924/temp/catalog-clarion-cli.md | Adds generated subsystem catalog entry for clarion-cli. |
| docs/arch-analysis-2026-05-22-1924/temp/answer-systems-thinker.md | Adds analysis “open questions” synthesis artifact. |
| docs/arch-analysis-2026-05-22-1924/temp/answer-solution-architect.md | Adds analysis “architect answers” artifact. |
| docs/arch-analysis-2026-05-22-1924/temp/answer-security-engineer.md | Adds analysis security-focused artifact. |
| docs/arch-analysis-2026-05-22-1924/temp/answer-quality-engineer.md | Adds analysis quality-focused artifact. |
| docs/arch-analysis-2026-05-22-1924/temp/answer-python-engineer.md | Adds analysis Python-focused artifact. |
| docs/arch-analysis-2026-05-22-1924/04-final-report.md | Adds the final architecture-analysis report. |
| docs/arch-analysis-2026-05-22-1924/03-diagrams.md | Adds architecture diagrams (C4 + sequences + dependency graph). |
| docs/arch-analysis-2026-05-22-1924/00-coordination.md | Adds the analysis coordination plan. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -21,6 +56,7 @@ pub fn apply_write_pragmas(conn: &Connection) -> Result<()> { | |||
| ADR-011's synchronous=NORMAL durability posture requires WAL" | |||
| ))); | |||
| } | |||
| enforce_application_id(conn)?; | |||
| conn.execute_batch(concat!( | |||
| // STO-02 (ADR-035): the on-disk file is not a Clarion database, or | ||
| // was written by a newer build. Either condition is fatal for the | ||
| // server; the writer-actor refuses to spawn against it. Surfacing | ||
| // 500 here is defensive — in practice the HTTP API does not open | ||
| // its own writer, but the reader pool can encounter the same file | ||
| // header mismatches and we want a clear distinct response code. | ||
| StorageError::ForeignDatabase { .. } | StorageError::FutureUserVersion { .. } => { | ||
| ReadError { | ||
| status: StatusCode::INTERNAL_SERVER_ERROR, | ||
| code: ErrorCode::StorageError, | ||
| message: "file lookup storage rejected database header", | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9760ffe2a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pragma::apply_write_pragmas(&conn)?; | ||
| // STO-02: refuse a database whose `user_version` is strictly greater | ||
| // than CURRENT_SCHEMA_VERSION. Equal/less are normal — equal is the | ||
| // already-migrated steady state, less is handled by the migration | ||
| // runner (which `install` calls before the writer ever spawns). |
There was a problem hiding this comment.
Writer::spawn calls apply_write_pragmas before the forward-compatibility guard, but apply_write_pragmas can write PRAGMA application_id when it is 0. If a non-Clarion SQLite file has application_id=0 and a higher user_version, this path will first stamp it as Clarion (and change write pragmas) and only then return FutureUserVersion, so a rejected open still mutates the file. Run the version/header compatibility checks before any mutating PRAGMA writes, or split read-only validation from write-time tuning.
Useful? React with 👍 / 👎.
| @@ -43,3 +79,31 @@ pub fn apply_read_pragmas(conn: &Connection) -> Result<()> { | |||
| ))?; | |||
There was a problem hiding this comment.
The STO-02 guards are only applied on write connections; apply_read_pragmas never validates application_id or user_version. In clarion serve, the HTTP read API uses ReaderPool::with_reader (read-only path), so a foreign or forward-incompatible database can still be opened and queried instead of being rejected with the new ForeignDatabase/FutureUserVersion errors. This leaves the read surface inconsistent with the new storage-compatibility contract and makes the new HTTP classification branch effectively unreachable.
Useful? React with 👍 / 👎.
| ADR-011's synchronous=NORMAL durability posture requires WAL" | ||
| ))); | ||
| } | ||
| enforce_application_id(conn)?; |
There was a problem hiding this comment.
apply_write_pragmas executes PRAGMA journal_mode = WAL before checking whether the file is a Clarion database. For a foreign SQLite file with a non-Clarion application_id, this function returns ForeignDatabase but only after attempting to change journal mode (and potentially creating WAL side effects), so a database that should be rejected still gets mutated. Perform the application_id gate first, then apply mutating PRAGMAs only for accepted Clarion files.
Useful? React with 👍 / 👎.
|
Superseded by the Day-2 consolidation onto |
Summary
Foundational Day 2 storage work. Two commits.
Commit 1 — ADR-035 + STO-02
ADR-035 (Accepted 2026-05-23,
docs/clarion/adr/ADR-035-operational-tuning-discipline.md) is a level-5 (rules) Meadows intervention. From-scratch architecture review on 2026-05-22 (docs/arch-analysis-2026-05-22-1924/) surfaced five open questions in §8 that a follow-on SME roundtable reframed as a single missing-feedback-loop pattern. Every operational constant in Clarion source MUST declare four things — stated basis, override surface, retune trigger, coupling — adjacent to the constant. Plus file-LOC budgets (1500 LOC trigger declares a split-trigger condition) and crate-boundary budgets.STO-02 (gap-register) is the first wave of code changes ADR-035 commits the project to:
crates/clarion-storage/src/pragma.rs—application_id = 0x434C524E("CLRN") set lazily on first open of a fresh file; non-zero foreign IDs refused withStorageError::ForeignDatabase.crates/clarion-storage/src/schema.rs—CURRENT_SCHEMA_VERSION = 1with compile-time assertion againstMIGRATIONS[]last entry;verify_user_version()refuses to operate on a database whoseuser_versionis strictly greater than the build's known max.crates/clarion-storage/src/writer.rs—Writer::spawninvokesverify_user_version()so forward-incompat files are rejected before the writer-actor opens for business.crates/clarion-cli/src/http_read.rs::classify_read_error— 500STORAGE_ERRORfor the two new variants.plugins/python/.../pyright_session.py— newPyrightRunStatedataclass shared across session recycles, closing a latent bug where the 3-restart cap reset every 25-file recycle.analyze_failure_modes.rsintegration test forRunOutcome::HardFailedvsSoftFailed.schema_apply.rstests for the application_id + user_version discipline.Commit 2 — STO-04 e2e integrity_check
tests/e2e/sprint_1_walking_skeleton.shnow assertsPRAGMA integrity_check == 'ok'at the end of the walking-skeleton run. A v1.0-shipped binary cannot leave a corrupt.clarion/clarion.dbon the operator's disk without a CI signal.The backup-procedure half of STO-04 (operator-facing guidance: shutdown →
wal_checkpoint(TRUNCATE)→ file copy) shipped in PR #15 (docs/clarion/1.0/operations.md).Closes (filigree)
clarion-f2a984fd6d— STO-02 application_id PRAGMA + user_version forward-incompatclarion-ee22d1d72c— STO-04 backup + integrity_check (both halves now closed)Forward-references the v1.1 backlog of operational constants needing declared bases (
V11-STO-*group + ADR-035's lint-script enforcement).Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo build --workspace --binscargo nextest run --workspace --all-features— 509 passed, 2 skipped (+1 new test inanalyze_failure_modes.rs, +N inschema_apply.rs)RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-featurescargo deny checkruff check plugins/pythonruff format --check plugins/pythonmypy --strict plugins/pythonpytest plugins/python— 143 passedbash tests/e2e/sprint_1_walking_skeleton.sh(now includes the integrity_check final assertion)Conflict note
PR #19 (
v1.0/06-storage-cross-process-lock) and this PR both touchcrates/clarion-cli/src/http_read.rsfor orthogonal reasons (this PR adds the new match arms forForeignDatabase/FutureUserVersion; PR #19 includes rustfmt whitespace fixes). The overlap is harmless whitespace; first-merger wins, second auto-rebases.🤖 Generated with Claude Code