[LouisClt]

Related GitHub Issue

Closes: #2448

Description

On Windows, getSystemInfoSection() now uses os.version() instead of os-name. os.version() is a Node.js built-in and does not start a process. Other platforms still use os-name, which does not start a process there.

Points to check:

• The prompt text changes on Windows, from Windows 11 to the full product name, for example Windows 11 Pro.
• If os.version() returns an empty string, the section uses win32 <release>, as the old fallback did.

Test Procedure

• cd src && npx vitest run core/prompts/sections/__tests__/system-info.spec.ts: 7 tests pass. Two new Windows tests check that os-name is not called. They fail on the old code.
• cd src && npx vitest run core/prompts core/webview/__tests__/generateSystemPrompt.spec.ts: 435 tests pass.
• tsc --noEmit and ESLint pass. I removed two unnecessary as any casts from the spec, and its entry in eslint-suppressions.json.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I reviewed my own code with care.
• Writing Style: The PR title and description use Simplified Technical English (ASD-STE100): short sentences, plain words, active voice, one idea per sentence, no contractions.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Test Helpers (test changes only): Shared test helpers (src/test-utils/, @/utils/test-utils) are used only for mechanical duplication; test intent and scenario-specific mocks remain visible.
• Visual Snapshot (UI changes only): No UI change.
• Documentation Impact: I checked if my changes need documentation updates (see "Documentation Updates" section below).
• Contribution Guidelines: I read and agree to the Contributor Guidelines.

Visual Snapshots

No UI change.

Videos (interaction / animation only)

No UI change.

Documentation Updates

• No documentation updates are required.

Additional Notes

I used Claude Code to find the cause and write the fix and the tests.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved operating-system information reporting. Windows now uses its version when available and falls back to platform and release details if the version is empty or unavailable. Other platforms use the detected OS name when available, with the same fallback if detection fails.

Windows system information now uses os.version() when available. If it returns an empty value or throws, the code uses the platform and release. Other platforms continue to use osName() with the same fallback when that lookup throws.

Changes: System Information

Layer / File(s)	Summary
OS lookup and validation src/core/prompts/sections/system-info.ts, src/core/prompts/sections/__tests__/system-info.spec.ts, src/eslint-suppressions.json	getSystemInfoSection now uses getOsInfo. Windows uses a nonempty os.version() result, or falls back to platform and release. Linux falls back when osName() throws. Tests check these paths and verify that Windows does not call os-name. The test file’s ESLint suppression is removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: Minimal · up to d06a9

Windows system prompt generation no longer starts a process to look up the OS name, which removes the reported request delays. The prompt text may now show a full product name such as "Windows 11 Pro". No merge-blocking risk was found.

Pre-merge checks: 8

