v1.0 tag-cut: operator rollback + storage ops + trust statements (PR 3/4)#15
Closed
tachyon-beep wants to merge 1 commit into
Closed
v1.0 tag-cut: operator rollback + storage ops + trust statements (PR 3/4)#15tachyon-beep wants to merge 1 commit into
tachyon-beep wants to merge 1 commit into
Conversation
…OG drift fixes Operator-facing surface for v1.0: - v1.0-release-rollback.md (new): h+30min "bad release shipped" runbook covering `gh release edit --prerelease`, asset-handling policy (don't delete unless sensitive), Sigstore Rekor non-revocability, v1.0.1 supersession publication, downstream notification, postmortem. - operations.md (new): local-first .clarion/ storage constraints. NFS prohibition; no concurrent `clarion analyze` (enforced post-STO-01); supported backup procedure (shutdown -> wal_checkpoint(TRUNCATE) -> file copy); restore path; forward-reference to v1.1 `clarion db backup` subcommand. - README §Storage + cross-link to operations.md. - v1.0-release-governance.md: "See also" rollback runbook. - secret-scanning.md: drop "in v0.1" qualifier; append "Trust assumption: loopback-no-token mode" section; append "Pre-WP5 catalogue upgrade requirement" section. - clarion-http-read-api.md: append loopback-no-token trust section. - CHANGELOG.md: error-enum fix UNAUTHORIZED -> UNAUTHENTICATED + add BATCH_TOO_LARGE; ADR-count phrasing fix; WP9-B deferred-to "v0.2" retarget -> "v1.1"; new "Python plugin imports REGISTRY" asterisk bullet; new SEC-03 Pre-WP5 catalogue upgrade limitation. Closes gap-register items DOC-01, DOC-02, DOC-08, DOC-09, DOC-10, DOC-11, GOV-03, SEC-02 (doc half), SEC-03.
There was a problem hiding this comment.
Pull request overview
Updates operator- and release-facing documentation for the v1.0 tag-cut, focusing on rollback/yank procedure, storage operational constraints, and explicit trust assumptions around loopback/no-token mode.
Changes:
- Add v1.0 rollback/yank runbook and link it from the release-governance doc.
- Add v1.0 storage operations doc and link it from the top-level README.
- Update secret-scanning + HTTP read API docs with loopback-no-token trust assumptions; update CHANGELOG contract notes/known limitations.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a “Storage and operations” section linking to the new ops doc. |
| docs/operator/v1.0-release-rollback.md | New post-publish rollback/yank runbook for bad releases. |
| docs/operator/v1.0-release-governance.md | Adds cross-link to the rollback runbook. |
| docs/operator/secret-scanning.md | Adds loopback-no-token trust assumption and upgrade requirement notes. |
| docs/operator/clarion-http-read-api.md | Documents loopback-no-token trust assumption for the HTTP read API. |
| docs/clarion/1.0/operations.md | New storage constraints + backup/restore procedure doc. |
| CHANGELOG.md | Aligns documented HTTP error codes / limitations with v1.0 contract notes. |
Comments suppressed due to low confidence (2)
docs/clarion/1.0/operations.md:58
- This section states that v1.0 enforces a single-analyzer rule via an exclusive
fs2lock on.clarion/clarion.lock, but the current codebase does not includefs2nor any lock acquisition inclarion analyze. Either update the document to reflect current behavior/limitations, or ensure the locking implementation lands before v1.0 is documented as enforcing it.
Only one `clarion analyze` may run against a given project root at a time.
The v1.0 binary enforces this with an exclusive `fs2` advisory lock on
`.clarion/clarion.lock`, acquired at the start of `analyze` and held for the
writer-actor lifetime. A second `clarion analyze` against the same
`.clarion/` fails fast with a clear "another clarion analyze is in progress
against this project" error rather than racing the first analyzer's run
state. (See STO-01 in the v1.0 tag-cut gap register for the originating
docs/clarion/1.0/operations.md:73
- This paragraph references
recover_preexisting_running_runsand claims the nextclarion analyzesweepsruns.status='running'rows tofailedafter abnormal exit, but no such function or sweep logic exists in the current codebase. Please update this to point at the actual implementation (if any) or rephrase as a known limitation / planned v1.1 follow-up so operators aren’t relying on recovery behavior that doesn’t happen.
**Same-process restart caveat (STO-05):** the fs2 lock is released when
the analyzer process exits, including on crash. If the previous analyzer
exited abnormally, the next `clarion analyze` will sweep
`runs.status='running'` rows to `'failed'` (see
`recover_preexisting_running_runs`). There is no PID column or heartbeat
on `runs` at v1.0 — a same-host restart cannot distinguish "previous
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+107
to
+109
| The local-first storage model, the no-NFS constraint, the no-double-analyze | ||
| constraint (fs2 advisory lock), and the v1.0 backup/restore procedure are | ||
| documented in |
Comment on lines
+61
to
64
| `BATCH_TOO_LARGE`, `STORAGE_ERROR`, `INTERNAL`. ETag / `If-None-Match` supported. | ||
| - `POST /api/v1/files/batch` — bulk resolution, up to 256 queries per | ||
| request, single pooled `ReaderPool` checkout per batch. Four-way | ||
| partitioning: `resolved` / `not_found` / `briefing_blocked` / `errors`. |
Comment on lines
+70
to
+72
| does not hold.** Any local process — any UID with read access to the | ||
| loopback bind socket — can read the entire non-blocked catalogue, | ||
| including every file's `entity_id`, `canonical_path`, `language`, and |
Comment on lines
+103
to
+107
| different.** Any local process — any UID with read access to the loopback | ||
| bind socket — can read the entire non-blocked catalogue, including every | ||
| file's `entity_id`, `canonical_path`, `language`, and `content_hash`. This | ||
| is the documented v1.0 trust matrix; it is not a defect, but it is a | ||
| constraint operators must understand. |
Comment on lines
+15
to
+22
| ``` | ||
| .clarion/ | ||
| ├── clarion.db SQLite database (entities, edges, runs, findings, summary_cache) | ||
| ├── clarion.db-wal SQLite WAL companion file | ||
| ├── clarion.db-shm SQLite shared-memory file | ||
| ├── clarion.lock fs2 advisory lock file (writer claim) | ||
| ├── instance_id Stable per-project instance ID | ||
| └── ... plugin caches, scanner baseline, etc. |
![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: bf764cb1cf
ℹ️ 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".
Comment on lines
+53
to
+56
| The v1.0 binary enforces this with an exclusive `fs2` advisory lock on | ||
| `.clarion/clarion.lock`, acquired at the start of `analyze` and held for the | ||
| writer-actor lifetime. A second `clarion analyze` against the same | ||
| `.clarion/` fails fast with a clear "another clarion analyze is in progress |
There was a problem hiding this comment.
This section states that v1.0 already enforces single-analyzer execution with an fs2 lock on .clarion/clarion.lock, but at this commit the CLI does not include fs2 in crates/clarion-cli/Cargo.toml and the analyze path has no lock acquisition (try_lock_exclusive/clarion.lock) logic. Publishing this as current behavior can cause operators to run concurrent clarion analyze processes under a false safety assumption, which risks exactly the run-state races this section warns about.
Useful? React with 👍 / 👎.
Comment on lines
+82
to
+83
| The Clarion `serve` startup banner emits a `[TRUST]` line warning when | ||
| loopback-no-token mode is active (forward-reference: the banner code is a |
There was a problem hiding this comment.
This sentence states that clarion serve currently emits a [TRUST] startup warning for loopback-no-token mode, but at this commit no such banner exists in the HTTP read server code (the only startup trust warning path is the non-loopback unauthenticated warning in crates/clarion-cli/src/http_read.rs). Operators may rely on a warning that never appears and miss an unauthenticated local deployment on shared hosts.
Useful? React with 👍 / 👎.
Closed
11 tasks
Collaborator
Author
|
Superseded — the v1.0 tag-cut work in this stacked chain has all landed on |
tachyon-beep
added a commit
that referenced
this pull request
Jun 8, 2026
… serve, config passthrough (clarion-c326ee6857) Implements the 4 still-valid deferred findings from the PR#21 review; the other 8 were verified already-fixed/outdated against current code (rc3) and need only thread resolution (recorded on the issue). #4 writer.rs begin_run TOCTOU: if begin_write_tx exhausts its retries after the runs row was auto-committed 'running', re-mark the row 'failed' under a fresh implicit tx (mirrors the CommitRun failure-remark idiom) so it isn't stranded phantom-running with current_run unset. Deliberately does NOT move the INSERT inside the tx (the ticket's literal suggestion) — that would hide the running row from cross-process analyze_status until the first batch COMMIT (the regression review #15 warns about). Best-effort; mark_stale_running_runs_failed remains the startup backstop. #15 writer.rs resume_run: capture the row's prior (status, completed_at) before flipping it to 'running' and restore them if begin_write_tx fails — a pre-existing completed run must not be left flipped to running. Both paths get deterministic, single-threaded coverage via two test seams (grab a competing write lock after the insert/update; release it on the failure path so the best-effort cleanup can re-acquire the lock) — the contention harness the ticket said #4/#15 needed, without threads or wall-clock races. #8 reader.rs open_validated: reject an unmigrated DB (new schema::reject_unmigrated_for_read → StorageError::UnmigratedIndex) so a header-valid but empty/externally-created file is refused at serve instead of auto-materialised and answered with zero rows. Keyed on user_version (0 = unmigrated), NEVER on row counts — an installed-but-unanalyzed index is user_version=CURRENT with zero entities and stays a valid serve target. #12 serve --config passthrough: serve forwards its resolved on-disk --config to analyze_start-spawned analyze (ServerState::with_analyze_config → spawn_analyze --config) so the child parses the same configuration the operator launched serve with, instead of re-discovering config and silently diverging. Tests: 2 writer failpoint tests, 2 reader schema-validation tests, 1 spawn_analyze --config forward/omit test. 972 storage+mcp+cli tests green; clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Operator-facing surface for v1.0. Single commit, 7 files (2 new + 5 modified), +400 lines.
New files
docs/operator/v1.0-release-rollback.md— h+30min "bad release shipped" runbook. Coversgh release edit --prereleaseas first action; asset-handling policy (don't delete unless sensitive); Sigstore Rekor non-revocability; v1.0.1 supersession publication; downstream notification; postmortem requirement.docs/clarion/1.0/operations.md— local-first.clarion/storage constraints. NFS prohibition; no concurrentclarion analyze(enforced after STO-01 lands); supported backup procedure (shutdown →wal_checkpoint(TRUNCATE)→ file copy); restore path; forward-reference to v1.1clarion db backupsubcommand.Modified
README.md— new §Storage section linking tooperations.md.docs/operator/v1.0-release-governance.md— "See also" cross-link to rollback runbook.docs/operator/secret-scanning.md— drop "in v0.1" qualifier; append "Trust assumption: loopback-no-token mode" section; append "Pre-WP5 catalogue upgrade requirement" section.docs/operator/clarion-http-read-api.md— append loopback-no-token trust section.CHANGELOG.md— error-enum fixUNAUTHORIZED → UNAUTHENTICATED+ addBATCH_TOO_LARGE; ADR-count phrasing fix; WP9-B "deferred to v0.2" → "v1.1"; new "Python plugin importswardline.core.registry.REGISTRY" asterisk-2 bullet; new SEC-03 "Pre-WP5 catalogue upgrade requirement" entry.Closes (filigree)
clarion-00a6bec9da— DOC-01 CHANGELOGUNAUTHORIZED→UNAUTHENTICATEDclarion-a16fa8cc12— DOC-02 CHANGELOGBATCH_TOO_LARGEmissingclarion-77f24eb561— DOC-08 secret-scanning v0.1 referenceclarion-88d2ef40b6— DOC-09 Wardline REGISTRY asterisk in CHANGELOGclarion-29847f9abd— DOC-10 CHANGELOG ADR-count phrasingclarion-04ec1044e9— DOC-11 storage operator constraints in READMEclarion-a4fb59a96a— GOV-03 rollback / yank runbookclarion-42f4fee904— SEC-02 loopback-no-token trust documentation (doc half; code half in PR 4)clarion-d59fc0b798— SEC-03 pre-WP5 catalogue upgrade requirementTest plan
Doc-only change. Cross-references verified (governance → rollback, README → operations, secret-scanning ↔ http-read-api). CHANGELOG entries land in their canonical sections without conflict with PR 2.
🤖 Generated with Claude Code