From 488d52d13b9f27ac51c50872a2c6aa8741ce578f Mon Sep 17 00:00:00 2001 From: Joseph Brinkman Date: Thu, 2 Jul 2026 08:28:09 -0400 Subject: [PATCH] feat: Move evaluation file setup from runtime to build-time in Dockerfile --- ...ove-evaluation-file-setup-to-build-time.md | 217 +++++++++++++ internal/eval/dockerfile/base.Dockerfile | 7 + internal/eval/runner.go | 12 +- internal/eval/sandbox/build_context.go | 296 ++++++++++++++++++ internal/eval/sandbox/container.go | 141 +++++++-- internal/eval/sandbox/image_manager.go | 2 +- internal/eval/sandbox/installation_test.go | 150 +++++++-- internal/eval/sandbox/mock_github.go | 87 ----- 8 files changed, 767 insertions(+), 145 deletions(-) create mode 100644 .kiro-krew/specs/issue-192-move-evaluation-file-setup-to-build-time.md create mode 100644 internal/eval/sandbox/build_context.go diff --git a/.kiro-krew/specs/issue-192-move-evaluation-file-setup-to-build-time.md b/.kiro-krew/specs/issue-192-move-evaluation-file-setup-to-build-time.md new file mode 100644 index 0000000..9548b9c --- /dev/null +++ b/.kiro-krew/specs/issue-192-move-evaluation-file-setup-to-build-time.md @@ -0,0 +1,217 @@ +# Design Specification: Move Evaluation File Setup from Runtime to Build-time in Dockerfile + +**Issue**: #192 - Move evaluation file setup from runtime to build-time in Dockerfile +**Closes**: #192 + +## Problem Analysis + +The current evaluation framework performs file operations at runtime that should happen at Docker build time, causing permission errors and unnecessary complexity: + +### Current Issues: +- **Permission Errors**: Runtime file operations fail with "Permission denied" errors in containers +- **Runtime Complexity**: `SetupGitHubMocking()` and `ConfigureMockGitHubPath()` functions perform file copying at runtime +- **Performance Impact**: Each container startup requires file setup operations +- **Reliability Issues**: Runtime operations can fail unpredictably + +### Current Architecture: +``` +Container Start → Runtime Setup Functions → File Operations → Permission Errors + ↓ ↓ ↓ ↓ +Container.Create() → SetupGitHubMocking() → Copy mock files → mkdir: Permission denied + → ConfigureMockGitHubPath() → PATH setup +``` + +## Solution Approach + +Transform the evaluation framework to embed all required files at Docker build time, creating self-contained, immutable containers. + +### Target Architecture: +``` +Dockerfile Build → Embed Files → Immutable Container → Direct Execution + ↓ ↓ ↓ ↓ + COPY commands → All files baked in → No runtime setup → Fast startup +``` + +## Relevant Files + +### Files to Modify: +1. **`internal/eval/sandbox/container.go`** - Dockerfile generation logic +2. **`internal/eval/sandbox/mock_github.go`** - Remove runtime functions +3. **`internal/eval/runner.go`** - Remove runtime setup calls +4. **`internal/eval/dockerfile/base.Dockerfile`** - Add COPY commands + +### Files to Reference: +1. **`internal/eval/sandbox/testdata/github-cli-mock/`** - Mock files to embed +2. **`.kiro/agents/`** - Agent configurations to embed +3. **`.kiro-krew/evals/`** - Evaluation rubrics and test cases to embed +4. **`internal/eval/sandbox/installation_test.go`** - Test updates needed + +### New Files to Create: +1. **`internal/eval/dockerfile/evaluation.Dockerfile`** - New build-time template +2. **Build context preparation scripts** - For copying files to build context + +## Team Orchestration + +This is a focused architectural change that can be implemented by a single developer with the following coordination points: + +### Dependencies: +- **Dockerfile Templates**: Update base templates to include COPY operations +- **Build Context**: Ensure all required files are available in Docker build context +- **Testing**: Verify all evaluation tests pass without runtime setup + +### Integration Points: +- **Image Manager**: Update to handle new build-time file embedding +- **Debug Mode**: Ensure generated Dockerfiles include proper file paths +- **CI/CD**: Verify binary distribution compatibility + +## Step-by-Step Task Breakdown + +### Task 1: Update Dockerfile Generation Logic +**File**: `internal/eval/sandbox/container.go` +**Acceptance Criteria**: +- [ ] `GenerateDockerfileWithPlatform()` includes COPY commands for all required files +- [ ] Files are copied from embedded filesystem or build context +- [ ] Generated Dockerfile creates all necessary directories with proper permissions +- [ ] kiro-krew binary, agent configs, mock files, and evaluation files are embedded + +**Implementation Details**: +```dockerfile +# Add to generated Dockerfile: +COPY --chown=sandbox:sandbox kiro-krew-binary /usr/local/bin/kiro-krew +COPY --chown=sandbox:sandbox .kiro/ /workspace/.kiro/ +COPY --chown=sandbox:sandbox github-cli-mock/ /workspace/.kiro/skills/github-cli/ +COPY --chown=sandbox:sandbox .kiro-krew/evals/ /workspace/.kiro-krew/evals/ +RUN mkdir -p /workspace/.kiro/skills && chown -R sandbox:sandbox /workspace/.kiro +ENV PATH="/workspace/.kiro/skills/github-cli:$PATH" +``` + +### Task 2: Create Build Context Preparation +**File**: `internal/eval/sandbox/build_context.go` (new) +**Acceptance Criteria**: +- [ ] Function to prepare build context with all required files +- [ ] Handle embedded filesystems (`//go:embed` directives) +- [ ] Support binary distribution (files from installed locations) +- [ ] Create temporary build context directory structure + +**Implementation Details**: +```go +type BuildContext struct { + TempDir string + Files map[string][]byte +} + +func PrepareBuildContext(krewBinary string) (*BuildContext, error) +func (bc *BuildContext) AddAgentConfigs() error +func (bc *BuildContext) AddMockFiles() error +func (bc *BuildContext) AddEvaluationFiles() error +func (bc *BuildContext) Cleanup() error +``` + +### Task 3: Remove Runtime Setup Functions +**File**: `internal/eval/sandbox/mock_github.go` +**Acceptance Criteria**: +- [ ] Remove `SetupGitHubMocking()` function entirely +- [ ] Remove `ConfigureMockGitHubPath()` function entirely +- [ ] Keep embedded filesystem for build-time use +- [ ] Update function documentation to reflect build-time approach + +### Task 4: Update Runner to Remove Runtime Calls +**File**: `internal/eval/runner.go` +**Acceptance Criteria**: +- [ ] Remove calls to `SetupGitHubMocking()` in `invokeAgent()` +- [ ] Remove calls to `ConfigureMockGitHubPath()` in `invokeAgent()` +- [ ] Remove MockGitHub configuration checks +- [ ] Update error handling to remove mocking-related error paths + +**Location**: Around line 704 in `invokeAgent()` function + +### Task 5: Update Base Dockerfile Template +**File**: `internal/eval/dockerfile/base.Dockerfile` +**Acceptance Criteria**: +- [ ] Include directory creation for `.kiro` and `.kiro-krew` paths +- [ ] Set proper ownership and permissions for sandbox user +- [ ] Add environment variables for PATH configuration +- [ ] Maintain Alpine Linux base and security practices + +### Task 6: Embed File Resources +**Files**: Multiple locations +**Acceptance Criteria**: +- [ ] Add `//go:embed` directive for agent configurations +- [ ] Add `//go:embed` directive for evaluation files +- [ ] Update existing GitHub mock embed to work with build context +- [ ] Handle kiro-krew binary embedding or build context inclusion + +### Task 7: Update Integration Tests +**File**: `internal/eval/sandbox/installation_test.go` +**Acceptance Criteria**: +- [ ] Remove tests for `SetupGitHubMocking()` +- [ ] Remove tests for `ConfigureMockGitHubPath()` +- [ ] Add tests for build-time file embedding +- [ ] Verify containers start with all files present +- [ ] Test PATH configuration works correctly + +### Task 8: Update Container Build Process +**File**: `internal/eval/sandbox/container.go` +**Acceptance Criteria**: +- [ ] `BuildImageFromDockerfile()` prepares build context with all files +- [ ] Build context includes kiro-krew binary from current installation +- [ ] Temporary build context is cleaned up after image build +- [ ] Image Manager compatibility maintained + +## Validation Commands + +### Build and Test Commands: +```bash +# Build evaluation images +go run ./cmd/kiro-krew eval --agent architect --case basic-spec-generation + +# Verify container contents +docker run --rm ls -la /workspace/.kiro/agents/ +docker run --rm ls -la /workspace/.kiro/skills/github-cli/ +docker run --rm ls -la /workspace/.kiro-krew/evals/ + +# Test GitHub mocking +docker run --rm /workspace/.kiro/skills/github-cli/gh --help + +# Test kiro-krew binary +docker run --rm kiro-krew --version + +# Run full evaluation suite +go test ./internal/eval/... -v + +# Test binary distribution compatibility +# 1. Build binary: go build ./cmd/kiro-krew +# 2. Move to temp location outside source tree +# 3. Run evaluations from that location +``` + +### Verification Checklist: +- [ ] All evaluation tests pass without permission errors +- [ ] Containers start faster (no runtime file setup) +- [ ] GitHub CLI mock works correctly from embedded files +- [ ] Agent configurations loaded from container filesystem +- [ ] Evaluation rubrics and cases accessible from container +- [ ] Works with binary distribution (no source code access) +- [ ] Docker images follow best practices (proper layering, caching) + +## Success Metrics + +1. **Zero Permission Errors**: No "Permission denied" errors during evaluation runs +2. **Faster Startup**: Container startup time reduced by eliminating runtime file operations +3. **Immutable Containers**: All required files baked into image at build time +4. **Binary Distribution Compatible**: Works when kiro-krew distributed as standalone binary +5. **Test Suite Passes**: All existing evaluation tests continue to pass + +## Constraints Addressed + +- **Binary Distribution**: Solution works without source code access by detecting kiro-krew binary location +- **Immutable Containers**: All files embedded at build time, no runtime modifications +- **Docker Best Practices**: Proper layer optimization, security, and caching +- **Backward Compatibility**: Existing evaluation test cases continue to work + +## Risk Mitigation + +1. **Image Size**: Use multi-stage builds and .dockerignore to optimize image size +2. **Build Context Size**: Only copy necessary files, exclude large directories like `.git` +3. **File Permissions**: Set proper ownership and permissions in Dockerfile +4. **Cross-platform**: Ensure embedded files work across different host architectures \ No newline at end of file diff --git a/internal/eval/dockerfile/base.Dockerfile b/internal/eval/dockerfile/base.Dockerfile index 51bf43a..7efb71e 100644 --- a/internal/eval/dockerfile/base.Dockerfile +++ b/internal/eval/dockerfile/base.Dockerfile @@ -10,6 +10,13 @@ RUN apk add --no-cache \ # Create non-root user for security RUN adduser -D -s /bin/bash sandbox +# Create directories for embedded files with proper ownership +RUN mkdir -p /workspace/.kiro/agents && \ + mkdir -p /workspace/.kiro/skills/github-cli && \ + mkdir -p /workspace/.kiro-krew/evals && \ + chown -R sandbox:sandbox /workspace/.kiro && \ + chown -R sandbox:sandbox /workspace/.kiro-krew + # Set working directory WORKDIR /workspace diff --git a/internal/eval/runner.go b/internal/eval/runner.go index 92e813f..395bfbb 100644 --- a/internal/eval/runner.go +++ b/internal/eval/runner.go @@ -699,16 +699,8 @@ func invokeAgentInContainer(agent, prompt string, cConfig *ContainerConfig) (str return "", CostInfo{}, nil, fmt.Errorf("validating kiro-cli: %w", err) } - // Setup GitHub mocking if enabled - if cConfig.MockGitHub { - if err := c.SetupGitHubMocking(ctx, cConfig.WorkspaceDir); err != nil { - return "", CostInfo{}, nil, fmt.Errorf("setting up GitHub mocking: %w", err) - } - - if err := c.ConfigureMockGitHubPath(ctx); err != nil { - return "", CostInfo{}, nil, fmt.Errorf("configuring mock GitHub PATH: %w", err) - } - } + // GitHub mocking is now configured at build time through environment variables + // and embedded files in the container image fmt.Printf(" Container setup: %v\n", time.Since(setupStart)) diff --git a/internal/eval/sandbox/build_context.go b/internal/eval/sandbox/build_context.go new file mode 100644 index 0000000..353d33d --- /dev/null +++ b/internal/eval/sandbox/build_context.go @@ -0,0 +1,296 @@ +package sandbox + +import ( + "embed" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "strings" +) + +//go:embed testdata/github-cli-mock/* +var githubCliMock embed.FS + +// BuildContext manages preparation of Docker build context with all required files +type BuildContext struct { + TempDir string + KrewBinary string + Files map[string][]byte + cleanupFuncs []func() error +} + +// PrepareBuildContext creates a temporary directory with all files needed for Docker build +func PrepareBuildContext(krewBinary string) (*BuildContext, error) { + // Create temporary directory + tempDir, err := os.MkdirTemp("", "kiro-krew-build-*") + if err != nil { + return nil, fmt.Errorf("creating temp directory: %w", err) + } + + bc := &BuildContext{ + TempDir: tempDir, + KrewBinary: krewBinary, + Files: make(map[string][]byte), + cleanupFuncs: []func() error{ + func() error { return os.RemoveAll(tempDir) }, + }, + } + + // Add all required files to build context + if err := bc.AddKrewBinary(); err != nil { + bc.Cleanup() + return nil, fmt.Errorf("adding kiro-krew binary: %w", err) + } + + if err := bc.AddAgentConfigs(); err != nil { + bc.Cleanup() + return nil, fmt.Errorf("adding agent configs: %w", err) + } + + if err := bc.AddMockFiles(); err != nil { + bc.Cleanup() + return nil, fmt.Errorf("adding mock files: %w", err) + } + + if err := bc.AddEvaluationFiles(); err != nil { + bc.Cleanup() + return nil, fmt.Errorf("adding evaluation files: %w", err) + } + + return bc, nil +} + +// AddKrewBinary copies the kiro-krew binary to build context +func (bc *BuildContext) AddKrewBinary() error { + krewPath := bc.KrewBinary + if krewPath == "" { + // Try to find kiro-krew in PATH + path, err := exec.LookPath("kiro-krew") + if err != nil { + return fmt.Errorf("kiro-krew binary not found in PATH and no explicit path provided: %w", err) + } + krewPath = path + } + + // Copy binary to build context + destPath := filepath.Join(bc.TempDir, "kiro-krew") + content, err := os.ReadFile(krewPath) + if err != nil { + return fmt.Errorf("reading kiro-krew binary from %s: %w", krewPath, err) + } + + if err := os.WriteFile(destPath, content, 0755); err != nil { + return fmt.Errorf("writing kiro-krew binary to build context: %w", err) + } + + bc.Files["kiro-krew"] = content + return nil +} + +// AddAgentConfigs copies .kiro/agents/ directory to build context +func (bc *BuildContext) AddAgentConfigs() error { + // Look for .kiro/agents in current working directory and parent directories + agentDir, err := bc.findAgentDirectory() + if err != nil { + return fmt.Errorf("finding agent directory: %w", err) + } + + // Create .kiro/agents in build context + destDir := filepath.Join(bc.TempDir, ".kiro", "agents") + if err := os.MkdirAll(destDir, 0755); err != nil { + return fmt.Errorf("creating agent directory in build context: %w", err) + } + + // Copy all agent files + return filepath.Walk(agentDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if info.IsDir() { + return nil + } + + relPath, err := filepath.Rel(agentDir, path) + if err != nil { + return err + } + + content, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading agent config %s: %w", path, err) + } + + destPath := filepath.Join(destDir, relPath) + if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil { + return fmt.Errorf("creating directory for %s: %w", destPath, err) + } + + if err := os.WriteFile(destPath, content, 0644); err != nil { + return fmt.Errorf("writing agent config to build context: %w", err) + } + + bc.Files[filepath.Join(".kiro/agents", relPath)] = content + return nil + }) +} + +// AddMockFiles copies GitHub CLI mock files to build context +func (bc *BuildContext) AddMockFiles() error { + // Create github-cli-mock directory in build context + destDir := filepath.Join(bc.TempDir, "github-cli-mock") + if err := os.MkdirAll(destDir, 0755); err != nil { + return fmt.Errorf("creating mock directory in build context: %w", err) + } + + // Copy files from embedded filesystem + return fs.WalkDir(githubCliMock, "testdata/github-cli-mock", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { + return nil + } + + relPath, err := filepath.Rel("testdata/github-cli-mock", path) + if err != nil { + return err + } + + content, err := githubCliMock.ReadFile(path) + if err != nil { + return fmt.Errorf("reading mock file %s: %w", path, err) + } + + destPath := filepath.Join(destDir, relPath) + if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil { + return fmt.Errorf("creating directory for %s: %w", destPath, err) + } + + mode := os.FileMode(0644) + if strings.Contains(relPath, "gh") || strings.HasSuffix(relPath, ".sh") { + mode = 0755 // Make executable + } + + if err := os.WriteFile(destPath, content, mode); err != nil { + return fmt.Errorf("writing mock file to build context: %w", err) + } + + bc.Files[filepath.Join("github-cli-mock", relPath)] = content + return nil + }) +} + +// AddEvaluationFiles copies .kiro-krew/evals/ directory to build context if it exists +func (bc *BuildContext) AddEvaluationFiles() error { + // Look for .kiro-krew/evals in current working directory and parent directories + evalsDir, err := bc.findEvalsDirectory() + if err != nil { + // Evaluation files are optional, so just log and continue + return nil + } + + // Create .kiro-krew/evals in build context + destDir := filepath.Join(bc.TempDir, ".kiro-krew", "evals") + if err := os.MkdirAll(destDir, 0755); err != nil { + return fmt.Errorf("creating evals directory in build context: %w", err) + } + + // Copy all evaluation files + return filepath.Walk(evalsDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if info.IsDir() { + return nil + } + + relPath, err := filepath.Rel(evalsDir, path) + if err != nil { + return err + } + + content, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading eval file %s: %w", path, err) + } + + destPath := filepath.Join(destDir, relPath) + if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil { + return fmt.Errorf("creating directory for %s: %w", destPath, err) + } + + if err := os.WriteFile(destPath, content, 0644); err != nil { + return fmt.Errorf("writing eval file to build context: %w", err) + } + + bc.Files[filepath.Join(".kiro-krew/evals", relPath)] = content + return nil + }) +} + +// findAgentDirectory searches for .kiro/agents directory in current and parent directories +func (bc *BuildContext) findAgentDirectory() (string, error) { + dir, err := os.Getwd() + if err != nil { + return "", err + } + + for { + agentDir := filepath.Join(dir, ".kiro", "agents") + if info, err := os.Stat(agentDir); err == nil && info.IsDir() { + return agentDir, nil + } + + parent := filepath.Dir(dir) + if parent == dir { + break // Reached filesystem root + } + dir = parent + } + + return "", fmt.Errorf(".kiro/agents directory not found in current directory or any parent directory") +} + +// findEvalsDirectory searches for .kiro-krew/evals directory in current and parent directories +func (bc *BuildContext) findEvalsDirectory() (string, error) { + dir, err := os.Getwd() + if err != nil { + return "", err + } + + for { + evalsDir := filepath.Join(dir, ".kiro-krew", "evals") + if info, err := os.Stat(evalsDir); err == nil && info.IsDir() { + return evalsDir, nil + } + + parent := filepath.Dir(dir) + if parent == dir { + break // Reached filesystem root + } + dir = parent + } + + return "", fmt.Errorf(".kiro-krew/evals directory not found") +} + +// Cleanup removes temporary files and directories +func (bc *BuildContext) Cleanup() error { + var errors []string + for _, cleanup := range bc.cleanupFuncs { + if err := cleanup(); err != nil { + errors = append(errors, err.Error()) + } + } + + if len(errors) > 0 { + return fmt.Errorf("cleanup errors: %s", strings.Join(errors, "; ")) + } + + return nil +} diff --git a/internal/eval/sandbox/container.go b/internal/eval/sandbox/container.go index c61ce5b..2985a72 100644 --- a/internal/eval/sandbox/container.go +++ b/internal/eval/sandbox/container.go @@ -363,6 +363,7 @@ func (c *Container) CleanupWithDebugInfo(ctx context.Context, failed bool) error } // GenerateDockerfileWithPlatform creates a custom Dockerfile with platform-specific kiro-cli installation +// and embedded files for build-time setup func (c *Container) GenerateDockerfileWithPlatform(projectPath, platform string) (string, error) { projects := DetectProject(projectPath) @@ -399,6 +400,36 @@ func (c *Container) GenerateDockerfileWithPlatform(projectPath, platform string) // Add user and workspace setup dockerfile.WriteString("RUN adduser -D -s /bin/bash sandbox\n") dockerfile.WriteString("RUN mkdir -p /workspace && chown sandbox:sandbox /workspace\n") + + // Create directories for embedded files with proper ownership + dockerfile.WriteString("RUN mkdir -p /workspace/.kiro/agents && \\\n") + dockerfile.WriteString(" mkdir -p /workspace/.kiro/skills/github-cli && \\\n") + dockerfile.WriteString(" mkdir -p /workspace/.kiro-krew/evals && \\\n") + dockerfile.WriteString(" chown -R sandbox:sandbox /workspace/.kiro && \\\n") + dockerfile.WriteString(" chown -R sandbox:sandbox /workspace/.kiro-krew\n\n") + + // Copy kiro-krew binary from build context + dockerfile.WriteString("# Copy kiro-krew binary from build context\n") + dockerfile.WriteString("COPY --chown=sandbox:sandbox kiro-krew /usr/local/bin/kiro-krew\n") + dockerfile.WriteString("RUN chmod +x /usr/local/bin/kiro-krew\n\n") + + // Copy agent configurations from build context + dockerfile.WriteString("# Copy agent configurations\n") + dockerfile.WriteString("COPY --chown=sandbox:sandbox .kiro/agents/ /workspace/.kiro/agents/\n\n") + + // Copy GitHub CLI mock files from build context + dockerfile.WriteString("# Copy GitHub CLI mock files\n") + dockerfile.WriteString("COPY --chown=sandbox:sandbox github-cli-mock/ /workspace/.kiro/skills/github-cli/\n") + dockerfile.WriteString("RUN chmod +x /workspace/.kiro/skills/github-cli/gh\n\n") + + // Copy evaluation files from build context + dockerfile.WriteString("# Copy evaluation files\n") + dockerfile.WriteString("COPY --chown=sandbox:sandbox .kiro-krew/evals/ /workspace/.kiro-krew/evals/\n\n") + + // Configure PATH to include mock GitHub CLI + dockerfile.WriteString("# Configure PATH for mock GitHub CLI\n") + dockerfile.WriteString("ENV PATH=\"/workspace/.kiro/skills/github-cli:$PATH\"\n\n") + dockerfile.WriteString("WORKDIR /workspace\n") dockerfile.WriteString("USER sandbox\n") dockerfile.WriteString("CMD [\"/bin/bash\"]\n") @@ -419,7 +450,7 @@ func (c *Container) GenerateDockerfileWithPlatform(projectPath, platform string) return dockerfileContent, nil } -// BuildImageFromDockerfile builds a Docker image from generated Dockerfile content +// BuildImageFromDockerfile builds a Docker image from generated Dockerfile content with build context func (c *Container) BuildImageFromDockerfile(ctx context.Context, dockerfile string, imageName string, platform string) error { // Use image manager for reuse if available if c.imageManager != nil { @@ -432,32 +463,22 @@ func (c *Container) BuildImageFromDockerfile(ctx context.Context, dockerfile str return nil } - return c.buildImageDirect(ctx, dockerfile, imageName, platform) + return c.buildImageWithContext(ctx, dockerfile, imageName, platform) } -// buildImageDirect performs the actual Docker image build without imageManager delegation. -func (c *Container) buildImageDirect(ctx context.Context, dockerfile string, imageName string, platform string) error { - - // Create tar archive with Dockerfile - var buf bytes.Buffer - tw := tar.NewWriter(&buf) - - dockerfileInfo := &tar.Header{ - Name: "Dockerfile", - Size: int64(len(dockerfile)), - Mode: 0644, - } - - if err := tw.WriteHeader(dockerfileInfo); err != nil { - return fmt.Errorf("writing dockerfile header: %w", err) - } - - if _, err := tw.Write([]byte(dockerfile)); err != nil { - return fmt.Errorf("writing dockerfile content: %w", err) +// buildImageWithContext performs Docker image build with prepared build context containing all required files +func (c *Container) buildImageWithContext(ctx context.Context, dockerfile string, imageName string, platform string) error { + // Prepare build context with all required files + buildContext, err := PrepareBuildContext("") + if err != nil { + return fmt.Errorf("preparing build context: %w", err) } + defer buildContext.Cleanup() - if err := tw.Close(); err != nil { - return fmt.Errorf("closing tar writer: %w", err) + // Create tar archive with Dockerfile and build context files + tarData, err := c.createBuildContextTar(dockerfile, buildContext) + if err != nil { + return fmt.Errorf("creating build context tar: %w", err) } // Build the image @@ -467,10 +488,11 @@ func (c *Container) buildImageDirect(ctx context.Context, dockerfile string, ima } if c.debugMode { - fmt.Printf("🔧 Debug: Building image %s for platform %s\n", imageName, platform) + fmt.Printf("🔧 Debug: Building image %s for platform %s with build context\n", imageName, platform) + fmt.Printf("🔧 Debug: Build context contains %d files\n", len(buildContext.Files)) } - resp, err := c.client.ImageBuild(ctx, bytes.NewReader(buf.Bytes()), buildOptions) + resp, err := c.client.ImageBuild(ctx, tarData, buildOptions) if err != nil { return fmt.Errorf("building image: %w", err) } @@ -609,6 +631,75 @@ func (c *Container) verifyKiroCLIInstallation(ctx context.Context) error { return nil } +// createBuildContextTar creates a tar archive containing the Dockerfile and build context files +func (c *Container) createBuildContextTar(dockerfile string, buildContext *BuildContext) (*bytes.Reader, error) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + // Add Dockerfile + dockerfileHeader := &tar.Header{ + Name: "Dockerfile", + Size: int64(len(dockerfile)), + Mode: 0644, + } + if err := tw.WriteHeader(dockerfileHeader); err != nil { + return nil, fmt.Errorf("writing dockerfile header: %w", err) + } + if _, err := tw.Write([]byte(dockerfile)); err != nil { + return nil, fmt.Errorf("writing dockerfile content: %w", err) + } + + // Add all build context files + err := filepath.Walk(buildContext.TempDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if info.IsDir() { + return nil + } + + // Get relative path from build context temp dir + relPath, err := filepath.Rel(buildContext.TempDir, path) + if err != nil { + return err + } + + // Read file content + content, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("reading build context file %s: %w", path, err) + } + + // Add to tar + header := &tar.Header{ + Name: relPath, + Size: int64(len(content)), + Mode: int64(info.Mode().Perm()), + } + + if err := tw.WriteHeader(header); err != nil { + return fmt.Errorf("writing header for %s: %w", relPath, err) + } + + if _, err := tw.Write(content); err != nil { + return fmt.Errorf("writing content for %s: %w", relPath, err) + } + + return nil + }) + + if err != nil { + return nil, fmt.Errorf("walking build context: %w", err) + } + + if err := tw.Close(); err != nil { + return nil, fmt.Errorf("closing tar writer: %w", err) + } + + return bytes.NewReader(buf.Bytes()), nil +} + // Close closes the Docker client func (c *Container) Close() error { return c.client.Close() diff --git a/internal/eval/sandbox/image_manager.go b/internal/eval/sandbox/image_manager.go index 7197ede..750dc28 100644 --- a/internal/eval/sandbox/image_manager.go +++ b/internal/eval/sandbox/image_manager.go @@ -76,7 +76,7 @@ func (im *ImageManager) BuildForEvaluation(ctx context.Context, dockerfile, plat } defer container.Close() - if err := container.buildImageDirect(ctx, dockerfile, imageName, platform); err != nil { + if err := container.buildImageWithContext(ctx, dockerfile, imageName, platform); err != nil { return "", fmt.Errorf("building image %s: %w", imageName, err) } diff --git a/internal/eval/sandbox/installation_test.go b/internal/eval/sandbox/installation_test.go index 49f8707..ed77021 100644 --- a/internal/eval/sandbox/installation_test.go +++ b/internal/eval/sandbox/installation_test.go @@ -27,6 +27,15 @@ func TestDockerfileGeneration_IncludesKiroCLI(t *testing.T) { "kirocli-x86_64-linux-musl.zip", "chmod 755 kirocli/bin/kiro-cli", "mv kirocli/bin/kiro-cli /usr/local/bin/kiro-cli", + "# Copy kiro-krew binary from build context", + "COPY --chown=sandbox:sandbox kiro-krew /usr/local/bin/kiro-krew", + "# Copy agent configurations", + "COPY --chown=sandbox:sandbox .kiro/agents/ /workspace/.kiro/agents/", + "# Copy GitHub CLI mock files", + "COPY --chown=sandbox:sandbox github-cli-mock/ /workspace/.kiro/skills/github-cli/", + "# Copy evaluation files", + "COPY --chown=sandbox:sandbox .kiro-krew/evals/ /workspace/.kiro-krew/evals/", + "ENV PATH=\"/workspace/.kiro/skills/github-cli:$PATH\"", }, }, { @@ -37,6 +46,15 @@ func TestDockerfileGeneration_IncludesKiroCLI(t *testing.T) { "kirocli-aarch64-linux-musl.zip", "chmod 755 kirocli/bin/kiro-cli", "mv kirocli/bin/kiro-cli /usr/local/bin/kiro-cli", + "# Copy kiro-krew binary from build context", + "COPY --chown=sandbox:sandbox kiro-krew /usr/local/bin/kiro-krew", + "# Copy agent configurations", + "COPY --chown=sandbox:sandbox .kiro/agents/ /workspace/.kiro/agents/", + "# Copy GitHub CLI mock files", + "COPY --chown=sandbox:sandbox github-cli-mock/ /workspace/.kiro/skills/github-cli/", + "# Copy evaluation files", + "COPY --chown=sandbox:sandbox .kiro-krew/evals/ /workspace/.kiro-krew/evals/", + "ENV PATH=\"/workspace/.kiro/skills/github-cli:$PATH\"", }, }, } @@ -238,6 +256,57 @@ func TestDockerfileGeneration_ProjectDetection(t *testing.T) { assert.Contains(t, dockerfile, "USER sandbox") } +func TestBuildContext_Preparation(t *testing.T) { + t.Run("PrepareBuildContext", func(t *testing.T) { + // Create a minimal build context without actual kiro-krew binary + tempDir := t.TempDir() + + // Create mock kiro-krew binary + mockBinary := filepath.Join(tempDir, "kiro-krew") + err := os.WriteFile(mockBinary, []byte("mock binary"), 0755) + require.NoError(t, err) + + buildContext, err := PrepareBuildContext(mockBinary) + require.NoError(t, err) + defer buildContext.Cleanup() + + // Verify build context structure + assert.NotEmpty(t, buildContext.TempDir) + assert.Equal(t, mockBinary, buildContext.KrewBinary) + assert.NotEmpty(t, buildContext.Files) + + // Verify kiro-krew binary is in files map + _, exists := buildContext.Files["kiro-krew"] + assert.True(t, exists, "kiro-krew binary should be in build context") + }) + + t.Run("AddMockFiles", func(t *testing.T) { + tempDir := t.TempDir() + bc := &BuildContext{ + TempDir: tempDir, + Files: make(map[string][]byte), + } + + err := bc.AddMockFiles() + require.NoError(t, err) + + // Verify mock files were added + foundMockFiles := false + for path := range bc.Files { + if strings.Contains(path, "github-cli-mock") { + foundMockFiles = true + break + } + } + assert.True(t, foundMockFiles, "Mock files should be added to build context") + + // Verify mock directory exists in temp dir + mockDir := filepath.Join(tempDir, "github-cli-mock") + _, err = os.Stat(mockDir) + assert.NoError(t, err, "Mock directory should exist in build context") + }) +} + func TestBuildTimeVsRuntime_Installation(t *testing.T) { t.Run("BuildTimeInstallation", func(t *testing.T) { // Test that Dockerfile generation includes build-time installation @@ -272,6 +341,28 @@ func TestBuildTimeVsRuntime_Installation(t *testing.T) { // Expect error since we don't have a running container with kiro-cli assert.Error(t, err, "Should fail verification when kiro-cli not installed") }) + + t.Run("BuildTimeFileEmbedding", func(t *testing.T) { + // Test that Dockerfile includes all required COPY commands + tempDir := t.TempDir() + c := &Container{} + platform, err := DetectHostArchitecture() + require.NoError(t, err) + dockerfile, err := c.GenerateDockerfileWithPlatform(tempDir, platform) + require.NoError(t, err) + + // Verify build-time file copying commands are present + assert.Contains(t, dockerfile, "COPY --chown=sandbox:sandbox kiro-krew /usr/local/bin/kiro-krew") + assert.Contains(t, dockerfile, "COPY --chown=sandbox:sandbox .kiro/agents/ /workspace/.kiro/agents/") + assert.Contains(t, dockerfile, "COPY --chown=sandbox:sandbox github-cli-mock/ /workspace/.kiro/skills/github-cli/") + assert.Contains(t, dockerfile, "COPY --chown=sandbox:sandbox .kiro-krew/evals/ /workspace/.kiro-krew/evals/") + assert.Contains(t, dockerfile, "ENV PATH=\"/workspace/.kiro/skills/github-cli:$PATH\"") + + // Verify directory creation commands + assert.Contains(t, dockerfile, "mkdir -p /workspace/.kiro/agents") + assert.Contains(t, dockerfile, "mkdir -p /workspace/.kiro/skills/github-cli") + assert.Contains(t, dockerfile, "mkdir -p /workspace/.kiro-krew/evals") + }) } func TestPlatformSpecificBinaries(t *testing.T) { @@ -423,42 +514,57 @@ func TestGenerateDockerfile_ErrorHandling(t *testing.T) { assert.Contains(t, err.Error(), "unsupported platform") } -func TestMockGitHub_Functions(t *testing.T) { - // Test mock functions for coverage even though they're not installation-related - skipIfNoDocker(t) +func TestMockGitHub_SimulateResponse(t *testing.T) { + // Test mock response simulation functions that are still present + + // Test SimulateGitHubResponse + response := SimulateGitHubResponse("issue", []string{"create"}) + assert.Equal(t, 12345, response.IssueNumber) + + response = SimulateGitHubResponse("pr", []string{"create"}) + assert.Equal(t, 42, response.PRNumber) + response = SimulateGitHubResponse("unknown", []string{}) + assert.Equal(t, "success", response.Status) +} + +func TestContainer_CreateBuildContextTar(t *testing.T) { c, err := NewContainer("alpine:3.19") require.NoError(t, err) defer c.Close() - ctx := context.Background() + // Create a build context with some test files + tempDir := t.TempDir() - config := &container.Config{ - Image: "alpine:3.19", - Cmd: []string{"sleep", "30"}, - } - err = c.Create(ctx, config, &container.HostConfig{}) + // Create test files in build context + testFile1 := filepath.Join(tempDir, "test1.txt") + testFile2 := filepath.Join(tempDir, "subdir", "test2.txt") + + err = os.WriteFile(testFile1, []byte("content1"), 0644) require.NoError(t, err) - err = c.Start(ctx) + err = os.MkdirAll(filepath.Dir(testFile2), 0755) require.NoError(t, err) - defer c.Cleanup(ctx) - err = c.SetupGitHubMocking(ctx, "/workspace") - assert.NoError(t, err, "SetupGitHubMocking should not fail") + err = os.WriteFile(testFile2, []byte("content2"), 0644) + require.NoError(t, err) - // Test ConfigureMockGitHubPath (requires running container) - // This would need a running container to work properly + buildContext := &BuildContext{ + TempDir: tempDir, + Files: map[string][]byte{ + "test1.txt": []byte("content1"), + "subdir/test2.txt": []byte("content2"), + }, + } - // Test SimulateGitHubResponse - response := SimulateGitHubResponse("issue", []string{"create"}) - assert.Equal(t, 12345, response.IssueNumber) + dockerfile := "FROM alpine:3.19\nRUN echo test" - response = SimulateGitHubResponse("pr", []string{"create"}) - assert.Equal(t, 42, response.PRNumber) + tarReader, err := c.createBuildContextTar(dockerfile, buildContext) + require.NoError(t, err) + assert.NotNil(t, tarReader) - response = SimulateGitHubResponse("unknown", []string{}) - assert.Equal(t, "success", response.Status) + // The tar should contain the dockerfile and build context files + // We don't need to verify the tar contents in detail for this test } func TestContainer_CompleteInstallationFlow(t *testing.T) { diff --git a/internal/eval/sandbox/mock_github.go b/internal/eval/sandbox/mock_github.go index 1b5fcd4..8f737e8 100644 --- a/internal/eval/sandbox/mock_github.go +++ b/internal/eval/sandbox/mock_github.go @@ -1,99 +1,12 @@ package sandbox import ( - "archive/tar" - "bytes" - "context" "embed" - "fmt" - "io/fs" - "path/filepath" - - "github.com/docker/docker/api/types/container" ) //go:embed testdata/github-cli-mock/* var mockGitHubSkill embed.FS -// SetupGitHubMocking replaces .kiro/skills/github-cli/ with mock version in container -func (c *Container) SetupGitHubMocking(ctx context.Context, workspacePath string) error { - if c.containerID == "" { - return fmt.Errorf("container not created - call Create() before SetupGitHubMocking") - } - - skillPath := filepath.Join(workspacePath, ".kiro", "skills", "github-cli") - - // Create skills directory if it doesn't exist - if _, err := c.ExecWithOutput(ctx, []string{"mkdir", "-p", filepath.Dir(skillPath)}); err != nil { - return err - } - - // Remove existing github-cli skill if present - if _, err := c.ExecWithOutput(ctx, []string{"rm", "-rf", skillPath}); err != nil { - return err - } - - // Create mock skill directory - if _, err := c.ExecWithOutput(ctx, []string{"mkdir", "-p", skillPath}); err != nil { - return err - } - - // Copy mock skill files - return fs.WalkDir(mockGitHubSkill, "testdata/github-cli-mock", func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - - if d.IsDir() { - return nil - } - - relPath, _ := filepath.Rel("testdata/github-cli-mock", path) - destPath := filepath.Join(skillPath, relPath) - - content, err := mockGitHubSkill.ReadFile(path) - if err != nil { - return err - } - - return c.copyContentToContainer(ctx, destPath, content, 0755) - }) -} - -// copyContentToContainer copies byte content to a file in the container -func (c *Container) copyContentToContainer(ctx context.Context, destPath string, content []byte, mode int64) error { - // Create tar archive with the content - var buf bytes.Buffer - tw := tar.NewWriter(&buf) - - header := &tar.Header{ - Name: filepath.Base(destPath), - Mode: mode, - Size: int64(len(content)), - } - - if err := tw.WriteHeader(header); err != nil { - return err - } - - if _, err := tw.Write(content); err != nil { - return err - } - - if err := tw.Close(); err != nil { - return err - } - - // Copy to container - return c.client.CopyToContainer(ctx, c.containerID, filepath.Dir(destPath), &buf, container.CopyToContainerOptions{}) -} - -// ConfigureMockGitHubPath configures PATH to use mock gh binary in container -func (c *Container) ConfigureMockGitHubPath(ctx context.Context) error { - // Add mock skill directory to PATH so 'gh' resolves to our mock - return c.Exec(ctx, []string{"bash", "-c", "echo 'export PATH=/workspace/.kiro/skills/github-cli:$PATH' >> /home/sandbox/.bashrc"}) -} - // GitHubMockResponse represents a mock GitHub API response type GitHubMockResponse struct { IssueNumber int `json:"issue_number,omitempty"`