stack icon indicating copy to clipboard operation
stack copied to clipboard

faster mock freestyle

Open BilalG1 opened this issue 1 month ago • 3 comments

Summary by CodeRabbit

  • New Features

    • Added environment variable handling and logging interception for improved script execution visibility.
  • Bug Fixes

    • Enhanced error handling with structured error responses and logs.
  • Chores

    • Updated dependencies: arktype and react-dom.
    • Improved temporary work directory management and cleanup process.
    • Optimized script execution model for better performance.

✏️ Tip: You can customize this high-level summary in your review settings.

BilalG1 avatar Nov 26 '25 22:11 BilalG1

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
stack-backend Ready Ready Preview Comment Dec 1, 2025 5:21pm
stack-dashboard Ready Ready Preview Comment Dec 1, 2025 5:21pm
stack-demo Ready Ready Preview Comment Dec 1, 2025 5:21pm
stack-docs Ready Ready Preview Comment Dec 1, 2025 5:21pm

vercel[bot] avatar Nov 26 '25 22:11 vercel[bot]

Walkthrough

The job execution system was refactored to use dynamic import of user scripts instead of spawning separate processes. Dependencies were updated, work directory creation was reorganized, and new features for environment variable and logging interception with cleanup were added.

Changes

Cohort / File(s) Summary
Dependency updates
docker/dependencies/freestyle-mock/Dockerfile, package.json
Updated arktype from 2.1.2 to 2.1.20; added react-dom 19.1.1. Modified package.json structure to include type: module and base dependency handling on config.nodeModules.
Execution model refactor
server.ts
Removed runner.ts generation and execution; replaced with dynamic import of user script. Added preinstalledNodeModules map and baseWorkDir constant. Reorganized work directory creation to use base directory as root for job-specific directories.
Job execution logic
server.ts
Implemented environment variable application and restoration around script execution. Added logging interception via console method override to capture output while preserving original behavior. Introduced conditional Bun install logic based on module differences from preinstalled set.
Error handling and cleanup
server.ts
Enhanced error catching from user script execution with JSON error response and logs. Added cleanup procedure to restore environment variables, console methods, and remove working directory. Maintains response structure of { result, logs } or error JSON.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server as server.ts
    participant FS as File System
    participant Script as script.ts (User)

    rect rgb(200, 220, 255)
    Note over Server,Script: Old Flow (Removed)
    Server->>FS: Write script.ts & runner.ts
    Server->>Server: Spawn runner process
    Server->>Script: Execute via runner
    Script-->>Server: Process result
    end

    rect rgb(220, 255, 220)
    Note over Server,Script: New Flow (Current)
    Client->>Server: Submit job
    Server->>FS: Ensure baseWorkDir exists
    Server->>FS: Create job-specific workDir
    Server->>FS: Write script.ts
    alt Modules differ
        Server->>Server: Run Bun install
    end
    Server->>Server: Apply config.envVars
    Server->>Server: Override console methods
    Server->>Script: Dynamic import & execute
    Script-->>Server: Execution result
    rect rgb(255, 240, 200)
    Note over Server: Cleanup Phase
    Server->>Server: Restore environment vars
    Server->>Server: Restore console methods
    Server->>FS: Remove workDir
    end
    Server-->>Client: Return { result, logs } or error
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Execution model change: The shift from spawning a separate process to dynamic import requires careful verification of error handling, return value propagation, and async behavior.
  • Environment variable handling: Verify that all config.envVars are correctly applied and restored, especially in error paths and edge cases.
  • Logging interception: Review console method override logic to ensure it captures all output types and properly restores original behavior.
  • Work directory management: Confirm that baseWorkDir creation, job-specific directory handling, and cleanup logic work correctly across success/error scenarios.
  • Conditional Bun install: Validate that the preinstalledNodeModules comparison logic correctly determines when to reinstall dependencies.

Poem

🐰 No more spawning runners across the warren, We hop-import scripts with speeds most sparkling, Environment carrots tucked, logs captured tight, We nibble and execute, then clean up our night, The burrow runs faster—what a delightful sight! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description contains only a template comment with no actual content describing the changes, objectives, or rationale for the modifications. Add a meaningful description explaining the performance improvements, changes made to the execution model, and any migration notes or testing recommendations for reviewers.
Title check ❓ Inconclusive The title 'faster mock freestyle' is vague and generic, lacking specific details about what was optimized or how the mock freestyle was improved. Provide a more specific title that describes the actual optimization, such as 'Optimize freestyle mock execution with dynamic imports' or 'Speed up mock freestyle by removing separate runner process'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch speedup-mock-freestyle

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.

coderabbitai[bot] avatar Nov 26 '25 22:11 coderabbitai[bot]

Greptile Overview

Greptile Summary

Optimized the mock freestyle Docker build by preinstalling common dependencies (arktype, react, react-dom, @react-email/components) and switching from spawned process execution to in-process dynamic imports. This significantly speeds up build times by skipping dependency installation when user scripts only need preinstalled modules.

Key Changes:

  • Updated arktype to 2.1.20 and added react-dom 19.1.1 to preinstalled dependencies
  • Added preinstalledNodeModules Map to track versions and skip bun install when possible
  • Changed temp directory from /tmp to /app/tmp for better organization
  • Eliminated separate runner.ts file and spawned Bun process
  • User code now executes in-process via import(\file://${scriptFile}`)` instead of spawning separate process
  • Simplified console logging capture and environment variable handling by patching globals directly

Tradeoffs:

  • Speed vs Isolation: The in-process approach is faster but removes process isolation. If user code calls process.exit(), has memory leaks, or infinite loops, it can crash the entire server affecting all concurrent requests
  • Better for Development: This change prioritizes developer experience (faster iterations) over production-grade isolation

Confidence Score: 3/5

  • This PR is safe to merge for development/testing environments but has architectural tradeoffs that reduce fault isolation
  • The changes successfully optimize build speed through dependency preinstallation and in-process execution. However, the architectural shift from spawned processes to in-process dynamic imports removes crash isolation - user code can now take down the entire server. This is acceptable for a mock/development service but would be concerning for production. The code is functionally correct but introduces stability risks under untrusted code execution.
  • The Dockerfile requires attention for the process isolation tradeoff - consider if this is acceptable for your use case

Important Files Changed

File Analysis

Filename Score Overview
docker/dependencies/freestyle-mock/Dockerfile 3/5 Optimized Docker build by preinstalling dependencies and switching from spawned process to in-process execution; potential security implications from running untrusted code in main process

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server
    participant FileSystem
    participant UserCode
    
    Client->>Server: POST /execute/v1/script
    Note over Server: Parse script & config
    
    Server->>FileSystem: mkdir baseWorkDir (/app/tmp)
    Server->>FileSystem: mkdir workDir (job-UUID)
    Server->>FileSystem: Write script.ts
    Server->>FileSystem: Write package.json
    
    alt needsInstall (dependencies not preinstalled)
        Server->>Server: spawn("bun", ["install"])
        Note over Server: Install dependencies
    end
    
    Note over Server: Patch console methods
    Note over Server: Set environment variables
    
    Server->>UserCode: import(`file://${scriptFile}`)
    UserCode-->>Server: Execute in same process
    
    alt Success
        UserCode-->>Server: Return result
        Server->>Client: 200 {result, logs}
    else Error
        UserCode-->>Server: Throw error
        Server->>Client: 500 {error, logs}
    end
    
    Note over Server: Restore console & env vars
    Server->>FileSystem: rm workDir (cleanup)

greptile-apps[bot] avatar Nov 26 '25 22:11 greptile-apps[bot]