Skip to content

fix(node): fallback auto backend mode in headless runs#293

Merged
RtlZeroMemory merged 1 commit intomainfrom
codex/fix-node-auto-execution-mode
Mar 18, 2026
Merged

fix(node): fallback auto backend mode in headless runs#293
RtlZeroMemory merged 1 commit intomainfrom
codex/fix-node-auto-execution-mode

Conversation

@RtlZeroMemory
Copy link
Copy Markdown
Owner

@RtlZeroMemory RtlZeroMemory commented Mar 17, 2026

Summary

  • make executionMode: "auto" fall back to inline when worker mode is unavailable in headless runs
  • share the worker-environment support check between selection and worker startup so empty nativeShimModule strings are rejected
  • update the Node backend docs to match the actual auto-mode behavior

Testing

  • npm run build
  • npm run lint
  • npm run typecheck
  • node scripts/run-tests.mjs --filter "config_guards"
  • node scripts/run-tests.mjs --filter "worker_integration"
  • node scripts/run-tests.mjs

Evidence

  • selectNodeBackendExecutionMode({ requestedExecutionMode: "auto", fpsCap: 60, hasAnyTty: false }) now resolves to worker but selects inline with a fallback reason
  • explicit executionMode: "worker" still stays on worker and does not silently downgrade
  • worker environment checks now treat nativeShimModule: "" as unsupported

Summary by CodeRabbit

  • Bug Fixes

    • Node backend auto execution mode now intelligently falls back to inline when worker prerequisites are unavailable, improving reliability in headless environments.
  • Tests

    • Added tests for auto-mode fallback behavior and worker environment support validation.

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 17, 2026

📝 Walkthrough

Walkthrough

The PR enhances the Node backend's auto execution mode to implement a fallback mechanism: when worker mode would be selected but environment prerequisites (TTY availability or nativeShimModule presence) are unmet, it falls back to inline mode. A new hasWorkerEnvironmentSupport function validates these prerequisites across runtime and test code.

Changes

Cohort / File(s) Summary
Documentation Updates
CHANGELOG.md, docs/architecture/index.md, docs/backend/node.md, docs/backend/worker-model.md, docs/packages/node.md
Consistent updates to auto execution mode descriptions, adding explicit fallback conditions: inline when fpsCap ≤ 30; otherwise prefer worker and fall back to inline when no TTY or nativeShimModule is available.
Auto Mode Implementation
packages/node/src/backend/nodeBackend/executionMode.ts
Introduces hasWorkerEnvironmentSupport() helper function that checks for TTY or non-empty nativeShimModule; modifies selectNodeBackendExecutionMode() to use this helper for auto mode fallback logic and updates assertWorkerEnvironmentSupported() to leverage the new helper.
Configuration Documentation
packages/node/src/backend/nodeBackend/shared.ts
Updates JSDoc comment for NodeBackendConfig.executionMode to reflect the new auto mode fallback behavior.
Test Coverage
packages/node/src/__tests__/config_guards.test.ts
Adds comprehensive tests for auto mode fallback behavior, explicit worker mode behavior, and hasWorkerEnvironmentSupport() validation with multiple input scenarios (undefined, empty string, mock shim, native shim enabled).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Fix native worker-thread teardown crash #254: Modifies Node backend worker execution-mode logic and test assertions with overlapping changes to executionMode and config_guards logic, though with opposing directionality regarding native-worker fallback behavior.

Poem

🐰 Hops through the modes with newfound grace,
Auto now checks the TTY's face!
When worker can't dance, inline takes the stage,
A fuzzy friend's code-review page! 🌙

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 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: fixing auto execution mode to fall back to inline in headless runs, which is the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-node-auto-execution-mode
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
packages/node/src/backend/nodeBackend/executionMode.ts (1)

40-45: Treat whitespace-only nativeShimModule values as unsupported.

Right now " " is considered supported and can bypass inline fallback in auto mode even though it is effectively unusable.

♻️ Proposed hardening
 export function hasWorkerEnvironmentSupport(
   nativeShimModule: string | undefined,
   hasAnyTty: boolean,
 ): boolean {
-  return hasAnyTty || (typeof nativeShimModule === "string" && nativeShimModule.length > 0);
+  return hasAnyTty || (typeof nativeShimModule === "string" && nativeShimModule.trim().length > 0);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/node/src/backend/nodeBackend/executionMode.ts` around lines 40 - 45,
The helper hasWorkerEnvironmentSupport currently treats whitespace-only
nativeShimModule values like "   " as valid; update the check in
hasWorkerEnvironmentSupport to treat a string of only whitespace as unsupported
by verifying trimmed length (e.g., typeof nativeShimModule === "string" &&
nativeShimModule.trim().length > 0) so only non-empty, non-whitespace module
names enable worker environment support while preserving the hasAnyTty branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/node/src/backend/nodeBackend/executionMode.ts`:
- Around line 40-45: The helper hasWorkerEnvironmentSupport currently treats
whitespace-only nativeShimModule values like "   " as valid; update the check in
hasWorkerEnvironmentSupport to treat a string of only whitespace as unsupported
by verifying trimmed length (e.g., typeof nativeShimModule === "string" &&
nativeShimModule.trim().length > 0) so only non-empty, non-whitespace module
names enable worker environment support while preserving the hasAnyTty branch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0d069eab-d3b6-44a5-9384-1f5be3f8fdba

📥 Commits

Reviewing files that changed from the base of the PR and between 2980659 and 8e93379.

📒 Files selected for processing (8)
  • CHANGELOG.md
  • docs/architecture/index.md
  • docs/backend/node.md
  • docs/backend/worker-model.md
  • docs/packages/node.md
  • packages/node/src/__tests__/config_guards.test.ts
  • packages/node/src/backend/nodeBackend/executionMode.ts
  • packages/node/src/backend/nodeBackend/shared.ts

@RtlZeroMemory RtlZeroMemory merged commit dead50c into main Mar 18, 2026
32 checks passed
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