Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions e2e-tests/fixtures/add-safe-dependency.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
I found a safe package to add for this app.

<dyad-add-dependency packages="lodash"></dyad-add-dependency>

Please review and approve the dependency addition.
5 changes: 5 additions & 0 deletions e2e-tests/fixtures/add-unsafe-dependency.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
I found a package to add for this app.

<dyad-add-dependency packages="axois"></dyad-add-dependency>

Please review and approve the dependency addition.
136 changes: 136 additions & 0 deletions e2e-tests/socket_firewall.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import { expect } from "@playwright/test";
import fs from "node:fs/promises";
import path from "node:path";
import {
testWithConfigSkipIfWindows,
Timeout,
type PageObject,
} from "./helpers/test_helper";

const originalNpmCache = process.env.npm_config_cache;
const originalNpmStoreDir = process.env.npm_config_store_dir;
const originalPnpmStoreDir = process.env.pnpm_config_store_dir;

const testSkipIfWindows = testWithConfigSkipIfWindows({
preLaunchHook: async ({ userDataDir }) => {
const npmCacheDir = path.join(userDataDir, "npm-cache");
const pnpmStoreDir = path.join(userDataDir, "pnpm-store");

await fs.mkdir(npmCacheDir, { recursive: true });
await fs.mkdir(pnpmStoreDir, { recursive: true });

process.env.npm_config_cache = npmCacheDir;
process.env.npm_config_store_dir = pnpmStoreDir;
process.env.pnpm_config_store_dir = pnpmStoreDir;
},
postLaunchHook: async () => {
if (originalNpmCache === undefined) {
delete process.env.npm_config_cache;
} else {
process.env.npm_config_cache = originalNpmCache;
}

if (originalNpmStoreDir === undefined) {
delete process.env.npm_config_store_dir;
} else {
process.env.npm_config_store_dir = originalNpmStoreDir;
}

if (originalPnpmStoreDir === undefined) {
delete process.env.pnpm_config_store_dir;
} else {
process.env.pnpm_config_store_dir = originalPnpmStoreDir;
}
},
});

async function openMinimalBuildChat(po: PageObject) {
await po.setUp();

await po.navigation.goToSettingsTab();
await expect(
po.page.getByRole("switch", { name: "Block unsafe npm packages" }),
).toBeChecked();

await po.navigation.goToAppsTab();
await po.importApp("minimal");
await po.chatActions.waitForChatCompletion({ timeout: Timeout.LONG });
await po.chatActions.clickNewChat();
await po.chatActions.selectChatMode("build");

const appPath = await po.appManagement.getCurrentAppPath();
return {
packageJsonPath: path.join(appPath, "package.json"),
pnpmLockPath: path.join(appPath, "pnpm-lock.yaml"),
};
}

testSkipIfWindows(
"build mode - safe npm package installs through the real socket firewall path",
async ({ po }) => {
const { packageJsonPath, pnpmLockPath } = await openMinimalBuildChat(po);
const initialPackageJson = await fs.readFile(packageJsonPath, "utf8");
const initialPnpmLock = await fs.readFile(pnpmLockPath, "utf8");

await po.sendPrompt("tc=add-safe-dependency");
await expect(po.page.getByTestId("approve-proposal-button")).toBeVisible({
timeout: Timeout.LONG,
});

await po.approveProposal();
await expect(async () => {
const packageJson = JSON.parse(
await fs.readFile(packageJsonPath, "utf8"),
);
expect(packageJson.dependencies?.lodash).toEqual(expect.any(String));
expect(await fs.readFile(pnpmLockPath, "utf8")).not.toBe(initialPnpmLock);
}).toPass({
timeout: Timeout.EXTRA_LONG,
});

await expect(
po.page.getByText(/Failed to add dependencies:/),
).not.toBeVisible();

expect(await fs.readFile(packageJsonPath, "utf8")).not.toBe(
initialPackageJson,
);
},
);

