[Audit] Help system & KB seeding — 7 verified findings (4 medium, 3 low)

[cuttlefisch]

Batched findings for the help-kb-seed capability slice, from the pre-v0.15 codebase audit (epic #592).

Traced end-to-end across every crate the capability touches, then independently re-verified by a
reviewer briefed to refute each claim. Only survivors appear here, at the verifier's corrected severity.

7 findings — 4 medium, 3 low. Tick off individually; split any one out
if it turns out to need real design work.

---

1. read_project_context byte-slices a UTF-8 string at a fixed 8000-byte offset — panics on any CLAUDE.md/README with a multi-byte char straddling that boundary

bug · medium

Impact. A project whose CLAUDE.md/README.md has an em-dash, arrow, box-drawing char or any non-ASCII byte spanning offset 8000 makes mae-agent abort at startup and makes the MCP initialize handshake panic — deterministically, for that project, forever. That handshake is the entry point for the v0.15 external-editor MCP pairing initiative (ADR-050/#375), so the failure mode is 'MAE won't connect to VS Code in this repo' with a stack trace instead of a message. The fix is one call to floor_char_boundary/char_indices, but nothing in the test suite would catch it because the only oversized-input fixture is ASCII (principle #14: a cherry-picked unicorn value that dodges the edge).

Evidence

crates/ai/src/guidance.rs:16-17,45-50:

const PROJECT_CONTEXT_MAX_CHARS: usize = 8000;
...
        if let Ok(content) = std::fs::read_to_string(&path) {
            let truncated = if content.len() > PROJECT_CONTEXT_MAX_CHARS {
                format!("{}...\n[truncated]", &content[..PROJECT_CONTEXT_MAX_CHARS])

content.len() and &content[..N] are both BYTE-indexed on a String; the constant is named _CHARS. &content[..8000] panics (byte index 8000 is not a char boundary) whenever byte 8000 is a UTF-8 continuation byte. This repo's own files are near-misses: CLAUDE.md is 58,327 bytes with 405 continuation bytes (0.69% of offsets) and README.md is 29,301 bytes with 962 (3.28%) — both happen to land on ASCII at 8000 today, so the shipped test read_project_context_truncates_oversized_files (guidance.rs:160-167) passes because its fixture is "x".repeat(...), pure ASCII.
Callers that would take the panic: crates/agent-cli/src/main.rs:214 (build_guidance_context — every mae-agent session start), crates/mae/src/main.rs:881 (MCP initialize instructions construction), crates/mae/src/bootstrap.rs:923 (read_project_context directly), crates/ai/src/tool_impls/guidance_export.rs:92 (kb_export_guidance).

Verification

Code confirmed verbatim: crates/ai/src/guidance.rs:16-17 const PROJECT_CONTEXT_MAX_CHARS: usize = 8000; and :45-47 let truncated = if content.len() > PROJECT_CONTEXT_MAX_CHARS { format!("{}...\n[truncated]", &content[..PROJECT_CONTEXT_MAX_CHARS]) } — both byte-indexed on a String, so a continuation byte at offset 8000 panics. All four cited callers exist: crates/agent-cli/src/main.rs:213-214, crates/mae/src/main.rs:880-885 (the MCP initialize instructions construction, ADR-063 Phase A), crates/mae/src/bootstrap.rs:923 if let Some(ctx) = mae_ai::guidance::read_project_context(&cwd), crates/ai/src/tool_impls/guidance_export.rs:92. The test-gap claim holds too: guidance.rs:159-167 read_project_context_truncates_oversized_files writes "x".repeat(PROJECT_CONTEXT_MAX_CHARS + 500) — pure ASCII, so it can never hit the boundary.

Scope correction from verification: Real latent panic, but 'high' overstates reachability and the finding itself concedes why: it fires only when byte 8000 specifically is a UTF-8 continuation byte, which this repo's own CLAUDE.md and README.md do not satisfy today. It is a ~1-3%-of-non-ASCII-projects coin flip, not a deterministic failure for any project with a non-ASCII character. Deterministic-and-permanent once a given project does land on it, which is why it still warrants medium rather than low.

---

2. Manual-KB integrity validation is inert: KNOWN_CHECKSUMS is empty so every file validates as Valid, and the shipped .sha256 sidecar is never read

bug · medium

Impact. The advertised property — 'we detect a tampered or foreign mae-manual.cozo at a well-known path' — does not exist. well_known_paths includes /usr/share/mae, /usr/local/share/mae, /opt/homebrew/share/mae and every assets/ dir on the path from the binary upward (manual_kb.rs:133-164); anything dropped there is loaded into the help KB and shown to the user (and fed to the AI via kb_get/kb_search_context) as authoritative MAE documentation, always reported as Valid. It also burns a startup-path directory hash for nothing. This is a documented-intent-vs-reality gap (a release-process TODO that never landed), not a design decision.

Evidence

crates/mae/src/manual_kb.rs:37-45:

/// Known SHA-256 checksums of official mae-manual.cozo releases.
/// Updated at release time by CI. Newest first.
const KNOWN_CHECKSUMS: &[(&str, &str)] = &[
    // Checksums will be populated by the release process.
    // Format: ("version", "sha256hex")
];

and manual_kb.rs:181-187:

    // In dev builds (no checksums populated), treat as valid.
    if KNOWN_CHECKSUMS.is_empty() {
        return ManualValidation::Valid;
    }
    ManualValidation::Unknown

Nothing populates it: grep -rn KNOWN_CHECKSUMS over the repo (excluding target/) hits only manual_kb.rs, and neither .github/workflows/release.yml nor Makefile edits that file. Meanwhile mae-manual.cozo.sha256 IS produced (crates/mae/src/bin/build_manual_kb.rs:61 kb_build::write_checksum_sidecar) and shipped/installed — Makefile:88, Makefile:442, assets/install.sh:436-437, .github/workflows/release.yml:58,294,322 — but grep shows no code path ever reads it back.
Consequence in bootstrap: crates/mae/src/bootstrap.rs:2175-2180's ManualValidation::Unknown => warn!("manual KB checksum does not match any known release") is unreachable, and compute_db_checksum (manual_kb.rs:194) runs a full recursive SHA-256 over the store directory on every startup purely to be discarded.

Verification

All of it verified. crates/mae/src/manual_kb.rs:37-45 — const KNOWN_CHECKSUMS: &[(&str, &str)] = &[ // Checksums will be populated by the release process. ]; — empty. validate_checksum (:167-189) loops over it, then if KNOWN_CHECKSUMS.is_empty() { return ManualValidation::Valid; }, so ManualValidation::Unknown is unreachable and the bootstrap.rs:2175-2180 warn! arm is dead code. A repo-wide grep for KNOWN_CHECKSUMS (excluding target/) returns only the three manual_kb.rs lines; neither the Makefile nor .github/workflows/release.yml touches it. The sidecar IS produced and shipped (crates/mae/src/bin/build_manual_kb.rs:60-68, release.yml:58/294/322) but nothing reads it back — the only consumer of a .sha256 file anywhere is verify_adr_kb_sync.rs, and that only checks whether git touched it. The wasted-work claim is also real and non-trivial: locate_and_validate:83-88 calls compute_db_checksum(candidate) on every startup, and assets/mae-manual.cozo is a 32 MB sled directory, so that is a full 32 MB SHA-256 per launch whose result is discarded.

Scope correction from verification: The 'advertised property' framing is too strong — I found no claim of manual-KB integrity checking in docs/ or SECURITY.md; the promise exists only in the code comment. Re-frame as: a never-implemented release-process TODO leaves a dead validation branch plus an unconditional ~32 MB SHA-256 on the startup path. The startup cost, not the security gap, is what makes this worth medium.

---

3. A bundled guidance KB copied into the data dir is never refreshed on upgrade — users keep the old DevPractices/MaePractices content forever, silently

bug · medium

Impact. ai_guidance_kb defaults to "DevPractices" in the shipped init.scm template, and its content is injected into every AI session's system prompt and every MCP initialize.instructions. So the one KB whose whole purpose is 'standing practices the agent must follow' is frozen at whatever version the user first installed, with no log line, no :kb-instances indication, and no command to force a refresh — the user's only recourse is to know to delete the file by hand. Given ADR-076 positions these as MAE's own dogfooded guidance, shipping corrections to them has no delivery mechanism.

Evidence

crates/mae/src/guidance_kb_engine.rs:113-125:

/// Copy a (possibly directory-based, e.g. sled) KB asset from `src` to
/// `dst`, unless `dst` already exists (an earlier session/run already
/// copied it — don't redo the work every startup).
fn copy_kb_asset(src: &Path, dst: &Path) -> std::io::Result<()> {
    if dst.exists() {
        return Ok(());
    }

and guidance_kb_engine.rs:149-153:

pub fn ensure_registered_with_path(descriptor: &BundledGuidanceKb, data_dir: &Path, path: PathBuf) {
    let registry = mae_kb::federation::KbRegistry::load(data_dir);
    if registry.find(descriptor.instance_name).is_some() {
        return;
    }

Neither branch compares versions or checksums. After the first run, ~/.local/share/mae/mae-devpractices.cozo exists and the registry has a DevPractices row, so every later startup — including after mae upgrade installs a newer assets/mae-devpractices.cozo next to the binary — short-circuits at dst.exists() and re-registers nothing. The doc comment's stated rationale is only 'don't redo the work every startup', i.e. a cost optimisation, not an intentional pinning policy; ADR-076's documented invariant is the different one ('a contributor's own same-named registration always wins'), which stays satisfied by a version check.
Contrast the manual KB, which is re-read from the shipped asset every startup (load_nodes_readonly, manual_kb.rs:120) and therefore does track upgrades.

Verification

Both short-circuits verified verbatim. crates/mae/src/guidance_kb_engine.rs:113-125 fn copy_kb_asset(src: &Path, dst: &Path) … { if dst.exists() { return Ok(()); } under the doc comment "unless dst already exists (an earlier session/run already copied it — don't redo the work every startup)", i.e. a cost rationale, not a pinning policy. And :149-153 pub fn ensure_registered_with_path(…) { let registry = KbRegistry::load(data_dir); if registry.find(descriptor.instance_name).is_some() { return; }. Neither compares a version or checksum. The DevPractices default is confirmed: crates/mae/src/config.rs:1373-1379 has the test default_template_defaults_ai_guidance_kb_to_devpractices, and options.rs:500-512 documents ai_guidance_kb's shipped default as "DevPractices". The ADR-076 invariant cited in the module doc (guidance_kb_engine.rs:11-17) is indeed the different 'your own same-named registration always wins' property; grep -i 'upgrade|refresh|stale|newer|version' docs/adr/076-bundled-kb-system-devpractices.md returns nothing, so no-refresh-on-upgrade is not a documented decision. The manual-KB contrast holds: manual_kb.rs:118-131 load_nodes_readonly re-reads the located asset into a throwaway temp copy on every startup, so the manual does track upgrades while the guidance KB does not.

---

4. 16 registered Scheme primitives have no scheme: help node — :help lies about the API, and the only parity guard is a hand-listed subset

parity-gap · medium

Impact. :help scheme:kb-graph-view-open finds no node and silently falls through open_help_at's fuzzy-search fallback to some unrelated node or the index (help_ops.rs:774-789), and concept:scheme-api under-reports the surface. Every one of these 16 has a documented MCP tool (kb_graph_view_open, kb_related, kb_register, …), so the AI peer can discover the capability from its tool list while the human's :help cannot — the exact asymmetry principle #3 forbids. It is also mechanically preventable today: CODE_MAP.json is already generated and CI-gated, so a coverage test comparing it to SCHEME_API_FUNCTIONS is a few lines.

Evidence

crates/core/src/kb_seed/scheme_api.rs:7 — pub(crate) const SCHEME_API_FUNCTIONS: &[(&str,&str,&str,&str,&str)] is hand-maintained (192 entries). Diffing its names against the 212 real vm.register_fn registrations in docs/CODE_MAP.json.scheme_primitives (which the CI-gated make code-map already produces) gives 16 user-facing primitives that are registered but have NO doc entry and also NO assets/manual/scheme-*.org override:
kb-graph-view-open, kb-graph-view-close, kb-graph-view-navigate, kb-graph-view-refresh, kb-graph-view-select-current, kb-graph-view-set-depth, kb-graph-view-set-pinned, kb-graph-view-state, kb-graph-view-toggle-overlay, kb-graph-view-zoom-to (crates/scheme/src/runtime/kb_graph_view.rs)
kb-graph, kb-neighborhood, kb-related, kb-shortest-path (crates/scheme/src/runtime/kb_queries.rs)
kb-register (crates/scheme/src/runtime/kb_primitives.rs)
kb-export-subgraph-html (crates/scheme/src/runtime/kb_export.rs)
Confirmed absent from disk: ls assets/manual/scheme-kb* yields only scheme-kb-agenda.org, scheme-kb-history.org, scheme-kb-raw-query.org, scheme-kb-restore.org.
The only guard is crates/core/src/kb_seed/scheme_api.rs:1560 kb_sharing_actions_have_scheme_api_docs, which checks a hand-written 24-name required list of KB-sharing/collab actions — it can never notice a primitive nobody thought to add to that list.
(The 11 names in the table with no Rust registration — describe-group, it-test, should*, before-each, after-each, wait-until — are legitimately defined in scheme/lib/mae-test.scm, not a defect.)

Verification

I recomputed the diff myself. docs/CODE_MAP.json.scheme_primitives has 212 entries; diffing against the tuple names in crates/core/src/kb_seed/scheme_api.rs's SCHEME_API_FUNCTIONS yields 31 registered-but-undocumented names, of which 15 are internal test-* runner primitives (test-buffer-string, test-sync-content, …) and the remaining 16 are exactly the user-facing list claimed: kb-graph-view-{open,close,navigate,refresh,select-current,set-depth,set-pinned,state,toggle-overlay,zoom-to}, kb-graph, kb-neighborhood, kb-related, kb-shortest-path, kb-register, kb-export-subgraph-html. ls assets/manual/ | grep scheme-kb returns only scheme-kb-agenda.org, scheme-kb-history.org, scheme-kb-raw-query.org, scheme-kb-restore.org — no override for any of the 16. The guard at scheme_api.rs:1560 kb_sharing_actions_have_scheme_api_docs is exactly as described: a hand-written 24-name required array of KB-sharing/collab actions, structurally unable to notice a name nobody added. The reverse direction (documented-but-unregistered) is also correctly explained away: my diff returns the 11 mae-test.scm names plus 13 *variable* entries, no phantom functions.

Scope correction from verification: One nuance in the consequence: the fallback at help_ops.rs:774-789 is not fully silent — if fuzzy search finds nothing it does set_status("No help node: {} — showing index"). It is silent only in the worse case, where fuzzy search happens to return an unrelated node and that node is opened as if it were the answer.

---

5. Every live-rendered command help page says "See also: cmd:move-right" — a hardcoded placeholder in a second, divergent copy of the command-node body

bug · low

Impact. All ~560 command help pages tell the reader to 'see also' an arbitrary cursor-movement command, and lose the two links (index, concept:command) that make command docs navigable back to the manual. It ships as visible nonsense in the primary human help surface. The root cause is the second implementation: install_command_nodes (seed) and describe_command_live (runtime) both format a command body, and only the seeded one is covered by tests (command_node_body_has_source_and_backlinks, mod.rs:1291) — consolidating to one formatter is the principle-#15 fix, not patching the string.

Evidence

crates/core/src/editor/help_ops.rs:705-708:

        out.push_str(&format!(
            "\nSee also: [[cmd:move-right]], [[category:{}]]\n",
            category
        ));

cmd:move-right is a literal, not derived from cmd_name — grep -rn "cmd:move-right" over the whole repo returns exactly this one line. describe_command_live is the path kb_populate_buffer takes for EVERY cmd: node (help_ops.rs:812-817: if node_id.starts_with("cmd:") { ... if let Some(live_text) = self.describe_command_live(cmd_name)), so it is what :help cmd:save, :describe-command, and clicking any [[cmd:…]] link actually render.
It is also a divergent duplicate of the seeded body in crates/core/src/kb_seed/mod.rs:450-457, which correctly emits See also: [[index]], [[concept:command]], [[category:{category}]] — the live path silently drops [[index]] and [[concept:command]].

Verification

Verified. crates/core/src/editor/help_ops.rs:705-708 is out.push_str(&format!("\nSee also: [[cmd:move-right]], [[category:{}]]\n", category)); — cmd:move-right is a literal with no relation to cmd_name, and a repo-wide grep (excluding target/) returns that one line only. It is the live path for every command node: help_ops.rs:812-817 if node_id.starts_with("cmd:") { let cmd_name = …; if let Some(live_text) = self.describe_command_live(cmd_name) {. The seeded twin at crates/core/src/kb_seed/mod.rs:450-457 emits See also: [[index]], [[concept:command]], [[category:{category}]], so the live path does drop two links, and the two bodies are independently formatted (principle #15).

Scope correction from verification: Confirmed but low, not medium: the entire user impact is one wrong and two missing cross-reference links in the See-also line. Nothing malfunctions, no content is lost, and the rest of the live body (doc, category, source, keybindings, hooks) is correct. Worth fixing as the consolidation it points at, not as a functional defect.

---

6. Project-context injection size and file list are hardcoded, while the sibling guidance-KB budget is a registered option

missing-config · low

Impact. How much of a project's own guidance file reaches the AI, and which file counts as 'the' project context, is exactly the kind of per-user/per-project behaviour principle #7 requires to go through the registry so it is reachable from (set-option!), :set and :set-save. Today a user with a 40 KB CLAUDE.md silently gets the first 8 KB with no way to raise it, and a project using AGENTS.md gets nothing. The inconsistency with ai_guidance_inline_budget_chars five lines away in the same feature shows this is an oversight rather than a considered fixed constant.

Evidence

crates/ai/src/guidance.rs:16-17:

const PROJECT_CONTEXT_FILES: &[&str] = &["CLAUDE.md", "README.md", "README.org", ".project"];
const PROJECT_CONTEXT_MAX_CHARS: usize = 8000;

Neither has an OptionRegistry entry (grep -n project_context crates/core/src/options.rs → nothing) and neither carries a rationale comment. The directly adjacent, semantically identical knob for the other half of the same function IS an option: crates/core/src/options.rs:513-521 opt!("ai_guidance_inline_budget_chars", …, OptionKind::Int, "8000", Some("ai.guidance_inline_budget_chars"), &[]). Only the first matching file is read (guidance.rs:40-56) — a project with both an AGENTS.md convention and a README gets no say.

Verification

crates/ai/src/guidance.rs:16-17 declares both constants with no rationale comment above them (the surrounding doc block at :1-11 explains the module's sharing across surfaces, not the numbers). grep -n project_context crates/core/src/options.rs returns nothing. The sibling is registered: options.rs:513-521 opt!("ai_guidance_inline_budget_chars", …, OptionKind::Int, "8000", Some("ai.guidance_inline_budget_chars"), &[]) with a full written rationale. read_project_context (:39-56) does return on the first matching filename, so a project with both AGENTS.md-style conventions and a README has no say. Genuinely user-visible behaviour (how much of a project's own guidance reaches the AI), no documented rationale — principle #7 applies.

---

7. MAX_RELATED cap on the help view's Related block is an undocumented magic number, unlike the MAX_NEIGHBORHOOD_LINKS cap right beside it

missing-config · low

Impact. The number of related-topic suggestions in the help view is precisely a 'user-visible behaviour that could reasonably differ between users' — and because there is no overflow note, silently showing 8 of 24 is indistinguishable from 'there are only 8'. The contrast with the documented cap in the same file is what makes this a finding rather than style: CLAUDE.md #7 accepts a fixed constant with a written rationale, and this one has none.

Evidence

crates/core/src/editor/help_ops.rs:53:

    const MAX_RELATED: usize = 8;

No comment, no option. Twenty lines further down the file, the analogous cap carries a full written rationale and is treated as a deliberate fixed constant — help_ops.rs:88-101:

/// Cap on outgoing/incoming links actually rendered (title-resolved) per
/// direction, regardless of a node's true degree.
/// Regression fix: ... for a true hub node (`index`: 1300+ edges), that's a
/// synchronous multi-second stall on every single click, reported live.
const MAX_NEIGHBORHOOD_LINKS: usize = 50;

The caller also asks the query layer for 24 related nodes (query.related(node_id, 24), help_ops.rs:217 and again at 303 and 872) and then throws away all but 8 — a second unexplained constant, repeated in three places.
Unlike MAX_NEIGHBORHOOD_LINKS, render_related_block does NOT emit a '… (N more)' note when it truncates (help_ops.rs:54-77 vs 143-148), so the user cannot tell the list was cut.

Verification

crates/core/src/editor/help_ops.rs:53 is const MAX_RELATED: usize = 8; inside render_related_block with no comment and no option. MAX_NEIGHBORHOOD_LINKS at :88-101 does carry the full regression rationale quoted in the finding. .take(MAX_RELATED) at :59 with no overflow note (:52-77 has no '... (N more)' emit), whereas render_neighborhood_links:143-148 does if total > MAX_NEIGHBORHOOD_LINKS { out.push_str(&format!(" ... ({} more)\n", …)) }. The 24 is real and repeated three times: query.related(node_id, 24) at :217, kb.related(node_id, 24) at :303, q.related(&node_id, 24)/self.kb.primary.related(&node_id, 24) at :873/:875. Extra corroboration the finding did not spot: MAX_NEIGHBORHOOD_LINKS's own doc comment claims it is "mirroring render_related_block's cap-with-count-note shape" — which is false, render_related_block has no count note.

Scope correction from verification: 'Silently showing 8 of 24' overstates the typical case: related() returns ranked results and the block first filters out ids already rendered in the Neighborhood section (:56-58), so the list is often shorter than 8 anyway and the truncation is frequently not exercised. The defensible core is the undocumented constant plus the missing overflow note, which is why low is right.

---

From the pre-v0.15 codebase audit (epic #592). Traced end-to-end across crates, then independently re-verified by a reviewer briefed to refute it; severity is the verifier's corrected value. In this batch, 8 of 11 claimed-high were downgraded.

[cuttlefisch]

Part of the pre-v0.15 codebase audit — tracking epic #592.