From 58f4d4538b27f10a465faedb052504f92e285524 Mon Sep 17 00:00:00 2001 From: Will Chen <7344640+wwwillchen@users.noreply.github.com> Date: Wed, 8 Jul 2026 17:21:37 -0700 Subject: [PATCH 1/6] Auto-deny ignored pnpm builds --- plans/pnpm-auto-deny-ignored-builds.md | 187 +++++++++ src/__tests__/app_upgrade_utils.test.ts | 4 + src/ipc/handlers/app_upgrade_handlers.ts | 48 ++- .../processors/executeAddDependency.test.ts | 61 ++- src/ipc/processors/executeAddDependency.ts | 62 ++- src/ipc/services/app_runtime_service.test.ts | 49 ++- src/ipc/services/app_runtime_service.ts | 165 ++++++-- src/ipc/utils/app_upgrade_utils.ts | 72 +++- src/ipc/utils/cloud_sandbox_provider.test.ts | 25 ++ src/ipc/utils/cloud_sandbox_provider.ts | 28 +- src/ipc/utils/socket_firewall.test.ts | 179 ++++++++- src/ipc/utils/socket_firewall.ts | 365 ++++++++++++++++-- src/lib/posthogTelemetry.test.ts | 9 + src/lib/posthogTelemetry.ts | 4 + 14 files changed, 1175 insertions(+), 83 deletions(-) create mode 100644 plans/pnpm-auto-deny-ignored-builds.md diff --git a/plans/pnpm-auto-deny-ignored-builds.md b/plans/pnpm-auto-deny-ignored-builds.md new file mode 100644 index 0000000000..9bfd783fdc --- /dev/null +++ b/plans/pnpm-auto-deny-ignored-builds.md @@ -0,0 +1,187 @@ +# Auto-Deny Ignored pnpm Builds + +> Drafted 2026-07-08 from an investigation session (verified against pnpm 11.10.0 and 10.33.2) + +## Summary + +When a user installs a package whose build scripts are not in Dyad's curated allow-list (e.g. `core-js`), pnpm skips the build and — under pnpm 11's `strictDepBuilds: true` default — any later **fresh** `pnpm install` that lacks Dyad's `--config.strictDepBuilds=false` flag fails hard with `ERR_PNPM_IGNORED_BUILDS` (exit 1). Users get stuck on the install screen with no recoverable action, and exported repos fail on Vercel/Netlify/CI. + +Fix: after every Dyad-run install, record an explicit `pkg: false` decision in `pnpm-workspace.yaml`'s `allowBuilds` map for each ignored build (outside the Dyad-managed block, with a Dyad marker), commit it, and emit telemetry so frequently-denied-but-legit packages can be promoted to the remote allow-list. + +## Problem Statement + +### Verified failure mechanics + +pnpm 11 defaults `strictDepBuilds` to `true`: + +- `pnpm install` with a dependency whose build was ignored → `ERR_PNPM_IGNORED_BUILDS`, **exit 1** — but only when pnpm actually (re)installs packages. An "Already up to date" no-op install exits 0 and never re-evaluates build scripts. +- With `--config.strictDepBuilds=false` (Dyad's `PNPM_INSTALL_POLICY_ARGS`), the same install exits 0 with only a warning box. +- An explicit `pkg: false` entry under `allowBuilds` in `pnpm-workspace.yaml` silences **both** the error and the warning, on pnpm 11.x and 10.x. Build behavior is unchanged (the build was already being skipped). +- `node_modules/.modules.yaml` records **implicitly** ignored builds only: an unlisted package lands in `ignoredBuilds`, but once it is explicitly `false` it drops out. (Detection of auto-deny candidates therefore reads exactly the right set; promotion-time repair cannot rely on `ignoredBuilds` — see Promotion & repair.) +- Flipping an entry `false` → `true` and re-running a plain `pnpm install` does **not** run the previously-skipped build ("Already up to date", no postinstall). Only a fresh install or an explicit `pnpm rebuild ` executes it; `pnpm rebuild ` was verified to run the skipped postinstall and exit 0. +- Known pnpm quirk (out of scope): `allowBuilds` name-matching does not work for `file:` dependencies — even `pkg: true` fails a strict install with `ERR_PNPM_IGNORED_BUILDS: pkg@file:...`. Dyad apps use registry deps, so this is noted but unhandled. + +### Why users get stuck + +1. User asks for a feature; AI runs ``. Install succeeds (Dyad passes the policy flags) with an "Ignored build scripts" warning. `node_modules/.modules.yaml` records `ignoredBuilds: ["core-js@3.49.0"]`. Nothing is recorded in the repo. +2. The app now works — until a **fresh install** happens through any path that doesn't carry `--config.strictDepBuilds=false`: + - **Rebuild** (`restartApp({ removeNodeModules: true })`) on an app with a custom `installCommand` (custom commands run verbatim; `getCommand()` in `app_runtime_service.ts` skips `getPnpmInstallCommand()` entirely). + - The Capacitor upgrade path (`app_upgrade_handlers.ts:109`, intentionally strict) and component-tagger add (`app_upgrade_utils.ts:132`). + - **Everything outside Dyad**: Vercel/Netlify deploys of the exported repo, GitHub Actions, the user's own terminal, other editors. +3. In the run flow the command is a single `install && dev` chain, so exit 1 short-circuits: the dev server never starts, no preview URL ever appears, and the preview panel sits forever on the ignored-builds error. Retrying Rebuild fails identically. **Restart** appears to work (no-op install exits 0), which makes the failure look nondeterministic to users. + +### Why this matters durably + +Once an unlisted-build package is in the lockfile, the repo is poisoned for every standard `pnpm install` consumer. The only official fix (`pnpm approve-builds`) is interactive and unreachable from Dyad's UI. Most Dyad users cannot evaluate "should this package run install scripts" — they just need the app to keep working with the same security posture Dyad already applies (builds skipped unless allow-listed). + +## Goals + +- A user who installs any package always gets back to a working preview — Rebuild included — with zero new prompts. +- Exported repos install cleanly (`pnpm install`, no special flags) on CI/deploy platforms. +- Preserve the supply-chain posture: never run a build script that isn't on the curated allow-list; never use `dangerouslyAllowAllBuilds`. +- Feed telemetry so the remote allow-list (`https://api.dyad.sh/v1/default-approve-builds.txt`, 1h TTL) can be curated from real-world denial frequency. + +## Non-Goals + +- An interactive per-package approval UI (possible phase 3; silent-deny + telemetry is the right default). +- Changing which builds actually run today. +- Removing `--config.strictDepBuilds=false` from existing Dyad paths (keep as belt-and-suspenders for the first install, before denials are recorded). + +## Design + +### 1. Detection: read `.modules.yaml`, don't scrape output + +After every Dyad-run pnpm install/add that succeeds, read `node_modules/.modules.yaml` and parse `ignoredBuilds` (array of `name@version` strings; strip versions — `allowBuilds` keys are bare names). This is authoritative and avoids parsing ANSI-laden PTY output. (`pnpm ignored-builds` is a non-interactive fallback but spawning is unnecessary when the file is readable.) + +Hook points (all already call or sit next to the allow-builds plumbing in `socket_firewall.ts`): + +- `executeAddDependency.ts` → after `runAddDependencyCommand` succeeds +- `app_runtime_service.ts` → after the install phase of a local/docker run (see §5 for the custom-command reactive path) +- `cloud_sandbox_provider.ts` → alongside `commitPnpmAllowBuildsConfigIfChanged` + +### 2. Recording: `pkg: false` outside the managed block, with a marker + +Write denial entries into the top-level `allowBuilds:` map in `pnpm-workspace.yaml`: + +```yaml +allowBuilds: + core-js: false # dyad-auto-denied + # dyad-default-allow-builds begin + ...managed block, rewritten from local/remote list... + # dyad-default-allow-builds end +``` + +Rules: + +- **Never inside the managed block** — `buildAllowBuildsManagedBlock` rewrites it wholesale from the local/remote list on every `ensurePnpmAllowBuildsConfigured` call. +- **Tag each auto-written line** with a trailing `# dyad-auto-denied` comment. This distinguishes Dyad's automatic decision from a human's deliberate `pkg: false`. Critical because `updatePnpmAllowBuildsConfigContentWithSource` filters managed entries that already exist outside the block — without the marker, a later remote-list promotion of that package to `true` would be permanently shadowed by our own auto-denial. +- **Promotion**: when the resolved allow-list (remote or local) contains a package that currently has a `# dyad-auto-denied` entry, remove the denial so the managed `pkg: true` takes effect. Never touch untagged (user-authored) entries. See "Promotion & repair" below for how the skipped build then actually gets run. +- Skip packages already `true` in the resolved allow-list or already present (any value) outside the block. Quote scoped names via the existing `quoteYamlMapKey`. +- Idempotent and append-only per install: new ignores get added; existing entries untouched. + +### 2b. Promotion & repair (decided: middle ground) + +Editing YAML grants _permission_; it does not run the build. Verified: after a `false` → `true` flip, a plain up-to-date `pnpm install` skips the build entirely, and `.modules.yaml` `ignoredBuilds` cannot be used to detect the gap (explicitly-denied packages are never recorded there). So: + +- The promotion pass stays a **pure file transform** (no spawning inside `ensurePnpmAllowBuildsConfigured`), but it **returns the list of promoted package names** it just un-denied. +- Callers that already spawn pnpm (app-run install phase, `executeAddDependency`, cloud sandbox command builder) run a **best-effort `pnpm rebuild `** after their install step — e.g. the run flow conditionally builds the chain as `install && (rebuild || true) && dev`. Rebuild failure must not block the dev server; if the build genuinely mattered, the app is no worse off than before promotion. +- The promoted-names return value is the _only_ trigger — no `.modules.yaml` inspection, no rebuild on ordinary runs. Promotions are rare (only when the curated list changes), so 99.9% of runs add zero work. +- If the same run happened to do a fresh install (which already ran the build), the extra rebuild is redundant but harmless and rare; not worth optimizing away in Phase 1. + +#### Lifecycle walkthrough: deny → curate → promote + +1. **T0**: user installs `core-js-2` (unlisted, build genuinely needed). Install succeeds under Dyad flags; proactive pass writes `core-js-2: false # dyad-auto-denied` outside the managed block, commits, emits telemetry. App installs cleanly everywhere; if the build was load-bearing the package misbehaves at runtime (status quo today, but now visible in telemetry). +2. **T1**: curation adds `core-js-2` to the remote allow-list (1h TTL, no release needed). +3. **T2**: on each app's next start/add-dependency, the promotion pass deletes the tagged line, the managed rewrite emits `core-js-2: true`, the caller runs best-effort `pnpm rebuild core-js-2`, and the change is committed. The app self-heals with zero user interaction; later deploys/CI fresh-install and build it natively. + +Without the promotion pass this lifecycle **deadlocks at T2**: `parseAllowBuildsExistingKeys` filters any managed-list package that already exists outside the block, so Dyad's own T0 denial would shadow the curated `true` forever. The `# dyad-auto-denied` marker is what distinguishes revisable-by-Dyad entries from human decisions. + +Ownership semantics for unmanaged entries when the curated list later adds the same package: + +| Unmanaged entry at T2 | Outcome | +| ------------------------------------------ | -------------------------------------------------------------------------------------------------- | +| `pkg: false # dyad-auto-denied` | Promoted: line removed, managed `true` takes over, `pnpm rebuild pkg` runs | +| `pkg: false` (user-authored) | Untouched; existing-keys filter keeps `pkg` out of the managed block — the human deny durably wins | +| `pkg: true` (user-authored/approve-builds) | Untouched; filter avoids a managed duplicate; build already allowed | + +Consistency invariant: promotion runs **before** the managed-block rewrite in the same transform, and the existing-keys filter is the backstop — no state can contain both an outside `false` and a managed `true` for the same key (YAML duplicate keys are parser-dependent; we never emit them). + +Caveat (accepted): a manual `pnpm approve-builds` run rewrites `pnpm-workspace.yaml` through a YAML serializer and strips all comments — managed-block markers and `# dyad-auto-denied` tags alike. Existing code already re-appends a fresh managed block when markers vanish; stripped denials simply become user-authored (never promoted, still correct and installable). Telemetry fired at deny time, so curation signal is not lost. + +### 3. Commit + +Reuse the `commitPnpmAllowBuildsConfigIfChanged` pattern: `gitAdd` + `gitCommit("[dyad] record denied pnpm dependency builds")`. This is what makes the fix travel with exports/deploys. + +### 4. Telemetry + +Emit one event per install that produced new denials, via the existing main→renderer telemetry channel (`system.onTelemetryEvent` → PostHog): event `pnpm:build-auto-denied`, properties `{ packages: ["core-js@3.49.0"], source: "add-dependency" | "app-run" | "self-heal" | "cloud-sandbox" }`. This is the input signal for curating the remote allow-list. + +**Must bypass the non-Pro 10% sampling.** The renderer's `before_send` (`renderer.tsx`) drops ~90% of non-Pro events unless `shouldBypassNonProTelemetrySampling` (`src/lib/posthogTelemetry.ts`) matches. `pnpm:build-auto-denied` matches none of the current bypass rules (not error-shaped, no bypassed prefix), so without an explicit entry the curation signal would shrink 10× and skew toward Pro users' package mix — free users are the volume that curation depends on. Add the event name (or a shared `pnpm:build-` prefix, if a promotion-success event is added later) to the bypass list. Volume is safe to exempt: the event fires at most once per install _that produces new denials_, so it is rare and self-extinguishing (after the first denial is recorded, later installs of the same app emit nothing). + +### 5. Reactive self-heal for strict paths + +Apps with custom `installCommand` (and the Capacitor flow) hit `ERR_PNPM_IGNORED_BUILDS` **before** any proactive denial exists (fresh `node_modules`, e.g. Rebuild). On install failure: + +1. Detect `ERR_PNPM_IGNORED_BUILDS` in the failure output (stable error code string, present even in PTY output). +2. Read the ignored package names from `node_modules/.modules.yaml` — **verified**: pnpm links packages and writes `ignoredBuilds` before erroring, even on the exit-1 failure. Fall back to parsing the error line (`Ignored build scripts: core-js@3.49.0, foo@1.2.3`) only if the file is unreadable; the error line can wrap in narrow PTYs when many packages are listed. +3. Write denials (§2), commit (§3), emit telemetry with `source: "self-heal"`. +4. Retry the install **once**. Verified: explicit `false` entries make strict-default installs exit 0. + +This fixes the Rebuild-stuck-forever loop without weakening the intentionally-strict paths. + +### 6. Agent visibility + +Append a line to the install results written back into the `` tag: `Note: build scripts for core-js were not run (Dyad security policy).` If the app later fails at runtime because a denied package genuinely needed its build (native addon → `Cannot find module '.../Release/*.node'`), the AI has the context to explain/react instead of flailing. + +## Additional pnpm edge cases (considered) + +- **pnpm 10.x compat** — verified: pnpm 10.33 honors `allowBuilds` (both `true` and `false`). Caveat: Dyad's availability probe accepts any pnpm version (the 10.16 check only gates a warning), and pnpm 10 builds older than the `allowBuilds` map would silently ignore the managed block. Low priority; consider a version floor note if support tickets appear. +- **Non-registry deps (`file:`, `git:`)** — verified asymmetry: `pkg: false` matches by bare name and suppresses the strict error, but `pkg: true` does **not** match (strict install fails even when "allowed"). Auto-deny therefore works on them; promotion cannot — acceptable, since the curated list never contains such names. Self-heal must not assume a denied non-registry package is later allowable by name. +- **Transitive deps are the common case** — `ignoredBuilds` reports the whole tree (user installs A, native B arrives transitively). Auto-deny handles this naturally since we deny exactly what `.modules.yaml` reports; telemetry captures packages the user never chose. +- **Stale denials** — if the dep tree later drops a denied package, its `false` entry remains. Inert cruft; garbage collection is a non-goal. +- **`package.json` `pnpm.*` settings in imported apps** (`onlyBuiltDependencies`, `ignoredBuiltDependencies`) — pnpm merges sources; a package ignored at that level never appears in `ignoredBuilds`, so the design is self-consistent. Do not attempt to reconcile. +- **npm fallback runs all build scripts unconditionally** — pre-existing posture when pnpm is unavailable, not a regression; noted as a known inconsistency. +- **Docker mode uses the container's pnpm** — version-dependent behaviors above apply per-environment against the shared workspace YAML. +- **Concurrency** — temp-file + rename write is atomic; restart path holds `withLock(appId)`. Acceptable. +- **Aliased deps (`alias@npm:real`) / user-global `.npmrc`** — deny by the real package name as reported in `ignoredBuilds` (pnpm resolves aliases there); global `strict-dep-builds=false` merely removes the failure mode. + +## Risks & Mitigations + +| Risk | Mitigation | +| --------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Package genuinely needs its build; auto-deny converts install-time error into obscure runtime error | No regression vs today (build already skipped by `strictDepBuilds=false`); telemetry → remote-list promotion loop; agent-visible note (§6); optional phase-3 UI ("X was blocked from running install scripts — Allow and rebuild") | +| Denial shadows a future curated promotion | `# dyad-auto-denied` marker + promotion pass in §2 | +| YAML corruption of user-edited workspace files | Reuse the existing line-based editing + marker approach in `socket_firewall.ts`, extend its unit tests; atomic temp-file write already exists | +| Self-heal retry loops | Retry exactly once per install invocation | +| npm-based apps | Unaffected (no `pnpm-workspace.yaml` involvement) | + +## Implementation Phases + +**Phase 1 — proactive denial (core)** + +- `readIgnoredBuilds(appPath)` util (parse `node_modules/.modules.yaml`). +- `recordDeniedBuilds(appPath, packages)` in `socket_firewall.ts`: marker-tagged `false` entries outside the managed block + commit helper. +- Promotion pass inside the allow-builds rewrite; `ensurePnpmAllowBuildsConfigured` returns `{ changed, promotedPackages }`. +- Callers run best-effort `pnpm rebuild ` after their install step (§2b). +- Wire into `executeAddDependency`, app-run install completion, cloud sandbox. +- Telemetry event, added to `shouldBypassNonProTelemetrySampling` so free-user events are never sampled out (§4); unit test alongside the existing bypass tests. +- Unit tests alongside the existing `socket_firewall` allow-builds tests (marker round-trip, promotion returns promoted names, dedupe vs managed block, scoped-name quoting, user-authored `false` untouched). + +**Phase 2 — reactive self-heal** + +- `ERR_PNPM_IGNORED_BUILDS` detection on install failure in `app_runtime_service` (custom commands) and upgrade flows; deny + retry once. +- E2E: app with custom `pnpm install` command + unlisted-build dep → Rebuild reaches preview. + +**Phase 3 (optional, later)** + +- Problems-panel affordance to flip a denial to `true` + `pnpm rebuild`. +- Remote-list curation dashboard fed by the telemetry. + +## Resolved Questions + +- ~~Should the promotion pass run `pnpm rebuild ` immediately, or defer to the next fresh install?~~ **Decided: middle ground (§2b).** The YAML pass stays pure and returns promoted names; callers that already spawn pnpm run a best-effort `pnpm rebuild ` after install. Verified empirically that neither a plain up-to-date install nor `.modules.yaml` inspection can substitute: the flip alone never runs the build, and explicitly-denied packages are absent from `ignoredBuilds`. +- ~~Should denials recorded during cloud-sandbox installs also be committed locally, or only in the sandbox file map?~~ **Decided: commit locally** (same `commitPnpmAllowBuildsConfigIfChanged`-style flow as the other hook points). The local repo is the source of truth the sandbox file map is built from, so committing locally keeps sandbox restarts and exports consistent. + +## Open Questions + +- Event naming/property conventions for PostHog — align with existing telemetry taxonomy before Phase 1 lands. diff --git a/src/__tests__/app_upgrade_utils.test.ts b/src/__tests__/app_upgrade_utils.test.ts index ef7de447f9..455f7d3b77 100644 --- a/src/__tests__/app_upgrade_utils.test.ts +++ b/src/__tests__/app_upgrade_utils.test.ts @@ -33,6 +33,10 @@ vi.mock("../ipc/utils/git_utils", () => ({ gitCommit: vi.fn().mockResolvedValue(undefined), })); +vi.mock("../ipc/utils/telemetry", () => ({ + sendTelemetryEvent: vi.fn(), +})); + describe("isComponentTaggerUpgradeNeeded Heuristics", () => { const mockPath = "/mock/app"; diff --git a/src/ipc/handlers/app_upgrade_handlers.ts b/src/ipc/handlers/app_upgrade_handlers.ts index 1d169e3549..06cbd1279f 100644 --- a/src/ipc/handlers/app_upgrade_handlers.ts +++ b/src/ipc/handlers/app_upgrade_handlers.ts @@ -9,6 +9,7 @@ import { DyadError, DyadErrorKind } from "@/errors/dyad_error"; import { isComponentTaggerUpgradeNeeded, applyComponentTagger, + simpleSpawnWithDeniedPnpmBuildSelfHeal, } from "../utils/app_upgrade_utils"; import fs from "node:fs"; import path from "node:path"; @@ -87,12 +88,23 @@ async function applyCapacitor({ appPath: string; }) { // Install Capacitor dependencies - await simpleSpawn({ - command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} add --ignore-workspace-root-check @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 || npm install @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 --legacy-peer-deps`, - cwd: appPath, - successMessage: "Capacitor dependencies installed successfully", - errorPrefix: "Failed to install Capacitor dependencies", - }); + try { + await simpleSpawnWithDeniedPnpmBuildSelfHeal({ + command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} add --ignore-workspace-root-check @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4`, + cwd: appPath, + successMessage: "Capacitor dependencies installed successfully with pnpm", + errorPrefix: "Failed to install Capacitor dependencies via pnpm", + }); + } catch (pnpmErr) { + logger.info("pnpm install failed, falling back to npm", pnpmErr); + await simpleSpawn({ + command: + "npm install @capacitor/core@7.4.4 @capacitor/cli@7.4.4 @capacitor/ios@7.4.4 @capacitor/android@7.4.4 --legacy-peer-deps", + cwd: appPath, + successMessage: "Capacitor dependencies installed successfully with npm", + errorPrefix: "Failed to install Capacitor dependencies", + }); + } // Initialize Capacitor await simpleSpawn({ @@ -105,12 +117,24 @@ async function applyCapacitor({ // Intentionally omit PNPM_INSTALL_POLICY_ARGS because: // 1. confirmModulesPurge will almost never be needed for capacitor (i.e. user would need to switch from npm to pnpm and not triggered a rebuild). // 2. strictBuildDeps should be kept true (default value) in case capacitor has native deps. - await simpleSpawn({ - command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} install --prod=false || npm install --include=dev --legacy-peer-deps`, - cwd: appPath, - successMessage: "Development dependencies installed successfully", - errorPrefix: "Failed to install development dependencies", - }); + try { + await simpleSpawnWithDeniedPnpmBuildSelfHeal({ + command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} install --prod=false`, + cwd: appPath, + successMessage: + "Development dependencies installed successfully with pnpm", + errorPrefix: "Failed to install development dependencies via pnpm", + }); + } catch (pnpmErr) { + logger.info("pnpm install failed, falling back to npm", pnpmErr); + await simpleSpawn({ + command: "npm install --include=dev --legacy-peer-deps", + cwd: appPath, + successMessage: + "Development dependencies installed successfully with npm", + errorPrefix: "Failed to install development dependencies", + }); + } // Add iOS and Android platforms await simpleSpawn({ diff --git a/src/ipc/processors/executeAddDependency.test.ts b/src/ipc/processors/executeAddDependency.test.ts index 00915a760f..559600e0d8 100644 --- a/src/ipc/processors/executeAddDependency.test.ts +++ b/src/ipc/processors/executeAddDependency.test.ts @@ -17,7 +17,10 @@ const { commitPnpmAllowBuildsConfigIfChangedMock, ensureSocketFirewallInstalledMock, getPnpmMinimumReleaseAgeSupportMock, + readPnpmIgnoredBuildsMock, + recordDeniedPnpmBuildsMock, runCommandMock, + sendTelemetryEventMock, readEffectiveSettingsMock, dbUpdateSetMock, dbUpdateWhereMock, @@ -25,7 +28,10 @@ const { commitPnpmAllowBuildsConfigIfChangedMock: vi.fn(), ensureSocketFirewallInstalledMock: vi.fn(), getPnpmMinimumReleaseAgeSupportMock: vi.fn(), + readPnpmIgnoredBuildsMock: vi.fn(), + recordDeniedPnpmBuildsMock: vi.fn(), runCommandMock: vi.fn(), + sendTelemetryEventMock: vi.fn(), readEffectiveSettingsMock: vi.fn(), dbUpdateSetMock: vi.fn(), dbUpdateWhereMock: vi.fn(), @@ -58,10 +64,16 @@ vi.mock("@/ipc/utils/socket_firewall", async () => { commitPnpmAllowBuildsConfigIfChangedMock, ensureSocketFirewallInstalled: ensureSocketFirewallInstalledMock, getPnpmMinimumReleaseAgeSupport: getPnpmMinimumReleaseAgeSupportMock, + readPnpmIgnoredBuilds: readPnpmIgnoredBuildsMock, + recordDeniedPnpmBuilds: recordDeniedPnpmBuildsMock, runCommand: runCommandMock, }; }); +vi.mock("@/ipc/utils/telemetry", () => ({ + sendTelemetryEvent: sendTelemetryEventMock, +})); + describe("executeAddDependency", () => { beforeEach(() => { vi.clearAllMocks(); @@ -74,7 +86,11 @@ describe("executeAddDependency", () => { minimumReleaseAgeSupported: true, version: "10.16.0", }); - commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue(undefined); + commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue({ + promotedPackages: [], + }); + readPnpmIgnoredBuildsMock.mockResolvedValue([]); + recordDeniedPnpmBuildsMock.mockResolvedValue({ deniedBuilds: [] }); readEffectiveSettingsMock.mockResolvedValue({ blockUnsafeNpmPackages: true, }); @@ -578,4 +594,47 @@ describe("executeAddDependency", () => { 'installed <react>', }); }); + + it("records denied pnpm builds and surfaces the security-policy note", async () => { + ensureSocketFirewallInstalledMock.mockResolvedValue({ + available: false, + }); + runCommandMock.mockResolvedValueOnce({ + stdout: "installed via pnpm", + stderr: "", + }); + readPnpmIgnoredBuildsMock.mockResolvedValue([ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ]); + recordDeniedPnpmBuildsMock.mockResolvedValue({ + deniedBuilds: [{ packageName: "core-js", packageSpec: "core-js@3.49.0" }], + }); + + const result = await executeAddDependency({ + packages: ["core-js"], + message: { + id: 1, + content: + '', + } as any, + appPath: "/tmp/app", + }); + + expect(result.installResults).toContain("installed via pnpm"); + expect(result.installResults).toContain( + "Note: build scripts for core-js were not run (Dyad security policy).", + ); + expect(sendTelemetryEventMock).toHaveBeenCalledWith( + "pnpm:build-auto-denied", + { + packages: ["core-js@3.49.0"], + source: "add-dependency", + }, + ); + expect(dbUpdateSetMock).toHaveBeenCalledWith({ + content: expect.stringContaining( + "Note: build scripts for core-js were not run", + ), + }); + }); }); diff --git a/src/ipc/processors/executeAddDependency.ts b/src/ipc/processors/executeAddDependency.ts index 737c493a5c..72a46c7dc2 100644 --- a/src/ipc/processors/executeAddDependency.ts +++ b/src/ipc/processors/executeAddDependency.ts @@ -12,8 +12,11 @@ import { getCommandExecutionDisplayDetails, getPackageManagerCommandEnv, getPnpmMinimumReleaseAgeSupport, + readPnpmIgnoredBuilds, + recordDeniedPnpmBuilds, runCommand, } from "@/ipc/utils/socket_firewall"; +import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; import { choosePackageManagerFromSignal, getPackageManagerSignal, @@ -169,6 +172,34 @@ async function runAddDependencyCommand( } } +function formatDeniedBuildsNote(packageNames: string[]): string { + if (packageNames.length === 0) { + return ""; + } + + const packageList = packageNames.join(", "); + return `\n\nNote: build scripts for ${packageList} were not run (Dyad security policy).`; +} + +async function rebuildPromotedPnpmBuilds( + appPath: string, + packageNames: string[], +): Promise { + if (packageNames.length === 0) { + return; + } + + try { + await runCommand("pnpm", ["rebuild", ...packageNames], { + cwd: appPath, + env: getPackageManagerCommandEnv(), + timeoutMs: ADD_DEPENDENCY_INSTALL_TIMEOUT_MS, + }); + } catch { + // Best effort: if the build is still broken, the install should not regress. + } +} + export async function installPackages({ packages, appPath, @@ -223,9 +254,10 @@ export async function installPackages({ ) { warningMessages.push(pnpmSupport.warningMessage); } - if (packageManager === "pnpm") { - await commitPnpmAllowBuildsConfigIfChanged(appPath); - } + const promotedPackages = + packageManager === "pnpm" + ? (await commitPnpmAllowBuildsConfigIfChanged(appPath)).promotedPackages + : []; const { succeeded, installResults, lastError } = await runAddDependencyCommand( buildAddDependencyCommand(packages, packageManager, useSocketFirewall, { @@ -241,8 +273,30 @@ export async function installPackages({ }); } + await rebuildPromotedPnpmBuilds(appPath, promotedPackages); + + let installResultsWithPolicyNotes = installResults; + if (packageManager === "pnpm") { + const ignoredBuilds = await readPnpmIgnoredBuilds(appPath); + const { deniedBuilds } = await recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + }); + if (deniedBuilds.length > 0) { + sendTelemetryEvent("pnpm:build-auto-denied", { + packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), + source: "add-dependency", + }); + installResultsWithPolicyNotes += formatDeniedBuildsNote( + Array.from( + new Set(deniedBuilds.map((ignoredBuild) => ignoredBuild.packageName)), + ).sort((left, right) => left.localeCompare(right)), + ); + } + } + return { - installResults, + installResults: installResultsWithPolicyNotes, warningMessages, }; } diff --git a/src/ipc/services/app_runtime_service.test.ts b/src/ipc/services/app_runtime_service.test.ts index f5f6247c8b..857fb1038e 100644 --- a/src/ipc/services/app_runtime_service.test.ts +++ b/src/ipc/services/app_runtime_service.test.ts @@ -26,7 +26,11 @@ const { minimumReleaseAgeSupported: false, })), ensurePnpmAllowBuildsConfiguredMock: - vi.fn<(args: unknown) => Promise<{ changed: boolean }>>(), + vi.fn< + ( + args: unknown, + ) => Promise<{ changed: boolean; promotedPackages: string[] }> + >(), readSettingsMock: vi.fn<() => Record>(() => ({ runtimeMode2: "host", })), @@ -82,6 +86,11 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({ npm_config_pm_on_fail: "ignore", }), getPnpmMinimumReleaseAgeSupport: () => getPnpmMinimumReleaseAgeSupportMock(), + isPnpmIgnoredBuildsError: (error: unknown) => + String(error).includes("ERR_PNPM_IGNORED_BUILDS"), + parsePnpmIgnoredBuildsFromOutput: vi.fn(() => []), + readPnpmIgnoredBuilds: vi.fn(async () => []), + recordDeniedPnpmBuilds: vi.fn(async () => ({ deniedBuilds: [] })), PNPM_INSTALL_POLICY_ARGS: [ "--config.pm-on-fail=ignore", "--minimum-release-age=1440", @@ -89,6 +98,10 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({ PNPM_PM_ON_FAIL_IGNORE_ARG: "--config.pm-on-fail=ignore", })); +vi.mock("@/ipc/utils/telemetry", () => ({ + sendTelemetryEvent: vi.fn(), +})); + vi.mock("@/ipc/utils/cloud_sandbox_provider", () => ({ buildCloudSandboxFileMap: vi.fn(), CloudSandboxApiError: class CloudSandboxApiError extends Error { @@ -187,7 +200,10 @@ describe("executeApp", () => { minimumReleaseAgeSupported: false, }); ensurePnpmAllowBuildsConfiguredMock.mockReset(); - ensurePnpmAllowBuildsConfiguredMock.mockResolvedValue({ changed: false }); + ensurePnpmAllowBuildsConfiguredMock.mockResolvedValue({ + changed: false, + promotedPackages: [], + }); readSettingsMock.mockReset(); readSettingsMock.mockReturnValue({ runtimeMode2: "host", @@ -307,6 +323,35 @@ describe("executeApp", () => { ); }); + it("rebuilds promoted pnpm builds before starting the dev server", async () => { + const process = new FakeChildProcess(101); + spawnMock.mockReturnValueOnce(process); + getPnpmMinimumReleaseAgeSupportMock.mockResolvedValue({ + available: true, + minimumReleaseAgeSupported: true, + }); + ensurePnpmAllowBuildsConfiguredMock.mockResolvedValue({ + changed: true, + promotedPackages: ["core-js", "@scope/native"], + }); + + await executeApp({ + appPath: "/tmp/app", + appId: 1, + event: createEvent(), + isNeon: false, + }); + + expect(spawnMock).toHaveBeenCalledWith( + "pnpm --config.pm-on-fail=ignore --minimum-release-age=1440 install && (pnpm rebuild 'core-js' '@scope/native' || true) && pnpm --config.pm-on-fail=ignore run dev --port 32101", + [], + expect.objectContaining({ + cwd: "/tmp/app", + shell: true, + }), + ); + }); + it("does not warn about old pnpm for apps that explicitly use npm", async () => { const appPath = await createTempAppDir(); try { diff --git a/src/ipc/services/app_runtime_service.ts b/src/ipc/services/app_runtime_service.ts index 2fa537ca79..f8bd41afe9 100644 --- a/src/ipc/services/app_runtime_service.ts +++ b/src/ipc/services/app_runtime_service.ts @@ -36,10 +36,15 @@ import { ensurePnpmAllowBuildsConfigured, getPackageManagerCommandEnv, getPnpmMinimumReleaseAgeSupport, + isPnpmIgnoredBuildsError, + parsePnpmIgnoredBuildsFromOutput, type PackageManager, PNPM_PM_ON_FAIL_IGNORE_ARG, PNPM_INSTALL_POLICY_ARGS, + readPnpmIgnoredBuilds, + recordDeniedPnpmBuilds, } from "@/ipc/utils/socket_firewall"; +import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; import { choosePackageManagerFromSignal, getPackageManagerSignal, @@ -90,6 +95,33 @@ function getPnpmRunCommand(): string { return `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} run dev`; } +function quoteShellArg(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'`; +} + +function getBestEffortPnpmRebuildCommand( + packageNames: string[], +): string | null { + if (packageNames.length === 0) { + return null; + } + + return `(pnpm rebuild ${packageNames.map(quoteShellArg).join(" ")} || true)`; +} + +function buildPnpmInstallAndRunCommand(input: { + promotedPackages: string[]; + port: number; +}): string { + return [ + getPnpmInstallCommand(), + getBestEffortPnpmRebuildCommand(input.promotedPackages), + `${getPnpmRunCommand()} --port ${input.port}`, + ] + .filter(Boolean) + .join(" && "); +} + function getNpmInstallCommand(): string { return "npm install --legacy-peer-deps"; } @@ -113,9 +145,14 @@ async function getDefaultCommand({ }): Promise { const port = getAppPort(appId); if (runtimeMode === "docker") { - await ensurePnpmAllowBuildsConfigured({ appPath }); + const allowBuildsResult = await ensurePnpmAllowBuildsConfigured({ + appPath, + }); return { - command: `${getPnpmInstallCommand()} && ${getPnpmRunCommand()} --port ${port}`, + command: buildPnpmInstallAndRunCommand({ + promotedPackages: allowBuildsResult.promotedPackages, + port, + }), isCustom: false, packageManager: "pnpm", }; @@ -147,9 +184,12 @@ async function getDefaultCommand({ }; } - await ensurePnpmAllowBuildsConfigured({ appPath }); + const allowBuildsResult = await ensurePnpmAllowBuildsConfigured({ appPath }); return { - command: `${getPnpmInstallCommand()} && ${getPnpmRunCommand()} --port ${port}`, + command: buildPnpmInstallAndRunCommand({ + promotedPackages: allowBuildsResult.promotedPackages, + port, + }), isCustom: false, packageManager: "pnpm", }; @@ -373,6 +413,7 @@ async function executeAppLocalNode({ isNeon, installCommand, startCommand, + ignoredBuildsSelfHealAttempted = false, }: { appPath: string; appId: number; @@ -380,6 +421,7 @@ async function executeAppLocalNode({ isNeon: boolean; installCommand?: string | null; startCommand?: string | null; + ignoredBuildsSelfHealAttempted?: boolean; }): Promise { const command = await getCommand({ runtimeMode: "host", @@ -459,6 +501,30 @@ Details: ${details || "n/a"} appId, isNeon, event, + onPnpmIgnoredBuildsFailure: + command.isCustom && !ignoredBuildsSelfHealAttempted + ? async (output) => { + const healed = await selfHealDeniedPnpmBuilds({ + appPath, + output, + telemetrySource: "self-heal", + }); + if (!healed) { + return false; + } + + await executeAppLocalNode({ + appPath, + appId, + event, + isNeon, + installCommand, + startCommand, + ignoredBuildsSelfHealAttempted: true, + }); + return true; + } + : undefined, }); } @@ -565,14 +631,18 @@ function listenToProcess({ appId, isNeon, event, + onPnpmIgnoredBuildsFailure, }: { process: ChildProcess; appId: number; isNeon: boolean; event: Electron.IpcMainInvokeEvent; + onPnpmIgnoredBuildsFailure?: (output: string) => Promise; }) { + let processOutput = ""; spawnedProcess.stdout?.on("data", async (data) => { const message = util.stripVTControlCharacters(data.toString()); + processOutput += message; logger.debug( `App ${appId} (PID: ${spawnedProcess.pid}) stdout: ${message}`, ); @@ -622,6 +692,7 @@ function listenToProcess({ spawnedProcess.stderr?.on("data", async (data) => { const message = util.stripVTControlCharacters(data.toString()); + processOutput += message; logger.error( `App ${appId} (PID: ${spawnedProcess.pid}) stderr: ${message}`, ); @@ -642,25 +713,46 @@ function listenToProcess({ }); spawnedProcess.on("close", (code, signal) => { - logger.log( - `App ${appId} (PID: ${spawnedProcess.pid}) process closed with code ${code}, signal ${signal}.`, - ); - flushAllAppOutputs(); - const currentAppInfo = runningApps.get(appId); - if (!currentAppInfo || currentAppInfo.process !== spawnedProcess) { - removeAppIfCurrentProcess(appId, spawnedProcess); - return; - } + void (async () => { + logger.log( + `App ${appId} (PID: ${spawnedProcess.pid}) process closed with code ${code}, signal ${signal}.`, + ); + flushAllAppOutputs(); + const currentAppInfo = runningApps.get(appId); + if (!currentAppInfo || currentAppInfo.process !== spawnedProcess) { + removeAppIfCurrentProcess(appId, spawnedProcess); + return; + } - safeSend(event.sender, "app:output", { - type: "app-exit", - message: `App process exited with code ${code ?? "null"}`, - appId, - exitCode: code, - signal, - timestamp: Date.now(), - }); - removeAppIfCurrentProcess(appId, spawnedProcess); + if ( + code !== 0 && + onPnpmIgnoredBuildsFailure && + isPnpmIgnoredBuildsError(processOutput) + ) { + let retried = false; + try { + retried = await onPnpmIgnoredBuildsFailure(processOutput); + } catch (error) { + logger.warn( + `Failed to self-heal pnpm ignored builds for app ${appId}:`, + error, + ); + } + if (retried) { + return; + } + } + + safeSend(event.sender, "app:output", { + type: "app-exit", + message: `App process exited with code ${code ?? "null"}`, + appId, + exitCode: code, + signal, + timestamp: Date.now(), + }); + removeAppIfCurrentProcess(appId, spawnedProcess); + })(); }); spawnedProcess.on("error", (err) => { @@ -671,6 +763,35 @@ function listenToProcess({ }); } +async function selfHealDeniedPnpmBuilds({ + appPath, + output, + telemetrySource, +}: { + appPath: string; + output: string; + telemetrySource: "self-heal"; +}): Promise { + const ignoredBuildsFromModulesYaml = await readPnpmIgnoredBuilds(appPath); + const ignoredBuilds = + ignoredBuildsFromModulesYaml.length > 0 + ? ignoredBuildsFromModulesYaml + : parsePnpmIgnoredBuildsFromOutput(output); + const { deniedBuilds } = await recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + }); + if (deniedBuilds.length === 0) { + return false; + } + + sendTelemetryEvent("pnpm:build-auto-denied", { + packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), + source: telemetrySource, + }); + return true; +} + async function executeAppInDocker({ appPath, appId, diff --git a/src/ipc/utils/app_upgrade_utils.ts b/src/ipc/utils/app_upgrade_utils.ts index c34b92bb08..2cab33d5d2 100644 --- a/src/ipc/utils/app_upgrade_utils.ts +++ b/src/ipc/utils/app_upgrade_utils.ts @@ -4,7 +4,14 @@ import path from "node:path"; import { gitAddAll, gitCommit } from "./git_utils"; import { simpleSpawn } from "./simpleSpawn"; import { DyadError, DyadErrorKind } from "@/errors/dyad_error"; -import { PNPM_PM_ON_FAIL_IGNORE_ARG } from "./socket_firewall"; +import { + isPnpmIgnoredBuildsError, + parsePnpmIgnoredBuildsFromOutput, + PNPM_PM_ON_FAIL_IGNORE_ARG, + readPnpmIgnoredBuilds, + recordDeniedPnpmBuilds, +} from "./socket_firewall"; +import { sendTelemetryEvent } from "./telemetry"; export const logger = log.scope("app_upgrade_utils"); @@ -46,6 +53,67 @@ type ApplyComponentTaggerOptions = { installDependencies?: boolean; }; +export async function selfHealDeniedPnpmBuildsFromError({ + appPath, + error, + source, +}: { + appPath: string; + error: unknown; + source: "self-heal"; +}): Promise { + if (!isPnpmIgnoredBuildsError(error)) { + return false; + } + + const ignoredBuildsFromModulesYaml = await readPnpmIgnoredBuilds(appPath); + const errorOutput = error instanceof Error ? error.message : String(error); + const ignoredBuilds = + ignoredBuildsFromModulesYaml.length > 0 + ? ignoredBuildsFromModulesYaml + : parsePnpmIgnoredBuildsFromOutput(errorOutput); + const { deniedBuilds } = await recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + }); + if (deniedBuilds.length === 0) { + return false; + } + + sendTelemetryEvent("pnpm:build-auto-denied", { + packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), + source, + }); + return true; +} + +export async function simpleSpawnWithDeniedPnpmBuildSelfHeal({ + command, + cwd, + successMessage, + errorPrefix, +}: { + command: string; + cwd: string; + successMessage: string; + errorPrefix: string; +}): Promise { + try { + await simpleSpawn({ command, cwd, successMessage, errorPrefix }); + } catch (error) { + const healed = await selfHealDeniedPnpmBuildsFromError({ + appPath: cwd, + error, + source: "self-heal", + }); + if (!healed) { + throw error; + } + + await simpleSpawn({ command, cwd, successMessage, errorPrefix }); + } +} + export async function applyComponentTagger( appPath: string, options: ApplyComponentTaggerOptions = {}, @@ -128,7 +196,7 @@ export async function applyComponentTagger( if (installDependencies) { try { - await simpleSpawn({ + await simpleSpawnWithDeniedPnpmBuildSelfHeal({ command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} add --ignore-workspace-root-check -D @dyad-sh/react-vite-component-tagger`, cwd: appPath, successMessage: diff --git a/src/ipc/utils/cloud_sandbox_provider.test.ts b/src/ipc/utils/cloud_sandbox_provider.test.ts index acbdc66613..8f762e1fbd 100644 --- a/src/ipc/utils/cloud_sandbox_provider.test.ts +++ b/src/ipc/utils/cloud_sandbox_provider.test.ts @@ -501,6 +501,9 @@ describe("cloud_sandbox_provider sandbox creation", () => { beforeEach(() => { commitPnpmAllowBuildsConfigIfChangedMock.mockReset(); + commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue({ + promotedPackages: [], + }); fetchMock = vi.fn(async () => { return new Response( JSON.stringify({ @@ -543,6 +546,28 @@ describe("cloud_sandbox_provider sandbox creation", () => { ); }); + it("rebuilds promoted pnpm builds in the default install command", async () => { + commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue({ + promotedPackages: ["core-js", "@scope/native"], + }); + + await createCloudSandbox({ + appId: 42, + appPath: "/tmp/app", + installCommand: null, + startCommand: undefined, + }); + + const [, init] = fetchMock.mock.calls[0]; + expect(JSON.parse(String(init?.body))).toEqual({ + appId: 42, + appPath: "/tmp/app", + installCommand: + "pnpm --config.pm-on-fail=ignore --config.confirmModulesPurge=false --config.strictDepBuilds=false install && (pnpm rebuild 'core-js' '@scope/native' || true)", + startCommand: "pnpm --config.pm-on-fail=ignore run dev", + }); + }); + it("preserves explicit custom commands after trimming", async () => { await createCloudSandbox({ appId: 42, diff --git a/src/ipc/utils/cloud_sandbox_provider.ts b/src/ipc/utils/cloud_sandbox_provider.ts index ca0b53b081..3315c41b89 100644 --- a/src/ipc/utils/cloud_sandbox_provider.ts +++ b/src/ipc/utils/cloud_sandbox_provider.ts @@ -138,6 +138,23 @@ function getDefaultStartCommand(): string { return `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} run dev`; } +function quoteShellArg(value: string): string { + return `'${value.replace(/'/g, "'\\''")}'`; +} + +function appendBestEffortPnpmRebuild( + installCommand: string, + packageNames: string[], +): string { + if (packageNames.length === 0) { + return installCommand; + } + + return `${installCommand} && (pnpm rebuild ${packageNames + .map(quoteShellArg) + .join(" ")} || true)`; +} + function getDefaultCloudSandboxErrorMessage(status: number): string { if (status === 401 || status === 403) { return "Dyad couldn’t authorize the cloud sandbox request. Please try again."; @@ -702,11 +719,18 @@ class DyadEngineCloudSandboxProvider implements CloudSandboxProvider { installCommand?: string | null; startCommand?: string | null; }) { + let promotedPackages: string[] = []; if (!input.installCommand?.trim()) { - await commitPnpmAllowBuildsConfigIfChanged(input.appPath); + promotedPackages = ( + await commitPnpmAllowBuildsConfigIfChanged(input.appPath) + ).promotedPackages; } - const { installCommand, startCommand } = resolveCloudSandboxCommands(input); + const { installCommand: resolvedInstallCommand, startCommand } = + resolveCloudSandboxCommands(input); + const installCommand = input.installCommand?.trim() + ? resolvedInstallCommand + : appendBestEffortPnpmRebuild(resolvedInstallCommand, promotedPackages); const response = await cloudSandboxFetch("/sandboxes", { method: "POST", body: JSON.stringify({ diff --git a/src/ipc/utils/socket_firewall.test.ts b/src/ipc/utils/socket_firewall.test.ts index 37ed2ae47d..22b89576c0 100644 --- a/src/ipc/utils/socket_firewall.test.ts +++ b/src/ipc/utils/socket_firewall.test.ts @@ -1,6 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { PtyCommandExecutionError } from "@/ipc/utils/pty_command_runner"; -import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import os from "node:os"; @@ -30,6 +30,9 @@ import { getPackageManagerCommandEnv, getPnpmMinimumReleaseAgeSupport, PACKAGE_MANAGER_PROBE_TIMEOUT_MS, + parsePnpmIgnoredBuildsFromModulesYaml, + readPnpmIgnoredBuilds, + recordDeniedPnpmBuilds, resolveExecutableName, runCommand, SOCKET_FIREWALL_PROBE_TIMEOUT_MS, @@ -412,7 +415,7 @@ describe("updatePnpmAllowBuildsConfigContent", () => { appPath: tempDir, allowBuildsText, }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); await expect( readFile(path.join(tempDir, "pnpm-workspace.yaml"), "utf8"), @@ -457,7 +460,7 @@ describe("updatePnpmAllowBuildsConfigContent", () => { text: () => Promise.resolve(remoteAllowBuildsText), }), }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); await expect( readFile(path.join(tempDir, "pnpm-workspace.yaml"), "utf8"), @@ -492,13 +495,13 @@ describe("updatePnpmAllowBuildsConfigContent", () => { appPath: firstTempDir, remoteAllowBuildsTextFetcher, }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); await expect( ensurePnpmAllowBuildsConfigured({ appPath: secondTempDir, remoteAllowBuildsTextFetcher, }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); expect(remoteAllowBuildsTextFetcher).toHaveBeenCalledTimes(1); await expect( @@ -550,7 +553,7 @@ describe("updatePnpmAllowBuildsConfigContent", () => { appPath: firstTempDir, remoteAllowBuildsTextFetcher, }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); dateNowSpy.mockReturnValue(startMs + DYAD_ALLOW_BUILDS_CACHE_TTL_MS + 1); @@ -559,7 +562,7 @@ describe("updatePnpmAllowBuildsConfigContent", () => { appPath: secondTempDir, remoteAllowBuildsTextFetcher, }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); expect(remoteAllowBuildsTextFetcher).toHaveBeenCalledTimes(2); await expect( @@ -604,7 +607,7 @@ describe("updatePnpmAllowBuildsConfigContent", () => { text: () => Promise.resolve(""), }), }), - ).resolves.toEqual({ changed: false }); + ).resolves.toEqual({ changed: false, promotedPackages: [] }); await expect(readFile(configPath, "utf8")).resolves.toBe(existingConfig); } finally { @@ -623,7 +626,7 @@ describe("updatePnpmAllowBuildsConfigContent", () => { text: () => Promise.resolve(""), }), }), - ).resolves.toEqual({ changed: true }); + ).resolves.toEqual({ changed: true, promotedPackages: [] }); await expect( readFile(path.join(tempDir, "pnpm-workspace.yaml"), "utf8"), @@ -632,6 +635,164 @@ describe("updatePnpmAllowBuildsConfigContent", () => { await rm(tempDir, { recursive: true, force: true }); } }); + + it("records ignored builds as tagged denials outside the managed block", async () => { + const tempDir = await mkdtemp(path.join(os.tmpdir(), "dyad-pnpm-deny-")); + try { + await expect( + recordDeniedPnpmBuilds({ + appPath: tempDir, + allowBuildsText, + ignoredBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + { + packageName: "@scope/native", + packageSpec: "@scope/native@1.2.3", + }, + { packageName: "sharp", packageSpec: "sharp@0.34.0" }, + ], + }), + ).resolves.toEqual({ + deniedBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + { packageName: "@scope/native", packageSpec: "@scope/native@1.2.3" }, + ], + }); + + await expect( + readFile(path.join(tempDir, "pnpm-workspace.yaml"), "utf8"), + ).resolves.toBe( + [ + "allowBuilds:", + ' "@scope/native": false # dyad-auto-denied', + " core-js: false # dyad-auto-denied", + " # dyad-default-allow-builds begin", + " # dyad-default-allow-builds-schema=v1", + " # dyad-default-allow-builds-data-version=2026-05-21.1", + " # dyad-default-allow-builds-channel=local", + ' "@swc/core": true', + " sharp: true", + " # dyad-default-allow-builds end", + "", + "packages:", + " - .", + "minimumReleaseAge: 1440", + "", + ].join("\n"), + ); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); + + it("promotes tagged denials when the allow-list later includes the package", async () => { + const tempDir = await mkdtemp(path.join(os.tmpdir(), "dyad-pnpm-promote-")); + const configPath = path.join(tempDir, "pnpm-workspace.yaml"); + const promotedAllowBuildsText = [ + "# dyad-default-allow-builds-schema=v1", + "# dyad-default-allow-builds-data-version=2026-05-22.1", + "# dyad-default-allow-builds-channel=local", + "core-js", + "sharp", + "", + ].join("\n"); + + try { + await writeFile( + configPath, + [ + "allowBuilds:", + " core-js: false # dyad-auto-denied", + " user-denied: false", + " # dyad-default-allow-builds begin", + " # dyad-default-allow-builds-schema=v1", + " # dyad-default-allow-builds-data-version=2026-05-21.1", + " # dyad-default-allow-builds-channel=local", + " sharp: true", + " # dyad-default-allow-builds end", + "minimumReleaseAge: 1440", + "", + ].join("\n"), + ); + + await expect( + ensurePnpmAllowBuildsConfigured({ + appPath: tempDir, + allowBuildsText: promotedAllowBuildsText, + }), + ).resolves.toEqual({ changed: true, promotedPackages: ["core-js"] }); + + await expect(readFile(configPath, "utf8")).resolves.toBe( + [ + "allowBuilds:", + " user-denied: false", + " # dyad-default-allow-builds begin", + " # dyad-default-allow-builds-schema=v1", + " # dyad-default-allow-builds-data-version=2026-05-22.1", + " # dyad-default-allow-builds-channel=local", + " core-js: true", + " sharp: true", + " # dyad-default-allow-builds end", + "minimumReleaseAge: 1440", + "", + "packages:", + " - .", + "", + ].join("\n"), + ); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); +}); + +describe("readPnpmIgnoredBuilds", () => { + it("parses ignored build specs from node_modules/.modules.yaml", async () => { + expect( + parsePnpmIgnoredBuildsFromModulesYaml( + [ + "layoutVersion: 5", + "ignoredBuilds:", + " - core-js@3.49.0", + ' - "@scope/native@1.2.3"', + "pendingBuilds: []", + "", + ].join("\n"), + ), + ).toEqual([ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + { packageName: "@scope/native", packageSpec: "@scope/native@1.2.3" }, + ]); + }); + + it("reads ignored builds from the app path", async () => { + const tempDir = await mkdtemp( + path.join(os.tmpdir(), "dyad-pnpm-ignored-builds-"), + ); + try { + await writeFile( + path.join(tempDir, "node_modules", ".modules.yaml"), + "ignoredBuilds: [core-js@3.49.0]\n", + ).catch(async (error) => { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + await mkdir(path.join(tempDir, "node_modules"), { + recursive: true, + }); + await writeFile( + path.join(tempDir, "node_modules", ".modules.yaml"), + "ignoredBuilds: [core-js@3.49.0]\n", + ); + }); + + await expect(readPnpmIgnoredBuilds(tempDir)).resolves.toEqual([ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ]); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); }); describe("buildAddDependencyCommand", () => { diff --git a/src/ipc/utils/socket_firewall.ts b/src/ipc/utils/socket_firewall.ts index 1c3cf1ca16..15c9bc5fb9 100644 --- a/src/ipc/utils/socket_firewall.ts +++ b/src/ipc/utils/socket_firewall.ts @@ -59,6 +59,8 @@ const DYAD_ALLOW_BUILDS_BEGIN = "# dyad-default-allow-builds begin"; const DYAD_ALLOW_BUILDS_END = "# dyad-default-allow-builds end"; const LEGACY_DYAD_ALLOW_BUILDS_BEGIN = "# dyad-default-allow-builds=v1 begin"; const LEGACY_DYAD_ALLOW_BUILDS_END = "# dyad-default-allow-builds=v1 end"; +const DYAD_AUTO_DENIED_ALLOW_BUILDS_COMMENT = "# dyad-auto-denied"; +const PNPM_IGNORED_BUILDS_ERROR_CODE = "ERR_PNPM_IGNORED_BUILDS"; const DYAD_ALLOW_BUILDS_METADATA_PATTERN = /^#\s*(dyad-default-allow-builds-(?:schema|data-version|channel))=(.+)$/; const DYAD_ALLOW_BUILDS_REMOTE_URL = @@ -180,6 +182,10 @@ type RemoteAllowBuildsCacheEntry = { source: AllowBuildsSource; expiresAtMs: number; }; +export type PnpmIgnoredBuild = { + packageName: string; + packageSpec: string; +}; const remoteAllowBuildsCache = new WeakMap< AllowBuildsTextFetcher, @@ -375,19 +381,102 @@ function parseAllowBuildsExistingKeys(lines: string[]): Set { } const rawKey = match[1].trim(); - try { - keys.add( - rawKey.startsWith('"') - ? JSON.parse(rawKey) - : rawKey.replace(/^'|'$/g, ""), - ); - } catch { - keys.add(rawKey); - } + keys.add(parseYamlMapKey(rawKey)); } return keys; } +function parseYamlMapKey(rawKey: string): string { + try { + return rawKey.startsWith('"') + ? JSON.parse(rawKey) + : rawKey.replace(/^'|'$/g, ""); + } catch { + return rawKey; + } +} + +function parseAllowBuildsLine(line: string): { key: string } | null { + const match = line.match(/^\s{2}((?:"(?:[^"\\]|\\.)+"|'[^']+'|[^:#]+)):\s*/); + if (!match) { + return null; + } + + return { key: parseYamlMapKey(match[1].trim()) }; +} + +function removeAutoDeniedPromotedBuilds( + lines: string[], + source: AllowBuildsSource, +): string[] { + const promotedPackageSet = new Set(source.packages); + const range = getTopLevelAllowBuildsRange(lines); + if (!range || promotedPackageSet.size === 0) { + return []; + } + + const managedBlock = findAllowBuildsManagedBlock(lines); + const promotedPackages: string[] = []; + for (let index = range.end - 1; index > range.start; index -= 1) { + if ( + managedBlock && + index >= managedBlock.beginIndex && + index <= managedBlock.endIndex + ) { + continue; + } + + const parsedLine = parseAllowBuildsLine(lines[index]); + if ( + parsedLine && + promotedPackageSet.has(parsedLine.key) && + lines[index].includes(DYAD_AUTO_DENIED_ALLOW_BUILDS_COMMENT) + ) { + lines.splice(index, 1); + promotedPackages.push(parsedLine.key); + } + } + + return promotedPackages.sort((left, right) => left.localeCompare(right)); +} + +function insertAutoDeniedBuilds( + lines: string[], + source: AllowBuildsSource, + packageNames: string[], +): string[] { + const sourcePackageSet = new Set(source.packages); + const range = getTopLevelAllowBuildsRange(lines); + const existingKeys = parseAllowBuildsExistingKeys( + range ? lines.slice(range.start + 1, range.end) : [], + ); + const newDeniedPackageNames = Array.from(new Set(packageNames)) + .filter((packageName) => { + return ( + !sourcePackageSet.has(packageName) && !existingKeys.has(packageName) + ); + }) + .sort((left, right) => left.localeCompare(right)); + + if (newDeniedPackageNames.length === 0) { + return []; + } + + if (!range) { + return []; + } + + lines.splice( + range.start + 1, + 0, + ...newDeniedPackageNames.map( + (packageName) => + ` ${quoteYamlMapKey(packageName)}: false ${DYAD_AUTO_DENIED_ALLOW_BUILDS_COMMENT}`, + ), + ); + return newDeniedPackageNames; +} + function hasTopLevelConfigKey(lines: string[], key: string): boolean { return lines.some((line) => new RegExp(`^${key}:\\s*(?:#.*)?$|^${key}:\\s+`).test(line), @@ -416,18 +505,20 @@ export function updatePnpmAllowBuildsConfigContent( return updatePnpmAllowBuildsConfigContentWithSource( existingContent, parseDefaultAllowBuilds(allowBuildsText), - ); + ).content; } function updatePnpmAllowBuildsConfigContentWithSource( existingContent: string, source: AllowBuildsSource, -): string { + autoDeniedPackageNames: string[] = [], +): { content: string; promotedPackages: string[]; deniedPackages: string[] } { const lines = existingContent ? existingContent.split(/\r?\n/) : []; if (lines.at(-1) === "") { lines.pop(); } + const promotedPackages = removeAutoDeniedPromotedBuilds(lines, source); const managedBlock = findAllowBuildsManagedBlock(lines); if (managedBlock) { const { beginIndex, endIndex } = managedBlock; @@ -449,7 +540,16 @@ function updatePnpmAllowBuildsConfigContentWithSource( endIndex - beginIndex + 1, ...buildAllowBuildsManagedBlock(filteredSource, indent), ); - return formatPnpmWorkspaceConfigContent(lines); + const deniedPackages = insertAutoDeniedBuilds( + lines, + source, + autoDeniedPackageNames, + ); + return { + content: formatPnpmWorkspaceConfigContent(lines), + promotedPackages, + deniedPackages, + }; } const range = getTopLevelAllowBuildsRange(lines); @@ -466,15 +566,34 @@ function updatePnpmAllowBuildsConfigContentWithSource( 0, ...buildAllowBuildsManagedBlock(filteredSource, " "), ); - return formatPnpmWorkspaceConfigContent(lines); + const deniedPackages = insertAutoDeniedBuilds( + lines, + source, + autoDeniedPackageNames, + ); + return { + content: formatPnpmWorkspaceConfigContent(lines), + promotedPackages, + deniedPackages, + }; } const prefix = lines.length > 0 ? [...lines, ""] : []; - return formatPnpmWorkspaceConfigContent([ + const nextLines = [ ...prefix, "allowBuilds:", ...buildAllowBuildsManagedBlock(source, " "), - ]); + ]; + const deniedPackages = insertAutoDeniedBuilds( + nextLines, + source, + autoDeniedPackageNames, + ); + return { + content: formatPnpmWorkspaceConfigContent(nextLines), + promotedPackages, + deniedPackages, + }; } async function fetchRemoteAllowBuildsSource( @@ -578,7 +697,7 @@ export async function ensurePnpmAllowBuildsConfigured({ appPath: string; allowBuildsText?: string; remoteAllowBuildsTextFetcher?: AllowBuildsTextFetcher; -}): Promise<{ changed: boolean }> { +}): Promise<{ changed: boolean; promotedPackages: string[] }> { const configPath = path.join(appPath, "pnpm-workspace.yaml"); try { let existingContent = ""; @@ -595,39 +714,47 @@ export async function ensurePnpmAllowBuildsConfigured({ allowBuildsText, remoteAllowBuildsTextFetcher, }); - const nextContent = allowBuildsSource + const updateResult = allowBuildsSource ? updatePnpmAllowBuildsConfigContentWithSource( existingContent, allowBuildsSource, ) - : formatPnpmWorkspaceConfigContent( - existingContent - ? existingContent.split(/\r?\n/).filter((_, index, lines) => { - return index !== lines.length - 1 || lines[index] !== ""; - }) - : [], - ); + : { + content: formatPnpmWorkspaceConfigContent( + existingContent + ? existingContent.split(/\r?\n/).filter((_, index, lines) => { + return index !== lines.length - 1 || lines[index] !== ""; + }) + : [], + ), + promotedPackages: [], + deniedPackages: [], + }; + const nextContent = updateResult.content; if (nextContent === existingContent) { - return { changed: false }; + return { changed: false, promotedPackages: [] }; } await fs.mkdir(path.dirname(configPath), { recursive: true }); const tempPath = `${configPath}.tmp`; await fs.writeFile(tempPath, nextContent); await fs.rename(tempPath, configPath); - return { changed: true }; + return { + changed: true, + promotedPackages: updateResult.promotedPackages, + }; } catch (error) { logger.warn("Failed to update pnpm allowBuilds config:", error); - return { changed: false }; + return { changed: false, promotedPackages: [] }; } } export async function commitPnpmAllowBuildsConfigIfChanged( appPath: string, -): Promise { +): Promise<{ promotedPackages: string[] }> { const result = await ensurePnpmAllowBuildsConfigured({ appPath }); if (!result.changed) { - return; + return { promotedPackages: result.promotedPackages }; } try { @@ -639,6 +766,186 @@ export async function commitPnpmAllowBuildsConfigIfChanged( } catch (error) { logger.warn("Failed to commit pnpm allowBuilds config:", error); } + return { promotedPackages: result.promotedPackages }; +} + +export function getPnpmIgnoredBuildPackageName(packageSpec: string): string { + const atIndex = packageSpec.lastIndexOf("@"); + if (atIndex <= 0) { + return packageSpec.trim(); + } + return packageSpec.slice(0, atIndex).trim(); +} + +function parseIgnoredBuildsInlineValue(value: string): string[] { + const trimmedValue = value.trim(); + if (!trimmedValue || trimmedValue === "[]") { + return []; + } + + if (trimmedValue.startsWith("[") && trimmedValue.endsWith("]")) { + return trimmedValue + .slice(1, -1) + .split(",") + .map((entry) => entry.trim().replace(/^['"]|['"]$/g, "")) + .filter(Boolean); + } + + return []; +} + +export function parsePnpmIgnoredBuildsFromModulesYaml( + content: string, +): PnpmIgnoredBuild[] { + const lines = content.split(/\r?\n/); + const packageSpecs: string[] = []; + let inIgnoredBuilds = false; + + for (const line of lines) { + const ignoredBuildsMatch = line.match(/^ignoredBuilds:\s*(.*)$/); + if (ignoredBuildsMatch) { + inIgnoredBuilds = true; + packageSpecs.push( + ...parseIgnoredBuildsInlineValue(ignoredBuildsMatch[1]), + ); + continue; + } + + if (!inIgnoredBuilds) { + continue; + } + + if (line.trim() && !/^\s/.test(line)) { + break; + } + + const listItemMatch = line.match(/^\s*-\s*(.+?)\s*$/); + if (listItemMatch) { + packageSpecs.push(listItemMatch[1].replace(/^['"]|['"]$/g, "")); + } + } + + return Array.from(new Set(packageSpecs)) + .filter(Boolean) + .map((packageSpec) => ({ + packageName: getPnpmIgnoredBuildPackageName(packageSpec), + packageSpec, + })); +} + +export function parsePnpmIgnoredBuildsFromOutput( + output: string, +): PnpmIgnoredBuild[] { + const match = output.match(/Ignored build scripts:\s*([^\n\r]+)/i); + if (!match) { + return []; + } + + return match[1] + .split(",") + .map((entry) => entry.trim()) + .filter(Boolean) + .map((packageSpec) => ({ + packageName: getPnpmIgnoredBuildPackageName(packageSpec), + packageSpec, + })); +} + +export async function readPnpmIgnoredBuilds( + appPath: string, +): Promise { + try { + const modulesYamlPath = path.join(appPath, "node_modules", ".modules.yaml"); + const content = await fs.readFile(modulesYamlPath, "utf8"); + return parsePnpmIgnoredBuildsFromModulesYaml(content); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + logger.debug("Failed to read pnpm ignored builds:", error); + } + return []; + } +} + +export function isPnpmIgnoredBuildsError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return message.includes(PNPM_IGNORED_BUILDS_ERROR_CODE); +} + +export async function recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + allowBuildsText, + remoteAllowBuildsTextFetcher, +}: { + appPath: string; + ignoredBuilds: PnpmIgnoredBuild[]; + allowBuildsText?: string; + remoteAllowBuildsTextFetcher?: AllowBuildsTextFetcher; +}): Promise<{ deniedBuilds: PnpmIgnoredBuild[] }> { + const packageNames = ignoredBuilds.map( + (ignoredBuild) => ignoredBuild.packageName, + ); + if (packageNames.length === 0) { + return { deniedBuilds: [] }; + } + + const configPath = path.join(appPath, "pnpm-workspace.yaml"); + try { + let existingContent = ""; + try { + existingContent = await fs.readFile(configPath, "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error; + } + } + + const allowBuildsSource = await resolveAllowBuildsSource({ + existingContent, + allowBuildsText, + remoteAllowBuildsTextFetcher, + }); + if (!allowBuildsSource) { + return { deniedBuilds: [] }; + } + + const updateResult = updatePnpmAllowBuildsConfigContentWithSource( + existingContent, + allowBuildsSource, + packageNames, + ); + if (updateResult.content === existingContent) { + return { deniedBuilds: [] }; + } + + await fs.mkdir(path.dirname(configPath), { recursive: true }); + const tempPath = `${configPath}.tmp`; + await fs.writeFile(tempPath, updateResult.content); + await fs.rename(tempPath, configPath); + + const deniedPackageNameSet = new Set(updateResult.deniedPackages); + const deniedBuilds = ignoredBuilds.filter((ignoredBuild) => + deniedPackageNameSet.has(ignoredBuild.packageName), + ); + if (deniedBuilds.length === 0) { + return { deniedBuilds: [] }; + } + + try { + await gitAdd({ path: appPath, filepath: "pnpm-workspace.yaml" }); + await gitCommit({ + path: appPath, + message: "[dyad] record denied pnpm dependency builds", + }); + } catch (error) { + logger.warn("Failed to commit denied pnpm builds config:", error); + } + + return { deniedBuilds }; + } catch (error) { + logger.warn("Failed to record denied pnpm builds:", error); + return { deniedBuilds: [] }; + } } export function resolveExecutableName( diff --git a/src/lib/posthogTelemetry.test.ts b/src/lib/posthogTelemetry.test.ts index e4c24589b3..d9982d5f55 100644 --- a/src/lib/posthogTelemetry.test.ts +++ b/src/lib/posthogTelemetry.test.ts @@ -177,6 +177,15 @@ describe("shouldBypassNonProTelemetrySampling", () => { ).toBe(true); }); + it("always sends pnpm build policy telemetry for non-Pro sampling", () => { + expect( + shouldBypassNonProTelemetrySampling({ + event: "pnpm:build-auto-denied", + properties: { packages: ["core-js@3.49.0"] }, + }), + ).toBe(true); + }); + it("does not bypass unrelated sandbox telemetry", () => { expect( shouldBypassNonProTelemetrySampling({ diff --git a/src/lib/posthogTelemetry.ts b/src/lib/posthogTelemetry.ts index 0999eacf2f..9be4c34471 100644 --- a/src/lib/posthogTelemetry.ts +++ b/src/lib/posthogTelemetry.ts @@ -86,6 +86,10 @@ export function shouldBypassNonProTelemetrySampling( return true; } + if (eventName?.startsWith("pnpm:build-")) { + return true; + } + if (eventName === "app:initial-load") { return true; } From e656ab7f11dafb8a80bba3f3c31a7d44cb0458b8 Mon Sep 17 00:00:00 2001 From: Will Chen <7344640+wwwillchen@users.noreply.github.com> Date: Wed, 8 Jul 2026 17:35:31 -0700 Subject: [PATCH 2/6] Add e2e coverage for pnpm ignored builds --- e2e-tests/package_manager.spec.ts | 121 +++++++++++++++++++ src/ipc/services/app_runtime_service.test.ts | 113 ++++++++++++++++- src/ipc/services/app_runtime_service.ts | 5 + 3 files changed, 235 insertions(+), 4 deletions(-) diff --git a/e2e-tests/package_manager.spec.ts b/e2e-tests/package_manager.spec.ts index c0e198f0df..f6101d52d3 100644 --- a/e2e-tests/package_manager.spec.ts +++ b/e2e-tests/package_manager.spec.ts @@ -217,6 +217,15 @@ const upgradePnpmTestSkipIfWindows = testWithConfigSkipIfWindows({ postLaunchHook: restorePackageManagerCache, }); +const realPnpmStrictBuildsTestSkipIfWindows = testWithConfigSkipIfWindows({ + preLaunchHook: async ({ userDataDir, fakeLlmPort }) => { + execFileSync("pnpm", ["--version"], { encoding: "utf8" }); + await configurePackageManagerCache(userDataDir); + process.env.DYAD_DEFAULT_APPROVE_BUILDS_URL = `http://localhost:${fakeLlmPort}/api/default-approve-builds.txt`; + }, + postLaunchHook: restorePackageManagerCache, +}); + async function openMinimalBuildChat(po: PageObject) { await po.setUp(); await po.page.evaluate( @@ -252,6 +261,62 @@ async function openMinimalBuildChat(po: PageObject) { }; } +async function getCurrentAppId(po: PageObject): Promise { + const appPath = await po.appManagement.getCurrentAppPath(); + const currentAppName = await po.appManagement.getCurrentAppName(); + const apps = await po.page.evaluate(async () => { + return (window as any).electron.ipcRenderer.invoke("list-apps", undefined); + }); + const matchingApp = apps.apps.find( + (app: { id: number; name: string; resolvedPath?: string }) => + app.resolvedPath === appPath || app.name === currentAppName, + ); + if (!matchingApp) { + throw new Error(`Could not find current app ${currentAppName}`); + } + return matchingApp.id; +} + +async function addLocalDependencyWithIgnoredBuild(appPath: string) { + const dependencyDir = path.join(appPath, "packages", "fake-build-dep"); + await fs.mkdir(dependencyDir, { recursive: true }); + await fs.writeFile( + path.join(dependencyDir, "package.json"), + `${JSON.stringify( + { + name: "fake-build-dep", + version: "1.0.0", + scripts: { + postinstall: "node postinstall.js", + }, + }, + null, + 2, + )}\n`, + ); + await fs.writeFile( + path.join(dependencyDir, "postinstall.js"), + [ + 'require("node:fs").writeFileSync(', + ' require("node:path").join(__dirname, "postinstall-ran.txt"),', + ' "yes",', + ");", + "", + ].join("\n"), + ); + + const packageJsonPath = path.join(appPath, "package.json"); + const packageJson = JSON.parse(await fs.readFile(packageJsonPath, "utf8")); + packageJson.dependencies = { + ...packageJson.dependencies, + "fake-build-dep": "file:./packages/fake-build-dep", + }; + await fs.writeFile( + packageJsonPath, + `${JSON.stringify(packageJson, null, 2)}\n`, + ); +} + function extendSocketFirewallTestTimeout(testInfo: TestInfo) { testInfo.setTimeout(SOCKET_FIREWALL_TEST_TIMEOUT); } @@ -476,3 +541,59 @@ upgradePnpmTestSkipIfWindows( await expect(warningBanner).toBeHidden({ timeout: Timeout.MEDIUM }); }, ); + +realPnpmStrictBuildsTestSkipIfWindows( + "custom pnpm install auto-denies ignored builds and recovers preview", + async ({ po }, testInfo) => { + testInfo.setTimeout(SOCKET_FIREWALL_TEST_TIMEOUT); + + await po.setUp(); + await po.importApp("minimal"); + await po.previewPanel.expectPreviewIframeIsVisible( + SOCKET_FIREWALL_TEST_TIMEOUT, + ); + + const appPath = await po.appManagement.getCurrentAppPath(); + const appId = await getCurrentAppId(po); + await addLocalDependencyWithIgnoredBuild(appPath); + + await po.page.evaluate( + async ({ appId }) => { + await (window as any).electron.ipcRenderer.invoke( + "update-app-commands", + { + appId, + installCommand: "pnpm --config.strictDepBuilds=true install", + startCommand: "pnpm run dev -- --host 127.0.0.1", + }, + ); + }, + { appId }, + ); + + await po.previewPanel.clickRebuild(); + await expect(po.previewPanel.locateLoadingAppPreview()).toBeVisible({ + timeout: Timeout.MEDIUM, + }); + await po.previewPanel.expectPreviewIframeIsVisible( + SOCKET_FIREWALL_TEST_TIMEOUT, + ); + + const pnpmWorkspaceConfig = await fs.readFile( + path.join(appPath, "pnpm-workspace.yaml"), + "utf8", + ); + expect(pnpmWorkspaceConfig).toContain( + "fake-build-dep: false # dyad-auto-denied", + ); + await expect(async () => { + const modulesConfig = await fs.readFile( + path.join(appPath, "node_modules", ".modules.yaml"), + "utf8", + ); + expect(modulesConfig).not.toContain( + "fake-build-dep@file:packages/fake-build-dep", + ); + }).toPass({ timeout: Timeout.EXTRA_LONG }); + }, +); diff --git a/src/ipc/services/app_runtime_service.test.ts b/src/ipc/services/app_runtime_service.test.ts index 857fb1038e..4045571675 100644 --- a/src/ipc/services/app_runtime_service.test.ts +++ b/src/ipc/services/app_runtime_service.test.ts @@ -1,6 +1,6 @@ import { EventEmitter } from "node:events"; import type { ChildProcess } from "node:child_process"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import type { IpcMainInvokeEvent, WebContents } from "electron"; @@ -10,7 +10,10 @@ const { getPnpmMinimumReleaseAgeSupportMock, ensurePnpmAllowBuildsConfiguredMock, readSettingsMock, + readPnpmIgnoredBuildsMock, + recordDeniedPnpmBuildsMock, safeSendMock, + sendTelemetryEventMock, spawnMock, killPortMock, startProxyMock, @@ -34,7 +37,10 @@ const { readSettingsMock: vi.fn<() => Record>(() => ({ runtimeMode2: "host", })), + readPnpmIgnoredBuildsMock: vi.fn(), + recordDeniedPnpmBuildsMock: vi.fn(), safeSendMock: vi.fn(), + sendTelemetryEventMock: vi.fn(), spawnMock: vi.fn(), killPortMock: vi.fn<() => Promise>(async () => {}), startProxyMock: vi.fn(), @@ -89,8 +95,10 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({ isPnpmIgnoredBuildsError: (error: unknown) => String(error).includes("ERR_PNPM_IGNORED_BUILDS"), parsePnpmIgnoredBuildsFromOutput: vi.fn(() => []), - readPnpmIgnoredBuilds: vi.fn(async () => []), - recordDeniedPnpmBuilds: vi.fn(async () => ({ deniedBuilds: [] })), + readPnpmIgnoredBuilds: (...args: unknown[]) => + readPnpmIgnoredBuildsMock(...args), + recordDeniedPnpmBuilds: (...args: unknown[]) => + recordDeniedPnpmBuildsMock(...args), PNPM_INSTALL_POLICY_ARGS: [ "--config.pm-on-fail=ignore", "--minimum-release-age=1440", @@ -99,7 +107,7 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({ })); vi.mock("@/ipc/utils/telemetry", () => ({ - sendTelemetryEvent: vi.fn(), + sendTelemetryEvent: (...args: unknown[]) => sendTelemetryEventMock(...args), })); vi.mock("@/ipc/utils/cloud_sandbox_provider", () => ({ @@ -190,6 +198,24 @@ async function withCorepackProjectSpecEnv( } } +async function waitForAssertion(assertion: () => void): Promise { + const deadline = Date.now() + 1_000; + let lastError: unknown; + while (Date.now() < deadline) { + try { + assertion(); + return; + } catch (error) { + lastError = error; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + } + if (lastError) { + throw lastError; + } + assertion(); +} + describe("executeApp", () => { beforeEach(() => { runningApps.clear(); @@ -208,7 +234,12 @@ describe("executeApp", () => { readSettingsMock.mockReturnValue({ runtimeMode2: "host", }); + readPnpmIgnoredBuildsMock.mockReset(); + readPnpmIgnoredBuildsMock.mockResolvedValue([]); + recordDeniedPnpmBuildsMock.mockReset(); + recordDeniedPnpmBuildsMock.mockResolvedValue({ deniedBuilds: [] }); safeSendMock.mockReset(); + sendTelemetryEventMock.mockReset(); spawnMock.mockReset(); killPortMock.mockReset(); killPortMock.mockResolvedValue(undefined); @@ -582,6 +613,80 @@ describe("executeApp", () => { }); }); + it("clears node_modules before retrying custom pnpm commands after ignored builds are denied", async () => { + const appPath = await createTempAppDir(); + const nodeModulesPath = path.join(appPath, "node_modules"); + await mkdir(nodeModulesPath, { recursive: true }); + readPnpmIgnoredBuildsMock.mockResolvedValue([ + { + packageSpec: "fake-build-dep@file:packages/fake-build-dep", + packageName: "fake-build-dep", + }, + ]); + recordDeniedPnpmBuildsMock.mockResolvedValue({ + deniedBuilds: [ + { + packageSpec: "fake-build-dep@file:packages/fake-build-dep", + packageName: "fake-build-dep", + }, + ], + }); + + try { + const firstProcess = new FakeChildProcess(101); + const secondProcess = new FakeChildProcess(102); + spawnMock + .mockReturnValueOnce(firstProcess) + .mockReturnValueOnce(secondProcess); + + await executeApp({ + appPath, + appId: 1, + event: createEvent(), + isNeon: false, + installCommand: "pnpm --config.strictDepBuilds=true install", + startCommand: "pnpm run dev", + }); + + firstProcess.stderr.emit( + "data", + "ERR_PNPM_IGNORED_BUILDS Ignored build scripts: fake-build-dep@file:packages/fake-build-dep", + ); + firstProcess.emit("close", 1, null); + + await waitForAssertion(() => { + expect(spawnMock).toHaveBeenCalledTimes(2); + }); + await expect(stat(nodeModulesPath)).rejects.toThrow(); + expect(recordDeniedPnpmBuildsMock).toHaveBeenCalledWith({ + appPath, + ignoredBuilds: [ + { + packageSpec: "fake-build-dep@file:packages/fake-build-dep", + packageName: "fake-build-dep", + }, + ], + }); + expect(sendTelemetryEventMock).toHaveBeenCalledWith( + "pnpm:build-auto-denied", + { + packages: ["fake-build-dep@file:packages/fake-build-dep"], + source: "self-heal", + }, + ); + expect(runningApps.get(1)?.process).toBe( + secondProcess as unknown as ChildProcess, + ); + expect(safeSendMock).not.toHaveBeenCalledWith( + expect.anything(), + "app:output", + expect.objectContaining({ type: "app-exit" }), + ); + } finally { + await rm(appPath, { recursive: true, force: true }); + } + }); + it("starts the proxy on the deterministic port without killing the occupant", async () => { const terminate = vi.fn(); startProxyMock.mockImplementation(async (_originalUrl, opts) => { diff --git a/src/ipc/services/app_runtime_service.ts b/src/ipc/services/app_runtime_service.ts index f8bd41afe9..a73d134f1a 100644 --- a/src/ipc/services/app_runtime_service.ts +++ b/src/ipc/services/app_runtime_service.ts @@ -785,6 +785,11 @@ async function selfHealDeniedPnpmBuilds({ return false; } + await fs.promises.rm(path.join(appPath, "node_modules"), { + recursive: true, + force: true, + }); + sendTelemetryEvent("pnpm:build-auto-denied", { packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), source: telemetrySource, From 8a4ed5453060e576abab632b833fd179ce44719e Mon Sep 17 00:00:00 2001 From: Will Chen <7344640+wwwillchen@users.noreply.github.com> Date: Wed, 8 Jul 2026 17:57:04 -0700 Subject: [PATCH 3/6] Fix review findings: JSON .modules.yaml parsing, Windows-safe rebuild chain, bounded output tail - pnpm 10.x/11.x write node_modules/.modules.yaml as JSON; parse JSON first with the block-YAML line parser as fallback, so the proactive auto-deny path actually detects ignored builds on real installs - Replace the inline "(pnpm rebuild 'x' || true)" chain with a shared getBestEffortPnpmRebuildCommand that emits unquoted validated names and an echo fallback, both of which work under cmd.exe as well as sh - Cap the self-heal process-output buffer at a 64KB rolling tail and only accumulate when a self-heal callback is attached, so long-running dev servers don't grow memory unboundedly - Strip the trailing period/box borders in the ignored-builds output parser - Add unit coverage: real JSON .modules.yaml fixture, rebuild command quoting/filtering, output-parser forms Co-Authored-By: Claude Fable 5 --- src/ipc/processors/executeAddDependency.ts | 3 + src/ipc/services/app_runtime_service.test.ts | 6 +- src/ipc/services/app_runtime_service.ts | 36 +++---- src/ipc/utils/cloud_sandbox_provider.test.ts | 2 +- src/ipc/utils/cloud_sandbox_provider.ts | 12 +-- src/ipc/utils/socket_firewall.test.ts | 99 +++++++++++++++++--- src/ipc/utils/socket_firewall.ts | 77 +++++++++++++-- 7 files changed, 187 insertions(+), 48 deletions(-) diff --git a/src/ipc/processors/executeAddDependency.ts b/src/ipc/processors/executeAddDependency.ts index 72a46c7dc2..a9056e0e7a 100644 --- a/src/ipc/processors/executeAddDependency.ts +++ b/src/ipc/processors/executeAddDependency.ts @@ -278,6 +278,9 @@ export async function installPackages({ let installResultsWithPolicyNotes = installResults; if (packageManager === "pnpm") { const ignoredBuilds = await readPnpmIgnoredBuilds(appPath); + // Promotions were already applied (and rebuilt) by the pre-install + // commitPnpmAllowBuildsConfigIfChanged call above, so this record pass + // only ever adds denials for builds the install just ignored. const { deniedBuilds } = await recordDeniedPnpmBuilds({ appPath, ignoredBuilds, diff --git a/src/ipc/services/app_runtime_service.test.ts b/src/ipc/services/app_runtime_service.test.ts index 4045571675..f58abf5ff3 100644 --- a/src/ipc/services/app_runtime_service.test.ts +++ b/src/ipc/services/app_runtime_service.test.ts @@ -92,6 +92,10 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({ npm_config_pm_on_fail: "ignore", }), getPnpmMinimumReleaseAgeSupport: () => getPnpmMinimumReleaseAgeSupportMock(), + getBestEffortPnpmRebuildCommand: (packageNames: string[]) => + packageNames.length === 0 + ? null + : `(pnpm rebuild ${packageNames.join(" ")} || echo pnpm rebuild skipped)`, isPnpmIgnoredBuildsError: (error: unknown) => String(error).includes("ERR_PNPM_IGNORED_BUILDS"), parsePnpmIgnoredBuildsFromOutput: vi.fn(() => []), @@ -374,7 +378,7 @@ describe("executeApp", () => { }); expect(spawnMock).toHaveBeenCalledWith( - "pnpm --config.pm-on-fail=ignore --minimum-release-age=1440 install && (pnpm rebuild 'core-js' '@scope/native' || true) && pnpm --config.pm-on-fail=ignore run dev --port 32101", + "pnpm --config.pm-on-fail=ignore --minimum-release-age=1440 install && (pnpm rebuild core-js @scope/native || echo pnpm rebuild skipped) && pnpm --config.pm-on-fail=ignore run dev --port 32101", [], expect.objectContaining({ cwd: "/tmp/app", diff --git a/src/ipc/services/app_runtime_service.ts b/src/ipc/services/app_runtime_service.ts index a73d134f1a..3ee4659cc6 100644 --- a/src/ipc/services/app_runtime_service.ts +++ b/src/ipc/services/app_runtime_service.ts @@ -43,6 +43,7 @@ import { PNPM_INSTALL_POLICY_ARGS, readPnpmIgnoredBuilds, recordDeniedPnpmBuilds, + getBestEffortPnpmRebuildCommand, } from "@/ipc/utils/socket_firewall"; import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; import { @@ -95,20 +96,6 @@ function getPnpmRunCommand(): string { return `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} run dev`; } -function quoteShellArg(value: string): string { - return `'${value.replace(/'/g, "'\\''")}'`; -} - -function getBestEffortPnpmRebuildCommand( - packageNames: string[], -): string | null { - if (packageNames.length === 0) { - return null; - } - - return `(pnpm rebuild ${packageNames.map(quoteShellArg).join(" ")} || true)`; -} - function buildPnpmInstallAndRunCommand(input: { promotedPackages: string[]; port: number; @@ -639,10 +626,23 @@ function listenToProcess({ event: Electron.IpcMainInvokeEvent; onPnpmIgnoredBuildsFailure?: (output: string) => Promise; }) { + // Rolling tail, kept only while a self-heal callback could still use it: + // dev servers run for hours and unbounded accumulation would leak memory. + // The ERR_PNPM_IGNORED_BUILDS marker appears at the end of a failed + // install, so a bounded tail is sufficient for the close-handler check. + const MAX_PROCESS_OUTPUT_TAIL_LENGTH = 64 * 1024; let processOutput = ""; + const appendProcessOutput = (message: string) => { + if (!onPnpmIgnoredBuildsFailure) { + return; + } + processOutput = (processOutput + message).slice( + -MAX_PROCESS_OUTPUT_TAIL_LENGTH, + ); + }; spawnedProcess.stdout?.on("data", async (data) => { const message = util.stripVTControlCharacters(data.toString()); - processOutput += message; + appendProcessOutput(message); logger.debug( `App ${appId} (PID: ${spawnedProcess.pid}) stdout: ${message}`, ); @@ -692,7 +692,7 @@ function listenToProcess({ spawnedProcess.stderr?.on("data", async (data) => { const message = util.stripVTControlCharacters(data.toString()); - processOutput += message; + appendProcessOutput(message); logger.error( `App ${appId} (PID: ${spawnedProcess.pid}) stderr: ${message}`, ); @@ -777,6 +777,10 @@ async function selfHealDeniedPnpmBuilds({ ignoredBuildsFromModulesYaml.length > 0 ? ignoredBuildsFromModulesYaml : parsePnpmIgnoredBuildsFromOutput(output); + // recordDeniedPnpmBuilds may also promote previously auto-denied packages + // as a side effect; no explicit `pnpm rebuild` is needed here because + // node_modules is removed below, so the retry's fresh install runs build + // scripts for newly-allowed packages natively. const { deniedBuilds } = await recordDeniedPnpmBuilds({ appPath, ignoredBuilds, diff --git a/src/ipc/utils/cloud_sandbox_provider.test.ts b/src/ipc/utils/cloud_sandbox_provider.test.ts index 8f762e1fbd..d80c3fa61f 100644 --- a/src/ipc/utils/cloud_sandbox_provider.test.ts +++ b/src/ipc/utils/cloud_sandbox_provider.test.ts @@ -563,7 +563,7 @@ describe("cloud_sandbox_provider sandbox creation", () => { appId: 42, appPath: "/tmp/app", installCommand: - "pnpm --config.pm-on-fail=ignore --config.confirmModulesPurge=false --config.strictDepBuilds=false install && (pnpm rebuild 'core-js' '@scope/native' || true)", + "pnpm --config.pm-on-fail=ignore --config.confirmModulesPurge=false --config.strictDepBuilds=false install && (pnpm rebuild core-js @scope/native || echo pnpm rebuild skipped)", startCommand: "pnpm --config.pm-on-fail=ignore run dev", }); }); diff --git a/src/ipc/utils/cloud_sandbox_provider.ts b/src/ipc/utils/cloud_sandbox_provider.ts index 3315c41b89..4bd1bc6962 100644 --- a/src/ipc/utils/cloud_sandbox_provider.ts +++ b/src/ipc/utils/cloud_sandbox_provider.ts @@ -5,6 +5,7 @@ import path from "node:path"; import log from "electron-log"; import { commitPnpmAllowBuildsConfigIfChanged, + getBestEffortPnpmRebuildCommand, PNPM_INSTALL_POLICY_ARGS, PNPM_PM_ON_FAIL_IGNORE_ARG, } from "@/ipc/utils/socket_firewall"; @@ -138,21 +139,16 @@ function getDefaultStartCommand(): string { return `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} run dev`; } -function quoteShellArg(value: string): string { - return `'${value.replace(/'/g, "'\\''")}'`; -} - function appendBestEffortPnpmRebuild( installCommand: string, packageNames: string[], ): string { - if (packageNames.length === 0) { + const rebuildCommand = getBestEffortPnpmRebuildCommand(packageNames); + if (!rebuildCommand) { return installCommand; } - return `${installCommand} && (pnpm rebuild ${packageNames - .map(quoteShellArg) - .join(" ")} || true)`; + return `${installCommand} && ${rebuildCommand}`; } function getDefaultCloudSandboxErrorMessage(status: number): string { diff --git a/src/ipc/utils/socket_firewall.test.ts b/src/ipc/utils/socket_firewall.test.ts index 22b89576c0..fbd953f3d7 100644 --- a/src/ipc/utils/socket_firewall.test.ts +++ b/src/ipc/utils/socket_firewall.test.ts @@ -26,11 +26,13 @@ import { DYAD_ALLOW_BUILDS_CACHE_TTL_MS, ensurePnpmAllowBuildsConfigured, ensureSocketFirewallInstalled, + getBestEffortPnpmRebuildCommand, getManagedPnpmBinDir, getPackageManagerCommandEnv, getPnpmMinimumReleaseAgeSupport, PACKAGE_MANAGER_PROBE_TIMEOUT_MS, parsePnpmIgnoredBuildsFromModulesYaml, + parsePnpmIgnoredBuildsFromOutput, readPnpmIgnoredBuilds, recordDeniedPnpmBuilds, resolveExecutableName, @@ -747,7 +749,39 @@ describe("updatePnpmAllowBuildsConfigContent", () => { }); describe("readPnpmIgnoredBuilds", () => { - it("parses ignored build specs from node_modules/.modules.yaml", async () => { + it("parses the JSON .modules.yaml written by pnpm 10.x/11.x", async () => { + // Captured from a real `pnpm install` with pnpm 11.10.0 — the file is + // JSON despite the .yaml extension. + expect( + parsePnpmIgnoredBuildsFromModulesYaml( + JSON.stringify( + { + hoistedDependencies: {}, + hoistPattern: ["*"], + included: { dependencies: true, devDependencies: true }, + ignoredBuilds: ["core-js@3.49.0", "@scope/native@1.2.3"], + layoutVersion: 5, + pendingBuilds: [], + }, + null, + 2, + ), + ), + ).toEqual([ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + { packageName: "@scope/native", packageSpec: "@scope/native@1.2.3" }, + ]); + }); + + it("returns no ignored builds for JSON .modules.yaml without the key", async () => { + expect( + parsePnpmIgnoredBuildsFromModulesYaml( + JSON.stringify({ layoutVersion: 5, pendingBuilds: [] }), + ), + ).toEqual([]); + }); + + it("parses ignored build specs from block-style YAML .modules.yaml", async () => { expect( parsePnpmIgnoredBuildsFromModulesYaml( [ @@ -769,22 +803,17 @@ describe("readPnpmIgnoredBuilds", () => { const tempDir = await mkdtemp( path.join(os.tmpdir(), "dyad-pnpm-ignored-builds-"), ); + const modulesYamlContent = `${JSON.stringify( + { ignoredBuilds: ["core-js@3.49.0"], layoutVersion: 5 }, + null, + 2, + )}\n`; try { + await mkdir(path.join(tempDir, "node_modules"), { recursive: true }); await writeFile( path.join(tempDir, "node_modules", ".modules.yaml"), - "ignoredBuilds: [core-js@3.49.0]\n", - ).catch(async (error) => { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") { - throw error; - } - await mkdir(path.join(tempDir, "node_modules"), { - recursive: true, - }); - await writeFile( - path.join(tempDir, "node_modules", ".modules.yaml"), - "ignoredBuilds: [core-js@3.49.0]\n", - ); - }); + modulesYamlContent, + ); await expect(readPnpmIgnoredBuilds(tempDir)).resolves.toEqual([ { packageName: "core-js", packageSpec: "core-js@3.49.0" }, @@ -795,6 +824,48 @@ describe("readPnpmIgnoredBuilds", () => { }); }); +describe("parsePnpmIgnoredBuildsFromOutput", () => { + it("parses the strict-mode error line", () => { + expect( + parsePnpmIgnoredBuildsFromOutput( + "[ERR_PNPM_IGNORED_BUILDS] Ignored build scripts: core-js@3.49.0, @scope/native@1.2.3\n", + ), + ).toEqual([ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + { packageName: "@scope/native", packageSpec: "@scope/native@1.2.3" }, + ]); + }); + + it("strips the trailing period and box borders from the warning-box form", () => { + expect( + parsePnpmIgnoredBuildsFromOutput( + "│ Ignored build scripts: core-js@3.49.0. │\n", + ), + ).toEqual([{ packageName: "core-js", packageSpec: "core-js@3.49.0" }]); + }); +}); + +describe("getBestEffortPnpmRebuildCommand", () => { + it("returns null when no packages are promoted", () => { + expect(getBestEffortPnpmRebuildCommand([])).toBeNull(); + }); + + it("emits unquoted names with a cross-shell fallback", () => { + // Single quotes are literal and `true` is not a command under cmd.exe, + // so the command must avoid both to be safe on Windows. + expect(getBestEffortPnpmRebuildCommand(["core-js", "@scope/native"])).toBe( + "(pnpm rebuild core-js @scope/native || echo pnpm rebuild skipped)", + ); + }); + + it("drops names that are not plain npm package names", () => { + expect( + getBestEffortPnpmRebuildCommand(["core-js", "bad name; rm -rf /"]), + ).toBe("(pnpm rebuild core-js || echo pnpm rebuild skipped)"); + expect(getBestEffortPnpmRebuildCommand(["$(evil)"])).toBeNull(); + }); +}); + describe("buildAddDependencyCommand", () => { it.each<[PackageManager, boolean, { command: string; args: string[] }]>([ [ diff --git a/src/ipc/utils/socket_firewall.ts b/src/ipc/utils/socket_firewall.ts index 15c9bc5fb9..ecb9bf99e9 100644 --- a/src/ipc/utils/socket_firewall.ts +++ b/src/ipc/utils/socket_firewall.ts @@ -769,6 +769,26 @@ export async function commitPnpmAllowBuildsConfigIfChanged( return { promotedPackages: result.promotedPackages }; } +const SAFE_PNPM_PACKAGE_NAME_PATTERN = /^(@[a-z0-9-_.]+\/)?[a-z0-9-_.]+$/i; + +export function getBestEffortPnpmRebuildCommand( + packageNames: string[], +): string | null { + // This command string runs under cmd.exe on Windows (where single quotes + // are literal and `true` is not a command) and sh elsewhere, so neither + // quoting nor `|| true` is portable. npm package names never need quoting; + // drop anything that doesn't look like one, and use `echo` — a builtin + // that succeeds in both shells — as the best-effort fallback. + const safePackageNames = packageNames.filter((packageName) => + SAFE_PNPM_PACKAGE_NAME_PATTERN.test(packageName), + ); + if (safePackageNames.length === 0) { + return null; + } + + return `(pnpm rebuild ${safePackageNames.join(" ")} || echo pnpm rebuild skipped)`; +} + export function getPnpmIgnoredBuildPackageName(packageSpec: string): string { const atIndex = packageSpec.lastIndexOf("@"); if (atIndex <= 0) { @@ -794,9 +814,46 @@ function parseIgnoredBuildsInlineValue(value: string): string[] { return []; } +function parseIgnoredBuildsFromJson(content: string): string[] | null { + let parsed: unknown; + try { + parsed = JSON.parse(content); + } catch { + return null; + } + + if (typeof parsed !== "object" || parsed === null) { + return null; + } + + const ignoredBuilds = (parsed as { ignoredBuilds?: unknown }).ignoredBuilds; + if (ignoredBuilds === undefined) { + return []; + } + if (!Array.isArray(ignoredBuilds)) { + return null; + } + + return ignoredBuilds.filter( + (entry): entry is string => typeof entry === "string", + ); +} + export function parsePnpmIgnoredBuildsFromModulesYaml( content: string, ): PnpmIgnoredBuild[] { + // pnpm 10.x/11.x write .modules.yaml as JSON (a YAML subset); older + // versions used block-style YAML. Try JSON first, then the line parser. + const jsonPackageSpecs = parseIgnoredBuildsFromJson(content); + if (jsonPackageSpecs !== null) { + return Array.from(new Set(jsonPackageSpecs)) + .filter(Boolean) + .map((packageSpec) => ({ + packageName: getPnpmIgnoredBuildPackageName(packageSpec), + packageSpec, + })); + } + const lines = content.split(/\r?\n/); const packageSpecs: string[] = []; let inIgnoredBuilds = false; @@ -841,14 +898,18 @@ export function parsePnpmIgnoredBuildsFromOutput( return []; } - return match[1] - .split(",") - .map((entry) => entry.trim()) - .filter(Boolean) - .map((packageSpec) => ({ - packageName: getPnpmIgnoredBuildPackageName(packageSpec), - packageSpec, - })); + return ( + match[1] + .split(",") + // The warning-box form ends the list with a period and pads with box + // border characters; specs never legitimately end with either. + .map((entry) => entry.trim().replace(/[\s│.]+$/u, "")) + .filter(Boolean) + .map((packageSpec) => ({ + packageName: getPnpmIgnoredBuildPackageName(packageSpec), + packageSpec, + })) + ); } export async function readPnpmIgnoredBuilds( From 9f4e09572277fededde509021be1ab0d7dc79ac0 Mon Sep 17 00:00:00 2001 From: Will Chen <7344640+wwwillchen@users.noreply.github.com> Date: Wed, 8 Jul 2026 18:10:10 -0700 Subject: [PATCH 4/6] Address PR review comments - Record ignored builds after successful default app installs (host and Docker) once the dev-server URL appears, so first-install-via-run apps also get pkg:false entries committed - Detect "Ignored build scripts" in cloud sandbox log streams and record denials locally (node_modules never syncs back from the sandbox) - Record denials against existing config when the remote allow-list fetch fails instead of skipping entirely - Wrap the process close-handler async IIFE in a try/catch that falls back to removeAppIfCurrentProcess - Emit a user-visible app:output message when runtime self-heal records denials and reinstalls - Document why the upgrade-path self-heal retry doesn't need to remove node_modules Co-Authored-By: Claude Fable 5 --- src/ipc/handlers/app_handlers.ts | 1 + src/ipc/services/app_runtime_service.test.ts | 102 ++++++++++- src/ipc/services/app_runtime_service.ts | 169 +++++++++++++++---- src/ipc/utils/app_upgrade_utils.ts | 6 + src/ipc/utils/socket_firewall.test.ts | 54 ++++++ src/ipc/utils/socket_firewall.ts | 59 +++++-- 6 files changed, 340 insertions(+), 51 deletions(-) diff --git a/src/ipc/handlers/app_handlers.ts b/src/ipc/handlers/app_handlers.ts index 99c6620892..c4c5f84410 100644 --- a/src/ipc/handlers/app_handlers.ts +++ b/src/ipc/handlers/app_handlers.ts @@ -900,6 +900,7 @@ export function registerAppHandlers() { startCloudSandboxLogStream({ appId, + appPath, event, sandboxId: appInfo.cloudSandboxId, cloudLogAbortController: appInfo.cloudLogAbortController, diff --git a/src/ipc/services/app_runtime_service.test.ts b/src/ipc/services/app_runtime_service.test.ts index f58abf5ff3..3995c03823 100644 --- a/src/ipc/services/app_runtime_service.test.ts +++ b/src/ipc/services/app_runtime_service.test.ts @@ -12,6 +12,7 @@ const { readSettingsMock, readPnpmIgnoredBuildsMock, recordDeniedPnpmBuildsMock, + parsePnpmIgnoredBuildsFromOutputMock, safeSendMock, sendTelemetryEventMock, spawnMock, @@ -39,6 +40,9 @@ const { })), readPnpmIgnoredBuildsMock: vi.fn(), recordDeniedPnpmBuildsMock: vi.fn(), + parsePnpmIgnoredBuildsFromOutputMock: vi.fn< + (output: string) => { packageName: string; packageSpec: string }[] + >(() => []), safeSendMock: vi.fn(), sendTelemetryEventMock: vi.fn(), spawnMock: vi.fn(), @@ -98,7 +102,8 @@ vi.mock("@/ipc/utils/socket_firewall", () => ({ : `(pnpm rebuild ${packageNames.join(" ")} || echo pnpm rebuild skipped)`, isPnpmIgnoredBuildsError: (error: unknown) => String(error).includes("ERR_PNPM_IGNORED_BUILDS"), - parsePnpmIgnoredBuildsFromOutput: vi.fn(() => []), + parsePnpmIgnoredBuildsFromOutput: (output: string) => + parsePnpmIgnoredBuildsFromOutputMock(output), readPnpmIgnoredBuilds: (...args: unknown[]) => readPnpmIgnoredBuildsMock(...args), recordDeniedPnpmBuilds: (...args: unknown[]) => @@ -134,7 +139,12 @@ vi.mock("@/ipc/utils/start_proxy_server", () => ({ startProxy: (...args: unknown[]) => startProxyMock(...args), })); -import { ensureProxyForRunningApp, executeApp } from "./app_runtime_service"; +import { + ensureProxyForRunningApp, + executeApp, + startCloudSandboxLogStream, +} from "./app_runtime_service"; +import { streamCloudSandboxLogs } from "@/ipc/utils/cloud_sandbox_provider"; import { processCounter, runningApps } from "@/ipc/utils/process_manager"; import { DyadError, DyadErrorKind } from "@/errors/dyad_error"; @@ -242,6 +252,8 @@ describe("executeApp", () => { readPnpmIgnoredBuildsMock.mockResolvedValue([]); recordDeniedPnpmBuildsMock.mockReset(); recordDeniedPnpmBuildsMock.mockResolvedValue({ deniedBuilds: [] }); + parsePnpmIgnoredBuildsFromOutputMock.mockReset(); + parsePnpmIgnoredBuildsFromOutputMock.mockReturnValue([]); safeSendMock.mockReset(); sendTelemetryEventMock.mockReset(); spawnMock.mockReset(); @@ -387,6 +399,92 @@ describe("executeApp", () => { ); }); + it("records ignored builds once the default install reaches the dev server", async () => { + const process = new FakeChildProcess(101); + spawnMock.mockReturnValueOnce(process); + getPnpmMinimumReleaseAgeSupportMock.mockResolvedValue({ + available: true, + minimumReleaseAgeSupported: true, + }); + const ignoredBuilds = [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ]; + readPnpmIgnoredBuildsMock.mockResolvedValue(ignoredBuilds); + recordDeniedPnpmBuildsMock.mockResolvedValue({ + deniedBuilds: ignoredBuilds, + }); + + await executeApp({ + appPath: "/tmp/app", + appId: 1, + event: createEvent(), + isNeon: false, + }); + + process.stdout.emit("data", "Local: http://localhost:32101/\n"); + process.stdout.emit("data", "Local: http://localhost:32101/\n"); + + await waitForAssertion(() => { + expect(recordDeniedPnpmBuildsMock).toHaveBeenCalledWith({ + appPath: "/tmp/app", + ignoredBuilds, + }); + expect(sendTelemetryEventMock).toHaveBeenCalledWith( + "pnpm:build-auto-denied", + { + packages: ["core-js@3.49.0"], + source: "app-run", + }, + ); + }); + // One-shot: repeated URL output must not re-record. + expect(recordDeniedPnpmBuildsMock).toHaveBeenCalledTimes(1); + }); + + it("records ignored builds surfaced in cloud sandbox logs", async () => { + const event = createEvent(); + runningApps.set(7, { + process: null, + processId: 1, + mode: "cloud", + rendererSender: event.sender, + cloudSandboxId: "sb-1", + lastViewedAt: Date.now(), + } as any); + const ignoredBuilds = [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ]; + vi.mocked(streamCloudSandboxLogs).mockImplementation(async function* () { + yield "Ignored build scripts: core-js@3.49.0."; + }); + parsePnpmIgnoredBuildsFromOutputMock.mockReturnValue(ignoredBuilds); + recordDeniedPnpmBuildsMock.mockResolvedValue({ + deniedBuilds: ignoredBuilds, + }); + + startCloudSandboxLogStream({ + appId: 7, + appPath: "/tmp/cloud-app", + event, + sandboxId: "sb-1", + cloudLogAbortController: new AbortController(), + }); + + await waitForAssertion(() => { + expect(recordDeniedPnpmBuildsMock).toHaveBeenCalledWith({ + appPath: "/tmp/cloud-app", + ignoredBuilds, + }); + expect(sendTelemetryEventMock).toHaveBeenCalledWith( + "pnpm:build-auto-denied", + { + packages: ["core-js@3.49.0"], + source: "cloud-sandbox", + }, + ); + }); + }); + it("does not warn about old pnpm for apps that explicitly use npm", async () => { const appPath = await createTempAppDir(); try { diff --git a/src/ipc/services/app_runtime_service.ts b/src/ipc/services/app_runtime_service.ts index 3ee4659cc6..d365b5f4fd 100644 --- a/src/ipc/services/app_runtime_service.ts +++ b/src/ipc/services/app_runtime_service.ts @@ -486,6 +486,7 @@ Details: ${details || "n/a"} listenToProcess({ process: spawnedProcess, appId, + appPath, isNeon, event, onPnpmIgnoredBuildsFailure: @@ -500,6 +501,15 @@ Details: ${details || "n/a"} return false; } + // Per "Transparent Over Magical": tell the user why the + // process restarted instead of silently reinstalling. + safeSend(event.sender, "app:output", { + type: "stdout", + message: + "[dyad] pnpm blocked dependency build scripts. Recorded the decision in pnpm-workspace.yaml and reinstalling...", + appId, + }); + await executeAppLocalNode({ appPath, appId, @@ -613,15 +623,40 @@ export function registerCloudSandboxSyncUpdateListener(): void { cloudSandboxSyncUpdateListenerRegistered = true; } +// Records builds that a successful install skipped (the "Ignored build +// scripts" warning path) so the decision lands in pnpm-workspace.yaml and a +// later plain `pnpm install` (export/CI/Rebuild) cannot fail on +// ERR_PNPM_IGNORED_BUILDS. Best-effort: reads [] when .modules.yaml is +// absent (npm apps, Docker-volume installs). +async function recordIgnoredBuildsAfterInstall(appPath: string): Promise { + try { + const ignoredBuilds = await readPnpmIgnoredBuilds(appPath); + const { deniedBuilds } = await recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + }); + if (deniedBuilds.length > 0) { + sendTelemetryEvent("pnpm:build-auto-denied", { + packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), + source: "app-run", + }); + } + } catch (error) { + logger.warn("Failed to record ignored pnpm builds after install:", error); + } +} + function listenToProcess({ process: spawnedProcess, appId, + appPath, isNeon, event, onPnpmIgnoredBuildsFailure, }: { process: ChildProcess; appId: number; + appPath?: string; isNeon: boolean; event: Electron.IpcMainInvokeEvent; onPnpmIgnoredBuildsFailure?: (output: string) => Promise; @@ -632,6 +667,7 @@ function listenToProcess({ // install, so a bounded tail is sufficient for the close-handler check. const MAX_PROCESS_OUTPUT_TAIL_LENGTH = 64 * 1024; let processOutput = ""; + let ignoredBuildsRecordedAfterInstall = false; const appendProcessOutput = (message: string) => { if (!onPnpmIgnoredBuildsFailure) { return; @@ -680,6 +716,13 @@ function listenToProcess({ const urlMatch = message.match(/(https?:\/\/localhost:\d+\/?)/); if (urlMatch) { const originalUrl = urlMatch[1]; + // The dev-server URL appearing means the install phase completed + // successfully — the one point in the `install && dev` chain where + // ignored builds can be read and recorded. + if (appPath && !ignoredBuildsRecordedAfterInstall) { + ignoredBuildsRecordedAfterInstall = true; + void recordIgnoredBuildsAfterInstall(appPath); + } await ensureProxyForRunningApp({ appId, event, @@ -714,44 +757,54 @@ function listenToProcess({ spawnedProcess.on("close", (code, signal) => { void (async () => { - logger.log( - `App ${appId} (PID: ${spawnedProcess.pid}) process closed with code ${code}, signal ${signal}.`, - ); - flushAllAppOutputs(); - const currentAppInfo = runningApps.get(appId); - if (!currentAppInfo || currentAppInfo.process !== spawnedProcess) { - removeAppIfCurrentProcess(appId, spawnedProcess); - return; - } - - if ( - code !== 0 && - onPnpmIgnoredBuildsFailure && - isPnpmIgnoredBuildsError(processOutput) - ) { - let retried = false; - try { - retried = await onPnpmIgnoredBuildsFailure(processOutput); - } catch (error) { - logger.warn( - `Failed to self-heal pnpm ignored builds for app ${appId}:`, - error, - ); - } - if (retried) { + try { + logger.log( + `App ${appId} (PID: ${spawnedProcess.pid}) process closed with code ${code}, signal ${signal}.`, + ); + flushAllAppOutputs(); + const currentAppInfo = runningApps.get(appId); + if (!currentAppInfo || currentAppInfo.process !== spawnedProcess) { + removeAppIfCurrentProcess(appId, spawnedProcess); return; } - } - safeSend(event.sender, "app:output", { - type: "app-exit", - message: `App process exited with code ${code ?? "null"}`, - appId, - exitCode: code, - signal, - timestamp: Date.now(), - }); - removeAppIfCurrentProcess(appId, spawnedProcess); + if ( + code !== 0 && + onPnpmIgnoredBuildsFailure && + isPnpmIgnoredBuildsError(processOutput) + ) { + let retried = false; + try { + retried = await onPnpmIgnoredBuildsFailure(processOutput); + } catch (error) { + logger.warn( + `Failed to self-heal pnpm ignored builds for app ${appId}:`, + error, + ); + } + if (retried) { + return; + } + } + + safeSend(event.sender, "app:output", { + type: "app-exit", + message: `App process exited with code ${code ?? "null"}`, + appId, + exitCode: code, + signal, + timestamp: Date.now(), + }); + removeAppIfCurrentProcess(appId, spawnedProcess); + } catch (error) { + // The close handler is a critical lifecycle point; never let an + // unexpected error leave a stale runningApps entry behind. + logger.error( + `Unexpected error in close handler for app ${appId}:`, + error, + ); + removeAppIfCurrentProcess(appId, spawnedProcess); + } })(); }); @@ -994,6 +1047,7 @@ ${errorOutput || "(empty)"}`, listenToProcess({ process, appId, + appPath, isNeon, event, }); @@ -1086,6 +1140,7 @@ async function executeAppInCloud({ startCloudSandboxLogStream({ appId, + appPath, event, sandboxId, cloudLogAbortController, @@ -1094,10 +1149,52 @@ async function executeAppInCloud({ export function startCloudSandboxLogStream(input: { appId: number; + appPath?: string; event: Electron.IpcMainInvokeEvent; sandboxId: string; cloudLogAbortController: AbortController; }) { + // The sandbox install runs remotely and node_modules is never synced back, + // so the only way to observe ignored builds is the "Ignored build scripts" + // line in the streamed install output. Keep a bounded tail across chunks + // (the line may be split) and record denials locally once, best-effort. + const MAX_LOG_TAIL_LENGTH = 16 * 1024; + let logTail = ""; + let ignoredBuildsRecorded = false; + const maybeRecordIgnoredBuilds = (message: string) => { + if (!input.appPath || ignoredBuildsRecorded) { + return; + } + logTail = (logTail + message).slice(-MAX_LOG_TAIL_LENGTH); + const ignoredBuilds = parsePnpmIgnoredBuildsFromOutput(logTail); + if (ignoredBuilds.length === 0) { + return; + } + ignoredBuildsRecorded = true; + const appPath = input.appPath; + void (async () => { + try { + const { deniedBuilds } = await recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + }); + if (deniedBuilds.length > 0) { + sendTelemetryEvent("pnpm:build-auto-denied", { + packages: deniedBuilds.map( + (ignoredBuild) => ignoredBuild.packageSpec, + ), + source: "cloud-sandbox", + }); + } + } catch (error) { + logger.warn( + "Failed to record ignored pnpm builds from cloud sandbox logs:", + error, + ); + } + })(); + }; + void (async () => { try { for await (const message of streamCloudSandboxLogs( @@ -1109,6 +1206,8 @@ export function startCloudSandboxLogStream(input: { return; } + maybeRecordIgnoredBuilds(message); + addLog({ level: "info", type: "server", diff --git a/src/ipc/utils/app_upgrade_utils.ts b/src/ipc/utils/app_upgrade_utils.ts index 2cab33d5d2..9b7da64c9b 100644 --- a/src/ipc/utils/app_upgrade_utils.ts +++ b/src/ipc/utils/app_upgrade_utils.ts @@ -53,6 +53,12 @@ type ApplyComponentTaggerOptions = { installDependencies?: boolean; }; +// Unlike the app-runtime self-heal, this does NOT remove node_modules before +// the retry. That is safe: pnpm re-evaluates previously ignored builds +// against allowBuilds on every install — an explicit `pkg: false` entry +// silences ERR_PNPM_IGNORED_BUILDS even on an "Already up to date" fast-path +// run (verified against pnpm 11.10) — and these upgrade callers are +// add/install commands whose retry re-links real work anyway. export async function selfHealDeniedPnpmBuildsFromError({ appPath, error, diff --git a/src/ipc/utils/socket_firewall.test.ts b/src/ipc/utils/socket_firewall.test.ts index fbd953f3d7..0fd42cea09 100644 --- a/src/ipc/utils/socket_firewall.test.ts +++ b/src/ipc/utils/socket_firewall.test.ts @@ -746,6 +746,60 @@ describe("updatePnpmAllowBuildsConfigContent", () => { await rm(tempDir, { recursive: true, force: true }); } }); + + it("records denials against existing config when the remote allow-list is unavailable", async () => { + const tempDir = await mkdtemp( + path.join(os.tmpdir(), "dyad-pnpm-deny-offline-"), + ); + const configPath = path.join(tempDir, "pnpm-workspace.yaml"); + try { + const existingConfig = [ + "allowBuilds:", + " # dyad-default-allow-builds begin", + " # dyad-default-allow-builds-schema=v1", + " # dyad-default-allow-builds-data-version=2026-05-21.1", + " # dyad-default-allow-builds-channel=remote", + " sharp: true", + " # dyad-default-allow-builds end", + "packages:", + " - .", + "minimumReleaseAge: 1440", + "", + ].join("\n"); + await writeFile(configPath, existingConfig); + + const failingFetcher = vi.fn(async () => ({ + ok: false, + text: async () => "", + })); + + await expect( + recordDeniedPnpmBuilds({ + appPath: tempDir, + remoteAllowBuildsTextFetcher: failingFetcher, + ignoredBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + { packageName: "sharp", packageSpec: "sharp@0.34.0" }, + ], + }), + ).resolves.toEqual({ + deniedBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ], + }); + + const nextConfig = await readFile(configPath, "utf8"); + expect(nextConfig).toContain("core-js: false # dyad-auto-denied"); + // The remote-managed block must be preserved untouched. + expect(nextConfig).toContain( + "# dyad-default-allow-builds-channel=remote", + ); + expect(nextConfig).toContain("sharp: true"); + expect(nextConfig).not.toContain("sharp: false"); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); }); describe("readPnpmIgnoredBuilds", () => { diff --git a/src/ipc/utils/socket_firewall.ts b/src/ipc/utils/socket_firewall.ts index ecb9bf99e9..31b611586f 100644 --- a/src/ipc/utils/socket_firewall.ts +++ b/src/ipc/utils/socket_firewall.ts @@ -442,10 +442,10 @@ function removeAutoDeniedPromotedBuilds( function insertAutoDeniedBuilds( lines: string[], - source: AllowBuildsSource, + allowedPackages: ReadonlySet, packageNames: string[], ): string[] { - const sourcePackageSet = new Set(source.packages); + const sourcePackageSet = allowedPackages; const range = getTopLevelAllowBuildsRange(lines); const existingKeys = parseAllowBuildsExistingKeys( range ? lines.slice(range.start + 1, range.end) : [], @@ -542,7 +542,7 @@ function updatePnpmAllowBuildsConfigContentWithSource( ); const deniedPackages = insertAutoDeniedBuilds( lines, - source, + new Set(source.packages), autoDeniedPackageNames, ); return { @@ -568,7 +568,7 @@ function updatePnpmAllowBuildsConfigContentWithSource( ); const deniedPackages = insertAutoDeniedBuilds( lines, - source, + new Set(source.packages), autoDeniedPackageNames, ); return { @@ -586,7 +586,7 @@ function updatePnpmAllowBuildsConfigContentWithSource( ]; const deniedPackages = insertAutoDeniedBuilds( nextLines, - source, + new Set(source.packages), autoDeniedPackageNames, ); return { @@ -932,6 +932,35 @@ export function isPnpmIgnoredBuildsError(error: unknown): boolean { return message.includes(PNPM_IGNORED_BUILDS_ERROR_CODE); } +function insertAutoDeniedBuildsIntoContent( + existingContent: string, + packageNames: string[], +): { content: string; promotedPackages: string[]; deniedPackages: string[] } { + const lines = existingContent ? existingContent.split(/\r?\n/) : []; + if (lines.at(-1) === "") { + lines.pop(); + } + + const deniedPackages = insertAutoDeniedBuilds( + lines, + new Set(), + packageNames, + ); + if (deniedPackages.length === 0) { + return { + content: existingContent, + promotedPackages: [], + deniedPackages: [], + }; + } + + return { + content: formatPnpmWorkspaceConfigContent(lines), + promotedPackages: [], + deniedPackages, + }; +} + export async function recordDeniedPnpmBuilds({ appPath, ignoredBuilds, @@ -966,15 +995,17 @@ export async function recordDeniedPnpmBuilds({ allowBuildsText, remoteAllowBuildsTextFetcher, }); - if (!allowBuildsSource) { - return { deniedBuilds: [] }; - } - - const updateResult = updatePnpmAllowBuildsConfigContentWithSource( - existingContent, - allowBuildsSource, - packageNames, - ); + const updateResult = allowBuildsSource + ? updatePnpmAllowBuildsConfigContentWithSource( + existingContent, + allowBuildsSource, + packageNames, + ) + : // A remote-managed config whose fetch failed (offline/API outage): + // skip the managed rewrite but still record denials against the + // existing content — insertAutoDeniedBuilds already skips packages + // present anywhere in the allowBuilds map, managed block included. + insertAutoDeniedBuildsIntoContent(existingContent, packageNames); if (updateResult.content === existingContent) { return { deniedBuilds: [] }; } From fac6aa705959abf2d6bf3339182f009bac344174 Mon Sep 17 00:00:00 2001 From: Will Chen <7344640+wwwillchen@users.noreply.github.com> Date: Wed, 8 Jul 2026 18:14:00 -0700 Subject: [PATCH 5/6] Wire pnpm ignored-builds self-heal into Docker custom commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Custom install && start chains run strict pnpm inside the container too; mirror the host retry path (no host node_modules cleanup — the container volume is what matters and explicit false entries pass fast-path installs). Co-Authored-By: Claude Fable 5 --- src/ipc/services/app_runtime_service.ts | 54 +++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/src/ipc/services/app_runtime_service.ts b/src/ipc/services/app_runtime_service.ts index d365b5f4fd..15ec3b40fa 100644 --- a/src/ipc/services/app_runtime_service.ts +++ b/src/ipc/services/app_runtime_service.ts @@ -820,10 +820,15 @@ async function selfHealDeniedPnpmBuilds({ appPath, output, telemetrySource, + removeNodeModules = true, }: { appPath: string; output: string; telemetrySource: "self-heal"; + // Docker installs use the container volume, not host node_modules, and an + // explicit `pkg: false` entry passes even a fast-path install — so the + // Docker caller skips the host cleanup. + removeNodeModules?: boolean; }): Promise { const ignoredBuildsFromModulesYaml = await readPnpmIgnoredBuilds(appPath); const ignoredBuilds = @@ -842,10 +847,12 @@ async function selfHealDeniedPnpmBuilds({ return false; } - await fs.promises.rm(path.join(appPath, "node_modules"), { - recursive: true, - force: true, - }); + if (removeNodeModules) { + await fs.promises.rm(path.join(appPath, "node_modules"), { + recursive: true, + force: true, + }); + } sendTelemetryEvent("pnpm:build-auto-denied", { packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), @@ -861,6 +868,7 @@ async function executeAppInDocker({ isNeon, installCommand, startCommand, + ignoredBuildsSelfHealAttempted = false, }: { appPath: string; appId: number; @@ -868,6 +876,7 @@ async function executeAppInDocker({ isNeon: boolean; installCommand?: string | null; startCommand?: string | null; + ignoredBuildsSelfHealAttempted?: boolean; }): Promise { const containerName = `dyad-app-${appId}`; @@ -1044,12 +1053,49 @@ ${errorOutput || "(empty)"}`, lastViewedAt: Date.now(), }); + // Mirrors the host path: custom `install && start` chains run strict pnpm + // inside the container, so an ERR_PNPM_IGNORED_BUILDS exit needs the same + // record-denials-and-retry treatment (executeAppInDocker is restart-safe — + // it stops and removes the previous container first). + const hasCustomCommands = !!installCommand?.trim() && !!startCommand?.trim(); listenToProcess({ process, appId, appPath, isNeon, event, + onPnpmIgnoredBuildsFailure: + hasCustomCommands && !ignoredBuildsSelfHealAttempted + ? async (output) => { + const healed = await selfHealDeniedPnpmBuilds({ + appPath, + output, + telemetrySource: "self-heal", + removeNodeModules: false, + }); + if (!healed) { + return false; + } + + safeSend(event.sender, "app:output", { + type: "stdout", + message: + "[dyad] pnpm blocked dependency build scripts. Recorded the decision in pnpm-workspace.yaml and reinstalling...", + appId, + }); + + await executeAppInDocker({ + appPath, + appId, + event, + isNeon, + installCommand, + startCommand, + ignoredBuildsSelfHealAttempted: true, + }); + return true; + } + : undefined, }); } From d4dad159486795984abbcba6295670a669403707 Mon Sep 17 00:00:00 2001 From: Will Chen <7344640+wwwillchen@users.noreply.github.com> Date: Wed, 8 Jul 2026 19:16:30 -0700 Subject: [PATCH 6/6] Address second round of PR review comments - Convert pnpm 11 placeholder allowBuilds entries ("set this to true or false") into tagged denials during every Dyad rewrite; verified the placeholder is written by non-strict installs, does not satisfy strict mode, and previously shadowed the auto-deny pass entirely - Create the allowBuilds key when the offline denial-only path runs against a config that lost it - Extract recordAndReportDeniedPnpmBuilds + resolvePnpmIgnoredBuilds into pnpm_denied_builds.ts so the record+telemetry contract has one owner across add-dependency, app-run, self-heal, and cloud-sandbox flows Co-Authored-By: Claude Fable 5 --- .../processors/executeAddDependency.test.ts | 41 +++-- src/ipc/processors/executeAddDependency.ts | 16 +- src/ipc/services/app_runtime_service.ts | 44 ++---- src/ipc/utils/app_upgrade_utils.ts | 25 +-- src/ipc/utils/pnpm_denied_builds.ts | 56 +++++++ src/ipc/utils/socket_firewall.test.ts | 142 ++++++++++++++++++ src/ipc/utils/socket_firewall.ts | 75 ++++++++- 7 files changed, 314 insertions(+), 85 deletions(-) create mode 100644 src/ipc/utils/pnpm_denied_builds.ts diff --git a/src/ipc/processors/executeAddDependency.test.ts b/src/ipc/processors/executeAddDependency.test.ts index 559600e0d8..d232aa932c 100644 --- a/src/ipc/processors/executeAddDependency.test.ts +++ b/src/ipc/processors/executeAddDependency.test.ts @@ -17,10 +17,9 @@ const { commitPnpmAllowBuildsConfigIfChangedMock, ensureSocketFirewallInstalledMock, getPnpmMinimumReleaseAgeSupportMock, - readPnpmIgnoredBuildsMock, - recordDeniedPnpmBuildsMock, + resolvePnpmIgnoredBuildsMock, + recordAndReportDeniedPnpmBuildsMock, runCommandMock, - sendTelemetryEventMock, readEffectiveSettingsMock, dbUpdateSetMock, dbUpdateWhereMock, @@ -28,10 +27,9 @@ const { commitPnpmAllowBuildsConfigIfChangedMock: vi.fn(), ensureSocketFirewallInstalledMock: vi.fn(), getPnpmMinimumReleaseAgeSupportMock: vi.fn(), - readPnpmIgnoredBuildsMock: vi.fn(), - recordDeniedPnpmBuildsMock: vi.fn(), + resolvePnpmIgnoredBuildsMock: vi.fn(), + recordAndReportDeniedPnpmBuildsMock: vi.fn(), runCommandMock: vi.fn(), - sendTelemetryEventMock: vi.fn(), readEffectiveSettingsMock: vi.fn(), dbUpdateSetMock: vi.fn(), dbUpdateWhereMock: vi.fn(), @@ -64,14 +62,13 @@ vi.mock("@/ipc/utils/socket_firewall", async () => { commitPnpmAllowBuildsConfigIfChangedMock, ensureSocketFirewallInstalled: ensureSocketFirewallInstalledMock, getPnpmMinimumReleaseAgeSupport: getPnpmMinimumReleaseAgeSupportMock, - readPnpmIgnoredBuilds: readPnpmIgnoredBuildsMock, - recordDeniedPnpmBuilds: recordDeniedPnpmBuildsMock, runCommand: runCommandMock, }; }); -vi.mock("@/ipc/utils/telemetry", () => ({ - sendTelemetryEvent: sendTelemetryEventMock, +vi.mock("@/ipc/utils/pnpm_denied_builds", () => ({ + resolvePnpmIgnoredBuilds: resolvePnpmIgnoredBuildsMock, + recordAndReportDeniedPnpmBuilds: recordAndReportDeniedPnpmBuildsMock, })); describe("executeAddDependency", () => { @@ -89,8 +86,10 @@ describe("executeAddDependency", () => { commitPnpmAllowBuildsConfigIfChangedMock.mockResolvedValue({ promotedPackages: [], }); - readPnpmIgnoredBuildsMock.mockResolvedValue([]); - recordDeniedPnpmBuildsMock.mockResolvedValue({ deniedBuilds: [] }); + resolvePnpmIgnoredBuildsMock.mockResolvedValue([]); + recordAndReportDeniedPnpmBuildsMock.mockResolvedValue({ + deniedBuilds: [], + }); readEffectiveSettingsMock.mockResolvedValue({ blockUnsafeNpmPackages: true, }); @@ -603,10 +602,10 @@ describe("executeAddDependency", () => { stdout: "installed via pnpm", stderr: "", }); - readPnpmIgnoredBuildsMock.mockResolvedValue([ + resolvePnpmIgnoredBuildsMock.mockResolvedValue([ { packageName: "core-js", packageSpec: "core-js@3.49.0" }, ]); - recordDeniedPnpmBuildsMock.mockResolvedValue({ + recordAndReportDeniedPnpmBuildsMock.mockResolvedValue({ deniedBuilds: [{ packageName: "core-js", packageSpec: "core-js@3.49.0" }], }); @@ -624,13 +623,13 @@ describe("executeAddDependency", () => { expect(result.installResults).toContain( "Note: build scripts for core-js were not run (Dyad security policy).", ); - expect(sendTelemetryEventMock).toHaveBeenCalledWith( - "pnpm:build-auto-denied", - { - packages: ["core-js@3.49.0"], - source: "add-dependency", - }, - ); + expect(recordAndReportDeniedPnpmBuildsMock).toHaveBeenCalledWith({ + appPath: "/tmp/app", + ignoredBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ], + source: "add-dependency", + }); expect(dbUpdateSetMock).toHaveBeenCalledWith({ content: expect.stringContaining( "Note: build scripts for core-js were not run", diff --git a/src/ipc/processors/executeAddDependency.ts b/src/ipc/processors/executeAddDependency.ts index a9056e0e7a..8c57505183 100644 --- a/src/ipc/processors/executeAddDependency.ts +++ b/src/ipc/processors/executeAddDependency.ts @@ -12,11 +12,12 @@ import { getCommandExecutionDisplayDetails, getPackageManagerCommandEnv, getPnpmMinimumReleaseAgeSupport, - readPnpmIgnoredBuilds, - recordDeniedPnpmBuilds, runCommand, } from "@/ipc/utils/socket_firewall"; -import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; +import { + recordAndReportDeniedPnpmBuilds, + resolvePnpmIgnoredBuilds, +} from "@/ipc/utils/pnpm_denied_builds"; import { choosePackageManagerFromSignal, getPackageManagerSignal, @@ -277,19 +278,16 @@ export async function installPackages({ let installResultsWithPolicyNotes = installResults; if (packageManager === "pnpm") { - const ignoredBuilds = await readPnpmIgnoredBuilds(appPath); + const ignoredBuilds = await resolvePnpmIgnoredBuilds(appPath); // Promotions were already applied (and rebuilt) by the pre-install // commitPnpmAllowBuildsConfigIfChanged call above, so this record pass // only ever adds denials for builds the install just ignored. - const { deniedBuilds } = await recordDeniedPnpmBuilds({ + const { deniedBuilds } = await recordAndReportDeniedPnpmBuilds({ appPath, ignoredBuilds, + source: "add-dependency", }); if (deniedBuilds.length > 0) { - sendTelemetryEvent("pnpm:build-auto-denied", { - packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), - source: "add-dependency", - }); installResultsWithPolicyNotes += formatDeniedBuildsNote( Array.from( new Set(deniedBuilds.map((ignoredBuild) => ignoredBuild.packageName)), diff --git a/src/ipc/services/app_runtime_service.ts b/src/ipc/services/app_runtime_service.ts index 15ec3b40fa..2afd27dde6 100644 --- a/src/ipc/services/app_runtime_service.ts +++ b/src/ipc/services/app_runtime_service.ts @@ -41,11 +41,12 @@ import { type PackageManager, PNPM_PM_ON_FAIL_IGNORE_ARG, PNPM_INSTALL_POLICY_ARGS, - readPnpmIgnoredBuilds, - recordDeniedPnpmBuilds, getBestEffortPnpmRebuildCommand, } from "@/ipc/utils/socket_firewall"; -import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; +import { + recordAndReportDeniedPnpmBuilds, + resolvePnpmIgnoredBuilds, +} from "@/ipc/utils/pnpm_denied_builds"; import { choosePackageManagerFromSignal, getPackageManagerSignal, @@ -630,17 +631,12 @@ export function registerCloudSandboxSyncUpdateListener(): void { // absent (npm apps, Docker-volume installs). async function recordIgnoredBuildsAfterInstall(appPath: string): Promise { try { - const ignoredBuilds = await readPnpmIgnoredBuilds(appPath); - const { deniedBuilds } = await recordDeniedPnpmBuilds({ + const ignoredBuilds = await resolvePnpmIgnoredBuilds(appPath); + await recordAndReportDeniedPnpmBuilds({ appPath, ignoredBuilds, + source: "app-run", }); - if (deniedBuilds.length > 0) { - sendTelemetryEvent("pnpm:build-auto-denied", { - packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), - source: "app-run", - }); - } } catch (error) { logger.warn("Failed to record ignored pnpm builds after install:", error); } @@ -830,18 +826,15 @@ async function selfHealDeniedPnpmBuilds({ // Docker caller skips the host cleanup. removeNodeModules?: boolean; }): Promise { - const ignoredBuildsFromModulesYaml = await readPnpmIgnoredBuilds(appPath); - const ignoredBuilds = - ignoredBuildsFromModulesYaml.length > 0 - ? ignoredBuildsFromModulesYaml - : parsePnpmIgnoredBuildsFromOutput(output); + const ignoredBuilds = await resolvePnpmIgnoredBuilds(appPath, output); // recordDeniedPnpmBuilds may also promote previously auto-denied packages // as a side effect; no explicit `pnpm rebuild` is needed here because // node_modules is removed below, so the retry's fresh install runs build // scripts for newly-allowed packages natively. - const { deniedBuilds } = await recordDeniedPnpmBuilds({ + const { deniedBuilds } = await recordAndReportDeniedPnpmBuilds({ appPath, ignoredBuilds, + source: telemetrySource, }); if (deniedBuilds.length === 0) { return false; @@ -854,10 +847,6 @@ async function selfHealDeniedPnpmBuilds({ }); } - sendTelemetryEvent("pnpm:build-auto-denied", { - packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), - source: telemetrySource, - }); return true; } @@ -1220,18 +1209,13 @@ export function startCloudSandboxLogStream(input: { const appPath = input.appPath; void (async () => { try { - const { deniedBuilds } = await recordDeniedPnpmBuilds({ + // Output-only on purpose: the install ran remotely, so the local + // .modules.yaml (if any) does not describe this sandbox. + await recordAndReportDeniedPnpmBuilds({ appPath, ignoredBuilds, + source: "cloud-sandbox", }); - if (deniedBuilds.length > 0) { - sendTelemetryEvent("pnpm:build-auto-denied", { - packages: deniedBuilds.map( - (ignoredBuild) => ignoredBuild.packageSpec, - ), - source: "cloud-sandbox", - }); - } } catch (error) { logger.warn( "Failed to record ignored pnpm builds from cloud sandbox logs:", diff --git a/src/ipc/utils/app_upgrade_utils.ts b/src/ipc/utils/app_upgrade_utils.ts index 9b7da64c9b..097909490a 100644 --- a/src/ipc/utils/app_upgrade_utils.ts +++ b/src/ipc/utils/app_upgrade_utils.ts @@ -6,12 +6,12 @@ import { simpleSpawn } from "./simpleSpawn"; import { DyadError, DyadErrorKind } from "@/errors/dyad_error"; import { isPnpmIgnoredBuildsError, - parsePnpmIgnoredBuildsFromOutput, PNPM_PM_ON_FAIL_IGNORE_ARG, - readPnpmIgnoredBuilds, - recordDeniedPnpmBuilds, } from "./socket_firewall"; -import { sendTelemetryEvent } from "./telemetry"; +import { + recordAndReportDeniedPnpmBuilds, + resolvePnpmIgnoredBuilds, +} from "./pnpm_denied_builds"; export const logger = log.scope("app_upgrade_utils"); @@ -72,25 +72,14 @@ export async function selfHealDeniedPnpmBuildsFromError({ return false; } - const ignoredBuildsFromModulesYaml = await readPnpmIgnoredBuilds(appPath); const errorOutput = error instanceof Error ? error.message : String(error); - const ignoredBuilds = - ignoredBuildsFromModulesYaml.length > 0 - ? ignoredBuildsFromModulesYaml - : parsePnpmIgnoredBuildsFromOutput(errorOutput); - const { deniedBuilds } = await recordDeniedPnpmBuilds({ + const ignoredBuilds = await resolvePnpmIgnoredBuilds(appPath, errorOutput); + const { deniedBuilds } = await recordAndReportDeniedPnpmBuilds({ appPath, ignoredBuilds, - }); - if (deniedBuilds.length === 0) { - return false; - } - - sendTelemetryEvent("pnpm:build-auto-denied", { - packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), source, }); - return true; + return deniedBuilds.length > 0; } export async function simpleSpawnWithDeniedPnpmBuildSelfHeal({ diff --git a/src/ipc/utils/pnpm_denied_builds.ts b/src/ipc/utils/pnpm_denied_builds.ts new file mode 100644 index 0000000000..947ba9cd1e --- /dev/null +++ b/src/ipc/utils/pnpm_denied_builds.ts @@ -0,0 +1,56 @@ +import { + parsePnpmIgnoredBuildsFromOutput, + readPnpmIgnoredBuilds, + recordDeniedPnpmBuilds, + type PnpmIgnoredBuild, +} from "@/ipc/utils/socket_firewall"; +import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; + +export type PnpmDeniedBuildsTelemetrySource = + | "add-dependency" + | "app-run" + | "self-heal" + | "cloud-sandbox"; + +/** + * Resolves the ignored-builds list the way every local flow should: + * `.modules.yaml` is authoritative when present; the install/error output is + * the fallback (e.g. narrow-PTY wrapping makes the output less reliable). + */ +export async function resolvePnpmIgnoredBuilds( + appPath: string, + fallbackOutput?: string, +): Promise { + const ignoredBuildsFromModulesYaml = await readPnpmIgnoredBuilds(appPath); + if (ignoredBuildsFromModulesYaml.length > 0) { + return ignoredBuildsFromModulesYaml; + } + return fallbackOutput ? parsePnpmIgnoredBuildsFromOutput(fallbackOutput) : []; +} + +/** + * Records tagged denials for the given ignored builds and emits the + * `pnpm:build-auto-denied` telemetry event when anything new was denied. + * Single owner of the record + telemetry contract for all call sites. + */ +export async function recordAndReportDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + source, +}: { + appPath: string; + ignoredBuilds: PnpmIgnoredBuild[]; + source: PnpmDeniedBuildsTelemetrySource; +}): Promise<{ deniedBuilds: PnpmIgnoredBuild[] }> { + const { deniedBuilds } = await recordDeniedPnpmBuilds({ + appPath, + ignoredBuilds, + }); + if (deniedBuilds.length > 0) { + sendTelemetryEvent("pnpm:build-auto-denied", { + packages: deniedBuilds.map((ignoredBuild) => ignoredBuild.packageSpec), + source, + }); + } + return { deniedBuilds }; +} diff --git a/src/ipc/utils/socket_firewall.test.ts b/src/ipc/utils/socket_firewall.test.ts index 0fd42cea09..ab2e2ae524 100644 --- a/src/ipc/utils/socket_firewall.test.ts +++ b/src/ipc/utils/socket_firewall.test.ts @@ -747,6 +747,148 @@ describe("updatePnpmAllowBuildsConfigContent", () => { } }); + it("converts pnpm placeholder entries into tagged denials", async () => { + const tempDir = await mkdtemp( + path.join(os.tmpdir(), "dyad-pnpm-placeholder-"), + ); + const configPath = path.join(tempDir, "pnpm-workspace.yaml"); + try { + // pnpm 11 appends this placeholder after a non-strict install with + // ignored builds; it does not satisfy strict mode and must not be + // treated as a user decision. + await writeFile( + configPath, + [ + "allowBuilds:", + " core-js: set this to true or false", + " # dyad-default-allow-builds begin", + " # dyad-default-allow-builds-schema=v1", + " # dyad-default-allow-builds-data-version=2026-05-21.1", + " # dyad-default-allow-builds-channel=local", + " sharp: true", + " # dyad-default-allow-builds end", + "packages:", + " - .", + "minimumReleaseAge: 1440", + "", + ].join("\n"), + ); + + await expect( + recordDeniedPnpmBuilds({ + appPath: tempDir, + allowBuildsText, + ignoredBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ], + }), + ).resolves.toEqual({ + deniedBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ], + }); + + const nextConfig = await readFile(configPath, "utf8"); + expect(nextConfig).toContain("core-js: false # dyad-auto-denied"); + expect(nextConfig).not.toContain("set this to true or false"); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); + + it("resolves placeholder entries even without an ignored-builds list", async () => { + const tempDir = await mkdtemp( + path.join(os.tmpdir(), "dyad-pnpm-placeholder-ensure-"), + ); + const configPath = path.join(tempDir, "pnpm-workspace.yaml"); + try { + await writeFile( + configPath, + [ + "allowBuilds:", + " core-js: set this to true or false", + " sharp: set this to true or false", + " # dyad-default-allow-builds begin", + " # dyad-default-allow-builds-schema=v1", + " # dyad-default-allow-builds-data-version=2026-05-21.1", + " # dyad-default-allow-builds-channel=local", + ' "@swc/core": true', + " # dyad-default-allow-builds end", + "packages:", + " - .", + "minimumReleaseAge: 1440", + "", + ].join("\n"), + ); + + // App-start ensure pass: sharp is in the allow-list so its placeholder + // resolves to the managed `true`; core-js becomes a tagged denial. + await expect( + ensurePnpmAllowBuildsConfigured({ + appPath: tempDir, + allowBuildsText, + }), + ).resolves.toEqual({ changed: true, promotedPackages: [] }); + + const nextConfig = await readFile(configPath, "utf8"); + expect(nextConfig).toContain("core-js: false # dyad-auto-denied"); + expect(nextConfig).toContain("sharp: true"); + expect(nextConfig).not.toContain("set this to true or false"); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); + + it("creates the allowBuilds key when recording offline denials into a config without one", async () => { + const tempDir = await mkdtemp( + path.join(os.tmpdir(), "dyad-pnpm-deny-no-key-"), + ); + const configPath = path.join(tempDir, "pnpm-workspace.yaml"); + try { + // Contrived: remote-channel metadata forces the offline denial-only + // path, but the allowBuilds key itself was stripped by an external + // tool. Denials must still be recorded, not silently dropped. + await writeFile( + configPath, + [ + "# dyad-default-allow-builds begin", + "# dyad-default-allow-builds-schema=v1", + "# dyad-default-allow-builds-data-version=2026-05-21.1", + "# dyad-default-allow-builds-channel=remote", + "# dyad-default-allow-builds end", + "packages:", + " - .", + "", + ].join("\n"), + ); + + const failingFetcher = vi.fn(async () => ({ + ok: false, + text: async () => "", + })); + + await expect( + recordDeniedPnpmBuilds({ + appPath: tempDir, + remoteAllowBuildsTextFetcher: failingFetcher, + ignoredBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ], + }), + ).resolves.toEqual({ + deniedBuilds: [ + { packageName: "core-js", packageSpec: "core-js@3.49.0" }, + ], + }); + + const nextConfig = await readFile(configPath, "utf8"); + expect(nextConfig).toContain("allowBuilds:"); + expect(nextConfig).toContain("core-js: false # dyad-auto-denied"); + } finally { + await rm(tempDir, { recursive: true, force: true }); + } + }); + it("records denials against existing config when the remote allow-list is unavailable", async () => { const tempDir = await mkdtemp( path.join(os.tmpdir(), "dyad-pnpm-deny-offline-"), diff --git a/src/ipc/utils/socket_firewall.ts b/src/ipc/utils/socket_firewall.ts index 31b611586f..3b8b214e6f 100644 --- a/src/ipc/utils/socket_firewall.ts +++ b/src/ipc/utils/socket_firewall.ts @@ -396,13 +396,56 @@ function parseYamlMapKey(rawKey: string): string { } } -function parseAllowBuildsLine(line: string): { key: string } | null { - const match = line.match(/^\s{2}((?:"(?:[^"\\]|\\.)+"|'[^']+'|[^:#]+)):\s*/); +function parseAllowBuildsLine( + line: string, +): { key: string; value: string } | null { + const match = line.match( + /^\s{2}((?:"(?:[^"\\]|\\.)+"|'[^']+'|[^:#]+)):\s*(.*?)\s*(?:#.*)?$/, + ); if (!match) { return null; } - return { key: parseYamlMapKey(match[1].trim()) }; + return { key: parseYamlMapKey(match[1].trim()), value: match[2].trim() }; +} + +// pnpm 11 appends `pkg: set this to true or false` placeholder entries to +// allowBuilds after a non-strict install that ignored builds. A placeholder +// neither satisfies strict mode (installs still fail with +// ERR_PNPM_IGNORED_BUILDS) nor represents a human decision, so Dyad treats +// these as its own to resolve: remove them and let the caller convert them +// into tagged denials (or a managed `true` when the allow-list covers them). +const PNPM_PLACEHOLDER_ALLOW_BUILDS_VALUE_PATTERN = + /^["']?set this to true or false["']?$/i; + +function removePlaceholderAllowBuildsEntries(lines: string[]): string[] { + const range = getTopLevelAllowBuildsRange(lines); + if (!range) { + return []; + } + + const managedBlock = findAllowBuildsManagedBlock(lines); + const placeholderPackages: string[] = []; + for (let index = range.end - 1; index > range.start; index -= 1) { + if ( + managedBlock && + index >= managedBlock.beginIndex && + index <= managedBlock.endIndex + ) { + continue; + } + + const parsedLine = parseAllowBuildsLine(lines[index]); + if ( + parsedLine && + PNPM_PLACEHOLDER_ALLOW_BUILDS_VALUE_PATTERN.test(parsedLine.value) + ) { + lines.splice(index, 1); + placeholderPackages.push(parsedLine.key); + } + } + + return placeholderPackages; } function removeAutoDeniedPromotedBuilds( @@ -519,6 +562,13 @@ function updatePnpmAllowBuildsConfigContentWithSource( } const promotedPackages = removeAutoDeniedPromotedBuilds(lines, source); + // Placeholder entries become tagged denials below (via the deny list), or + // simply drop away when the allow-list now covers the package (the managed + // block rewrite emits `pkg: true` for those). + const packagesToDeny = [ + ...autoDeniedPackageNames, + ...removePlaceholderAllowBuildsEntries(lines), + ]; const managedBlock = findAllowBuildsManagedBlock(lines); if (managedBlock) { const { beginIndex, endIndex } = managedBlock; @@ -543,7 +593,7 @@ function updatePnpmAllowBuildsConfigContentWithSource( const deniedPackages = insertAutoDeniedBuilds( lines, new Set(source.packages), - autoDeniedPackageNames, + packagesToDeny, ); return { content: formatPnpmWorkspaceConfigContent(lines), @@ -569,7 +619,7 @@ function updatePnpmAllowBuildsConfigContentWithSource( const deniedPackages = insertAutoDeniedBuilds( lines, new Set(source.packages), - autoDeniedPackageNames, + packagesToDeny, ); return { content: formatPnpmWorkspaceConfigContent(lines), @@ -587,7 +637,7 @@ function updatePnpmAllowBuildsConfigContentWithSource( const deniedPackages = insertAutoDeniedBuilds( nextLines, new Set(source.packages), - autoDeniedPackageNames, + packagesToDeny, ); return { content: formatPnpmWorkspaceConfigContent(nextLines), @@ -941,10 +991,21 @@ function insertAutoDeniedBuildsIntoContent( lines.pop(); } + const packagesToDeny = [ + ...packageNames, + ...removePlaceholderAllowBuildsEntries(lines), + ]; + if (packagesToDeny.length > 0 && !getTopLevelAllowBuildsRange(lines)) { + if (lines.length > 0 && lines.at(-1) !== "") { + lines.push(""); + } + lines.push("allowBuilds:"); + } + const deniedPackages = insertAutoDeniedBuilds( lines, new Set(), - packageNames, + packagesToDeny, ); if (deniedPackages.length === 0) { return {