fix(apps): write Deno runtime config to temp directory#41338
fix(apps): write Deno runtime config to temp directory#41338AlgoArtist06 wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: cfb2530 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Resolve ephemeral runtime imports packages/apps/src/server/runtime/deno/AppsEngineDenoRuntime.ts |
Relative import targets are resolved against the static configuration directory and normalized, while non-relative targets remain unchanged. |
Isolate and clean runtime artifacts packages/apps/src/server/runtime/deno/AppsEngineDenoRuntime.ts |
Each controller uses a unique temporary runtime directory for Deno artifacts and removes it after failures or stopApp(). |
Validate runtime cleanup and release metadata packages/apps/tests/server/runtime/DenoRuntimeSubprocessController.test.ts, .changeset/tidy-snails-smile.md |
Tests verify generated configuration contents, isolated directories, and cleanup; the changeset records a patch release and non-root startup fix. |
Estimated code review effort: 2 (Simple) | ~10 minutes
Suggested labels: type: bug
Suggested reviewers: d-gubert, ggazzo
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly and concisely describes the main change: moving the Deno runtime config write location to a temp directory. |
| Linked Issues check | ✅ Passed | The changes address issue #41006 by writing the ephemeral Deno config to a writable temp directory so non-root containers can start apps. |
| Out of Scope Changes check | ✅ Passed | The path resolution and cleanup updates are supporting implementation details for the same Deno config write-location fix, with no unrelated changes apparent. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
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 @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
The implementation and regression coverage have been locally verified ( |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #41338 +/- ##
===========================================
- Coverage 69.04% 69.02% -0.02%
===========================================
Files 3757 3757
Lines 147735 147735
Branches 26395 26355 -40
===========================================
- Hits 102000 101973 -27
- Misses 41234 41262 +28
+ Partials 4501 4500 -1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Summary
Closes #41006
Testing
../../node_modules/.bin/tsc -p tsconfig.json --noEmit../../node_modules/.bin/eslint src/server/runtime/deno/AppsEngineDenoRuntime.ts tests/server/runtime/DenoRuntimeSubprocessController.test.tsThe full Deno runtime integration test could not complete locally because the environment has Deno 2.9.2 while the repository pins Deno 2.3.1; its shared startup hook timed out before the regression assertions ran.
Summary by CodeRabbit