[tayfuryldz]

Fixes #530.

Routes daemon, transaction-store, and MCP audit-key lookup through the existing resolve_audit_key_path helper. The Postgres path keeps its existing production-default behaviour by passing default_database_path() to the shared resolver.

A source-level regression test prevents these call sites from reintroducing direct SYSKNIFE_AUDIT_KEY_PATH resolution.

Validation:

• cargo fmt --all --check
• cargo test -p sysknife-daemon audit_key_env_resolution_is_centralised
• cargo check -p sysknife-daemon -p sysknife-cli
• git diff --check

[vladimirrott]

Reviewed at b5097e67. The refactor is right. Two blocking items, both small.

All four sites now call resolve_audit_key_path, and each passes the path its old inline copy used: TransactionStore::open passes its own db_path, the two MCP tools pass default_database_path(), so nothing moves on disk for anybody. Dropping the unused PathBuf import from mcp_server.rs came along with it. The baseline goes 1,912 to 1,913 in all four places.

Your test does catch a copy coming back where one used to be. The merge gate put an inline resolution back into mcp_server.rs and watched it go red:

$ maintainer-merge verify 540 b5097e67dbad0c69be1edc013f79dd36e942d7cb "audit_key_env_resolution_is_centralised" 's#let key_path = sysknife_daemon::audit_chain::resolve_audit_key_path(&db_path);#let key_path = std::env::var("SYSKNIFE_AUDIT_KEY_PATH").map(std::path::PathBuf::from).unwrap_or_else(|_| sysknife_daemon::audit_chain::resolve_audit_key_path(\&db_path));#' rust
  rust suite: evidence of 1 executed unit(s), passing unmutated
  applying the mutation and re-running
  receipt recorded for #540 at b5097e67 (observed: clean pass, mutated fail)

Blocking 1: a copy in any other file passes

#530 asked for a test "that fails when a fifth copy appears". The test reads three named files, so a copy anywhere else goes unseen. I planted one in apps/sysknife-cli/src/runner.rs, at the audit verify call site, and ran your test in a container:

$ podman run --rm --network=none -v "$W:/repo:z" -v "$HOME/.cargo:/cargo:O" -w /repo -e CARGO_HOME=/cargo -e CARGO_TARGET_DIR=/repo/.container-target -e CARGO_NET_OFFLINE=true -e CARGO_BUILD_JOBS=6 docker.io/library/rust:1-slim cargo test -p sysknife-daemon --lib --offline audit_key_env_resolution_is_centralised
running 1 test
test audit_chain::tests::audit_key_env_resolution_is_centralised ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 910 filtered out; finished in 0.00s

Walking crates/sysknife-daemon/src and apps/sysknife-cli/src at test time with std::fs, from env!("CARGO_MANIFEST_DIR"), and checking every .rs file except audit_chain.rs closes it. It also survives someone moving the MCP tools into a new module, and it removes the include_str! reach from the daemon crate into the CLI's source tree.

Blocking 2: the Postgres comment still describes code that is not there

main.rs:497-499 still reads "Resolution mirrors TransactionStore::open: env var > sibling of database_path > production default." The code after it resolves beside default_database_path(), and build_postgres_audit has no database_path in scope at all.

I said on #530 that this one needed a decision from me. The decision is to keep your behaviour. A Postgres install has no database file for the key to sit beside, and moving the fallback would make AuditKey::load_or_generate mint a new key on the next start for every existing Postgres deployment, whose chain then fails to verify against the old one. So the call stays as you wrote it and the comment should say that: env var, else beside the default SQLite path, because Postgres has no local database file.

One thing that is not about your code

I offered #530 to syf2211 on #528 yesterday, 29 September at 06:24 UTC, before you opened this, and they have not answered on #530 yet. I hold an offer for 72 hours, so if they take it by 2 October, 06:24 UTC, I owe you an apology and a different issue. Past that, and with the two items above in, this lands. I should have written the offer on #530 itself, where you would have seen it, and I did not.

[vladimirrott]

Reviewed at 3035a488. Blocking 2 is closed. Blocking 1 is still open, one directory further down.

The comment in main.rs now says what the code does, including why the key stays beside the default SQLite path, so a reader of build_postgres_audit no longer goes looking for a database_path that was never in scope.

read_dir does not descend

