[APPS-2792] Harden local Node execution: signal reporting, shutdown cleanup, edge-case coverage - #471
Closed
tyffical wants to merge 1 commit into
Conversation
tyffical
added a commit
that referenced
this pull request
Jul 28, 2026
- npm run dev's /__dd/executeAction now runs backend functions via executeScriptLocally (a real forked child process) instead of the cloud preview-async round trip, per the design doc's Rollout section: no customer-facing toggle, local execution is the new unconditional default. - Preserves today's cloud round trip as a new, additive /__dd/executeActionViaCloud endpoint rather than deleting it -- no `npm run dev:verify` script exists yet to replace it (that requires create-apps scaffolding template changes, outside this package), and the existing dev-server.test.ts suite has extensive coverage of the cloud path that would otherwise be silently orphaned. - /__dd/executeAction no longer requires Datadog credentials to be configured, unlike /__dd/executeActionViaCloud -- local execution's $.Actions calls are stubbed today (see local-execution.ts), so there's nothing for it to authenticate yet. - Wires killAllLocalExecutionChildren() (from #471) into the dev server's own shutdown via server.httpServer's 'close' event, so killing the dev server mid-execution doesn't leave an orphaned child process. - Registers local-exec-child.js in the package's buildPlugin.toBuild config (same mechanism apps-runtime.ts already uses) so it's actually copied into the published dist/src output next to the compiled bundle. local-execution.ts resolves it at runtime via a __dirname-relative path, which only works if the file exists on disk next to the bundle -- found by running a real npm-linked build against a scaffolded app, since the existing jest tests resolve local-execution.ts's source directly and never exercise the published package layout. $.Actions calls remain stubbed until the single-action execution endpoint resolves (design doc's Open Dependency section) -- this is explicitly not customer-ready yet, same as the rest of this PR stack. Milestone 2 of APPS-2792's local Node execution kickoff plan.
… coverage - Report the OS signal (SIGSEGV/SIGKILL/etc.) when a child is killed rather than exiting normally, instead of an opaque "exited with code null" -- distinguishes an OOM kill or native-module crash from a plain crash. - Track live children in a Set; export killAllLocalExecutionChildren() for the dev server to call on its own shutdown so executions in flight don't leave orphaned Node processes behind. - Add regression coverage for behavior that already worked but had no test: non-serializable return values (circular references) produce a clean rejection instead of hanging, and concurrent $.Actions calls within a single execution resolve to their own correct results. Milestone 1 of APPS-2792's local Node execution kickoff plan.
tyffical
force-pushed
the
tiffany.trinh/apps-2792-harden-local-execution
branch
from
July 29, 2026 18:12
4ac0179 to
780b7c7
Compare
tyffical
added a commit
that referenced
this pull request
Jul 29, 2026
- npm run dev's /__dd/executeAction now runs backend functions via executeScriptLocally (a real forked child process) instead of the cloud preview-async round trip, per the design doc's Rollout section: no customer-facing toggle, local execution is the new unconditional default. - Preserves today's cloud round trip as a new, additive /__dd/executeActionViaCloud endpoint rather than deleting it -- no `npm run dev:verify` script exists yet to replace it (that requires create-apps scaffolding template changes, outside this package), and the existing dev-server.test.ts suite has extensive coverage of the cloud path that would otherwise be silently orphaned. - /__dd/executeAction no longer requires Datadog credentials to be configured, unlike /__dd/executeActionViaCloud -- local execution's $.Actions calls are stubbed today (see local-execution.ts), so there's nothing for it to authenticate yet. - Wires killAllLocalExecutionChildren() (from #471) into the dev server's own shutdown via server.httpServer's 'close' event, so killing the dev server mid-execution doesn't leave an orphaned child process. - Registers local-exec-child.js in the package's buildPlugin.toBuild config (same mechanism apps-runtime.ts already uses) so it's actually copied into the published dist/src output next to the compiled bundle. local-execution.ts resolves it at runtime via a __dirname-relative path, which only works if the file exists on disk next to the bundle -- found by running a real npm-linked build against a scaffolded app, since the existing jest tests resolve local-execution.ts's source directly and never exercise the published package layout. $.Actions calls remain stubbed until the single-action execution endpoint resolves (design doc's Open Dependency section) -- this is explicitly not customer-ready yet, same as the rest of this PR stack. Milestone 2 of APPS-2792's local Node execution kickoff plan.
Contributor
Author
|
Superseded: a team sync (2026-08-04, Terrapin) decided to drop This PR's core work (error-edge-case coverage, shutdown cleanup, secrets posture — all for a forked child process) doesn't apply to the new in-process model. Likely salvageable for the reimplementation: the $.Actions Proxy's shape (nested-property-path walk → {fqn, inputs, connectionId}) — it still needs to exist, just as a direct in-process call instead of an IPC round trip. Closing, not deleting the branch, in case anything needs to be referenced later. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Changes
SIGSEGV,SIGKILL) when a child is killed rather than exiting normally, instead of an opaque "exited with code null"packages/plugins/apps/src/vite/local-execution.tsSet; exportkillAllLocalExecutionChildren()for the dev server to call on its own shutdown so in-flight executions don't leave orphaned Node processespackages/plugins/apps/src/vite/local-execution.tsgetMostRecentlyForkedChildForTest()— test-only accessor to exercise real signal-based killspackages/plugins/apps/src/vite/local-execution.ts$.Actionscalls within one execution resolve to their own correct resultspackages/plugins/apps/src/vite/local-execution.integration.test.tspackages/plugins/apps/src/vite/local-execution.integration.test.tsQA Instructions
yarn eslint packages/plugins/apps/src/vite/local-execution.ts packages/plugins/apps/src/vite/local-execution.integration.test.ts # Expected: no output, clean exit ✅ VERIFIEDManual QA — not yet possible, same as #461
Same reasoning as #461, which this PR is stacked on:
createDevServerMiddlewaredoesn't call anything inlocal-execution.ts, so there's nonpm run devrequest path reaching this code yet — nothing to click through locally or on staging. Did do real local + staging manual QA on the separately-scoped #460 (ssr:true + noExternal fix), since that one is on a live path today. Manual QA for this file becomes possible once Milestone 2 wires it into the dev server.Blast Radius
local-execution.ts) is not wired intocreateDevServerMiddlewareyet — Milestone 2, a separate stacked PR — so this change has zero effect on any currently-shipping behavior.executeActionRemotelyremains a stub — real wiring is blocked on the single-action execution endpoint decision (design doc's Open Dependency section), not this PR's scope.Out of Scope / Follow-ups
handleExecuteActionintegration into realdev-server.tsDocumentation