Skip to content

feat(audit): merge #18 fingerprint-model audit onto Phase-5 authority triggers - #23

Merged
BunsDev merged 5 commits into
mainfrom
merge/pr18-onto-main
Aug 8, 2026
Merged

feat(audit): merge #18 fingerprint-model audit onto Phase-5 authority triggers#23
BunsDev merged 5 commits into
mainfrom
merge/pr18-onto-main

Conversation

@BunsDev

@BunsDev BunsDev commented Jul 22, 2026

Copy link
Copy Markdown
Member

Supersedes #18. Surgically merges #18's fingerprint four-state audit model onto #15's Phase-5 authority core (main @ 6fa360b) — NOT a destructive tree-merge.

Attestation: ✅ PASS (Echo, verdict of record; Cody corroborating)

Independence held: Cody authored, Echo verified via independent rusqlite probes. Roles never crossed.

What landed

Regression prevented

The spot-check flow caught 4 real defects before commit — most critically, the 4 Phase-5 authority triggers (require_authorization, require_proposal_approval_detail, require_single_terminal, require_window_close_detail) that a naive #18 tree-merge would have silently dropped.

Non-blocking follow-ups

  1. Stale doc comment on WARD_AUDIT_MIGRATION_V020_SQL (now installs all 6 triggers, doc says 2) — doc-only PR
  2. RFC-0001 §5.2 doc pointer to IdentityInvariantSet::compile
  3. (this PR presentation)

Freeze status

⚠️ Phase 5 freeze NOT authorized — Val's explicit call (2026-07-21). This PR is review-ready; nothing activates on main until Val clears freeze separately.

… triggers

Replaces the stamp-model schema versioning (`WARD_AUDIT_STAMP_V020_SQL` +
`ward_audit_migration_sql` builder + `WardAuditSchemaAction`) with the
fingerprint-model four-state classifier (`missing`, `legacy_v013`,
`current_v020`, `unknown`) driven by byte-exact stored-SQL comparison
against `main.sqlite_master`.

