Project: doctolib-worktrees · Branch: HEAD · Mode: ` · **Model:**claude-opus-4-7`
Stats: 1 user messages, 23 tool calls, tools used: Bash, Read.
Conversation
Turn 1 — User
Review PR #7487 from doctolib/doctolib-pro-desktop titled "refactor(FHIR-2177): remove dead useConnectorContextFromElectronSettings flag".
PR Description
Removes the dead useConnectorContextFromElectronSettings flag from TiConnectSettings and all related code. The flag defaulted to false and nothing in DDV or the monolith ever set it to true after the TiConnectChangeMode IPC event was removed in FHIR-2040. This made the true branch in ConfigurationService.obtainConfiguration() permanently unreachable in production.
Files Changed (7 files, +35/-81 lines)
src/modules/de/kbv-ti-connect/services/configuration-service/index.ts (key logic change)
- // Keep existing behavior: when enabled, settings provide the connector context.
- if (settings.useConnectorContextFromElectronSettings) {
- contextToUse = {
- MandantId: settings.mandantId,
- ClientSystemId: settings.clientSystemId,
- WorkplaceId: settings.workplaceId,
- UserId: settings.userId,
- }
- } else if (contextFromRequest) {
+ if (contextFromRequest) {
contextToUse = this.normalizeRequestContext(contextFromRequest)
} else {
src/modules/de/kbv-ti-connect/services/types.ts
export interface TiConnectSettings {
mandantId: string
workplaceId: string
userId: string
- useConnectorContextFromElectronSettings: boolean
serverCertificateFingerprint: string
Also removed from UserEditableTiConnectSetting exclusion union.
src/modules/de/kbv-ti-connect/services/ti-connect-settings.ts
export const EMPTY_TI_SETTINGS: TiConnectSettings = {
mandantId: '',
workplaceId: '',
userId: '',
- useConnectorContextFromElectronSettings: false,
serverCertificateFingerprint: '',
src/modules/de/kbv-ti-connect/services/mock/mockStore.ts
- useConnectorContextFromElectronSettings: true,
Note: the mock store previously had this set to true, meaning tests were exercising the settings-based context path before this PR.
src/modules/de/gematik-kim/services/__tests__/account.test.ts
Removed useConnectorContextFromElectronSettings: false from a mock fixture.
src/modules/de/kbv-ti-connect/main/__tests__/ti-connect.test.ts
Removed useConnectorContextFromElectronSettings: true from a mock fixture.
Full current configuration-service/index.ts (after change):
export default class ConfigurationService {
static readonly configurationService = { name: 'ConfigurationService' }
readonly #authSignAlgorithm: CryptType
readonly #context: ConnectorContext
readonly #config: ConnectorConfiguration
readonly #isJwaSelectionEnabled: boolean
readonly #isEccSignatureEnforced: boolean
private constructor({ authSignAlgorithm, context, config, isJwaSelectionEnabled, isEccSignatureEnforced }) { ... }
private static readonly normalizeRequestContext = (contextFromRequest: ConnectorContextInput): ConnectorContext => {
if ('MandantId' in contextFromRequest) {
return { MandantId: contextFromRequest.MandantId, ClientSystemId: contextFromRequest.ClientSystemId, WorkplaceId: contextFromRequest.WorkplaceId, UserId: contextFromRequest.UserId || '' }
}
return { MandantId: contextFromRequest.mandantId, ClientSystemId: contextFromRequest.clientSystemId, WorkplaceId: contextFromRequest.workplaceId, UserId: contextFromRequest.userId ?? '' }
}
public static obtainConfiguration(traceId: string | number, contextFromRequest?: ConnectorContextInput): ConfigurationService {
let contextToUse: ConnectorContext
const { settings } = TiConnectRepository
if (!settings) {
throw new Error(i18n.t('ti_connect.error_messages.missing_connector_context'))
}
if (contextFromRequest) {
contextToUse = this.normalizeRequestContext(contextFromRequest)
} else {
kbvTiConnectLogger.error(...)
throw new Error(i18n.t('ti_connect.error_messages.missing_connector_context'))
}
…(truncated, 771 more chars)
Turn 1 — Assistant
I'll review this PR carefully. Let me first check the actual files in the repository to verify the claims made in the PR description.
Tools used:
Bash:find /Users/USER/workspace/doctolib/worktrees -type d -name "doctolib-pro-desktop*" 2>/dev/null | head -20
Tool results:
→ result (ok): (Bash completed with no output)
I don't have access to the actual repo files locally — the worktrees directory is empty. I'll review based on the diff and description provided in the PR, using my judgment to identify high-confidence issues.
Tools used:
Bash:ls /Users/USER/workspace/doctolib/worktrees/ 2>&1; ls /Users/USER/workspace/doctolib/ 2>&1 | head -20
Tool results:
→ result (ok): cleanup-merged-worktrees.sh doctolib treatment-plan actions billeo-engine c3-rest-api-guidelines docto-crypt doctoboot doctolib doctolib-pro-desktop github-workspaces kube misc oxygen prompts software-design-training terraform-aws-secrets terraform-infra test-failures ti-client treatment-plan unified-healthcare-data-platform-infra upfront
Tools used:
Bash:ls /Users/USER/workspace/doctolib/doctolib-pro-desktop/ 2>&1 | head -20
Tool results:
→ result (ok): AGENTS.md appData biome.json bugs catalog-info.yaml CLAUDE.md config CONTRIBUTING.md coverage ddv.code-workspace dist Dockerfile docs index.html instance.id lefthook.yml licenses logs mise.lock mkdocs.yml
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && git log --oneline -5 2>&1; git branch --show-current 2>&1
Tool results:
→ result (ok): 3b7320cf6 chore(deps): bump dompurify from 3.3.3 to 3.4.0 (#7459) 79da7b88b chore(deps): update doctolib github actions (#7462) be4b60942 chore(SECOPS-804): add mise.lock to pin tool checksums (#7448) de7ed05f8 feat(wolf-4377): Move to the General documents group (#7467) 85537cb03 ci: [ci skip] edit request claude workflow master
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && gh pr view 7487 --json title,state,files,additions,deletions 2>&1 | head -100
Tool results:
→ result (ok): {"additions":35,"deletions":81,"files":[{"path":"src/modules/de/gematik-kim/services/tests/account.test.ts","additions":0,"deletions":1,"changeType":"MODIFIED"},{"path":"src/modules/de/kbv-ti-connect/main/tests/ti-connect.test.ts","additions":0,"deletions":1,"changeType":"MODIFIED"},{"path":"src/modules/de/kbv-ti-connect/services/configuration-service/tests/index.test.ts","additions":34,"deletions":66,"changeType":"MODIFIED"},{"path":"src/modules/de/kbv-ti-connect/services/configurat …(truncated, 535 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && gh pr diff 7487 2>&1
Tool results:
→ result (ok): diff --git a/src/modules/de/gematik-kim/services/tests/account.test.ts b/src/modules/de/gematik-kim/services/tests/account.test.ts index 49af5cbe08..1cea5bf357 100644 --- a/src/modules/de/gematik-kim/services/tests/account.test.ts +++ b/src/modules/de/gematik-kim/services/tests/account.test.ts @@ -26,7 +26,6 @@ const mockTiSettings: TiConnectSettings = { mandantId: '', workplaceId: '', userId: '', - useConnectorContextFromElectronSettings: false, serverCertificateFin …(truncated, 12889 more chars)
Now let me look at the actual current state of key files to check for correctness and any potential data persistence/migration concerns.
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/configuration-service/index.ts
Tool results:
→ result (ok): 1 import i18n from 'i18next'
2
3 import {
4 ConnectorConfiguration,
5 ConnectorContext,
6 ConnectorContextInput,
7 CryptType,
8 } from '@module/de/kbv-ti-connect/services/types'
9 import { extraLogParser, getNestedKeys } from 'src/utils/extraLogParser.ts'
10
11 import { kbvTiConnectLogger } from '../logger'
12 import { TiConnectRepository } from '../ti-connect-repository'
13
14 export default class ConfigurationService {
15 static readonly configurationService = { name: 'Configurati
…(truncated, 4880 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/types.ts
Tool results:
→ result (ok): 1 export type TiConnectAction = 2 | 'startConnection' 3 | 'testConnectorConfiguration' 4 | 'subscribeToEventService' 5 | 'unsubscribeFromEventService' 6 | 'readVsd' 7 | 'readKvk' 8 | 'getSmcbPinStatus' 9 | 'changePin' 10 | 'handleSmcb' 11 | 'unblockPin' 12 | 'enableOfflineMode' 13 | 'enableOnlineMode' 14 | 'verifyCertificate' 15 | 'readCardCertificate' 16 | 'externalAuthenticate' 17 | 'idpAuthenticate' 18 | 'persistConfigurationAndRestartEventServer' 19 | 'che …(truncated, 6982 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && grep -rn "useConnectorContextFromElectronSettings" --include="*.ts" --include="*.tsx" --include="*.js" 2>&1 | head -50
Tool results:
→ result (ok): src/modules/de/gematik-kim/services/tests/account.test.ts:29: useConnectorContextFromElectronSettings: false, src/modules/de/kbv-ti-connect/main/tests/ti-connect.test.ts:111: useConnectorContextFromElectronSettings: true, src/modules/de/kbv-ti-connect/services/ti-connect-settings.ts:15: useConnectorContextFromElectronSettings: false, src/modules/de/kbv-ti-connect/services/types.ts:160: useConnectorContextFromElectronSettings: boolean src/modules/de/kbv-ti-connect/services/types.ts: …(truncated, 588 more chars)
Good — searches confirm current state before this PR. Let me check for migration-related concerns in the electron-store code.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && grep -rn "migration\|migrate" src/modules/de/kbv-ti-connect/ --include="*.ts" 2>&1 | head -20
Tool results:
→ result (ok): src/modules/de/kbv-ti-connect/services/ti-connect-store.ts:21:function migrate(): void { src/modules/de/kbv-ti-connect/services/ti-connect-store.ts:31: migrate()
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/ti-connect-store.ts
Tool results:
→ result (ok): 1 import { moduleStore, Store } from '@core/store'
2
3 import { TICONNECT_SCHEMA_LABEL, TiConnectSchema } from '../constants/types'
4
5 const store = moduleStore(TICONNECT_SCHEMA_LABEL) as Store<TiConnectSchema>
6
7 /*
8 * Migrate from legacyStore to dedicated TI Connect store.
9 * All keys remains the same
10 /
11 const storeVersion0 = 0
12
13 const latestVersion = storeVersion0
14
15 export const TI_SCHEMA_KEY_INFO = 'ti_connection_info'
16 export const TI_SCHEMA_KEY_SETTINGS = 'ti_c
…(truncated, 465 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && find src/modules/de/kbv-ti-connect -name "ti-connect-repository*" -type f 2>&1
Tool results:
→ result (ok): src/modules/de/kbv-ti-connect/services/ti-connect-repository.ts
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/ti-connect-repository.ts
Tool results:
→ result (ok): 1 import { getTiConnectStore, TI_SCHEMA_KEY_INFO, TI_SCHEMA_KEY_SETTINGS } from './ti-connect-store'
2 import { TiConnectionInfo, TiConnectSettings } from './types'
3
4 /
5 * TiConnectRepository is a wrapper around the store to retrieve and persist Ti Connect data
6 */
7 export class TiConnectRepository {
8 / Retrieve Ti Connect Settings from the store */
9 static get settings(): TiConnectSettings | undefined {
10 return getTiConnectStore().get(TI_SCHEMA_KEY_SETTINGS)
11 }
12
1
…(truncated, 1071 more chars)
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && find src/modules/de/kbv-ti-connect -name "types.ts" -path "*/constants/*" 2>&1 ; grep -n "TiConnectSchema" src/modules/de/kbv-ti-connect/ …(truncated, 23 more chars)
Tool results:
→ result (ok): src/modules/de/kbv-ti-connect/constants/types.ts 7:export interface TiConnectSchema {
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/constants/types.ts
Tool results:
→ result (ok): 1 import { TiConnectionInfo, TiConnectSettings } from '../services/types'
2
3 /
4 * TiConnect storage schema
5 * Name: 'ticonnect'
6 */
7 export interface TiConnectSchema {
8 /
9 * Schema version
10 */
11 schema_version: number
12 ti_connect: TiConnectSettings // Labs
13 ti_connection_info: TiConnectionInfo
14 }
15
16 export const TICONNECT_SCHEMA_LABEL = 'ticonnect'
17
Now let me check the tests file completely, check where mandantId/clientSystemId/workplaceId/userId in settings are used, and check for edge cases.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && grep -rn "settings\.mandantId\|settings\.workplaceId\|settings\.userId\|settings\.clientSystemId" --include="*.ts" --include="*.tsx" 2>&1 …(truncated, 11 more chars)
Tool results:
→ result (ok): src/modules/de/kbv-ti-connect/services/configuration-service/index.ts:77: MandantId: settings.mandantId, src/modules/de/kbv-ti-connect/services/configuration-service/index.ts:78: ClientSystemId: settings.clientSystemId, src/modules/de/kbv-ti-connect/services/configuration-service/index.ts:79: WorkplaceId: settings.workplaceId, src/modules/de/kbv-ti-connect/services/configuration-service/index.ts:80: UserId: settings.userId,
Interesting — the settings context fields (mandantId, clientSystemId, workplaceId, userId) are ONLY used at the deleted code path. They have no other readers in the codebase.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && grep -rn "mandantId\|clientSystemId\|workplaceId\|userId" --include="*.ts" --include="*.tsx" src/modules/de/kbv-ti-connect/ 2>&1 | grep - …(truncated, 34 more chars)
Tool results:
→ result (ok): src/modules/de/kbv-ti-connect/services/types.ts:156: clientSystemId: string src/modules/de/kbv-ti-connect/services/types.ts:157: mandantId: string src/modules/de/kbv-ti-connect/services/types.ts:158: workplaceId: string src/modules/de/kbv-ti-connect/services/types.ts:159: userId: string src/modules/de/kbv-ti-connect/services/types.ts:185: mandantId: string src/modules/de/kbv-ti-connect/services/types.ts:186: clientSystemId: string src/modules/de/kbv-ti-connect/services/types.ts:187: workp …(truncated, 2120 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/ti-connect-settings.ts
Tool results:
→ result (ok): 1 import { TiConnectSettings } from './types'
2
3 export const EMPTY_TI_SETTINGS: TiConnectSettings = {
4 connectorHost: '',
5 connectorPort: '',
6 eventServiceHost: '',
7 eventServicePort: '',
8 keyStorePath: '',
9 cert: '',
10 privateKey: '',
11 clientSystemId: '',
12 mandantId: '',
13 workplaceId: '',
14 userId: '',
15 useConnectorContextFromElectronSettings: false,
16 serverCertificateFingerprint: '',
17 serverCertificate: '',
18 isCertificatesVerificationEnabl
…(truncated, 713 more chars)
Interesting — isTiContextLocal at line 37-39 is a dead function related to the same flag. Let me verify.
Tools used:
Bash:cd /Users/USER/workspace/doctolib/doctolib-pro-desktop && grep -rn "isTiContextLocal" --include="*.ts" --include="*.tsx" 2>&1
Tool results:
→ result (ok): src/modules/de/kbv-ti-connect/main/tests/module.test.ts:18: isTiContextLocal: vi.fn(), src/modules/de/kbv-ti-connect/main/tests/module.test.ts:60:import { isTiConfigured, isTiContextLocal } from '@module/de/kbv-ti-connect/services/ti-connect-settings' src/modules/de/kbv-ti-connect/main/tests/module.test.ts:68:const mockedIsTiContextLocal = vi.mocked(isTiContextLocal) src/modules/de/kbv-ti-connect/services/ti-connect-settings.ts:37:export const isTiContextLocal = (settings: TiConnect …(truncated, 735 more chars)
isTiContextLocal IS still used in module.ts line 46. Let me look at that.
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/main/module.ts(1–101)
Tool results:
→ result (ok): 1 import { v4 } from 'uuid'
2
3 import { DdvModule } from '@core/modules-management/main'
4 import { isTiConfigured, isTiContextLocal } from '@module/de/kbv-ti-connect/services/ti-connect-settings'
5 import { extraLogParser } from 'src/utils/extraLogParser'
6
7 import { registerTiConnectEvents, removeTiConnectEvents } from './ti-connect'
8
9 import { tiConnectModuleMetadata } from '../constants/module'
10 import TiConnectionChecker from '../services/connection-checker'
11 import { kbvTiConnec
…(truncated, 1934 more chars)
isTiContextLocal is still used. Good. It's a related but separate concern.
Now let me verify the other tests weren't broken. Look at full tests file.
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/configuration-service/__tests__/index.test.ts
Tool results:
→ result (ok): 1 import i18n from 'i18next'
2 import { MockInstance } from 'vitest'
3
4 import { TiConnectSettings } from '@module/de/kbv-ti-connect/services/types'
5
6 import ConfigurationService from '../../configuration-service'
7 import { kbvTiConnectLogger } from '../../logger'
8 import { testStore } from '../../mock/mockStore'
9 import { TiConnectRepository } from '../../ti-connect-repository'
10
11 vi.mock('../../logger', () => ({
12 kbvTiConnectLogger: {
13 debug: vi.fn(),
14 error: vi.fn(
…(truncated, 8926 more chars)
Now let me check the CLAUDE.md for project-specific rules.
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/CLAUDE.md
Tool results:
→ result (ok): 1 # CLAUDE.md
2
3 This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
4
5 @AGENTS.md
6
7 ## Claude Code Specific Behavior
8
9 The following rules apply specifically to Claude Code and supplement AGENTS.md.
10
11 ### Memory
12
13 - Auto-memory location: .claude/memory/MEMORY.md
14 - Update memory after user corrections and when new stable patterns are discovered
15 - Do not write speculative or session-specific context to memory
16
17 ##
…(truncated, 883 more chars)
Tools used:
Read:/Users/USER/workspace/doctolib/doctolib-pro-desktop/AGENTS.md
Tool results:
→ result (ok): 1 # AGENTS.md
2
3 This file provides guidance to AI code assistants when working with code in this repository.
4
5 ## DDV Context
6
7 - What is DDV? Desktop client for Doctolib Pro (200k+ practitioners, 280k devices)
8 - Tech Stack: Electron + TypeScript + React + Node.js
9 - Platforms: Windows 10+, macOS 12+ (Intel & Apple Silicon)
10 - Team: COPE team owns core modules, feature teams own business modules
11 - For humans: See README.md for project overview an
…(truncated, 5625 more chars)
Now I have enough context. Let me also verify equivalence of the logic transformation carefully.
Before:
if (settings.useConnectorContextFromElectronSettings) {
contextToUse = {...from settings}
} else if (contextFromRequest) {
contextToUse = normalize(contextFromRequest)
} else {
log; throw
}
After:
if (contextFromRequest) {
contextToUse = normalize(contextFromRequest)
} else {
log; throw
}
The refactor is equivalent IF AND ONLY IF settings.useConnectorContextFromElectronSettings is always false in production. The PR description claims this via FHIR-2040. Note that the mock store previously set it to true, but tests reliably passed — including on the new behavior.
One subtle behavior change worth calling out: when BOTH useConnectorContextFromElectronSettings: true AND contextFromRequest were provided, the old path preferred settings; the new code now uses contextFromRequest. This is intentional (since the flag is dead), but it's a behavior change technically.
Now I have all I need. Let me produce the review as Conventional Comments formatted output.
Review of PR #7487: refactor(FHIR-2177): remove dead useConnectorContextFromElectronSettings flag
Files reviewed:
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/configuration-service/index.ts
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/types.ts
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/ti-connect-settings.ts
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/mock/mockStore.ts
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/services/configuration-service/__tests__/index.test.ts
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/kbv-ti-connect/main/__tests__/ti-connect.test.ts
- /Users/USER/workspace/doctolib/doctolib-pro-desktop/src/modules/de/gematik-kim/services/__tests__/account.test.ts
Additional context inspected (not modified):
- ti-connect-store.ts (migration scaffolding)
- ti-connect-repository.ts (persistence layer)
- main/module.ts (uses isTiContextLocal, which keeps mandantId/clientSystemId/workplaceId in use)
- constants/types.ts (TiConnectSchema re-exports TiConnectSettings as the persisted shape)
Critical (90–100)
None.
Important (80–89)
issue (migration, blocking): confidence 88 — src/modules/de/kbv-ti-connect/services/ti-connect-store.ts:11-23
The persisted electron-store schema is TiConnectSettings (see constants/types.ts:12 – ti_connect: TiConnectSettings). After this PR, an installed user's on-disk store will still contain useConnectorContextFromElectronSettings: true|false, but the TypeScript type no longer declares it. The store is read untyped into that interface on every boot, and there is a migrate() function in ti-connect-store.ts that already exists for exactly this kind of schema evolution (storeVersion0 / latestVersion).
Two concerns:
1. Users who had useConnectorContextFromElectronSettings: true persisted (the mock store had it true, implying it was possible in real installs) will now silently have that field ignored. Per the PR description this is intentional because the IPC that set it was removed in FHIR-2040, but stale data remains in the JSON file forever.
2. No schema version bump was added. If this repo's convention is to version store schemas when removing fields, this should follow that pattern.
suggestion (non-blocking): Either bump latestVersion to 1 and extend migrate() to delete settings.useConnectorContextFromElectronSettings from persisted ti_connect, or add a comment justifying why leaving orphan data is acceptable here (the field is never read again, so it's cosmetic). At minimum, document the decision in the PR description.
issue (dead code, non-blocking): confidence 85 — src/modules/de/kbv-ti-connect/services/types.ts:156-159 and src/modules/de/kbv-ti-connect/services/ti-connect-settings.ts:37-39
After this PR, the only remaining readers of settings.mandantId / settings.clientSystemId / settings.workplaceId / settings.userId in non-test code are:
- ti-connect-settings.ts:38 — isTiContextLocal(), which is still called from main/module.ts:46 inside TiConnectModule.initialize() to gate TiConnectionChecker.connectToTiConnector().
The inline comment at main/module.ts:42-45 already says this check is broken ("this check would work only if connector context is local but that is not the case") and marks it as pending removal. These settings fields are therefore exclusively kept alive by a known-broken certification check. They are not the focus of this PR, so this is out of scope, but worth flagging so a follow-up ticket removes isTiContextLocal and the four persisted fields together.
suggestion (non-blocking): Open a follow-up ticket referencing FHIR-2047 to remove isTiContextLocal, the dead TiConnectionChecker branch, and the four mandantId/clientSystemId/workplaceId/userId persisted fields. Do not expand the scope of this PR.
issue (test coverage regression, blocking): confidence 82 — src/modules/de/kbv-ti-connect/services/configuration-service/__tests__/index.test.ts:83-115
The test file removed the when context usage from ddv settings is enabled describe block, which is correct. However, it also dropped the explicit when context usage from ddv settings is disabled outer describe wrapper and the nested structure is now flattened. That's fine stylistically, but two regressions in intent:
-
The
should throw an error when context is not present in the requesttest previously asserted thati18nSpywas called twice (line 97). In the oldelse if (contextFromRequest) / elsestructure,i18n.twas called once when building theErrorfor the logger and once when throwing the outerError. In the new code atconfiguration-service/index.ts:85-97, the same two calls still happen, so thetoHaveBeenCalledTimes(2)expectation is preserved — good. -
There is no regression test that verifies the previously settings-preferred path is gone. If
useConnectorContextFromElectronSettingswere ever reintroduced onTiConnectSettingsin the future (via a sloppy merge or a new feature), nothing in the suite would fail to catch it. Low risk, but worth a comment.
suggestion (non-blocking): Consider adding a type-level assertion or a test that mocks settings with unexpected extra keys and verifies contextFromRequest always wins, to prevent accidental reintroduction.
issue (behavior change disclosure, non-blocking): confidence 80 — src/modules/de/kbv-ti-connect/services/configuration-service/index.ts:74-82
Subtle semantic shift worth calling out in the PR description: previously, when BOTH settings.useConnectorContextFromElectronSettings === true AND a contextFromRequest was provided, the settings-based context won. After the PR, contextFromRequest wins unconditionally. The PR description argues this is equivalent because the flag was permanently false post-FHIR-2040, which is correct — but the deleted test when context usage from ddv settings is enabled at old line 52-81 was doing exactly that: it passed contextFromRequest alongside useConnectorContextFromElectronSettings: true and asserted that settings won. That test passing before was not proof of production behavior, it was proof of the dead-code branch behavior. Removing it is correct; just make sure the PR description (or commit message) spells out that this is not a behavioral change in production but is a behavioral change if you measured it against the test suite.
suggestion (non-blocking): Add a sentence in the PR description: "Precedence between settings and contextFromRequest is no longer meaningful because the fl
…(truncated, 1376 more chars)