feat(config): user-level (global) config with per-repo consent - #1559
Conversation
Sorry, something went wrong.
Implements the global user config layer from spec docs/tasks/1448-user-level-config.md. - Registry: userConfig.consent section with get/set/list/clear per-repo decisions (TTL-exempt, pruned by missing-path only) - Config: resolveUserConfigPath (env var → XDG → APPDATA → ~/.codegraph fallback), sanitizeUserLayer (absolute dbPath guard), appliesTo glob matching, layered merge DEFAULTS → global → project → env → secrets, per-layer excludeTests shorthand hoisting - Consent model: disabled > enabled > appliesTo glob > undecided (§4.1/4.2) Non-interactive contexts (CI, MCP, programmatic) never prompt - loadConfigWithProvenance: per-key source map for --explain - setUserConfigOverride: CLI preAction hook wires --user-config/--no-user-config - computeConfigHash: stable hash of build-relevant config keys; stored as build_meta.config_hash; triggers full rebuild on change (closes pre-existing project-config incremental gap, see #1557) - promptForConsentIfNeeded: async TTY-gated prompt fired before build - CLI: codegraph config command (--explain, --enable-global, --disable-global, --list-global); global --user-config [path] / --no-user-config flags - Build pipeline: threads userConfig, emits ℹ notice when global layer injects build-affecting keys, stores config_hash in finalize - Lazy config in options.ts: defers loadConfig until first property access so CLI flags are parsed before config is evaluated - Tests: 26 new unit tests for config-user, 18 new tests for registry consent - Docs: configuration.md global config section; README pointer Deferred (#1558): --init / --edit scaffolding helpers
Codegraph Impact Analysis43 functions changed → 128 callers affected across 92 files
|
Sorry, something went wrong.
Greptile SummaryThis PR implements the user-level (global) config layer from spec #1448: a ~/.config/codegraph/config.json file that merges below project config, gated by per-repo consent stored in registry.json. All three TOCTOU/stale-state issues flagged in the previous review round are addressed in the current commit via _lastAppliedGlobalConfig caching and pre-cache-hit state updates.
Confidence Score: 5/5Safe to merge; the consent model is opt-in by design, no existing build behaviour changes for repos that never touch the new flags. All three issues from the prior review round are demonstrably fixed. The consent-aware cache key (cwd::globalPath) means a newly-recorded consent decision is never served stale from cache. The only new findings are a misleading 'undecided' label in --explain output for appliesTo-matched repos, and a minor TypeScript type gap in the Proxy trap — neither affects correctness or security. src/infrastructure/config.ts — the resolveConsent appliesTo branch and the --explain UX gap are worth a follow-up; everything else is straightforward. Important Files Changed
Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/fea..." | Re-trigger Greptile |
Sorry, something went wrong.
| const cached = _configCache.get(cacheKey); | ||
| if (cached) return structuredClone(cached); | ||
|
|
||
| _lastAppliedGlobalPath = applied ? globalPath : null; |
There was a problem hiding this comment.
_lastAppliedGlobalPath is only updated on a cache miss. On a cache hit, getLastAppliedGlobalPath() in setupPipeline returns the value written by whatever previous loadConfig call last missed the cache — which may belong to a different repo or different opts. In programmatic usage (multiple buildGraph calls in the same process, a common pattern since buildGraph is a public API), the build-time "global config applied" notice will show or be suppressed based on stale state from the prior call. The fix is to update _lastAppliedGlobalPath before the early-return on a cache hit.
| const cached = _configCache.get(cacheKey); | |
| if (cached) return structuredClone(cached); | |
| _lastAppliedGlobalPath = applied ? globalPath : null; | |
| const cached = _configCache.get(cacheKey); | |
| if (cached) { | |
| _lastAppliedGlobalPath = applied ? globalPath : null; | |
| return structuredClone(cached); | |
| } | |
| _lastAppliedGlobalPath = applied ? globalPath : null; |
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — _lastAppliedGlobalPath (and the new _lastAppliedGlobalConfig) are now set before the cache-hit early-return, so every call to loadConfig — whether it hits or misses the cache — reflects the current repo/opts. Commit f32fde4.
Sorry, something went wrong.
| let globalRaw: Record<string, unknown> | null = null; | ||
| if (applied && globalPath) { | ||
| const userFileData = loadUserConfigFile(globalPath); | ||
| if (userFileData) { | ||
| globalRaw = sanitizeUserLayer(userFileData.globalConfig); | ||
| for (const k of Object.keys(globalRaw)) provenance[k] = 'user'; | ||
| } | ||
| } | ||
|
|
||
| // Layer 2: project | ||
| let projectRaw: Record<string, unknown> | null = null; | ||
| for (const name of CONFIG_FILES) { | ||
| const filePath = path.join(cwd, name); | ||
| if (fs.existsSync(filePath)) { | ||
| try { | ||
| const raw = JSON.parse(fs.readFileSync(filePath, 'utf-8')) as Record<string, unknown>; | ||
| projectRaw = raw; | ||
| for (const k of Object.keys(raw)) provenance[k] = 'project'; | ||
| break; | ||
| } catch { | ||
| // ignore | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Layer 3+: env overrides (LLM keys) | ||
| const ENV_LLM_KEYS = ['CODEGRAPH_LLM_PROVIDER', 'CODEGRAPH_LLM_API_KEY', 'CODEGRAPH_LLM_MODEL']; | ||
| if (ENV_LLM_KEYS.some((k) => process.env[k] !== undefined)) { | ||
| provenance.llm = 'env'; | ||
| } | ||
|
|
||
| void globalRaw; | ||
| void projectRaw; // used for provenance tracking above | ||
|
|
||
| const config = loadConfig(cwd, opts); | ||
| return { config, provenance, appliedGlobalPath: applied ? globalPath : null, consentDecision }; | ||
| } |
There was a problem hiding this comment.
Double global-file read + TOCTOU window in loadConfigWithProvenance
loadConfigWithProvenance reads the global config file once here to build the provenance map, then delegates to loadConfig which reads the same file a second time to build the merged config. If the file changes on disk between the two reads (e.g., during a slow build on a shared machine), the provenance map and the returned config will disagree about which keys came from the user layer — the --explain output would be misleading. Extracting a loadConfigFromLayers(...) helper that returns both the merged result and the per-layer raw data would eliminate the duplicate I/O and the window entirely.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — loadConfigWithProvenance now calls loadConfig first (which populates _lastAppliedGlobalConfig), then reads from that cache for the global-layer provenance walk instead of re-opening the file. The new getLastAppliedGlobalConfig() getter is also used in pipeline.ts for the build notice. Commit f32fde4.
Sorry, something went wrong.
| // ── Build-time global-config notice ────────────────────────────── | ||
| const appliedGlobalPath = getLastAppliedGlobalPath(); | ||
| if (appliedGlobalPath) { | ||
| const buildAffectingKeys = [ | ||
| 'include', | ||
| 'exclude', | ||
| 'ignoreDirs', | ||
| 'extensions', | ||
| 'aliases', | ||
| 'build', | ||
| ]; | ||
| try { | ||
| const raw = JSON.parse(fs.readFileSync(appliedGlobalPath, 'utf-8')) as Record< | ||
| string, | ||
| unknown | ||
| >; | ||
| const globalData: Record<string, unknown> = | ||
| 'appliesTo' in raw && raw.config ? (raw.config as Record<string, unknown>) : raw; | ||
| const injectedKeys = buildAffectingKeys.filter((k) => k in globalData); | ||
| if (injectedKeys.length > 0) { | ||
| process.stderr.write( | ||
| `ℹ global config applied (${appliedGlobalPath}) — injecting: ${injectedKeys.join(', ')} · --no-user-config to ignore\n`, | ||
| ); | ||
| } | ||
| } catch { | ||
| // Non-critical — skip on any read error | ||
| } | ||
| } |
There was a problem hiding this comment.
Global config file read a second time for the build notice
loadConfig (line 191) already parses and caches the global file; getLastAppliedGlobalPath() returns the path. But the notice code at line 217 opens and parses the same file a second time via fs.readFileSync. Besides the redundant I/O, there is a small TOCTOU window: if the file is replaced between the loadConfig call and this read, the injected-keys list shown in the notice could differ from the keys that are actually in the merged config. The already-loaded ParsedUserConfig data (from inside loadConfig) is not surfaced; consider exposing it alongside the path via getLastAppliedGlobalPath or returning it from loadConfig as optional metadata.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed — the build notice in setupPipeline now reads from getLastAppliedGlobalConfig() (the sanitized data cached by loadConfig) instead of re-reading the file via fs.readFileSync. This eliminates the redundant I/O and the TOCTOU window. Commit f32fde4.
Sorry, something went wrong.
When opts.exclude is introduced on a second build, the config hash changes and promotes to forceFullRebuild. handleFullBuild deleted nodes/edges but left file_hashes intact, so previously-indexed files that are now excluded remained visible in file_hashes after the rebuild. Adds DELETE FROM file_hashes to the full rebuild statement so stale entries from excluded files are purged. insertNodes then re-inserts fresh hashes only for the files that were actually collected.
- XDG_CONFIG_HOME: honour on all platforms (including Windows) — the previous code checked XDG_CONFIG_HOME only on non-Windows, so the Windows CI test 'uses XDG_CONFIG_HOME when set' returned null. - _lastAppliedGlobalPath stale on cache hit (P1): move the assignment before the early-return so programmatic callers making multiple buildGraph calls in the same process get the correct value for the build-time notice (Greptile P1, comment #3418566672). - Eliminate TOCTOU double file read: add _lastAppliedGlobalConfig alongside _lastAppliedGlobalPath; loadConfig populates it once; pipeline.ts build notice and loadConfigWithProvenance both read from the cache rather than re-opening the file (Greptile P2 comments #3418566729, #3418566865). - Remove void no-op suppressions in loadConfigWithProvenance — both variables are read in their respective loops and need no suppression (Greptile P2, outside-diff comment). - Redundant --json branch in codegraph config default case: both branches emitted identical JSON; unified into one write; the discovery hint on stderr is now suppressed by --json (Greptile P2, outside-diff comment).
|
Addressed all Greptile review feedback:
Also fixed two CI test failures:
|
Sorry, something went wrong.
Sorry, something went wrong.
|
Addressed the correctness gap Greptile flagged in the summary: _lastAppliedGlobalConfig was reset to null before the cache check but never restored on a cache hit, so any programmatic caller that invoked loadConfig before loadConfigWithProvenance got a provenance map with no user attribution for global-layer keys. Fix (commit fa7dc18): added a parallel _globalConfigCache map keyed by the same cache key. On cache save, both caches are written together. On cache hit, _lastAppliedGlobalConfig is restored from _globalConfigCache before returning — so loadConfigWithProvenance always sees the correct global layer regardless of call order. Both clearConfigCache and setUserConfigOverride clear the parallel cache too. A new unit test (returns correct user provenance even when loadConfig was called first) guards this path. |
Sorry, something went wrong.
Sorry, something went wrong.
Summary
Implements the global user-level config layer from spec docs/tasks/1448-user-level-config.md.
No Rust changes
Native engine is a pure consumer — it receives loadConfig(rootDir) via JSON in the TS pipeline. The merge happens once in TS and reaches native automatically. Per CLAUDE.md and spec §9: no Rust changes, no two-source divergence risk.
Test plan
Deferred (per spec §16)
Closes #1448