feat: Add artifact execution framework with support for Node.js, Python, and Web artifacts #530
Conversation
- Add ArtifactExecutor for running Node.js applications from .unit files - Support npm install and node index.js execution with real-time output - Add NodeJsExecutionView UI component for terminal-like output display - Update ArtifactBundle to handle NODEJS artifact type - Extend GenerateTestUnit to create Express.js demo applications - Add execution button to ArtifactPreviewPanel for Node.js artifacts
…n, and Web artifacts
…rt/stop controls - Add ProcessManager to manage long-running processes like Express.js servers - Update Node.js execution to support server processes with start/stop UI controls - Add auto-dependency detection for Node.js artifacts from require/import statements - Improve artifact selection logic to avoid package.json as main content - Add server URL display and stop button in NodeJsExecutionView - Extend ArtifactPreviewPanel with onFixRequest callback for auto-f
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis PR adds Node.js artifact support to AutoDev Unit, enabling generation and execution of Node.js/React applications. It introduces artifact fixing capabilities, process management for long-running servers, and platform-specific execution handlers for Node.js, Python, and web artifacts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as UI/ViewModel
participant Agent as ArtifactAgent
participant Executor as ArtifactExecutor<br/>(Factory)
participant NodeExec as NodeJsArtifact<br/>Executor
participant ProcMgr as ProcessManager
participant NodeRuntime as Node.js<br/>Runtime
User->>UI: Click Execute for Node.js artifact
activate UI
UI->>Executor: executeArtifact(unitFilePath)
activate Executor
Executor->>Executor: Extract .unit bundle
Executor->>Executor: Read ARTIFACT.md, determine type
Executor->>Executor: Get executor for NODEJS type
rect rgb(200, 220, 250)
Note over Executor,NodeExec: Execution Phase
Executor->>NodeExec: execute(extractDir, onOutput)
activate NodeExec
NodeExec->>NodeExec: Validate package.json exists
NodeExec->>NodeExec: Detect main file (index.js)
NodeExec->>ProcMgr: startProcess(npm install)
activate ProcMgr
ProcMgr->>NodeRuntime: npm install
NodeRuntime-->>ProcMgr: Dependencies installed
deactivate ProcMgr
NodeExec->>ProcMgr: startProcess(node index.js)
activate ProcMgr
ProcMgr->>NodeRuntime: Launch Node.js process
NodeRuntime-->>ProcMgr: Process running, serverUrl detected
ProcMgr-->>NodeExec: (processId, output)
deactivate ProcMgr
NodeExec-->>Executor: ExecutionResult.Success(output, serverUrl, processId)
deactivate NodeExec
end
Executor-->>UI: ExecutionResult
deactivate Executor
UI->>UI: Update state (serverUrl, processId, running)
UI->>User: Display execution output & server link
deactivate UI
opt On Execution Error
User->>UI: Click "Fix with AI"
activate UI
UI->>Agent: fix(originalArtifact, errorMessage)
activate Agent
Agent->>Agent: buildFixPrompt(artifact, error)
Agent->>Agent: generate() with fix context
Agent-->>UI: ArtifactResult with fixed code
deactivate Agent
UI->>UI: selectBestArtifact(result.artifacts)
UI->>User: Show fixed artifact for retry
deactivate UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Rationale: The PR introduces substantial, interconnected changes across multiple domains—artifact execution with platform-specific executors, long-running process management with concurrent tracking, LLM-driven artifact repair workflows, and cross-platform UI updates (6+ platform variants). The heterogeneous nature (new public APIs, process lifecycle logic, streaming optimizations, menu integration) and complexity of executor implementations (recovery logic, dependency parsing, server detection) require careful review despite coherent architectural patterns. Possibly related PRs
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (26)
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 a cross-platform artifact “.unit” execution framework so AutoDev can package and run interactive outputs (Node.js, Python, and Web). Changes:
Technical Notes: Uses Kotlin Multiplatform expect/actual for preview panels; introduces streaming throttling in Sketch rendering to reduce recompositions during LLM streaming. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| .start() | ||
|
|
||
| // Read output in background | ||
| coroutineScope { |
There was a problem hiding this comment.
coroutineScope { launch { … } } waits for the launched reader to complete, so startHttpServer() will suspend until the server process exits and execute() will effectively hang. Consider launching output reading in a scope that doesn’t block returning the Process.
🤖 Was this useful? React with 👍 or 👎
| // Verify package.json content | ||
| val packageEntry = entries.find { it.name == "package.json" }!! | ||
| val packageContent = zip.getInputStream(packageEntry).bufferedReader().readText() | ||
| assertTrue(packageContent.contains("\"type\": \"module\""), "package.json should have module type") |
There was a problem hiding this comment.
This test asserts "type": "module" in package.json, but ArtifactBundle.generatePackageJson() explicitly avoids setting type: module for NODEJS, so this will fail (also applies to the similar assertion later in this file).
🤖 Was this useful? React with 👍 or 👎
| // Match require('package') or require("package") | ||
| val requirePattern = Regex("""require\s*\(\s*['"]([^'"./][^'"]*)['"]\s*\)""") | ||
| requirePattern.findAll(content).forEach { match -> | ||
| val packageName = match.groupValues[1].split("/").first() |
There was a problem hiding this comment.
Dependency detection uses split("/").first(), which will mis-handle scoped packages like @scope/pkg by turning them into @scope (invalid). This seems likely to generate incorrect package.json dependencies for many modern npm packages.
🤖 Was this useful? React with 👍 or 👎
| .redirectErrorStream(true) | ||
|
|
||
| val process = processBuilder.start() | ||
| val outputBuilder = StringBuilder() |
There was a problem hiding this comment.
outputBuilder is appended from outputJob while also being read to produce the returned string, but StringBuilder isn’t thread-safe—this is a data race on Dispatchers.IO (Guideline: thread_safety).
🤖 Was this useful? React with 👍 or 👎
| logger.info("ArtifactExecutorFactory") { "🚀 Executing artifact from: $unitFilePath" } | ||
|
|
||
| // Step 1: Extract .unit file | ||
| val tempDir = Files.createTempDirectory("autodev-artifact-${UUID.randomUUID()}") |
There was a problem hiding this comment.
executeArtifact() creates a temp extraction directory but never cleans it up, so repeated executions can leak disk space unless something else removes it later (Guideline: no_memory_leaks).
🤖 Was this useful? React with 👍 or 👎
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive artifact execution framework that enables AutoDev to create, package, and execute interactive artifacts across multiple platforms (Node.js, Python, Web). The implementation includes sophisticated features like dependency detection, process management, and auto-fix capabilities.
Key Changes
- Adds artifact executors for Node.js, Python, and Web artifacts with platform-specific execution logic
- Implements automatic dependency detection from require/import statements for Node.js artifacts
- Introduces ProcessManager for managing long-running processes (e.g., Express.js servers)
- Adds AI-powered auto-fix functionality for failed artifact executions
- Implements streaming optimization with throttled content updates to reduce UI flickering
- Registers
.unitfile type with proper MIME type associations
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| ArtifactExecutor.kt | Base interface defining contract for artifact executors with validation and execution methods |
| NodeJsArtifactExecutor.kt | Executes Node.js artifacts with npm dependency management and server process handling |
| PythonArtifactExecutor.kt | Executes Python artifacts with PEP 723 metadata parsing and pip dependency installation |
| WebArtifactExecutor.kt | Executes web artifacts by starting local HTTP servers with Python or Node.js |
| ProcessManager.kt | Manages long-running processes with start/stop controls and output streaming |
| ArtifactExecutorFactory.kt | Factory pattern implementation for selecting appropriate executors based on artifact type |
| ArtifactBundle.kt | Enhanced with auto-dependency detection, selectBestArtifact logic, and Node.js support |
| ArtifactBundlePacker.kt | Updated to include index.js mapping for Node.js artifacts |
| ArtifactAgent.kt | Added fix() method for AI-powered error correction of failed artifacts |
| ArtifactAgentTemplate.kt | Extended with Node.js guidelines and templates for artifact generation |
| ArtifactPreviewPanel.jvm.kt | Desktop implementation with Node.js execution UI, play/stop controls, and error handling |
| ArtifactPreviewPanel (other platforms) | Updated signatures to support onFixRequest callback across all platforms |
| ArtifactPage.kt | Integrated fix request functionality into artifact workflow |
| ArtifactAgentViewModel.kt | Added fixArtifact method with streaming support and best artifact selection |
| SketchRenderer.kt | Implemented throttled content rendering to reduce recomposition frequency during streaming |
| NodeJsArtifactBundleTest.kt | Comprehensive tests for Node.js bundle creation, roundtrip, and validation |
| ArtifactExecutorTest.kt | Tests for artifact execution, extraction, and dependency detection |
| GenerateTestUnit.kt | Extended utility to generate test Express.js applications |
| build.gradle.kts | Changed MIME type from custom to application/zip for better OS integration |
| Main.kt | Added openUnitBundle functionality with file chooser integration |
| AutoDevMenuBar.kt | Added "Open Unit Bundle" menu item |
| Keymap.kt | Added keyboard shortcut (Cmd/Ctrl+Shift+U) for opening unit bundles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val tempUnitFile = File.createTempFile("artifact-${artifact.identifier}", ".unit") | ||
| // Note: Don't deleteOnExit for server processes - they need the files |
There was a problem hiding this comment.
The temp file created with File.createTempFile is not cleaned up. Since the comment explicitly says "Don't deleteOnExit for server processes - they need the files", this could lead to disk space issues over time as temp files accumulate. Consider implementing a cleanup strategy for stopped processes or exposing cleanup options to the user.
| val importPattern = Regex("""import\s+.*?\s+from\s+['"]([^'"./][^'"]*)['"]\s*;?""") | ||
| importPattern.findAll(content).forEach { match -> | ||
| val packageName = match.groupValues[1].split("/").first() | ||
| dependencies.add(packageName) | ||
| } |
There was a problem hiding this comment.
The regular expression pattern for detecting imports doesn't handle all valid ES6 import syntaxes. For example, it won't match "import * as express from 'express'" or "import {createServer} from 'http'". Consider using a more comprehensive pattern or multiple patterns to cover all import variations.
| val expressAppCode = """ | ||
| import express from 'express'; | ||
| const app = express(); | ||
| const PORT = process.env.PORT || 3000; | ||
| app.use(express.json()); | ||
| app.get('/', (req, res) => { | ||
| res.json({ | ||
| message: 'Hello from Express.js!', | ||
| timestamp: new Date().toISOString(), | ||
| version: '1.0.0' | ||
| }); | ||
| }); | ||
| app.get('/api/health', (req, res) => { | ||
| res.json({ status: 'ok', uptime: process.uptime() }); | ||
| }); | ||
| app.post('/api/echo', (req, res) => { | ||
| res.json({ | ||
| received: req.body, | ||
| timestamp: new Date().toISOString() | ||
| }); | ||
| }); | ||
| app.listen(PORT, () => { | ||
| console.log(`🚀 Express.js server running on http://localhost:${'$'}{PORT}`); | ||
| console.log(`📡 Health check: http://localhost:${'$'}{PORT}/api/health`); | ||
| console.log(`📝 Echo endpoint: POST http://localhost:${'$'}{PORT}/api/echo`); | ||
| }); | ||
| """.trimIndent() |
There was a problem hiding this comment.
The Express.js demo code uses ES6 import syntax which requires "type": "module" in package.json or .mjs file extension to work in Node.js. Without this configuration, the code will fail with "SyntaxError: Cannot use import statement outside a module". Either add "type": "module" to the bundle generation or use CommonJS require() syntax.
| val codeArtifact = nodeJsArtifacts.find { artifact -> | ||
| val content = artifact.content.trim() | ||
| // Skip if it's clearly JSON (package.json) | ||
| !(content.startsWith("{") && content.contains("\"name\"") && content.contains("\"dependencies\"")) |
There was a problem hiding this comment.
The detection of package.json content is too simplistic. The pattern checks if content starts with "{" and contains "name", which could incorrectly match legitimate JavaScript objects or JSON configurations. Consider using a more robust check, such as attempting to parse as JSON and checking for specific package.json fields like "dependencies" or "version" in addition to "name".
| private suspend fun openUnitBundleFile(uiState: DesktopUiState) { | ||
| try { | ||
| val fileChooser = createFileChooser() | ||
| val selectedPath = fileChooser.chooseFile( | ||
| title = "Open Unit Bundle", | ||
| fileExtensions = listOf("unit") | ||
| ) | ||
|
|
||
| selectedPath?.let { path -> | ||
| AutoDevLogger.info("AutoDevMain") { "📦 Opening Unit Bundle: $path" } | ||
|
|
||
| // Switch to Artifact mode | ||
| uiState.updateAgentType(AgentType.ARTIFACT) | ||
|
|
||
| // Load the bundle | ||
| val success = withContext(Dispatchers.IO) { |
There was a problem hiding this comment.
The withContext(Dispatchers.IO) block calls File operations but doesn't handle the case where the context file doesn't exist or is malformed. The null return at the end silently swallows all exceptions. Consider logging warnings for specific failure cases (file not found, parse error, etc.) to aid debugging.
| content = """ | ||
| import express from 'express'; | ||
| const app = express(); | ||
| app.get('/', (req, res) => res.json({ message: 'Hello' })); | ||
| app.listen(3000, () => console.log('Server running')); | ||
| """.trimIndent() | ||
| ) | ||
|
|
||
| val bundle = ArtifactBundle.fromArtifact( | ||
| artifact = artifact, | ||
| conversationHistory = emptyList(), | ||
| modelInfo = null | ||
| ).copy( | ||
| dependencies = mapOf("express" to "^4.18.2") | ||
| ) | ||
|
|
||
| val outputFile = File(tempDir, "express-app.unit") | ||
| val packer = ArtifactBundlePacker() | ||
| val result = packer.pack(bundle, outputFile.absolutePath) | ||
|
|
||
| assertTrue(result is PackResult.Success, "Pack should succeed") | ||
|
|
||
| // Verify ZIP contents | ||
| ZipFile(outputFile).use { zip -> | ||
| val entries = zip.entries().toList() | ||
| val entryNames = entries.map { it.name } | ||
|
|
||
| // Should contain all required files | ||
| assertTrue(entryNames.contains("index.js"), "Should contain index.js") | ||
| assertTrue(entryNames.contains("package.json"), "Should contain package.json") | ||
| assertTrue(entryNames.contains("ARTIFACT.md"), "Should contain ARTIFACT.md") | ||
| assertTrue(entryNames.contains(".artifact/context.json"), "Should contain context.json") | ||
|
|
||
| // Verify index.js content | ||
| val indexEntry = entries.find { it.name == "index.js" }!! | ||
| val indexContent = zip.getInputStream(indexEntry).bufferedReader().readText() | ||
| assertTrue(indexContent.contains("express"), "index.js should contain express import") | ||
| assertTrue(indexContent.contains("app.listen"), "index.js should contain app.listen") | ||
|
|
||
| // Verify package.json content | ||
| val packageEntry = entries.find { it.name == "package.json" }!! | ||
| val packageContent = zip.getInputStream(packageEntry).bufferedReader().readText() | ||
| assertTrue(packageContent.contains("\"type\": \"module\""), "package.json should have module type") | ||
| assertTrue(packageContent.contains("\"express\""), "package.json should contain express dependency") | ||
| assertTrue(packageContent.contains("\"start\": \"node index.js\""), "package.json should have start script") |
There was a problem hiding this comment.
The test uses ES6 import syntax but the package.json doesn't include "type": "module", which will cause the Node.js code to fail at runtime. Either add "type": "module" to the generated package.json or use CommonJS require() syntax in the test.
| val outputJob = CoroutineScope(Dispatchers.IO).launch { | ||
| try { | ||
| process.inputStream.bufferedReader().use { reader -> | ||
| reader.lineSequence().forEach { line -> | ||
| outputBuilder.appendLine(line) | ||
| onOutput?.invoke("$line\n") | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| if (e !is CancellationException) { | ||
| logger.warn("ProcessManager") { "Output reading error: ${e.message}" } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The process output job is launched in a new CoroutineScope without lifecycle management. If the parent coroutine is cancelled, this job will continue running and potentially leak resources. Consider using the parent scope or passing a scope parameter to ensure proper cancellation propagation.
| */ | ||
| private fun findAvailablePort(startPort: Int = 8000): Int { | ||
| for (port in startPort..9000) { | ||
| try { | ||
| ServerSocket(port).use { socket -> | ||
| return socket.localPort | ||
| } | ||
| } catch (e: Exception) { | ||
| // Port is in use, try next |
There was a problem hiding this comment.
The ServerSocket is used only to find an available port but is closed immediately after creation. This creates a race condition where another process could grab the port before the HTTP server starts. Consider keeping the socket open and passing it to the HTTP server, or use a retry mechanism if port binding fails.
| */ | |
| private fun findAvailablePort(startPort: Int = 8000): Int { | |
| for (port in startPort..9000) { | |
| try { | |
| ServerSocket(port).use { socket -> | |
| return socket.localPort | |
| } | |
| } catch (e: Exception) { | |
| // Port is in use, try next | |
| * | |
| * Uses an ephemeral port (0) and verifies that it falls within the desired range. | |
| */ | |
| private fun findAvailablePort(startPort: Int = 8000): Int { | |
| ServerSocket(0).use { socket -> | |
| val port = socket.localPort | |
| if (port in startPort..9000) { | |
| return port |
| return dependencies | ||
| .filter { it !in builtInModules && !it.startsWith("node:") } | ||
| .associateWith { packageVersions[it] ?: "latest" } |
There was a problem hiding this comment.
The dependency detection excludes packages starting with "node:" but doesn't filter them from the builtInModules set before checking. This means "node:fs" would be excluded but "fs" might not be if it somehow wasn't in the builtInModules set. Consider normalizing the package name by removing the "node:" prefix before filtering against builtInModules.
| files.filterKeys { key -> key !in coreFileNames }.forEach { (key, value) -> | ||
| put(key, value) | ||
| } |
There was a problem hiding this comment.
The code filters core file names to prevent conflicts, but uses a simple set check. If additional files contain paths with subdirectories (e.g., ".artifact/context.json"), the filtering won't work correctly since CONTEXT_JSON is defined as ".artifact/context.json" but files map keys might not match exactly. Consider normalizing paths before comparison.
| files.filterKeys { key -> key !in coreFileNames }.forEach { (key, value) -> | |
| put(key, value) | |
| } | |
| // Normalize paths to improve conflict detection (e.g., "subdir/.artifact/context.json") | |
| fun String.normalizePathForComparison(): String = | |
| this.replace('\\', '/').removePrefix("./") | |
| fun conflictsWithCore(key: String): Boolean { | |
| val normalizedKey = key.normalizePathForComparison() | |
| return coreFileNames.any { core -> | |
| val normalizedCore = core.normalizePathForComparison() | |
| normalizedKey == normalizedCore || normalizedKey.endsWith("/$normalizedCore") | |
| } | |
| } | |
| files.forEach { (key, value) -> | |
| if (!conflictsWithCore(key)) { | |
| put(key, value) | |
| } | |
| } |
Overview
This PR introduces a comprehensive artifact execution framework that enables AutoDev to create, package, and execute interactive artifacts across multiple platforms.
Key Features
1. Artifact Bundle System
.unitfiles (ZIP-based)2. Artifact Executors
3. UI Components
4. File Association
.unitfile type registration with ZIP MIME type.unitfilesTechnical Improvements
expect/actualTesting
ArtifactBundlePackerRelated Issues
Fixes #526
Checklist
Pull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.