testSkipIfWindows(
"build mode - blocked unsafe npm package shows the real socket verdict and preserves app files",
async ({ po }) => {
const { packageJsonPath, pnpmLockPath } = await openMinimalBuildChat(po);
const initialPackageJson = await fs.readFile(packageJsonPath, "utf8");
const initialPnpmLock = await fs.readFile(pnpmLockPath, "utf8");

await po.sendPrompt("tc=add-unsafe-dependency");
await expect(po.page.getByTestId("approve-proposal-button")).toBeVisible({
timeout: Timeout.LONG,
});

await po.approveProposal();

const errorCard = po.page.getByRole("button", {
name: /Failed to add dependencies: axois\./i,
});
await expect(errorCard).toBeVisible({
timeout: Timeout.EXTRA_LONG,
});

await errorCard.click();
await expect(errorCard).toContainText(/blocked npm package/i, {
timeout: Timeout.MEDIUM,
});
await expect(errorCard).toContainText(/axois/i, {
timeout: Timeout.MEDIUM,
});
await expect(errorCard).toContainText(/malware/i, {
timeout: Timeout.MEDIUM,
});

expect(await fs.readFile(packageJsonPath, "utf8")).toBe(initialPackageJson);
expect(await fs.readFile(pnpmLockPath, "utf8")).toBe(initialPnpmLock);
},
);
4 changes: 4 additions & 0 deletions rules/adding-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,7 @@ When adding a new toggle/setting to the Settings page:
3. Add a `SETTING_IDS` entry and search index entry in `src/lib/settingsSearchIndex.ts`
4. Create a switch component (e.g., `src/components/MySwitch.tsx`) - follow `AutoApproveSwitch.tsx` as a template
5. Import and add the switch to the relevant section in `src/pages/settings.tsx`

For settings whose default can be overridden remotely:

- Prefer leaving the raw stored field unset until the user explicitly changes it, then compute the effective value as `stored value ?? remote default ?? built-in fallback`. Do not persist remote-applied defaults into `user-settings.json`.
6 changes: 6 additions & 0 deletions rules/e2e-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,17 @@ If this happens:

## Common flaky test patterns and fixes

- **After `po.importApp(...)`**: Some imports trigger an initial assistant turn (for example `minimal` generating `AI_RULES.md`) that can leave a visible `Retry` button in the chat. If the test is about a later prompt, first wait for that import-time turn to finish, then start a new chat before calling `sendPrompt()`, or helper methods that wait on `Retry` visibility may return too early.
- **After `page.reload()`**: Always add `await page.waitForLoadState("domcontentloaded")` before interacting with elements. Without this, the page may not have re-rendered yet.
- **Keyboard navigation events (ArrowUp/ArrowDown)**: Add `await page.waitForTimeout(100)` between sequential keyboard presses to let the UI state settle. Rapid keypresses can cause race conditions in menu navigation.
- **Navigation to tabs**: Use `await expect(link).toBeVisible({ timeout: Timeout.EXTRA_LONG })` before clicking tab links (especially in `goToAppsTab()`). Electron sidebar links can take time to render during app initialization.
- **Confirming flakiness**: Use `PLAYWRIGHT_RETRIES=0 PLAYWRIGHT_HTML_OPEN=never npm run e2e -- e2e-tests/<spec> --repeat-each=10` to reproduce flaky tests. `PLAYWRIGHT_RETRIES=0` is critical — CI defaults to 2 retries, hiding flakiness.

## Real Socket Firewall E2E tests

- When exercising the real `sfw` binary in E2E, set fresh per-test `npm_config_cache`, `npm_config_store_dir`, and `pnpm_config_store_dir` in the launch hooks. Reused caches/stores can make Socket Firewall report that it did not detect package fetches, which turns blocked-package tests into false negatives.
- For real-path blocked-package coverage, prefer `axois` over `lodahs`. `lodahs` can resolve to `0.0.1-security` and install successfully under `pnpm`, so it does not reliably reach the blocked-package UI.

## Waiting for button state transitions

