v1.0 tag-cut Day-2 (consolidated): smoke automation + STO-01/02/04, CI gates, ADR-035, docs (PR 5/5)#17
Closed
tachyon-beep wants to merge 9 commits into
Closed
Conversation
Closes the technical half of GOV-04: the operator-led smoke gate had been a markdown checklist that required a human to walk through each step. This PR adds `tests/e2e/external-operator-smoke.sh` which automates steps 1–7 (install, analyze, MCP query, idempotency, secret-plant + briefing-block verification, blocked-summary refusal) and generates a draft results file. Step 8 (operator-improvisation tally) remains human-judged because no automation can detect "operator read source outside the README to know what to do next". Verified end-to-end against `psf/requests@v2.32.3` (825 entities, 1254 edges, 8 MCP tools, 336 briefing-blocked entities post-plant, blocked- summary envelope correctly returned without LLM call). All 7 technical steps PASS; step 4.3(c) SKIPs cleanly when OPENROUTER_API_KEY is unset. The script supports two modes via env knobs: - CARGO_BUILD=1 (default): in-tree release build + editable plugin venv; what CI runs to validate the procedure stays working. - CARGO_BUILD=0 + CLARION_BIN=... + CLARION_PLUGIN_BIN=...: external operator scenario, where the binaries were installed via the GitHub Release archive + pipx and the operator just runs the script. Updates: - tests/e2e/external-operator-smoke.md: refocused as the operator-facing procedure overview pointing at the harness; retains the rationale for each step and step 8's human-judgement framing. - .gitignore: exclude `external-operator-smoke-results-*.md` so per-run artefacts don't accidentally get committed. Forward-references the v1.0 tag-cut gap register entry GOV-04 (`clarion-e464251ab3`); a dated result artefact will be archived at tag-cut time under `docs/implementation/v1.0-tag-cut/`.
There was a problem hiding this comment.
Pull request overview
Adds an automated end-to-end harness to replace the manual “external-operator smoke test” checklist for the v1.0 tag-cut gate, producing a draft results artifact while leaving the operator-judgement step as a manual fill-in.
Changes:
- Introduces
tests/e2e/external-operator-smoke.shto automate steps 1–7 (install/analyze/serve+MCP/idempotency/secret-block/blocked-summary). - Refactors
tests/e2e/external-operator-smoke.mdinto an operator-facing procedure overview that points at the harness and documents the modes/knobs. - Ignores per-run generated results artifacts in
.gitignore.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/e2e/external-operator-smoke.sh | New bash harness that executes the smoke walkthrough and writes a structured results markdown file. |
| tests/e2e/external-operator-smoke.md | Updated procedure doc to instruct running the harness (in-tree vs external binaries) and clarify step automation. |
| .gitignore | Excludes generated smoke result artifacts from git tracking. |
Comments suppressed due to low confidence (2)
tests/e2e/external-operator-smoke.sh:455
- When generating the results table, the script greps
tests/e2e/external-operator-smoke.mdusing a relative path. At this point the working directory is the cloned corpus ($WORK_DIR/corpus), so the grep will fail and the step description column will always fall back tosee mdin in-repo runs. Use an absolute path (e.g.$REPO_ROOT/tests/e2e/external-operator-smoke.md) orcd "$REPO_ROOT"before writing the report.
echo "| # | Step | Status | Detail |"
echo "|---|------|--------|--------|"
for step in 1 2 3 4.1 4.2 4.3a 4.3b 4.3c 5 6 7 8; do
echo "| $step | $(grep -A1 "^| $step " tests/e2e/external-operator-smoke.md 2>/dev/null | head -1 | cut -d'|' -f3 | sed 's/^ *//;s/ *$//' || echo "see md") | ${step_status[$step]:-MISSING} | ${step_detail[$step]:-} |"
done
tests/e2e/external-operator-smoke.sh:333
- The harness depends on
sqlite3for entity/edge counts (e.g. step 5), but there is no preflight check. Ifsqlite3is missing on a fresh machine, step 4.1 may record a FAIL (because it falls back to 0) but step 5 will later hard-exit underset -e, preventing the results file from being written. Add a preflightcommand -v sqlite3check (and/or make all sqlite3 queries non-fatal) so failures are reported consistently.
log "running clarion analyze (re-run for idempotency) ..."
if "$CLARION_BIN" analyze . >"$WORK_DIR/analyze2.out" 2>"$WORK_DIR/analyze2.err"; then
ENTITY_COUNT_2="$(sqlite3 .clarion/clarion.db 'SELECT COUNT(*) FROM entities WHERE kind != "subsystem"')"
EDGE_COUNT_2="$(sqlite3 .clarion/clarion.db 'SELECT COUNT(*) FROM edges')"
if [ "$ENTITY_COUNT_2" = "$ENTITY_COUNT_1" ] && [ "$EDGE_COUNT_2" = "$EDGE_COUNT_1" ]; then
record "5" "PASS" "idempotent: entities=$ENTITY_COUNT_2 edges=$EDGE_COUNT_2 unchanged"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+91
to
+93
| CLARION_VERSION="$("$CLARION_BIN" --version 2>&1 | head -1)" | ||
| PLUGIN_VERSION="$("$CLARION_PLUGIN_BIN" --version 2>&1 | head -1 || true)" | ||
| log "preflight: $CLARION_VERSION / $PLUGIN_VERSION" |
Comment on lines
+23
to
+37
| ## How to run the harness | ||
|
|
||
| ### CI / repo-internal smoke (in-tree build) | ||
|
|
||
| From the repo root on the target machine (Linux or macOS): | ||
|
|
||
| ```bash | ||
| bash tests/e2e/external-operator-smoke.sh | ||
| ``` | ||
|
|
||
| The harness builds the workspace in release mode, installs the plugin into the | ||
| editable venv at `plugins/python/.venv`, fetches the canonical corpus | ||
| (`psf/requests` at the pinned tag), and walks steps 1–7. Step 8 is left for | ||
| the operator to fill in. A draft results file is written to | ||
| `tests/e2e/external-operator-smoke-results-YYYY-MM-DD-<host>.md`. |
![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: cc9b899cb3
ℹ️ 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
+140
to
+141
| ENTITY_COUNT_1="$(sqlite3 .clarion/clarion.db 'SELECT COUNT(*) FROM entities WHERE kind != "subsystem"' || echo 0)" | ||
| EDGE_COUNT_1="$(sqlite3 .clarion/clarion.db 'SELECT COUNT(*) FROM edges' || echo 0)" |
There was a problem hiding this comment.
The harness uses the sqlite3 CLI to derive pass/fail criteria (entity counts, blocked counts, finding counts) but never checks that sqlite3 is installed; on minimal operator hosts without that binary, these substitutions degrade to 0/empty values and the script reports technical failures unrelated to Clarion behavior. This makes the smoke gate produce false negatives due to host tooling drift rather than product regressions.
Useful? React with 👍 / 👎.
Comment on lines
+358
to
+359
| elif [ "$BLOCKED_COUNT" -gt 0 ]; then | ||
| record "6" "PASS" "post-plant: $BLOCKED_COUNT blocked entities (no CLA-SEC-SECRET-DETECTED finding rows; finding code may have changed)" |
There was a problem hiding this comment.
The step-6 criteria in the updated procedure require both blocked entities and a CLA-SEC-SECRET-DETECTED finding, but this branch marks the step as PASS when BLOCKED_COUNT > 0 even if FINDING_COUNT == 0. That allows regressions in findings persistence/rule wiring to slip through while still reporting the smoke gate as green.
Useful? React with 👍 / 👎.
Closes clarion-c0caf61ab6 — STO-01 in docs/implementation/v1.0-tag-cut/ gap-register.md. Two concurrent `clarion analyze` invocations on the same project had no cross-process mutex: each opened its own writer- actor, each called `BeginRun` (INSERT a fresh `runs` row in `status='running'`), and each raced on entity/edge inserts under SQLite WAL. The in-process `ActorState::current_run` guard in clarion-storage prevented a single writer from issuing two BeginRuns; it did nothing across processes. Phase 3 clustering's fresh Connection::open at analyze.rs:687 then read through whatever entities the other process had committed. Fix: - Add `fs2 = "0.4"` to `[workspace.dependencies]` and to clarion-cli. - New module `crates/clarion-cli/src/analyze_lock.rs` exposes `acquire_analyze_lock(clarion_dir)` returning an `AnalyzeLockGuard` RAII handle. Opens (or creates) `.clarion/clarion.lock` and calls `FileExt::try_lock_exclusive()`. On contention returns an anyhow::Error naming the lock path so the operator can identify the conflict. - Wire the acquire at `analyze::run_with_options` immediately after the `.clarion/` existence check. Drop order is load-bearing: the guard outlives the writer-actor's `handle.await` at the bottom of the function — documented on `AnalyzeLockGuard`. Scope is analyze-only. The gap-register prescription parenthetically includes "serve write paths", but serve's LLM-summary writer only activates with --enable-llm-summary and emits per-MCP-request inserts that SQLite WAL serialises against analyze's writes; analyze+serve coexistence is a different concern outside STO-01's "second clarion analyze corrupts run state" scope. Capturing the analyze-only mutex here without also locking serve preserves the standard dogfood loop (re-analyze while MCP is running) and avoids over-fitting the fix. Tests: - analyze_lock::tests::second_acquire_fails_while_first_held - analyze_lock::tests::second_acquire_succeeds_after_first_drops - analyze_lock::tests::missing_clarion_dir_errors Gap-register note: STO-01 cited `run_lifecycle.rs:19-25` as evidence of the "UPDATE runs SET status='failed' WHERE status='running'" hazard. That SQL is actually in clarion-storage's `cleanup_after_channel_close` (writer.rs:262-281) and fires only on writer-channel close, not at analyze startup. The underlying claim (no cross-process lock) is correct; the citation needs a future gap-register pass. CI floor green: cargo fmt --check (new file clean — pre-existing http_read.rs drift filed as clarion-obs-24940665f3), cargo clippy --workspace --all-targets --all-features -D warnings, cargo build --workspace --bins, cargo nextest run --workspace --all-features (508 passed, 2 skipped), RUSTDOCFLAGS=-D warnings cargo doc, cargo deny check, tests/e2e/sprint_1_walking_skeleton.sh.
pyright==1.1.409 is a hard runtime dependency of clarion-plugin-python (pyproject.toml `dependencies`), so `pyright-langserver` is expected on PATH or in the active virtualenv after `pip install -e plugins/python`. The shared `pyright_langserver` fixture in test_pyright_session.py and test_extractor.py previously called `pytest.skip(...)` when the binary was missing. In CI this is a silent-skip: the entire pyright-dependent test class disappears, the job stays green, and a broken venv install ships unnoticed. Converted both fixtures to `pytest.fail(...)` with a diagnostic message naming the install command — a missing executable now hard-fails the suite. Closes clarion-dc99115f5d (v1.1 security-hardening item 3 in docs/implementation/v1.0-tag-cut/gap-register.md).
`summary_cache.entity_id` had no FK while its sibling caches
(`inferred_edge_cache.caller_entity_id`,
`entity_unresolved_call_sites.caller_entity_id`) declared
`REFERENCES entities(id) ON DELETE CASCADE`. Confirmed bug per gap
register, not intentional asymmetry: production writes via
`upsert_summary_cache` already pass through `entity_by_id`-loaded
entities (clarion-mcp/src/lib.rs:1186), and `PRAGMA foreign_keys = ON`
is applied on both write and read connections, so the constraint will
be enforced without changing any production path.
Edit applied in place in `0001_initial_schema.sql` per ADR-024 — no
v1.0 tag exists yet, so the retirement trigger ("first external
operator pulls a published Clarion build and produces a
schema_migrations ledger") has not fired. The gap register's
"requires table-rebuild migration" framing assumed a post-tag fix;
landing it now skips that ceremony.
Two tests updated to satisfy the new FK:
- `migration_0001_extends_summary_cache_for_mcp_staleness_tracking`
(schema_apply.rs) now seeds the parent entity row, mirroring the
pattern the neighbouring inferred_edge_cache test already uses.
- `summary_cache_writer_commands_do_not_require_active_analyze_run`
(writer_actor.rs) seeds the parent entity via direct SQL (new
`seed_entity_row` helper) — bypassing the writer's
`BeginRun → InsertEntity` protocol preserves the test's original
intent of verifying that summary_cache writer commands work
without an active analyze run.
Re-labels the issue release:v1.1 → release:v1.0 since the fix shipped
in v1.0 scope, and strikes STO-03 from the v1.1 storage-hardening list
in the gap register.
Closes clarion-4c49ccf5d0.
…OV-02) - Wire tests/e2e/sprint_2_mcp_surface.sh + phase3_subsystems.sh into the ci.yml walking-skeleton job and the release.yml verify job (TEST-01, TEST-02). - release.yml verify: fetch-depth 0 + assert tagged commit is an ancestor of origin/main, closing the tag-lineage supply-chain bypass (CI-01). - release-subjects: include clarion_plugin_python*.tar.gz in the SLSA provenance subjects so the Python sdist is attested (CI-03). - New verify-published-release job: re-download public assets, recompute SHA256 against published sidecars, cosign verify-blob against Rekor (CI-04). - check-github-release-governance.py: assert an active ruleset targets refs/tags/v*; self-test covers present + absent fixtures (GOV-02 script half). Local: both new e2e scripts exit 0; YAML valid; governance --self-test passes. CI-green + workflow_dispatch dry-run remain push-verification residue. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-04/05, SEC-02/03, DOC-08) - sprint_1_walking_skeleton.sh: assert PRAGMA integrity_check == ok after analyze, failing the e2e on a torn/corrupt DB (STO-04a). - operations.md: backup procedure rationale (WAL pages live in -wal; naive cp during live analyze yields a torn copy) and same-process-restart caveat; clarion db backup + runs.owner_pid/heartbeat_at deferred to v1.1 (STO-04b/05). - secret-scanning.md + clarion-http-read-api.md: state the loopback-no-token trust assumption and that the startup banner now ships and logs at WARN; drop stale v0.1/v0.2 qualifiers (SEC-02 docs, SEC-03, DOC-08). - http_read.rs: rustfmt reflow only (HEAD failed cargo fmt --check after 8e5804b committed the SEC-02 banner un-formatted); no behavior change. Gates: fmt/clippy/build pass; nextest 508 passed/2 skipped; sprint_1 e2e exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…b to release:v1.1 (DOC-06) Commits the in-flight requirements.md/system-design.md deferral sweep that annotates every requirement narrowed out of the v1.0 MVP (per ADR-030 + Sprint 2 scope amendment §3/§4) with a deferred-scope callout. Per the DOC-06 reconciliation, the deferral vocabulary is aligned to the release-label scheme: the sweep's blockquote callouts (and the two capabilities-table rows) now say "v1.1" / "release:v1.1" instead of "v0.2", matching CHANGELOG/CLAUDE.md/loom.md. Deliberately NOT renamed (would create drift, not alignment): Accepted-ADR table summaries (ADR-003/015/016/017/018 — immutable, and the ADR files themselves say v0.2), the v0.1-path / v0.2-retirement design-ladder pairs, the Non-Goals roadmap, and Shuttle (v0.2+). Those pair v0.2 with v0.1 on the design-milestone axis, distinct from the v1.x release axis. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
This was referenced May 28, 2026
tachyon-beep
changed the title
Collaborator
Author
Consolidated Day-2 tag-cut workThis branch now carries the full Day-2 set; PRs #18/#19/#20 were closed as superseded into here (their work was duplicated on a divergent line; reconciled per maintainer decision). Commits on top of
Verified green on the consolidated tree: Push-only residue (unprovable locally): CI-green on push, Still operator-gated for tag-cut: GOV-01 (branch/ruleset + RELEASE_GOVERNANCE_TOKEN), GOV-02 ruleset creation, GOV-04 (fresh-VM smoke), STO-03 (published_build.txt at tag). Note: the governance script now also requires the |
Supporting (non-normative) review artifacts under the canonical reviews/ location per CLAUDE.md precedence note. Not cited as decision sources. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eb748d507
ℹ️ 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".
| if pattern in {tag_pattern, "~ALL"} or normalized in {desired, "*"}: | ||
| return True | ||
| # An include like `v*` matches the concrete tag families we cut. | ||
| if fnmatchcase("v1.0.0", normalized) or fnmatchcase(desired, normalized): |
There was a problem hiding this comment.
When a tag ruleset includes a narrower glob such as refs/tags/v1.0.0 or refs/tags/v1.*, this sample-tag check still returns true because v1.0.0 matches that include. check_governance then reports that refs/tags/v* is protected even though future release tags like v2.0.0 can bypass the rule, so the release-governance guard can pass with insufficient tag protection.
Useful? React with 👍 / 👎.
8 tasks
Collaborator
Author
|
Superseded — the v1.0 tag-cut work in this stacked chain has all landed on |
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
Closes the technical half of GOV-04. The external-operator smoke gate had been a markdown checklist requiring a human walkthrough; this PR adds an automated harness that exercises steps 1–7 and generates a draft results file. Step 8 (operator improvisation tally) remains human-judged.
Single commit, 3 files (1 new + 2 modified), +588 lines.
Verification
Verified end-to-end against
psf/requests@v2.32.3on Linux x86_64:find_entity/callers_ofreturned non-empty results.CLA-SEC-SECRET-DETECTEDfindings.summaryon blocked entity returned thebriefing_blockedenvelope without an LLM call.All 7 technical steps PASS; step 4.3(c)
summarySKIPs cleanly whenOPENROUTER_API_KEYis unset.Files
tests/e2e/external-operator-smoke.sh(new, executable) — harness.tests/e2e/external-operator-smoke.md— refocused as the operator-facing procedure overview pointing at the harness..gitignore— exclude per-run results files.Modes
CARGO_BUILD=1CARGO_BUILD=0+CLARION_BIN=...+CLARION_PLUGIN_BIN=...Closes (filigree)
Forward-references
clarion-e464251ab3(GOV-04). The dated result artifact for the v1.0 tag-cut will be produced at Day 3 by running this harness on a fresh Linux VM and a fresh macOS host, and archived underdocs/implementation/v1.0-tag-cut/external-operator-smoke-result-*.md.Test plan
bash -n tests/e2e/external-operator-smoke.sh(syntax)psf/requests@v2.32.3on linux-x86_64: 7/7 PASS + 1 SKIPaarch64-apple-darwin(deferred to Day 3 operator work)Notes
The script does not (yet) gate CI — wiring it into a workflow is an operator decision; the alternative is running it as part of the Day 3 tag-cut procedure. The script's exit code is suitable for either choice.
🤖 Generated with Claude Code