fix(cli): make boolean flags able to say false

`--is-active false` turned sharing ON and reported success. Booleans were
declared presence-only, so the flag meant `true` and commander dropped the
`false` as an argument the command had no use for — silently, because excess
arguments are ignored by default.

A required boolean now takes its value (`--is-active <true|false>`): it is a
state to set, not a switch to flip on, and as a presence flag it could only
ever send one of the two values it needs to express.

Optional booleans stay presence-flags — `--deployed-only` reads better than
`--deployed-only true` — but each also gets `--no-<name>`. Omitting one means
"leave it alone", which is not the same as setting it false; without the
negation there was no way to disable an MCP server or unlock a folder.

Excess arguments are now an error on every generated command, so a value
attached to the wrong flag stops rather than being silently discarded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0148a65fkPhP4N8tgGPYtRTU

Author: TheodoreSpeaks

packages/sim-cli/src/runtime/build.test.ts

@@ -28,6 +28,14 @@ vi.mock('../context.js', () => ({
 function program(): Command {
   const root = new Command('sim').exitOverride()
   for (const group of buildGeneratedCommands(new Set())) root.addCommand(group)
+  // Recursively, not just on the root: a parse error raised by a leaf (an
+  // unknown option, an excess argument) exits the process otherwise, which a
+  // test cannot assert on.
+  const override = (command: Command) => {
+    command.exitOverride()
+    command.commands.forEach(override)
+  }
+  override(root)
   return root
 }
 
@@ -270,3 +278,53 @@ describe('rows whose content sits in a wrapper', () => {
     expect(lines[1]).toContain('E')
   })
 })
+
+describe('boolean flags', () => {
+  it('takes an explicit value when the field is required', async () => {
+    // As a presence-only flag this could only ever send `true`: `--is-active
+    // false` turned sharing ON and reported success, with the `false` dropped
+    // as a stray argument.
+    const [, options] = await run([
+      'files',
+      'share',
+      'set',
+      'f_1',
+      '--is-active',
+      'false',
+      '--auth-type',
+      'public',
+    ])
+    expect(options.body).toMatchObject({ isActive: false })
+
+    const [, on] = await run([
+      'files',
+      'share',
+      'set',
+      'f_1',
+      '--is-active',
+      'true',
+      '--auth-type',
+      'public',
+    ])
+    expect(on.body).toMatchObject({ isActive: true })
+  })
+
+  it('negates an optional boolean, which omitting it cannot do', async () => {
+    // Omitting `enabled` means "leave it alone"; there was no way to say false,
+    // so an MCP server could not be disabled or a folder unlocked.
+    const [, off] = await run(['mcp-servers', 'update', 'mcp_1', '--no-enabled'])
+    expect(off.body).toMatchObject({ enabled: false })
+
+    const [, on] = await run(['mcp-servers', 'update', 'mcp_1', '--enabled'])
+    expect(on.body).toMatchObject({ enabled: true })
+
+    const [, absent] = await run(['mcp-servers', 'update', 'mcp_1', '--name', 'x'])
+    expect(absent.body).not.toHaveProperty('enabled')
+  })
+
+  it('rejects an argument the command has no meaning for', async () => {
+    await expect(run(['mcp-servers', 'update', 'mcp_1', '--enabled', 'bogus'])).rejects.toThrow(
+      /too many arguments/
+    )
+  })
+})

packages/sim-cli/src/runtime/build.ts

@@ -205,7 +205,27 @@ function addFieldOption(
   }
 
   if (descriptor.kind === 'boolean') {
+    // A required boolean is a state to set, not a switch to flip on: it takes
+    // the value explicitly. As a presence-only flag it could only ever send
+    // `true`, so `--is-active false` set sharing ON — commander read the flag as
+    // true and dropped the `false` as a stray argument.
+    if (descriptor.required) {
+      command.addOption(
+        new Option(`${short}--${name} <true|false>`, flag.describe ?? `Set ${field}`).choices([
+          'true',
+          'false',
+        ])
+      )
+      return
+    }
+
+    // Optional booleans stay presence-flags — `--deployed-only` reads better
+    // than `--deployed-only true` — but every one of them also gets a negation,
+    // because for a state field (`enabled`, `locked`) omitting the flag means
+    // "leave it alone", which is not the same as setting it false. Without this
+    // there was no way to disable an MCP server or unlock a folder.
     command.option(`${short}--${name}`, flag.describe ?? `Set ${field}`)
+    command.option(`--no-${name}`, `Set ${field} to false`)
     return
   }
 
@@ -246,6 +266,10 @@ function buildLeaf(operation: V2OperationName, spec: CommandSpec, leafName: stri
   // NAME, so `sim tables upsert` would never match it and would silently fall
   // through to the group's help. Arguments have to be declared separately.
   const command = new Command(leafName)
+  // Commander ignores arguments beyond those declared. That silence is how
+  // `--is-active false` ran as though the `false` had never been typed; an
+  // argument the command has no meaning for is a mistake worth stopping on.
+  command.allowExcessArguments(false)
   for (const param of operationSpec.pathParams) {
     command.argument(`<${param}>`)
   }