Skip to content

The MCP audit-verify tool keeps reporting intact over an erased store after #461 #466

Description

@vladimirrott

#461 makes sysknife audit verify report cannot_verify and exit 2 over an
empty transaction log with no external anchor, which is what #338 asked for. The
MCP tool that publishes the same verdict to an agent keeps saying intact.

AuditVerifyReport says in as many words that it mirrors the CLI:

$ sed -n '297,298p' apps/sysknife-cli/src/mcp_server.rs
/// Output of `sysknife_audit_verify`. Mirrors the JSON shape produced by
/// the CLI's `sysknife audit verify --json` command.

Measured at #461's head 83aefe12, with a scratch test in mcp_server::tests
calling outcome_to_report on the empty-store verification and panicking with
whatever it returned:

SCRATCH rc=101
running 1 test
test mcp_server::tests::scratch_mcp_status_for_an_empty_unanchored_store ... FAILED
thread 'mcp_server::tests::scratch_mcp_status_for_an_empty_unanchored_store' panicked at apps/sysknife-cli/src/mcp_server.rs:1543:9:
MCP sysknife_audit_verify over an empty unanchored store reports status="intact" rows_checked=0

The scratch test was removed; the worktree came back to zero dirty files.

Why it happens

outcome_to_report builds status from the chain outcome and then corrects it
from verification.exit_code():

$ sed -n '1303p;1373,1382p' apps/sysknife-cli/src/mcp_server.rs
    let overall = verification.exit_code();
    // `status` is the headline an MCP client is most likely to read alone, so
    // it must reflect the worst of the three checks, not just the first.
    if report.status == "intact" {
        report.status = match overall {
            0 => "intact",
            1 => "broken",
            _ => "cannot_verify",
        }
        .to_string();
    }

AuditVerification::exit_code() is the daemon-side aggregate over the chain,
the approval events and the binding. It knows nothing about checkpoint anchors,
and sysknife_audit_verify never calls verify_configured_anchor:

$ grep -c verify_configured_anchor apps/sysknife-cli/src/mcp_server.rs
0

So from the MCP surface every log is unanchored, and an empty one is always the
inconclusive case. The CLI's combined_verification_exit_code reaches 2 through
a clause the MCP path has no equivalent of.

Why it matters

The MCP tool is the surface an agent reads without a human in the loop, which is
the worse half of #338 rather than the milder one. An agent asked "is this
machine's audit trail sound" over a wiped store gets status: "intact" and
reports back that it is.

The file already records the last time these two surfaces disagreed about the
same database, on the census fields:

$ sed -n '1285,1288p' apps/sysknife-cli/src/mcp_server.rs
    // the census can only reach the report one way. The first version of this
    // change let `cannot_verify_report` invent its own zeros while a real census
    // sat in `verification`, and the MCP tool then published different numbers
    // than `sysknife audit verify --json` did for the same database.

This is the same class, one release later.

Scope

  • Make sysknife_audit_verify report cannot_verify for an Intact chain with
    rows_checked: 0. The MCP path has no anchor, so there is no anchor.is_none()
    to consult and no reason to add one; decide and say in the PR whether the
    right home is outcome_to_report or a shared helper the CLI also calls.
  • A shared helper is worth considering rather than assuming. The CLI predicate
    takes an anchor and the MCP path has none, so the two are not the same
    function yet. Two parallel copies of the same rule is the shape that produced
    this issue; say which you chose and why.
  • Update the AuditVerifyReport doc comment if the mirror claim is narrowed.

Tests first

Write the failing test before the fix and show it red. The existing
a_cannot_verify_report_keeps_the_census_it_was_given and
a_broken_event_chain_does_not_make_the_transaction_chain_look_broken are the
pattern to follow: construct the AuditVerification, call outcome_to_report,
assert on the report.

Then mutation-prove it the way this repository asks: revert your production hunk
and show the new test going red, and paste the output. A test that stays green
with the fix removed is the defect this tracker keeps catching.

Blocked on

#461, which introduces the divergence. Nothing to do here until that merges, and
the numbers above were measured at its head rather than at main.

Difficulty

medium. The change is small; the judgement about where the rule should live so
it cannot drift again is the work.

Getting started

CONTRIBUTING.md
has the build and test commands. No CLA and no copyright waiver. The project is MIT.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingclaimedSomeone has said in the thread that they are working on thishelp wantedExtra attention is neededmediumDifficulty: needs familiarity with one subsystem

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions