Skip to content

fix: recognize Windows drive-letter absolute paths as local paths in DependencyReference#435

Merged
danielmeppiel merged 4 commits intomainfrom
copilot/fix-windows-path-handling
Mar 24, 2026
Merged

fix: recognize Windows drive-letter absolute paths as local paths in DependencyReference#435
danielmeppiel merged 4 commits intomainfrom
copilot/fix-windows-path-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 24, 2026

is_local_path() didn't handle Windows absolute paths with drive letters (C:\..., D:/...), causing Windows CI to raise ValueError: Use 'user/repo'... for any tmp_path-based local path before local validation logic could run.

Description

DependencyReference.is_local_path() checked for ./, ../, /, ~\, .\, ..\ prefixes but missed the Windows absolute path pattern X:\ / X:/. On Windows, pytest's tmp_path produces paths like C:\Users\RUNNER\AppData\... which fell through to remote repo format validation and raised ValueError.

Fix — one additional guard in is_local_path():

# Windows absolute paths: drive letter + colon + separator (C:\ or C:/)
if len(s) >= 3 and s[0].isalpha() and s[1] == ':' and s[2] in ('\\', '/'):
    return True

Safe on Linux/macOS: Unix paths start with /, and user/repo shorthand never has : at index 1.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

4 new tests added to TestIsLocalPath: backslash separator, forward-slash separator, uppercase drive letter, lowercase drive letter.

Original prompt

Problem

The CI/CD Pipeline on the main branch is failing on the Windows runner with 4 test failures in tests/unit/test_install_command.py::TestLocalPathValidationMessages:

  • test_local_path_failure_reason_nonexistent
  • test_local_path_failure_reason_file_not_dir
  • test_local_path_failure_reason_no_markers
  • test_local_path_failure_reason_valid_apm_yml

All fail with the same error:

ValueError: Use 'user/repo' or 'github.com/user/repo' or 'dev.azure.com/org/project/repo' format

The error originates from src/apm_cli/models/dependency/reference.py line 703.

Context

  • This failure only occurs on the Windows runner — not on Linux/macOS.
  • Recent PRs have been fixing path-related issues, so this is likely a regression related to Windows path handling (e.g., backslashes \ vs forward slashes /, drive letter prefixes like C:\, etc.).
  • The tests provide local filesystem paths as input, but the parsing logic in reference.py is incorrectly interpreting these Windows-style paths as remote repository references and raising a ValueError before the local path validation logic gets a chance to run.

What to Fix

  1. Look at src/apm_cli/models/dependency/reference.py around line 703 and the logic that determines whether an input string is a local path vs a remote repo reference.
  2. Look at recent PRs/commits that modified path handling logic in this file for context on what changed.
  3. Look at the failing tests in tests/unit/test_install_command.py::TestLocalPathValidationMessages to understand what inputs they provide and what behavior they expect.
  4. Fix the parsing logic so that Windows-style local paths (with backslashes and/or drive letters) are correctly identified as local paths before the remote repo format validation runs.
  5. Make sure the fix doesn't break Linux/macOS behavior.

Latest Failing Run

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Fix CI on main branch in APM

Assistant:

I'd like to help you fix the CI on the main branch, but I need a bit more clarification:

Could you please provide the full repository name (in owner/repo format) for "APM"? For example, is it something like your-org/APM or danielmeppiel/APM?

User: Microsoft/apm

Assistant:

Let me start by investigating the current CI failures on the main branch of the repository.

Assistant: Let me look at the most recent CI/CD Pipeline failure to understand what's going wrong.

Assistant: I found the issue. The most recent CI/CD Pipeline run (#23474188874) on main is failing with 4 test failures — all in the same test class. Here's the summary:

Failing Tests

All 4 failures are in tests/unit/test_install_command.py::TestLocalPathValidationMessages:

Test Error
test_local_path_failure_reason_nonexistent ValueError: Use 'user/repo' or 'github.com/user/repo' or 'dev.azure.com/org/project/repo' format
test_local_path_failure_reason_file_not_dir Same ValueError
test_local_path_failure_reason_no_markers Same ValueError
test_local_path_failure_reason_valid_apm_yml Same ValueError

Root Cause

The error originates from src/apm_cli/models/dependency/reference.py (line 703). It appears that the code path for parsing a dependency reference is raising a ValueError (expecting a user/repo format) before the local path validation logic gets a chance to run. The tests expect local path-specific failure messages, but instead the input is being interpreted as a remote repo reference and rejected.

This is likely a regression where the input parsing order was changed — remote repo format validation now runs before local path checks.

Would you like me to create a PR to fix this?

I can open a pull request that investigates reference.py and the failing tests, and fixes the parsing logic so local paths are properly validated before the remote repo format check kicks in. Just confirm and I'll get started!

User: Yes, consider that this happens only on Windows runner and we were fixing stuff around paths on previous PRs

Assistant: I've kicked off the Copilot coding agent to create a PR that fixes the Windows path parsing regression in Microsoft/apm. It will:

  1. Investigate the parsing logic in src/apm_cli/models/dependency/reference.py around line 703
  2. Review the recent path-related changes for context
  3. Examine the failing TestLocalPathValidationMessages tests to understand expected behavior
  4. Fix the logic so Windows-style paths (backslashes, drive letters like C:\) are correctly recognized as local paths before the remote repo format check runs
  5. Ensure Linux/macOS be...

This pull request was created from Copilot chat.


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

…reference.py

Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/apm/sessions/9f97de46-d7db-4624-8633-30fdefbcf106
Copilot AI changed the title [WIP] Fix CI/CD pipeline issues on Windows runner fix: recognize Windows drive-letter absolute paths as local paths in DependencyReference Mar 24, 2026
Copilot AI requested a review from danielmeppiel March 24, 2026 07:17
@danielmeppiel danielmeppiel requested review from Copilot and removed request for danielmeppiel March 24, 2026 07:29
@danielmeppiel danielmeppiel marked this pull request as ready for review March 24, 2026 07:29
@danielmeppiel danielmeppiel self-requested a review as a code owner March 24, 2026 07:29
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

Fixes Windows CI failures by ensuring DependencyReference.is_local_path() recognizes Windows drive-letter absolute paths so they are treated as local dependencies (instead of falling into remote repo validation).

Changes:

  • Extend DependencyReference.is_local_path() to detect X:\... and X:/... patterns.
  • Add unit tests covering drive-letter absolute paths with both separators and letter casing.

Reviewed changes

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

File Description
src/apm_cli/models/dependency/reference.py Updates local-path detection to include Windows drive-letter absolute paths.
tests/unit/test_local_deps.py Adds regression tests for Windows absolute path detection.

danielmeppiel and others added 2 commits March 24, 2026 09:06
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel merged commit 1e34cbd into main Mar 24, 2026
6 checks passed
@danielmeppiel danielmeppiel deleted the copilot/fix-windows-path-handling branch March 24, 2026 10:24
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.

3 participants