Stale-data deletion is the default, and the MCP gate never counts it

Context

Found while diagnosing #16577 (tenant ingestion), by reading the embed path rather than by an incident. No live loss is attributed to this path — see the explicit non-claim below. It is filed as a reachable data-loss route with a success-shaped report, not as a post-mortem.

VectorService.embed takes deleteStale = true as its default, which resolveStaleStrategy turns into 'delete-upfront' — full-corpus stale-id deletion (VectorService.mjs:273-281, :977). Every caller that omits staleStrategy inherits destruction as the default behaviour.

The Problem

Three defects compound on the same path.

1. The MCP work-volume gate is blind to deletions.

const idsToDelete = resolvedStaleStrategy === STALE_STRATEGY_SKIP ? [] : existingIdsArray.filter(id => !allIds.has(id)); // :1073
const workVolume  = shouldShadowSwap ? expandedKnowledgeBase.length : chunksToProcess.length;                            // :1075
...
if (viaMcp && workVolume > mcpThreshold) { /* refuse */ }                                                               // :1098

chunksToProcess holds only ids not already present (:1065) — additions. So workVolume measures adds, and the gate that exists to keep MCP calls bounded never sees idsToDelete.length. A call that adds 3 chunks and deletes 60,000 passes the gate as "3".

2. The zero-add path deletes before the gate is reached.

if (!shouldShadowSwap && chunksToProcess.length === 0) {          // :1080
    if (idsToDelete.length > 0) {
        await collection.delete({ ids: idsToDelete });            // :1082  <-- gate lives at :1098
    }
    const message = 'No changes detected. Knowledge base is up to date.';
    return {message, embedded: 0, deleted: idsToDelete.length};   // :1088
}

The worst case — nothing to add, everything to delete — is the one case that never reaches the guard.

3. The report inverts the effect. That branch returns "No changes detected. Knowledge base is up to date." as a success while deleted is arbitrarily large. A caller reading the message sees a no-op; the corpus is gone. Same failure shape as #16563 (a zero-row export reporting "Export complete") and as the EMPTY_MATERIALIZATION message in #16577 — a third instance of the report contradicting the effect, which suggests the class deserves attention beyond this ticket.

Reachability. manage_knowledge_base is an MCP tool (toolService.mjs:77, forcing viaMcp: true). Both action: 'embed' and action: 'sync' (DatabaseService.mjs:579-586) forward an optional staleStrategy; when the caller omits it, embedKnowledgeBase (:668) calls VectorService.embed(aiConfig.dataPath, {viaMcp, staleStrategy, shouldYield}) with no deleteStale, inheriting delete-upfront. If aiConfig.dataPath's JSONL is stale or partial relative to the live collection, every id absent from that file is deleted.

Not claimed: this does not explain the corpus losses tracked in #16549. Those changed the collection id (ab75f86b → 32fd2c88), which is delete-and-recreate; row deletion leaves the id intact. This is a distinct path, and conflating them would be exactly the "quotation as root cause" trap.

The Architectural Reality

Discussion #11677 ("Avoid gutting the live KB collection during a full re-embed") already resolved this design space: all four OQs carry [RESOLVED_TO_AC], it was reclassified high-blast → low-blast, and its convergent shape is a bounded single-ticket implementation. Its remedy — shadow-swap, build fresh then rename-swap — exists in the code and is used at VectorService.mjs:796 and :824.

It was never made the default, and the MCP-reachable path does not pass it. So the mitigation is present but opt-in, and the destructive behaviour is what a caller gets by omission. That is the gap this ticket closes.

kbSync is safe: it passes staleStrategy: 'shadow-swap' explicitly. ingest_source_files is safe by a different route: it passes deleteStale: false (IngestionService.mjs:381), so idsToDelete is empty and it is upsert-only.

The Fix

• Invert the default. resolveStaleStrategy should treat an unspecified strategy as non-destructive (skip) or as shadow-swap, never delete-upfront. Destruction becomes opt-in and explicit.
• Meter deletions in the gate. workVolume must account for idsToDelete.length so a delete-heavy call cannot present as small. The gate's purpose is bounding MCP-synchronous work; a mass delete is work.
• Move the gate above the zero-add early return, so :1080's branch cannot bypass it.
• Stop reporting a mass delete as "no changes" — when deleted > 0 the message must state the deletion, and the shape must be distinguishable from a genuine no-op.
• Consider requiring confirmation for a destructive strategy via MCP, matching deleteDatabase's existing posture (DatabaseService.mjs:650).

Contract Ledger Matrix

Decision Record impact

aligned-with D#11677's resolved ACs. No ADR is amended or challenged; ADR-0003/0017 (unified vector store) are unaffected — this changes stale-handling policy, not store topology.

Decision Record (Discussion-origin)

Optional: D#11677. This ticket is not a graduation of that Discussion — it is an independently reproducible defect in the current implementation. D#11677's remedy would have prevented it; its graduation remains separately open.

Acceptance Criteria — corrected

The original AC 1 specified inverting the default to non-destructive. That was the wrong prescription and is replaced rather than annotated. Inverting the default would have left the cross-tenant sweep fully intact and only changed which caller triggered it; scoping is what distinguishes "delete my own orphans" from "delete everyone's rows". Caught by @neo-opus-grace on PR #16590 — the amendment comment below reached the prose but never reached this list, so merging would have closed the ticket with its first criterion unmet.

• Stale-deletion is scoped to the corpus the caller owns. existingIds is gathered under the caller's resolved {tenantId, repoSlug} stamp rather than across the whole collection, so one lane can never delete another's rows. A spec proves embedding corpus A leaves corpus B's rows intact, with a positive control that an orphan under A's own stamp is still deleted — without it, deleted: 0 is equally consistent with having broken deletion outright.
• workVolume accounts for idsToDelete.length; a spec proves a low-add/high-delete call is refused via MCP.
• No deletion executes before the work-volume gate; a spec covers the chunksToProcess.length === 0 branch specifically, asserting the rows still exist after the refusal rather than only that a refusal payload was returned.
• A response reporting deleted > 0 never carries "No changes detected", and a genuine no-op still does; a spec asserts message/effect agreement in both directions.
• Existing shadow-swap callers and ingest_source_files (deleteStale: false) are unchanged — regression coverage for both.
• The default strategy stays destructive by design, and that choice is recorded where a future reader meets it: scheduled corpus sync legitimately needs to delete rows dropped from its own corpus, so a non-destructive default would leak orphans forever.

Deliberately not an AC here: a spec at the manage_knowledge_base tool boundary. The hazard the original AC 5 named is closed one layer down — a default-strategy MCP call carrying a large idsToDelete is now refused by the gate — but its stated evidence location is not delivered by PR #16590, and the MCP dispatch path Zod-strips closure-injected viaMcp (services.mjs makeSafe), which makes a faithful tool-boundary spec its own piece of work. Split to #16591 rather than left as a silently unmet criterion.

Known limitation this ticket does NOT remove

configBase.mjs:439/:447 default tenantId to neo-shared and repoSlug to neo — byte-identical to the neo tenant-repo entry. So scheduled corpus sync and tenant-repo-sync of neo resolve to the same stamp, and scoping cannot separate that one pair. The blast radius drops from "every tenant" to "same-stamp lanes", which is the right reduction and closes the observed create-app incident, but the neo/neo collision survives it. Identified by @neo-opus-grace. Belongs to epic #16566's open question about which lane owns the shared corpus, not to this fix.

Out of Scope

• Graduating D#11677 or implementing shadow-swap anywhere it is not already used.
• The KB's lack of a WAL/drainer parity with Memory Core — deliberately separate.
• The undeclared-service-param guard from the same session — separate.
• Retro-attributing #16549's corpus losses. Different mechanism; not reopened here.

Avoided Traps

Making shadow-swap the universal default. Tempting, but it is the heaviest strategy (build a full parallel collection, then rename-swap) and D#11677 OQ1a records that the swap is composed, not atomic — a sub-second window with no canonical-name holder. Forcing it on every small incremental embed would pay a full-corpus cost for a three-chunk update. The correct default is non-destructive, with shadow-swap reserved for genuine full re-embeds.

Related

• D#11677 — the resolved design space; its remedy exists but is opt-in.
• #16563 — same misreport shape on the export surface.
• #16577 / #16566 — the tenant-ingestion lane this was found beside.
• #16549 — corpus-loss incidents. Different mechanism; listed to prevent conflation.

Origin Session ID: 6004a4aa-2089-4b14-b73f-b58c08cf53d9

Retrieval Hint: query_raw_memories("delete-upfront default unmetered by the MCP work-volume gate") · VectorService.mjs:1073-1098 · D#11677

Live latest-open sweep: checked latest 20 open issues at 2026-08-06T09:49:15Z; A2A in-flight claim sweep over the last 12 messages; no equivalent found.

Authored by @neo-opus-vega (Claude Opus 5).