Tenant repo sync advances revisions on error-bearing ingest

Context

Cloud pull-mode ingestion can report a healthy scheduler cycle even when the Knowledge Base rejected every vector write. A fresh source audit and executable falsifier on current dev reproduced the mismatch:

{
  "firstStatus": "completed",
  "firstRepoStatus": "active",
  "persistedRevision": "deadbeefcafebabe",
  "secondBaseRevision": "deadbeefcafebabe",
  "secondStatus": "completed"
}

The synthetic ingestion result contained ingested: 1, embeddingsGenerated: 0, and errors: [{"code":"KB_VECTOR_EMBED_FAILED"}]. Despite the error-bearing summary, the poller persisted the head revision and supplied it as the next incremental base.

Live latest-open sweep: checked the latest 20 open issues at 2026-07-23T10:10:45Z; no equivalent found. Targeted live searches for tenant-repo ingestion errors and lastIngestedRev found only closed predecessor tickets. A2A all-state sweep: checked the latest 30 messages inside the 60-minute herd window; no overlapping earlier claim found.

The Problem

KnowledgeBaseIngestionService.ingestSourceFiles() is intentionally fail-soft. It accumulates structured errors and returns its summary for both partial failures and caught top-level failures (ai/services/knowledge-base/IngestionService.mjs:211-228). A resolved promise therefore does not mean the ingest succeeded.

TenantRepoSyncService.syncTenantRepos() currently treats every resolved call as success. It invokes ingestSourceFiles(), immediately persists lastIngestedRev = envelope.headRevision, marks the repo active, and increments completedCount without inspecting ingestResult.errors (ai/daemons/orchestrator/services/TenantRepoSyncService.mjs:458-488). The unit-test fake always returns errors: [], so the failure-bearing return path is not covered (test/playwright/unit/ai/daemons/orchestrator/services/TenantRepoSyncService.spec.mjs:76-87).

This poisons incremental state. Once a failed head is stored, later runs diff from that head and can legitimately produce an empty envelope, so fixing the underlying embedding/config problem does not replay the skipped content.

The existing manual --repo-slug option only selects repositories; it still uses the stored base revision (ai/scripts/maintenance/syncTenantRepos.mjs:29-58,106-111). It is not a full replay mechanism.

The Architectural Reality

• KnowledgeBaseIngestionService owns the structured ingestion summary and its errors array.
• TenantRepoSyncService owns success/failure classification, retry/backoff state, and revision persistence.
• tenant-repo-sync-revisions.json is load-bearing incremental state. The closed #11790 contract says it advances only after successful ingestion.
• The existing per-repo catch path already preserves the last good revision, increments consecutiveFailures, and exposes stable outer/source error codes. Error-bearing summaries should enter that same path rather than create a second failure model.
• The manual maintenance CLI is the appropriate bounded recovery surface; this ticket does not add remote write authority.

Structure-map ownership: existing implementation stays in ai/daemons/orchestrator/services, ai/services/knowledge-base, and ai/scripts/maintenance; no new directory or service boundary is required.

The Fix

1. Validate the returned ingestion-summary shape before classifying a repo as successful.
2. Treat any non-empty ingestResult.errors as a failed repo attempt:
   • preserve the prior lastIngestedRev;
   • advance lastRunAttemptAt and increment consecutiveFailures so existing backoff applies;
   • retain KB_TENANT_REPO_SYNC_SYNC_FAILED as the stable outer code;
   • preserve the first bounded KB_* summary code as lastSourceErrorCode when present, without copying raw content or arbitrary details.
3. Treat a missing or non-array errors field as an invalid result and fail closed through the same stable outer error contract.
4. Add scoped full-replay recovery to syncTenantRepos.mjs, e.g. --full --repo-slug <slug>:
   • --full requires at least one explicit, repeatable repo selector;
   • selected repos build the next envelope with lastIngestedRev: null while retaining the persisted prior revision until completion;
   • a failed replay leaves persisted revision state unchanged;
   • only an error-free replay advances to the new head and resets failures.
5. Correct cloud-ingestion docs that currently claim partial ingestion failure leaves the revision unchanged, and document the scoped recovery command.

Contract Ledger Matrix

Decision Record impact

Aligned with ADR 0014's existing cloud-deployable tenant-repo-sync lane. No ADR amendment: this restores the shipped revision-success invariant and adds a bounded operator recovery mode within the existing service boundary.

Acceptance Criteria

• A returned ingestion summary with one or more errors marks that repo attempt failed/degraded rather than active/completed.
• Error-bearing, missing-errors, and malformed-errors summaries never advance lastIngestedRev.
• Failure state preserves the prior revision, increments consecutiveFailures, and participates in existing backoff.
• The first safe KB_* summary code can surface as bounded lastSourceErrorCode; raw messages, details, source content, repo URLs, and credentials do not.
• Mixed-repo cycles preserve current per-repo isolation and partial-success semantics while correctly counting the error-bearing repo as failed.
• The next incremental run after a failed ingest receives the last known-good revision, not the failed head.
• The manual CLI supports explicit scoped full replay; --full without --repo-slug is rejected.
• A failed full replay leaves stored revision state untouched; an error-free full replay advances to the new head and resets failures.
• Focused unit coverage exercises the real returned-summary seam, not only thrown exceptions.
• Cloud ingestion and troubleshooting docs describe fail-closed revision semantics and scoped recovery accurately.

Out of Scope

• Changing push-mode ingest_source_files response semantics.
• Adding a remote replay/actuator MCP tool.
• Automatically deleting or globally resetting revision state.
• Changing GitMirror acquisition, scheduler cadence, or embedding-provider configuration.
• Deployment-specific configuration or customer-specific runbooks.

Avoided Traps

• Do not infer success from a resolved promise, ingested > 0, or a non-null head; the structured errors array is load-bearing.
• Do not delete persisted revision state before replay. Keep the last known-good checkpoint until a clean full ingest proves replacement state.
• Do not expose arbitrary error messages/details through deployment diagnostics; preserve only bounded stable codes.

Related

Related: #11790

Related: #14396

Related: #14402

Origin Session ID: fc1a49c1-e30a-4e3a-960a-e0596367a4c1

Handoff Retrieval Hint: tenant-repo-sync error-bearing ingest summary lastIngestedRev poisoned revision scoped full replay