From 968e1fa950f76d94b281049a8beb7a65475b1dde Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Wed, 22 Apr 2026 23:09:16 +0000 Subject: [PATCH 1/2] fix(init): cancel in-flight Mastra requests on teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create an `AbortController` alongside the `MastraClient` in `runWizard`, pass its signal via `abortSignal` in `ClientOptions`, and abort it from a `using` disposable so any in-flight fetches are canceled on every exit path (success, error, cancellation). Why this matters: - `MastraClient` has no `close()`/`dispose()` API. Without an explicit abort, keep-alive sockets in Bun's fetch dispatcher can hold the event loop alive past the wizard's natural exit, causing the shell to appear stuck. The original `process.exit(0)` workaround (removed in #802) papered over this symptom; this PR addresses the root cause. - Explicit cancellation means we no longer rely on Bun's fetch dispatcher to auto-unref idle sockets. Cross-runtime robust. Implementation notes: - `AbortController` is created per-`runWizard` call. Scope matches the `MastraClient`. - `using _mastraCleanup` disposable calls `abortController.abort()` on every exit path. Idempotent (signal's `aborted` flag guards against double-abort). - Custom `fetch` wrapper preserves `init.signal` via the spread — MastraClient's per-request signals still reach the underlying `fetch` call. - No `run.cancel()` — the server observes the dropped fetch connection and cancels the run server-side without an extra round-trip during teardown. Tests: - Capture `ClientOptions` from each MastraClient instance via a prototype `getWorkflow` hook that reads `this.options`. - Assert `abortSignal` is aborted after success, tool-error, and WizardCancelledError paths. - Assert the signal is forwarded live to MastraClient at construction (not pre-aborted). --- src/lib/init/wizard-runner.ts | 21 +++++ test/lib/init/wizard-runner.test.ts | 117 +++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 3 deletions(-) diff --git a/src/lib/init/wizard-runner.ts b/src/lib/init/wizard-runner.ts index 8c1f2ec00..ef6a07f2d 100644 --- a/src/lib/init/wizard-runner.ts +++ b/src/lib/init/wizard-runner.ts @@ -415,11 +415,32 @@ async function runWizardInner(initialOptions: WizardOptions): Promise { const token = context.authToken; + // AbortController bound to the MastraClient lifecycle. Aborting on + // teardown (success OR failure, via `using` below) cancels any in-flight + // fetches — releasing keep-alive sockets so the event loop drains and + // `sentry init` returns to the shell promptly. Without this, a stuck or + // idle socket in Bun's fetch dispatcher can hold the process alive past + // the wizard's natural exit. + const abortController = new AbortController(); + using _mastraCleanup = { + [Symbol.dispose]: (): void => { + // Guard against double-abort — calling abort() on an already-aborted + // signal is a no-op in Node/Bun, but explicit check documents intent. + if (!abortController.signal.aborted) { + abortController.abort(); + } + }, + }; + const client = new MastraClient({ baseUrl: MASTRA_API_URL, headers: token ? { Authorization: `Bearer ${token}` } : {}, + abortSignal: abortController.signal, fetch: ((url, init) => { const traceData = getTraceData(); + // Preserve `init.signal` via the spread — MastraClient may pass its + // own per-request signal, and the client-level `abortSignal` is + // forwarded through the same channel. return fetch(url, { ...init, headers: { diff --git a/test/lib/init/wizard-runner.test.ts b/test/lib/init/wizard-runner.test.ts index f607568db..370639e25 100644 --- a/test/lib/init/wizard-runner.test.ts +++ b/test/lib/init/wizard-runner.test.ts @@ -98,6 +98,12 @@ let precomputeSentryDetectionSpy: ReturnType; let closeFreshTtyForwardingSpy: ReturnType; let getWorkflowSpy: ReturnType; let stderrSpy: ReturnType; +/** + * ClientOptions captured from each MastraClient instance constructed by + * runWizard. Used by the MastraClient lifecycle suite to assert that the + * `abortSignal` passed at construction time is aborted on teardown. + */ +let capturedClientOptions: { abortSignal?: AbortSignal }[] = []; beforeEach(() => { mockStartResult = { status: "success", result: { platform: "React" } }; @@ -171,9 +177,18 @@ beforeEach(() => { const workflow = { createRun: mock(() => Promise.resolve(run)), }; - getWorkflowSpy = spyOn(MastraClient.prototype, "getWorkflow").mockReturnValue( - workflow as any - ); + capturedClientOptions = []; + getWorkflowSpy = spyOn( + MastraClient.prototype, + "getWorkflow" + ).mockImplementation(function (this: MastraClient) { + // `this` is the MastraClient instance. `BaseResource.options` holds the + // full ClientOptions passed to the constructor — including abortSignal. + capturedClientOptions.push( + (this as unknown as { options: { abortSignal?: AbortSignal } }).options + ); + return workflow as any; + }); }); afterEach(() => { @@ -507,3 +522,99 @@ describe("runWizard", () => { expect(spinnerMock.stop).toHaveBeenCalledWith("Using existing project"); }); }); + +describe("runWizard — MastraClient lifecycle", () => { + test("aborts the MastraClient signal after a successful run", async () => { + await runWizard(makeOptions()); + + expect(capturedClientOptions).toHaveLength(1); + const signal = capturedClientOptions[0]?.abortSignal; + expect(signal).toBeInstanceOf(AbortSignal); + // Using the non-null assertion safely — we asserted toBeInstanceOf above. + expect((signal as AbortSignal).aborted).toBe(true); + }); + + test("aborts the MastraClient signal when a tool throws", async () => { + const payload: ToolPayload = { + type: "tool", + operation: "run-commands", + cwd: "/tmp/test", + params: { commands: ["npm install @sentry/node"] }, + }; + mockStartResult = { + status: "suspended", + suspended: [["install-deps"]], + steps: { + "install-deps": { suspendPayload: payload }, + }, + }; + executeToolSpy.mockRejectedValue(new Error("tool blew up")); + + await expect(runWizard(makeOptions())).rejects.toThrow(WizardError); + + expect(capturedClientOptions).toHaveLength(1); + const signal = capturedClientOptions[0]?.abortSignal; + expect(signal).toBeInstanceOf(AbortSignal); + expect((signal as AbortSignal).aborted).toBe(true); + }); + + test("aborts the MastraClient signal on cancellation", async () => { + const payload: ToolPayload = { + type: "tool", + operation: "run-commands", + cwd: "/tmp/test", + params: { commands: ["npm install @sentry/node"] }, + }; + mockStartResult = { + status: "suspended", + suspended: [["install-deps"]], + steps: { + "install-deps": { suspendPayload: payload }, + }, + }; + executeToolSpy.mockRejectedValue(new WizardCancelledError()); + + await runWizard(makeOptions()); + + expect(capturedClientOptions).toHaveLength(1); + const signal = capturedClientOptions[0]?.abortSignal; + expect(signal).toBeInstanceOf(AbortSignal); + expect((signal as AbortSignal).aborted).toBe(true); + }); + + test("forwards a live abortSignal to MastraClient at construction", async () => { + // Before the wizard exits, the signal MUST NOT yet be aborted — the + // MastraClient is using it for in-flight fetches throughout the run. + // We observe this by checking the signal state inside the workflow + // handler (which executes while the wizard's `using` scope is active). + let signalDuringRun: AbortSignal | undefined; + startAsyncMock = mock(() => { + signalDuringRun = capturedClientOptions[0]?.abortSignal; + return Promise.resolve(mockStartResult); + }); + // Re-rig the workflow mock to use the new startAsyncMock. + const workflow = { + createRun: mock(() => + Promise.resolve({ + startAsync: startAsyncMock, + resumeAsync: mock(() => Promise.resolve({ status: "success" })), + }) + ), + }; + getWorkflowSpy.mockImplementation(function (this: MastraClient) { + capturedClientOptions.push( + (this as unknown as { options: { abortSignal?: AbortSignal } }).options + ); + return workflow as any; + }); + + await runWizard(makeOptions()); + + expect(signalDuringRun).toBeInstanceOf(AbortSignal); + // Signal was live at time of fetch dispatch. Post-run assertion in + // other tests proves it's aborted by teardown. + // (The .aborted state at capture time can race with the synchronous + // wizard completion path in tests that resolve instantly, so we only + // assert the signal identity is correct here.) + }); +}); From a52a5fec869992f427c0a71dcc71ba1d1bb4b286 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 23 Apr 2026 09:43:08 +0000 Subject: [PATCH 2/2] fix(init): drop redundant abort guard and strengthen signal-live test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-merge review feedback cleanup: - Remove the `if (!signal.aborted)` guard around `abortController.abort()`. `AbortController.abort()` is spec-idempotent — the guard added a line for zero behavior. The comment now states the idempotence property directly. - Rewrite the "forwards a live abortSignal" test to actually prove what it claims. The previous version captured the signal from `startAsyncMock` and only re-asserted identity against `capturedClientOptions[0]` — tautological. New version reads `signal.aborted` from the `getWorkflow` spy (which runs synchronously at `new MastraClient(...)` time, before any fetch dispatch) and asserts it's `false`. Then asserts that by the time `runWizard` returns, the same signal is `true`. Proves both that the signal is live during construction AND that teardown aborts it. --- src/lib/init/wizard-runner.ts | 7 ++-- test/lib/init/wizard-runner.test.ts | 53 +++++++++++++---------------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/src/lib/init/wizard-runner.ts b/src/lib/init/wizard-runner.ts index ef6a07f2d..3eccb0fb1 100644 --- a/src/lib/init/wizard-runner.ts +++ b/src/lib/init/wizard-runner.ts @@ -424,11 +424,8 @@ async function runWizardInner(initialOptions: WizardOptions): Promise { const abortController = new AbortController(); using _mastraCleanup = { [Symbol.dispose]: (): void => { - // Guard against double-abort — calling abort() on an already-aborted - // signal is a no-op in Node/Bun, but explicit check documents intent. - if (!abortController.signal.aborted) { - abortController.abort(); - } + // AbortController.abort() is spec-idempotent, so no guard needed. + abortController.abort(); }, }; diff --git a/test/lib/init/wizard-runner.test.ts b/test/lib/init/wizard-runner.test.ts index 370639e25..42aeab12f 100644 --- a/test/lib/init/wizard-runner.test.ts +++ b/test/lib/init/wizard-runner.test.ts @@ -582,39 +582,34 @@ describe("runWizard — MastraClient lifecycle", () => { expect((signal as AbortSignal).aborted).toBe(true); }); - test("forwards a live abortSignal to MastraClient at construction", async () => { - // Before the wizard exits, the signal MUST NOT yet be aborted — the - // MastraClient is using it for in-flight fetches throughout the run. - // We observe this by checking the signal state inside the workflow - // handler (which executes while the wizard's `using` scope is active). - let signalDuringRun: AbortSignal | undefined; - startAsyncMock = mock(() => { - signalDuringRun = capturedClientOptions[0]?.abortSignal; - return Promise.resolve(mockStartResult); - }); - // Re-rig the workflow mock to use the new startAsyncMock. - const workflow = { - createRun: mock(() => - Promise.resolve({ - startAsync: startAsyncMock, - resumeAsync: mock(() => Promise.resolve({ status: "success" })), - }) - ), - }; + test("signal is live (not pre-aborted) while the wizard is running", async () => { + // `getWorkflow` runs BEFORE `startAsync` (client.getWorkflow is called + // synchronously right after `new MastraClient(...)`), so the signal + // observed at that time is the same instance that in-flight fetches + // would see during the wizard. If the signal were somehow pre-aborted + // at construction, it would be aborted here too. This proves the + // `using _mastraCleanup` disposable does NOT fire until teardown. + let abortedAtConstruction: boolean | undefined; getWorkflowSpy.mockImplementation(function (this: MastraClient) { - capturedClientOptions.push( - (this as unknown as { options: { abortSignal?: AbortSignal } }).options - ); - return workflow as any; + const opts = ( + this as unknown as { options: { abortSignal?: AbortSignal } } + ).options; + capturedClientOptions.push(opts); + abortedAtConstruction = opts.abortSignal?.aborted; + return { + createRun: mock(() => + Promise.resolve({ + startAsync: startAsyncMock, + resumeAsync: mock(() => Promise.resolve({ status: "success" })), + }) + ), + } as any; }); await runWizard(makeOptions()); - expect(signalDuringRun).toBeInstanceOf(AbortSignal); - // Signal was live at time of fetch dispatch. Post-run assertion in - // other tests proves it's aborted by teardown. - // (The .aborted state at capture time can race with the synchronous - // wizard completion path in tests that resolve instantly, so we only - // assert the signal identity is correct here.) + expect(abortedAtConstruction).toBe(false); + // And teardown aborted it by the time the wizard returned. + expect(capturedClientOptions[0]?.abortSignal?.aborted).toBe(true); }); });