Check name	Status	Explanation
Title check		The title names the main change: the Windows OS name lookup no longer starts PowerShell. It matches the linked issue #2448 and the summary of system-info.ts, which now uses os.version() on Windows. It…
Description check		The description follows the repository template. It links issue #2448, explains the how (os.version() replaces os-name on Windows), lists reviewer checks (prompt text change, empty-version fallback), …
Linked Issues check		[#2448] The issue requires that OS name lookup on Windows does not block the extension host. At the reviewed head, getOsInfo() in src/core/prompts/sections/system-info.ts checks os.platform() === "win…
Out of Scope Changes check		Changes from the merge base cover src/core/prompts/sections/system-info.ts (the new getOsInfo() helper and its call site in getSystemInfoSection()), the matching spec src/core/prompts/sections/__tests…
Regression Evidence		The PR changes getSystemInfoSection through a new getOsInfo() helper (src/core/prompts/sections/system-info.ts). Its changed behaviors each have focused unit coverage in system-info.spec.ts, at …
Security Boundaries		Summary: No changed path leaks secrets or PII, executes unvalidated input, or bypasses approval or allowlist controls. Evidence (diff of src/core/prompts/sections/system-info.ts): - The diff…
Persistence Integrity		The check "Persistence integrity" applies only to changed persistence paths: unawaited operations, non-atomic writes, missing rollback or partial-failure handling, or persisted defaults not propagated…
Lifecycle Resource Cleanup		The PR changes one source file, src/core/prompts/sections/system-info.ts, plus its tests and an eslint suppression entry. The source diff adds a synchronous helper, getOsInfo(). On Windows it retu…

Full details: Title check

Explanation

The title names the main change: the Windows OS name lookup no longer starts PowerShell. It matches the linked issue #2448 and the summary of system-info.ts, which now uses os.version() on Windows. It uses the conventional fix(scope) form and is 10 words long, so it stays concise.

Full details: Description check

Explanation

The description follows the repository template. It links issue #2448, explains the how (os.version() replaces os-name on Windows), lists reviewer checks (prompt text change, empty-version fallback), and gives a test procedure with results. The checklist is filled in. The Discord contact field under Get in Touch is missing, and the Visual Snapshots and Videos sections are marked 'No UI change' rather than removed, which is acceptable. The description is mostly complete.

Full details: Linked Issues check

Explanation

[#2448] The issue requires that OS name lookup on Windows does not block the extension host. At the reviewed head, getOsInfo() in src/core/prompts/sections/system-info.ts checks os.platform() === "win32" and returns os.version() || fallback. os.version() is a Node.js built-in and starts no process, so the os-name path (wmic, then PowerShell via execaSync) is not reached on Windows. getSystemInfoSection() calls getOsInfo() on every request. If os.version() returns an empty string or throws, the code uses the existing fallback os.platform() plus os.release(). The PR description reports two new Windows tests that assert os-name is not called and fail on the old code. I did not open the spec file in this pass, so that test result rests on the PR description and the change summary. The issue does not require changes on other platforms, and those still call osName() as before.

Full details: Out of Scope Changes check

Explanation

Changes from the merge base cover src/core/prompts/sections/system-info.ts (the new getOsInfo() helper and its call site in getSystemInfoSection()), the matching spec src/core/prompts/sections/tests/system-info.spec.ts (Windows tests and mock cleanup), and src/eslint-suppressions.json (removal of the suppression entry for that spec after its as-any casts were removed). The source and test changes implement or verify the Windows OS name requirement of #2448. The lint suppression removal follows from the spec cleanup. The non-Windows path keeps osName() with the same fallback. I found no unrelated changes.

Full details: Regression Evidence

Explanation

The PR changes getSystemInfoSection through a new getOsInfo() helper (src/core/prompts/sections/system-info.ts). Its changed behaviors each have focused unit coverage in system-info.spec.ts, at the same layer as the function: - Windows, non-empty os.version(): the new test "reads the product name from os.version() without calling os-name" asserts Operating System: Windows 11 Pro and that os-name is not called. - Windows, empty os.version() (unset branch): "falls back to platform and release when os.version() is empty" asserts win32 10.0.26300 and that os-name is not called. - Windows, os.version() throws (error branch): the parameterized it.each case for win32 mocks os.version to throw and asserts the platform-and-release fallback. - Non-Windows, os-name returns: the existing "should return system info with os-name when available" test (linux) covers this path. - Non-Windows, os-name throws (error branch): the parameterized it.each case for linux covers this. The old Windows-with-os-name-failure test was replaced by these cases, so no affected branch is dropped. The vi.restoreAllMocks() addition keeps the spies isolated between tests. The ESLint suppression removal and the as any cleanup are mechanical. The PR has no durable UI change, so the Playwright snapshot requirement does not apply.

Full details: Security Boundaries

Explanation

Summary: No changed path leaks secrets or PII, executes unvalidated input, or bypasses approval or allowlist controls. Evidence (diff of src/core/prompts/sections/system-info.ts): - The diff changes only how the OS string is produced. On win32, getOsInfo() now returns os.version(), or os.platform() + os.release() when that is empty. Other platforms still call osName(). Any throw falls back to os.platform() + os.release(). - os.version() is a Node.js built-in. It does not start a process, so the change removes a wmic/PowerShell execaSync call rather than adding one. This removes the blocking behavior described in issue #2448 and adds no new execution surface. - The value goes into the system prompt as a product or version string. It is not a credential, token, or user-identifying field, and it adds no new data flow. - No approval, allowlist, or tool-policy logic changes. policy.tools handling is untouched. - Changed files are the spec, the section source, and eslint-suppressions.json. None of them touches auth, secrets, or command execution paths. Scope note: The check fails only on the listed conditions. The prompt text change ("Windows 11" to a full product name) is a behavior change, not a security boundary issue.

Full details: Persistence Integrity

Explanation

The check "Persistence integrity" applies only to changed persistence paths: unawaited operations, non-atomic writes, missing rollback or partial-failure handling, or persisted defaults not propagated to consumers. The PR (#2449, author LouisClt) changes three files: src/core/prompts/sections/system-info.ts, its spec, and src/eslint-suppressions.json. The diff in system-info.ts adds a getOsInfo() helper. It selects os.version() on Windows and osName() elsewhere, and falls back to os.platform() plus os.release() on error. getSystemInfoSection then uses that string to build prompt text. No file read, write, storage, or state-sync code changes. The other two files are a test and a lint-suppression entry. The check is therefore inapplicable to this PR, and the verdict is PASS.

Full details: Lifecycle Resource Cleanup

Explanation

The PR changes one source file, src/core/prompts/sections/system-info.ts, plus its tests and an eslint suppression entry. The source diff adds a synchronous helper, getOsInfo(). On Windows it returns os.version() when that value is nonempty, and otherwise returns os.platform() plus os.release(). On other platforms it calls osName(). Any throw falls back to the platform and release string. getSystemInfoSection() now calls this helper in place of its inline osName() try/catch. This change does not create or hold any lifecycle resource. The diff adds no listener, watcher, provider, timer, task, or subscription. It adds no async work, cancellation path, or disposal path. The only runtime effect is that Windows no longer runs os-name, which starts wmic or PowerShell through execaSync. That removes a blocking child process from each request, as issue #2448 asks. It does not duplicate work or leak a process. The check fails only on a changed lifecycle path that leaks a resource or duplicates work after cancellation, disposal, or restart. No such path exists in the changed code, so the check passes.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Zoo-Code-Org/Zoo-Code
• Branch: main
• File: .coderabbit.yaml
• Commit: f2b4d1c
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 3e75ed71-a693-40cc-a227-8245744be672

Commits

Reviewing files that changed from the base of the PR and between 6089bdb and d06a98c.

Files selected for processing: 2

• src/core/prompts/sections/__tests__/system-info.spec.ts
• src/core/prompts/sections/system-info.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Recent review details

Additional context used

Path-based instructions: 5

Treat model, provider, MCP, path, command, and tool data as untrusted.

CodeRabbit configuration file

Files:

• src/core/prompts/sections/__tests__/system-info.spec.ts
• src/core/prompts/sections/system-info.ts

Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.

CodeRabbit configuration file

Files:

• src/core/prompts/sections/__tests__/system-info.spec.ts

Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.

CodeRabbit configuration file

Files:

• src/core/prompts/sections/__tests__/system-info.spec.ts
• src/core/prompts/sections/system-info.ts

Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.

CodeRabbit configuration file

Files:

• src/core/prompts/sections/__tests__/system-info.spec.ts
• src/core/prompts/sections/system-info.ts

Act as an adversarial second-opinion reviewer.

CodeRabbit configuration file

Files:

• src/core/prompts/sections/__tests__/system-info.spec.ts
• src/core/prompts/sections/system-info.ts

[github-actions]

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: The required review sequence passed. Remaining merge requirements apply.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[edelauna]

Nice! Thanks for this contribution, just had 1 nit.

[edelauna]

On win32 os.version() runs outside any try/catch, unlike the old osName() call. Can it throw here (I believe Node wraps it so a failed uv_os_uname raises ERR_SYSTEM_ERROR)? If so, every prompt build on Windows would fail instead of using the platform release fallback. Would it make sense to put both branches inside the same try?

[LouisClt]

Thanks for the comment. In current Node, os.version() cannot throw: os computes the value once when the module loads. But it costs nothing to put it in the try block, so I did it in d06a98c.

[edelauna]

ty