diff --git a/packages/shared/package.json b/packages/shared/package.json index 00aafed93..6a6443084 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -40,6 +40,7 @@ "@standard-schema/spec": "1.1.0", "@clack/prompts": "1.0.1", "commander": "12.1.0", + "napi-postinstall": "0.3.4", "zod": "4.3.6" } } diff --git a/packages/shared/scripts/postinstall.d.ts b/packages/shared/scripts/postinstall.d.ts new file mode 100644 index 000000000..a78372752 --- /dev/null +++ b/packages/shared/scripts/postinstall.d.ts @@ -0,0 +1,14 @@ +// Type declarations for the self-contained postinstall script. The script itself +// (postinstall.js) ships verbatim in the published package; this .d.ts is dev-only +// (not copied by tools/dist-appkit.ts) and exists so the unit test can import the +// script under TypeScript without enabling allowJs. + +/** + * Pre-fetch the @ast-grep/napi native host binding at install time. No-op unless + * running under npm. Best-effort and NON-FATAL: runs in a child process bounded by + * a hard timeout, and any failure or timeout logs a single warning and returns. + */ +export function ensureAstGrepBinding(): void; + +/** Print the hint telling users how to set up AI assistant instructions. */ +export function printSetupHint(): void; diff --git a/packages/shared/scripts/postinstall.js b/packages/shared/scripts/postinstall.js index 22f1c09dd..0aaccdd87 100644 --- a/packages/shared/scripts/postinstall.js +++ b/packages/shared/scripts/postinstall.js @@ -1,6 +1,82 @@ #!/usr/bin/env node -console.log(""); -console.log("[@databricks/appkit] To setup AI assistant instructions, run:"); -console.log(""); -console.log(" npx appkit setup --write"); -console.log(""); +// This script is copied STANDALONE into the published @databricks/appkit package +// (see tools/dist-appkit.ts). It must stay SELF-CONTAINED: it may only import +// external runtime deps (declared in packages/shared/package.json -> dependencies) +// and must NEVER import from appkit's own src/dist — those paths do not exist in +// the published layout. +import { execFileSync } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +// Hard ceiling (ms) on the best-effort native-binding pre-fetch. napi-postinstall +// shells out to a SYNCHRONOUS `npm install` internally, which an in-process timer +// could not interrupt — so the pre-fetch runs in a child process that we kill if it +// exceeds this deadline. A timeout (like any other failure) is non-fatal: a still- +// missing binding is handled gracefully by AppKit's lazy @ast-grep/napi loader. +const PREFETCH_TIMEOUT_MS = 60_000; + +// Runs in the child: re-resolve and materialize the @ast-grep/napi host binding. +// `.catch(exit 1)` keeps the child quiet and turns a rejection into a non-zero exit +// that the parent treats as a failed (non-fatal) pre-fetch. +const PREFETCH_SCRIPT = + "Promise.resolve(require('napi-postinstall').checkAndPreparePackage('@ast-grep/napi')).catch(() => process.exit(1))"; + +/** + * Pre-fetch the @ast-grep/napi native host binding at install time. + * + * `appkit generate-types` loads @ast-grep/napi, which resolves a platform-specific + * optional dependency (e.g. @ast-grep/napi-linux-x64-gnu). npm sometimes silently + * skips that optional binary (npm/cli#4828, or a supply-chain cutoff), which makes + * the binding fail to load and crashes the app's own postinstall on Databricks Apps. + * `napi-postinstall` re-resolves and materializes the correct native package. + * + * Best-effort and NON-FATAL: + * - Runs only under npm (the package manager with the optional-dep bug). Under + * pnpm/yarn-PnP the approach does not apply, so it is a no-op. + * - Runs in a child process bounded by PREFETCH_TIMEOUT_MS, so a hung/slow fetch + * is killed rather than blocking `npm install` indefinitely (napi-postinstall's + * internal `npm install` is synchronous, so an in-process timer cannot bound it). + * - Any failure or timeout prints a single warning and returns normally. A failed + * pre-fetch must NEVER break `npm install`. + */ +export function ensureAstGrepBinding() { + // Only npm exhibits the optional-dependency skip this works around. Other package + // managers (pnpm, yarn PnP) either resolve correctly or don't support this flow. + if (!process.env.npm_config_user_agent?.startsWith("npm/")) { + return; + } + + // Package root (…/scripts/postinstall.js -> package dir) so the child resolves + // `napi-postinstall` and `@ast-grep/napi` from the installed node_modules tree. + const pkgDir = path.dirname(path.dirname(fileURLToPath(import.meta.url))); + + try { + execFileSync(process.execPath, ["-e", PREFETCH_SCRIPT], { + cwd: pkgDir, + stdio: "inherit", + timeout: PREFETCH_TIMEOUT_MS, + }); + } catch (err) { + // Non-fatal: execFileSync throws on a non-zero child exit AND on a timeout kill. + // console.error writes to stderr. + console.error( + `[@databricks/appkit] Could not pre-fetch @ast-grep/napi native binding: ${err?.message ?? err}`, + ); + } +} + +/** Print the hint telling users how to set up AI assistant instructions. */ +export function printSetupHint() { + console.log(""); + console.log("[@databricks/appkit] To setup AI assistant instructions, run:"); + console.log(""); + console.log(" npx appkit setup --write"); + console.log(""); +} + +// Only run side effects when executed directly (e.g. as the package postinstall), +// not when imported from a test. +if (fileURLToPath(import.meta.url) === process.argv[1]) { + ensureAstGrepBinding(); + printSetupHint(); +} diff --git a/packages/shared/src/scripts/postinstall.test.ts b/packages/shared/src/scripts/postinstall.test.ts new file mode 100644 index 000000000..85beedec6 --- /dev/null +++ b/packages/shared/src/scripts/postinstall.test.ts @@ -0,0 +1,90 @@ +import { + afterEach, + beforeEach, + describe, + expect, + type Mock, + test, + vi, +} from "vitest"; + +// The system under test lives in the standalone, self-contained postinstall script +// that ships verbatim in the published package (see tools/dist-appkit.ts). We import +// it here (not a copy) so the published behavior is what gets exercised. The script's +// top-level main guard means importing it does NOT trigger the install side effect. + +// vi.mock factories are hoisted above the imports, so the spy must be created in a +// hoisted block too (a plain const would be in the TDZ when the factory runs). +const { execFileSync } = vi.hoisted(() => ({ + execFileSync: vi.fn(), +})); + +// ensureAstGrepBinding runs the pre-fetch in a time-bounded child process via +// execFileSync. Mock it so the test never spawns a real process or touches the +// network/filesystem. +vi.mock("node:child_process", () => ({ execFileSync })); + +import { ensureAstGrepBinding } from "../../scripts/postinstall.js"; + +describe("ensureAstGrepBinding (postinstall native-binding pre-fetch)", () => { + const prevUserAgent = process.env.npm_config_user_agent; + let consoleError: Mock; + + beforeEach(() => { + vi.clearAllMocks(); + // Silence the non-fatal warning (console.error -> stderr) so failing-path + // tests don't pollute output, and so we can assert it fired exactly once. + consoleError = vi + .spyOn(console, "error") + .mockImplementation(() => {}) as Mock; + }); + + afterEach(() => { + vi.restoreAllMocks(); + if (prevUserAgent === undefined) { + delete process.env.npm_config_user_agent; + } else { + process.env.npm_config_user_agent = prevUserAgent; + } + }); + + test("pre-fetches @ast-grep/napi in a time-bounded child under an npm user agent", () => { + process.env.npm_config_user_agent = "npm/10.0.0 node/v22.0.0 darwin arm64"; + + ensureAstGrepBinding(); + + expect(execFileSync).toHaveBeenCalledTimes(1); + const [bin, args, opts] = execFileSync.mock.calls[0] as [ + string, + string[], + { timeout?: number }, + ]; + expect(bin).toBe(process.execPath); + expect(args.join(" ")).toContain("@ast-grep/napi"); + // The pre-fetch MUST be bounded so a hung/slow fetch cannot block `npm install`. + expect(opts.timeout).toBeGreaterThan(0); + }); + + test("is non-fatal when the pre-fetch fails or times out", () => { + process.env.npm_config_user_agent = "npm/10.0.0 node/v22.0.0 darwin arm64"; + // execFileSync throws on both a non-zero child exit and a timeout kill. + execFileSync.mockImplementation(() => { + throw new Error("ETIMEDOUT"); + }); + + // Must not throw — a failed/timed-out pre-fetch must never break `npm install`. + expect(() => ensureAstGrepBinding()).not.toThrow(); + // And it warns exactly once on stderr (via console.error). + expect(consoleError).toHaveBeenCalledTimes(1); + expect(consoleError.mock.calls[0]?.[0]).toContain("@ast-grep/napi"); + }); + + test("does nothing under a non-npm user agent (e.g. pnpm)", () => { + process.env.npm_config_user_agent = + "pnpm/10.21.0 npm/? node/v22.0.0 darwin arm64"; + + ensureAstGrepBinding(); + + expect(execFileSync).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/shared/tsconfig.json b/packages/shared/tsconfig.json index 5e195c3b6..efd3b9d52 100644 --- a/packages/shared/tsconfig.json +++ b/packages/shared/tsconfig.json @@ -7,6 +7,6 @@ "@/*": ["src/*"] } }, - "include": ["src/**/*"], + "include": ["src/**/*", "scripts/**/*.d.ts"], "exclude": ["node_modules", "dist", "src/**/fixtures"] } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6bf9f44a1..2669354b7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -554,6 +554,9 @@ importers: commander: specifier: 12.1.0 version: 12.1.0 + napi-postinstall: + specifier: 0.3.4 + version: 0.3.4 zod: specifier: 4.3.6 version: 4.3.6 @@ -8938,6 +8941,11 @@ packages: napi-build-utils@2.0.0: resolution: {integrity: sha512-GEbrYkbfF7MoNaoh2iGG84Mnf/WZfB0GdGEsM8wz7Expx/LlWf5U8t9nvJKXSp3qr5IsEbK04cBGhol/KwOsWA==} + napi-postinstall@0.3.4: + resolution: {integrity: sha512-PHI5f1O0EP5xJ9gQmFGMS6IZcrVvTjpXjz7Na41gTE7eE2hK11lg04CECCYEEjdc17EV4DO+fkGEtt7TpTaTiQ==} + engines: {node: ^12.20.0 || ^14.18.0 || >=16.0.0} + hasBin: true + natural-compare@1.4.0: resolution: {integrity: sha512-OWND8ei3VtNC9h7V60qff3SVobHr996CTwgxubgyQYEpg290h9J0buyECNNJexkFm5sOajh5G116RYA1c8ZMSw==} @@ -21960,6 +21968,8 @@ snapshots: napi-build-utils@2.0.0: optional: true + napi-postinstall@0.3.4: {} + natural-compare@1.4.0: {} negotiator@0.6.3: {}