Skip to content

Move evaluation file setup from runtime to build-time in Dockerfile - #194

Open
jbrinkman wants to merge 1 commit into
mainfrom
spec/issue-192-77414
Open

Move evaluation file setup from runtime to build-time in Dockerfile#194
jbrinkman wants to merge 1 commit into
mainfrom
spec/issue-192-77414

Conversation

@jbrinkman

Copy link
Copy Markdown
Owner

Summary

This PR transforms the evaluation framework to embed all required files at Docker build time instead of performing runtime file operations, eliminating permission errors and improving performance.

Problem Solved

  • Permission Errors: Runtime file operations were failing with "Permission denied" errors in containers
  • Runtime Complexity: SetupGitHubMocking() and ConfigureMockGitHubPath() functions performed unnecessary file copying at runtime
  • Performance Impact: Each container startup required file setup operations
  • Reliability Issues: Runtime operations could fail unpredictably

Changes Made

Key Files Modified/Created:

  • internal/eval/sandbox/build_context.go (new) - Build context preparation logic
  • internal/eval/sandbox/container.go - Updated Dockerfile generation with COPY commands
  • internal/eval/sandbox/mock_github.go - Removed runtime setup functions
  • internal/eval/runner.go - Removed runtime setup calls
  • internal/eval/dockerfile/base.Dockerfile - Added directory creation
  • internal/eval/sandbox/installation_test.go - Updated tests for build-time approach
  • internal/eval/sandbox/image_manager.go - Fixed method references

Architecture Changes:

  1. Build-time File Embedding: All files (kiro-krew binary, agent configs, GitHub CLI mocks, evaluation files) now embedded in Docker images during build
  2. Runtime Function Removal: Completely removed SetupGitHubMocking() and ConfigureMockGitHubPath() functions
  3. Build Context System: New BuildContext struct manages file preparation for Docker builds
  4. Dockerfile Generation: Updated to include proper COPY commands with ownership and PATH configuration

Testing

QA Results:

  • Formatting: PASS (task fmt:check)
  • Linting: PASS (task lint)
  • Build: PASS (task build)
  • Core Tests: PASS (Docker tests appropriately skip when daemon unavailable)

Validation:

  • All 8 tasks from architect specification completed successfully
  • Build-time COPY commands verified in generated Dockerfiles
  • Runtime functions properly removed from codebase
  • File embedding and build context preparation working correctly
  • Test suite updated to reflect build-time architecture

Benefits

  • 🚀 Faster Startup: Containers start immediately without runtime setup
  • 🔒 Immutable Containers: All required files baked into image at build time
  • Zero Permission Errors: No runtime file operations to fail
  • 📦 Binary Distribution Compatible: Works with standalone kiro-krew binary
  • 🐳 Docker Best Practices: Proper layering, caching, and security

Closes #192

@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a80400b5-47d9-479e-8641-6690daed1c35

📥 Commits

Reviewing files that changed from the base of the PR and between 5580958 and 488d52d.

📒 Files selected for processing (8)
  • .kiro-krew/specs/issue-192-move-evaluation-file-setup-to-build-time.md
  • internal/eval/dockerfile/base.Dockerfile
  • internal/eval/runner.go
  • internal/eval/sandbox/build_context.go
  • internal/eval/sandbox/container.go
  • internal/eval/sandbox/image_manager.go
  • internal/eval/sandbox/installation_test.go
  • internal/eval/sandbox/mock_github.go
💤 Files with no reviewable changes (1)
  • internal/eval/sandbox/mock_github.go
👮 Files not reviewed due to content moderation or server errors (7)
  • internal/eval/dockerfile/base.Dockerfile
  • internal/eval/sandbox/installation_test.go
  • internal/eval/sandbox/container.go
  • .kiro-krew/specs/issue-192-move-evaluation-file-setup-to-build-time.md
  • internal/eval/runner.go
  • internal/eval/sandbox/image_manager.go
  • internal/eval/sandbox/build_context.go

📝 Walkthrough

[!WARNING]

Walkthrough skipped

File diffs could not be summarized.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch spec/issue-192-77414

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move evaluation file setup from runtime to build-time in Dockerfile

1 participant