fix(test): use --experimental-strip-types in Worker execArgv

[carlos-alm]

Summary

• Snapshot concurrent-save tests passed --strip-types to Worker execArgv on Node ≥ 23, but Node 24 removed it from the Worker allowlist — both workers exited with ERR_WORKER_INVALID_EXEC_ARGV before posting a message, so Promise.allSettled returned two rejections and the two race tests failed locally on main.
• Switch unconditionally to --experimental-strip-types, which is accepted by Worker execArgv on every supported Node version (≥ 22.6) and is a harmless no-op on Node ≥ 23.6 where type stripping is default-on.

Test plan

• npx vitest run tests/unit/snapshot.test.ts — 29/29 pass locally on Node 24.10
• Re-ran 5× to check for flakiness — all green
• npx biome check tests/unit/snapshot.test.ts — clean

Closes #1117

[claude]

Claude finished @carlos-alm's task —— View job

---

PR Review Complete

Reviewing PR #1164: fix(test): use --experimental-strip-types in Worker execArgv

Todo List:

• Read and analyze the modified test file
• Phase 1: Root Cause Analysis
• Phase 2: Configuration Change Detection
• Phase 3: Test Integrity Check
• Phase 4: Code Quality Review
• Phase 5: Backlog Compliance Assessment
• Provide final rating and recommendation

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Node.js 24 removed the short-lived --strip-types flag from the Worker execArgv allowlist (flag was briefly available in Node 23). The snapshot concurrent-save tests were conditionally using --strip-types for Node ≥ 23, which caused ERR_WORKER_INVALID_EXEC_ARGV on Node 24, making both workers exit before posting messages and causing test failures.

Why This Approach: Switching to --experimental-strip-types unconditionally is justified because:

• It's accepted by Worker execArgv across all supported Node versions (≥ 22.6)
• It's a harmless no-op on Node ≥ 23.6 where type stripping is default-on
• The codebase already uses this pattern elsewhere (see vitest.config.ts, CI workflows, scripts)

Risk Assessment: LOW RISK - This is a legitimate bug fix addressing a Node.js API compatibility issue, not working around a deeper problem.

Phase 2: Configuration Change Detection ✅

NO HIGH-RISK CHANGES DETECTED:

• ✅ No biome/linting rules disabled or relaxed
• ✅ No test timeouts increased or coverage thresholds lowered
• ✅ No build configuration or module resolution modified
• ✅ No TypeScript strict mode flags disabled

The change is purely fixing a Node.js flag compatibility issue in test setup, not silencing any warnings or relaxing quality controls.

Phase 3: Test Integrity Check ✅

NO RED FLAGS DETECTED:

• ✅ No assertions removed or weakened
• ✅ No tests made less strict
• ✅ Test logic and validation remain identical
• ✅ Only the Node.js flag configuration was changed
• ✅ No eslint-disable or workaround comments added

The test behavior is preserved - only the Worker execArgv setup was fixed to work across Node versions. The race condition tests at lines 182-241 maintain their full concurrency validation.

Phase 4: Code Quality Review ✅

TECHNICAL QUALITY: The changes are clean and well-implemented:

• ✅ Simplified logic: Removed unnecessary Node version detection code (lines 152-157 in old version)
• ✅ Comprehensive documentation: Added excellent inline comment explaining the Node.js flag behavior across versions (lines 157-161)
• ✅ Consistency: Aligns with existing patterns used throughout the codebase (vitest.config.ts, CI workflows)
• ✅ No performance impact: Identical runtime behavior, just different flag name
• ✅ No security concerns: Worker execArgv flag change only

POTENTIAL IMPROVEMENTS: None identified - the implementation is appropriate and follows established patterns.

Phase 5: Backlog Compliance Assessment ✅

Root Cause Analysis

Problem Being Solved: Node.js 24 compatibility issue where Worker execArgv rejected --strip-types flag, causing snapshot race condition tests to fail
Why This Approach: Use --experimental-strip-types which is accepted across all supported Node versions and is consistent with existing codebase patterns
Risk Assessment: Low risk - legitimate platform compatibility fix, not masking deeper issues

Backlog Compliance

• Zero-dep: ✓ — No new dependencies, only flag name change
• Foundation-aligned: ✓ — Maintains zero-infrastructure principle, no violations detected
• Problem-fit: 3 — Useful platform compatibility fix, doesn't directly address core "lost AI" problem but ensures tests run reliably across Node versions
• Breaking: No — Pure internal test change, no API or behavior impact
• Tier: 1

Critical Concerns

None identified. This is a straightforward, well-documented platform compatibility fix.

Final Recommendation

• Rating: ⭐⭐⭐⭐☆ (4/5)
• Action: APPROVE
• Reasoning: Clean implementation of a legitimate Node.js compatibility fix with excellent documentation. Simplifies code while maintaining identical functionality. Follows established codebase patterns. No test integrity or quality concerns.

Minor observation: The fix addresses a recurring pattern in this codebase (Node.js type stripping flag compatibility) - this same issue was resolved in other areas (CI workflows, vitest config, scripts). The PR properly aligns the test infrastructure with the established solution.

---

[greptile-apps]

Greptile Summary

This PR fixes two concurrent-save race tests that were failing on Node 24 because --strip-types (the unprefixed flag used for Node ≥ 23) is no longer in the Worker execArgv allowlist and caused ERR_WORKER_INVALID_EXEC_ARGV. The fix replaces the version-conditional flag selection with the unconditional --experimental-strip-types, which is accepted across all supported Node versions (≥ 22.6) and is a no-op on Node ≥ 23.6 where type-stripping is on by default.

• Removes the nodeMajor version check and the conditional flag array, simplifying the Worker setup to a single constant.
• Adds an inline comment explaining why the prefixed flag is used and why the unprefixed one can no longer be used in Workers.

Confidence Score: 5/5

Safe to merge — the change is a minimal, well-scoped fix to a test-only flag that was breaking Worker startup on Node 24.

The fix replaces a version-conditional execArgv flag with a single unconditional flag that is documented to be valid across all supported Node versions. No production code is touched, and the PR description notes the test suite passed 5 consecutive runs on Node 24.10.

No files require special attention.

Important Files Changed

Filename	Overview
tests/unit/snapshot.test.ts	Removes the Node.js major-version branch and unconditionally passes --experimental-strip-types to Worker execArgv, fixing ERR_WORKER_INVALID_EXEC_ARGV on Node 24 where --strip-types was removed from the Worker allowlist.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Test: spawn race workers] --> B{Before this PR}
    B --> C{nodeMajor >= 23?}
    C -- Yes --> D["execArgv: --strip-types"]
    C -- No --> E["execArgv: --experimental-strip-types"]
    D -- Node 24 --> F["ERR_WORKER_INVALID_EXEC_ARGV"]
    F --> G[Worker exits before posting message]
    G --> H[Test FAIL]

    A --> I{After this PR}
    I --> J["execArgv: --experimental-strip-types"]
    J --> K[Worker starts successfully]
    K --> L[Test PASS]

Loading

_{Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/1117-snapsh..." | Re-trigger Greptile}