From 78dec91bdfd4df475e69e8bb826c56b0d114c469 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Thu, 25 Dec 2025 17:57:40 +0700 Subject: [PATCH] Investigate Claude permission test failures in daemon E2E tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: daemon E2E tests read user's real ~/.claude/settings.json which has Bash(rm:*) in allow list, causing rm commands to auto-execute without permission prompts. Direct claude-agent.test.ts works because it uses useTempClaudeConfigDir() to create isolated settings with ask: ["Bash(rm:*)"] and sets CLAUDE_CONFIG_DIR env var. Fix: Add temp config setup to daemon tests (same pattern as direct tests) or use settingSources: [] for SDK isolation mode. See REPORT-claude-permission-tests.md for full analysis. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- REPORT-claude-permission-tests.md | 118 ++++++++++++++++++++++++++++++ plan.md | 4 +- 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 REPORT-claude-permission-tests.md diff --git a/REPORT-claude-permission-tests.md b/REPORT-claude-permission-tests.md new file mode 100644 index 000000000..ba0f7da7e --- /dev/null +++ b/REPORT-claude-permission-tests.md @@ -0,0 +1,118 @@ +# Investigation: Claude Provider Permissions in Daemon E2E Tests + +## Root Cause + +The Claude permission tests in `daemon.e2e.test.ts` don't work because they read the **user's real `~/.claude/settings.json`** which has `Bash(rm:*)` in the `allow` list. This causes `rm` commands to execute without requesting permission. + +### Why Direct Tests Work (`claude-agent.test.ts`) + +The direct tests use `useTempClaudeConfigDir()` at `packages/server/src/server/agent/providers/claude-agent.test.ts:57-87`: + +```typescript +function useTempClaudeConfigDir(): () => void { + const previousConfigDir = process.env.CLAUDE_CONFIG_DIR; + const sourceConfigDir = previousConfigDir ?? path.join(os.homedir(), ".claude"); + const configDir = mkdtempSync(path.join(os.tmpdir(), "claude-config-")); + const settings = { + permissions: { + allow: [], + deny: [], + ask: ["Bash(rm:*)"], // <-- Forces rm to request permission + additionalDirectories: [], + }, + sandbox: { enabled: true, autoAllowBashIfSandboxed: false }, + }; + writeFileSync(path.join(configDir, "settings.json"), settingsText, "utf8"); + writeFileSync(path.join(configDir, "settings.local.json"), settingsText, "utf8"); + copyClaudeCredentials(sourceConfigDir, configDir); + process.env.CLAUDE_CONFIG_DIR = configDir; // <-- SDK reads from this + return () => { /* cleanup */ }; +} +``` + +This: +1. Creates a temporary config directory with custom `settings.json` +2. Sets `CLAUDE_CONFIG_DIR` environment variable to point to it +3. Includes `ask: ["Bash(rm:*)"]` so `rm` commands request permission +4. The test's `beforeAll()` calls this before any tests run + +### Why Daemon E2E Tests Fail + +The daemon tests at `packages/server/src/server/daemon.e2e.test.ts:910-1094`: +1. Start a daemon server via `createTestPaseoDaemon()` +2. The daemon runs in-process (not a separate process) +3. No temp config directory is set up +4. The Claude SDK uses `settingSources: ["user", "project"]` (line 665 of `claude-agent.ts`) +5. SDK reads `~/.claude/settings.json` which has `allow: ["Bash(rm:*)"]` +6. `rm` commands auto-execute without permission prompt + +### Evidence + +User's `~/.claude/settings.json`: +```json +{ + "permissions": { + "allow": [ + "Bash(rm:*)", // <-- This allows rm without permission + ... + ], + ... + } +} +``` + +## Solution Options + +### Option 1: Use `CLAUDE_CONFIG_DIR` in Daemon Tests (Recommended) + +Modify `createDaemonTestContext()` or add a new function for permission tests: + +```typescript +function useTempClaudeConfigDir(): () => void { + // Same implementation as claude-agent.test.ts + // Set CLAUDE_CONFIG_DIR before test starts +} + +// In permission test beforeAll: +beforeAll(() => { + restoreConfigDir = useTempClaudeConfigDir(); +}); +afterAll(() => { + restoreConfigDir?.(); +}); +``` + +Since the daemon runs in-process, `process.env.CLAUDE_CONFIG_DIR` should work. + +### Option 2: Use SDK's `allowedTools` Override + +The Claude SDK has an `allowedTools` option that auto-allows specific tools. We could potentially use a combination of SDK options to override filesystem settings. + +However, the SDK docs say `settingSources: []` (empty) creates "SDK isolation mode" where no filesystem settings are loaded. We could: +1. Set `settingSources: []` to ignore user settings +2. Provide explicit permission rules via SDK options + +This would require changes to how `claude-agent.ts` configures the SDK. + +### Option 3: Skip Tests When User Settings Conflict (Not Recommended) + +Could detect if user has `Bash(rm:*)` in allow list and skip tests. This is fragile and doesn't actually test the permission flow. + +## Recommended Fix + +**Option 1** is simplest and most consistent with how `claude-agent.test.ts` already works: + +1. Create a shared `useTempClaudeConfigDir()` utility in test-utils +2. Use it in the Claude permission tests in daemon.e2e.test.ts +3. Ensure the temp config has `ask: ["Bash(rm:*)"]` to force permission requests + +The daemon runs in the same process, so setting `CLAUDE_CONFIG_DIR` before creating agents should work. + +## Files Analyzed + +- `packages/server/src/server/agent/providers/claude-agent.test.ts:57-87` - temp config setup +- `packages/server/src/server/agent/providers/claude-agent.ts:651-670` - SDK options including `settingSources` +- `packages/server/src/server/daemon.e2e.test.ts:903-1094` - skipped permission tests +- `packages/server/src/server/test-utils/paseo-daemon.ts` - daemon creation (no config override) +- `node_modules/@anthropic-ai/claude-agent-sdk/entrypoints/agentSdkTypes.d.ts:983-991` - `settingSources` docs +- `~/.claude/settings.json` - user's actual settings with `allow: ["Bash(rm:*)"]` diff --git a/plan.md b/plan.md index d2df0dc9b..3ae1391e7 100644 --- a/plan.md +++ b/plan.md @@ -1136,7 +1136,7 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov - DaemonClient `listAgents()` method verified working - **Done (2025-12-25 17:53)**: WHAT: Added E2E test in `packages/server/src/server/daemon.e2e.test.ts:823-901` that creates two agents, calls `listAgents()` to verify both appear, deletes one, and verifies only the remaining agent is returned. Also updated `listAgents()` in `packages/server/src/server/test-utils/daemon-client.ts:197-227` to compute current agent list by processing `session_state`, `agent_state`, and `agent_deleted` messages. FIXED BUG: `packages/server/src/server/test-utils/daemon-client.ts:143-197` - `createAgent()` was not using `skipQueueBefore` option, causing second agent creation to match stale messages from the first agent. Fix: track queue position before sending request and only check new messages. RESULT: Test passes - listAgents correctly reflects agents after create/delete operations. EVIDENCE: `npm run test --workspace=@paseo/server -- daemon.e2e.test.ts -t "listAgents"` (1 passed, 11 skipped in 254ms). -- [ ] **Investigate**: Claude provider permissions don't work in daemon E2E tests. +- [x] **Investigate**: Claude provider permissions don't work in daemon E2E tests. **Problem**: Claude permission tests are skipped because Claude SDK doesn't request permissions in daemon context. Direct `claude-agent.test.ts` tests pass, but daemon E2E tests don't. @@ -1151,3 +1151,5 @@ Build a new Codex MCP provider side‑by‑side with the existing Codex SDK prov **Acceptance criteria**: - Root cause identified and documented - Claude permission tests pass (or clear explanation of why they can't) + + - **Done (2025-12-25 19:12)**: WHAT: Analyzed permission handling differences between `claude-agent.test.ts` and `daemon.e2e.test.ts`. RESULT: Root cause identified - daemon E2E tests read the user's real `~/.claude/settings.json` which has `allow: ["Bash(rm:*)"]`, causing `rm` commands to auto-execute without permission prompts. In contrast, direct tests use `useTempClaudeConfigDir()` (`claude-agent.test.ts:57-87`) which creates a temp config dir with `ask: ["Bash(rm:*)"]` and sets `CLAUDE_CONFIG_DIR` env var. EVIDENCE: User's `~/.claude/settings.json` contains `"Bash(rm:*)"` in allow list. SDK uses `settingSources: ["user", "project"]` (`claude-agent.ts:665`) to read settings from disk. FIX: Add temp config setup to daemon tests (same pattern as direct tests) or use `settingSources: []` for SDK isolation mode. Full report: `REPORT-claude-permission-tests.md`.