Skip to content

fix: clean up main abort listeners#590

Closed
JustYannicc wants to merge 115 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/perf-wave4-main-abort-listeners
Closed

fix: clean up main abort listeners#590
JustYannicc wants to merge 115 commits into
SuperCmdLabs:mainfrom
JustYannicc:codex/perf-wave4-main-abort-listeners

Conversation

@JustYannicc

Copy link
Copy Markdown
Collaborator

What changed

  • Added idempotent cleanup/finalize paths for src/main/ai-provider.ts HTTP requests and Whisper uploads so tracked abort listeners are removed on success, HTTP error, request error, and explicit abort.
  • Kept streaming cancellation semantics for 2xx AI responses by retaining the HTTP abort listener until the response stream ends/closes/errors.
  • Updated the ollama-pull main-process handler to use a named abort listener and centralized finalizer for request-map and listener cleanup.
  • Extended provider/Ollama tests with tracked AbortSignal add/remove counts and a narrow main-process Ollama source guard.

Why

Long-running AI and Ollama paths reused caller-owned abort signals while lower-level helpers attached inline one-shot listeners. Successful and failed requests could leave listener closures attached until the caller signal was garbage collected, and abort/error races had no shared cleanup path.

Compatibility impact

Behavior is intended to be preserving: cancellation still uses the existing Request aborted and Transcription aborted errors, HTTP error messages are unchanged, and Ollama aborts still suppress user-facing pull errors when the controller is aborted.

How tested

  • node --test scripts/test-ai-provider-gemini-prompt-streaming.mjs
  • node --test scripts/test-ai-provider-whisper-upload.mjs
  • node --test scripts/test-ollama-pull-listener-cleanup.mjs
  • pnpm run build:main
  • pnpm run typecheck:renderer
  • Codex LSP diagnostics on src/main/ai-provider.ts and src/main/main.ts: no diagnostics found.

Performance evidence

  • HTTP helper before: successful, HTTP-error, and request-error requests retained one abort listener on the caller signal. After: tracked counts balance to adds=1 removes=1 active=0 for success, HTTP error, request error, and abort; during a successful streaming response the listener remains active after headers (adds=1 removes=0 active=1) and is removed when the stream completes (adds=1 removes=1 active=0).
  • Whisper upload before: success/error/abort paths had no shared removal path for the upload abort listener. After: success, HTTP error, request error, and mid-flight abort each end at adds=1 removes=1 active=0; pre-aborted requests remain adds=0 removes=0 active=0.
  • Ollama pull renderer baseline remains documented by the existing test: before remount cleanup listener counts were progress=6 done=6 error=6; after cleanup they are progress=0 done=0 error=0. The new main-process guard verifies ollama-pull uses a named onAbort, calls removeEventListener, and centralizes active request deletion in finishPullRequest().

Stack validation

  • Started from 206eab8280a2eb91a57e4e74ea6df99e45a55cb4 on codex/perf-wave3-integration-stack-20260704 lineage.
  • Branch: codex/perf-wave4-main-abort-listeners.
  • No coordinator markdown, playbooks, prompts, or generated lockfiles are included.

Replaces

None.

…perf-wave2-integration-stack-20260704

# Conflicts:
#	src/renderer/src/raycast-api/hooks/use-cached-promise.ts
…perf-wave2-integration-stack-20260704

# Conflicts:
#	src/renderer/src/raycast-api/list-runtime-renderers.tsx
#	src/renderer/src/raycast-api/list-runtime.tsx
…perf-wave2-integration-stack-20260704

# Conflicts:
#	src/renderer/src/CameraExtension.tsx
…perf-wave2-integration-stack-20260704

# Conflicts:
#	src/renderer/src/raycast-api/list-runtime.tsx
…perf-wave2-integration-stack-20260704

# Conflicts:
#	scripts/bench-script-command-discovery.mjs
#	scripts/benchmark-file-search-delete-batch.mjs
#	scripts/lib/script-command-runner-harness.mjs
#	scripts/lib/ts-import.mjs
#	scripts/test-file-search-delete-batch.mjs
#	scripts/test-script-command-runner.mjs
#	src/main/file-search-index.ts
#	src/main/main.ts
…perf-wave2-integration-stack-20260704

# Conflicts:
#	scripts/lib/script-command-runner-harness.mjs
#	scripts/lib/ts-import.mjs
#	scripts/test-extension-lifecycle-sandbox.mjs
#	scripts/test-script-command-runner.mjs
#	src/main/main.ts
#	src/renderer/src/ExtensionView.tsx
…perf-wave2-integration-stack-20260704

# Conflicts:
#	.github/workflows/project-checks.yml
#	scripts/test-root-search-perf.mjs
#	src/renderer/src/CameraExtension.tsx
#	src/renderer/src/raycast-api/list-runtime.tsx
…erf-wave3-integration-stack-20260704

# Conflicts:
#	package.json
#	scripts/measure-extension-bundle-cache.mjs
#	scripts/test-icon-runtime-file-icon-cache.mjs
#	src/main/extension-runner.ts
#	src/renderer/src/components/HiddenExtensionRunners.tsx
#	src/renderer/src/raycast-api/icon-runtime-render.tsx
…/perf-wave3-integration-stack-20260704

# Conflicts:
#	scripts/test-browser-search-performance.mjs
#	scripts/test-root-search-perf.mjs
@JustYannicc

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #601, which fully replaces this granular PR in the wave4 runtime/main consolidation. Literal validation now passes with /opt/homebrew/opt/node@22/bin/npm run build:main, and #601 is ready for review.

@JustYannicc JustYannicc closed this Jul 5, 2026
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