Skip to content

Update src/cli/system.test.ts - #70

Merged
alvinunreal merged 5 commits into
masterfrom
clean-install
Jan 22, 2026
Merged

Update src/cli/system.test.ts#70
alvinunreal merged 5 commits into
masterfrom
clean-install

Conversation

@alvinunreal

Copy link
Copy Markdown
Owner

Summary

Changes

alvinunreal and others added 5 commits January 22, 2026 11:41
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Refactored src/cli/config-manager.ts by splitting it into four focused modules: paths.ts (path utilities), system.ts (system checks), providers.ts (provider configs), and config-io.ts (I/O operations). Also renamed src/features/ directory to src/background/ for better clarity.

Key Changes

  • Split large config-manager.ts into modular files for better maintainability
  • Added comprehensive test coverage for all new modules (system.test.ts, paths.test.ts, config-io.test.ts, providers.test.ts)
  • Improved writeConfig with atomic write pattern (write to .tmp, then rename) and automatic backup (.bak files)
  • Enhanced error handling in parseConfigFile to return error details instead of silently returning null
  • Added OpenCodeConfig interface to types.ts for better type safety
  • Renamed features/ to background/ directory for clearer naming

Issues Found

  • Critical: Windows support regression in paths.ts - removed Windows-specific logic that checked for configs in %APPDATA% vs ~/.config, breaking compatibility with existing Windows installations

Confidence Score: 2/5

  • Not safe to merge - breaks Windows support for existing users
  • The refactoring is well-structured with good test coverage, but the Windows path handling regression is a critical breaking change that will cause existing Windows users to lose access to their configs
  • src/cli/paths.ts needs immediate attention to restore Windows compatibility

Important Files Changed

Filename Overview
src/cli/paths.ts New file with path utilities extracted from config-manager, but removed Windows-specific logic that breaks Windows support
src/cli/system.ts New file with system utilities (opencode/tmux checks, version fetching) - clean extraction with no issues
src/cli/config-io.ts New file with config I/O operations - added atomic writes and better error handling
src/cli/providers.ts New file with provider configs and model mappings - clean extraction with no changes to logic
src/cli/config-manager.ts Refactored to barrel file that exports from paths, providers, system, and config-io modules

Sequence Diagram

sequenceDiagram
    participant User
    participant Install
    participant ConfigIO
    participant Paths
    participant System
    participant Providers

    User->>Install: Run install command
    Install->>System: isOpenCodeInstalled()
    System-->>Install: boolean
    
    Install->>ConfigIO: detectCurrentConfig()
    ConfigIO->>Paths: getExistingConfigPath()
    Paths-->>ConfigIO: config path
    ConfigIO->>ConfigIO: parseConfig(path)
    ConfigIO-->>Install: DetectedConfig
    
    Install->>ConfigIO: addPluginToOpenCodeConfig()
    ConfigIO->>Paths: ensureConfigDir()
    Paths->>Paths: getConfigDir()
    ConfigIO->>Paths: getExistingConfigPath()
    ConfigIO->>ConfigIO: parseConfig(path)
    ConfigIO->>ConfigIO: writeConfig(path, config)
    ConfigIO-->>Install: ConfigMergeResult
    
    Install->>ConfigIO: addAuthPlugins(installConfig)
    ConfigIO->>System: fetchLatestVersion(package)
    System-->>ConfigIO: version string
    ConfigIO->>ConfigIO: writeConfig(path, config)
    ConfigIO-->>Install: ConfigMergeResult
    
    Install->>ConfigIO: addProviderConfig(installConfig)
    ConfigIO->>Providers: GOOGLE_PROVIDER_CONFIG
    ConfigIO->>ConfigIO: writeConfig(path, config)
    ConfigIO-->>Install: ConfigMergeResult
    
    Install->>ConfigIO: writeLiteConfig(installConfig)
    ConfigIO->>Providers: generateLiteConfig(installConfig)
    Providers-->>ConfigIO: config object
    ConfigIO->>Paths: getLiteConfig()
    ConfigIO->>ConfigIO: atomic write with backup
    ConfigIO-->>Install: ConfigMergeResult
    
    Install-->>User: Installation complete
Loading

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