When clicking a button that triggers an async operation and changes its text/state (e.g., "Run Security Review" → "Running Security Review..."), wait for the loading state to appear and disappear rather than just waiting for the original button to be hidden:
Expand Down
4 changes: 4 additions & 0 deletions rules/electron-ipc.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ When modifying `ChatResponseChunkSchema` or adding new `safeSend("chat:response:

**Zod schema contract changes:** Making a field optional (e.g., `messages` → `messages.optional()`) causes TypeScript errors in all consumers that assume the field is always present. Search for all destructuring/usage sites and add guards before committing.

## End-of-turn warnings

When a main-process workflow needs to show a user-facing warning toast after a turn completes, thread it through every completion path, not just `chat:response:end`. Build-mode auto-approve and local-agent flows use `ChatResponseEndSchema`, while manual proposal approval uses `ApproveProposalResultSchema`; surface the warning in both `useStreamChat` and `ChatInput` so the behavior stays consistent.

## React + IPC integration pattern

When creating hooks/components that call IPC handlers:
Expand Down
50 changes: 50 additions & 0 deletions src/__tests__/local_agent_handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ vi.mock("@/ipc/handlers/compaction/compaction_handler", () => ({

import { handleLocalAgentStream } from "@/pro/main/ipc/handlers/local_agent/local_agent_handler";
import { DyadError, DyadErrorKind } from "@/errors/dyad_error";
import { buildAgentToolSet } from "@/pro/main/ipc/handlers/local_agent/tool_definitions";

// ============================================================================
// Tests
Expand Down Expand Up @@ -421,6 +422,54 @@ describe("handleLocalAgentStream", () => {
});
});

describe("Warning propagation", () => {
it("includes warning messages in the error payload when a tool fails after warning", async () => {
const { event, getMessagesByChannel } = createFakeEvent();
mockSettings = buildTestSettings({ enableDyadPro: true });
mockChatData = buildTestChat();

const warningMessage = "Firewall checks were skipped for this install.";
vi.mocked(buildAgentToolSet).mockImplementationOnce((ctx) => {
return {
warn_then_fail: {
execute: async () => {
ctx.onWarningMessage?.(warningMessage);
throw new Error("Simulated tool failure");
},
},
} as any;
});

mockStreamTextImpl = (options) => ({
fullStream: (async function* () {
yield* [];
await options.tools.warn_then_fail.execute();
})(),
response: Promise.resolve({ messages: [] }),
steps: Promise.resolve([]),
});

await handleLocalAgentStream(
event,
{ chatId: 1, prompt: "test" },
new AbortController(),
{
placeholderMessageId: 10,
systemPrompt: "You are helpful",
dyadRequestId,
},
);

const errorMessages = getMessagesByChannel("chat:response:error");
expect(errorMessages).toHaveLength(1);
expect(errorMessages[0].args[0]).toMatchObject({
chatId: 1,
error: expect.stringContaining("Simulated tool failure"),
warningMessages: [warningMessage],
});
});
});

describe("Context compaction setting", () => {
it("should not run pending compaction when context compaction is disabled", async () => {
// Arrange
Expand Down Expand Up @@ -1043,6 +1092,7 @@ describe("handleLocalAgentStream", () => {
if (attemptCount === 1) {
return {
fullStream: (async function* () {
yield* [];
throw {
type: "error",
sequence_number: 0,
Expand Down
45 changes: 45 additions & 0 deletions src/__tests__/readSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import path from "node:path";
import { safeStorage } from "electron";
import {
readSettings,
resolveEffectiveSettings,
readEffectiveSettingsAsync,
getSettingsFilePath,
encrypt,
decrypt,
} from "@/main/settings";
import { getUserDataPath } from "@/paths/paths";
import { UserSettings } from "@/lib/schemas";
import { DyadError, DyadErrorKind } from "@/errors/dyad_error";
import { getRemoteDesktopConfig } from "@/ipc/shared/remote_desktop_config";

// Mock dependencies
vi.mock("node:fs");
Expand All @@ -24,11 +27,16 @@ vi.mock("electron", () => ({
vi.mock("@/paths/paths", () => ({
getUserDataPath: vi.fn(),
}));
vi.mock("@/ipc/shared/remote_desktop_config", () => ({
getRemoteDesktopConfig: vi.fn(),
getCachedRemoteDesktopConfig: vi.fn(() => null),
}));

const mockFs = vi.mocked(fs);
const mockPath = vi.mocked(path);
const mockSafeStorage = vi.mocked(safeStorage);
const mockGetUserDataPath = vi.mocked(getUserDataPath);
const mockGetRemoteDesktopConfig = vi.mocked(getRemoteDesktopConfig);

describe("readSettings", () => {
const mockUserDataPath = "/mock/user/data";
Expand Down Expand Up @@ -113,6 +121,7 @@ describe("readSettings", () => {
expect(result.telemetryConsent).toBe("opted_in");
expect(result.hasRunBefore).toBe(true);
// Should still have defaults for missing properties
expect(result.blockUnsafeNpmPackages).toBeUndefined();
expect(result.enableAutoUpdate).toBe(true);
expect(result.releaseChannel).toBe("stable");
});
Expand Down Expand Up @@ -540,6 +549,42 @@ describe("readSettings", () => {
});
});

describe("effective settings", () => {
it("applies the remote default when the user has not explicitly set the setting", async () => {
mockGetRemoteDesktopConfig.mockResolvedValue({
defaults: { blockUnsafeNpmPackages: false },
});
mockFs.existsSync.mockReturnValue(true);
mockFs.readFileSync.mockReturnValue(JSON.stringify({}));

const result = await readEffectiveSettingsAsync();

expect(result.blockUnsafeNpmPackages).toBe(false);
expect(mockFs.writeFileSync).not.toHaveBeenCalled();
});

it("does not override an explicitly stored local value", () => {
mockFs.existsSync.mockReturnValue(true);
mockFs.readFileSync.mockReturnValue(JSON.stringify({}));

const result = resolveEffectiveSettings({
...readSettings(),
blockUnsafeNpmPackages: true,
});

expect(result.blockUnsafeNpmPackages).toBe(true);
});

it("falls back to the built-in default when remote config is missing", () => {
mockFs.existsSync.mockReturnValue(true);
mockFs.readFileSync.mockReturnValue(JSON.stringify({}));

const result = resolveEffectiveSettings(readSettings(), null);

expect(result.blockUnsafeNpmPackages).toBe(true);
});
});

describe("getSettingsFilePath", () => {
it("should return correct settings file path", () => {
const result = getSettingsFilePath();
Expand Down
30 changes: 30 additions & 0 deletions src/components/BlockUnsafeNpmPackagesSwitch.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { Label } from "@/components/ui/label";
import { Switch } from "@/components/ui/switch";
import { useSettings } from "@/hooks/useSettings";

export function BlockUnsafeNpmPackagesSwitch() {
const { settings, updateSettings } = useSettings();

return (
<div className="space-y-1">
<div className="flex items-center space-x-2">
<Switch
id="block-unsafe-npm-packages"
aria-label="Block unsafe npm packages"
checked={settings?.blockUnsafeNpmPackages ?? true}
onCheckedChange={(checked) => {
updateSettings({
blockUnsafeNpmPackages: checked,
});
}}
/>
<Label htmlFor="block-unsafe-npm-packages">
Block unsafe npm packages
</Label>
</div>
<div className="text-sm text-gray-500 dark:text-gray-400">
Uses socket.dev to detect unsafe packages and blocks them
</div>
</div>
);
}
8 changes: 7 additions & 1 deletion src/components/chat/ChatInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import { useAttachments } from "@/hooks/useAttachments";
import { AttachmentsList } from "./AttachmentsList";
import { DragDropOverlay } from "./DragDropOverlay";
import { FileAttachmentTypeDialog } from "./FileAttachmentTypeDialog";
import { showExtraFilesToast, showInfo } from "@/lib/toast";
import { showExtraFilesToast, showInfo, showWarning } from "@/lib/toast";
import { useSummarizeInNewChat } from "./SummarizeInNewChatButton";
import { ChatInputControls } from "../ChatInputControls";
import { ChatErrorBox } from "./ChatErrorBox";
Expand Down Expand Up @@ -621,6 +621,12 @@ export function ChatInput({ chatId }: { chatId?: number }) {
posthog,
});
}
for (const warningMessage of result.warningMessages ?? []) {
showWarning(warningMessage);
}
if (!result.success) {
setError(result.error ?? "An error occurred while approving");
}
} catch (err) {
console.error("Error approving proposal:", err);
setError((err as Error)?.message || "An error occurred while approving");
Expand Down
Loading
Loading