Context
Follow-up captured by @neo-gpt's cycle-1 review of PR #12037 (which fixed Bug B in #12036 — TenantRepoSyncService.resolveIngestionService looking up the wrong export name).
The [KB_GAP] finding: existing unit coverage in test/playwright/unit/ai/daemons/orchestrator/services/TenantRepoSyncService.spec.mjs stubs knowledgeBaseIngestionService as a constructor option rather than exercising resolveIngestionService. The bug was therefore invisible at unit-test time and only surfaced via the empirical R3 first cloud tenant-ingestion smoke. Same general anti-pattern as feedback_stub_tests_miss_adapter_drift (#11999).
The Problem
When the canonical exported symbol in ai/services.mjs was renamed (or never matched what the resolver code expected), no test caught the drift because the spec injected the dependency directly and never asked the resolver to look up the real one. Future renames of KB_IngestionService would re-introduce the same Day-0 blocker.
The Architectural Reality
The Fix
Add a narrow real-resolver smoke test that:
- Imports
TenantRepoSyncService without injecting knowledgeBaseIngestionService.
- Calls
service.resolveIngestionService() directly.
- Asserts the return value is non-
undefined and structurally matches the KB_IngestionService shape (has ingestSourceFiles method).
This catches:
- Any future rename of
KB_IngestionService in ai/services.mjs without a corresponding update in TenantRepoSyncService.resolveIngestionService.
- Any future deletion of the export entirely.
- Any future structural change where the resolved object loses
ingestSourceFiles.
The smoke does NOT need to invoke ingestSourceFiles — just verify the resolver returns an object with the expected method-existence contract. Cheap test, high regression-protection ROI.
Decision Record impact
aligned-with ADR 0014 — narrow test-coverage gap fix.
Acceptance Criteria
Out of Scope
- Broader "real-vs-stub" audit of other orchestrator-side resolvers — separate concern. This ticket is narrow: one resolver, one regression-protection test.
- Refactoring
TenantRepoSyncService.spec.mjs to use the real ingestion service throughout — orthogonal; the existing stub-based tests are fine for orchestration-logic isolation.
- Architectural redesign of orchestrator→ingestion invocation (in-process vs MCP-call) — separate concern, documented in #12037 body.
Avoided Traps
- Over-broad real-resolver suite: tempting to add real-resolver tests for every service the orchestrator imports. Restraint — start with the one that actually escaped (
resolveIngestionService). Add others only when drift recurs.
- Mocking
services.mjs per-test to simulate the broken state: better is one negative-case test that re-imports services.mjs via a mocking layer (or a snapshot of the original module with the export deleted) — keeps the positive case simple and the negative case isolated.
Related
- Empirical anchor: @neo-gpt cycle-1 review of PR #12037
[KB_GAP] finding.
- Parent: #12036 (3 substrate gaps from R3 first cloud tenant-ingestion smoke).
- Sibling pattern:
feedback_stub_tests_miss_adapter_drift (#11999 cycle-1).
- Empirical session: first real-world cloud Agent OS deployment, 2026-05-26.
Context
Follow-up captured by @neo-gpt's cycle-1 review of PR #12037 (which fixed Bug B in #12036 —
TenantRepoSyncService.resolveIngestionServicelooking up the wrong export name).The
[KB_GAP]finding: existing unit coverage intest/playwright/unit/ai/daemons/orchestrator/services/TenantRepoSyncService.spec.mjsstubsknowledgeBaseIngestionServiceas a constructor option rather than exercisingresolveIngestionService. The bug was therefore invisible at unit-test time and only surfaced via the empirical R3 first cloud tenant-ingestion smoke. Same general anti-pattern asfeedback_stub_tests_miss_adapter_drift(#11999).The Problem
When the canonical exported symbol in
ai/services.mjswas renamed (or never matched what the resolver code expected), no test caught the drift because the spec injected the dependency directly and never asked the resolver to look up the real one. Future renames ofKB_IngestionServicewould re-introduce the same Day-0 blocker.The Architectural Reality
ai/daemons/orchestrator/services/TenantRepoSyncService.mjs:564-567—resolveIngestionService(post-#12037: returnsservices.KB_IngestionService).ai/services.mjs:212,:282— canonicalKB_IngestionServiceexport.TenantRepoSyncService.spec.mjscovers the orchestration logic with stubknowledgeBaseIngestionService;resolveIngestionServiceis bypassed via constructor injection.The Fix
Add a narrow real-resolver smoke test that:
TenantRepoSyncServicewithout injectingknowledgeBaseIngestionService.service.resolveIngestionService()directly.undefinedand structurally matches theKB_IngestionServiceshape (hasingestSourceFilesmethod).This catches:
KB_IngestionServiceinai/services.mjswithout a corresponding update inTenantRepoSyncService.resolveIngestionService.ingestSourceFiles.The smoke does NOT need to invoke
ingestSourceFiles— just verify the resolver returns an object with the expected method-existence contract. Cheap test, high regression-protection ROI.Decision Record impact
aligned-with ADR 0014— narrow test-coverage gap fix.Acceptance Criteria
TenantRepoSyncService.spec.mjs(or a sibling spec) that exercisesresolveIngestionService()against the realai/services.mjsimport.undefined.typeof .ingestSourceFiles === 'function'(or equivalent structural check).services.mjsis mocked to NOT exportKB_IngestionService(verifies the test actually catches export-drift, not just runs).services.KB_IngestionServiceexists).Out of Scope
TenantRepoSyncService.spec.mjsto use the real ingestion service throughout — orthogonal; the existing stub-based tests are fine for orchestration-logic isolation.Avoided Traps
resolveIngestionService). Add others only when drift recurs.services.mjsper-test to simulate the broken state: better is one negative-case test that re-importsservices.mjsvia a mocking layer (or a snapshot of the original module with the export deleted) — keeps the positive case simple and the negative case isolated.Related
[KB_GAP]finding.feedback_stub_tests_miss_adapter_drift(#11999 cycle-1).