-
Notifications
You must be signed in to change notification settings - Fork 18.5k
fix(core): plugins are always reinstalled #9675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 27 commits
beff163
9a84dba
be79d7c
71a274c
1d954eb
1038fc1
4d4c02c
11a5976
0f282dc
d04f4d8
9ce1240
b9a2ff4
9d0a325
c37becb
e6030de
caeaa38
0b783cd
19c2aa4
87e0c43
3170262
7bcf7ef
0609a92
b57788b
3169695
3fb7de1
ef0767e
ac56535
83f833c
efd1649
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { readableStreamToText, semver } from "bun" | ||
| import { Log } from "../util/log" | ||
|
|
||
| export namespace PackageRegistry { | ||
| const log = Log.create({ service: "bun" }) | ||
|
|
||
|
|
||
|
rekram1-node marked this conversation as resolved.
Outdated
|
||
| function which() { | ||
| return process.execPath | ||
|
neriousy marked this conversation as resolved.
|
||
| } | ||
|
|
||
| export async function info(pkg: string, field: string, cwd?: string): Promise<string | null> { | ||
| const result = Bun.spawn([which(), "info", pkg, field], { | ||
| cwd, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| env: { | ||
|
Comment on lines
+12
to
+16
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can't we use BunProc here instead? it already bakes in the which() logic
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah ig circular dep |
||
| ...process.env, | ||
| BUN_BE_BUN: "1", | ||
| }, | ||
| }) | ||
|
|
||
| const code = await result.exited | ||
| const stdout = result.stdout ? await readableStreamToText(result.stdout) : "" | ||
| const stderr = result.stderr ? await readableStreamToText(result.stderr) : "" | ||
|
|
||
| if (code !== 0) { | ||
| log.warn("bun info failed", { pkg, field, code, stderr }) | ||
| return null | ||
| } | ||
|
|
||
| const value = stdout.trim() | ||
| if (!value) return null | ||
| return value | ||
| } | ||
|
|
||
| export async function isOutdated(pkg: string, cachedVersion: string, cwd?: string): Promise<boolean> { | ||
| const latestVersion = await info(pkg, "version", cwd) | ||
| if (!latestVersion) { | ||
| log.warn("Failed to resolve latest version, using cached", { pkg, cachedVersion }) | ||
| return false | ||
| } | ||
|
|
||
| const isRange = /[\s^~*xX<>|=]/.test(cachedVersion) | ||
| if (isRange) return !semver.satisfies(latestVersion, cachedVersion) | ||
|
|
||
| return semver.order(cachedVersion, latestVersion) === -1 | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,19 @@ mock.module("open", () => ({ | |
| }, | ||
| })) | ||
|
|
||
| async function waitFor(check: () => boolean, timeoutMs = 8_000) { | ||
| const start = Date.now() | ||
| async function loop(): Promise<void> { | ||
| if (check()) return | ||
| if (Date.now() - start > timeoutMs) { | ||
| throw new Error(`Timed out waiting for condition after ${timeoutMs}ms`) | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| return loop() | ||
| } | ||
| await loop() | ||
| } | ||
|
|
||
| // Mock UnauthorizedError | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are these changes actually needed??
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reverted those change but threw in a bigger timeout singe |
||
| class MockUnauthorizedError extends Error { | ||
| constructor() { | ||
|
|
@@ -133,20 +146,17 @@ test("BrowserOpenFailed event is published when open() throws", async () => { | |
| }) | ||
|
|
||
| // Run authenticate with a timeout to avoid waiting forever for the callback | ||
| const authPromise = MCP.authenticate("test-oauth-server") | ||
| // Attach a handler immediately so callback shutdown rejections | ||
| // don't show up as unhandled between tests. | ||
| const authPromise = MCP.authenticate("test-oauth-server").catch(() => undefined) | ||
|
|
||
| // Wait for the browser open attempt (error fires at 10ms, but we wait for event to be published) | ||
| await new Promise((resolve) => setTimeout(resolve, 200)) | ||
| // Wait until we see the failure event (Config.get() can be slow in tests) | ||
| await waitFor(() => events.length === 1) | ||
|
|
||
| // Stop the callback server and cancel any pending auth | ||
| await McpOAuthCallback.stop() | ||
|
|
||
| // Wait for authenticate to reject (due to server stopping) | ||
| try { | ||
| await authPromise | ||
| } catch { | ||
| // Expected to fail | ||
| } | ||
| await authPromise | ||
|
|
||
| unsubscribe() | ||
|
|
||
|
|
@@ -187,20 +197,19 @@ test("BrowserOpenFailed event is NOT published when open() succeeds", async () = | |
| }) | ||
|
|
||
| // Run authenticate with a timeout to avoid waiting forever for the callback | ||
| const authPromise = MCP.authenticate("test-oauth-server-2") | ||
| const authPromise = MCP.authenticate("test-oauth-server-2").catch(() => undefined) | ||
|
|
||
| // Wait for the browser open attempt and the 500ms error detection timeout | ||
| await new Promise((resolve) => setTimeout(resolve, 700)) | ||
| // Wait until open() is called (Config.get() can be slow in tests) | ||
| await waitFor(() => openCalledWith !== undefined) | ||
|
|
||
| // See note in the previous test: give authenticate() time to move past | ||
| // the 500ms open() error-detection window. | ||
| await new Promise((resolve) => setTimeout(resolve, 600)) | ||
|
|
||
| // Stop the callback server and cancel any pending auth | ||
| await McpOAuthCallback.stop() | ||
|
|
||
| // Wait for authenticate to reject (due to server stopping) | ||
| try { | ||
| await authPromise | ||
| } catch { | ||
| // Expected to fail | ||
| } | ||
| await authPromise | ||
|
|
||
| unsubscribe() | ||
|
|
||
|
|
@@ -237,20 +246,20 @@ test("open() is called with the authorization URL", async () => { | |
| openCalledWith = undefined | ||
|
|
||
| // Run authenticate with a timeout to avoid waiting forever for the callback | ||
| const authPromise = MCP.authenticate("test-oauth-server-3") | ||
| const authPromise = MCP.authenticate("test-oauth-server-3").catch(() => undefined) | ||
|
|
||
| // Wait for the browser open attempt and the 500ms error detection timeout | ||
| await new Promise((resolve) => setTimeout(resolve, 700)) | ||
| // Wait until open() is called (Config.get() can be slow in tests) | ||
| await waitFor(() => openCalledWith !== undefined) | ||
|
|
||
| // authenticate() waits ~500ms to detect async open() failures before it | ||
| // starts awaiting the OAuth callback promise. If we stop the callback | ||
| // server before that, the rejection can be reported as unhandled. | ||
| await new Promise((resolve) => setTimeout(resolve, 600)) | ||
|
|
||
| // Stop the callback server and cancel any pending auth | ||
| await McpOAuthCallback.stop() | ||
|
|
||
| // Wait for authenticate to reject (due to server stopping) | ||
| try { | ||
| await authPromise | ||
| } catch { | ||
| // Expected to fail | ||
| } | ||
| await authPromise | ||
|
|
||
| // Verify open was called with a URL | ||
| expect(openCalledWith).toBeDefined() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.