Skip to content

Fall back to installed lower-scope SDK versions when higher-priority config is missing#661

Merged
aooohan merged 2 commits intomainfrom
copilot/fix-go-version-compatibility
Apr 26, 2026
Merged

Fall back to installed lower-scope SDK versions when higher-priority config is missing#661
aooohan merged 2 commits intomainfrom
copilot/fix-go-version-compatibility

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 23, 2026

A project-local .vfox.toml entry (e.g. golang = "1.24.0") currently overrides global config even when that version is not installed, which can remove go from PATH inside the workspace. This change keeps scope priority while skipping uninstalled candidates and selecting the highest-priority installed configured version.

  • Resolution behavior: prefer installed config by scope order

    • Added shared tool-resolution logic to walk configured versions in priority order (Project -> Session -> Global) and pick the first installed runtime.
    • If a higher-priority scope points to a missing version, resolution now falls through to lower-priority installed versions instead of failing effective tool availability.
  • Activation/env pipeline integration

    • Updated activate and env command paths to use installed-aware resolution before symlink/env generation.
    • Existing scope semantics are preserved; only candidate selection changes when configured versions are missing.
  • Chain API support for ordered lookup

    • Extended VfoxTomlChain with GetToolConfigsByPriority(name) to return all scope configs for a tool from high to low priority.
    • This avoids ad-hoc lookup duplication and centralizes multi-scope tool selection inputs.
  • Targeted coverage

    • Added chain-level test coverage for ordered per-scope config retrieval used by fallback selection.
// high -> low priority, choose first installed
toolConfigs := chain.GetToolConfigsByPriority("golang")
for _, tc := range toolConfigs {
    if sdkObj.CheckRuntimeExist(sdk.Version(tc.Config.Version)) {
        // use tc.Config.Version at tc.Scope
        break
    }
}

@bytemain bytemain marked this pull request as ready for review April 23, 2026 08:01
Copilot AI review requested due to automatic review settings April 23, 2026 08:01
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

❌ Patch coverage is 27.77778% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.16%. Comparing base (4b5c932) to head (08544c0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/commands/tool_resolution.go 0.00% 14 Missing ⚠️
cmd/commands/activate.go 0.00% 4 Missing ⚠️
cmd/commands/env.go 0.00% 4 Missing ⚠️
internal/env/vfox_toml_chain.go 71.42% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #661      +/-   ##
==========================================
+ Coverage   24.09%   24.16%   +0.07%     
==========================================
  Files          82       83       +1     
  Lines        7084     7104      +20     
==========================================
+ Hits         1707     1717      +10     
- Misses       5201     5209       +8     
- Partials      176      178       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates SDK version resolution so that activate/env prefer the highest-priority installed configured version across scopes, instead of letting a higher-priority but missing version effectively remove the SDK from PATH.

Changes:

  • Added VfoxTomlChain.GetToolConfigsByPriority(name) to retrieve per-scope tool configs in priority order (Project → Session → Global).
  • Introduced shared installed-aware resolution helper (resolveInstalledToolConfig) and integrated it into activate and env symlink/env generation paths.
  • Added unit test coverage for ordered per-scope config retrieval in VfoxTomlChain.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/env/vfox_toml_chain.go Adds ScopedToolConfig and GetToolConfigsByPriority for ordered multi-scope lookup.
internal/env/vfox_toml_chain_test.go Adds test verifying priority ordering of tool configs across scopes.
cmd/commands/tool_resolution.go New helper to pick the first installed configured version by scope priority.
cmd/commands/env.go Switches envFlag path to installed-aware resolution (no longer trusts merged version when missing).
cmd/commands/activate.go Switches activation symlink/env generation to installed-aware resolution.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cmd/commands/env.go
Comment thread cmd/commands/tool_resolution.go
@bytemain
Copy link
Copy Markdown
Member

bytemain commented Apr 26, 2026

@aooohan 这个问题是:
if you global install golang 1.26, and you open a workspace than contain a .vfox.toml file [tools] golang = "1.24.0"

then you cannot use go in this folder, unless you install go specific version

如果您全局安装了Go 1.26,然后打开一个包含 .vfox.toml 文件[tools] golang = "1.24.0" 的工作区,那么在这个文件夹中无法使用 go,除非执行 vfox use

@aooohan
Copy link
Copy Markdown
Member

aooohan commented Apr 26, 2026

如果您全局安装了Go 1.26,然后打开一个包含 .vfox.toml 文件[tools] golang = "1.24.0" 的工作区,那么在这个文件夹中无法使用 go,除非执行 vfox use

这个对于golang倒是没问题,但对于java这种兼容性极差的版本gap,会有问题呀。

@bytemain
Copy link
Copy Markdown
Member

但是这个不对齐以前的表现或者其他工具的表现诶

@aooohan
Copy link
Copy Markdown
Member

aooohan commented Apr 26, 2026

但是这个不对齐以前的表现或者其他工具的表现诶

那既然这样,就合并吧。

@aooohan aooohan merged commit 2c00f33 into main Apr 26, 2026
12 checks passed
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.

4 participants