Skip to content

CHEF-33964 & CHEF-33965 & CHEF-33975: Cookstyle version enforcement and customization docs - #274

Merged
nikhil2611 merged 5 commits into
mainfrom
nikhil/CHEF-33964-33965-cookstyle-upgrade-and-docs
Aug 11, 2026
Merged

nikhil2611 merged 5 commits into
mainfrom
nikhil/CHEF-33964-33965-cookstyle-upgrade-and-docs

Conversation

@nikhil2611

@nikhil2611 nikhil2611 commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

This PR combines CHEF-33964 and CHEF-33965 to address the VSCode vs CLI linting conflicts issue.

Problem

The VSCode extension was giving different linting suggestions than command-line Cookstyle, particularly around array formatting (["a", "b"] vs %w[a b]). This was caused by shipping an outdated Cookstyle version (< 8.6.10 with RuboCop 1.25) while users' CLI had newer versions.

Solution

1. Enforce Minimum Cookstyle Version (CHEF-33964)

  • Added version check requiring Cookstyle 8.6.10 or later
  • Shows warning message with upgrade instructions if outdated version detected
  • Ensures VSCode and CLI use the same modern RuboCop version for consistent behavior

2. Document Customization Options (CHEF-33965)

  • Added README documentation for customizing Cookstyle/RuboCop behavior
  • Documented two methods:
    • Creating .rubocop.yml in workspace root (auto-discovered)
    • Using rubocop.configFile VSCode setting for custom config path
  • Included examples for preventing %-escaped literals for users who prefer bracket-style arrays

Testing

✅ Verified both customization methods work correctly with Cookstyle 8.6.10:

  • Workspace .rubocop.yml is automatically discovered by Cookstyle
  • --config flag properly applies custom configuration files
  • Both VSCode and CLI show identical linting behavior

Changes

  • extension.ts: Added checkCookstyleVersion() function with minimum version enforcement
  • README.md: Added "Customizing Cookstyle Behavior" section with examples

Fixes


Note: This PR replaces #271 and #272 with a cleaner commit history.

…ion docs

This PR combines both tickets to address the VSCode vs CLI linting conflicts:

Changes:
- Add minimum Cookstyle version check (8.6.10+) to ensure consistent behavior
- Show warning message if user has outdated Cookstyle installation
- Add README documentation for customizing Cookstyle/RuboCop behavior
- Document two customization methods:
  * Creating .rubocop.yml in workspace root
  * Using rubocop.configFile setting for custom config path
- Provide examples for preventing %-escaped literals if users prefer

Root Cause Fixed:
The issue was caused by old Cookstyle (< 8.6.10 with RuboCop 1.25) giving
different linting suggestions than modern Cookstyle. By enforcing minimum
version 8.6.10+, both VSCode and command-line Cookstyle now use the same
RuboCop version, providing consistent linting behavior.

Customization Tested:
Both documented customization methods verified working with Cookstyle 8.6.10:
- Workspace .rubocop.yml automatically discovered
- --config flag properly applies custom configuration

Fixes: CHEF-33964, CHEF-33965
Signed-off-by: nikhil2611 <ngupta@progress.com>

Copilot AI left a comment

Copy link
Copy Markdown

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 aims to eliminate VS Code vs CLI Cookstyle/RuboCop linting discrepancies by enforcing a minimum Cookstyle version in the extension and documenting how users can customize Cookstyle behavior via configuration.

Changes:

  • Add a Cookstyle version check on extension activation to warn when the installed linter is below Cookstyle 8.6.10.
  • Update README with minimum Cookstyle requirements and instructions/examples for customizing rules via .rubocop.yml and rubocop.configFile.

Reviewed changes

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

File Description
README.md Documents Cookstyle minimum version requirement and configuration/customization options.
extension.ts Adds runtime Cookstyle version detection and warning messaging on activation.
Suppressed comments (2)

