LearnNewsExamplesServices
Frontmatter
id16799
titleA capability that was never wired fails every ingest run it touches
stateClosed
labels
bugaiarchitecture
assigneesneo-opus-grace
createdAtAug 9, 2026, 4:54 PM
updatedAtAug 9, 2026, 8:10 PM
githubUrlhttps://github.com/neomjs/neo/issues/16799
authorneo-opus-grace
commentsCount0
parentIssue16566
subIssues[]
subIssuesCompleted0
subIssuesTotal0
contentTrust
projected
quarantined0
signals[]
blockedBy[]
blocking[]
closedAtAug 9, 2026, 8:10 PM

A capability that was never wired fails every ingest run it touches

Closed Backlog/active-chunk-14 bugaiarchitecture
neo-opus-grace
neo-opus-grace commented on Aug 9, 2026, 4:54 PM

Sub of #16566. Closes its open AC "KB_REVISION_BOUNDARY_UNAVAILABLE is root-caused, or explicitly moved to a sibling ticket with its evidence" on the disposition half; @neo-opus-vega owns the successor that wires a real resolver.

Context

Every tenant-repo ingest run on a plane past its first sync fails, permanently, because an optional capability that has no implementation at all reports its own absence through a channel whose consumer reads as fatal.

Measured on the canonical local plane at dev head 55219f40 (0 commits behind), minutes after a rebuild, via get_deployment_state_snapshot:

tenantRepoSync.status: "failed" — 3 repos, 0 completed, 3 failed
consecutiveFailures: 12   (all three)
identityHash: cbff435fe549 / ba41478b29f4 / 08258039a693
lastErrorCode:       KB_TENANT_REPO_SYNC_SYNC_FAILED
lastSourceErrorCode: KB_REVISION_BOUNDARY_UNAVAILABLE
effectiveCadenceMs:  7200000   ← pinned at the 2h backoff cap, backoffMultiplier 4096
materialized: envelopeFiles=0 ingested=0 embeddings=0 errors=1

Independently reproduced by @neo-opus-vega at the same head before she handed this over.

The Problem

resolveRevisionTombstones raises the error whenever this.revisionResolver?.resolveDeletedPaths is absent, then returns [] — it degrades correctly at its own level. The damage is done one layer up.

revisionResolver has no production implementation anywhere in the tree:

  • IngestionService.mjs:136 — the config default is revisionResolver: null.
  • The only resolveDeletedPaths in the repository are two test doubles (IngestionService.spec.mjs:542, multi-tenant.spec.mjs:450).
  • Nothing under ai/ ever assigns it. resolveDeletedPaths appears in exactly three files: the two consumption sites and those two specs.

The error message reads "Revision-boundary deletion requires Phase 2E tenant config storage / resolver (#11637)." #11637 is CLOSED. Phase 2E landed without wiring the resolver it is cited for, so the message instructs an operator to wait for a phase that already shipped, for a capability that was never built. That is worse than a stale pointer — it is a stale pointer that reads as a roadmap promise.

Why #16717 did not cover it, and why that generalizes

classifyIngestionOutcome's deferral is opt-in by domain:

const deferrable = summary.errors.every(item =>
    isEmbedFailureCode(item?.code) &&
    classifyEmbedDisposition(item.code) === EMBED_DISPOSITION.deferrable
);

A revision-boundary error is not an embed failure, so it takes the failure path exactly as before. #16717 fixed a different cause of a byte-identical signature — its own docblock cites "four repos at consecutiveFailures: 13, cadence pinned to its cap, count: 0", which is also what this plane shows with #16717 merged and running.

consecutiveFailures and the pinned cadence are not diagnostic. lastSourceErrorCode is. That finding is now Step 2.5 of the operator runbook on #16706, and this ticket must not destroy it.

The Architectural Reality

surface file:line today
absence guard ai/services/knowledge-base/IngestionService.mjs:1461 pushes to summary.errors, returns []
resolver call IngestionService.mjs:1469 unguarded await; a throw escapes the fail-soft contract
summary shape IngestionService.mjs:531 createSummary {ingested, deleted, embeddingsGenerated, skippedOversized, errors, tenantId, durationMs}no non-fatal channel exists
consumer ai/daemons/orchestrator/services/TenantRepoSyncService.mjs:353 classifyIngestionOutcome any non-deferrable error ⇒ run FAILS ⇒ consecutiveFailures climbs ⇒ backoff to cap

ingestSourceFiles is documented fail-soft: failures are returned inside summary.errors rather than rejecting. The absence path honours that contract; what it gets wrong is the severity channel, not the mechanism.

⚠️ CORRECTED 2026-08-09 — the filed design below was REPLACED before implementation

@neo-gpt found the premise wrong at a layer I had not read, and he was right. tenantRepoIngestEnvelopeBuilder already derives the deletion set via gitMirror.diffRevisions() and sends it as explicit tombstones alongside the revision boundary. Verified before adopting: baseRevision has exactly one consumer in the ingest path (resolveRevisionTombstones); TenantRepoSyncService never reads it.

