Skip to content

fix(feature-flags): don't connect to db before model is loaded (fixes #2066 unit regression + latent boot bug) #2073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jung-thomas merged 1 commit into from
Aug 28, 2026
Merged

fix(feature-flags): don't connect to db before model is loaded (fixes #2066 unit regression + latent boot bug) #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions srv/lib/feature-flags/db-flags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ function isFresh() {
/** Fire-and-forget background refresh, deduplicated. Never throws. */
function scheduleRefresh() {
if (STATE.refreshing) return;
// Do NOT trigger a `cds.connect.to('db')` before the model is loaded. A
// synchronous isFlagEnabled() reader can fire during the `cds.on('bootstrap')`
// phase (e.g. the MCP_AUTH_ENABLED kill switch in server.js) — at that point
// `cds.model` is not yet linked. cds.connect.to('db') caches cds.db eagerly
// (cds.services.db = the pending promise) and resolves its model from the
// partial, service-less model that exists mid-bootstrap; that poisoned db
// instance then persists, so cqn4sql can't resolve service-qualified targets
// and every later read throws "Query was not inferred and includes '*'".
// Skipping the refresh here is harmless: the getter returns the flag's
// declared registry default (exactly the intended cold-cache behavior) and
// the boot warm-up in server.js's `served` handler performs the real refresh
// once the model is fully loaded. Runtime-safe on every dialect (the poison
// is model resolution, not SQLite-specific).
if (!cds.model) return;
STATE.refreshing = refreshFeatureFlags()
.catch(() => {})
.finally(() => { STATE.refreshing = null; });
Expand Down
14 changes: 9 additions & 5 deletions test/unit/kg-path-between-handler.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const { getConceptsForUser } = await import('../../srv/lib/kg/concepts-for-user.
const { kgPathV2 } = await import('../../srv/lib/kg-path-v2-client.js')
const { findLearningPathHandler, FIND_LEARNING_PATH_TOOL } =
await import('../../srv/lib/kg/joule-tool-find-path.js')
// KG_PATH_V2_ENABLED migrated env → DB (ImsConfig key flag.kg.pathV2, #2060):
// the v2 branch is gated by isFlagEnabled(), so stubEnv no longer toggles it.
const { __setFlagForTest, __resetFlagsForTest } = await import('../../srv/lib/feature-flags/db-flags.js')

// ---------------------------------------------------------------------------
// Test helpers
Expand Down Expand Up @@ -515,6 +518,7 @@ describe('findLearningPathHandler — KG_PATH_V2 engine (issue #1253)', () => {
})
afterEach(() => {
vi.unstubAllEnvs()
__resetFlagsForTest()
})

// db.run dispatcher for the v2 branch: TUTORIALS hydration + CONCEPTS names.
Expand All @@ -530,7 +534,7 @@ describe('findLearningPathHandler — KG_PATH_V2 engine (issue #1253)', () => {
}

it('collapsed [A,B] path: 2 steps, B last, bridge lists concept names', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([
{ pathRank: 1, hopCount: 2, vertices: ['tutorial:abap-create-basic-app', 'concept:abap-cloud', 'concept:rap-bo', 'tutorial:abap-create-project'] },
])
Expand Down Expand Up @@ -558,7 +562,7 @@ describe('findLearningPathHandler — KG_PATH_V2 engine (issue #1253)', () => {
})

it('intermediate tutorial on path renders as an ordered middle step', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([
{ pathRank: 1, hopCount: 3, vertices: ['tutorial:a', 'concept:x', 'tutorial:m', 'concept:y', 'tutorial:b'] },
])
Expand All @@ -578,7 +582,7 @@ describe('findLearningPathHandler — KG_PATH_V2 engine (issue #1253)', () => {
})

it('direct tutorial↔tutorial path (no interior concepts) → Directly connected', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([
{ pathRank: 1, hopCount: 1, vertices: ['tutorial:a', 'tutorial:b'] },
])
Expand All @@ -595,7 +599,7 @@ describe('findLearningPathHandler — KG_PATH_V2 engine (issue #1253)', () => {
})

it('emits engine:v2 in path_returned telemetry', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([
{ pathRank: 1, hopCount: 1, vertices: ['tutorial:a', 'tutorial:b'] },
])
Expand All @@ -612,7 +616,7 @@ describe('findLearningPathHandler — KG_PATH_V2 engine (issue #1253)', () => {
})

it('v2 empty → falls through to v1 neighbor render (engine:v1)', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([])
kgQuery.mockResolvedValue({ response: buildJsonResponse([{ slug: 'cap-getting-started', pathType: 'SHARED_CONCEPT', rank: 3 }]) })
// v1 branch uses makeDb() dispatcher (TASKRECORDS/TUTORIALCONCEPTLINKS/TUTORIALS).
Expand Down
12 changes: 8 additions & 4 deletions test/unit/kg-path-v2-or-v1.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ vi.mock('../../srv/lib/kg-sparql-client.js', () => ({
const { kgPathV2 } = await import('../../srv/lib/kg-path-v2-client.js')
const { kgQuery } = await import('../../srv/lib/kg-sparql-client.js')
const { findPathV2OrV1 } = await import('../../srv/lib/kg-path.js')
// KG_PATH_V2_ENABLED migrated env → DB (ImsConfig key flag.kg.pathV2, #2060):
// kg-path.js now reads it via isFlagEnabled(), so stubEnv no longer toggles it.
const { __setFlagForTest, __resetFlagsForTest } = await import('../../srv/lib/feature-flags/db-flags.js')

const PFX = 'https://developers.sap.com/kg/tutorial/'
function v1Json(slugs) {
Expand All @@ -41,11 +44,12 @@ beforeEach(() => {
})
afterEach(() => {
vi.unstubAllEnvs()
__resetFlagsForTest()
})

describe('findPathV2OrV1 engine selection', () => {
it('flag on + v2 non-empty → returns engine v2 with vertices', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([
{ pathRank: 1, hopCount: 2, vertices: [`tutorial:a`, `concept:x`, `tutorial:b`] },
])
Expand All @@ -57,7 +61,7 @@ describe('findPathV2OrV1 engine selection', () => {
})

it('flag on + v2 empty → falls through to v1', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockResolvedValue([])
kgQuery.mockResolvedValue({ response: v1Json(['b']) })
const out = await findPathV2OrV1({ db, fromSlug: 'a', toSlug: 'b' })
Expand All @@ -66,7 +70,7 @@ describe('findPathV2OrV1 engine selection', () => {
})

it('flag on + v2 throws → falls through to v1 (fail-open)', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'true')
__setFlagForTest('KG_PATH_V2_ENABLED', true)
kgPathV2.mockRejectedValue(Object.assign(new Error('boom'), { code: 'ETIMEDOUT' }))
kgQuery.mockResolvedValue({ response: v1Json(['b']) })
const out = await findPathV2OrV1({ db, fromSlug: 'a', toSlug: 'b' })
Expand All @@ -75,7 +79,7 @@ describe('findPathV2OrV1 engine selection', () => {
})

it('flag off → v1 directly, kgPathV2 never called', async () => {
vi.stubEnv('KG_PATH_V2_ENABLED', 'false')
__setFlagForTest('KG_PATH_V2_ENABLED', false)
kgQuery.mockResolvedValue({ response: v1Json(['b']) })
const out = await findPathV2OrV1({ db, fromSlug: 'a', toSlug: 'b' })
expect(out.engine).toBe('v1')
Expand Down
Loading