-
-
Notifications
You must be signed in to change notification settings - Fork 429
feat: replace YoloToggle with PermissionModeSelector for Claude sessions #529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 4 commits
546912a
5ec4d78
990075d
bc15024
8846bc5
43fb6d8
22f8a78
482bc97
c1dc12e
8cdaf73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -414,11 +414,12 @@ export class ApiClient { | |
| yolo?: boolean, | ||
| sessionType?: 'simple' | 'worktree', | ||
| worktreeName?: string, | ||
| effort?: string | ||
| effort?: string, | ||
| permissionMode?: string | ||
| ): Promise<SpawnResponse> { | ||
| return await this.request<SpawnResponse>(`/api/machines/${encodeURIComponent(machineId)}/spawn`, { | ||
| method: 'POST', | ||
| body: JSON.stringify({ directory, agent, model, modelReasoningEffort, yolo, sessionType, worktreeName, effort }) | ||
| body: JSON.stringify({ directory, agent, model, modelReasoningEffort, yolo, sessionType, worktreeName, effort, permissionMode }) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MAJOR] Suggested fix: import { PermissionModeSchema } from '@hapi/protocol/schemas'
const spawnBodySchema = z.object({
directory: z.string().min(1),
agent: z.enum(['claude', 'codex', 'cursor', 'gemini', 'opencode']).optional(),
model: z.string().optional(),
effort: z.string().optional(),
modelReasoningEffort: z.string().optional(),
yolo: z.boolean().optional(),
permissionMode: PermissionModeSchema.optional(),
sessionType: z.enum(['simple', 'worktree']).optional(),
worktreeName: z.string().optional()
})
const result = await engine.spawnSession(
machineId,
parsed.data.directory,
parsed.data.agent,
parsed.data.model,
parsed.data.modelReasoningEffort,
parsed.data.yolo,
parsed.data.sessionType,
parsed.data.worktreeName,
undefined,
parsed.data.effort,
parsed.data.permissionMode
)There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MAJOR] Suggested fix: import { PermissionModeSchema } from '@hapi/protocol/schemas'
const spawnBodySchema = z.object({
directory: z.string().min(1),
agent: z.enum(['claude', 'codex', 'cursor', 'gemini', 'opencode']).optional(),
model: z.string().optional(),
effort: z.string().optional(),
modelReasoningEffort: z.string().optional(),
yolo: z.boolean().optional(),
permissionMode: PermissionModeSchema.optional(),
sessionType: z.enum(['simple', 'worktree']).optional(),
worktreeName: z.string().optional()
})
const result = await engine.spawnSession(
machineId,
parsed.data.directory,
parsed.data.agent,
parsed.data.model,
parsed.data.modelReasoningEffort,
parsed.data.yolo,
parsed.data.sessionType,
parsed.data.worktreeName,
undefined,
parsed.data.effort,
parsed.data.permissionMode
) |
||
| }) | ||
|
Comment on lines
420
to
423
|
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import type { AgentType, ClaudePermissionMode } from './types' | ||
| import { CLAUDE_PERMISSION_MODE_OPTIONS } from './types' | ||
| import { useTranslation } from '@/lib/use-translation' | ||
|
|
||
| export function PermissionModeSelector(props: { | ||
| agent: AgentType | ||
| permissionMode: ClaudePermissionMode | ||
| isDisabled: boolean | ||
| onPermissionModeChange: (value: ClaudePermissionMode) => void | ||
| }) { | ||
| const { t } = useTranslation() | ||
|
|
||
| if (props.agent !== 'claude') { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <div className="flex flex-col gap-1.5 px-3 py-3"> | ||
| <label className="text-xs font-medium text-[var(--app-hint)]"> | ||
| {t('newSession.permissionMode')} | ||
| </label> | ||
| <select | ||
| value={props.permissionMode} | ||
| onChange={(e) => props.onPermissionModeChange(e.target.value as ClaudePermissionMode)} | ||
| disabled={props.isDisabled} | ||
| className="w-full px-3 py-2 text-sm rounded-lg border border-[var(--app-divider)] bg-[var(--app-bg)] text-[var(--app-text)] focus:outline-none focus:ring-2 focus:ring-[var(--app-link)] disabled:opacity-50" | ||
| > | ||
| {CLAUDE_PERMISSION_MODE_OPTIONS.map((option) => ( | ||
| <option key={option.value} value={option.value}> | ||
| {option.label} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| ) | ||
| } |
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,22 +10,22 @@ import { useActiveSuggestions, type Suggestion } from '@/hooks/useActiveSuggesti | |||||
| import { useDirectorySuggestions } from '@/hooks/useDirectorySuggestions' | ||||||
| import { useRecentPaths } from '@/hooks/useRecentPaths' | ||||||
| import { useTranslation } from '@/lib/use-translation' | ||||||
| import type { AgentType, ClaudeEffort, CodexReasoningEffort, SessionType } from './types' | ||||||
| import type { AgentType, ClaudeEffort, ClaudePermissionMode, CodexReasoningEffort, SessionType } from './types' | ||||||
| import { ActionButtons } from './ActionButtons' | ||||||
| import { AgentSelector } from './AgentSelector' | ||||||
| import { DirectorySection } from './DirectorySection' | ||||||
| import { MachineSelector } from './MachineSelector' | ||||||
| import { ModelSelector } from './ModelSelector' | ||||||
| import { ClaudeEffortSelector } from './ClaudeEffortSelector' | ||||||
| import { ReasoningEffortSelector } from './ReasoningEffortSelector' | ||||||
| import { PermissionModeSelector } from './PermissionModeSelector' | ||||||
| import { | ||||||
| loadPreferredAgent, | ||||||
| loadPreferredYoloMode, | ||||||
| loadPreferredPermissionMode, | ||||||
| savePreferredAgent, | ||||||
| savePreferredYoloMode, | ||||||
| savePreferredPermissionMode, | ||||||
| } from './preferences' | ||||||
| import { SessionTypeSelector } from './SessionTypeSelector' | ||||||
| import { YoloToggle } from './YoloToggle' | ||||||
| import { formatRunnerSpawnError } from '../../utils/formatRunnerSpawnError' | ||||||
|
|
||||||
| export function NewSession(props: { | ||||||
|
|
@@ -53,7 +53,7 @@ export function NewSession(props: { | |||||
| const [model, setModel] = useState('auto') | ||||||
| const [effort, setEffort] = useState<ClaudeEffort>('auto') | ||||||
| const [modelReasoningEffort, setModelReasoningEffort] = useState<CodexReasoningEffort>('default') | ||||||
| const [yoloMode, setYoloMode] = useState(loadPreferredYoloMode) | ||||||
| const [permissionMode, setPermissionMode] = useState<ClaudePermissionMode>(loadPreferredPermissionMode) | ||||||
| const [sessionType, setSessionType] = useState<SessionType>('simple') | ||||||
| const [worktreeName, setWorktreeName] = useState('') | ||||||
| const [directoryCreationConfirmed, setDirectoryCreationConfirmed] = useState(false) | ||||||
|
|
@@ -76,8 +76,8 @@ export function NewSession(props: { | |||||
| }, [agent]) | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| savePreferredYoloMode(yoloMode) | ||||||
| }, [yoloMode]) | ||||||
| savePreferredPermissionMode(permissionMode) | ||||||
| }, [permissionMode]) | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| if (props.machines.length === 0) return | ||||||
|
|
@@ -289,7 +289,8 @@ export function NewSession(props: { | |||||
| model: resolvedModel, | ||||||
| effort: resolvedEffort, | ||||||
| modelReasoningEffort: resolvedModelReasoningEffort, | ||||||
| yolo: yoloMode, | ||||||
| yolo: permissionMode === 'bypassPermissions', | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [MAJOR] This still derives Suggested fix: const claudePermissionMode = agent === 'claude' ? permissionMode : undefined
const result = await spawnSession({
machineId,
directory: trimmedDirectory,
agent,
model: resolvedModel,
effort: resolvedEffort,
modelReasoningEffort: resolvedModelReasoningEffort,
yolo: claudePermissionMode === 'bypassPermissions',
permissionMode: claudePermissionMode,
sessionType,
worktreeName: sessionType === 'worktree' ? (worktreeName.trim() || undefined) : undefined
})
|
||||||
| yolo: permissionMode === 'bypassPermissions', | |
| yolo: agent === 'claude' && permissionMode === 'bypassPermissions' ? true : undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[MAJOR] This still derives yolo from the saved Claude permissionMode even when the current agent is not Claude. Since PermissionModeSelector is hidden for non-Claude agents, a saved bypassPermissions value can silently spawn Codex/Gemini/Cursor/OpenCode with unsafe mode and no visible control.
Suggested fix:
const claudePermissionMode = agent === 'claude' ? permissionMode : undefined
const result = await spawnSession({
machineId,
directory: trimmedDirectory,
agent,
model: resolvedModel,
effort: resolvedEffort,
modelReasoningEffort: resolvedModelReasoningEffort,
yolo: claudePermissionMode === 'bypassPermissions',
permissionMode: claudePermissionMode,
sessionType,
worktreeName: sessionType === 'worktree' ? (worktreeName.trim() || undefined) : undefined
})There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Major] Hidden Claude bypass state still sets YOLO for non-Claude spawns. PermissionModeSelector returns null for non-Claude agents, but this line still sends yolo: true whenever the persisted Claude mode is bypassPermissions. Gate both yolo and permissionMode on the selected agent.
Suggested fix:
const claudePermissionMode = agent === 'claude' ? permissionMode : undefined
const result = await spawnSession({
machineId,
directory: trimmedDirectory,
agent,
model: resolvedModel,
effort: resolvedEffort,
modelReasoningEffort: resolvedModelReasoningEffort,
yolo: claudePermissionMode === 'bypassPermissions',
permissionMode: claudePermissionMode,
sessionType,
worktreeName: sessionType === 'worktree' ? (worktreeName.trim() || undefined) : undefined
})| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| import type { AgentType } from './types' | ||
| import type { AgentType, ClaudePermissionMode } from './types' | ||
| import { CLAUDE_PERMISSION_MODES } from '@hapi/protocol' | ||
|
|
||
| const AGENT_STORAGE_KEY = 'hapi:newSession:agent' | ||
| const YOLO_STORAGE_KEY = 'hapi:newSession:yolo' | ||
| const PERMISSION_MODE_STORAGE_KEY = 'hapi:newSession:permissionMode' | ||
|
|
||
| const VALID_AGENTS: AgentType[] = ['claude', 'codex', 'cursor', 'gemini', 'opencode'] | ||
|
|
||
|
|
@@ -40,3 +42,28 @@ export function savePreferredYoloMode(enabled: boolean): void { | |
| // Ignore storage errors | ||
| } | ||
| } | ||
|
|
||
| export function loadPreferredPermissionMode(): ClaudePermissionMode { | ||
| try { | ||
| const stored = localStorage.getItem(PERMISSION_MODE_STORAGE_KEY) | ||
| if (stored && (CLAUDE_PERMISSION_MODES as readonly string[]).includes(stored)) { | ||
| return stored as ClaudePermissionMode | ||
| } | ||
| // Migrate from legacy yolo toggle | ||
| if (localStorage.getItem(YOLO_STORAGE_KEY) === 'true') { | ||
| savePreferredPermissionMode('bypassPermissions') | ||
| return 'bypassPermissions' | ||
| } | ||
| } catch { | ||
| // Ignore storage errors | ||
| } | ||
| return 'default' | ||
| } | ||
|
Comment on lines
+46
to
+61
|
||
|
|
||
| export function savePreferredPermissionMode(mode: ClaudePermissionMode): void { | ||
| try { | ||
| localStorage.setItem(PERMISSION_MODE_STORAGE_KEY, mode) | ||
| } catch { | ||
| // Ignore storage errors | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,7 @@ type SpawnInput = { | |||||
| effort?: string | ||||||
| modelReasoningEffort?: string | ||||||
| yolo?: boolean | ||||||
| permissionMode?: string | ||||||
|
||||||
| permissionMode?: string | |
| permissionMode?: Parameters<ApiClient['spawnSession']>[10] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Minor]
permissionModeshould use the shared schema instead ofz.string()plus a cast. As written, invalid values and modes that are valid globally but invalid for the requested agent can pass the API boundary and fail later in the runner/CLI path.Suggested fix: