Skip to content

fix: preserve global config during transient init misses #37

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

Open
kingrubic wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: preserve global config during transient init misses #37

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
26 changes: 26 additions & 0 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,21 @@ function buildConfig(fileConfig: OpenCodeMemConfig) {
}

const _globalFileConfig = loadConfigFromPaths(CONFIG_FILES);
let lastKnownGlobalConfig = _globalFileConfig;

function isCurrentConfigFromLastKnownGlobal(): boolean {

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`isCurrentConfigFromLastKnownGlobal` has a cyclomatic complexity of 8 with "medium" risk


A function with high cyclomatic complexity can be hard to understand and
maintain. Cyclomatic complexity is a software metric that measures the number of
independent paths through a function. A higher cyclomatic complexity indicates
that the function has more decision points and is more complex.

if (Object.keys(lastKnownGlobalConfig).length === 0) return false;
const globalConfig = buildConfig(lastKnownGlobalConfig);
return (
CONFIG.embeddingModel === globalConfig.embeddingModel &&

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

CONFIG.embeddingDimensions === globalConfig.embeddingDimensions &&

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

CONFIG.embeddingApiUrl === globalConfig.embeddingApiUrl &&

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

CONFIG.embeddingApiKey === globalConfig.embeddingApiKey &&

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

CONFIG.opencodeProvider === globalConfig.opencodeProvider &&

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

CONFIG.opencodeModel === globalConfig.opencodeModel &&

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

CONFIG.autoCaptureEnabled === globalConfig.autoCaptureEnabled

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'CONFIG' was used before it was defined


Variables, functions and types should always be used after they've been defined. This issue will flag any code snippets that use variables or types before definition.

);
}
Comment on lines +588 to +600

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected function declaration in the global scope, wrap in an IIFE for a local variable, assign as global property for a global variable


It is considered a best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script. Global variables created from a script can produce name collisions with global variables created from another script, which will usually lead to runtime errors or unexpected behavior. It is mostly useful for browser scripts.

Comment on lines +588 to +600

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid the side-effecting buildConfig call inside a boolean predicate, and reuse the result.

isCurrentConfigFromLastKnownGlobal calls buildConfig(lastKnownGlobalConfig) purely to compare fields, but buildConfig calls setLogLevel(fileConfig.logLevel) and throws on invalid config (lines 568-583). Two problems:

  1. A predicate that mutates global log level / can throw is surprising and makes initConfig reset behavior fragile if lastKnownGlobalConfig is ever malformed.
  2. The same buildConfig(lastKnownGlobalConfig) is computed again at line 640, so it runs twice on the restore path.

Consider computing the rebuilt config once and passing it in:

♻️ Proposed refactor 
-function isCurrentConfigFromLastKnownGlobal(): boolean {
-  if (Object.keys(lastKnownGlobalConfig).length === 0) return false;
-  const globalConfig = buildConfig(lastKnownGlobalConfig);
-  return (
+function isCurrentConfigFromLastKnownGlobal(globalConfig: ReturnType<typeof buildConfig>): boolean {
+  return (
     CONFIG.embeddingModel === globalConfig.embeddingModel &&
     CONFIG.embeddingDimensions === globalConfig.embeddingDimensions &&
     CONFIG.embeddingApiUrl === globalConfig.embeddingApiUrl &&
     CONFIG.embeddingApiKey === globalConfig.embeddingApiKey &&
     CONFIG.opencodeProvider === globalConfig.opencodeProvider &&
     CONFIG.opencodeModel === globalConfig.opencodeModel &&
     CONFIG.autoCaptureEnabled === globalConfig.autoCaptureEnabled
   );
 }

Then in the guard, build once and branch on emptiness:

-    if (isCurrentConfigFromLastKnownGlobal()) {
-      Object.assign(CONFIG, buildConfig(lastKnownGlobalConfig));
-    } else {
-      Object.assign(CONFIG, buildConfig({}));
-    }
+    const lastKnown =
+      Object.keys(lastKnownGlobalConfig).length > 0 ? buildConfig(lastKnownGlobalConfig) : null;
+    if (lastKnown && isCurrentConfigFromLastKnownGlobal(lastKnown)) {
+      Object.assign(CONFIG, lastKnown);
+    } else {
+      Object.assign(CONFIG, buildConfig({}));
+    }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config.ts` around lines 588 - 600, The isCurrentConfigFromLastKnownGlobal
function calls buildConfig inside a boolean predicate, which causes side effects
(setLogLevel calls) and can throw exceptions unexpectedly. Additionally, the
same buildConfig(lastKnownGlobalConfig) computation happens again at line 640,
causing duplicate work. Refactor by computing the rebuilt config once at the
call site (in initConfig) and passing the result to
isCurrentConfigFromLastKnownGlobal as a parameter, or restructure the function
to check for emptiness of lastKnownGlobalConfig first before building and
comparing, avoiding the redundant buildConfig calls and isolating side effects
to a single location.


/**
* Global CONFIG singleton — resolves from opencode-mem0.json and env.
Expand Down Expand Up @@ -617,6 +632,17 @@ export function initConfig(directory: string): void {
];
const globalConfig = loadConfigFromPaths(CONFIG_FILES);
const projectConfig = loadConfigFromPaths(projectPaths);
if (Object.keys(globalConfig).length > 0) {
lastKnownGlobalConfig = globalConfig;
}
if (Object.keys(globalConfig).length === 0 && Object.keys(projectConfig).length === 0) {
if (isCurrentConfigFromLastKnownGlobal()) {
Object.assign(CONFIG, buildConfig(lastKnownGlobalConfig));
} else {
Object.assign(CONFIG, buildConfig({}));
}
return;
}
const merged = deepMerge(globalConfig, projectConfig);
Object.assign(CONFIG, buildConfig(merged));
}
Expand Down
20 changes: 20 additions & 0 deletions tests/config-resolution.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,24 @@ describe("project-scoped config resolution", () => {
expect(CONFIG.autoCaptureEnabled).toBe(true); // default value
expect(CONFIG.opencodeProvider).toBeUndefined();
});

it("uses the last known global config when config paths are temporarily unavailable", () => {
(globalThis as any).__mockFs.existsSync = (p: unknown) => {

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected any. Specify a different type


The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.

const path = String(p);
return path.includes(".config/opencode/opencode-mem0");
};
(globalThis as any).__mockFs.readFileSync = () =>

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected any. Specify a different type


The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.

JSON.stringify({
embeddingModel: "text-embedding-3-small",
embeddingDimensions: 1536,
});
initConfig("/some/project");
expect(CONFIG.embeddingModel).toBe("text-embedding-3-small");
expect(CONFIG.embeddingDimensions).toBe(1536);

(globalThis as any).__mockFs.existsSync = () => false;

@deepsource-io deepsource-io Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected any. Specify a different type


The any type can sometimes leak into your codebase. TypeScript compiler skips the type checking of the any typed variables, so it creates a potential safety hole, and source of bugs in your codebase. We recommend using unknown or never type variable.

initConfig("/some/project");
expect(CONFIG.embeddingModel).toBe("text-embedding-3-small");
expect(CONFIG.embeddingDimensions).toBe(1536);
});
});