21 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/cli/paths.ts
Comment on lines +5 to +13
export function getConfigDir(): string {
// Keep this aligned with OpenCode itself and the plugin config loader:
// base dir is $XDG_CONFIG_HOME (if set) else ~/.config, and OpenCode config lives under /opencode.
const userConfigDir = process.env.XDG_CONFIG_HOME
? process.env.XDG_CONFIG_HOME
: join(homedir(), ".config")

return join(userConfigDir, "opencode")
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

logic: removed Windows-specific path handling that existed in the previous implementation - this breaks Windows support where configs could be in %APPDATA% or .config

The old implementation checked for existing configs in ~/.config/opencode on Windows and fell back to %APPDATA% if not found. Now it always uses ~/.config/opencode on Windows, which may not align with where users' existing configs are located.

Suggested change
export function getConfigDir(): string {
// Keep this aligned with OpenCode itself and the plugin config loader:
// base dir is $XDG_CONFIG_HOME (if set) else ~/.config, and OpenCode config lives under /opencode.
const userConfigDir = process.env.XDG_CONFIG_HOME
? process.env.XDG_CONFIG_HOME
: join(homedir(), ".config")
return join(userConfigDir, "opencode")
}
export function getConfigDir(): string {
if (process.platform === "win32") {
const homedirPath = homedir()
const crossPlatformDir = join(homedirPath, ".config")
const appdataDir = process.env.APPDATA ?? join(homedirPath, "AppData", "Roaming")
const crossPlatformConfig = join(crossPlatformDir, "opencode", "opencode.json")
const crossPlatformConfigJsonc = join(crossPlatformDir, "opencode", "opencode.jsonc")
if (existsSync(crossPlatformConfig) || existsSync(crossPlatformConfigJsonc)) {
return join(crossPlatformDir, "opencode")
}
return join(appdataDir, "opencode")
}
// Keep this aligned with OpenCode itself and the plugin config loader:
// base dir is $XDG_CONFIG_HOME (if set) else ~/.config, and OpenCode config lives under /opencode.
const userConfigDir = process.env.XDG_CONFIG_HOME
? process.env.XDG_CONFIG_HOME
: join(homedir(), ".config")
return join(userConfigDir, "opencode")
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/cli/paths.ts
Line: 5:13

Comment:
**logic:** removed Windows-specific path handling that existed in the previous implementation - this breaks Windows support where configs could be in `%APPDATA%` or `.config`

The old implementation checked for existing configs in `~/.config/opencode` on Windows and fell back to `%APPDATA%` if not found. Now it always uses `~/.config/opencode` on Windows, which may not align with where users' existing configs are located.

```suggestion
export function getConfigDir(): string {
  if (process.platform === "win32") {
    const homedirPath = homedir()
    const crossPlatformDir = join(homedirPath, ".config")
    const appdataDir = process.env.APPDATA ?? join(homedirPath, "AppData", "Roaming")

    const crossPlatformConfig = join(crossPlatformDir, "opencode", "opencode.json")
    const crossPlatformConfigJsonc = join(crossPlatformDir, "opencode", "opencode.jsonc")

    if (existsSync(crossPlatformConfig) || existsSync(crossPlatformConfigJsonc)) {
      return join(crossPlatformDir, "opencode")
    }

    return join(appdataDir, "opencode")
  }

  // Keep this aligned with OpenCode itself and the plugin config loader:
  // base dir is $XDG_CONFIG_HOME (if set) else ~/.config, and OpenCode config lives under /opencode.
  const userConfigDir = process.env.XDG_CONFIG_HOME
    ? process.env.XDG_CONFIG_HOME
    : join(homedir(), ".config")

  return join(userConfigDir, "opencode")
}
```

How can I resolve this? If you propose a fix, please make it concise.

@alvinunreal
alvinunreal merged commit a79f5eb into master Jan 22, 2026
3 checks passed
@mhenke
mhenke deleted the clean-install branch July 10, 2026 16:14
mhenke pushed a commit to mhenke/oh-my-opencode-slim that referenced this pull request Jul 17, 2026
* Clean cli

* Rename features to background

* Fix test

* Update src/cli/providers.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* Update src/cli/system.test.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

---------

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant