Python script-only packages: follow-on QA fixes - #50143
Conversation
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
This PR addresses follow-on QA fixes for “Python script-only packages,” improving validation messaging consistency across add/edit flows, enabling .py setup_experience_platform parity with .sh, and enhancing install-time diagnostics and script temp-file handling in orbit.
Changes:
- Backend: make installer validation errors action-neutral; improve unsupported-type messaging; allow
.pysetup-experience platforms; correct “macOS and Linux hosts” wording. - Agent (orbit): choose temp script extension based on shebang (
.py/.sh/.ps1) and surface execve failures (exit code-1) into script output. - Frontend/tests: normalize edit-modal error prefixing to avoid doubled verbs; ensure add flow tarball errors include the correct verb; update/extend tests accordingly.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| changes/41470-python-script-only-followups | User-visible change entry (content excluded by policy). |
| server/service/integration_enterprise_test.go | Updates integration expectations for action-neutral backend error messages. |
| server/fleet/software_test.go | Adds coverage for -1 (script timeout/could-not-run) output enhancement behavior. |
| server/fleet/software_installer.go | Adds “couldn’t run script” copy for exit code -1; allows .py setup-experience platforms. |
| server/fleet/software_installer_test.go | Adds tests for allowed setup-experience platforms (including .py). |
| server/fleet/scripts.go | Clarifies semantics of ExitCodeScriptTimeout = -1 in comments. |
| orbit/pkg/installer/installer.go | Uses shebang-based temp script extensions; folds execve errors into output when exit -1 and output is empty. |
| orbit/pkg/installer/installer_test.go | Adds tests for script extension selection and execve error surfacing. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx | Ensures edit flow always shows a single consistent “Couldn’t edit software.” prefix. |
| frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tests.tsx | Adds unit tests for edit modal error-message normalization. |
| frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/helpers.tsx | Ensures tarball “learn more” message includes add-flow verb prefix. |
| frontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/helpers.tests.tsx | Updates/adds tests for new add-flow error behavior. |
| ee/server/service/software_installers.go | Makes shared validation messages action-neutral; improves unsupported-format message; fixes platform rejection wording. |
| ee/server/service/software_installers_test.go | Updates tests to match “macOS and Linux hosts” and .py setup-experience platforms. |
| cmd/fleetctl/integrationtest/gitops/software_test.go | Updates GitOps integration tests for the new unsupported-format error message. |
Files excluded by content exclusion policy (1)
- changes/41470-python-script-only-followups
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #50143 +/- ##
==========================================
+ Coverage 68.08% 68.09% +0.01%
==========================================
Files 3936 3936
Lines 250647 250705 +58
Branches 13275 13279 +4
==========================================
+ Hits 170642 170708 +66
+ Misses 64693 64686 -7
+ Partials 15312 15311 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughUpdated installer validation messages to report content mismatches and correct add/edit actions. Added Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/server/service/software_installers.go (1)
2551-2558: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse an extension-specific message for shell/shebang mismatches.
At Line [2556], a
.shfile with a valid Python shebang reaches this branch, butErrUnsupportedInterpreterexplicitly lists Python as supported. The error therefore says the interpreter is unsupported when the actual problem is that a shell package uses the wrong interpreter. Return a shell-specific validation message here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ee/server/service/software_installers.go` around lines 2551 - 2558, Update the `.sh` validation branch in the extension switch to return a shell-specific message when `directExecute` is true and `kind != fleet.ShebangShell`, rather than using `fleet.ErrUnsupportedInterpreter`. Keep the existing `InternalErr` context and validation conditions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ee/server/service/software_installers.go`:
- Around line 2551-2558: Update the `.sh` validation branch in the extension
switch to return a shell-specific message when `directExecute` is true and `kind
!= fleet.ShebangShell`, rather than using `fleet.ErrUnsupportedInterpreter`.
Keep the existing `InternalErr` context and validation conditions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 31bef814-4542-4f0a-8858-330fbebd8b16
📒 Files selected for processing (15)
changes/41470-python-script-only-followupscmd/fleetctl/integrationtest/gitops/software_test.goee/server/service/software_installers.goee/server/service/software_installers_test.gofrontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/helpers.tests.tsxfrontend/pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/helpers.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tests.tsxfrontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsxorbit/pkg/installer/installer.goorbit/pkg/installer/installer_test.goserver/fleet/scripts.goserver/fleet/software_installer.goserver/fleet/software_installer_test.goserver/fleet/software_test.goserver/service/integration_enterprise_test.go
There was a problem hiding this comment.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.
Files excluded by content exclusion policy (1)
- changes/41470-python-script-only-followups
RachelElysia
left a comment
RachelElysia
left a comment
There was a problem hiding this comment.
thanks for the tests too!
…rm (#50254) **Related issue:** Resolves #50106 Cherry-picks only the #50106 slice from #50143 (merged to `main`) into the 4.90 RC. `.py` script-only packages now accept `setup_experience_platform` (`darwin`/`linux`), matching `.sh` — previously `AllowedSetupExperiencePlatformsForExtension` returned `nil` for `.py`, so every value was rejected with an empty allowlist (`allowed: `). This is an `~unreleased` bug (first ships in 4.90.0); the other five fixes from #50143 remain on `main`. # Checklist for submitter ## Testing - [x] Added/updated automated tests For unreleased bug fixes in a release candidate, one of: - [x] Confirmed that the fix is not expected to adversely impact load test results
Related issues: Resolves #50068, Resolves #50106, Resolves #50107, Resolves #50108, Resolves #50110, Resolves #50114
Follow-on fixes from QA of #41470 (Python script-only packages):
.pypackages acceptsetup_experience_platform(darwin/linux), matching.sh(.pypackages cannot be assignedsetup_experience_platform—AllowedSetupExperiencePlatformsForExtensionreturns nil for.py#50106).-1) now renders a diagnostic instead of empty output, and orbit surfaces the underlying execve error (Install failure with exit code -1 reports no output, leaving no diagnostic for the operator #50108)..sh/.pypackages says "macOS and Linux hosts" instead of "linux" (Install rejection message says script packages are "linux only" when they also run on macOS #50110)..py/.sh/.ps1), so tracebacks reference the right file type (.pypackage install script is written to the host with a.shextension, causing confusing tracebacks #50114).Checklist for submitter
changes/.Testing
fleetd/orbit/Fleet Desktop
runtime.GOOSis used as needed to isolate changes.SoftwareInstallDetailscontract is unchanged).Summary by CodeRabbit
.sh/.pypackages display macOS+Linux support.