Your walk reads crates/sysknife-daemon/src and apps/sysknife-cli/src, and std::fs::read_dir lists one directory. The daemon keeps three subdirectories under src, and 45 of its 73 .rs files live in them:

crates/sysknife-daemon/src/actions/     42 files
crates/sysknife-daemon/src/transport/    2 files
crates/sysknife-daemon/src/store/        1 file   (postgres.rs)

Your walk sees the 28 at the top level and none of the other 45.

I appended the same six-line inline resolution twice, changing only where it landed, and ran your test in a container with no network:

--- planted in crates/sysknife-daemon/src/transactions.rs (top level)
test audit_chain::tests::audit_key_env_resolution_is_centralised ... FAILED
panicked at crates/sysknife-daemon/src/audit_chain.rs:1764:17:
/repo/crates/sysknife-daemon/src/transactions.rs must resolve the audit key through resolve_audit_key_path

--- planted in crates/sysknife-daemon/src/store/postgres.rs (one level down)
test audit_chain::tests::audit_key_env_resolution_is_centralised ... ok
test result: ok. 1 passed; 0 failed

The failure message is good: it names the file, which is what someone hitting this in six months needs. The reach is the only problem, and store/postgres.rs is a live audit backend rather than a hypothetical, so a copy there is the one I would most expect.

Recursing closes it. A small stack beats pulling in walkdir for a test:

let mut roots = vec![daemon_src.clone(), cli_src.clone()];
while let Some(dir) = roots.pop() {
    for entry in std::fs::read_dir(&dir).expect("source directory must be readable") {
        let path = entry.expect("source entry must be readable").path();
        if path.is_dir() {
            roots.push(path);
            continue;
        }
        // …the extension and audit_chain.rs check you already have
    }
}

Assert the file count too. The tree has 73 today, so files_checked >= 60 leaves room to move files without the test going red for the wrong reason. A walk that reaches nothing and a tree with no copies both end green, and #530 is specifically about the check still working after someone moves code around.

The hold I owe you

I offered #530 to syf2211 on 29 September at 06:24 UTC and said I hold an offer for 72 hours. They have not answered on #530, and the issue still has no assignee. That hold runs to 2 October, 06:24 UTC. Past it, with the recursion in, this lands and I will assign #530 to you.

Nothing here is waiting on you except the walk. Push it whenever suits, and I will re-run both plantings against the new head.

[vladimirrott]

Reviewed at b8fc4bd9. Approved, and I am landing it now.

The walk recurses with a stack and refuses to pass on fewer than 60 files, which closes both halves of what I asked for. I re-ran the planting that passed last time, the same inline resolution one directory below src/, through the merge gate:

  planted in crates/sysknife-daemon/src/store/postgres.rs
  rust suite: evidence of 1 executed unit(s), passing unmutated
  applying the mutation and re-running
  receipt recorded for #540 at b8fc4bd9 (observed: clean pass, mutated fail)

The floor bites on its own too. With roots.push(path) removed the walk sees only the top level, and your assertion names the number:

FAIL audit_chain::tests::audit_key_env_resolution_is_centralised
  source walk unexpectedly checked only 27 Rust files

27 is the top-level count exactly: 72 files, minus the 45 in actions/, store/ and transport/. So the floor measures what it claims to.

The hold

I released #530 from syf2211 early, on #530 itself, so nothing stands between this and main. I said I would assign #530 to you. GitHub refused because you have not posted on that issue, so the record is your PR closing it.

I will write the CHANGELOG entry when it merges.

Next, if you want it: #474. refuse.rs runs every model-authored string through normalise_free_text before it reaches the operator's terminal, and propose_plan.rs does not, so the plan summary, explanation and step summaries a human approves arrive verbatim. #521 was about the strings an MCP client reads before it acts, and this is the string an operator reads before saying yes. Nobody holds it. Say so on #474 and I will hold it for you there.

[vladimirrott]

@tayfuryldz, a correction to the last paragraph of my review: #474 is not free. I offered it to another contributor on 28 September, tied to a pull request of theirs that is about to land, and I missed that when I wrote to you. I am sorry.

I will put the next issue that fits your MCP and CLI work in front of you on that issue's own thread, where the offer is visible to everyone, including me.

[vladimirrott]

@tayfuryldz this shipped in v0.25.0, on crates.io and npm. Thank you for it.