35 lines
6.8 KiB
Markdown
35 lines
6.8 KiB
Markdown
# Independent REST/persistence review — 2026-09-11
|
||
|
||
Reviewer: Codex-ytr83r /root/server_audit. Reviewed checkout `ff615d0ee027997ea43d6d3bf95eabf946828561`; root is independently importing later authoring snapshots. Ownership: this report only. No production changes, Cargo/build commands, native input, live service, or filesystem fault injection. Coordination: Gitea issue #1 through #146, ownership #147, open PR inventory empty, Wiki Home and Authoring20260911.
|
||
|
||
## Verdict and ranked findings
|
||
|
||
**No concrete P0/P1/P2 implementation defect found** in the reviewed server persistence, conditional writes, source/model adoption, or restart admission paths. This is source review and limited static contract checking, not runtime release acceptance.
|
||
|
||
1. **P3 baseline documentation inconsistency — resolved by incoming integration.** At reviewed `ff615d0`, `docs/server.md:3` calls the shared model schema 5. Actual `crates/oa-core/src/model.rs:9` fixes `SCHEMA_VERSION` to 7, `docs/api.rst:159` says exports use schema/source 7, and OpenAPI `Project.properties.schema_version.const` is 7. Root confirmed incoming `c6d296a` already corrects this label to schema 7. No new edit is needed; this is a resolved baseline finding, not an outstanding defect in the integrated tree or a missing server feature.
|
||
|
||
No speculative cleanup findings are included. The gaps below concern verification, not verified product defects.
|
||
|
||
## Verified facts and falsifiers
|
||
|
||
| Guarantee traced | Evidence | Meaningful falsifier / existing test |
|
||
| --- | --- | --- |
|
||
| Final ETag comparison is serialized with commit, including no-op candidates | `crates/oa-server/src/store.rs:447` compares before `prepared.changed`; snapshot preparation at `store.rs:249` is outside the commit lock | Two candidates prepared from one ETag: exactly one changed commit, loser 412, only winner survives restart. Existing deterministic store test and `crates/oa-server/src/preparation_http_tests.rs:12` force the actual preparation interleaving. |
|
||
| Pre-install write failure cannot adopt candidate memory | `store.rs:469` calls write before adoption at 470; temporary write/sync/persist at 510–515 | Force destination replacement failure in isolated store; assert unchanged ETag, complete model and source. Existing real HTTP tests at `tests/http.rs:1514` and `tests/http/source_projects.rs:167` do this; latter restores original disk bytes and checks restart/temp cleanup. |
|
||
| Model and authored source are one atomic resource | `store.rs:144` uses command identity correspondence; replacement at 164 uses UUID correspondence; envelope serialization writes both; `PreparedProject::changed` compares both | Comment-only edit must advance ETag; stale semantic writer must lose. Existing `tests/http/source_projects.rs:684`; source no-op/restart at 92; identity swaps/dry-run at 262 and equal-model changed provenance at 359. |
|
||
| Startup refuses inconsistent or unstabilized current-format pairs | `store.rs:270` opens/locks; regular-file check at 296, bounded read at 316, independent source/model comparison around 345–354 | Corrupt JSON, zero revision, filename/ID mismatch, absent persistent source IDs, model/source disagreement and second owner must fail without rewrite. Existing `tests/http.rs:1543` and `tests/http/source_projects.rs:736`. |
|
||
| Post-install Unix sync failure has an uncertain response, while installed state and memory agree | `store.rs:470` adopts before directory sync at 471; `store.rs:521` documents that boundary; `error.rs:136` tells clients to reload | Inject only the post-install directory-sync failure: response 503, GET returns new complete pair/revision, restart returns same pair, stale original ETag refuses. No deterministic test for this specific failure was found. Ordinary replacement-failure tests do not cover it. |
|
||
| HTTP timeout cannot release the worker permit while its blocking task continues | `crates/oa-server/src/lib.rs:114` has request timeout; `blocking` at 190 captures owned worker permit inside closure | Hold admitted work beyond deadline, observe 504, then release work: one complete outcome, no duplicate revision, stale ETag refuses, permits eventually recover. The existing preparation gate could support a focused test; no deadline-crossing test was found. This is an unexecuted falsifier, not a reported bug. |
|
||
| Source7 nested identities and operations have service-level parity tests | `tests/http/source7_contracts.rs:189` covers authored bundle, typed edit, invalid/stale rejection and restart | Run the test against the integrated source; preserve full model, source bytes and identities through restart. Merely compiling does not establish this. |
|
||
|
||
Executed static checks: PowerShell parsed checked-in OpenAPI; **all 24 router paths exactly match its path inventory**; **all 489 local schema references resolve**. Confirmed OpenAPI current schema constant 7. These checks do not validate every command shape or execute the HTTP handlers.
|
||
|
||
## Remaining release evidence, in priority order
|
||
|
||
1. Root's serialized exact-source gates: `cargo test --locked -p oa-server`, `cargo clippy --locked -p oa-server --all-targets -- -D warnings`, and formatting. Prior Gitea/Wiki results belong to other explicitly identified checkpoints. This reviewer claims no test pass for this checkout or the eventual integrated checkout.
|
||
2. Run `python crates/oa-server/tests/semantic_schema.py --check --validate` with jsonschema available, then `python crates/oa-server/tests/semantic_http_contract.py ABSOLUTE_PATH_TO_MATCHING_SERVER_BINARY`. The latter compares served and checked-in OpenAPI and validates actual responses against both. No Python command was available through this review shell's PATH lookup, so these were not executed here.
|
||
3. Fresh native Linux current-source suite and optimized binary smoke: temporary local store; token/auth, exclusive ownership, conditional write and restart, paired source/model exports, delete and restart, SIGTERM drain. `docs/server.md:147` expressly limits prior Linux evidence to the old V2 baseline. Container build/run remains separate when Docker deployment is claimed (`docs/server.md:149`).
|
||
4. Add or run bounded controlled-failure probes for the two documented but uncovered boundaries above: HTTP cancellation/deadline after admission, and Unix directory-sync failure after installation. Do not weaken assertions to pre-install rollback after a post-install fault. Both must assert complete-pair coherence and honest unknown outcome; power-loss survival is explicitly outside the documented guarantee.
|
||
|
||
The documented exclusions are coherent with implementation: one local store owner; no out-of-band file synchronization; current revision only; no server undo, idempotency receipt or automatic retry of uncertain create/import. No evidence here justifies inventing those capabilities or marking their deliberate absence a defect. Root retains integration, any repairs, runtime verification and final release verdict.
|