v1.0: agent orientation — install --skills/--hooks/--all, hook session-start, MCP instructions/resources/prompt (PR 6/x)#21
Merged
Conversation
- Introduced CodexCliConfig and ClaudeCliConfig structs to manage configurations for Codex and Claude CLI providers. - Updated LlmConfig to include new CLI configurations and modified default implementations. - Enhanced LlmProviderKind enum to support new Codex CLI and Claude CLI providers. - Implemented provider selection logic for Codex CLI and Claude CLI, requiring live opt-in for usage. - Added tests to validate the behavior of Codex CLI and Claude CLI providers under various configurations. - Updated documentation to include details on configuring and using Codex CLI and Claude CLI providers. - Enhanced usage statistics to track cached input tokens for better performance metrics.
Add Clarion WP10 HTTP read endpoints, federation contract artifacts, Sprint 3 closeout docs, and WP5 secret scanner review fixes.
…ementation/
Move scattered planning, review, and agent-handoff material into a single
labeled archive under docs/implementation/ so the release-facing tree
(docs/{suite,clarion/v0.1,clarion/adr,operator,federation}/) only surfaces
canonical material:
docs/clarion/v0.1/plans/ -> docs/implementation/v0.1-scope-plans/
docs/clarion/v0.1/reviews/ -> docs/implementation/v0.1-reviews/
docs/superpowers/handoffs/ -> docs/implementation/handoffs/
docs/superpowers/plans/ -> docs/implementation/agent-plans/
docs/arch-analysis-2026-05-18-1244/
-> docs/implementation/arch-analysis-2026-05-18-1244/
Update path references in retained docs (suite/, clarion/adr/*, clarion/v0.1/*.md,
clarion/{README,adr/README}.md, docs/README.md) to point at the new locations.
Fix internal cross-references in moved docs whose depth changed. Patch two
pre-existing broken refs in agent-plans/ while in the area.
Rewrite docs/implementation/README.md as the archive index. Update
docs/clarion/v0.1/README.md, docs/clarion/README.md, and docs/README.md to
describe the archive as non-normative supporting context.
Refresh stale "Clarion: not yet built" facts in docs/suite/briefing.md to
reflect the Sprint-1 walking-skeleton ship state (tag v0.1-sprint-1).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ADR-013: harden pre-ingest secret scanner contracts, audit evidence, MCP briefing-block signals, and operator docs. Verification: cargo fmt --all -- --check; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo build --workspace --bins; cargo nextest run --workspace --all-features; RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features; cargo deny check; bash tests/e2e/sprint_1_walking_skeleton.sh; git diff --check.
…docset
Touch only retained release-facing docs (docs/{suite,clarion/v0.1,clarion/adr}/);
no decision changes. Edits are status-line, link-target, or path-citation
corrections reflecting state since the 2026-04-18 design baseline.
Suite:
- loom.md §9: Clarion status was "Designed; implementation not yet started" —
update to walking-skeleton-merged / v0.1-build-in-flight to match the
briefing.md fix.
ADR cross-references that were "(pending)" for ADRs since Accepted:
- ADR-011 → ADR-005 (now Accepted)
- ADR-012 → ADR-005 (now Accepted)
- ADR-021 → ADR-012 (now Accepted)
ADR body-text path citations updated to the centralized archive layout:
- ADR-024: "docs/superpowers/plans/ and docs/clarion/v0.1/reviews/"
→ "docs/implementation/ (agent-plans/, v0.1-reviews/)"
- ADR-026: "docs/superpowers/handoffs/..." → "docs/implementation/handoffs/..."
- ADR-015: inline-code v0.1-scope-commitments.md path → new archive path
- ADR-023, adr/README.md: stale link display-text matched to relocated URL.
v0.1 design docset:
- system-design.md §12 "Writing cadence": ADR backlog list was 2026-04-18
baseline state; reflect that ADR-023–ADR-032 are post-baseline and live
in the ADR index, ADR-008 is Superseded.
- detailed-design.md §11: add coverage note that the navigation table reflects
baseline ADR-001–ADR-022 only; refer post-baseline ADRs to the index.
- requirements.md, detailed-design.md preambles: add ADR-README pointer;
label pre-restructure reviews as archived.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reconciles CON-FILIGREE-02 against the Sprint 2 scope amendment (WP9-B + WP10 deferred to v0.2). The constraint is rewritten to describe v1.0's shadow-registry mode and forward-points to the amendment. The reqs that the amendment defers also get a deferral banner inline so future readers see the v1.0/v2.0 split at the requirement boundary, not just in the amendment memo: - REQ-FINDING-03/04/05/06 (Filigree-side findings + dedup + SARIF) - REQ-INTEG-FILIGREE-01/02/03/04/05 (cross-product wire surfaces) Closes filigree clarion-066f90d0dc. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Closes the v1.0 publish track (clarion-443320f078 Thread 1: B.1, B.2, C.2,
C.3, WS-D). The external-operator smoke test surfaced four publish-blocker
bugs in the documented happy path; all are fixed in this commit.
Docs / release ergonomics
- README.md at repo root (B.1, clarion-380250e6ed). v1.0 framing, the
seven MCP tools as a table, quick-start, project layout, contributing
pointer.
- docs/operator/getting-started.md (B.2, clarion-459e186028 +
C.3 clarion-6150779248). Single-flow tutorial: install, analyse a
small repo, connect MCP client, ask three pre-scripted questions,
verify secret-block. Plugin-discovery PATH guidance folded in.
- ADR-033 — v1.0 distribution via GitHub Releases (Linux x86_64 +
macOS x86_64/aarch64; promote to crates.io/PyPI at v2.0). Windows is
explicitly out (setrlimit is Unix-only).
- .github/workflows/release.yml — tag-triggered binary matrix + Python
sdist + SHA256s + auto-generated notes. workflow_dispatch supported
for rc1 validation without a tag push.
- LICENSE (MIT) materialised at repo root.
Source fixes surfaced by WS-D (clarion-3e0e481ef7)
1. install.rs: the `clarion.yaml` stub used Rust `\<newline>` string
line-continuations, which strip leading whitespace — the file shipped
unindented and YAML-invalid. Switched to raw newlines + explicit
indentation with a leading comment warning against the pattern.
2. writer.rs / writer_actor.rs: `clarion analyze` was not idempotent —
a second invocation against an unchanged corpus crashed with
`UNIQUE constraint failed: entities.id`. The entity-insert path now
UPSERTs on `id`, preserving `created_at` + `first_seen_commit` and
refreshing the rest from the latest run's record. Matches the
`INSERT OR IGNORE` discipline already in `insert_edge`. New
regression test: `insert_entity_is_idempotent_across_runs`.
3. discovery.rs: pipx-installed plugins live behind a symlink in
`~/.local/bin/` pointing to `~/.local/share/pipx/venvs/<pkg>/bin/`,
with the manifest under that venv's `share/clarion/plugins/...`.
Plain install-prefix lookup from `~/.local/bin/` looked at
`~/.local/share/` and missed the manifest entirely. Added a
symlink-resolved install-prefix fallback that canonicalises the
executable and retries the lookup under the resolved prefix.
4. cli.rs: `clarion analyze --help` text still claimed "Sprint 1: no
plugins are loaded". Rewritten to describe v1.0 behaviour, including
the idempotency contract and the `skipped_no_plugins` exit-0
diagnostic.
Version + attribution
- Workspace + path-deps + plugins/python: 0.1.0-dev → 1.0.0.
- OpenRouter attribution `referer` defaults: qacona/clarion →
tachyon-beep/clarion across crates + docs + tests.
Closes filigree clarion-380250e6ed, clarion-459e186028,
clarion-6150779248, clarion-25f5ff9de0, clarion-3e0e481ef7.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- AGENTS.md: per-repo agent instructions anchor written by `filigree doctor` on first invocation under v2.0.3. - .mcp.json: auto-reformatted by filigree-mcp (whitespace normalisation + trailing newline + dropped redundant empty `env`); no functional change.
C1 — HTTP /api/v1/files now refuses briefing-blocked entities. resolve_file surfaces briefing_blocked from properties_json via the new entity_briefing_block_reason helper; http_read::get_file returns 404 + an audit tracing::warn! instead of leaking the entity_id/content_hash. Storage contract covered by three query_helpers tests. The federation read surface is now consistent with MCP summary/callers_of_inferred which already gate on this property. Integration test for the HTTP surface is filed separately (clarion-5d6c01fe1a) — the spawn-serve harness hangs on the blocked-entity codepath, root cause TBD. C2 — Sidecar walk for .env/.env.* files now logs each walk error and emits a tail count, mirroring analyze.rs::collect_source_files. Filtering walk errors with filter_map(Result::ok) hid ACL/perm/dangling-symlink failures from the security gate. C3 — Claude CLI provider no longer falls back to raw stdout when no `result` event or `structured_output`/`structuredOutput`/`result` field is present. Both the event-selection step and claude_structured_output_json now return InvalidResponse (retryable) so callers do not persist event payloads as summaries. C4 — docs/operator/secret-scanning.md no longer references a non-existent `--no-llm` flag; the real control is `llm.allow_live_provider: false` or leaving `CLARION_LLM_LIVE` unset, both of which keep the recording provider as the only dispatch path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
I13 — secret-scan finding-anchor `file_content_hash` now logs a tracing::warn! naming the path + error when fs::read fails. The anchor still lands without a content_hash but the audit trail records why, including the hint that entity briefing-block lookups may fail for that path. I14 — `parse_codex_jsonl_usage` no longer silently swallows malformed JSONL lines. Each malformed line logs a warn with the parse error and an 80-char snippet; a tail-warn reports the total skipped count and flags that `session_token_ceiling` enforcement will diverge from true accounting until those lines are recoverable. I15 — `plugin::host::apply_briefing_block` now removes any plugin-supplied `briefing_blocked` entry from `raw.extra` before optionally inserting the scanner-authored one. The scanner is the sole authority for this property; a malicious or buggy plugin cannot unblock an entity by emitting `"briefing_blocked": <anything>` and cannot over-block by injecting a reason the scanner did not assign. I16 — `CodexCliProvider::from_config` and `ClaudeCliProvider::from_config` now validate the configured `executable` via `which::which` at construction time. A typo in `clarion.yaml` aborts at `clarion serve` startup with `LlmProviderError::Cli` (non-retryable) rather than exploding on the first MCP request. New workspace dependency: `which = "6"`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`pre_ingest` now emits the per-detection `CLA-SEC-SECRET-DETECTED` finding unconditionally — both when the operator overrides and when the file is blocked. The override finding `CLA-SEC-UNREDACTED-SECRETS-ALLOWED` remains additive, recording the operator decision and the detection list as evidence, but no longer replaces the per-(rule,file,line) audit row. ADR-013 §"Override — --allow-unredacted-secrets" specifies this shape: "each affected file emits `CLA-SEC-UNREDACTED-SECRETS-ALLOWED` (severity ERROR)" *in addition to* the per-detection `CLA-SEC-SECRET-DETECTED`. The prior code suppressed `CLA-SEC-SECRET-DETECTED` in the override branch and hid the audited population from `filigree list --rule-id=CLA-SEC-SECRET-DETECTED` — precisely the query a security review would run. Tests updated: - `non_tty_override_confirmed_allows_briefing_and_records_stats` now asserts `secret_count == 1` alongside `override_count == 1`. - `baseline_suppression_and_override_admission_are_audited_together` now asserts `secret_count == 1` (the live overridden detection; the baseline-suppressed fixture contributes 0). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
I6 — `baseline_does_not_suppress_when_hash_drifts_at_same_line` pins the "baseline benign at line 1, file mutated to a *different* secret at line 1" case. Suppression keys on (hashed_secret, line_number, rule_type); the regression net was missing. I7 — `high_entropy_hex_fires_on_lockfile_shas_but_baseline_suppresses_them` documents the v0.1 path for HighEntropyHex false positives. Lockfile integrity hashes and git SHA-1s fire the rule (40-char hex / entropy ≥ 3.0); tightening the rule would let real low-entropy secrets through, so the accepted resolution is the operator-baseline workflow already shipped under ADR-013. The fixture asserts both fixtures suppress cleanly via baseline. I10 — Five new unit tests for CLI provider failure paths: - claude_cli_parser_rejects_empty_stdout_as_retryable_invalid_response - claude_cli_parser_rejects_non_json_stdout - claude_cli_parser_refuses_raw_stdout_when_no_structured_output_or_result_event (the §C3 contract: stdout without a `result` event or `structured_output`/`structuredOutput`/`result` field returns InvalidResponse, not a persisted raw payload) - claude_cli_parser_accepts_result_event_with_string_result_payload - cli_status_retryable_treats_signal_kill_as_retryable - cli_status_retryable_treats_clean_nonzero_exit_as_non_retryable I11 — `tests/e2e/wp5_secret_scan.sh` now exercises four new scenarios beyond the happy path: baseline-suppression flow, confirmed `--allow-unredacted-secrets` admission (asserting both SECRET_DETECTED and UNREDACTED-SECRETS-ALLOWED land per §I5), malformed-baseline abort (exit non-zero, no runs row), retry-after-baseline-add (operator workflow: block → commit baseline → re-run unblocks). I12 — `McpConfig::from_yaml_str` rejects configs that name both `llm` and `llm_policy` at the top level with a new `ConfigError::AmbiguousLlmKey` (code `CLA-CONFIG-AMBIGUOUS-LLM-KEY`). Serde-norway aliases silently pick one, which was the classic copy-paste-migration pitfall the review flagged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Project rule: name the function, not the work-package label. WP-task tags leak from the implementation tracker into operator-facing surfaces and into comments that will rot once WP5 is closed. Replaced: - docs/operator/secret-scanning.md: "WP5 audit surface" → "scanner audit surface" - crates/clarion-cli/src/main.rs: ".env-supplied RUST_LOG ... before WP5's pre-ingest secret gate runs" → "... before the pre-ingest secret scanner runs" Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…a885) rustfmt: wrap the 80-char `br#"..."#` literal in the InvalidResponse parser test onto a continuation line. clippy::doc-markdown: backtick the (hashed_secret, line_number, rule_type) tuple identifiers in the baseline-drift test docstring. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resolve_file now fails closed when no cataloged kind='file' entity exists for the source path, instead of deriving a file identity from module/function anchors.
ADR-003 forbids '@' in entity identifiers, so Clarion must not manufacture core:file:{hash}@{path} IDs. The federation read path should return the stored file entity ID or no file.
Resolve deleted-on-disk catalog rows from stored entity data without requiring candidate file canonicalization. Propagate hash fallback read failures instead of returning an empty content_hash when the catalog has no hash. Prefer catalog/plugin and extension language inference before caller-supplied language fallback.
CONTRACT-3 of the Loom federation read API: stop masking briefing-blocked
file entities as 404 NOT_FOUND. Filigree needs to distinguish "Clarion does
not know this file" (404) from "Clarion knows this file but refuses to
expose it" (403) — the second case is actionable (escalate / wait for
re-scan) while the first is a registry-coverage gap.
Wire surface:
GET /api/v1/files?path=blocked.py
-> 403 Forbidden
-> {"error": "entity is briefing-blocked and cannot be exposed",
"code": "BRIEFING_BLOCKED"}
The 403 envelope deliberately does not include entity_id, content_hash,
canonical_path, or language, so Filigree cannot infer file identity from
a refused entity (the whole point of the block).
The closed error-code enum gains BRIEFING_BLOCKED and (in anticipation of
CONTRACT-2 bearer auth) UNAUTHORIZED. The federation fixture
`get-api-v1-files.demo-python.json` adds a `briefing_blocked_403` example
and widens the error_envelope status_any and code enum.
Side note: the `tracing::warn!` originally written inline at the 403
branch hangs the HTTP handler because the file's tracing convention is to
route every handler-side event through `HTTP_ERROR_DISPATCH` via
`tracing::dispatcher::with_default(...)`; a bare macro emits through the
process-wide global dispatcher and (under axum's TraceLayer-created span)
deadlocks until the request times out. The new `log_briefing_blocked_refusal`
helper follows the file's existing wrap pattern (see `log_read_server_error`).
This commit also bundles in-flight parallel-session work that was on the
working tree when the briefing-blocked change landed and that the contract
fix transitively depends on:
- `CanonicalProjectPath` newtype on `ResolvedFile.canonical_path` (proven
project-relative POSIX string at the type level).
- `InstanceId` newtype around the project UUID, replacing String at
load/serve boundaries.
- `ReaderPool::identity()` and `HttpReadServer::readers_identity` so a
refactor that re-opens the pool inside `run_http_read_server` fails
loud rather than silently splitting reader state. The identity is
captured *inside* the HTTP thread and shipped back to the supervisor
via a new `HttpReadReady` ready-signal payload.
- `CatchPanicLayer` + `format_dyn_error_chain`-backed panic for
unenumerated middleware `BoxError` variants, so a future regression
surfaces as a hard panic-then-500 rather than a silent string-formatted
500.
These were WP5 type-design follow-ups (clarion-2533594d79) running in a
parallel session; bundling here was the lowest-friction path to keep the
RC1 working tree green before tomorrow's federation prototype.
Tests:
- serve_http_files_endpoint_returns_briefing_blocked_for_blocked_entity
(new) asserts 403 + BRIEFING_BLOCKED code + absence of identity fields
from the response body.
- serve_http_responses_match_federation_fixture_contracts (existing) now
also validates the new `briefing_blocked_403` example end-to-end via
the fixture-driven walker (seeded via the new
`seed_briefing_blocked_file_entity` helper).
- Full clarion-cli serve suite: 22/22 passing.
Refs: ADR-014, Filigree task clarion-18b7c476b4 (C-2).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…quest Clarion-authored request memo for the Filigree-side work that the REQ-FINDING-05/-06 lifecycle tail (clarion-dd29e69e0e) depends on: a `scan_source`-scoped finding retention/prune surface (unblocks `--prune-unseen`) plus a scan-run-create contract decision. Placed under docs/federation/ (not filigree-side/, which is a read-only mirror of Filigree-authored artifacts per its README). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Implements the `--resume RUN_ID` path of the WP9-B finding-emission lifecycle tail (clarion-dd29e69e0e). Resuming reuses the prior run instead of starting a fresh one and re-emits to Filigree without flipping the prior run's findings to `unseen_in_latest`. Storage: - New `WriterCmd::ResumeRun` reopens an existing `runs` row (`UPDATE … SET status='running', completed_at=NULL`) instead of `INSERT`, which conflicted on the run PK — the documented reason resume was more than flag-wiring. Errors on an unknown run id. - `InsertFinding` is now idempotent (`ON CONFLICT(id) DO UPDATE`): a resume re-walk regenerates the same run-scoped finding ids and would otherwise fail on `UNIQUE constraint: findings.id` (only entities upserted before). The conflict clause refreshes analysis-derived columns and preserves `created_at` + the lifecycle columns (status/suppression_reason/filigree_issue_id), which are never mutated locally. CLI: - `run_lifecycle::open_run` centralises begin-vs-resume across all three run-opening call sites in analyze.rs. - Public `--resume RUN_ID` flag (conflicts_with the hidden `--run-id`); threads `mark_unseen=false` into Phase-8 emission and records it in `stats.json`. - Secret-scan findings now get **deterministic, run-scoped ids** (blake3 over anchor entity + rule + evidence) instead of a random UUID, so a resume re-walk upserts the same row rather than inserting a duplicate. Without this, only the phase3 producer was resume-safe; the secret producer would duplicate every secret finding on each resume. Tests: writer-level resume reuse / unknown-id / finding-idempotency; CLI end-to-end against a mock Filigree asserting mark_unseen true→false and a single `runs` row; and a secret-corpus resume test asserting no duplicate finding and a stable id across the fresh+resume pair. Docs: contracts.md (resume implemented; scan-run-create posture pending Filigree confirmation), requirements.md REQ-FINDING-05, CHANGELOG [Unreleased]. Still deferred under clarion-dd29e69e0e (both genuinely external, no speculative client built): the Phase-0 scan-run create handshake (a contract question for Filigree) and `--prune-unseen` (REQ-FINDING-06), blocked on a Filigree prune/retention route that does not yet exist — request filed at docs/federation/2026-05-30-prune-unseen-filigree-request.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…G-06) Completes the REQ-FINDING-06 piece of the WP9-B lifecycle tail (clarion-dd29e69e0e). After emission (Phase 8b), `clarion analyze --prune-unseen` asks Filigree to soft-archive its own stale `unseen_in_latest` Clarion findings. This corrects an earlier wrong conclusion: the prior session asserted "no Filigree prune route exists" and filed a request memo asking Filigree to build one. The route already exists — `POST /api/loom/findings/clean-stale` — verified against Filigree's handler (dashboard_routes/files.py) and its API tests. So this is a client against a real, verified surface, not a speculative one. - clarion-mcp: `CleanStaleRequest`/`CleanStaleResponse`, `clean_stale_url` (the loom route, distinct from the classic /api/v1 emission intake), and `FiligreeHttpClient::post_clean_stale`. - config: `integrations.filigree.prune_unseen_days` (default 30). - cli/analyze: `--prune-unseen` flag; Phase 8b runs after emission for the same non-hard-failed outcomes, scoped to `scan_source=clarion` (server-enforced, so it can only touch Clarion's findings). Result folded into `stats.json` (`filigree_prune`). Semantics — soft-archive, not delete: Filigree moves `unseen_in_latest` → `fixed` (auto-reopens on reappearance; ADR-015). Enrich-only: a Filigree outage or the integration being disabled is recorded in stats, never fails the run. Prune keys on `unseen_in_latest`, which only normal `mark_unseen=true` runs create — a `--resume` (mark_unseen=false) run produces no unseen state, so prune does not interact with resumes. Tests: clean-stale wire-shape unit tests (clarion-mcp) and three CLI e2e tests against a mock Filigree — post-after-emission (asserts the loom path + scan_source scoping), enrich-only degrade when unreachable, and disabled no-op. Docs: contracts.md pins the clean-stale route; requirements.md REQ-FINDING-06 marked implemented (with the soft-archive note); CHANGELOG; the request memo carries a withdrawal banner (prune ask moot, route exists), leaving only its §4 scan-run-create question live. Also corrected the --resume doc clause that falsely cross-linked prune as the fix for resume-stale findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Verified the clarion-workflow skill against the live MCP surface in clarion-mcp/src/lib.rs: all 18 tool names, behavioral descriptions, and input-arg schemas are accurate and current. The one genuine gap was the `clarion://context` resource (live entity/subsystem/finding counts + index freshness) — part of the MCP interface but undocumented in the skill that the server also serves verbatim as the `clarion-workflow` prompt. Out of scope (left for the navigation skill): CLI-only analyze flags (--resume, --prune-unseen) are not part of the MCP query surface. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…2.1.0 Suite-wide refresh of the embedded Filigree guidance to match the running 2.1.0 binary (filigree:instructions hash bump 857eb216 → 9dff6e6d): - "Ready ≠ startable": type-specific working statuses (tasks → in_progress, features → building, bugs walk triage → confirmed → fixing); start-work on a triage bug returns INVALID_TRANSITION and start-next-work skips it; --advance / advance=true walks the soft transitions. - New get_ready startable flag + next_action hint. - delete_issue / delete-issue documented as irreversible admin (undo_last cannot reverse it). - Batch-op envelope: newly_unblocked present only when non-empty. Touches AGENTS.md, CLAUDE.md, and the .agents/ filigree-workflow skill + references. No Clarion code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…liation design Design spec for Clarion's consume side now that Wardline SP4 emits findings into Filigree (scan_source=wardline, metadata.wardline.qualname, no entity_id). Flow B (designed): read-time lazy reconciliation in issues_for / orientation_pack. Key insight — metadata.wardline.qualname IS the entity_id's segment-3 qualname, so the match is a local catalog lookup, not the deferred resolve oracle. Enrich-only; file-scoped fetch + local qualname match honoring the normative divergence traps. One external dependency: a Filigree findings-read route, filed as a contract ask. Flow A (re-scope only): the NG-25 probe migration's external blocker has lifted (Wardline SP2d shipped the descriptor); 1f6241b329 / 22acf15fd7 become actionable. Deliverable per directive: spec + tracked issues, design now / build next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… contract ask Verified against Filigree source: GET /api/loom/findings already filters on scan_source + file_id and ScanFindingLoom carries metadata (wardline.qualname), and POST /api/v1/files:resolve (path→file_id) is already a consumed contract. So Flow B composes two existing routes — no new Filigree route, no federation request. Avoids repeating the withdrawn prune ask (7a93883). Flow B issue is therefore Clarion-side build only, not gated on any sibling work. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two test-only concerns surfaced while validating the WP9-B findings-federation branch (787b754, b4e1b96, 46fd825, 4e0f85c): - Isolation fix: analyze_failrun_exits_nonzero_with_run_row_marked_failed prepended its broken-plugin dir to the ambient PATH, so on a machine with a real clarion-plugin-python installed, discovery found it, the run completed, and the test saw "Unexpected success". It passed in CI only because CI has no plugin on PATH. Build the child PATH from the broken-plugin dir alone (the FailRun branch needs that dir present, so PATH="" would wrongly yield skipped_no_plugins) — mirrors the single-dir pattern already used by the analyze_resume_/analyze_prune_ tests. Now deterministic regardless of ambient install state. - Hardening (review nits, no production defects): * analyze_prune_unseen_posts_clean_stale_after_emission now asserts the request body carries "older_than_days":30 — the live Filigree route silently ignores a `days` field, so this pins the wire contract. * analyze_does_not_emit_when_emit_findings_false: enabled+!emit_findings makes zero scan-results POST (egress opt-in guard). * analyze_prune_unseen_is_best_effort_on_non_2xx: a 500 from clean-stale keeps the run completed and records CLA-INFRA-FILIGREE-UNREACHABLE (enrich-only). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ot files:resolve files:resolve is a route Clarion EXPOSES (returns Clarion entity_ids), wrong direction. Verified path: GET /api/loom/files?scan_source=wardline&path_prefix= (path->Filigree file_id, exact-path filter) then GET /api/loom/findings? scan_source=wardline&file_id=. Both existing loom read routes. Core match (python:function:<wardline.qualname>) confirmed against entity_id.json parity fixture for functions/methods (dotted, <locals> preserved). Co-Authored-By: Claude Opus 4.8 <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: a8c5ccca7b
ℹ️ 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".
Six TDD tasks turning the approved spec into bite-sized steps: wire types + parsers, pure qualname reconcile module, Filigree two-hop client fetch, wardline_ findings section in issues_for + orientation_pack, and the contracts.md pin. Enrich-only throughout; hermetic tests (injected lookup + mock TcpListener). Implements clarion-71f995b88a. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… P1) source_for_entity read and returned indexed bytes even when the entity's file was marked briefing_blocked by the pre-ingest scanner, bypassing the secret-redaction policy the summary/HTTP read surfaces already enforce. Guard with briefing_block_reason() before any file read; return source_status 'briefing_blocked' + reason, no bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4655512c80
ℹ️ 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".
…1 P1) analyze_start spawned clarion analyze inheriting the server's stdout; the child inits tracing at info, so its non-framed progress bytes could interleave with MCP JSON-RPC responses and corrupt the client. Redirect the child stdout+stderr to Stdio::null() (progress already goes via --progress-file). Test proves the child's fd 1 resolves to /dev/null, not an inherited pipe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…#21 P1) build_call_sites read the owner entity's file to populate line_text even when that owner's file was marked briefing_blocked, so requesting call_sites for an edge involving a secret-bearing owner disclosed source bytes the pre-ingest scanner withholds. resolve_owner now carries the owner's block state; blocked owners skip the file read entirely and emit line_text="" with briefing_blocked=true (edge structure preserved, content withheld). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…e 0001 edit (PR#21 P2) briefing_blocked was added in place to migration 0001. Existing databases already stamped at schema_migrations.version=1 skip 0001 on apply, so they never gained the generated column or index — project_status then hit 'no such column: entities.briefing_blocked'. Move the ALTER + partial index to a new migration 0002 and bump CURRENT_SCHEMA_VERSION to 2 so the runner applies it on upgrade. Test asserts the column is absent after 0001 alone and present after the runner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…speth (PR#21 P2) .mcp.json started the Clarion server with --path /home/john/elspeth, a personal path; anyone opening this repo would analyze the wrong tree. Point it at the repo root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ripple from shipping briefing_blocked as migration 0002: install + wp1 e2e tests asserted exactly one migration / MAX(version)=1; now expect both migrations (count 2, versions [1,2], MAX=2). Renamed the install test off the 0001-specific name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
https://github.com/tachyon-beep/clarion/blob/666979933ea079f37beef70a94e614790276aa71/crates/clarion-cli/src/analyze.rs#L1006-L1011
When a later analyze produces a different cluster for a module, this only inserts the new in_subsystem edge; the writer's INSERT OR IGNORE path never removes that module's old in_subsystem edge or the old subsystem entity. The read side then has multiple memberships for the same module, and subsystem_of_entity orders by sub.to_id and returns one arbitrarily stale match, so re-indexing after dependency changes can report obsolete subsystem membership until the DB is rebuilt.
ℹ️ 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".
Migration 0002 ran its ALTER + CREATE INDEX as autocommit statements, while apply_one records schema_migrations.version=2 only after execute_batch returns. An interruption (or a failure on the second statement) after the ALTER left the DB with briefing_blocked present but no version-2 row; the next startup reran the ALTER and died on a duplicate-column error, blocking upgrade. Mirror 0001: wrap the DDL and the version-row INSERT in one BEGIN/COMMIT so they commit atomically. apply_one's INSERT OR IGNORE then no-ops. Test injects a failure after the ALTER (squat the index name so CREATE INDEX collides) and asserts the column does not survive on a fresh connection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Collaborator
Author
Review thread disposition (chatgpt-codex-connector)Worked through all 21 threads. Verified each against HEAD ( Fixed on this branch
Note on Note on the hook Stale / overtaken by events (no code change needed)
Deferred — tracked in
|
Three defects in the new agent-orientation install feature, found in the codex review of PR #21. All latent at 1.0 (the feature has never shipped), fixed before merge per request. - #5 shell-escape the embedded hook path: the SessionStart command embedded the project path in double quotes, which does not neutralize `$`, backtick, or `\`. Add POSIX single-quote escaping (shell_single_quote) so the path is always a single literal argument. Test round-trips metacharacters through a real `sh`. - #10 refresh a stale Clarion hook: the idempotency check keyed on the HOOK_COMMAND substring and no-oped whenever any matching hook existed, so a pre-existing path-less hook (or one pinned to another project) was never updated on re-install. Now: exact-match -> no-op; Clarion-owned but different -> refresh in place; absent -> append. Idempotency preserved. - #6 reject a non-directory .clarion under --all: when .clarion existed as a regular file and a non-bare init ran without --force, install skipped DB creation and "succeeded" with skills/hooks atop a project with no clarion.db. Now bails with a clear non-directory error. Test asserts the failure (previously exited 0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6585a951cf
ℹ️ 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".
| // the primary ID and was explicitly called out as non-conforming in | ||
| // ADR-003. The path itself is the canonical identity. | ||
| let relative = display_relative(project_root, file); | ||
| format!("core:file:{relative}") |
There was a problem hiding this comment.
When a scanned sidecar path contains a colon (valid on Unix, e.g. config:prod.env), this direct format! stores core:file:config:prod.env, which EntityId::from_str rejects because the canonical-qualified-name segment contains the reserved : separator. Normal source-file anchors already call entity_id("core", "file", ...) and fail rather than persisting malformed IDs, so sidecar-only secret findings can leave invalid entity IDs that downstream code/consumers cannot parse under ADR-003; use the same assembler or reject/escape unsupported paths here.
Useful? React with 👍 / 👎.
| .map(str::trim) | ||
| .filter(|path| !path.is_empty())?; | ||
| let mut finding = Map::new(); | ||
| finding.insert("path".to_owned(), json!(path)); |
There was a problem hiding this comment.
When integrations.filigree.emit_findings is enabled, this serializes entities.source_file_path directly into the scan-results path, but those rows are stored as jailed/canonical absolute paths. The pinned federation contract sends project-relative paths (for example src/auth/tokens.py) and Filigree's dedup key includes path, so the same finding from two checkouts is keyed separately as /home/... vs /workspace/... and local checkout paths are leaked to the sibling. Convert the stored path to project-relative before building the request.
Useful? React with 👍 / 👎.
Comment on lines
+70
to
+71
| - name: cross-workspace version lockstep | ||
| run: python scripts/check-workspace-version-lockstep.py |
There was a problem hiding this comment.
In the release workflow's verify job, this jumps from the migration guard to workspace-version, but the CI Rust job also runs the pyright pin lockstep, Wardline version bounds, and entity-cap ADR/code lockstep guards. Since release.yml is triggered by tag pushes while ci.yml only runs for main branch pushes and PRs, a directly tagged main commit can still publish artifacts even though one of those release-critical guards would fail; add the missing guard steps to this verify job so it actually mirrors the CI floor it claims to enforce.
Useful? React with 👍 / 👎.
Comment on lines
+5008
to
+5010
| let content = file_content | ||
| .entry(path.to_owned()) | ||
| .or_insert_with(|| std::fs::read_to_string(path).ok()); |
There was a problem hiding this comment.
When a file changes after indexing, call_sites still reads the current file at the stored byte offsets and returns line_text with no content-hash check. Those offsets belong to the indexed version, so after even a small edit before the call site this can show an unrelated current line while presenting it as evidence for the stored edge; source_for_entity and summary reads already refuse drift for the same reason. Verify the owner entity's current hash (or suppress/flag line_text) before reading from disk here.
Useful? React with 👍 / 👎.
tachyon-beep
added a commit
that referenced
this pull request
May 30, 2026
Follow-ups from PR #21 review: canonicalise SessionStart hooks + harden tests
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
Gives Clarion Filigree-parity agent orientation so a consult-mode agent self-orients the moment it lands on a project — instead of re-grepping the tree. Stacked on
v1.0/05-smoke-automation(clean, 27 commits, zero divergence); retarget tomainafter #17 merges. Tracks epicclarion-8fe3060d4c.Built in 6 TDD phases (failing test → minimal code → green → commit), each independently gated and reviewed (spec-compliance + code-quality) before the next:
clarion install --skills— a bundledclarion-workflowSKILL.md compiled into the binary viainclude_str!(no new deps;cargo denyclean) and installed atomically (stage→rename) into.claude/skills/+.agents/skills/, fingerprint-aware (content-based drift detection, idempotent).project_snapshot(clarion-mcp) — one infallible function (counts + index staleness vs ingested source mtimes) consumed by both the hook and the MCP resource. Failures fold toStaleness::Unknown, never error.clarion hook session-start— fail-soft (always exits 0): resyncs the skill on drift, opens.clarion/clarion.dbread-only, prints live entity/subsystem/finding counts + a freshness-appropriate nudge.clarion install --hooks/--all— non-clobbering, atomic (temp+rename) merge of a SessionStart hook into.claude/settings.json.--all= init + skills + hooks, and is re-runnable (skips init when.clarion/exists, preserving the index).initializenow returns aninstructionsfield and advertisesprompts/resources;clarion://contextresource serves the live snapshot;clarion-workflowprompt serves the skill text (single source of truth viainclude_str!of the same SKILL.md). Bareclarion installsemantics unchanged (still refuses-if-exists).Two dogfood gaps documented in the skill and filed as
release:1.1follow-ups under the epic (find_entitykind filter; module→subsystem reverse lookup), plus a staleness-stat perf note for large repos.Test Plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo nextest run --workspace --all-features— 541 passed, 2 skippedRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-featurescargo deny check(confirms no new dependency)bash tests/e2e/sprint_2_mcp_surface.sh(assertsinitializeinstructions +clarion://contextread over the wire)bash tests/e2e/sprint_1_walking_skeleton.shtests/install.rsbare-install refuse-if-exists behavior preserved🤖 Generated with Claude Code