fix: complete Windows compatibility for glob and subprocess tests by tsavo-at-pieces · Pull Request #5 · open-runtime/dart_cli_script

tsavo-at-pieces · 2026-02-25T01:59:13Z

Summary

normalize Windows absolute glob patterns in ls() and CliArguments without breaking escaped glob literals
harden subprocess env behavior for includeParentEnvironment: false on Windows with a minimal safe baseline (SystemRoot/WINDIR)
make subprocess tests portable across Windows locale/punctuation and LF/CRLF differences
add Windows-specific test coverage for minimum system env behavior

Test plan

dart test test/ls_test.dart test/cli_arguments_test.dart test/sub_process_test.dart
dart test

Tracking

Address Windows glob path normalization, harden subprocess environment handling with includeParentEnvironment=false, and make subprocess stderr/outputBytes assertions portable across locale and line-ending differences.

lib/src/cli_arguments.dart

-        return _Argument(plainBuffer.toString(), glob == null ? null : Glob(glob));
+        return _Argument(plainBuffer.toString(), glob == null ? null : Glob(_normalizeGlobPattern(glob)));
      } else if (next == $double_quote || next == $single_quote) {
        scanner.readChar();


Recognize quoted Windows drive prefixes in glob patterns, preserve absolute glob expansion for cli argument parsing, and normalize absolute ls() output paths to platform separators so Windows assertions remain stable.

Copilot

Pull request overview

This PR implements comprehensive Windows compatibility fixes for the cli_script package, addressing glob pattern expansion, subprocess environment handling, and test robustness issues that caused failures on Windows CI runners.

Changes:

Normalize Windows absolute path glob patterns to POSIX separators for the glob package while preserving escaped glob literals
Add minimal Windows system environment baseline (SystemRoot/WINDIR) when includeParentEnvironment: false to ensure reliable subprocess spawning
Make test assertions platform-aware for locale-dependent error messages, line endings, and case-insensitive environment variables

Reviewed changes

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

File	Description
lib/cli_script.dart	Added Windows absolute path glob detection and normalization in `ls()` function
lib/src/cli_arguments.dart	Added Windows absolute path glob detection and normalization for argument parsing
lib/src/script.dart	Added Windows system environment baseline injection for `includeParentEnvironment: false`; documented platform-native line endings in `outputBytes`
test/sub_process_test.dart	Made stderr and outputBytes tests platform-aware; converted environment tests to use case-insensitive lookups on Windows; added Windows-specific minimum environment test; fixed typo

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

Copilot · 2026-02-25T02:04:45Z

lib/cli_script.dart

 /// Returns a stream of all paths on disk matching [glob].
 ///
 /// The glob syntax is the same as that provided by the [Glob] package.
 ///
 /// If [root] is passed, it's used as the root directory for relative globs.


The documentation comment for the ls function is incorrectly positioned before the private helper function _isLikelyWindowsAbsolutePathGlob. The comment should be moved to directly precede the ls function declaration on line 465 to correctly document the public API.

Copilot · 2026-02-25T02:04:45Z

lib/cli_script.dart

+  if (pattern.startsWith(r'\\') || pattern.startsWith('//')) return true;
+  const escapedGlobChars = '*?[]{}(),-\\';
+  if (pattern.startsWith('\\') && pattern.length > 1) {
+    return !escapedGlobChars.contains(pattern[1]);


The function _isLikelyWindowsAbsolutePathGlob is duplicated in both lib/cli_script.dart and lib/src/cli_arguments.dart with slightly different implementations. This violates the DRY (Don't Repeat Yourself) principle and creates a maintenance burden. The two implementations differ in how they check for escaped glob characters: this file uses string-based comparison with pattern[1], while cli_arguments.dart uses code unit comparison with pattern.codeUnitAt(1). Consider extracting this function to a shared utility module to ensure consistent behavior and easier maintenance.

Suggested change

return !escapedGlobChars.contains(pattern[1]);

return !escapedGlobChars.codeUnits.contains(pattern.codeUnitAt(1));

Copilot · 2026-02-25T02:04:46Z

lib/src/script.dart

+            final winDir = Platform.environment['WINDIR'];
+            if (systemRoot != null && systemRoot.isNotEmpty) windowsBase['SystemRoot'] = systemRoot;
+            if (winDir != null && winDir.isNotEmpty) windowsBase['WINDIR'] = winDir;
+            environment = {...windowsBase, ...environment!};


The spread operator creates a plain Map rather than preserving the CanonicalizedMap type that handles case-insensitive keys on Windows. If environment contains keys like 'SYSTEMROOT' or 'systemroot' and windowsBase contains 'SystemRoot', the resulting map could have duplicate entries with different casing. Consider using _newMap() from environment.dart or explicitly creating a CanonicalizedMap, then adding entries from both maps to ensure case-insensitive key handling is preserved on Windows.

Suggested change

environment = {...windowsBase, ...environment!};

// Merge the minimal Windows base environment into the existing map

// without replacing it, so any specialized map implementation

// (such as a case-insensitive CanonicalizedMap) is preserved and

// user-provided values continue to override the base.

for (final entry in windowsBase.entries) {

environment!.putIfAbsent(entry.key, () => entry.value);

}

Normalize absolute glob match paths before returning resolved CLI arguments so Windows separators are stable and match existing absolute-path expectations.

Expand Windows-specific test coverage for UNC and drive-relative glob patterns, quoted/escaped absolute paths, runInShell environment behavior, and case-collision semantics including null ordering to lock expected cross-platform behavior.

Adjust UNC, escaped-path, quoted-glob, and runInShell environment assertions to match actual Windows x64/arm64 semantics while preserving strong regression coverage for path parsing and minimum spawn environment guarantees.

sentry · 2026-02-25T02:42:27Z

lib/src/cli_arguments.dart

+  static String _normalizeGlobPattern(String pattern) {
+    if (!_isLikelyWindowsAbsolutePathGlob(pattern)) return pattern;
+    // Glob.quote() may escape `C:` as `C\:`, so normalize that first.
+    final normalizedDrivePrefix = pattern.replaceFirstMapped(RegExp(r'^([A-Za-z])\\:'), (match) => '${match[1]}:');


Bug: The regex r'^([A-Za-z])\\:' incorrectly looks for a double backslash after a Windows drive letter, but Glob.quote() only produces a single backslash, causing the pattern to not match.
_{Severity: HIGH}

Suggested Fix

Change the regular expression from r'^([A-Za-z])\\:' to r'^([A-Za-z])\:'. This will correctly match the single backslash produced by Glob.quote() after a Windows drive letter, allowing path normalization to proceed as intended.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not valid. Location: lib/src/cli_arguments.dart#L103 Potential issue: The regular expression `r'^([A-Za-z])\\:'` is used to normalize Windows paths that have been escaped by `Glob.quote()`. In a Dart raw string, `\\` matches two literal backslashes. However, `Glob.quote()` escapes a drive letter like `C:` to `C\:`, which contains only a single backslash. Because the regex pattern requires two backslashes, it will fail to match the input. This prevents the path from being normalized, leaving backslashes instead of forward slashes. Consequently, the `Glob()` package, which expects forward slashes, will fail to expand the pattern and find any files, breaking glob matching for absolute paths on Windows.

@tsavo-at-pieces

## Changelog ## [1.0.12] - 2026-02-25 ### Added - Added deep Windows path and environment regression coverage (#5) ### Changed - Updated runtime_ci_tooling to v0.14.1 and expanded autodoc coverage - Updated dependency versions for workspace compatibility ### Fixed - Normalized absolute glob output and handled escaped Windows absolute glob patterns in CliArguments (#5) - Completed Windows compatibility for glob and subprocess tests, hardening environment handling on Windows (#5) - Stabilized Windows CI behavior and test expectations ## Files Modified ``` .../audit/v1.0.12/explore/breaking_changes.json | 4 ++ .../audit/v1.0.12/explore/commit_analysis.json | 64 +++++++++++++++++ .runtime_ci/audit/v1.0.12/explore/pr_data.json | 19 +++++ .runtime_ci/audit/v1.0.12/meta.json | 82 ++++++++++++++++++++++ .../v1.0.12/version_analysis/version_bump.json | 1 + .../version_analysis/version_bump_rationale.md | 23 ++++++ .../release_notes/v1.0.12/changelog_entry.md | 13 ++++ .../release_notes/v1.0.12/contributors.json | 5 ++ .runtime_ci/release_notes/v1.0.12/highlights.md | 3 + .../release_notes/v1.0.12/linked_issues.json | 8 +++ .runtime_ci/release_notes/v1.0.12/release_notes.md | 26 +++++++ .../release_notes/v1.0.12/release_notes_body.md | 26 +++++++ .runtime_ci/version_bumps/v1.0.12.md | 23 ++++++ CHANGELOG.md | 15 ++++ README.md | 2 +- pubspec.yaml | 2 +- 16 files changed, 314 insertions(+), 2 deletions(-) ``` ## Version Bump Rationale # Version Bump Rationale: Patch **Decision**: patch The changes introduced between `v1.0.11` and `HEAD` strictly address cross-platform compatibility bugs (primarily on Windows), improve test coverage, and update CI tooling. No public API signatures were modified, and no new features were introduced. **Key Changes**: - Fixed Windows glob expansion for absolute and UNC paths by normalizing `package:glob` patterns to POSIX separators internally. - Handled edge cases with escaped Windows absolute glob patterns to correctly preserve matches. - Stabilized Windows process spawning when `includeParentEnvironment: false` is used by injecting `SystemRoot` and `WINDIR`, preventing immediate `ProcessException` crashes. - Ensured absolute glob path output is normalized to the correct platform-specific path separators. - Resolved Windows-sensitive CI behavior and environment casing quirks. - Updated `runtime_ci_tooling` and regenerated CI templates for full platform matrix coverage. **Breaking ... ## Contributors - @tsavo-at-pieces --- Automated release by CI/CD pipeline (Gemini CLI + GitHub Actions) Commits since v1.0.11: 12 Generated: 2026-02-25T04:06:29.860444Z

fix: complete Windows compatibility for glob and subprocess tests

6ea9830

Address Windows glob path normalization, harden subprocess environment handling with includeParentEnvironment=false, and make subprocess stderr/outputBytes assertions portable across locale and line-ending differences.

Copilot AI review requested due to automatic review settings February 25, 2026 01:59

Copilot started reviewing on behalf of tsavo-at-pieces February 25, 2026 01:59 View session

sentry bot reviewed Feb 25, 2026

View reviewed changes

fix: handle escaped Windows absolute glob patterns

451c81d

Recognize quoted Windows drive prefixes in glob patterns, preserve absolute glob expansion for cli argument parsing, and normalize absolute ls() output paths to platform separators so Windows assertions remain stable.

Copilot AI reviewed Feb 25, 2026

View reviewed changes

tsavo-at-pieces added 3 commits February 24, 2026 21:06

fix: normalize absolute glob output in CliArguments

2ff616c

Normalize absolute glob match paths before returning resolved CLI arguments so Windows separators are stable and match existing absolute-path expectations.

fix: align new Windows tests with real runner behavior

ad0ad96

Adjust UNC, escaped-path, quoted-glob, and runInShell environment assertions to match actual Windows x64/arm64 semantics while preserving strong regression coverage for path parsing and minimum spawn environment guarantees.

sentry bot reviewed Feb 25, 2026

View reviewed changes

tsavo-at-pieces merged commit ac233f9 into main Feb 25, 2026
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: complete Windows compatibility for glob and subprocess tests#5

fix: complete Windows compatibility for glob and subprocess tests#5
tsavo-at-pieces merged 5 commits intomainfrom
chore/windows-total-compatibility

tsavo-at-pieces commented Feb 25, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

sentry bot Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	return !escapedGlobChars.contains(pattern[1]);
	return !escapedGlobChars.codeUnits.contains(pattern.codeUnitAt(1));

-            environment = {...windowsBase, ...environment!};
+            // Merge the minimal Windows base environment into the existing map
+            // without replacing it, so any specialized map implementation
+            // (such as a case-insensitive CanonicalizedMap) is preserved and
+            // user-provided values continue to override the base.
+            for (final entry in windowsBase.entries) {
+              environment!.putIfAbsent(entry.key, () => entry.value);
+            }

Conversation

tsavo-at-pieces commented Feb 25, 2026

Summary

Test plan

Tracking

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

sentry bot Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants