KB_REVISION_BOUNDARY_INVALID on first tenant sync (null baseRevision)

Context

Surfaced during the same first-real-world cloud-deployment exercise that produced #12036 and #12032. On the first tenant repo sync (no prior lastIngestedRev), the orchestrator-CLI reports status: completed and ingests chunks successfully, but the metrics row carries event_type=error with error_code=KB_REVISION_BOUNDARY_INVALID. The bookkeeping-vs-execution divergence is the bug, not the ingestion itself.

The Problem

KnowledgeBaseIngestionService.resolveRevisionTombstones:

async resolveRevisionTombstones({baseRevision, headRevision, tenantContext, summary}) {
    if (!baseRevision || !headRevision) {
        summary.errors.push(this.createError({
            code   : 'KB_REVISION_BOUNDARY_INVALID',
            message: '`baseRevision` and `headRevision` must be provided together.'
        }));
        return [];
    }
    // ...
}

The validator demands both-or-neither. On first sync, baseRevision is legitimately null (no prior state to diff against — priorState?.lastIngestedRev || null), but headRevision is set (the new HEAD just resolved). The validator pushes a spurious error onto summary.errors, which propagates to kb_ingestion_metrics as event_type=error.

Empirical evidence

From R3 first-sync metrics row:

{
  "event_type": "error",
  "chunks_total": 5,
  "chunks_embedded": 5,
  "chunks_deleted": 0,
  "error_code": "KB_REVISION_BOUNDARY_INVALID",
  "detail": "{\"errors\":[{\"code\":\"KB_REVISION_BOUNDARY_INVALID\",\"message\":\"`baseRevision` and `headRevision` must be provided together.\"}]}"
}

chunks_total=5 and chunks_embedded=5 — ingestion fully succeeded. But event_type=error because the tombstone-resolution validator flagged the boundary as "invalid" when it's actually just "first-sync = no boundary to resolve."

The Architectural Reality

• KnowledgeBaseIngestionService.mjs:716-724 — resolveRevisionTombstones validator.
• Tombstone resolution is for finding deletions across a revision boundary. With no baseRevision, there's no boundary to traverse → no deletions to resolve → the correct behavior is "return empty tombstone list, no error."

The Fix

Two options; PR author picks the cleaner one:

1. Early-return for null-base: when baseRevision === null && headRevision != null, return [] without pushing an error. The both-null case (no boundary at all) is also a clean no-op. Only the mixed case (baseRevision != null && headRevision == null) should error.

      if (!baseRevision) {
       return []; // first sync — no boundary, no tombstones, no error
   }
   if (!headRevision) {
       summary.errors.push(this.createError({
           code   : 'KB_REVISION_BOUNDARY_INVALID',
           message: '`headRevision` required when `baseRevision` is provided.'
       }));
       return [];
   }

2. Caller-side guard: have the upstream caller (envelope builder or ingestSourceFiles) not invoke resolveRevisionTombstones when baseRevision is null. Cleaner separation of concerns, but pushes the first-sync awareness into a different layer.

Option (1) is preferred — keeps the boundary semantics localized to the method that owns them.

Decision Record impact

aligned-with ADR 0014 — narrow bug fix within existing substrate.

Acceptance Criteria

• resolveRevisionTombstones returns [] cleanly when baseRevision is null (first-sync case), without pushing KB_REVISION_BOUNDARY_INVALID onto summary.errors.
• First-sync kb_ingestion_metrics row carries event_type consistent with the actual outcome (completed when chunks ingested successfully, not error).
• Existing both-non-null happy path remains unchanged.
• Existing mixed-null case (one set, one null where both expected) still errors loudly.
• Unit test: first-sync scenario (baseRevision=null, headRevision=<hash>) produces no summary.errors entries.

Out of Scope

• Bug A / B / C from #12036 (separate substrate gaps).
• Architectural redesign of tombstone resolution (in-process vs MCP-call) — orthogonal to this fix.

Avoided Traps

• Treating the metric event_type=error as the cosmetic-only symptom: that's the surface signal, but the underlying spurious-error propagation pollutes downstream consumers (health dashboards, observability, retry-decision logic). Fix the validator, not just the metric writer.

Related

• Sibling: #12036 (3 substrate gaps from R3), #12032 (credentialRef file: scheme)
• Parent epic: #11731 (Server-side tenant-repo ingestion)
• Empirical anchor: first cloud-deployment tenant-ingestion attempt, 2026-05-26