Skip to content

fix(python): add zip path normalization test and Windows CI#13383

Open
czubocha wants to merge 10 commits intomainfrom
sc-3798
Open

fix(python): add zip path normalization test and Windows CI#13383
czubocha wants to merge 10 commits intomainfrom
sc-3798

Conversation

@czubocha
Copy link
Copy Markdown
Contributor

@czubocha czubocha commented Mar 2, 2026

Fixes #13307

Summary

  • Fix Python requirements packaging on Windows by normalizing zip file paths to forward slashes (/), conforming to the ZIP specification.
  • Update over 130 test assertions in the Python test suite to use forward slashes instead of OS-specific separators (${sep}).
  • Update ci-python.yml to automatically run Python tests simultaneously on both Ubuntu and Windows whenever Python-related paths are modified on main.

Root cause

When packaging Python requirements using globSync on Windows, backslashes (\) were being preserved in the ZIP archive entries. This caused a mismatch in paths during extraction and subsequent imports. Furthermore, the test suite previously relied on the OS-specific path separator (${sep}) to assert archive contents, causing tests to fail when the ZIP entries were correctly normalized.

Test plan

  • Run Windows CI (windows-latest) for Python tests to verify all tests pass with normalized paths.
  • Verify Ubuntu CI continues to pass without regressions.

Summary by CodeRabbit

  • Tests

    • Added test to validate zip file paths use consistent formatting across platforms.
  • Chores

    • Updated CI/CD workflow to execute tests across multiple operating systems (Ubuntu and Windows).
    • Enhanced Windows environment setup for command-line tools with automatic configuration.

Note

Medium Risk
Changes how Python requirements are written into ZIP artifacts (path normalization and glob behavior), which could affect packaging output across platforms, but it’s scoped and covered by updated/added tests plus new Windows CI.

Overview
Fixes Windows Python requirements packaging by ensuring injected ZIP entry paths are normalized to forward slashes (ZIP-spec compliant) in packages/serverless/lib/plugins/python/lib/inject.js.

Updates the Python packaging test suite to assert forward-slash paths and adds a dedicated test that fails if any ZIP entry contains backslashes.

Extends ci-python.yml to run on pushes to main and adds a multi-OS matrix (Ubuntu + Windows on push) with Windows-specific sls setup via a new .github/setup-sls-cmd.cjs wrapper.

Written by Cursor Bugbot for commit a2f3801. This will update automatically on new commits. Configure here.

@Mmarzex
Copy link
Copy Markdown
Contributor

Mmarzex commented Mar 2, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 2, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 469ca59 and a2f3801.

📒 Files selected for processing (3)
  • .github/workflows/ci-python.yml
  • packages/serverless/lib/plugins/python/lib/inject.js
  • packages/sf-core/tests/python/test.js

📝 Walkthrough

Walkthrough

Adds OS-aware CI for Python tests with an OS matrix and distinct Unix/Windows CLI setup steps, creates a Windows batch wrapper for the Framework CLI, updates tests to expect POSIX-style ZIP entry paths, and normalizes injected file paths to use forward slashes on Windows.

Changes

Cohort / File(s) Summary
CI workflow & setup script
.github/workflows/ci-python.yml, .github/setup-sls-cmd.cjs
Adds push trigger and OS matrix (Ubuntu/Windows) for Python CI; splits CLI linking into OS-specific steps and adds a Node script that writes sls.cmd into .github/bin for Windows.
Python packaging tests
packages/sf-core/tests/python/test.js
Replaces platform-dependent path separators with forward-slash paths in multiple assertions; adds a test to assert ZIP entries use forward slashes (POSIX-style) for packaged modules.
Path normalization in injector
packages/serverless/lib/plugins/python/lib/inject.js
Enables windowsPathsNoEscape for globbing and converts backslashes to forward slashes when building injection mappings so paths are POSIX-style.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰
I hopped through CI, with tiny paws,
Made slashes friendly to all OS laws.
A cmd was baked for Windows cheer,
Tests now agree, the paths are clear.
Hooray — cross-platform carrots near! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding zip path normalization test and Windows CI support for Python functionality.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sc-3798

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/ci-python.yml:
- Around line 18-24: The matrix definition uses an invalid runner label
'gh-windows-latest' for matrix.os which prevents the workflow from running;
update the matrix value in the matrix: os list (reference: matrix.os in the job
named 'Test: Python Requirements') replacing 'gh-windows-latest' with the
correct GitHub Actions runner label 'windows-latest' so the job can be scheduled
on Windows hosts.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 067c242 and 110eaef.

📒 Files selected for processing (2)
  • .github/workflows/ci-python.yml
  • packages/sf-core/tests/python/test.js

Comment thread .github/workflows/ci-python.yml Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 110eaefee5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci-python.yml Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.github/workflows/ci-python.yml (1)

21-24: ⚠️ Potential issue | 🔴 Critical

Invalid matrix runner label still blocks Windows CI.

gh-windows-latest is not a valid GitHub-hosted runner label, so this job cannot be scheduled on Windows.

Suggested fix
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-latest, gh-windows-latest]
+        os: [ubuntu-latest, windows-latest]
#!/bin/bash
set -euo pipefail

echo "Checking matrix labels in .github/workflows/ci-python.yml"
rg -n 'matrix:|os:|gh-windows-latest|windows-latest' .github/workflows/ci-python.yml -n -C2

echo
echo "Optional: run actionlint if available"
if command -v actionlint >/dev/null 2>&1; then
  actionlint .github/workflows/ci-python.yml
