refactor(cli): simplify agent mode validation flow
What I changed: - extracted a single missingModeError helper in agent/mode.ts so the command no longer duplicates the same command error payload in two branches - flattened control flow by returning early for --list and using one set-mode path guarded by explicit missing-mode validation - tightened tests/10-agent-mode.test.ts to assert the exact missing-mode error message instead of allowing a daemon-connection fallback Reasoning: - this keeps behavior the same while reducing branch duplication and making the validation contract explicit - the hardened test protects the intended precedence: missing required mode should fail before any daemon interaction Verification: - npm run typecheck - (cd packages/cli && npx tsx tests/10-agent-mode.test.ts) Notes for next agent: - repository had a pre-existing local modification in scripts/codex-refactor-loop.sh; I intentionally did not touch or stage it - no blockers encountered for this refactor
This commit is contained in:
@@ -37,6 +37,12 @@ export interface AgentModeOptions extends CommandOptions {
|
|||||||
list?: boolean
|
list?: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const missingModeError = (): CommandError => ({
|
||||||
|
code: 'MISSING_ARGUMENT',
|
||||||
|
message: 'Mode argument required unless --list is specified',
|
||||||
|
details: 'Usage: paseo agent mode <id> <mode> | paseo agent mode --list <id>',
|
||||||
|
})
|
||||||
|
|
||||||
// This command returns two different data shapes (set result vs mode list).
|
// This command returns two different data shapes (set result vs mode list).
|
||||||
// Keep `any` here to match the existing output wrapper generic contract.
|
// Keep `any` here to match the existing output wrapper generic contract.
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
@@ -53,12 +59,7 @@ export async function runModeCommand(
|
|||||||
|
|
||||||
// Validate arguments
|
// Validate arguments
|
||||||
if (!options.list && !normalizedMode) {
|
if (!options.list && !normalizedMode) {
|
||||||
const error: CommandError = {
|
throw missingModeError()
|
||||||
code: 'MISSING_ARGUMENT',
|
|
||||||
message: 'Mode argument required unless --list is specified',
|
|
||||||
details: 'Usage: paseo agent mode <id> <mode> | paseo agent mode --list <id>',
|
|
||||||
}
|
|
||||||
throw error
|
|
||||||
}
|
}
|
||||||
|
|
||||||
let client: Awaited<ReturnType<typeof connectToDaemon>> | undefined
|
let client: Awaited<ReturnType<typeof connectToDaemon>> | undefined
|
||||||
@@ -90,27 +91,22 @@ export async function runModeCommand(
|
|||||||
data: items,
|
data: items,
|
||||||
schema: modeListSchema,
|
schema: modeListSchema,
|
||||||
}
|
}
|
||||||
} else {
|
}
|
||||||
if (!normalizedMode) {
|
|
||||||
const error: CommandError = {
|
|
||||||
code: 'MISSING_ARGUMENT',
|
|
||||||
message: 'Mode argument required unless --list is specified',
|
|
||||||
details: 'Usage: paseo agent mode <id> <mode> | paseo agent mode --list <id>',
|
|
||||||
}
|
|
||||||
throw error
|
|
||||||
}
|
|
||||||
|
|
||||||
// Set the agent mode
|
if (!normalizedMode) {
|
||||||
await client.setAgentMode(resolvedId, normalizedMode)
|
throw missingModeError()
|
||||||
|
}
|
||||||
|
|
||||||
return {
|
// Set the agent mode
|
||||||
type: 'single',
|
await client.setAgentMode(resolvedId, normalizedMode)
|
||||||
data: {
|
|
||||||
agentId: resolvedId.slice(0, 7),
|
return {
|
||||||
mode: normalizedMode,
|
type: 'single',
|
||||||
},
|
data: {
|
||||||
schema: setModeSchema,
|
agentId: resolvedId.slice(0, 7),
|
||||||
}
|
mode: normalizedMode,
|
||||||
|
},
|
||||||
|
schema: setModeSchema,
|
||||||
}
|
}
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// Re-throw if it's already a CommandError
|
// Re-throw if it's already a CommandError
|
||||||
|
|||||||
@@ -125,12 +125,10 @@ try {
|
|||||||
// Should fail because mode is required unless --list is specified
|
// Should fail because mode is required unless --list is specified
|
||||||
assert.notStrictEqual(result.exitCode, 0, 'should fail without mode argument')
|
assert.notStrictEqual(result.exitCode, 0, 'should fail without mode argument')
|
||||||
const output = result.stdout + result.stderr
|
const output = result.stdout + result.stderr
|
||||||
const hasError =
|
assert(
|
||||||
output.toLowerCase().includes('missing') ||
|
output.includes('Mode argument required unless --list is specified'),
|
||||||
output.toLowerCase().includes('required') ||
|
'error should mention missing mode argument'
|
||||||
output.toLowerCase().includes('mode') ||
|
)
|
||||||
output.toLowerCase().includes('daemon') // If daemon error comes first, that's also valid
|
|
||||||
assert(hasError, 'error should mention missing mode or connection issue')
|
|
||||||
console.log('✓ agent mode requires mode argument when not using --list\n')
|
console.log('✓ agent mode requires mode argument when not using --list\n')
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
|
|||||||
Reference in New Issue
Block a user