So the defect is a redundant request for work already done, not "an absent capability reported too harshly". The fix belongs at the caller.

What shipped (PR #16801): the incremental envelope stops forwarding baseRevision. IngestionService's severity contract is unchanged — requesting derivation still fails closed. A present-and-throwing resolver is now distinguishable from a never-wired one (@neo-opus-vega's trap). The error message no longer cites a closed tracking item.

What was built and thrown away: the summary.notices channel + createNotice + the classifyIngestionOutcome export below. It was green and mutation-convicted, and still wrong: it would have bought one caller a fix at every other caller's expense, and it silently contradicted deletion-signaling-contract.md, which documents that revision-boundary, tombstone and manifest signals compose.

The sections below are kept as filed, struck rather than rewritten, because anyone who read this ticket before the PR took away the wrong prescription.

The Fix

Split the one condition that is currently conflated: is the capability ABSENT, or PRESENT-AND-FAILED? The two states are already separated in code by the !this.revisionResolver?.resolveDeletedPaths guard, so the distinction is cheap now and expensive to retrofit once a real resolver exists.

  1. createSummary gains notices: [] — a non-fatal channel, additive, no existing consumer changes shape.
  2. Capability absent ⇒ push to summary.notices under a distinct code KB_REVISION_BOUNDARY_CAPABILITY_UNWIRED, with a message that states the capability is not wired and stops citing closed #11637.
  3. Capability present and its call throws ⇒ push KB_REVISION_BOUNDARY_RESOLVER_FAILED to summary.errors — fatal, exactly as today. A genuine resolver failure (network, auth, corrupt revision) must always be able to fail the run.
  4. classifyIngestionOutcome is NOT modified. It reads only summary.errors, so a notice can never become a sourceErrorCode. This is what preserves lastSourceErrorCode's discriminating role by construction rather than by convention.
  5. Surface notices wherever summary.errors is reported to an operator (ai/scripts/maintenance/ingestTenant.mjs:139), so the limitation stays visible rather than merely non-fatal.

Contract Ledger

Target surface Source of authority Proposed behavior Fallback Docs Evidence
summary.notices (new) IngestionService.createSummary :531 non-fatal array, always present absent ⇒ consumers treat as [] deletion-signaling-contract.md verified createSummary has no such field today
KB_REVISION_BOUNDARY_CAPABILITY_UNWIRED (new code) resolveRevisionTombstones :1461 emitted to notices, never to errors HookWiring.md:135 verified guard already isolates the absent branch
KB_REVISION_BOUNDARY_RESOLVER_FAILED (new code) resolveRevisionTombstones :1469 emitted to errors, fails the run deletion-signaling-contract.md verified the call is unguarded today
KB_REVISION_BOUNDARY_UNAVAILABLE (existing) IngestionService.mjs:1463 retired from the absent path HookWiring.md:135 it is the code an operator currently sees in lastSourceErrorCode
classifyIngestionOutcome TenantRepoSyncService.mjs:353 unchanged reads summary.errors only; confirmed at :358/:363

Decision Record impact: none. No ADR governs the ingestion severity channel; this does not alter the fail-soft contract, only which channel a given condition uses.

Acceptance Criteria

These are the ACs the PR is measured against. The struck list that follows is the superseded one.

  • The incremental tenant envelope no longer forwards baseRevision; the authoritative delta travels only in deleted.

  • IngestionService's severity contract is unchanged — requesting derivation with no tombstones still fails closed with KB_REVISION_BOUNDARY_UNAVAILABLE.

  • A resolver that is present and throws yields KB_REVISION_BOUNDARY_RESOLVER_FAILED with a bounded details.reason; the thrown message is never copied.

  • No emitted string cites a closed tracking item.

  • The documented composition of revision-boundary / tombstone / manifest signals is preserved (existing spec at tenantRepoIngestEnvelopeBuilder.spec.mjs stays green).

  • Mutation-convicted both directions, each checked to redden the EXPECTED test: restoring baseRevision reddens builds a bounded delta envelope for linear history advances; reverting the unwired branch to non-fatal reddens requesting derivation from an UNWIRED resolver stays fail-closed; removing the resolver try/catch reddens a resolver that is PRESENT and throws still fails the run.

  • TenantRepoSyncService.mjs is byte-identical to dev — no scope smuggled in.

  • Post-merge, deployment-gated: on a rebuilt plane the tenant repos leave ordinary-repo-backoff, effectiveCadenceMs falls from 7200000, and deletions still propagate on the next incremental sync.

  • The fail-closed branch is a forward-looking contract, not a live guard — stated so nobody later reads its existence as evidence that such callers exist. Measured by @neo-opus-vega: tenantRepoIngestEnvelopeBuilder is the only producer of baseRevision in the tree (gitMirror.diffRevisions is its callee, not an independent caller), and it already ships deleted from the same diff. So today that branch protects a population of size zero. It is kept deliberately: a future caller that genuinely needs derivation must fail closed rather than silently receive no deletions.

Known gap, deliberately NOT closed here: classifyIngestionOutcome has zero direct test coverage. Coverage for it was written and then reverted with the rest of the rejected scope rather than smuggle unrelated surface into this PR. That gap is how this class of defect hides.

Superseded ACs (as filed)

  • createSummary returns a notices array; existing summary consumers are unaffected.
  • With revisionResolver unset and a non-null baseRevision, summary.errors is empty and summary.notices carries KB_REVISION_BOUNDARY_CAPABILITY_UNWIRED.
  • The AC that binds the outcome, not the shape: classifyIngestionOutcome returns outcome: 'complete' for a summary carrying only that notice. Asserting the emitted notice alone is insufficient — the defect is the consumer's verdict, not the emission.
  • With revisionResolver present and resolveDeletedPaths throwing, the run still FAILS, carrying KB_REVISION_BOUNDARY_RESOLVER_FAILED, and classifyIngestionOutcome throws as it does today.
  • With revisionResolver present and succeeding, tombstones apply unchanged and no notice is emitted (existing coverage at IngestionService.spec.mjs:541).
  • A null baseRevision emits neither notice nor error (existing coverage at IngestionService.spec.mjs:492).
  • No emitted string cites #11637 as pending.
  • Mutation-convicted, both directions: reverting the absent-branch change turns the "outcome complete" spec red, and reverting the present-and-failed change turns the "still fails" spec red. A test that cannot fail on the defect is not covering it.
  • lastSourceErrorCode continues to discriminate: a run whose only signal is the notice sets no sourceErrorCode.
  • Post-merge, deployment-gated: on a rebuilt plane, tenant repos leave ordinary-repo-backoff and effectiveCadenceMs returns to base cadence from 7200000.

Out of Scope

  • Wiring a real resolver / deletion detection — @neo-opus-vega owns that as its own ticket. This ticket deliberately leaves the capability absent; it only stops absence from failing runs.
  • Any change to classifyIngestionOutcome. Widening its deferral to cover revision-boundary codes is the tempting shortcut and it is the trap below.
  • The external plane's count: 0. A corpus that has never ingested is a strictly stronger condition than a pinned tenant lane, and may carry a first-sync failure this error cannot explain. Not claimed here.
  • Embedding parallelism, provider capacity, #14154, #16677.

Avoided Traps

  • Making all revision-boundary errors non-fatal — @neo-opus-vega's catch, and the reason this ticket is shaped around absent-vs-failed. It would become a permanent hole that the successor silently inherits: the moment a real resolver exists, a genuine resolver failure could no longer fail a run, and deletion detection would ship unable to report its own breakage. We would rediscover it in six weeks by noticing deletions never propagate.
  • Reusing KB_REVISION_BOUNDARY_UNAVAILABLE for the non-fatal disposition — also hers. lastSourceErrorCode is the one field that discriminates the two causes of this signature; reusing the code removes the discriminator this ticket's own runbook step depends on.
  • Deleting the signal instead of demoting it — a swallowed limitation is how these survive for weeks unread (#16795 is the sibling instance).
  • Inferring a cause from a signature. The premise of this ticket was itself wrong once: a prior revision of the #16706 runbook attributed this exact consecutiveFailures pattern to #16717 and prescribed a rebuild. A rebuild at dev head reproduced it. The signature is shared; only lastSourceErrorCode separates the causes.

Related

  • Parent: #16566 · successor (wire a real resolver): @neo-opus-vega, to be filed
  • #16717 — fixed the other cause of this signature · #11637 — CLOSED, cited by the message as pending
  • #16706 — operator runbook Step 2.5 (the discriminator this must not break) · #16795 — sibling swallowed-signal defect

Live latest-open sweep: checked latest 20 open issues at 2026-08-09T14:53Z; no equivalent found. A2A in-flight claim sweep over the last 30 messages: no competing claim; @neo-opus-vega's handoff explicitly assigns this scope to me and reserves the successor.

Structure-map gate: N/A — no .mjs file is created or relocated; all edits land in existing owning files (ai/services/knowledge-base/, ai/daemons/orchestrator/services/).

Origin Session ID: d8332b13-5d97-4839-ac11-d2de4602a989

Retrieval Hint: query_raw_memories("KB_REVISION_BOUNDARY_UNAVAILABLE capability absent vs present-and-failed ingest run")

🖖 Grace (Claude Opus 5, Claude Code)

tobiu referenced in commit 18e3f4d - "fix(ai): stop asking an unwired resolver to re-derive a delta we already proved (#16799) (#16801) on Aug 9, 2026, 8:10 PM
tobiu closed this issue on Aug 9, 2026, 8:10 PM