Skip to content

Fix specialization double-start race - #841

Open
Swapnil Nagar (swapnil-nagar) wants to merge 1 commit into
v3.xfrom
swapnil-nagar-fix-specialization-race
Open

Fix specialization double-start race#841
Swapnil Nagar (swapnil-nagar) wants to merge 1 commit into
v3.xfrom
swapnil-nagar-fix-specialization-race

Conversation

@swapnil-nagar

Copy link
Copy Markdown
Contributor

A specialization race can send WorkerInitRequest and FunctionEnvironmentReloadRequest for the same real app. Resetting app state before the second startup loses v4 registrations, while Node's module cache prevents their side effects from running again, leaving the worker on the default v3 model with no loadable functions.

This change preserves the initialized AppContext when both requests target the same directory while still applying the specialized environment and working directory. Normal placeholder specialization continues to reset and start the real app when its directory differs. Entry-point globbing also excludes node_modules so broad main patterns cannot load dependencies as application entry points.

Tests cover v4 model and function-registration preservation across the duplicate lifecycle sequence, normal placeholder specialization, and node_modules glob exclusion.

Fixes #838

Preserve initialized app state when worker init and environment reload target the same directory, and exclude node_modules from entry-point globs.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: eff15798-612c-4121-a6d4-ec5e093bff1f

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes duplicate app startup during specialization while excluding dependency files from entry-point globbing.

Changes:

  • Preserves app registrations for same-directory reloads.
  • Excludes node_modules from entry-point globs.
  • Adds specialization and glob-exclusion tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/eventHandlers/FunctionEnvironmentReloadHandler.ts Preserves same-directory app state.
src/startApp.ts Excludes dependency files from globbing.
test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts Tests specialization behavior.
test/eventHandlers/WorkerInitHandler.test.ts Tests glob exclusion.
test/eventHandlers/testApp/src/registerV4Function.js Adds v4 registration fixture.
test/eventHandlers/testApp/src/dependencyEntry.js Adds glob-matching fixture.
Suppressed comments (1)

src/eventHandlers/FunctionEnvironmentReloadHandler.ts:80

  • For this same-directory specialization path, all entry-point code and appStart hooks have already run before lines 67–69 install the specialized environment. Skipping startApp() therefore leaves any configuration captured from process.env initialized with placeholder values, even though the reload response succeeds. The lifecycle needs to defer the first app start until specialization, or perform a cache-safe restart that preserves/rebuilds registrations while running startup against the new environment.
            if (!functionAppDirectoryUnchanged) {
                await startApp(msg.functionAppDirectory);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +30 to +33
const functionAppDirectoryUnchanged =
!!worker.app.functionAppDirectory &&
!!msg.functionAppDirectory &&
isPathEqual(worker.app.functionAppDirectory, msg.functionAppDirectory);
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.

Specialization race: double startApp() with stale require cache leaves worker on default v3 model with all functions unloadable

2 participants