Skip to content

feat: add build cache support#233

Open
TorstenDittmann wants to merge 8 commits into
mainfrom
feat/node-modules-cache
Open

feat: add build cache support#233
TorstenDittmann wants to merge 8 commits into
mainfrom
feat/node-modules-cache

Conversation

@TorstenDittmann
Copy link
Copy Markdown
Contributor

@TorstenDittmann TorstenDittmann commented Jun 1, 2026

Summary

  • add optional cacheKey for build cache reuse
  • mount a shared Docker volume at build time and scope cache entries by key
  • configure package-manager caches for npm, Yarn, pnpm, and Bun under /cache/
  • add e2e coverage for build cache reuse and executable build output
  • document cacheKey and OPR_EXECUTOR_BUILD_CACHE_VOLUME

Testing

  • php -l app/controllers.php
  • php -l src/Executor/Runner/Adapter.php
  • php -l src/Executor/Runner/Docker.php
  • php -l tests/e2e/ExecutorTest.php
  • composer format:check
  • composer refactor:check
  • composer analyze
  • composer test:unit (fails locally: Swoole\Table class is unavailable)

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 1, 2026

Greptile Summary

This PR adds optional build-cache support to the executor: a named Docker volume is mounted at /cache (scoped to the caller-supplied cacheKey subpath) during the build container's lifetime, and well-known package-manager cache env vars (npm_config_cache, YARN_CACHE_FOLDER, BUN_INSTALL_CACHE_DIR, etc.) are redirected into that volume so consecutive builds with the same key reuse downloaded dependencies.

  • app/controllers.php accepts and regex-validates the new cacheKey param before forwarding it to the runner; Docker.php adds ensureBuildCacheSubpath and getPackageManagerCacheVariables to wire the cache paths into the build container environment.
  • composer.json depends on utopia-php/orchestration: dev-feature/mount-value-object — an untagged development branch — to get the new Mount::volume subpath API; e2e tests cover both the cache-miss and cache-hit flows.

Confidence Score: 3/5

Not safe to merge until the orchestration library is released as a tagged version; the dev-branch constraint in composer.json is the blocking concern.

The core cache logic is straightforward and cacheKey validation is solid, but the feature depends on utopia-php/orchestration pinned to an untagged dev branch. Any force-push or rebase before a stable tag is cut will break composer install in CI and production deployments that regenerate the lockfile.

composer.json — the dev-feature/mount-value-object orchestration constraint must be replaced with a tagged release before merging.

Important Files Changed

Filename Overview
composer.json Pins utopia-php/orchestration to an unstable dev branch rather than a tagged release, which is a deployment stability risk.
src/Executor/Runner/Docker.php Adds cache volume mounting, subpath init via helper container, and package-manager env var injection; one spurious pnpm_config_store_dir env var entry is ineffective.
app/controllers.php Adds cacheKey param with proper regex validation and forwards it to the runner; validation logic and call site look correct.
src/Executor/Runner/Adapter.php Adds optional cacheKey parameter to createRuntime signature with a safe default; no issues.
tests/e2e/ExecutorTest.php Adds cache-key validation test and a full cache-miss to cache-hit to execution e2e test; coverage looks appropriate.
composer.lock Locks the dev-branch orchestration commit and bumps console/validators; reproducible installs are fine today but tied to the unstable branch risk in composer.json.

Reviews (8): Last reviewed commit: "fix: isolate build cache by volume subpa..." | Re-trigger Greptile

Comment thread src/Executor/Runner/Docker.php Outdated
Comment thread src/Executor/Runner/Docker.php Outdated
Comment thread src/Executor/Runner/Docker.php Outdated
Comment thread src/Executor/Runner/Docker.php Outdated
@TorstenDittmann TorstenDittmann changed the title feat: add node_modules build cache feat: add build cache support Jun 1, 2026
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.

1 participant