-
-
Notifications
You must be signed in to change notification settings - Fork 415
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 all 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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| 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 | ||
|
|
@@ -282,14 +282,16 @@ export function NewSession(props: { | |
| const resolvedModelReasoningEffort = agent === 'codex' && modelReasoningEffort !== 'default' | ||
| ? modelReasoningEffort | ||
| : undefined | ||
| const claudePermissionMode = agent === 'claude' ? permissionMode : undefined | ||
|
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 now drops both Suggested fix: const nonClaudeYolo = agent !== 'claude' && yoloMode
const claudePermissionMode = agent === 'claude' ? permissionMode : undefined
await spawnSession({
...,
yolo: claudePermissionMode === 'bypassPermissions' || nonClaudeYolo,
permissionMode: claudePermissionMode,
}) |
||
| const result = await spawnSession({ | ||
| machineId, | ||
| directory: trimmedDirectory, | ||
| agent, | ||
| model: resolvedModel, | ||
| effort: resolvedEffort, | ||
| modelReasoningEffort: resolvedModelReasoningEffort, | ||
| yolo: yoloMode, | ||
| yolo: claudePermissionMode === '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 now derives Suggested fix: const [nonClaudeYoloMode, setNonClaudeYoloMode] = useState(loadPreferredYoloMode)
const claudePermissionMode = agent === 'claude' ? permissionMode : undefined
const nonClaudeYolo = agent !== 'claude' && nonClaudeYoloMode
await spawnSession({
...,
yolo: claudePermissionMode === 'bypassPermissions' || nonClaudeYolo,
permissionMode: claudePermissionMode,
})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 derives Suggested fix: const [nonClaudeYoloMode, setNonClaudeYoloMode] = useState(loadPreferredYoloMode)
const claudePermissionMode = agent === 'claude' ? permissionMode : undefined
const nonClaudeYolo = agent !== 'claude' && nonClaudeYoloMode
await spawnSession({
...,
yolo: claudePermissionMode === 'bypassPermissions' || nonClaudeYolo,
permissionMode: claudePermissionMode,
}) |
||
| permissionMode: claudePermissionMode, | ||
| sessionType, | ||
| worktreeName: sessionType === 'worktree' ? (worktreeName.trim() || undefined) : undefined | ||
| }) | ||
|
|
@@ -378,10 +380,11 @@ export function NewSession(props: { | |
| isDisabled={isFormDisabled} | ||
| onChange={setModelReasoningEffort} | ||
| /> | ||
| <YoloToggle | ||
| yoloMode={yoloMode} | ||
| <PermissionModeSelector | ||
| agent={agent} | ||
| permissionMode={permissionMode} | ||
| isDisabled={isFormDisabled} | ||
| onToggle={setYoloMode} | ||
| onPermissionModeChange={setPermissionMode} | ||
| /> | ||
|
|
||
| {(error ?? spawnError) ? ( | ||
|
|
||
| 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] This forwards any globally valid
PermissionModeSchemavalue without checking whether it is valid for the selected/default agent.buildCliArgs()then emits--permission-modefor any global mode, while the individual CLI commands reject modes outside their own flavor set, soagent: "codex", permissionMode: "bypassPermissions"fails later in the runner instead of returning 400 here.Suggested fix: