feat(artifact): implement Phase 2 Python Script Artifact (#526)#547
feat(artifact): implement Phase 2 Python Script Artifact (#526)#547
Conversation
- Add PEP723Parser (commonMain): cross-platform PEP 723 inline script metadata parser/generator with parse, generate, injectMetadata, and stripMetadata operations. - Add PythonArtifactAgent (commonMain): SubAgent that uses LLM to generate PEP 723-compliant Python scripts. - Add PythonArtifactPackager (jvmMain): packages Python scripts with dependency resolution via uv (preferred) or pip fallback, and embeds AutoDev Unit context in the PEP 723 header. - Refactor PythonArtifactExecutor: replace 40-line local PEP 723 parsing logic with delegation to shared PEP723Parser. - Add PEP723ParserTest: 10 unit tests covering parse, generate, inject, and strip operations.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds centralized PEP 723 metadata parsing/generation, an LLM-driven Python artifact sub-agent that produces scripts with embedded PEP 723 blocks, and a JVM packager that enriches scripts, resolves dependencies (UV then pip), and writes artifacts. Executor now delegates dependency parsing to the new parser. Changes
Sequence DiagramsequenceDiagram
participant User
participant Agent as PythonArtifactAgent
participant LLM as LLMService
participant Parser as PEP723Parser
participant Packager as PythonArtifactPackager
participant Resolver as UV/PIP Resolver
User->>Agent: submit prompt + desired deps
Agent->>LLM: SYSTEM_PROMPT + user prompt
LLM-->>Agent: streamed response with code block
Agent->>Agent: extractPythonCode()
Agent->>Parser: injectMetadata(script, deps, requiresPython, autodevContext)
Parser-->>Agent: enriched script (with PEP 723 block)
Agent-->>User: ToolResult with enriched script
User->>Packager: packageScript(enriched script, context, outputDir)
Packager->>Parser: parse(script)
Parser-->>Packager: PEP723Metadata
Packager->>Parser: generate(updated deps, requiresPython, autodevContext)
Parser-->>Packager: PEP 723 block
Packager->>Packager: write index.py, requirements.txt
Packager->>Resolver: try UV install
Resolver-->>Packager: UV success / UV fail -> pip
Packager-->>User: PackageResult.Success / Error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
🤖 Augment PR SummarySummary: Adds Phase 2 support for generating and packaging Python “script artifacts” using PEP 723 inline metadata. Changes:
Technical Notes: Dependency resolution is driven by PEP 723 metadata, enabling “single-file” scripts to be executed in a reproducible way while carrying AutoDev context for load-back/reversibility. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| ): CommandResult = withContext(Dispatchers.IO) { | ||
| try { | ||
| val processBuilder = ProcessBuilder() | ||
| .command("sh", "-c", command) |
There was a problem hiding this comment.
Using ProcessBuilder("sh", "-c", command) can enable shell injection if command incorporates untrusted values (e.g., dependency specs parsed from the script/LLM output). Consider executing via an argument list (no shell) or strictly validating/escaping any interpolated values before invocation.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| private suspend fun isCommandAvailable(cmd: String): Boolean = withContext(Dispatchers.IO) { | ||
| try { | ||
| val process = ProcessBuilder("which", cmd) |
There was a problem hiding this comment.
ProcessBuilder("which", cmd) (and later hardcoding sh) makes this packager Unix-specific; on Windows it will fail to detect/run uv/pip. If Windows is a supported IDE target, consider routing command execution through existing cross-platform shell helpers in the codebase.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| // Validate PEP 723 metadata is present; inject if missing | ||
| val meta = PEP723Parser.parse(scriptContent) | ||
| val finalScript = if (meta.rawBlock == null && input.dependencies.isNotEmpty()) { |
There was a problem hiding this comment.
This only injects PEP 723 metadata when input.dependencies is non-empty, so an empty-deps script could be returned without any header if the LLM omits it (despite the system rule that every script must start with metadata). Consider injecting a minimal block (at least requires-python) even when deps are empty, or aligning the rule/behavior.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| // ---- Parsing ---- | ||
|
|
||
| private val PEP723_BLOCK_PATTERN = Regex( | ||
| """#\s*///\s*script\s*\n(.*?)#\s*///""", |
There was a problem hiding this comment.
These regexes hardcode \n, so scripts with CRLF (\r\n) may fail to match/strip/inject the PEP 723 block (and related sections). Consider allowing optional \r in newline parts to be robust across platforms.
Severity: medium
Other Locations
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/artifact/PEP723Parser.kt:56
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Pull request overview
Implements Phase 2 “Python Script Artifact” support by introducing a shared PEP 723 metadata parser/generator, a Python artifact generation sub-agent, and a JVM-side packaging flow, plus refactoring the Python executor to reuse the shared parser.
Changes:
- Added
PEP723Parser(common) with parse/generate/inject/strip helpers for PEP 723 inline metadata. - Added
PythonArtifactAgent(common) to generate Python scripts with inline metadata via LLM output extraction. - Added
PythonArtifactPackager(JVM) to enrich scripts with AutoDev context and install dependencies via uv/pip; refactoredPythonArtifactExecutorto use the shared parser; added unit tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/artifact/PEP723Parser.kt | New shared PEP 723 parsing/generation utilities used by agent/executor/packager. |
| mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.kt | New LLM-powered sub-agent to produce Python artifact scripts and metadata. |
| mpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/PythonArtifactPackager.kt | New JVM packager to inject context and resolve dependencies with uv/pip. |
| mpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/executor/PythonArtifactExecutor.kt | Refactor to delegate dependency parsing to PEP723Parser. |
| mpp-core/src/jvmTest/kotlin/cc/unitmesh/agent/artifact/PEP723ParserTest.kt | New unit tests for PEP 723 parsing/generation/injection/stripping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private val PEP723_BLOCK_PATTERN = Regex( | ||
| """#\s*///\s*script\s*\n(.*?)#\s*///""", | ||
| RegexOption.DOT_MATCHES_ALL | ||
| ) |
There was a problem hiding this comment.
PEP723_BLOCK_PATTERN hard-codes \n line endings, so scripts with CRLF (\r\n) may fail to match and metadata won't be parsed/replaced/stripped correctly. Updating the regex to tolerate \r?\n (and ideally using multiline anchors) will make parsing more robust across platforms/editors.
| if (isCommandAvailable("uv")) { | ||
| onOutput?.invoke("📦 Installing dependencies via uv...\n") | ||
| val result = executeCommand("uv pip install ${dependencies.joinToString(" ")}", workDir.absolutePath, onOutput) | ||
| if (result.exitCode == 0) { |
There was a problem hiding this comment.
This builds a shell command by interpolating dependencies directly into a string that is executed via sh -c. Because dependency values come from script metadata, a crafted dependency like "requests"; rm -rf ... could lead to command injection. Avoid passing untrusted content through the shell: execute uv/pip with argument lists (ProcessBuilder args), or install via -r requirements.txt (which you already write) instead of inlining dependencies into the command string.
| * 1. **UV (preferred)** – uses `uv run` for zero-config execution with PEP 723 metadata. | ||
| * 2. **pip fallback** – generates a `requirements.txt` and calls `pip install`. |
There was a problem hiding this comment.
KDoc says the UV strategy uses uv run with PEP 723 metadata, but the implementation actually runs uv pip install ... (and never calls uv run). Either update the documentation to match what this class does, or adjust the implementation to use the documented UV workflow so future readers/operators aren’t misled.
| * 1. **UV (preferred)** – uses `uv run` for zero-config execution with PEP 723 metadata. | |
| * 2. **pip fallback** – generates a `requirements.txt` and calls `pip install`. | |
| * 1. **UV (preferred)** – installs dependencies via `uv pip install` using PEP 723 metadata | |
| * (or a generated requirements file) to resolve the environment. | |
| * 2. **pip fallback** – generates a `requirements.txt` and calls `pip install` directly. |
|
|
||
| // Try fenced python code block | ||
| val fencedPattern = Regex( | ||
| """```python\s*\n(.*?)```""", |
There was a problem hiding this comment.
extractPythonCode() only matches fenced code blocks that use a \n newline after ```python. Responses that use CRLF (\r\n) or put code immediately after the language tag can fail extraction. Consider making the regex more flexible (e.g., allow `\r?\n` and optional whitespace) so code extraction is reliable across platforms/LLM formatting.
| """```python\s*\n(.*?)```""", | |
| // Matches ```python, optional trailing text, optional CRLF/newline, then captures everything up to the next ``` or end of string | |
| """```python[^\n\r]*\r?\n?(.*?)(```|$)""", |
| val deps = PEP723Parser.parseDependencies(script) | ||
| assertTrue(deps.isEmpty()) | ||
| } | ||
|
|
There was a problem hiding this comment.
Current tests cover the happy path, but there are no tests for edge cases implied by the implementation changes: (1) parseDependencies() should not pick up a dependencies = [...] list that appears in normal Python code outside the PEP 723 header, and (2) CRLF line endings / single-quoted TOML strings should still be parsed if you intend full PEP 723 compliance. Adding targeted tests here would prevent regressions when tightening the parser.
| @Test | |
| fun `parseDependencies should ignore dependencies assignment in normal code`() { | |
| val script = """ | |
| import os | |
| dependencies = [ | |
| "should-not-be-picked", | |
| ] | |
| print("done") | |
| """.trimIndent() | |
| val deps = PEP723Parser.parseDependencies(script) | |
| assertTrue(deps.isEmpty()) | |
| } | |
| @Test | |
| fun `parseDependencies should handle CRLF and single-quoted strings`() { | |
| val script = | |
| "# /// script\r\n" + | |
| "# requires-python = '>=3.11'\r\n" + | |
| "# dependencies = [\r\n" + | |
| "# 'requests>=2.28.0',\r\n" + | |
| "# 'pandas>=2.0.0',\r\n" + | |
| "# ]\r\n" + | |
| "# ///\r\n" + | |
| "\r\n" + | |
| "print('ok')\r\n" | |
| val deps = PEP723Parser.parseDependencies(script) | |
| assertEquals(2, deps.size) | |
| assertEquals("requests>=2.28.0", deps[0]) | |
| assertEquals("pandas>=2.0.0", deps[1]) | |
| } |
| ) | ||
|
|
||
| private val DEPENDENCIES_PATTERN = Regex( | ||
| """dependencies\s*=\s*\[(.*?)\]""", |
There was a problem hiding this comment.
DEPENDENCIES_PATTERN uses \[(.*?)\] to capture the array contents, which will terminate at the first ] character. Valid dependency specifiers can contain ] (extras), e.g. requests[socks]>=2.0, which would cause truncated parsing. Consider parsing the dependencies section line-by-line until the closing # ] line (or use a minimal TOML array parser) instead of a raw bracket regex.
| """dependencies\s*=\s*\[(.*?)\]""", | |
| """dependencies\s*=\s*\[(.*)]""", |
| /** | ||
| * Extract only the dependency list from a Python script (convenience method). | ||
| */ | ||
| fun parseDependencies(pythonContent: String): List<String> { | ||
| val depsMatch = DEPENDENCIES_PATTERN.find(pythonContent) ?: return emptyList() | ||
| val depsContent = depsMatch.groupValues[1] | ||
|
|
||
| return DEP_ITEM_PATTERN.findAll(depsContent) | ||
| .map { it.groupValues[1] } | ||
| .toList() | ||
| } |
There was a problem hiding this comment.
parseDependencies() scans the entire input for dependencies = [...], so it can accidentally pick up a Python variable/string named dependencies outside the PEP 723 header. Since callers pass full script text (e.g., executor/agent), this can produce incorrect dependency installs. Consider first extracting the PEP 723 block with PEP723_BLOCK_PATTERN (or reusing parse()) and only searching within that block.
| private val REQUIRES_PYTHON_PATTERN = Regex( | ||
| """requires-python\s*=\s*"([^"]*)"""" | ||
| ) | ||
|
|
||
| private val DEPENDENCIES_PATTERN = Regex( | ||
| """dependencies\s*=\s*\[(.*?)\]""", | ||
| RegexOption.DOT_MATCHES_ALL | ||
| ) | ||
|
|
||
| private val DEP_ITEM_PATTERN = Regex("""["']([^"']+)["']""") | ||
|
|
||
| private val AUTODEV_SECTION_PATTERN = Regex( | ||
| """\[tool\.autodev-unit\]\s*\n(.*?)(?=#\s*///|\[tool\.|$)""", | ||
| RegexOption.DOT_MATCHES_ALL | ||
| ) | ||
|
|
||
| private val AUTODEV_KV_PATTERN = Regex( | ||
| """#\s*(\S+)\s*=\s*"([^"]*)"""" | ||
| ) |
There was a problem hiding this comment.
The parser currently only recognizes double-quoted TOML strings (e.g., requires-python = "...", # key = "..."). TOML also allows single-quoted strings, so valid PEP 723 metadata may be silently ignored. Consider extending REQUIRES_PYTHON_PATTERN / AUTODEV_KV_PATTERN to support both quote styles (or using a small TOML parser on the de-commented block).
| * Strip the PEP 723 metadata block from a Python script, returning only the code body. | ||
| */ | ||
| fun stripMetadata(pythonContent: String): String { | ||
| return PEP723_BLOCK_PATTERN.replace(pythonContent, "").trimStart('\n') |
There was a problem hiding this comment.
stripMetadata() only trims leading \n after removing the block; if the script uses CRLF the result can start with a stray \r. Consider trimming both \r and \n (or calling trimStart() without args if that's acceptable here).
| return PEP723_BLOCK_PATTERN.replace(pythonContent, "").trimStart('\n') | |
| return PEP723_BLOCK_PATTERN.replace(pythonContent, "").trimStart('\r', '\n') |
| // Validate PEP 723 metadata is present; inject if missing | ||
| val meta = PEP723Parser.parse(scriptContent) | ||
| val finalScript = if (meta.rawBlock == null && input.dependencies.isNotEmpty()) { | ||
| PEP723Parser.injectMetadata( | ||
| pythonContent = scriptContent, | ||
| dependencies = input.dependencies, | ||
| requiresPython = input.requiresPython | ||
| ) | ||
| } else { | ||
| scriptContent | ||
| } |
There was a problem hiding this comment.
The comment says “inject if missing”, but the code only injects a PEP 723 header when the input dependency list is non-empty. If the LLM omits the header and dependencies is empty, the resulting artifact can violate the agent’s own SYSTEM_PROMPT rule (“Every script MUST begin with an inline metadata block”). Consider injecting the header whenever meta.rawBlock == null (even with an empty dependency list) so requires-python is always present.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.kt (2)
109-117: Avoid double parsing of the final script.The script is parsed twice: once for dependencies (line 114) and once for
requiresPython(line 115). Consider reusing the parsed metadata.♻️ Proposed optimization
+ val finalMeta = PEP723Parser.parse(finalScript) + ToolResult.AgentResult( success = true, content = finalScript, metadata = mapOf( "type" to "python", - "dependencies" to PEP723Parser.parseDependencies(finalScript).joinToString(","), - "requiresPython" to (PEP723Parser.parse(finalScript).requiresPython ?: ">=3.11") + "dependencies" to finalMeta.dependencies.joinToString(","), + "requiresPython" to (finalMeta.requiresPython ?: ">=3.11") ) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.kt` around lines 109 - 117, In PythonArtifactAgent where you build the ToolResult.AgentResult from finalScript, avoid calling PEP723Parser.parse(finalScript) twice; instead call PEP723Parser.parse(finalScript) once into a local val (e.g., parsed = PEP723Parser.parse(finalScript)) and reuse parsed for both dependencies (or use PEP723Parser.parseDependencies(finalScript) once if needed) and for requiresPython (use parsed.requiresPython ?: ">=3.11") before constructing ToolResult.AgentResult so both metadata entries reuse the same parsed result.
3-5: Remove unused imports.
ArtifactContext,ConversationMessage, andModelInfoare imported but not used in this file.♻️ Proposed fix
package cc.unitmesh.agent.subagent -import cc.unitmesh.agent.artifact.ArtifactContext -import cc.unitmesh.agent.artifact.ConversationMessage -import cc.unitmesh.agent.artifact.ModelInfo import cc.unitmesh.agent.artifact.PEP723Parser🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.kt` around lines 3 - 5, Remove the unused imports ArtifactContext, ConversationMessage, and ModelInfo from the top of PythonArtifactAgent.kt; locate the import block that currently imports cc.unitmesh.agent.artifact.ArtifactContext, ConversationMessage, and ModelInfo and delete those three import entries so only actually used imports remain, then run a quick compile or linter to ensure no other references to ArtifactContext, ConversationMessage, or ModelInfo exist in the file.mpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/PythonArtifactPackager.kt (1)
155-187: Consider extracting shared command execution utilities.The
executeCommandandCommandResultimplementations are nearly identical to those inPythonArtifactExecutor. Consider extracting these into a shared utility class to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/PythonArtifactPackager.kt` around lines 155 - 187, The executeCommand function and CommandResult data class in PythonArtifactPackager duplicate logic already present in PythonArtifactExecutor; extract them into a shared utility (e.g., CommandUtils or ProcessRunner) and replace the local definitions by calling the shared function; specifically move the executeCommand implementation and the CommandResult type out of PythonArtifactPackager/PythonArtifactExecutor into the new shared utility, update both classes to use the shared executeCommand(...) and CommandResult, and ensure the onOutput callback and error handling behavior remain identical.mpp-core/src/jvmTest/kotlin/cc/unitmesh/agent/artifact/PEP723ParserTest.kt (1)
189-191: Consider usingassertFalsefor negated assertions.Using
assertFalse(result.contains("old-package"))is more idiomatic and provides clearer error messages thanassertTrue(!result.contains(...)).♻️ Suggested improvement
- // old package should be gone - assertTrue(!result.contains("old-package")) + // old package should be gone + assertFalse(result.contains("old-package"))Same applies to lines 211-212.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mpp-core/src/jvmTest/kotlin/cc/unitmesh/agent/artifact/PEP723ParserTest.kt` around lines 189 - 191, Replace the negated assertions that use assertTrue(!result.contains("old-package")) with the idiomatic assertFalse(result.contains("old-package")) to improve clarity and error messages; update the occurrence in PEP723ParserTest (the assertion referencing result.contains("old-package")) and the similar pair at the later occurrence referenced in lines 211-212 to use assertFalse instead of assertTrue(!...).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.kt`:
- Line 67: Remove emoji/UTF-8 characters from progress messages to satisfy WASM
compatibility: update the onProgress calls that currently use "🐍 Generating
Python script..." and the later call using "✅ ..." to plain ASCII text (e.g.,
"Generating Python script..." and "Generation complete" or similar). Locate the
onProgress invocations inside PythonArtifactAgent.kt (the calls to onProgress
that include emoji) and replace the message strings with equivalent emoji-free
messages, preserving punctuation and intent.
In
`@mpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/PythonArtifactPackager.kt`:
- Line 125: The command-injection risk comes from building a single shell string
with "uv pip install ${dependencies.joinToString(" ")}" inside
PythonArtifactPackager.kt and passing it to executeCommand; change to an
array/vararg-style invocation of executeCommand so each token is passed as a
separate argument (keep the "uv" and "pip" tokens and append dependencies as
individual args rather than interpolating a single shell string), or if array
execution is not available validate each entry in dependencies against a strict
whitelist regex (e.g., only allow letters, digits, dot, dash, underscore and
version operators) before joining; update the call site referencing
executeCommand and the dependencies.joinToString usage accordingly.
- Around line 144-152: The isCommandAvailable function currently calls the
Unix-only "which" binary; replace it with a cross-platform check: detect Windows
via System.getProperty("os.name") and either call "where" on Windows or better
yet implement a PATH scan by splitting System.getenv("PATH") with
File.pathSeparator and checking for an executable file matching the command (on
Windows also consider PATHEXT extensions) to determine availability; update the
isCommandAvailable suspend function to perform this filesystem-based check on
Dispatchers.IO instead of invoking "which".
---
Nitpick comments:
In
`@mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.kt`:
- Around line 109-117: In PythonArtifactAgent where you build the
ToolResult.AgentResult from finalScript, avoid calling
PEP723Parser.parse(finalScript) twice; instead call
PEP723Parser.parse(finalScript) once into a local val (e.g., parsed =
PEP723Parser.parse(finalScript)) and reuse parsed for both dependencies (or use
PEP723Parser.parseDependencies(finalScript) once if needed) and for
requiresPython (use parsed.requiresPython ?: ">=3.11") before constructing
ToolResult.AgentResult so both metadata entries reuse the same parsed result.
- Around line 3-5: Remove the unused imports ArtifactContext,
ConversationMessage, and ModelInfo from the top of PythonArtifactAgent.kt;
locate the import block that currently imports
cc.unitmesh.agent.artifact.ArtifactContext, ConversationMessage, and ModelInfo
and delete those three import entries so only actually used imports remain, then
run a quick compile or linter to ensure no other references to ArtifactContext,
ConversationMessage, or ModelInfo exist in the file.
In
`@mpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/PythonArtifactPackager.kt`:
- Around line 155-187: The executeCommand function and CommandResult data class
in PythonArtifactPackager duplicate logic already present in
PythonArtifactExecutor; extract them into a shared utility (e.g., CommandUtils
or ProcessRunner) and replace the local definitions by calling the shared
function; specifically move the executeCommand implementation and the
CommandResult type out of PythonArtifactPackager/PythonArtifactExecutor into the
new shared utility, update both classes to use the shared executeCommand(...)
and CommandResult, and ensure the onOutput callback and error handling behavior
remain identical.
In `@mpp-core/src/jvmTest/kotlin/cc/unitmesh/agent/artifact/PEP723ParserTest.kt`:
- Around line 189-191: Replace the negated assertions that use
assertTrue(!result.contains("old-package")) with the idiomatic
assertFalse(result.contains("old-package")) to improve clarity and error
messages; update the occurrence in PEP723ParserTest (the assertion referencing
result.contains("old-package")) and the similar pair at the later occurrence
referenced in lines 211-212 to use assertFalse instead of assertTrue(!...).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09102b6f-b29b-4471-8ea9-b5e9bdafff4b
📒 Files selected for processing (5)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/artifact/PEP723Parser.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/subagent/PythonArtifactAgent.ktmpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/PythonArtifactPackager.ktmpp-core/src/jvmMain/kotlin/cc/unitmesh/agent/artifact/executor/PythonArtifactExecutor.ktmpp-core/src/jvmTest/kotlin/cc/unitmesh/agent/artifact/PEP723ParserTest.kt
Summary
Implements Phase 2 of AutoDev Unit (Issue #526): Python Script Artifact support.
New Files
Modified
All tests passing.
Summary by CodeRabbit
New Features
Refactor
Tests