Open
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds a new transform feature that lets users run shell commands after files are copied, including CLI support, validation, and tests.
- Introduce
TransformConfigand integrate it intoConfigFileandMixtapeMatrix.run(). - Validate and block dangerous shell commands, update default config template.
- Add tests and YAML fixtures for valid and dangerous transform scenarios.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/test_main.py | Add tests for valid and dangerous transform commands |
| test/matrix.yaml | Include a simple transform commands example |
| test/dangerous_matrix.yaml | Add a dangerous command example for validation |
| mixtapematrix/routers/files.py | Update path validator to allow default example paths |
| mixtapematrix/main.py | Run transform.commands via subprocess.run |
| mixtapematrix/config.py | Define TransformConfig and integrate into ConfigFile |
Comments suppressed due to low confidence (5)
mixtapematrix/routers/files.py:29
- The validator only returns
pathwhen it starts with/example/; after the existence check you need a finalreturn pathfor all other valid cases.
if not os.path.exists(path):
mixtapematrix/config.py:16
- [nitpick] Pydantic does not use attribute docstrings for schema metadata; consider using
Field(..., description="…")so descriptions appear in generated schemas.
"""Source path is the directory to copy files from."""
test/test_main.py:38
- Consider adding a test for when the
transformsection is omitted to ensure no commands run and no errors occur.
def test_cli():
mixtapematrix/routers/files.py:23
- Add
from pydantic import field_validatorat the top of this file; otherwise the decorator will cause an ImportError.
@field_validator("path", mode="after")
test/test_main.py:25
- [nitpick] The body of
test_valid_transformis indented with 8 spaces instead of the project’s 4-space style; adjust for consistency.
matrix = MixtapeMatrix("test/matrix.yaml")
Comment on lines
+30
to
+33
| for command in self.config_data.transform.commands: | ||
| self.debug(f"Running command: {command}") | ||
| subprocess.run(command, shell=True, check=True) | ||
| self.debug(f"Command executed: {command}") |
There was a problem hiding this comment.
If transform is None, this will raise an AttributeError; guard this loop with if self.config_data.transform:.
Suggested change
| for command in self.config_data.transform.commands: | |
| self.debug(f"Running command: {command}") | |
| subprocess.run(command, shell=True, check=True) | |
| self.debug(f"Command executed: {command}") | |
| if self.config_data.transform: | |
| for command in self.config_data.transform.commands: | |
| self.debug(f"Running command: {command}") | |
| subprocess.run(command, shell=True, check=True) | |
| self.debug(f"Command executed: {command}") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.