fix: guard graph push volatile bank files

[moul]

Summary

• add a default graph_push guard that skips configured volatile bank files (activeContext.md, progress.md)
• add include_volatile opt-in with manage/admin enforcement and audit logging
• keep pushed as the numeric counter while adding pushed_files, skipped_volatile, warning/message fields
• document the new option and GRAPH_PUSH_VOLATILE_FILES policy

Tests

• /tmp/live-memory-graph-guard-venv/bin/python -m pytest tests/test_graph_volatile_guard.py -q
• /tmp/live-memory-graph-guard-venv/bin/python -m pytest tests/ -q

Note: uv run pytest tests/ -q could not start because existing uv.lock fails to parse at line 431 (live-memory package missing version).

[chrlesur]

Review

Verdict: request changes — the direction and the core implementation are right, and this is careful work. But this PR is the server-side guardrail announced for Wave B, and two of the four acted Wave B acceptance criteria are not covered yet. Both are small, high-leverage additions.

What holds up well (checks performed)

• Default skip + opt-in: volatile files are excluded by default; include_volatile=True is enforced manage/admin at the MCP tool level (admin console and CLI both go through the tool), and a write token is rejected — tested.
• Two-level audit logging (tool intent + bridge data-plane with the file list): both are useful, I would keep both.
• Normalized basename matching (NFC, backslash, nested paths): covers the corrupted-name and subfolder cases — tested (1.MEMORY_BANK/activeContext.md).
• No breaking change: pushed stays numeric; pushed_files / skipped_volatile / warning are additive.
• Orphan cleanup deliberately scoped to the full bank (not the filtered set), with the explanatory comment — this correctly neutralizes the destructive-cleanup risk for volatiles still present in the bank.
• Empty batch short-circuited (call_tools_batch not called when there is nothing to do) — tested.
• Full suite on the PR branch: 419 passed + 1 xfailed (baseline 413 + 6 new adversarial tests). import json already present in graph_bridge.py. Mergeable CLEAN on post-v2.5.4 main.

Blocking 1 — Wave B criterion "legacy volatiles detected"

Volatiles already ingested in Graph Memory stay completely invisible: nothing in the response signals them, and they will silently rot there (the exact staleness problem the doctrine wants to avoid). No destructive cleanup asked — that stays out of scope, rightly. The minimal ask (~5-10 lines + 1 test):

• when volatile filenames are found among existing_docs, expose them in the response as existing_volatile_in_graph (list) with cleanup_performed: false and an explicit warning telling the operator how to remediate (e.g. delete via Graph Memory tooling, or a one-off include_volatile push after cleanup).

Blocking 2 — Wave B criterion "memory_id separation documented"

The most dangerous failure mode of this tool remains undocumented: if a memory_id is shared between graph_push and canonical-document ingestion (RFCs, runbooks ingested agent-side), the orphan cleanup deletes those canonical docs — any document absent from the bank is treated as an orphan. Pre-existing behavior, not introduced here — but this PR is precisely the guardrail revision the docs have been pointing to. A few lines in the tool docstring + the README Graph section are enough ("never share a memory_id between bank pushes and canonical ingestion"); a code-level guard can come with Wave C.

Recommended (non-blocking)

• Document two edge behaviors: (a) a volatile file removed from the bank does get cleaned from the graph by the orphan pass (opportunistic cleanup); (b) the guard protects the configured names, not the concept — a progress-2026.md is pushable by default.
• GRAPH_PUSH_VOLATILE_FILES="" silently disables the guard entirely — worth one sentence in the config comment/docs.

Versioning note (for the maintainers)

New tool parameter + new response fields + a default-behavior change: per the versioning doctrine this PR consumes the reserved 2.6.0 minor, and the behavior change (volatiles no longer pushed by default) must be called out in the CHANGELOG.

[chrlesur]

Adversarial pass on the guard itself (complements the review above)

Ran a hostile-input pass directly against _volatile_policy_key / push() with the real code, verifying each hypothesis with inline repros. The two Wave-B criteria in the review above stand; this adds one blocking item they didn't cover and two verified secondary findings.

Blocking — case-sensitive bypass
The guard compares the filename case-sensitively (_volatile_policy_key does NFC + strip + basename, no casefold() — graph_bridge.py:63/78), and _sanitize_filename does not normalize case (consolidator.py:2057). So Progress.md, ActiveContext.md, activeContext.MD all sail past the default guard and get pushed. Reachable via a bank_write (manage) or an LLM/rules capitalization drift — no include_volatile opt-in, no audit. A safety filter that a capital letter defeats isn't a filter. Minimal fix: casefold() both sides of the membership test (and state the match is case-insensitive).

Recommended — all-volatile bank still runs orphan cleanup
The early return only triggers on not bank_files, never not push_bank_files (graph_bridge.py:486). A bank containing only volatile files therefore still connects, calls document_list, and runs orphan cleanup. On a shared memory_id, that deletes canonical documents while pushing nothing — the "No non-volatile bank files to push" message is misleading. test_only_volatile_files_returns_empty_pushed_not_error doesn't catch it because it runs with existing_documents=[]. Tied to the memory_id separation point already flagged.

Recommended — backup_restore bypasses sanitization
backup_restore copies backup keys verbatim into the space (backup.py:287), without _sanitize_filename. An older backup holding an invisible-char name (e.g. pro​gress.md) reintroduces a key the guard won't skip (the guard re-implements a narrower normalization than _sanitize_filename, which does strip those chars — consolidator.py:2081). Not a remote exploit, operator-debt territory, but the fix is free: have _volatile_policy_key reuse _sanitize_filename's normalization instead of a parallel one.

Already noted (recommended): GRAPH_PUSH_VOLATILE_FILES="" silently disables the guard; and the bundled RULES templates use other volatile names (activeAnalysis.md/analysisProgress.md, roadmapProgress.md, progression.md) that the default activeContext.md,progress.md doesn't cover — the doctrine claims a broader concept than the default enforces.

Net: direction is right, but as the Wave-B server guardrail this needs the case fix (blocking) plus the two criteria from the review before it lands. Still a 2.6.0-class change.