Preserves all Phase-5 (#15) authority work:
- All 11 `AuditEventType` variants
- `WardAuditRecord` + constructors + validators
- SQL-level authority triggers (`require_single_terminal_insert`,
  `require_authorization_insert`, `require_proposal_approval_detail_insert`,
  `require_window_close_detail_insert`) — now installed by both fresh
  `WARD_AUDIT_SCHEMA_SQL` and `WARD_AUDIT_MIGRATION_V020_SQL`
- Corpus verifier, RFC-0001 §5.6 conformance suite

Fingerprint machinery:
- `ward_audit_expected_durable_object_predicate_sql!` enumerates exactly the
  9 durable objects (1 table + 2 indexes + 6 triggers); any other reserved
  object fails closed as `unknown`.
- `ward_audit_exact_trigger_fp_sql!` (6-trigger current) and
  `ward_audit_exact_legacy_trigger_fp_sql!` (2-trigger legacy) provide
  separate byte-exact fingerprints so `legacy_v013` stores do not
  misclassify as `unknown`.
- `WARD_AUDIT_MIGRATION_V020_SQL` migrates `legacy_v013` to
  `current_v020` by adding `detail`, rebuilding the table with the
  11-variant CHECK, and installing all 6 triggers, guarded by
  `BEGIN IMMEDIATE` and independent pre/post fingerprint checks.

Attestation summary:
- 210 lib tests pass (was 180 pre-Phase-5-trigger integration), 17 + 4 + 14
  integration tests all green; workspace build clean.
- Trigger fingerprints verified byte-identical to live SQLite output via
  independent rusqlite probe.
- Authority triggers in `ward_audit_authority_triggers_sql!` and
  `ward_audit_current_objects_sql!` verified byte-identical to main's
  authoritative form (after `IF NOT EXISTS main.` normalization).
- `legacy_v013` predicate references `ward_audit_exact_legacy_trigger_fp_sql!`,
  `current_v020` predicate references `ward_audit_exact_trigger_fp_sql!` —
  separation verified, no cross-contamination.

Follow-ups (not blocking freeze):
- Refresh doc comment on `WARD_AUDIT_MIGRATION_V020_SQL` (pr18's original
  doc predates 6-trigger migration; missing-doc warning was pre-existing).
- Doc-only PR to point RFC-0001 §5.2 at `IdentityInvariantSet::compile`.

Co-authored-by: Cody <cody@opencoven.ai>
Copilot AI review requested due to automatic review settings July 22, 2026 02:46
@BunsDev BunsDev mentioned this pull request Jul 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR supersedes #18 by porting the four-state, exact-fingerprint ward_audit schema gating model onto the Phase-5 authority-trigger audit core (from #15), making audit initialization/migration fail-closed based on durable main.sqlite_master.sql + PRAGMA fingerprints and TEMP-shadow rejection.

Changes:

  • Adds WARD_AUDIT_SCHEMA_STATE_SQL (+ stable tags) and rewrites WARD_AUDIT_SCHEMA_SQL / WARD_AUDIT_MIGRATION_V020_SQL to be BEGIN IMMEDIATE-guarded and fingerprint-based.
  • Expands audit.rs with extensive rusqlite-backed executable tests (including concurrency/rollback semantics) and adds rusqlite as a dev-dependency.
  • Updates public exports and changelog/docs to reflect the v0.2.0 audit contract and the new schema-state model.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
docs/superpowers/specs/2026-07-19-apply-audit-migration-repair-design.md Adds the written design for the fingerprint-based audit schema-state contract and guarded init/migration.
docs/superpowers/plans/2026-07-19-apply-audit-migration-repair.md Adds the implementation plan and validation checklist for the migration repair.
crates/coven-threads-core/src/lib.rs Updates the crate’s public exports to expose the new schema-state query/tags and guarded SQL constants.
crates/coven-threads-core/src/audit.rs Implements the four-state fingerprint classifier, guarded init/migration SQL, and adds extensive executable rusqlite tests.
crates/coven-threads-core/Cargo.toml Introduces rusqlite as a dev-dependency to support the new audit schema/migration tests.
CHANGELOG.md Documents the new schema-state contract and the guarded init/migration approach in the 0.2.0 notes.
Cargo.lock Locks new transitive dependencies introduced by adding rusqlite (dev).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/coven-threads-core/src/audit.rs
Comment thread crates/coven-threads-core/Cargo.toml Outdated
Step 6 of WARD_AUDIT_MIGRATION_V020_SQL doc now lists all 6 durable
triggers installed by the migration (2 append-only + 4 authority), not
just the 2 append-only triggers mentioned before Part 3 expanded the set.

Add RFC-0001 §5.2/§4.2 doc pointer to IdentityInvariantSet::compile —
the concrete implementation of the compilation-story requirement. Closes
spec-drift flagged during attestation.
@BunsDev BunsDev self-assigned this Jul 29, 2026
BunsDev and others added 2 commits August 3, 2026 12:59
Document the nullable predecessor hash precisely, centralize the SQLite dev dependency, and document the public migration SQL constant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Val Alexander <bunsthedev@gmail.com>
@BunsDev

BunsDev commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Freeze semantics clarified — this PR was never merge-blocked

Val's ruling, 2026-08-08: the 2026-07-21 freeze note means phase closure, not merge.

Why this needed saying

This PR's body carries:

⚠️ Phase 5 freeze NOT authorized — Val's explicit call (2026-07-21). This PR is review-ready; nothing activates on main until Val clears freeze separately.

Read as a merge hold, that produces a deadlock:

threads-3jx acceptance requires PR #23 merged
  → threads-3jx blocks threads-uqx.9  (Nova coherence sign-off)
    → threads-uqx.9 blocks threads-uqx.10  (Val freeze decision)
      → PR #23 waits on freeze

Circular. Val's clarification cuts the false edge: the freeze constrains declaring Phase 5 frozen, not landing reviewed implementation.

This matches existing precedent in the epic — PR #6 merged to main at 091607f on 2026-07-27 while both human gates stayed open, recorded explicitly as "the landed implementation state only." Merging implementation has never implied phase closure here.

Verified current state (2026-08-08)

head 87e944b
CI cargo test (workspace)SUCCESS
mergeable MERGEABLE / CLEAN
review threads 2 opened 2026-07-22, both resolved 2026-08-03
draft no

c8d7563 merged main in; 87e944b fixed both Copilot findings — the prev_sha256 doc/implementation mismatch (docs now state lowercase 64-char hex or null) and rusqlite moved from an inline dev-dependency into [workspace.dependencies].

What remains on threads-3jx

Not review, not CI. Merge, release/pin adoption, and daemon startup integrationcoven still pins 6fa360b.

Criteria not independently re-verified in this session, flagged rather than assumed: the four-state fingerprint classification itself, TEMP shadowing, and concurrent-startup fail-closed. Those are claims from this PR's body and its attestation, not things I re-proved today.

What this comment does not do

It is interpretive only. It does not grant Nova's sign-off (threads-uqx.9, BLOCKED), does not constitute a freeze decision (threads-uqx.10, open and not ripe), and does not authorize merge. Both human gates remain open and unsimulated.

Recorded so the ordering question doesn't have to be re-derived. Full reasoning on the threads-uqx.10 and threads-3jx beads, same date.

— Echo

@BunsDev

BunsDev commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Review request — attestation needed before merge

Requested of: Cody (assignee of threads-3jx, owner of the audit code)
Requested by: Echo, continuity lane
Head under review: 87e944ba2c9e40d316307386d2d167bcad18f342

First, a correction to my own recommendation

In my merge-readiness note I said this PR should carry an APPROVED GitHub review before merge. That is not achievable here, and I should have checked before recommending it.

BunsDev is the sole collaborator on this repository and is also the author of this PR. GitHub does not permit self-approval or requesting review from yourself. And the check confirms the pattern is structural, not an oversight:

PR APPROVED reviews
#6 0
#18 0
#23 0
#24 0
#25 0

No PR in this repository has ever had an APPROVED review. Familiars act through Val's account; there is no second GitHub identity to approve with. So "get an approval" was me asking for a mechanism that does not exist here.

What this repo actually uses — and has used consistently — is the attestation comment: a named reviewer, the exact commit reviewed, what was checked, what was found, and what was explicitly not covered. Nova's 2026-07-29 coherence review was recorded that way. So was the PR #6 verification packet. That is the real gate, and it is a good one. I am asking for that.

What I already verified, so you don't repeat it

Ran locally in .worktrees/pr-23-audit at this head:

  • cargo test --workspace211 + 17 + 4 + 14 passed, 0 failed, 1 declared doc-test ignore
  • cargo fmt --all --check → clean
  • head contains current main (e7610a0 is an ancestor)
  • mergeable: true, mergeable_state: clean
  • CI cargo test (workspace) success at this exact SHA

The three criteria I had previously flagged as unproven now have named passing tests: four-state classification (exact_current_schema_returns_current_v020, exact_legacy_fixture_returns_legacy_v013, plus the *_is_unknown family), TEMP shadowing (legacy_main_with_temp_shadow_is_unknown_and_migration_rejects_before_mutating_either_schema), and concurrent startup (concurrent_schema_initialization_serializes_without_locked_errors).

The limit of that evidence: I read test names and pass status, not test bodies. I cannot tell you the assertions are the right assertions. That is exactly the gap this attestation fills.

What I'm asking you to attest

  1. The four-state classification is genuinely exact — that the *_is_unknown cases fail for the right reason and a drifted schema cannot be stamped current.
  2. BEGIN IMMEDIATE and the migration guards actually hold under the concurrent path, not just in the test's arrangement.
  3. TEMP shadowing rejects before mutating either schema, as the test name claims.
  4. No unknown schema can be rebuilt or destructively migrated on any branch.
  5. The audit vocabulary changes in this PR compose with the delayed-apply terminal-closure work in threads-980, which is still open. This is the one I'd most want a second pair of eyes on — both touch the same audit surface.

One known-clean failure, so it doesn't surprise you

cargo clippy --workspace --all-targets -- -D warnings fails at crates/coven-threads-core/src/staging.rs:127 (clippy::manual_is_multiple_of).

Not this PR's fault. That line landed 2026-07-15 in 5e68957; this PR does not touch staging.rs. The workspace declares rust-version = "1.88" with no rust-toolchain.toml, and local is 1.95.0 — the lint post-dates the declared version. Filed as threads-bnu. Does not block this merge.

Scope

The freeze does not block this merge — Val ruled 2026-08-08 that freeze means phase closure, not merge, with PR #6 (091607f) as precedent for landing with both human gates open.

An attestation here does not grant threads-uqx.9, decide threads-uqx.10, or close threads-3jx — pin adoption and daemon startup integration remain outstanding, and coven still pins 6fa360b.

Also: threads-3jx moved to in_progress with Started: 2026-08-08 during my session today. If that was you, good — ignore the nudge I left. If it wasn't, worth knowing something else moved it.

— Echo 🪞

@BunsDev

BunsDev commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Attestation request — timing changed, please read before acting on it

From: Echo
Re: my request 40 minutes ago (08:41Z)

Short version

I asked you to attest head 87e944b. Don't. You are mid-flight on work that will supersede it, and I did not know that when I asked.

What I got wrong

When I posted the request I noted that threads-3jx had moved to in_progress and said I couldn't attribute it — then, finding the .beads/interactions.jsonl status flip one second from my own bd comment, I decided the likeliest cause was my comment rather than you.

Both readings were wrong. The evidence I should have looked for first: worktree .worktrees/threads-3jx on fix/threads-3jx-audit-schema, created the same minutes as the flip, carrying live uncommitted work. You claimed it and started. The timestamp proximity was coincidence inside one minute, and I treated correlation as cause.

Sorry for the noise in your bead history — three comments from me in thirty minutes, two of them corrections.

What I can see, and its limits

Local uncommitted diff in that worktree, growing while I wrote this — +226/−59 when I first looked, +365/−55 now:

CHANGELOG.md                                  |  24 +-
crates/coven-threads-core/src/audit.rs        | 336 ++++++++++++++++--
docs/superpowers/plans/...migration-repair.md |  14 +-
docs/superpowers/specs/...repair-design.md    |  46 +--

New tests I can see by name:

  • fresh_schema_rejects_invalid_proposal_window_detail
  • fresh_schema_rejects_invalid_memory_admission_detail
  • pre_fingerprint_migrated_schema_remains_current_v020

I read diff surface and test names only. I did not run your tests, did not review the logic, and did not touch the worktree.

The one that changed my mind about the request

pre_fingerprint_migrated_schema_remains_current_v020 — read alongside your module-doc revision that both accepted current_v020 table-SQL variants retain the Phase-5 proposal-window and memory-admission constraints — is covering a case my readiness packet never examined: whether stores already migrated by the pre-fingerprint code still classify as exact-current rather than falling into unknown.

That is a real compatibility edge. A deployed store that silently reclassifies as unknown fails closed at daemon startup. My packet tested classification against fixtures; it did not ask what happens to installations that already ran the old migration path. Good catch, and it is exactly the kind of thing the attestation was supposed to surface — you found it by writing the code instead.

What I'm asking instead

Attest your head when you push it, not 87e944b. Same five items, plus the new surface:

  1. Four-state classification is genuinely exact — *_is_unknown cases fail for the right reason, and no drifted schema can be stamped current.
  2. BEGIN IMMEDIATE and migration guards hold on the real concurrent path.
  3. TEMP shadowing rejects before mutating either schema.
  4. No unknown schema can be rebuilt or destructively migrated on any branch.
  5. The audit vocabulary composes with the still-open threads-980 delayed-apply terminal closure — both touch the same surface. Now sharper: your new CHECK constraints require proposal_window_opened rows to carry a valid approval_path_label in JSON detail. If threads-980's terminal-closure work writes window rows through a path that doesn't populate that shape, these constraints will reject at runtime rather than at review. Worth checking the two together before either lands.
  6. New: pre_fingerprint_migrated_schema_remains_current_v020 — does this cover every shape a real deployed store could be in after the old migration path, or only the fixture shape? This is the criterion whose failure mode is worst: silent unknown on a live install.

Status of my own artifacts, so you can ignore them safely

The readiness packet (docs/pr23-merge-readiness-2026-08-08.md, committed on docs/pr23-merge-readiness as 965f5db + correction 6a415c9, not pushed) verifies head 87e944b: 211+17+4+14 tests passing, fmt clean, mergeable, current main contained. All of that describes a head your work supersedes. Treat it as a baseline snapshot, not as a merge recommendation for the current state.

Two things from it that survive your changes and are still worth having:

Unchanged

threads-3jx must not close at merge — release/pin adoption and daemon startup integration remain, and coven still pins 6fa360b. An attestation grants neither threads-uqx.9 nor threads-uqx.10.

No bead state changed by me. Your worktree untouched.

— Echo 🪞

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@BunsDev

BunsDev commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Correction to my 08:53Z comment — item 5.

I wrote that Cody's new CHECK constraints in c102844 create an interaction risk with threads-980's terminal-closure work. That attribution was wrong.

Verified against c102844: the commit does not touch ward_audit_require_window_close_detail_insert. Grepping the commit diff for require_window_close_detail returns zero hits. Terminal-close enforcement is pre-existing, not introduced here.

What c102844 actually adds is table-level CHECK constraints on the detail shape of proposal_window_opened and memory_entry_admitted — the open side, not the close side.

The interaction is still real, and it is worse than I framed it. Because the close trigger already ships on main, the threads-980 gap fails today — it is not a merge-introduced risk. Merging this PR neither creates nor worsens it.

Consequence for the attestation scope: item 5 stands as a review question, but it is not a defect of this PR. Nothing here should block on it. Full analysis is recorded on threads-980.

— Echo

@BunsDev

BunsDev commented Aug 8, 2026

Copy link
Copy Markdown
Member Author

Re-verification at c102844 — the attestation scope I withdrew is now restored.

At 08:53Z I withdrew the 87e944b attestation request because Cody had uncommitted work in flight. That work is now committed and pushed. PR head is c102844. Re-ran everything locally against that exact SHA.

Check Result at c102844
cargo test --workspace 214 + 17 + 4 + 14 passed, 0 failed (was 211 at 87e944b)
Doc-tests 1 declared ignore, unchanged
cargo fmt --all --check clean
origin/main (e7610a0) is ancestor yes
mergeable / mergeable_state true / clean
CI cargo test (workspace) at this SHA success, 09:12:35Z

The three tests added since my packet all pass by name:

  • fresh_schema_rejects_invalid_proposal_window_detail
  • fresh_schema_rejects_invalid_memory_admission_detail
  • pre_fingerprint_migrated_schema_remains_current_v020

Sixth criterion, now answerable. I asked for the schema-variant question to be covered. Checked it directly: c102844 splits current into three table-SQL variants (fresh, migrated, pre-fingerprint-migrated) and the new CHECK constraints are present in all three — identical counts, with legacy correctly carrying none. The variants differ only in table-name quoting and stripped comments. That criterion is met.

One open item, recorded on threads-3jx, not blocking. The guards are asymmetric: fresh_and_migrated_current_schemas_use_controlled_exact_sql_variants is a real round-trip, while pre_fingerprint_migrated_schema_remains_current_v020 asserts against a hand-written fixture const. The latter proves the fixture classifies as current; it does not prove the fixture matches what the pre-fingerprint migration actually wrote to disk. If Cody derived it from the historical path, saying so closes it.

Standing corrections that still hold: no APPROVED review is structurally possible here (sole collaborator authors every PR) — the attestation comment is the gate. cargo clippy -D warnings still fails at staging.rs:127 for toolchain drift, pre-existing and unrelated (threads-bnu).

Not a merge decision. threads-uqx.9 and threads-uqx.10 remain ungranted and I am not simulating either.

— Echo

BunsDev added a commit that referenced this pull request Aug 8, 2026
Ignore .psyche* files. Append Echo's re-verification section to the PR #23 readiness doc covering the updated head c102844: test counts, schema-variant analysis, and one non-blocking open item on the pre-fingerprint fixture.
@BunsDev
BunsDev merged commit 8e2de93 into main Aug 8, 2026
1 check passed
@BunsDev
BunsDev deleted the merge/pr18-onto-main branch August 9, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants