Repository navigation
fix: ensure teammateMode=bypassPermissions on native team setup - #998
Conversation
CC's native team permission gate uses the global `teammateMode` setting in ~/.claude/settings.json as the authority for teammate permissions. When set to "auto", tool approvals route to the team leader — which is an AI agent that can't approve, causing a deadlock where subagents hang forever waiting for Edit/Write permission. Genie already passes `--permission-mode bypassPermissions` per-session (provider-adapters.ts:251), but the global setting overrides it for the native team layer. This adds `ensureTeammateBypassPermissions()` to claude-settings.ts and calls it from `ensureNativeTeam()` so the global setting is always correct when teams are active. Also ensures `skipDangerousModePermissionPrompt: true` so agents spawned with `--dangerously-skip-permissions` don't hit interactive prompts.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c21301a6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dir = join(homedir(), '.claude'); | ||
| if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); |
There was a problem hiding this comment.
Respect CLAUDE_CONFIG_DIR when writing teammate settings
ensureTeammateBypassPermissions() now hardcodes ~/.claude, so when CLAUDE_CONFIG_DIR is set (which this codebase supports for team paths and tests), it updates the wrong settings file and can still leave the active Claude config in teammateMode: auto. In those environments, the deadlock fix does not apply and the function may also unexpectedly mutate a real home-directory config.
Useful? React with 👍 / 👎.
| if (settings.teammateMode !== 'bypassPermissions') { | ||
| settings.teammateMode = 'bypassPermissions'; |
There was a problem hiding this comment.
Guard parsed settings shape before property writes
The new parser path assumes settings.json is an object, but valid JSON like null, a string, or a number will pass JSON.parse and then throw at settings.teammateMode/assignment in strict mode. That turns a recoverable malformed-settings case into a hard failure during team setup instead of applying the intended safe defaults.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to automatically configure Claude settings to bypass permissions for native teams, preventing deadlocks during tool approvals. The feedback highlights several areas for improvement: using existing constants for directory paths to maintain consistency and respect environment variables, addressing potential race conditions during the read-modify-write cycle of the settings file, and converting synchronous file operations to asynchronous ones to avoid blocking the event loop in an otherwise async module.
| const dir = join(homedir(), '.claude'); | ||
| if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); |
There was a problem hiding this comment.
The directory path is already defined as a module-level constant CLAUDE_DIR at line 12. Re-calculating it here using homedir() is redundant. Additionally, note that claude-native-teams.ts respects the CLAUDE_CONFIG_DIR environment variable, while this file hardcodes the path. Using the existing constant helps maintain consistency within the file.
| const dir = join(homedir(), '.claude'); | |
| if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); | |
| if (!existsSync(CLAUDE_DIR)) mkdirSync(CLAUDE_DIR, { recursive: true }); |
| settings = JSON.parse(readFileSync(CLAUDE_SETTINGS_FILE, 'utf-8')); | ||
| } catch { | ||
| // Corrupted file — overwrite with safe defaults | ||
| } | ||
| } | ||
|
|
||
| let changed = false; | ||
| if (settings.teammateMode !== 'bypassPermissions') { | ||
| settings.teammateMode = 'bypassPermissions'; | ||
| changed = true; | ||
| } | ||
| if (settings.skipDangerousModePermissionPrompt !== true) { | ||
| settings.skipDangerousModePermissionPrompt = true; | ||
| changed = true; | ||
| } | ||
|
|
||
| if (changed) { | ||
| writeFileSync(CLAUDE_SETTINGS_FILE, `${JSON.stringify(settings, null, 2)}\n`, 'utf-8'); |
There was a problem hiding this comment.
This Read-Modify-Write (RMW) cycle on the global settings.json file is susceptible to race conditions. If multiple processes (e.g., multiple Genie instances or a user manually editing settings) attempt to update the file simultaneously, one process might overwrite changes made by another because the entire file is read into memory, modified, and then written back. For a shared configuration file, consider using a file lock or an atomic update mechanism to prevent potential data loss of other settings.
|
|
||
| // Ensure the global teammateMode is bypassPermissions so the native team | ||
| // permission gate doesn't route tool approvals to the leader (deadlock). | ||
| ensureTeammateBypassPermissions(); |
There was a problem hiding this comment.
The function ensureTeammateBypassPermissions is synchronous and performs blocking file I/O. Since it is called within the async function ensureNativeTeam, and this module otherwise uses node:fs/promises for filesystem operations, it would be more consistent to implement this as an asynchronous operation and await it here to avoid blocking the event loop.
Summary
ensureTeammateBypassPermissions()toclaude-settings.ts— idempotently setsteammateMode: "bypassPermissions"andskipDangerousModePermissionPrompt: truein~/.claude/settings.jsonensureNativeTeam()so the global setting is always correct when teams are activeRoot Cause
CC's native team permission gate has two layers:
--permission-mode bypassPermissions(genie already passes this)teammateModein~/.claude/settings.json(genie did NOT manage this)When
teammateMode: "auto", the global layer overrides the per-session flag, routing tool approvals to the team leader — which is a non-interactive AI agent, creating an unresolvable deadlock.Test plan
bun run check— 1813 tests pass, 0 failuresgenie spawnand confirm it can use Edit without permission prompt~/.claude/settings.jsongetsteammateMode: "bypassPermissions"after team setup