else
  echo "actionlint not installed in this environment"
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-python.yml around lines 21 - 24, The CI matrix uses an
invalid runner label ("gh-windows-latest") under the "matrix" -> "os" entries
which prevents scheduling Windows jobs; update the matrix entry to use the
correct GitHub-hosted runner label ("windows-latest") instead of
"gh-windows-latest" (so the "os" array becomes [ubuntu-latest, windows-latest])
and optionally validate the workflow with actionlint or the provided grep checks
to ensure no other invalid labels remain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/ci-python.yml:
- Around line 21-24: The CI matrix uses an invalid runner label
("gh-windows-latest") under the "matrix" -> "os" entries which prevents
scheduling Windows jobs; update the matrix entry to use the correct
GitHub-hosted runner label ("windows-latest") instead of "gh-windows-latest" (so
the "os" array becomes [ubuntu-latest, windows-latest]) and optionally validate
the workflow with actionlint or the provided grep checks to ensure no other
invalid labels remain.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 110eaef and a8c28be.

📒 Files selected for processing (1)
  • .github/workflows/ci-python.yml

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50a3907766

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci-python.yml Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.github/workflows/ci-python.yml (1)

24-24: ⚠️ Potential issue | 🔴 Critical

Use a valid GitHub-hosted Windows runner label.

Line 24 uses gh-windows-latest, which is not a valid runner label and prevents job scheduling on Windows.

Suggested fix
       matrix:
-        os: [ubuntu-latest, gh-windows-latest]
+        os: [ubuntu-latest, windows-latest]
#!/bin/bash
set -euo pipefail

echo "Checking matrix runner labels in .github/workflows/ci-python.yml"
cat -n .github/workflows/ci-python.yml | sed -n '20,26p'

if command -v actionlint >/dev/null 2>&1; then
  echo
  echo "Running actionlint on workflow file..."
  actionlint -oneline .github/workflows/ci-python.yml
else
  echo
  echo "actionlint not installed in this environment; install it and rerun for definitive validation."
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-python.yml at line 24, The workflow matrix uses an
invalid Windows runner label `gh-windows-latest` in the `os: [ubuntu-latest,
gh-windows-latest]` entry; update that value to a valid GitHub-hosted runner
label such as `windows-latest` (i.e., change `gh-windows-latest` →
`windows-latest`) so the job can schedule on Windows, then validate the workflow
with actionlint or by re-running the CI to ensure no other labels are invalid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/ci-python.yml:
- Line 24: The workflow matrix uses an invalid Windows runner label
`gh-windows-latest` in the `os: [ubuntu-latest, gh-windows-latest]` entry;
update that value to a valid GitHub-hosted runner label such as `windows-latest`
(i.e., change `gh-windows-latest` → `windows-latest`) so the job can schedule on
Windows, then validate the workflow with actionlint or by re-running the CI to
ensure no other labels are invalid.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a8c28be and 50a3907.

📒 Files selected for processing (1)
  • .github/workflows/ci-python.yml

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90c319e717

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/setup-sls-cmd.cjs
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.github/workflows/ci-python.yml (1)

24-24: ⚠️ Potential issue | 🔴 Critical

Invalid Windows runner label blocks job scheduling.

Line 24 uses gh-windows-latest, which is not a valid GitHub-hosted runner label; use windows-latest.

Suggested fix
-        os: [ubuntu-latest, gh-windows-latest]
+        os: [ubuntu-latest, windows-latest]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-python.yml at line 24, The matrix `os` entry uses an
invalid runner label `gh-windows-latest` which prevents job scheduling; update
the `os` matrix (the line containing `os: [ubuntu-latest, gh-windows-latest]`)
to replace `gh-windows-latest` with the supported `windows-latest` label so the
CI can run on Windows-hosted runners.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/ci-python.yml:
- Line 24: The matrix `os` entry uses an invalid runner label
`gh-windows-latest` which prevents job scheduling; update the `os` matrix (the
line containing `os: [ubuntu-latest, gh-windows-latest]`) to replace
`gh-windows-latest` with the supported `windows-latest` label so the CI can run
on Windows-hosted runners.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50a3907 and 90c319e.

📒 Files selected for processing (2)
  • .github/setup-sls-cmd.js
  • .github/workflows/ci-python.yml

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
.github/workflows/ci-python.yml (1)

24-24: ⚠️ Potential issue | 🔴 Critical

Invalid Windows runner label blocks job scheduling.

Line 24 uses gh-windows-latest, which is not a valid GitHub-hosted runner label. Use windows-latest so the matrix job can be scheduled.

Suggested fix
-        os: [ubuntu-latest, gh-windows-latest]
+        os: [ubuntu-latest, windows-latest]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci-python.yml at line 24, The GitHub Actions matrix entry
using the os key contains an invalid runner label 'gh-windows-latest' which
prevents job scheduling; update the matrix value for os (the line with "os:
[ubuntu-latest, gh-windows-latest]") to use the valid hosted runner label
'windows-latest' (i.e., replace 'gh-windows-latest' with 'windows-latest') so
the matrix job can run on Windows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/ci-python.yml:
- Line 24: The GitHub Actions matrix entry using the os key contains an invalid
runner label 'gh-windows-latest' which prevents job scheduling; update the
matrix value for os (the line with "os: [ubuntu-latest, gh-windows-latest]") to
use the valid hosted runner label 'windows-latest' (i.e., replace
'gh-windows-latest' with 'windows-latest') so the matrix job can run on Windows.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90c319e and 469ca59.

📒 Files selected for processing (2)
  • .github/setup-sls-cmd.cjs
  • .github/workflows/ci-python.yml

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba16d48f43

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci-python.yml Outdated
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90d6beafa0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/serverless/lib/plugins/python/lib/inject.js
@czubocha
Copy link
Copy Markdown
Contributor Author

czubocha commented Mar 3, 2026

@cursor review

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2f380135b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/ci-python.yml
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.

Dependencies not included in final zip package

2 participants