extension.ts:101

  • If the version check throws, cookstyleVersionChecked never gets set to true, so the warning dialog can be re-shown on subsequent triggers (e.g., configuration watcher calling validation paths). Mark the check as completed in the error path as well.
	} catch (err) {
		console.log("Could not check Cookstyle version:", err);
		vscode.window.showWarningMessage(

README.md:30

  • This bullet also calls 26.1.0 the “Latest version”, which will quickly go stale in documentation.
- Latest version (26.1.0) includes Cookstyle 8.7.6 and all Chef tools

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread extension.ts Resolved
Comment thread README.md Resolved
Comment thread README.md Resolved
- Fix spawnSync error handling to catch ENOENT when executable doesn't exist
- Remove extra closing parenthesis in README settings documentation
- Update 'latest' to 'For example' to avoid version info becoming outdated

Addresses Copilot AI review comments on PR #274

Signed-off-by: nikhil2611 <ngupta@progress.com>
Copilot AI review requested due to automatic review settings August 10, 2026 13:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (4)

extension.ts:86

  • The version comparison assumes the parsed version has exactly 3 numeric components. If cookstyle --version ever returns a shorter/atypical string (e.g., missing patch, pre-release, or additional text that makes parsing produce NaN), parts[i] comparisons will be unreliable and can incorrectly treat an old/unparseable version as compatible.
				// Parse version components
				let parts = version.split('.').map(Number);
				let minParts = MINIMUM_COOKSTYLE_VERSION.split('.').map(Number);
				
				// Check if version is below minimum
				let isOldVersion = false;
				for (let i = 0; i < 3; i++) {
					if (parts[i] < minParts[i]) {
						isOldVersion = true;
						break;
					} else if (parts[i] > minParts[i]) {
						break;
					}
				}

extension.ts:113

  • The warning in the catch block always suggests installing Chef Workstation, but this path is also hit when rubocop.path is configured incorrectly or points to a non-executable file. Including the configured path (and hinting at rubocop.path) would make this actionable for users who install Cookstyle via a gem or non-default location.
		console.log("Could not check Cookstyle version:", err);
		vscode.window.showWarningMessage(
			`Chef extension could not detect Cookstyle. Install the latest Chef Workstation for linting features.`,
			"Download Chef Workstation"
		).then(selection => {
			if (selection === "Download Chef Workstation") {
				vscode.env.openExternal(vscode.Uri.parse("https://downloads.chef.io/chef-workstation"));
			}
		});

README.md:36

  • The README suggests installing Cookstyle via gem install cookstyle, but the extension defaults to the Chef Workstation binary path and won’t automatically use the gem-installed executable unless users also set rubocop.path (or set it to a PATH-resolvable cookstyle). Adding that step avoids users following Option 2 and still getting a startup warning / no linting.
**Option 2: Install Cookstyle gem directly**
```bash
gem install cookstyle  # Installs latest version
**README.md:19**
* Typo: VS Code calls this the “Command Palette”, not “Command Pallette”.
  • You may lint the entire workspace, even if it is larger than the above threshold, using the Chef: Validate Entire Workspace command from the Command Pallette.
</details>

- Fix typo: 'Command Pallette' → 'Command Palette'
- Add note about configuring rubocop.path when using gem-installed Cookstyle
- Improve error message to show configured path for easier troubleshooting

These changes improve UX and help users diagnose configuration issues.

Signed-off-by: nikhil2611 <ngupta@progress.com>
Copilot AI review requested due to automatic review settings August 10, 2026 13:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread extension.ts Resolved
Comment thread extension.ts Resolved
Stromweld
Stromweld previously approved these changes Aug 10, 2026
Comment thread README.md Resolved
- Add workspace trust check to prevent executing untrusted binaries
- Add 5-second timeout to spawnSync to prevent extension startup hangs
- Add windowsHide flag to suppress console windows on Windows

These changes improve security by respecting VS Code Workspace Trust
and prevent potential extension host freezes during activation.

Addresses Copilot AI High priority security feedback.

Signed-off-by: nikhil2611 <ngupta@progress.com>
Copilot AI review requested due to automatic review settings August 10, 2026 15:36
Replace dead downloads.chef.io links with:
- Progress Customer Portal reference for downloads
- Chef Workstation installation docs (docs.chef.io/workstation/install/)

Addresses @Stromweld's review feedback about dead download link.

Signed-off-by: nikhil2611 <ngupta@progress.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (3)

extension.ts:40

  • checkCookstyleVersion() runs a spawnSync (up to 5s timeout) during activation, which can block the extension host and delay VS Code startup. Consider deferring the version check and re-running it when the workspace becomes trusted.
		checkCookstyleVersion();
		updateRubyFileCountAndValidate(true);

README.md:20

  • The README says Chef Workstation is required, but the same section immediately lists installing the Cookstyle gem directly as an alternative. Consider wording this as a Cookstyle 8.6.10+ requirement (with Chef Workstation as the recommended way to get it) to avoid conflicting guidance.
* **Requirements**: This extension requires [Chef Workstation](https://downloads.chef.io/chef-workstation) installed with [Cookstyle](https://github.com/chef/cookstyle) **8.6.10 or later**. For example, Chef Workstation 26.1.0 includes Cookstyle 8.7.6. Linting should "just work" on Windows, macOS, and Linux.

extension.ts:91

  • The version comparison assumes a 3-component semver. If Cookstyle ever reports a version like "8.6" (or anything that parses to NaN), the current logic will treat it as compatible because comparisons against NaN are always false. Normalizing to 3 components and treating parse failures as "old" avoids silently skipping the minimum-version enforcement.
				let parts = version.split('.').map(Number);
				let minParts = MINIMUM_COOKSTYLE_VERSION.split('.').map(Number);
				
				// Check if version is below minimum
				let isOldVersion = false;

Copilot AI review requested due to automatic review settings August 10, 2026 15:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (3)

extension.ts:66

  • spawnSync blocks the extension host event loop; running it during activation can noticeably delay startup (up to the 5s timeout). Consider switching to an async execFile/spawn-based version check (or deferring it) so activation stays responsive.
		let spawn = require("child_process").spawnSync;
		let result = spawn(rubocopPath, ["--version"], { encoding: "utf-8", timeout: 5000, windowsHide: true });

extension.ts:42

  • If the workspace starts out untrusted, checkCookstyleVersion() returns early and the version check will never run later (it’s only called once during activation). That means the minimum Cookstyle enforcement can be skipped entirely for users who trust the workspace after opening it.
	if (vscode.workspace.getConfiguration("rubocop").enable) {
		checkCookstyleVersion();
		updateRubyFileCountAndValidate(true);
		context.subscriptions.push(startLintingOnSaveWatcher());
		context.subscriptions.push(startLintingOnConfigurationChangeWatcher());

extension.ts:106

  • When cookstyle --version outputs stdout but the version regex doesn’t match, the code falls through without warning and still marks the version check as done. That means minimum-version enforcement can be silently skipped if the output format changes.
				} else {
					console.log(`Cookstyle version ${version} is compatible (minimum: ${MINIMUM_COOKSTYLE_VERSION})`);
				}
			}
		}

Copilot AI review requested due to automatic review settings August 10, 2026 15:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (3)

extension.ts:40

  • checkCookstyleVersion() uses spawnSync, which blocks the extension host during activation (up to the 5s timeout). Deferring the check avoids delaying activation/UI responsiveness; longer-term, consider switching the version check to an async exec instead of sync spawn.
	if (vscode.workspace.getConfiguration("rubocop").enable) {
		checkCookstyleVersion();
		updateRubyFileCountAndValidate(true);

README.md:20

  • The Requirements bullet says the extension "requires Chef Workstation" but the README also documents installing the Cookstyle gem directly (Option 2). This is contradictory; the requirement should be phrased as requiring Cookstyle 8.6.10+ (via Workstation or gem).
* **Requirements**: This extension requires [Chef Workstation](https://docs.chef.io/workstation/install/) installed with [Cookstyle](https://github.com/chef/cookstyle) **8.6.10 or later**. For example, Chef Workstation 26.1.0 includes Cookstyle 8.7.6. Linting should "just work" on Windows, macOS, and Linux.

extension.ts:76

  • This version parsing will treat any --version output that contains a x.y.z as a Cookstyle version. If rubocop.path points to non-Cookstyle (or Cookstyle prints version to stderr), the regex can mis-detect and incorrectly warn that Cookstyle is below 8.6.10. Consider validating that the output looks like Cookstyle and using stdout/stderr consistently before comparing versions.
		if (result.stdout) {
			let versionMatch = result.stdout.match(/(\d+\.\d+\.\d+)/);
			if (versionMatch) {
				let version = versionMatch[1];
				console.log(`Detected Cookstyle version: ${version}`);

@nikhil2611
nikhil2611 requested a review from Stromweld August 10, 2026 16:12
@nikhil2611
nikhil2611 merged commit 1fc939f into main Aug 11, 2026
10 checks passed
nikhil2611 added a commit that referenced this pull request Aug 11, 2026
- Fix spawnSync error handling to catch ENOENT when executable doesn't exist
- Remove extra closing parenthesis in README settings documentation
- Update 'latest' to 'For example' to avoid version info becoming outdated

Addresses Copilot AI review comments on PR #274

Signed-off-by: nikhil2611 <ngupta@progress.com>
@nikhil2611 nikhil2611 changed the title CHEF-33964 & CHEF-33965: Cookstyle version enforcement and customization docs CHEF-33964 & CHEF-33965 & CHEF-33975: Cookstyle version enforcement and customization docs Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants