Sprint 1: WP2 plugin host + WP3 Python plugin (walking skeleton)#1
Merged
Conversation
ADR-023 adopted at the zero-code frontier: Rust edition 2024, workspace [lints] with clippy::pedantic = "warn" + unsafe_code = "forbid", pinned rustfmt.toml + clippy.toml, cargo-nextest as the test runner, cargo-deny for supply-chain hygiene, GitHub Actions CI running fmt-check + pedantic clippy + nextest + cargo-doc + cargo-deny on every PR. Python side: ruff strict + mypy --strict from day 1 + pytest + pre-commit. Retrofit cost asymmetry is the justification — every surface above is near-free at zero code, materially expensive after any implementation lands. UQ-WP1-09's original "edition 2021, fine to document and move on" framing was the canonical tell for an unexamined default; reopened and re-resolved. UQ-WP3-10's "defer mypy" framing reopened and re-resolved. Doc edits: - docs/clarion/adr/ADR-023-tooling-baseline.md (new, Accepted) - docs/clarion/adr/README.md — add ADR-023 row - docs/implementation/sprint-1/wp1-scaffold.md — §2 ADR anchors, §4 deps table expanded, §5 UQ-WP1-09 re-resolved, §6 Task 1 expanded, §8 exit criteria add fmt/clippy-pedantic/deny/doc/CI gates - docs/implementation/sprint-1/wp3-python-plugin.md — §4 deps, §5 UQ-WP3-10 re-resolved, §6 Task 1 expanded, §8 exit criteria add ruff/mypy-strict/pytest/pre-commit/CI gates - docs/implementation/sprint-1/signoffs.md — Tier A.1 + A.3 add ADR-023 gate rows; A.6.2 extends to ADR-023 - docs/implementation/sprint-1/README.md — add ADR-023 reference - docs/superpowers/plans/2026-04-18-wp1-scaffold-storage.md — rewrite Task 1 (13 steps; ships rust-toolchain, rustfmt, clippy, deny, workspace [lints], CI workflow); swap cargo test → cargo nextest; Task 9 adds full ADR-023 gate sweep
Cargo workspace with clarion-core, clarion-storage, clarion-cli members, edition 2024, resolver 3, workspace [lints] block with clippy::pedantic = "warn" + unsafe_code = "forbid" (ADR-023). Every member crate declares lints.workspace = true so a later-added crate cannot drift off the floor. ADR-011 dep stack pinned at workspace level: rusqlite (bundled), deadpool-sqlite, tokio, thiserror, clap, tracing. rust-toolchain.toml pins stable with clippy + rustfmt + llvm-tools-preview components. rustfmt.toml, clippy.toml, deny.toml configured per ADR-023. GitHub Actions workflow runs fmt-check + pedantic clippy + cargo-nextest + cargo-doc + cargo-deny on every PR. python-plugin job arrives with WP3 Task 1. Resolves UQ-WP1-09 (reopened from the original "edition 2021, fine to document and move on" framing).
Commit Cargo.lock (reproducibility floor for cargo-deny + cargo-nextest). Tighten clarion-storage tokio dev-dep to only add `test-util` — the base features inherit from the workspace tokio dep; re-listing them obscured which features were dev-only and created a maintenance trap. Expand deny.toml license allowlist with Zlib + CC0-1.0 — both common-in- graph licenses that would otherwise trip cargo-deny on the first real dep pull. Alphabetised the list. Drop unused serde_json from clarion-cli — YAGNI; re-add at point of use in Task 5 or Task 7 if needed. Add concurrency block + llvm-tools-preview component in CI so: (a) rapid PR pushes cancel stale in-flight runs, and (b) the action-installed toolchain matches rust-toolchain.toml exactly (was missing llvm-tools-preview). Resolves code-quality review on 50df042: 1 Important, 2 Minor, 2 Nitpick.
entity_id() assembles {plugin_id}:{kind}:{canonical_qualified_name} with
ADR-022 grammar enforcement on plugin_id and kind. Rejects empty segments
and any segment containing ':' (UQ-WP1-07 resolution).
13 unit tests cover positive grammar cases (module fn, class method,
nested fn, core-reserved file/subsystem), empty-segment rejections,
grammar violations, and colon-in-segment detection.
canonical_qualified_name is validated for empty + colon only; its internal
shape is the emitting plugin's concern per ADR-022.
Make entity_id() empty-check ordering explicit so canonical_qualified_name
matches validate_grammar's empty-first-then-colon shape. Accidentally
correct before; trap for the next maintainer.
Replace #[derive(Deserialize)] on EntityId with a validating custom impl
routing through FromStr. Previously, any JSON string would silently
deserialise into a structurally invalid EntityId — dormant footgun that
would fire on the first Sprint 2 JSON-read path. Adds new error variant
MalformedId { value: String } for inputs that aren't 3 colon-separated
segments.
Implement FromStr for EntityId so Task 3 (SQL read path) has a
principled way to reconstruct an EntityId from the TEXT column without
re-implementing split-and-validate inline.
Add doc comment to EntityId::as_str() and # Errors section to entity_id().
5 new tests: FromStr round-trip, FromStr rejection of <3 segments,
FromStr rejection of empty segments via underlying validator, Deserialize
accepts valid IDs, Deserialize rejects unstructured strings.
Resolves code-quality review on c7630f1: 2 Important + 2 Minor.
Migration 0001 transcribes the full detailed-design.md §3 schema: tables (entities, entity_tags, edges, findings, summary_cache, runs, schema_migrations), entity_fts FTS5 virtual table, three FTS triggers (_ai/_au/_ad), priority + git_churn_count generated columns with partial indexes, and the guidance_sheets view (aggregating tags via correlated subquery against entity_tags — reconciles §3 shape with the normalised tag storage). schema::apply_migrations() reads embedded SQL via include_str!, tolerates re-runs (idempotent by construction), and records each apply in schema_migrations. pragma::apply_write_pragmas() + apply_read_pragmas() centralise ADR-011 connection-open invariants. StorageError wraps rusqlite::Error via thiserror (UQ-WP1-06 resolution). 8 integration tests in schema_apply.rs cover table/trigger/view presence, FTS queryability, generated-column round-trip, and idempotency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Make WAL enforcement a runtime error rather than a debug_assert. In release builds the assert is a no-op; silent `delete`-mode fallback under the ADR-011 synchronous=NORMAL policy is a data-loss risk on filesystems that don't support WAL (tmpfs, network mounts). Add StorageError::PragmaInvariant variant and return it on WAL failure. applied_count(): pre-check schema_migrations table existence so genuine query errors (disk I/O, lock contention) propagate rather than silently returning 0 via unwrap_or(0). StorageError::Pool now wraps deadpool_sqlite::PoolError structurally via #[from], preserving timeout/creation/recycle discrimination for pool diagnostics. Task 4 (reader pool) and Task 6 (writer actor) will exercise this. read_applied_versions: propagate version-conversion errors instead of silently truncating i64 → u32. No sane migration version triggers this, but clamping creates confusing downstream errors if a bogus row lands. Add fts_trigger_populates_entity_fts_on_insert test exercising the entities_ai trigger end-to-end — asserts that inserting an entity with summary.briefing.purpose produces a matching entity_fts row via MATCH. Clarify apply_one defence-in-depth comment. Change test priority value from "P1" to 2 (integer) to match project priority convention. Resolves code-quality review on 1623514: 3 Important, 3 Minor, 1 Nitpick. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ReaderPool wraps deadpool-sqlite (ADR-011 default max 16, configurable per call site). with_reader() acquires a pooled connection, applies the read-side PRAGMAs, and runs the caller's closure inside deadpool's interact() block so the runtime yields during SQLite I/O. Integration tests cover concurrent-read throughput (two readers via tokio::join!) and committed-snapshot visibility (reader sees a row committed on an out-of-band write connection).
Expand with_reader doc: note the 'static capture requirement (callers must own or clone into the closure) and distinguish Pool (exhaustion / timeout) from PoolInteract (closure panic / abort). Correct open doc: create_pool does not validate the SQLite file — connection errors surface on first with_reader call, not at pool-build time. Add three test-coverage gap fills in preparation for Task 6 hammering the pool under concurrent load: 1. pool_queues_when_exhausted_and_proceeds_after_release — N+1 simultaneous readers against max_size=N blocks correctly rather than erroring, and proceeds after one slot releases. 2. reader_error_propagates_and_connection_returns_to_pool — closure-level errors propagate as StorageError::Sqlite and the connection is returned to the pool cleanly (subsequent reads succeed). 3. reader_panic_is_caught_as_pool_interact_and_pool_remains_usable — closure panics surface as StorageError::PoolInteract; deadpool recycles or discards the poisoned connection; the pool remains usable. Resolves code-quality review on 55f6551: 4 Minor + 1 Nitpick. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
clarion install creates .clarion/{clarion.db,config.json,.gitignore} and
a clarion.yaml stub at the project root per detailed-design.md §File
layout. Refuses on existing .clarion/ (UQ-WP1-08). --force is recognised
by clap but errors out — Sprint 1 does not implement overwrite.
ADR-005 moved from Backlog to Accepted: .clarion/ git-tracking policy
(committed: clarion.db, config.json, .gitignore, runs/*/config.yaml,
stats.json, partial.json; excluded: WAL/SHM sidecars, *.shadow.db,
runs/*/log.jsonl, tmp/, logs/). UQ-WP1-04 resolved.
4 integration tests cover happy-path creation, migration-count
verification, overwrite refusal, and --force stub message.
- install.rs: trace-log when clarion.yaml is skipped, so the skip decision is observable and --force work (Sprint 2+) cannot silently regress. - install.rs: add path context to Connection::open error chain; without it, database-open failures surfaced as "unable to open database file" with no path in the anyhow chain. - install.rs: pre-check --path existence with a bespoke bail message before canonicalize(), so the common typo case produces a clear error instead of a raw OS errno. - tests/install.rs: new install_leaves_existing_clarion_yaml_untouched test asserting pre-existing clarion.yaml is preserved verbatim. No behavioural change to the happy path. 38 integration tests workspace-wide.
Writer::spawn() starts a tokio::spawn_blocking task owning the sole write rusqlite::Connection. Callers enqueue WriterCmd variants via a bounded mpsc channel (ADR-011 defaults: capacity 256, batch size 50). Each command carries a oneshot::Sender ack for per-command replies (UQ-WP1-03). WriterCmd variants: BeginRun, InsertEntity, CommitRun, FailRun. InsertEntity commits at batch boundaries; CommitRun/FailRun finalise run state. FailRun rolls back the pending transaction before updating the runs row. commits_observed (Arc<AtomicUsize>) counts COMMIT statements fired by the actor — exposed as a normal field on Writer so tests can assert the per-N cadence without #[cfg(test)] gating on the hot loop. 3 integration tests cover round-trip insert, batch-boundary commit cadence (150 inserts → 3 in-flight commits + 1 CommitRun = 4), and FailRun rollback.
State-machine correctness and L3 lock-in polish.
- writer.rs insert_entity / commit_run: flip state.in_tx=false BEFORE the
fallible execute_batch("COMMIT"), so a COMMIT failure leaves our state
machine accurately reflecting SQLite's aborted-transaction state.
Without this, the `?` on COMMIT short-circuited before the state update,
desynchronising the actor from SQLite and silently corrupting subsequent
inserts under disk-full / I/O failure.
- error.rs: add StorageError::WriterProtocol(String) variant. Replaces
the prior abuse of StorageError::Sqlite(InvalidQuery) for protocol
violations (InvalidQuery has a pre-defined rusqlite meaning unrelated
to writer-actor contract).
- writer.rs begin_run: return WriterProtocol on double-BeginRun.
- writer.rs insert_entity: new guard — return WriterProtocol if
InsertEntity arrives without a preceding BeginRun. Previously this
silently started a transaction outside any run context.
- writer.rs helpers: drop &Arc<AtomicUsize> in favour of &AtomicUsize at
insert_entity/commit_run/run_actor signatures — the Arc wrapper was
not used inside the helpers.
- writer.rs: doc commits_observed field semantics (counts CommitRun's
final commit plus per-batch commits); note the must-read-before-drop
lifecycle. send_wait is marked as intended-future-use for Task 7+.
- 2 new integration tests cover both new WriterProtocol error paths.
Writer test count 3 → 5; workspace 41 → 43.
clarion analyze opens .clarion/clarion.db, BeginRun → CommitRun with status 'skipped_no_plugins'. Warns via tracing::info! that no plugins are wired. Fails cleanly with a `clarion install`-pointing message if .clarion/ is missing. uuid v4 added as a workspace dependency for run-id generation. Minimal inline civil_from_unix_secs() avoids pulling chrono solely for ISO-8601 formatting; later WPs that need richer time handling can promote chrono to a workspace dependency then. 2 integration tests cover the happy path (one runs row, zero entities) and the missing-.clarion/ error path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- analyze.rs: switch BeginRun/CommitRun sends to Writer::send_wait. Eliminates the manual double-oneshot pattern and puts the first real caller on the L3 API that was added in Task 6. Removes the now-unused `let tx = writer.sender()` binding and the `oneshot` import. - analyze.rs: capture started_at and completed_at as separate timestamps. Sprint-1 runs are short, but the fields are semantically different and should not share a single `now()` call. - analyze.rs: downgrade drop(tx);drop(writer) to a single drop(writer) since Writer owns the sole sender after the send_wait refactor; document why the drop closes the channel. - analyze.rs: no-plugins line now uses tracing::warn! rather than tracing::info!. The handoff spec describes this as a "warning that no plugins are wired".
LlmProvider + NoopProvider in clarion-core. NoopProvider::name() panics loudly — if it ever fires, some WP1 code is reaching for a real provider before WP6 is wired. 2 tests: trait implementation and panic contract.
wp1_e2e.rs runs the README §3 demo script at WP1 scope: clarion install creates .clarion/; clarion analyze commits a run with status skipped_no_plugins and zero entities; migration 0001 recorded as schema_version 1. WP2+WP3 extend this test with non-zero entity asserts.
Tier A.1 boxes ticked in signoffs.md with the WP1-close commit date. README.md §4 lock-in table stamped for L1/L2/L3. wp1-scaffold.md §5 UQ resolutions recorded inline with outcome + resolving task. WP1 complete; ready for WP2 kickoff.
Merges sprint-1/wp1-scaffold into main. Lock-ins L1 (SQLite schema), L2 (entity-ID 3-segment format), L3 (writer-actor) shipped and stamped 2026-04-18. ADR-005 and ADR-023 Accepted. 48 tests, all 7 ADR-023 gates green on the merged tip. Unblocks WP2 (plugin host) and WP3 (Python plugin).
Nine findings from the wp2 plan review, addressed across three docs:
wp2-plugin-host.md §L5 — rewrite manifest schema to match ADR-021 §Layer 1
* [capabilities.runtime] sub-block with expected_max_rss_mb,
expected_entities_per_file, wardline_aware, reads_outside_project_root
* drop the drifted max_content_length_bytes / max_entities_per_run keys
(those are core-enforced with fixed defaults per ADR-021 §2b/§2c, not
plugin declarations)
* add reserved-edge-kind binding note (ADR-022) + rule-ID namespace
registry summary + manifest-validation finding-codes list
* note the TOML-vs-YAML encoding divergence from detailed-design §1 so
it doesn't read as silent drift
wp2-plugin-host.md Task 1 — ADR-022 identifier grammar enforcement
* kind strings must match [a-z][a-z0-9_]*
* rule-ID prefix must match CLA-[A-Z]+(-[A-Z0-9]+)+
* reject manifest declaring file/subsystem/guidance in entity_kinds
(CLA-INFRA-MANIFEST-RESERVED-KIND)
* reject CLA-INFRA-/CLA-FACT- rule_id_prefix (CLA-INFRA-RULE-ID-NAMESPACE)
* positive test: manifest listing 'contains' in edge_kinds parses ok
wp2-plugin-host.md Task 4 — defer CLA-INFRA-PLUGIN-ENTITY-OVERRUN-WARNING
to catalog-emitting Tier B sprint (Sprint 1 is one-file-per-invocation,
no useful surface area), with the subcode named so future implementers
find it
wp2-plugin-host.md Task 6 — ADR-021 §Layer 1 initialize rejection for
reads_outside_project_root: true
(CLA-INFRA-MANIFEST-UNSUPPORTED-CAPABILITY)
wp3-python-plugin.md Task 7 — sync the first real plugin.toml to the
revised [capabilities.runtime] schema
signoffs.md §A.2 — add A.2.10, A.2.11, A.2.12 for malformed-grammar,
reserved-kind, and unsupported-capability rejection tests
Cross-checked against ADR-021 §Layer 1 (canonical schema) and ADR-022
§Core owns (identifier grammar, reserved kinds, rule-ID namespace).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…+ ADR-022 grammar) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements Task 2 of WP2 (Sprint 1): LSP-style framing layer and typed JSON-RPC 2.0 protocol structs for the five L4 methods (initialize, initialized, analyze_file, shutdown, exit). - transport.rs: Frame, read_frame (max_bytes ceiling), write_frame, TransportError (six variants via thiserror); 7 inline tests including oversize-without-body-consume and two-consecutive-frames checks. - protocol.rs: JsonRpcVersion unit-struct (rejects non-"2.0" at deserialise time), RequestEnvelope, NotificationEnvelope, ResponseEnvelope/ResponsePayload, ProtocolError, typed param+result structs for all five methods (Option B: struct-per-method); 6 inline tests. AnalyzeFileResult.entities is Vec<Value> placeholder for Task 6. - plugin/mod.rs + lib.rs: wired with pub use re-exports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses review findings on 4b20244 (WP2 Task 2). Critical: - C1: Replace unit-struct params with empty-braced structs (InitializedNotification, ExitNotification, ShutdownParams, ShutdownResult). Unit structs serialise to JSON `null`, which violates JSON-RPC 2.0 §4.2 (params must be a structured value). Empty-braced form `struct Foo {}` serialises to `{}`. Dropped the now-redundant `to_value()` helpers. - C2: Custom Deserialize for ResponseEnvelope that enforces JSON-RPC 2.0 §5 exactly-one-of result/error. The previous `#[serde(flatten)]` externally-tagged enum silently dropped `error` when both keys were present, hiding misbehaving-plugin errors from the supervisor. Both-present and neither-present now yield loud deserialisation errors with descriptive messages (also fixes M7). Important: - I3: Retry EINTR (ErrorKind::Interrupted) in the body-read loop rather than propagating as TransportError::Io. SIGCHLD and other signals can cause spurious Interrupted errors on subprocess pipes; the manual loop now matches std::io::Read::read_exact behaviour while keeping the TruncatedBody{expected,actual} error detail. - I4: write_frame now calls writer.flush() before returning. Without flush, a BufWriter<ChildStdin> (which Task 6 will use) would buffer each frame without sending it — silent deadlock. - I5: Bound header-line length at MAX_HEADER_LINE_BYTES = 8 KiB (matching nginx's large_client_header_buffers default). Previously read_line was unbounded — a malicious plugin sending a 1 GB header line with no LF would exhaust host memory. Replaced read_line with a bounded read_bounded_line helper backed by fill_buf/consume. - I6: Use .trim() instead of .trim_start() when parsing header values, so trailing whitespace before CRLF (e.g. `Content-Length: 42 \r\n`) parses cleanly. Tests: +7 (58 -> 65). New tests cover each finding with a regression test; ADR-023 gates (cargo fmt, clippy -D warnings) clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds plugin/mock.rs (#[cfg(test)]-gated) with three canned behaviours (Compliant, Crashing, Oversize), a tick-based I/O model over Vec<u8> / Cursor<Vec<u8>>, and four inline tests (1 mandatory + 3 recommended). Wires #[cfg(test)] pub(crate) mod mock into plugin/mod.rs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… guards Four consolidated fixes that must land before Task 4 (limits) and Task 6 (supervisor): B1 — Separate `plugin_id` from `[plugin].name`. wp2 §L5 said plugin_id came from [plugin].name but examples showed name = "clarion-plugin-python" (hyphens) while wp3 §L7 used plugin_id = "python". Three docs contradicted. Resolution: add `plugin_id: String` validated against ADR-022 [a-z][a-z0-9_]*; name stays human-readable. Closes clarion-b11b3bb3d9. B2 — Change protocol params PathBuf → String. make_request uses .expect() on serde_json::to_value(path) which panics on non-UTF-8 paths (possible on Linux). Moving to String makes the panic statically impossible; Task 4's jail owns UTF-8 validation at the boundary. Closes clarion-77c6971e81. B3 — Add state guards to mock plugin's respond_initialize and respond_shutdown. Prior gaps would let Task 7 crash-loop tests silently emit double-responses or accept out-of-order shutdowns. B4 — Preserve remaining inbox bytes in mock tick() when dispatch errors. Prior behaviour silently dropped queued frames on mid-batch error, making multi- frame test scenarios unreliable. Test count: 69 → 74. Clippy + fmt clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ADR-021 defaults)
Implements ADR-021 §2a–§2d: the four core-enforced minimums that plugins
cannot opt out of.
§2a — jail.rs: `jail(root, candidate) -> Result<PathBuf, JailError>` resolves
both paths via `std::fs::canonicalize` (follows symlinks, UQ-WP2-03) and
asserts `starts_with`. `jail_to_string` convenience wrapper for the wire
boundary. `JailError::{EscapedRoot, Io, NonUtf8Path}`.
§2a — limits.rs: `PathEscapeBreaker` rolling-window counter trips on >10
escapes in 60 s (VecDeque<Instant>; clock injectable via `record_escape_at`
for deterministic tests). `BreakerState::{Open, Tripped}`.
§2b — limits.rs: `ContentLengthCeiling` newtype (Copy, 8 MiB default).
`transport::read_frame` signature changed from `max_bytes: usize` to
`ceiling: ContentLengthCeiling`; all call sites in transport.rs and mock.rs
updated.
§2c — limits.rs: `EntityCountCap` with `try_admit(&mut self, delta) ->
Result<(), CapExceeded>`. Default cap 500,000. Cumulative across calls.
§2d — limits.rs: `apply_prlimit_as(max_rss_mib: u64) -> io::Result<()>`.
Linux: sets RLIMIT_AS via nix 0.28. Non-Linux: one-shot eprintln warning,
Ok(). `effective_rss_mib(manifest, default)` computes min with 0-unset guard.
Five finding subcode constants exported (FINDING_PATH_ESCAPE, …_DISABLED,
…_FRAME_OVERSIZE, …_ENTITY_CAP, …_OOM_KILLED) for Task 6 import.
Deferred: CLA-INFRA-PLUGIN-ENTITY-OVERRUN-WARNING — requires Filigree ingest
path (WP5/WP6); documented in limits.rs module doc.
Adds nix 0.28 and tempfile (dev) to clarion-core deps.
Test count: 74 → 94. All four ADR-023 gates clean.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e-exports, doc/API nits Applies code-review fixes on 1002d53: - CapExceeded.observed → would_reach (cumulative total that would be reached; the error format string now matches the field semantic). - Re-export all five FINDING_* subcode constants and DEFAULT_MAX_RSS_MIB from plugin/mod.rs so Task 6's host code references them consistently with the other limits types. - record_escape_at is now pub(crate); the test hook does not belong on the public API surface. - apply_prlimit_as uses std::io::Error::from directly (nix 0.28 provides the impl); the outdated comment about From<Errno> for i32 is removed. - BreakerState is #[non_exhaustive] to leave room for a future HalfOpen. - jail_02 uses assert! instead of a silent skip when /tmp is unreachable; path construction fixed to use sibling TempDir name components. - limits.rs module doc: §2b–§2d → §2a–§2d (PathEscapeBreaker is §2a). - Test doc for apply_prlimit_linux_returns_ok notes the hard-limit side effect on the test binary. Ref: code-review findings I-1, I-2, I-3, M-1..M-5 on 1002d53. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds plugin/discovery.rs implementing the UQ-WP2-01 resolution: scan \$PATH for clarion-plugin-* executables, then load each plugin's plugin.toml from either the neighbor directory (first) or the install-prefix share/clarion/plugins/<suffix>/plugin.toml (fallback). Returns Vec<Result<DiscoveredPlugin, DiscoveryError>> so a single malformed plugin cannot hide its siblings from discovery. DiscoveryError distinguishes ManifestNotFound, ManifestInvalid, and I/O failures with per-case context (path, source). Tests cover: neighbor hit, install-prefix fallback, missing-manifest error, malformed-manifest error, non-matching name rejection, non-executable skip, and \$PATH shadow (first match wins). Ref: ADR-021 §L9, docs/implementation/sprint-1/wp2-plugin-host.md §L9. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n discovery Applies code-review fixes on 28f5676: - find_manifest now uses fs::metadata() via a probe_manifest helper so permission-denied errors surface as DiscoveryError::Io rather than silently producing ManifestNotFound. Path::is_file() returns false on EACCES indistinguishably from actual absence, which was hiding operator misconfigurations. - seen_dirs is now a HashSet<PathBuf>; the BTreeSet ordering was never used and inconsistent with seen_names. - Doc polish: discover_on_path is marked as a test-first entry point, seen_names insert has a clarifying comment on the UTF-8 invariant, and path_os test helper gets a doc line. Ref: code-review findings I-1, I-2, M-1..M-3 on 28f5676. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ontology
Implements Task 6 of WP2:
* `crates/clarion-core/src/plugin/host.rs` — PluginHost<R,W> supervisor
- `spawn()` for subprocess launch with RLIMIT_AS on Linux (pre_exec)
- `connect()` for in-process testing against MockPlugin
- `handshake()` — validates manifest capabilities at connection time;
refuses reads_outside_project_root with UNSUPPORTED-CAPABILITY finding
- `analyze_file()` — four-stage ADR-021 §Layer 2 validation pipeline:
(1) ontology gate: undeclared entity kinds dropped + UNDECLARED-KIND finding
(2) identity gate: entity_id(plugin_id, kind, qname) ≠ returned id dropped
+ ENTITY-ID-MISMATCH finding (UQ-WP2-11)
(3) path-jail gate: escaped paths dropped + PATH-ESCAPE finding; breaker
trip after >10 escapes in 60 s kills plugin + DISABLED-PATH-ESCAPE
(4) entity cap: run-cumulative 500k cap kills plugin + EntityCapExceeded
- `shutdown()` / `take_findings()`
- unsafe block for pre_exec is the single documented unsafe use; workspace
lint changed from forbid to deny with justification comment
* `crates/clarion-plugin-fixture/` — minimal test-fixture binary
- Speaks JSON-RPC 2.0 over stdin/stdout; handles initialize/analyze_file/
shutdown/exit; returns `fixture:widget:demo.sample` for every file
* `crates/clarion-core/tests/host_subprocess.rs` — T1 integration test
(runtime binary discovery via CARGO_TARGET_DIR / target/debug search)
* `crates/clarion-core/src/plugin/mock.rs` — added UndeclaredKind,
IdMismatch, EscapingPath(usize) behaviours plus factory methods
* `crates/clarion-core/src/lib.rs` — re-export prune (ticket clarion-29acbcd042):
implementation types removed from crate root; only facade types exported
* Tests T2–T6 live in host.rs #[cfg(test)] using in-process MockPlugin;
T1 lives in host_subprocess.rs and spawns the real fixture binary
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
crates/clarion-core/src/plugin/host.rs::shutdown()'s doc-comment warned callers not to call it after analyze_file returned PathEscapeBreakerTripped or EntityCapExceeded (each of those paths already ran do_shutdown internally) — a defensive caller that shutdown()'d anyway got HostError::Transport(Io(BrokenPipe)) for writing to a closed pipe. Fix: add a `terminated: bool` field, set to true at the start of do_shutdown (so a partial-failure shutdown still flags the host unusable). shutdown() returns Ok(()) as a no-op when terminated is already set. CLI wrappers that defensively shutdown() after an analyze_file kill path are now safe. T8f exercises the exact sequence: 11 escaping entities trip the breaker (internal do_shutdown runs), then two user-visible shutdown() calls return Ok without further wire traffic. Closes clarion-f30acbbb31.
…(T8g) crates/clarion-core/src/plugin/host.rs::analyze_file used path.to_string_lossy() to serialise the file path into the JSON-RPC analyze_file request. Linux filenames are arbitrary byte sequences; invalid-UTF-8 bytes became U+FFFD on the wire, the plugin then couldn't open the path, and the symptom bubbled up as "plugin returned no entities" — indistinguishable from "no entities were found in this file." Fix: use path.to_str() and short-circuit when it returns None. The host pushes a new FINDING_NON_UTF8_PATH (subcode CLA-INFRA-HOST-NON-UTF8-PATH), returns Ok(vec![]), and never touches the wire. The file is "skipped" from the run's perspective — the diagnostic is attributed to the host layer where it belongs rather than to a plugin bug. T8g (Unix only, because constructing a non-UTF-8 Path needs OsStrExt::from_bytes) exercises the path; the plugin is not asked. Closes clarion-3ede63f83a.
…prep) crates/clarion-core/src/plugin/host.rs::handshake deserialised the initialize response as a raw serde_json::Value, never extracting InitializeResult.ontology_version. WP6 cache keying (ADR-007) depends on that field; a plugin sending a malformed or absent ontology_version would have produced no signal until cache code tried to read it. Fix: deserialise the result into InitializeResult (the type already exists in protocol.rs). Reject empty/whitespace-only ontology_version with HostError::Protocol. Store the accepted value on the new ontology_version: Option<String> field and expose it via a pub getter so WP6 can consume it. Existing mocks (mock.rs, fixture binary) already emit ontology_version, so no test data changes are needed. Structural- InitializeResult deserialise means any future schema drift surfaces at handshake rather than silently. Closes clarion-a508e3e232.
crates/clarion-core/src/plugin/host.rs previously read exactly one frame at each request-response boundary (handshake, analyze_file, do_shutdown) and aborted on id mismatch. A plugin that double-sent a response, or that queued a pre-baked frame before being signalled to shutdown, could: - Defeat the breaker-kill path: do_shutdown's single read picked up the stale frame, id-mismatched, and returned HostError::Protocol which was logged-and-swallowed (clarion-c08586a2da). The plugin stayed alive until Drop — which on Unix does not kill. - Convert per-call plugin misbehaviour into a full run abort with entities already committed via SoftFailed (clarion-ff2831eec0). Fix: new `read_response_matching(expected_id, method)` helper that loops read_frame, discarding stale frames (mismatched id or unparse- able) up to MAX_DRAIN_FRAMES (16), and surfaces warn-level tracing for each discard. handshake, analyze_file, and do_shutdown all now route through it. The bounded budget prevents a hostile plugin from forcing an unbounded read loop. Closes clarion-c08586a2da (shutdown id race) and clarion-ff2831eec0 (outstanding-id validation).
…ersal crates/clarion-core/src/plugin/host.rs::PluginHost::spawn previously did Command::new(&manifest.plugin.executable) — the host ran whatever path the manifest named. A compromised plugin.toml with executable = "/bin/sh" (or "../../evil" or "python3") would spawn that binary with the operator's uid, env, and cwd. ADR-021's hybrid-authority model says: the core enforces minimums against a semi-trusted plugin. "Run the binary we discovered, not an arbitrary path the manifest names" belongs in that minimum set. Fix: spawn now takes a separate `executable: &Path` argument (the PATH-discovered binary from DiscoveredPlugin.executable). Command::new uses that path; the manifest's `plugin.executable` field is validated to be a bare basename that matches the discovered binary's filename. A path separator in the manifest is refused (HostError::Spawn); a basename mismatch is refused too. Call sites updated: - crates/clarion-cli/src/analyze.rs: run_plugin_blocking now takes the discovered executable and passes it to spawn; the outer run() threads plugin.executable into the spawn_blocking closure. - host_subprocess.rs T1: no longer overrides manifest.plugin.executable with the full path; passes the fixture binary path as the spawn arg, keeps the manifest's bare-basename declaration. - host_subprocess.rs T9: builds a symlink whose basename matches the manifest but targets /bin/true. Pointing spawn at /bin/true directly would now fail the basename check before forking — which tests a different property than the hang-avoidance probe. New T10 (path-separator refused) and T11 (basename-mismatch refused) are the negative tests A.2.4 wanted for the L9 lock-in. Closes clarion-0ad8d35e02.
crates/clarion-core/src/plugin/host.rs::spawn's pre_exec closure applied only RLIMIT_AS. After exec, the plugin inherited the host's RLIMIT_NOFILE (often 1024 or 1M) and RLIMIT_NPROC — a plugin could fork workers, open thousands of sockets/files, mmap anonymous pages under the AS cap, or spawn threads without bound between initialize and the first analyze_file response. Fix: new apply_prlimit_nofile_nproc in crates/clarion-core/src/plugin/limits.rs, called from the same pre_exec closure alongside apply_prlimit_as. Defaults: - NOFILE=256 (plugin ingest pattern is open/parse/close one file at a time; 256 covers every legitimate concurrent-open scenario without fd-flooding the host's own logging and SQLite fds) - NPROC=32 (Sprint 1 plugins are single-threaded or use tight tokio pools; 32 is an order-of-magnitude ceiling that stops fork-bomb / thread-flood while accommodating legitimate thread-pool use) Same async-signal-safety analysis as apply_prlimit_as: setrlimit is AS-safe per POSIX.1-2017 §2.4.3; u64 captures are Copy; no allocation, no Drop, no unwind in the closure body. Plugins that need higher limits can negotiate via a manifest extension in a later sprint; the fixed defaults cover every planned v0.1 consumer. Closes clarion-4229114f01.
Five related test additions (one strengthening, four new) that close the A.2 signoff test gaps filed during the phase-3 scrub: - T2 strengthened: after capability refusal, assert that NEITHER "analyze_file" NOR "initialized" notification is written to the host writer. The initialized-refusal property was documented in A.2.12's signoff language but never verified. Closes clarion-5578157797. - analyze_file_error_payload_returns_protocol_error: a plugin returning a JSON-RPC error response to analyze_file surfaces as HostError::Protocol with the code and message preserved. Exercises the ResponsePayload::Error arm that the mock never produced. Closes clarion-e190f1e72b. - content_length_ceiling_surfaces_through_plugin_host: wire-level oversize response surfaces as HostError::Transport(FrameTooLarge) through PluginHost::analyze_file — A.2.3 asked for both positive and negative Content-Length tests and the prior tests covered only the transport layer in isolation. Closes clarion-58eb4567b6. - cross_plugin_plugin_id_spoof_is_rejected: plugin with manifest.plugin_id="mock" cannot emit id="python:function:stub". T4 covered the wrong-qualified-name case; this covers the highest-value identity-fabrication scenario (one plugin spoofing another's namespace). Closes clarion-e7789f2f76. - analyze_file_drains_stale_frames_before_matching_response: exercises the drain-until-match logic added in the earlier response-id-race fix. A stale id=999_999 frame is followed by the real response at the expected id; analyze_file succeeds and returns the entity. Proves the happy path through the new read_response_matching helper. Closes clarion-049bbe44ce. All five tests live in crates/clarion-core/src/plugin/host.rs alongside the existing T1..T9 series. No production change.
crates/clarion-core/src/plugin/host.rs::analyze_file previously did:
let result_val: Value = read_response_matching(...)?;
let entities_raw: Vec<Value> = result_val
.get("entities")
.and_then(|v| v.as_array())
.cloned() // <-- deep clone of the entities array
.unwrap_or_default();
for raw_val in entities_raw { ... }
At an 8 MiB frame holding ~80k small entities, the `.cloned()` step
walks and deep-copies the entire Value tree once, on top of the
already-parsed ResponseEnvelope copy. RSS amplification was ~2-3x
the frame size just in host memory.
Fix: deserialise the result body via
`serde_json::from_value::<AnalyzeFileResult>(result_val)` — the
struct already existed in protocol.rs with `entities: Vec<Value>`.
The Vec is now moved out, not cloned. Per-entity `from_value` +
FINDING_MALFORMED_ENTITY path is preserved (AnalyzeFileResult's
field stays Vec<Value> so one bad entity still produces a finding
rather than aborting the whole batch).
Closes clarion-a0070781cd.
crates/clarion-core/src/plugin/host.rs::spawn set stderr to Stdio::inherit(). A plugin writing gigabytes to stderr would: - flood the operator's terminal / CI log / syslog; - deadlock the plugin on write(2) if stderr is a pipe that isn't being drained (CI wrapper with small fd buffer, syslog backoff); - inject ANSI escapes or forged tracing-style log lines that look like they came from the host. Fix: Stdio::piped() for stderr; a detached drain thread reads each chunk into a per-host bounded ring buffer (64 KiB default). Oldest bytes are discarded on overflow, so the plugin never blocks on stderr writes regardless of volume. Thread exits cleanly on EOF (child closes stderr) or read error. The tail is NOT forwarded to the operator's stdout/stderr verbatim — callers read it via `host.stderr_tail() -> Option<String>` and attach to findings, which sanitises the escape-injection attack. PluginHost gains: - `stderr_tail: Option<Arc<Mutex<VecDeque<u8>>>>` field (Some on spawn-backed hosts, None for in-process connect()) - `stderr_tail()` pub accessor that returns lossy-UTF-8 String - `STDERR_TAIL_BYTES` pub constant (64 KiB) T9b verifies the ring is wired on subprocess-backed hosts. Closes clarion-fd8e3fe576.
crates/clarion-core/src/plugin/discovery.rs accepted any clarion-plugin-* binary in any $PATH directory with exec-bit set, ignoring the directory's ownership/mode. On a multi-user or shared- CI machine, any user with write access to a $PATH directory became a plugin installer — ADR-021's "operator deliberately installs plugins" premise was not enforced. Fix: check the directory's mode after the canonical-dedup step and before reading entries. If `mode & 0o002 != 0` (world-writable), emit DiscoveryError::WorldWritableDir and skip without scanning. Canonical installs at /usr/local/bin (0o755), ~/.local/bin (0o755), and pipx venv bins are all 0o755 in practice — only pathologically misconfigured dirs fail this check. Helper `is_world_writable` is unix-only; non-unix always returns false (matches existing policy that discovery is unix-only). T8 covers the refusal path with a 0o777 tempdir. Closes clarion-5fbee689a3.
Starting prompt for the next Claude Code session. WP2 is code- complete after the 2026-04-23/24 scrub; 21 issues closed across two push-through rounds; 5 WP2 issues remain open with documented deferral rationale. The handoff covers: - A.2.1–A.2.12 signoff state (all have code + discriminating tests except A.2.9 which is a doc walk-through). - Two work options for the next session: A.2 formal lock-in (~30 min) or WP3 kickoff (the bigger slice). - Caller-observable WP2 changes WP3 must know about: spawn signature now takes discovered executable, stderr is piped, ontology_version must be present, drain-until-match semantics, resource limits on the plugin child, entity field + extra caps, manifest size caps. - Do-not-reopen table for the 5 remaining WP2 issues, each with the advisor's deferral reason cited verbatim. - Files-of-interest map for WP2 (don't edit) and WP3 (what you'll create). - Session hygiene pulled from the prior handoff — one logical fix per commit, ADR-023 gates on every commit, specific git add paths, filigree workflow discipline. Supersedes docs/superpowers/handoffs/2026-04-23-wp2-full-scrub-handoff.md.
Ticks A.2.1-A.2.12 with `locked on 2026-04-24` stamps on the four load-bearing L-rows (L4 JSON-RPC method set + transport, L5 plugin.toml schema, L6 core-enforced minimums, L9 discovery convention) per the signoff meta rule (every L-row tick carries a locked-on date that makes revisiting the design require a follow-up ADR + cross-WP impact analysis). Non-L ticks (A.2.5-A.2.8, A.2.9-A.2.12) are acceptance ticks without the lock-in contract. Marks UQ-WP2-01, 02, 03, 06, 07, 08, 09, 11 as resolved in wp2-plugin-host.md section 5, each citing the implementing Task/test or scrub commit. UQ-04, 05, 10 were already resolved by ADR references. UQ-WP2-07 diverges from its original proposal: plugin stderr is captured into a bounded 64 KiB ring buffer with host.stderr_tail() for diagnostics (scrub commit b3c91a7), not teed to tracing::info! as originally proposed. The scrub narrowed the surface because an unbounded tee lets a chatty plugin flood core-side log drains. Closes clarion-9dee2d24c3 (WP2 umbrella). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First WP3 task — lays down the clarion-plugin-python package at plugins/python/ with the ADR-023 Python tooling floor in place from the first commit (ruff ALL-minus-pragmatic, mypy --strict, pytest, pre-commit). Bootstrap __main__.py just writes a version stamp to stderr and exits 0; Task 2 replaces it with the JSON-RPC server loop. Four ADR-023 gates all green locally: - ruff check plugins/python: All checks passed - ruff format --check plugins/python: 4 files already formatted - mypy --strict plugins/python: Success, no issues (4 source files) - pytest plugins/python: 2 passed, 89% coverage CI workflow extended with a python-plugin job running the same four gates against Python 3.11 (pyproject.toml floor). Rust job unchanged. Deviation from WP3 doc Task 1 file list: .pre-commit-config.yaml lives at repo root, not plugins/python/. Justification — pre-commit's per-repo install model resolves the config file at the repo root (where .git lives); a config under plugins/python/ would not be found by `pre-commit install` run from the repo root. Hooks are scoped to plugins/python/**/*.py via the `files:` pattern so the behaviour is functionally equivalent. UQ-WP3-04 resolved: Python floor is 3.11 (pyproject.toml requires-python = \">=3.11\", mypy python_version = \"3.11\", CI runs setup-python with python-version: \"3.11\"). UQ-WP3-10 resolved by ADR-023: ruff/mypy-strict/pytest/pre-commit adopted day-1. Refs clarion-cd84959ee9 (WP3 umbrella). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ships the JSON-RPC server loop speaking WP2's L4 method set over
Content-Length framing, plus the plugin-side stdout discipline resolving
WP2 UQ-WP2-08. Response shapes match the Rust host's typed
InitializeResult / AnalyzeFileResult / ShutdownResult contracts in
crates/clarion-core/src/plugin/protocol.rs exactly:
- initialize returns {name, version, ontology_version, capabilities};
ontology_version = "0.1.0" is non-empty per WP2 scrub commit 1ac32b1.
- analyze_file returns {entities: []} (Task 7 wires extractor output).
- shutdown returns {} (empty ShutdownResult, not null — Rust serde would
reject null → typed struct).
- initialized / exit are notifications.
stdout_guard.install_stdio() captures the real stdin/stdout byte streams
and replaces sys.stdout with a guard that raises StdoutGuardError on any
write. Tested via subprocess (in-process test would break pytest's own
output capture).
Three integration tests in test_server.py drive the binary via
`sys.executable -m clarion_plugin_python` (PATH-independent): handshake
round-trip, analyze_file-before-initialized → -32002, unknown method →
-32601. Two subprocess tests in test_stdout_guard.py verify the guard
fires on print() and that the returned bytes streams are usable.
Coverage reports 0% for server/stdout_guard/__main__ because pytest-cov
doesn't track subprocess coverage; ADR-023 does not gate coverage in
Sprint 1.
pyproject.toml ruff config extended:
- ANN401 and TRY003 added to global ignore (legitimate Any on JSON-RPC
payloads; short composed exception messages are fine).
- S108 and E501 added to tests/** per-file-ignores (fake /tmp paths in
test fixtures; long assert messages).
- mccabe.max-complexity = 15 and pylint.max-returns = 10 /
max-branches = 15 — dispatch loops naturally exceed defaults.
UQ-WP3-12 resolved: initialize response identity matches manifest
({name, version, ontology_version} populated from package __version__
and the plugin-side ONTOLOGY_VERSION constant).
Four ADR-023 Python gates green: ruff check, ruff format --check,
mypy --strict, pytest (6 passed).
Refs clarion-cd84959ee9 (WP3 umbrella).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ships qualname.reconstruct_qualname() — the static equivalent of Python's runtime-bound __qualname__ attribute. This is the L7 lock-in from wp3-python-plugin.md: Clarion's Python plugin and Wardline's REGISTRY must produce byte-identical qualified-name segments (ADR-018), and this function is the executable spec for what "match Python's __qualname__" means in Clarion. Algorithm: walk AST parent scopes from immediate-parent outward, prepending `parent.<locals>.` for function parents and `parent.` for class parents. The `<locals>` marker — inserted only for function parents — is what distinguishes a nested closure (`outer.<locals>.inner`) from a method (`Foo.bar`). Nine test cases cover the documented __qualname__ surface: - Module-level function: hello - Module-level async function: aloha - Nested function: outer.<locals>.inner - Class method: Foo.bar (UQ-WP3-01 shape) - Nested class method: Outer.Inner.method (UQ-WP3-01 explicit) - Function nested in class method: Foo.bar.<locals>.inner - Class in function in class method: Foo.bar.<locals>.Local.meth (asymmetry: <locals> appears once, only for the function parent) - @typing.overload method: Foo.bar on every overload (UQ-WP3-07) - Deeply nested functions: a.<locals>.b.<locals>.c UQ-WP3-01 (nested class methods) and UQ-WP3-07 (overloaded methods) both resolved here; golden strings taken from the Python language reference. Also bumps pre-commit pins to match local tooling versions to eliminate format churn: ruff-pre-commit v0.6.9 → v0.15.11, mirrors-mypy v1.11.2 → v1.20.2. Without the bump, local `ruff format` and pre-commit's ruff-format hook disagreed on how to wrap long assert messages. Four ADR-023 Python gates green: 15 tests passed. Refs clarion-cd84959ee9 (WP3 umbrella). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new modules:
- entity_id.py — Python-side mirror of crates/clarion-core/src/entity_id.rs.
Same 3-segment `{plugin_id}:{kind}:{canonical_qualified_name}` format,
same ADR-022 grammar validation (`[a-z][a-z0-9_]*` for plugin_id/kind;
no-colon + non-empty for canonical_qualified_name), same ordering of
empty → colon → grammar checks so error classes match the Rust side
where overlapping cases exist (e.g. `py:thon` fires
SegmentContainsColonError before GrammarViolationError, as in Rust).
Task 5 will feed the shared fixtures/entity_id.json file through this
function and assert byte-for-byte parity with the Rust assembler.
- extractor.py — AST walker emitting one entity per FunctionDef /
AsyncFunctionDef. Entity shape matches the Rust fixture plugin's wire
layout: {id, kind, qualified_name, module_path, source_range}.
qualified_name = dotted_module + "." + __qualname__ (L7 reconstruction
via qualname.reconstruct_qualname).
UQs resolved by this task:
- UQ-WP3-02: SyntaxError → empty list + stderr log (run continues).
- UQ-WP3-05: module_dotted_name strips a leading `src/` segment.
- UQ-WP3-06: `pkg/__init__.py` → dotted `pkg` (module_path stays literal).
- UQ-WP3-11: empty/comment-only file → zero entities.
25 new tests (13 entity_id + 12 extractor), all passing. 100% line
coverage on both new modules.
Also adds pytest>=8.0 to pre-commit's mypy hook additional_dependencies —
the hook's isolated venv would otherwise fail to resolve
`pytest.CaptureFixture` annotations used by test modules.
Four ADR-023 Python gates green: 40 tests passed across the plugin.
Refs clarion-cd84959ee9 (WP3 umbrella).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…parity
Resolves UQ-WP3-08 by shipping /fixtures/entity_id.json at the repo root
with 20 representative {plugin_id, kind, canonical_qualified_name,
expected_entity_id} rows. Both sides consume it:
- crates/clarion-core/src/entity_id.rs gains
`shared_fixture_byte_for_byte_parity` — reads the JSON via
CARGO_MANIFEST_DIR, deserialises into a typed FixtureRow struct, and
asserts entity_id() produces the expected string on every row.
- plugins/python/tests/test_entity_id.py gains
`test_matches_shared_fixture` — reads the same JSON via a 4-parents-up
path resolution, loops over the rows, and asserts entity_id()
produces the expected string on every row.
The fixture covers: module-level function, class method, nested
function with <locals>, deeply nested function, function-in-class-
method, class-in-function-in-class-method (single <locals> at function
boundary), nested class method (no <locals> between class parents),
single-component name, snake_case, embedded digits, deep dotted
packages, the future `python:module:*` kind, core file entities
(top-level, nested, __init__.py), core subsystem hashes, and
hypothetical go/rust plugin entries.
Retroactively earns signoffs A.1.4: the WP1 tick claimed
`passes all rows in /fixtures/entity_id.json` against a file that
didn't exist yet. Now the fixture does exist, both test suites consume
it, and the A.1.4 proof pointer is truthful.
Also: fix pre-commit mypy hook to use `pass_filenames: false` so the
whole plugin tree is type-checked as a unit — the previous "check only
staged files in isolation" mode couldn't resolve intra-package imports
because the hook's isolated venv doesn't install the plugin package.
Rust clippy-pedantic + nextest + doc + deny all green.
Python: 41 tests passed (four ADR-023 gates green).
Refs clarion-cd84959ee9 (WP3 umbrella).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements wardline_probe.probe(min_version, max_version) — the L8 lock-in
per WP3 §L8. Returns one of three states matching the WP3 doc:
- {"status": "absent"} — wardline not installed (ImportError on
wardline.core.registry), or __version__ missing/non-string/invalid.
- {"status": "enabled", "version": "X.Y.Z"} — imported, version in
half-open range [min_version, max_version).
- {"status": "version_out_of_range", "version": "X.Y.Z"} — imported
but version outside the declared range.
The probe is deliberately fail-soft. Missing or out-of-range Wardline
does not fail the plugin — the integration is simply disabled in the
handshake's capabilities.wardline field, and the plugin proceeds to
extract entities normally. Sprint 1 does not consume REGISTRY; that
work lands in WP3-feature-complete per ADR-018.
server.handle_initialize wires the probe into the InitializeResult
capabilities field, using module-level WARDLINE_MIN_VERSION = "0.1.0"
and WARDLINE_MAX_VERSION = "0.2.0" constants that match the Task 7
plugin.toml [integrations.wardline] values.
New runtime dep: `packaging>=24` (for semver comparisons via
packaging.version.Version). This is the plugin's first runtime
dependency beyond the stdlib.
Eight failing tests written first, then impl:
- probe absent when registry import fails
- probe enabled in-range; at lower bound (inclusive)
- probe out-of-range at upper bound (exclusive); above upper bound
- probe absent when __version__ missing / not a string /
not valid semver
test_server.py::test_initialize_roundtrip relaxed to accept any of the
three legal wardline status values, since the test environment may or
may not have wardline installed.
Resolves UQ-WP3-03 (the probe runs against real `pip install wardline`
in dev venv; no stub fallback — symbol existence verified pre-sprint).
Four ADR-023 Python gates green: 49 tests passed; 100% coverage on
wardline_probe.py.
Refs clarion-cd84959ee9 (WP3 umbrella).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 7 lands the L5-compliant plugin.toml and the end-to-end wiring from analyze_file request → extractor → typed AnalyzeFileResult shape. plugins/python/plugin.toml follows the WP2 L5 schema exactly: - [plugin]: plugin_id="python", executable="clarion-plugin-python" (bare basename per WP2 scrub commit eb0a41d — host refuses any path component), extensions=["py"]. - [capabilities.runtime]: expected_max_rss_mb=512 (CPython+imports comfortably fit), wardline_aware=true, reads_outside_project_root=false (v0.1 rejects true at initialize). - [ontology]: entity_kinds=["function"] (Sprint 1 narrow scope), rule_id_prefix="CLA-PY-" (CLA-{plugin_id_upper}-* per ADR-022), ontology_version="0.1.0" (ADR-007 cache keying). - [integrations.wardline]: min_version="0.1.0", max_version="0.2.0" matching the server.py WARDLINE_* constants. Hatchling shared-data routes plugin.toml into <install-prefix>/share/clarion/plugins/clarion-plugin-python/plugin.toml where WP2 L9 discovery's install-prefix fallback finds it. Verified: after `pip install -e plugins/python[dev]`, `<venv>/share/clarion/plugins/clarion-plugin-python/plugin.toml` exists. server.handle_analyze_file now reads the file, extracts entities, and returns the typed AnalyzeFileResult envelope. The host sends absolute paths (crates/clarion-cli/src/analyze.rs canonicalises project_root and builds entries via entry.path()), so the plugin captures project_root from the initialize handshake and relativises incoming file_paths before passing them to extractor.extract (resolves UQ-WP3-05 by plugin-side relativisation rather than host-side as originally proposed). Handler signature refactor: handlers now take (params, state) instead of just (params). ServerState grows a `project_root: Path | None` field. dispatch() threads state through. New integration test test_analyze_file_returns_extracted_entities proves the full handshake → analyze → entity list pipeline: writes a demo.py to tmp_path, sets project_root=tmp_path at initialize, calls analyze_file with the demo's absolute path, asserts the returned entity ids are {python:function:demo.hello, python:function:demo.Foo.bar}. Four ADR-023 Python gates green: 50 tests passed. Refs clarion-cd84959ee9 (WP3 umbrella). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 8's round-trip proof: spawn the installed `clarion-plugin-python`
entry-point binary, complete the handshake, analyze the plugin's own
extractor.py, assert the expected entities come back.
Uses the entry-point binary (resolved via sysconfig.get_path("scripts"))
rather than `sys.executable -m`, so the full pip-install chain —
console_script generation, shebang rewriting, PATH placement — is
exercised end-to-end. pytest.skip gracefully if the binary is missing
so the test is friendly to someone running tests before
`pip install -e`.
Project_root = plugins/python/src in the handshake, which lets the
plugin relativise extractor.py to clarion_plugin_python/extractor.py
and produce qualified names like
`clarion_plugin_python.extractor.extract` — the expected L7 shape for
src-rooted packages.
Assertions:
- Four known functions appear: module_dotted_name, extract, _walk,
_build_entity (private AST-traversal helpers emit too — Sprint 1
has no public/private filter).
- Every entity carries kind="function" and
module_path="clarion_plugin_python/extractor.py".
- Graceful shutdown sequence returns ack {} then exit code 0.
Four ADR-023 Python gates green: 51 tests passed.
Refs clarion-cd84959ee9 (WP3 umbrella).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e fix
Task 9's Sprint 1 walking-skeleton demo — runs the README §3 command
sequence end-to-end and asserts the final sqlite3 output matches the
locked L2 format.
Exercises the full spine in one run:
- cargo build --workspace --release
- pip install -e plugins/python[dev]
- clarion install (L1 schema + .clarion/ creation)
- clarion analyze . (L4 transport + L5 manifest + L6 minimums +
L7 qualname + L8 probe + L9 discovery)
- sqlite3 .clarion/clarion.db → python:function:demo.hello|function
(L2 entity-ID format + L1 schema).
Test script at tests/e2e/sprint_1_walking_skeleton.sh with env
overrides (REPO_ROOT, VENV, CARGO_BUILD) for CI vs local invocation.
CI workflow gains a `walking-skeleton` job depending on both rust
and python-plugin jobs, installing sqlite3 and running the script
against a freshly-built workspace.
### Entity shape fix (surfaced by running the walking skeleton)
The host expects `{id, kind, qualified_name, source: {file_path, ...}}`
(crates/clarion-core/src/plugin/host.rs:132-154's RawEntity/RawSource
structs). The extractor was emitting `module_path` and `source_range`
at the top level, missing the required `source` object — the host
rejected every entity with CLA-INFRA-PLUGIN-MALFORMED-ENTITY
("missing field `source`"). Every Python-side unit test passed because
the host wasn't in the loop; the walking skeleton is precisely the
test that catches this class of silent wire-contract drift.
Restructured entity shape now matches the host's contract:
{
"id": "...",
"kind": "function",
"qualified_name": "...",
"source": {
"file_path": "<absolute path, for jail check>",
"source_range": {"start_line": ..., ...}
}
}
extract() now takes separate `file_path` (goes on wire verbatim, hits
host jail) and `module_prefix_path` (used for qualified-name dotting)
arguments. server.handle_analyze_file passes the host-provided
absolute path as file_path and the project-root-relative form as
module_prefix_path. Decoupling is necessary because the host sends
absolute paths (CLI canonicalises project_root and walks via
entry.path()) while the dotted qualified_name must derive from the
project-relative form to produce `python:function:demo.hello` rather
than `python:function:tmp.clarion-demo-xyz.demo.hello`.
Hatch shared-data target also corrected from
`share/clarion/plugins/clarion-plugin-python/` to
`share/clarion/plugins/python/` — discovery.rs strips the
`clarion-plugin-` prefix before computing the share/ subdir basename
(clarion-core/src/plugin/discovery.rs:252), so only the plugin_id
basename matches.
52 Python tests passing (one new test covers the new
module_prefix_path kwarg). Walking skeleton script exits 0 with the
expected sqlite3 output.
Refs clarion-cd84959ee9 (WP3 umbrella).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WP3 gate close: - wp3-python-plugin.md §5: 9 outstanding UQ-WP3-* marked resolved (01 nested class methods, 02 SyntaxError handling, 05 module-path normalisation, 06 __init__.py collapse, 07 typing.overload, 08 shared fixture, 09 stderr logging, 11 empty file, 12 initialize identity). UQ-WP3-03/04/10 were already resolved pre-task; now the entire §5 reads as "all 12 resolved". UQ-WP3-05's resolution documents the plugin-side relativisation deviation from the original host-side proposal. - signoffs.md §A.3: all 10 boxes ticked (A.3.1–A.3.10) with `locked on 2026-04-24` stamps on L7 (A.3.2 qualname) and L8 (A.3.3 Wardline probe). Each proof line cites the specific test file / commit hash rather than a generic assertion. - signoffs.md §A.4: all 3 boxes ticked (A.4.1 walking-skeleton script, A.4.2 sqlite3 output, A.4.3 no-regression). Proof points to `tests/e2e/sprint_1_walking_skeleton.sh` passing end-to-end at commit `7e7a85b`. - signoffs.md §A.6: A.6.2 (ADR-005 + ADR-023 both Accepted in the index) and A.6.3 (README §4 L-rows all stamped) ticked. A.6.1 (v0.1-plan.md WP sections narrowed) explicitly deferred to the Sprint 1 close issue (clarion-30ca615264) as it cross-cuts WP1/WP2/WP3 rather than belonging to any single WP close. - README.md §4: L4, L5, L6, L7, L8, L9 rows all carry `(locked on 2026-04-24)` stamps, matching signoffs.md and satisfying A.6.3's "every L-row" requirement. No code changes — this is purely the WP3 documentation close. The Sprint 1 close issue (clarion-30ca615264) remains open and now has exactly two items to close out: A.5 (cross-product verification: A.5.1/A.5.2 trivially true, A.5.3 needs a `pip install wardline` probe run, A.5.4 needs a no-divergence doc note) and A.6.1 (v0.1-plan.md narrowing). Both are small doc-only checks. Refs clarion-cd84959ee9 (WP3 umbrella — closed by this commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sprint 1 final close. Three remaining Tier-A items resolved:
A.5 (cross-product stance):
- A.5.1 / A.5.2 trivially satisfied — no Filigree changes; no Wardline
changes (only the version pin was bumped on the Clarion side, which
is not a Wardline edit).
- A.5.3: bumped plugin.toml [integrations.wardline] pin from
0.1.0/0.2.0 to 1.0.0/2.0.0 to match Wardline's actual current
version. Pre-sprint placeholder values were never re-checked when
Wardline shipped 1.0.0 between sprint kickoff and close. Probe now
returns {"status": "enabled", "version": "1.0.0"} against
`pip install -e /home/john/wardline` in the dev venv. server.py
WARDLINE_MIN/MAX_VERSION constants kept in sync with the manifest.
- A.5.4: real divergence found between Clarion's L7 qualname format
(combined `{dotted_module}.{__qualname__}` string) and Wardline's
FingerprintEntry storage (separate `module: str` + `qualified_name:
str` fields, where module is the file path and qualified_name is
bare __qualname__). Same information, different decompositions —
joining requires a translator on the Wardline side that composes
via Clarion's module_dotted_name rules. Documented in
wp3-python-plugin.md §L7 with a Sprint-1-close-dated callout, and
tracked for ADR-018 amendment in clarion-889200006a (P3, scoped to
sprint:2 / wp:9 — the join is not exercised in Sprint 1, only by
the WP9 cross-product work).
A.6 (documentation hygiene):
- A.6.1: each of WP1, WP2, WP3 sections in v0.1-plan.md gains a
"Sprint 1 delivery (narrow walking-skeleton scope)" callout naming
the L-rows shipped and the deferred items, with pointers to the
per-WP sprint-1 doc and the corresponding signoffs.md anchor. The
rest of each section continues to describe the v0.1 completion
target — the callout localises the "what's in Sprint 1 vs what's
later" question to the section heading.
- A.6.2 and A.6.3 were ticked at the WP3 close (commit a5444de).
Tier A is now fully ticked. Sprint 1 closes; clarion-30ca615264
ready for delivered-state transition.
52 Python tests + 175 Rust tests + walking-skeleton still green at
this commit.
Refs clarion-30ca615264 (Sprint 1 close — closed by this commit),
clarion-889200006a (ADR-018 amendment trigger).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![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: 553239c2d9
ℹ️ 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
+901
to
+902
| let candidate = Path::new(&raw.source.file_path); | ||
| let jailed = match jail_to_string(&project_root, candidate) { |
There was a problem hiding this comment.
This jail check passes raw.source.file_path directly into jail_to_string, so relative paths are canonicalized against the host process working directory instead of project_root. If a plugin legitimately emits project-relative paths (as allowed by the protocol) and the user runs clarion analyze /path/to/project from another directory, valid entities will be flagged as escapes and can eventually trip the path-escape breaker.
Useful? React with 👍 / 👎.
Comment on lines
+533
to
+537
| if let Err(e) = host.shutdown() { | ||
| tracing::warn!( | ||
| plugin_id = %plugin_id, | ||
| error = %e, | ||
| "best-effort host shutdown failed; falling back to kill()", |
There was a problem hiding this comment.
When host.shutdown() errors on the success path, the code only logs and kills the child but still returns Ok(BatchResult) because work_result remained Ok. That lets a plugin that becomes unresponsive during shutdown be counted as successful, so runs can be marked completed even though the protocol exchange failed.
Useful? React with 👍 / 👎.
Comment on lines
+585
to
+586
| if signal == 9 || signal == 11 { | ||
| findings.push(HostFinding::oom_killed(plugin_id, signal)); |
There was a problem hiding this comment.
This logic treats any SIGKILL/SIGSEGV as oom_killed, but the host itself calls child.kill() on several non-OOM paths (e.g., shutdown failure and other error handling). As a result, host-initiated kills are logged as OOM findings, which misdiagnoses failures and can hide the true cause in operator diagnostics.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements the Sprint 1 “walking skeleton” across the Rust core (storage + plugin host) and the Python plugin, and wires CI + an end-to-end script to prove clarion analyze persists a real Python function entity into SQLite.
Changes:
- Adds Rust workspace/tooling baseline (toolchain, fmt/clippy/deny/nextest, CI) and storage layer (schema migrations, writer actor, reader pool, integration tests).
- Adds plugin-host plumbing and a fixture plugin for subprocess integration testing, plus shared cross-language
EntityIdparity fixture. - Adds the Python plugin package (JSON-RPC server, stdout guard, extractor/qualname/entity-id, Wardline probe) with a dedicated Python CI job and a walking-skeleton e2e script.
Reviewed changes
Copilot reviewed 82 out of 89 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/sprint_1_walking_skeleton.sh | End-to-end demo: build core, install python plugin, run analyze, assert sqlite output |
| rustfmt.toml | Rustfmt baseline (edition 2024, width 100, Unix newlines) |
| rust-toolchain.toml | Toolchain components pinned for CI/dev (rustfmt/clippy/llvm-tools) |
| plugins/python/tests/test_wardline_probe.py | Unit tests for Wardline presence/version probe |
| plugins/python/tests/test_stdout_guard.py | Subprocess tests enforcing “stdout is JSON-RPC only” |
| plugins/python/tests/test_server.py | Subprocess integration tests for JSON-RPC framing + methods |
| plugins/python/tests/test_round_trip.py | Installed-entrypoint round-trip self-analysis test |
| plugins/python/tests/test_qualname.py | Qualname reconstruction tests matching Python __qualname__ semantics |
| plugins/python/tests/test_package.py | Package smoke test for version pin |
| plugins/python/tests/test_extractor.py | Extractor unit tests (AST → function entities) |
| plugins/python/tests/test_entity_id.py | Python EntityId tests + shared fixture parity check |
| plugins/python/tests/init.py | Marks tests package (empty) |
| plugins/python/src/clarion_plugin_python/wardline_probe.py | Wardline probe implementation used in initialize capabilities |
| plugins/python/src/clarion_plugin_python/stdout_guard.py | Installs guarded sys.stdout to prevent framing corruption |
| plugins/python/src/clarion_plugin_python/server.py | Python JSON-RPC server loop + dispatch + framing |
| plugins/python/src/clarion_plugin_python/qualname.py | L7 qualname reconstruction helper |
| plugins/python/src/clarion_plugin_python/py.typed | PEP 561 marker for typed package |
| plugins/python/src/clarion_plugin_python/extractor.py | AST walker emitting function entities + source ranges |
| plugins/python/src/clarion_plugin_python/entity_id.py | Python-side 3-segment EntityId assembler w/ validation |
| plugins/python/src/clarion_plugin_python/main.py | Console entry point calling server main() |
| plugins/python/src/clarion_plugin_python/init.py | Package init + __version__ |
| plugins/python/pyproject.toml | Python packaging + tooling config (ruff/mypy/pytest) |
| plugins/python/plugin.toml | Plugin manifest (ontology/capabilities/integrations) |
| plugins/python/README.md | Plugin dev install + tooling gates + references |
| fixtures/entity_id.json | Shared Rust/Python fixture for byte-for-byte EntityId parity |
| docs/superpowers/handoffs/2026-04-23-wp2-scrub-findings.md | WP2 scrub findings + dispositions log |
| docs/implementation/v0.1-plan.md | Sprint-close notes added per WP (WP1/WP2/WP3) |
| docs/implementation/sprint-1/wp1-scaffold.md | Updates toolchain/tooling baseline narrative + task ledger text |
| docs/implementation/sprint-1/README.md | Sprint 1 lock-in table updates + ADR-023 link |
| docs/clarion/adr/README.md | ADR index updated (adds ADR-005, ADR-023) |
| docs/clarion/adr/ADR-005-clarion-dir-tracking.md | New ADR defining .clarion/ git-tracking policy |
| deny.toml | cargo-deny v2 policy (licenses/advisories/bans/sources) |
| crates/clarion-storage/tests/writer_actor.rs | Writer-actor integration tests (commit cadence, rollback, self-heal) |
| crates/clarion-storage/tests/schema_apply.rs | Migration 0001 verification tests (tables/indexes/triggers/view/fts) |
| crates/clarion-storage/tests/reader_pool.rs | Reader pool concurrency + error/panic behavior tests |
| crates/clarion-storage/src/writer.rs | Writer actor implementation (batch commits, protocol) |
| crates/clarion-storage/src/schema.rs | Schema migration runner |
| crates/clarion-storage/src/reader.rs | Reader pool wrapper over deadpool-sqlite |
| crates/clarion-storage/src/pragma.rs | Connection PRAGMA application helpers |
| crates/clarion-storage/src/lib.rs | Storage crate exports |
| crates/clarion-storage/src/error.rs | Storage error type |
| crates/clarion-storage/src/commands.rs | Writer command protocol + data records |
| crates/clarion-storage/migrations/0001_initial_schema.sql | Migration 0001 (full schema shape + fts + view + generated cols) |
| crates/clarion-storage/Cargo.toml | Storage crate manifest |
| crates/clarion-plugin-fixture/src/main.rs | Minimal JSON-RPC fixture plugin binary for subprocess host tests |
| crates/clarion-plugin-fixture/src/lib.rs | Empty lib target for workspace hygiene |
| crates/clarion-plugin-fixture/Cargo.toml | Fixture crate manifest |
| crates/clarion-core/tests/host_subprocess.rs | Subprocess tests for PluginHost spawning + manifest checks |
| crates/clarion-core/tests/fixtures/sample.mt | Fixture source file for host subprocess test |
| crates/clarion-core/tests/fixtures/plugin.toml | Fixture plugin manifest for tests |
| crates/clarion-core/src/plugin/mod.rs | Plugin module facade + re-exports |
| crates/clarion-core/src/plugin/jail.rs | Path jail implementation + tests |
| crates/clarion-core/src/plugin/breaker.rs | Crash-loop breaker + tests |
| crates/clarion-core/src/llm_provider.rs | Stub LlmProvider trait + noop provider |
| crates/clarion-core/src/lib.rs | Core crate exports + re-export policy |
| crates/clarion-core/src/entity_id.rs | Rust EntityId implementation + shared fixture parity test |
| crates/clarion-core/Cargo.toml | Core crate manifest |
| crates/clarion-cli/tests/wp1_e2e.rs | WP1 end-to-end smoke test (install + analyze skipped) |
| crates/clarion-cli/tests/install.rs | clarion install integration tests |
| crates/clarion-cli/tests/analyze.rs | clarion analyze integration tests (no plugins + failrun path) |
| crates/clarion-cli/src/main.rs | CLI entry point |
| crates/clarion-cli/src/install.rs | Install command implementation + ADR-005 gitignore content |
| crates/clarion-cli/src/cli.rs | Clap CLI definitions |
| crates/clarion-cli/Cargo.toml | CLI crate manifest |
| clippy.toml | Clippy thresholds tuned for the repo |
| Cargo.toml | Workspace config, shared deps, shared lints, edition 2024 |
| .pre-commit-config.yaml | Pre-commit hooks for ruff + mypy on python plugin |
| .gitignore | Repo-level ignores for Rust/Python/IDE/coverage artifacts |
| .github/workflows/ci.yml | CI: Rust gates + python-plugin gates + walking-skeleton job |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+97
to
+98
| | L7 | Python qualified-name production format (third segment of L2) _(locked on 2026-04-24)_ | WP3 | [`wp3-python-plugin.md#l7--python-qualified-name-production-format`](./wp3-python-plugin.md#l7--python-qualified-name-production-format) | `↗` ADR-018 — Filigree triage and Wardline annotations key off this | | ||
| | L8 | Wardline `REGISTRY` direct-import + version-pin protocol _(locked on 2026-04-24)_ | WP3 | [`wp3-python-plugin.md#l8--wardline-registry-import--version-pin-protocol`](./wp3-python-plugin.md#l8--wardline-registry-import--version-pin-protocol) | `↗` Wardline — once pinned, `wardline.core.registry.REGISTRY` cannot be renamed without a coordinated bump | |
There was a problem hiding this comment.
The lock dates for L7/L8 here conflict with the WP3 close date elsewhere in this PR (e.g. v0.1-plan + wp3-python-plugin.md both reference 2026-04-28). If L7/L8 were locked on 2026-04-28 (per the sprint close), update these two rows to avoid a misleading timeline in the sprint README table.
Comment on lines
+41
to
+43
| - name: doc | ||
| run: cargo doc --workspace --no-deps --all-features | ||
|
|
There was a problem hiding this comment.
CI's doc step runs cargo doc without denying warnings, but the PR description/test plan calls out cargo doc -D warnings. RUSTFLAGS=-D warnings does not apply to rustdoc lints; use RUSTDOCFLAGS=-D warnings (or equivalent) so doc warnings fail the build as intended.
Comment on lines
+45
to
+52
| let table_exists: Option<String> = conn | ||
| .query_row( | ||
| "SELECT name FROM sqlite_master WHERE type='table' AND name='schema_migrations'", | ||
| [], | ||
| |row| row.get(0), | ||
| ) | ||
| .ok(); | ||
| if table_exists.is_none() { |
There was a problem hiding this comment.
read_applied_versions suppresses all rusqlite errors from the sqlite_master probe via .ok(), which can hide real corruption/permission issues and cause the migrator to behave as if no migrations were applied. Consider matching explicitly on rusqlite::Error::QueryReturnedNoRows (treat as "not present") and propagating any other error.
Comment on lines
+95
to
+103
| let table_exists: Option<String> = conn | ||
| .query_row( | ||
| "SELECT name FROM sqlite_master WHERE type='table' AND name='schema_migrations'", | ||
| [], | ||
| |row| row.get(0), | ||
| ) | ||
| .ok(); | ||
| if table_exists.is_none() { | ||
| return Ok(0); |
There was a problem hiding this comment.
applied_count uses the same .ok() pattern on the sqlite_master lookup, which will also treat unexpected SQLite errors as "table missing" and return Ok(0). To avoid masking real DB failures, consider propagating unexpected errors and only returning Ok(0) for the specific "table not found / no rows" case.
Two real environmental failures surfaced by the first PR-driven CI run:
### 1. clarion-plugin-fixture binary missing in CI
`wp2_e2e_smoke_fixture_plugin_round_trip` and the crash-isolation test
both fall back to searching `target/{debug,release}/` for the fixture
binary when `CARGO_BIN_EXE_clarion-plugin-fixture` isn't set. nextest's
propagation of CARGO_BIN_EXE_* for cross-package dev-dep binaries is
not reliable in the CI environment, so the binary search fails and the
tests panic before any test logic runs.
Fix: add `cargo build --workspace --bins` step before nextest in CI so
the fallback path always finds a built fixture binary on disk. This is
defensive — even if nextest's env-var setting is fixed in a future
release, the prebuild keeps the test from being toolchain-version-
dependent. Long-term replacement is `clarion-adeff0916d` (deferred WP2
issue: have the test build its own binary via escargot/build.rs).
### 2. World-writable PATH dirs trip WP2's refusal in CI
GHA `ubuntu-latest` ships with several world-writable directories on
PATH (`/usr/local/bin`, `/opt/pipx_bin`, `/usr/local/.ghcup/bin`).
WP2 scrub commit `7c0e396` made discovery refuse world-writable PATH
entries, and analyze.rs treats discovery refusal as a fail-run error.
Tests that inherit the runner's PATH therefore fail.
Two test fixes:
- `wp1_e2e.rs::wp1_walking_skeleton_end_to_end` — adds `.env("PATH",
"")` to both clarion install and clarion analyze invocations.
Matches the existing pattern in `tests/analyze.rs::analyze_without_
plugins_writes_skipped_run_row` (introduced in scrub commit `ad054bd`
for the same reason). The test asserts `skipped_no_plugins`, so an
empty PATH is the correct envelope.
- `wp2_e2e.rs` (two tests with synthetic-PATH construction) — drop the
`chain(env::split_paths(¤t_path))` part of the PATH builder.
The tests only need the test-controlled tempdir(s) on PATH; inheriting
the runner's PATH was a latent CI/dev-asymmetry bomb. The third
wp2_e2e test (broken-plugin case) already used only test dirs and
was unaffected.
Local verification: `cargo build --workspace --bins` + `cargo nextest
run --workspace --all-features` → 175/175 passed; clippy + fmt + doc
+ deny + Python gates + walking-skeleton script all green.
Co-Authored-By: Claude Opus 4.7 (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
clarion analyzepersistspython:function:demo.hello|functionend-to-end.clarion-9dee2d24c3), WP3 (clarion-cd84959ee9), Sprint 1 (clarion-30ca615264).What's in this branch (58 commits since main)
WP2 phase 3 scrub (~21 commits before the sprint close): drain-until-match response loop, ContentLengthCeiling::DEFAULT enforcement at fixture,
plugin.tomlsize +[integrations.*]entry caps, ProtocolError field truncation, RawEntity/RawSource extra bounds, structural double-shutdown guard, non-UTF-8 path refusal, ontology_version handshake validation, RLIMIT_NOFILE/NPROC, manifest-executable bare-basename refusal, stderr ring buffer, world-writable$PATHrefusal. Seedocs/superpowers/handoffs/2026-04-23-wp2-scrub-findings.mdfor triage details.WP3 baseline (10 TDD-disciplined commits):
ruffALL,mypy --strict,pytest,pre-commit)__qualname__fixtures/entity_id.jsonconsumed byte-for-byte by Rust + Pythonplugin.tomlmanifest +analyze_filewired to extractorSprint 1 close (3 doc commits): A.2 signoffs locked, A.3/A.4 signoffs locked, Tier A complete (A.5 cross-product + A.6 doc hygiene).
Real divergence found at A.5.4
Wardline's
FingerprintEntrystores(module, qualified_name)as separate fields; Clarion's L7 emits a combined dotted string. Same information, different decompositions — joining requires a translator. Pre-emptive trigger filed atclarion-889200006afor ADR-018 amendment when WP9 attempts the first cross-product join. Not blocking this sprint — the join is not exercised in Sprint 1.Test plan
rustjob green:cargo fmt --check,clippy --all-targets -D warnings,nextest --workspace --all-features,cargo doc -D warnings,cargo deny checkpython-pluginjob green:ruff check,ruff format --check,mypy --strict,pytestagainstplugins/python/walking-skeletonjob green:tests/e2e/sprint_1_walking_skeleton.shruns the README §3 demo end-to-end and asserts the sqlite3 output is exactlypython:function:demo.hello|functionLocal at HEAD
553239c: 175 Rust tests + 52 Python tests + walking-skeleton all green.🤖 Generated with Claude Code