fix: address quality issues in builder pipeline, config, and db connection handling#1788
Conversation
…docs check acknowledged) Impact: 1 functions changed, 2 affected
…n algorithm files docs check acknowledged: internal helper extraction only, no user-facing feature/language/architecture-table changes. Impact: 10 functions changed, 27 affected
Impact: 11 functions changed, 11 affected
…n readFileSafe readFileSafe's Atomics.wait busy-block froze the entire Node.js event loop (all I/O and timer callbacks) for up to 100ms per retry on the watch-mode hot path. Extracts journal.ts's existing sleepSync busy-spin helper into src/shared/sleep.ts so both readFileSafe and journal.ts's lock-retry loop share one implementation instead of duplicating it. docs check acknowledged: internal bug fix, no feature/language/architecture table changes warranted in README.md, CLAUDE.md, or ROADMAP.md. Impact: 2 functions changed, 31 affected
…complexity.ts Impact: 14 functions changed, 10 affected
Registers resolveSecrets' execFileSync timeout/maxBuffer in DEFAULTS.llm (apiKeyCommandTimeoutMs, apiKeyCommandMaxBufferBytes) and wires resolveSecrets to read them from config instead of hardcoding. Also adds three purely-additive @reserved DEFAULTS entries for constants hardcoded elsewhere in the codebase (build. largeCodebaseFileThreshold, db.busyTimeoutMs, community. capacityGrowthFactor) so their consumer files can be wired to them in follow-up commits. docs check acknowledged — no new feature/language/architecture change; docs/guides/configuration.md (the actual config reference) is already updated in this commit. Impact: 5 functions changed, 87 affected
…presentation/plot.ts Impact: 4 functions changed, 3 affected
…l/runContextCollectorWalk Pure extract-method decomposition of the three highest-complexity functions in extractors/javascript.ts (Titan phase 10, sync.json commit message shortened to fit the 100-char commitlint header limit). No extraction logic, node-type handling, or edge-case behavior changed -- verified byte-identical resolution-benchmark precision/recall across all 34 fixture languages and byte-identical codegraph query/where output for 3 real non-fixture files before/after. No Rust files touched, so native/WASM parity is unaffected by construction. docs check acknowledged: internal-only refactor, no new languages/commands/ architecture; README.md, CLAUDE.md, and ROADMAP.md are unaffected. Impact: 20 functions changed, 15 affected
…ative in build-edges.ts docs check acknowledged: pure internal extract-method refactor, no new features, commands, languages, or architecture changes — README/CLAUDE/ROADMAP do not need updates. Impact: 20 functions changed, 15 affected
…or in native-orchestrator.ts Pure extract-class decomposition of the DECOMPOSE-flagged worst offender in Titan phase 12 (halstead.bugs 1.17). tryNativeOrchestrator's native-DB lifecycle steps (open/build/backfill/handoff/close) are now owned by a NativeOrchestrationSession class; tryNativeOrchestrator becomes a thin sequencer of session method calls. No dispatch logic, fallback conditions, or error handling changed -- verified via full test suite (200/200 files, 3330/3330 tests), byte-identical resolution-benchmark output across all 34 fixture languages, and byte-identical native-engine DB dumps (full build + incremental early-exit) on tests/fixtures/sample-project before/after. tryNativeOrchestrator: cognitive 35->24, cyclomatic 34->25, halstead.bugs 1.17->0.83, mi 49.7->54.1. docs check acknowledged: pure internal extract-class refactor, no new features, commands, languages, or architecture changes -- README/CLAUDE/ ROADMAP do not need updates. Impact: 9 functions changed, 6 affected Impact: 9 functions changed, 6 affected
…tor in remote.ts Extract-method refactor only, no behavior change. embedRemote's per-batch body is split into executeRemoteEmbeddingRequest (build request body, fetch-with-timeout, map network/timeout/status failures to EngineError) and mapRemoteEmbeddingResponse (shape-check, index-sort, embedding-field check, cross-batch dimension-consistency check, Float32Array conversion), with the outer loop calling both in the same order as before. Drops embedRemote from cognitive=36/halstead.bugs=1.10 (DECOMPOSE-flagged worst offender in GAUNTLET) to cognitive=6/bugs=0.38; both new helpers are well within thresholds (cognitive 11 and 9, bugs 0.38 and 0.33). Deliberately does not fix the gauntlet's secondary finding that response.json() sits outside error handling (a malformed body throws a raw SyntaxError instead of EngineError) -- that's a behavior change, out of scope for this pure decomposition. Filed as #1745. docs check acknowledged: internal refactor only, no CLI/feature/language/ architecture surface changed -- README/CLAUDE.md/ROADMAP untouched by design. Impact: 3 functions changed, 10 affected
…dedupe consent glob-matching applyExcludeTestsShorthand mutated merged.query in place, which was still a live reference to the shared DEFAULTS.query singleton whenever no config layer had already overridden `query`. In long-running processes (e.g. `codegraph mcp --multi-repo`) this permanently leaked one repo's excludeTests setting into every subsequent loadConfig() call for any other repo. loadConfig now deep-clones DEFAULTS before merging so no layer can ever write onto a live DEFAULTS reference, and DEFAULTS itself is now deep-frozen so any future regression of this kind throws immediately instead of silently corrupting shared state. applyExcludeTestsShorthand was also hardened to copy-on-write its `query` key directly. Also dedupes the appliesTo-glob-matching logic (previously copy-pasted between resolveConsent and promptForConsentIfNeeded) into a shared matchesAppliesTo helper. No user-facing behavior, CLI surface, or language support changed — docs check acknowledged. Fixes #1725 Impact: 6 functions changed, 140 affected
…pe engine resolution openReadonlyWithNative opened the better-sqlite3 handle before resolving the engine, and engine resolution calls loadConfig(), which can throw (e.g. ConfigError from resolveSecrets on a malformed llm.apiKeyCommand). If that throw happened, the already-open DB handle was never closed -- a real leak on the hot path used by dataflow/hotspots/stats commands. Fix: resolve the engine (and thus loadConfig) before opening the DB, mirroring openRepo's existing, correct ordering. Extracted the shared engine-resolution logic (customDbPath > rootDir > loadConfig priority chain) into resolveDbEngine(), used by both openRepo and openReadonlyWithNative so the two call sites can't drift again. Added tests/unit/openReadonlyWithNative-leak.test.ts: tracks every better-sqlite3 Database instantiation and asserts zero occur when loadConfig throws. Verified this test fails against the pre-fix ordering (it recorded a leaked instance) and passes against the fix. docs check acknowledged: internal bug fix + dedup, no CLI surface, language support, or documented architecture/design decision changed. Impact: 3 functions changed, 38 affected
…ne and cli/commands/info.ts
Converts 13 comment-only/silent catch blocks in pipeline.ts, native-orchestrator.ts,
detect-changes.ts, helpers.ts, and info.ts from `catch { /* comment */ }` to
`catch (e) { debug(...) }`, using the existing infrastructure/logger.ts debug()
utility. Purely additive observability -- no control-flow changes, no change to
what errors are swallowed vs rethrown.
docs check acknowledged: internal logging-only change, no new feature/language/
architecture/command surface to document in README/CLAUDE.md/ROADMAP.md.
Impact: 13 functions changed, 17 affected
…(docs check acknowledged) Impact: 4 functions changed, 4 affected
…nfig (docs check acknowledged) Impact: 6 functions changed, 12 affected
…docs check acknowledged) Impact: 6 functions changed, 22 affected
Greptile SummaryThis PR fixes two genuine production bugs (DEFAULTS singleton mutation in
Confidence Score: 5/5Safe to merge — both targeted production bugs are correctly fixed, the refactoring preserves existing semantics, and each fix is backed by a focused regression test. The config mutation fix (deepFreeze + structuredClone + copy-on-write) and the DB resource-leak reordering are straightforward and correct. The function decompositions preserve identical behavior — tracing the native vs WASM/JS call paths through the new helpers matches the original branch-by-branch. The new tests directly reproduce the described failure modes. No files require special attention — all changes are targeted and backed by tests. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[loadConfig called] --> B[structuredClone DEFAULTS - fresh mutable copy per call]
B --> C[mergeConfig layers: global, repo, env]
C --> D{excludeTests in rawLayer?}
D -- yes --> E[copy-on-write: result.query = spread merged.query + excludeTests]
D -- no --> F[pass through]
E --> G[applyEnvOverrides / resolveSecrets]
F --> G
G --> H[return config - DEFAULTS unchanged]
subgraph orn [openReadonlyWithNative - fixed ordering]
I[resolveDbEngine called] --> J{engineOpt provided?}
J -- yes --> K[return engineOpt - no loadConfig call]
J -- no --> L[loadConfig called - may throw ConfigError]
L -- throws --> M[function exits - no DB handle opened]
L -- ok --> N[engine resolved]
K --> N
N --> O[openReadonlyOrFail - DB handle opened]
O --> P[return db + nativeDb + close]
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[loadConfig called] --> B[structuredClone DEFAULTS - fresh mutable copy per call]
B --> C[mergeConfig layers: global, repo, env]
C --> D{excludeTests in rawLayer?}
D -- yes --> E[copy-on-write: result.query = spread merged.query + excludeTests]
D -- no --> F[pass through]
E --> G[applyEnvOverrides / resolveSecrets]
F --> G
G --> H[return config - DEFAULTS unchanged]
subgraph orn [openReadonlyWithNative - fixed ordering]
I[resolveDbEngine called] --> J{engineOpt provided?}
J -- yes --> K[return engineOpt - no loadConfig call]
J -- no --> L[loadConfig called - may throw ConfigError]
L -- throws --> M[function exits - no DB handle opened]
L -- ok --> N[engine resolved]
K --> N
N --> O[openReadonlyOrFail - DB handle opened]
O --> P[return db + nativeDb + close]
end
Reviews (2): Last reviewed commit: "fix: resolve merge conflicts with main (..." | Re-trigger Greptile |
Codegraph Impact Analysis37 functions changed → 173 callers affected across 102 files
|
Impact: 6 functions changed, 13 affected
Summary
src/infrastructure/config.ts:applyExcludeTestsShorthandmutatedmerged.queryin place, corrupting the sharedDEFAULTS.queryobject for the remainder of a long-running process (e.g.codegraph mcp --multi-repo) whenever a repo's.codegraphrc.jsonused the top-levelexcludeTestsshorthand.src/db/connection.ts'sopenReadonlyWithNative: it opened the SQLite handle beforeloadConfig, so a config error would leak the already-open handle. Reordered to match the correct pattern already used byopenRepo.debug()logging to previously-silent/comment-only catch blocks across the builder pipeline andcli/commands/info.ts.buildChaContextinto three focused builder functions.purgeAndAddReverseDepsand wireCODEGRAPH_FAST_SKIP_DIAGthrough config.getOrCreateBatchStmtand dedupe batch-insert helpers inbuilder/helpers.ts.Titan Audit Context
Changes
src/infrastructure/config.ts,tests/unit/config.test.tssrc/db/connection.ts,tests/unit/openReadonlyWithNative-leak.test.tssrc/domain/graph/builder/{helpers,pipeline}.ts,src/domain/graph/builder/stages/{detect-changes,native-orchestrator}.ts,src/cli/commands/info.tssrc/domain/graph/builder/cha.tsMetrics Impact
Two of these are genuine production bugs (config-corruption-across-repos and a resource leak on config-error paths) found and fixed during the Titan GAUNTLET audit — both independently reproduced with standalone repros before the fix.
Test plan
Closes #1725