Skip to content

Clear app executor timeout timers#1419

Open
morluto wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
morluto:fix/app-executor-timeout-cleanup
Open

Clear app executor timeout timers#1419
morluto wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
morluto:fix/app-executor-timeout-cleanup

Conversation

@morluto

@morluto morluto commented Jul 15, 2026

Copy link
Copy Markdown

Fixes #1413

Summary

Completed app tool invocations now clear their losing timeout timers instead of retaining them until the configured deadline.

The change applies to the in-process executor and the emitted workerd and dynamic Worker drivers.

What changed

  • Wrap each timeout race in try/finally and clear the timer after the race settles.
  • Apply the same cleanup to the emitted workerd and dynamic Worker driver modules.
  • Bump DRIVER_VERSION from 1 to 2 so Worker Loader does not reuse an isolate containing the previous generated driver.
  • Add a process-liveness regression for the in-process executor.
  • Add worker-pool coverage that invokes a completed tool through the generated dynamic Worker driver.
  • Add an executor patch changeset.

The configured timeout, timeout error message, and app tool result contract are unchanged.

Regression proof

The regression starts a child process that completes an app tool invocation immediately with a 30-second timeout.

Before the fix, the child printed invocation complete but remained alive past the test's five-second process deadline because the losing timer was still active.

After the fix, the child exits successfully within that deadline.

Validation

From packages/plugins/apps:

  • bun run test -- src/executor/app-tool-executor.test.ts src/executor/app-tool-executor.conformance.test.ts: 2 files and 19 tests passed.
  • bunx vitest run --config vitest.worker.config.ts src/executor/dynamic-worker-app-tool-executor.worker.test.ts: 1 file and 1 test passed.
  • bun run typecheck: passed with unrelated existing Effect suggestions.

Additional validation:

  • bun run bootstrap passed.
  • Targeted lint and formatting checks passed.
  • git diff --check passed.
  • Branch autoreview against origin/main returned no actionable findings.

Limitations

The process-liveness regression directly proves timer release in the in-process Node executor. The workerd conformance tests and dynamic Worker pool test prove that the changed emitted drivers still invoke tools successfully, but those runtimes do not expose pending timer handles for the same direct assertion.

This change does not cancel the underlying tool invocation after a timeout. It only releases the losing timeout when the invocation settles first.

Review order

  1. The process fixture and process-liveness regression.
  2. The canonical in-process timer cleanup.
  3. Generated workerd and dynamic Worker driver parity, including cache invalidation.
  4. Dynamic Worker pool coverage.

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.

App tool invocations retain timeout timers after completion

1 participant