Skip to content

fix(cli): wrap fs.Parse with reorderFlagsBeforePositional in 7 commands#25

Merged
nnemirovsky merged 1 commit intomainfrom
fix-cli-flag-ordering
Apr 11, 2026
Merged

fix(cli): wrap fs.Parse with reorderFlagsBeforePositional in 7 commands#25
nnemirovsky merged 1 commit intomainfrom
fix-cli-flag-ordering

Conversation

@nnemirovsky
Copy link
Copy Markdown
Owner

Summary

Seven CLI subcommands called `fs.Parse(args)` directly instead of going through `reorderFlagsBeforePositional`, so any positional argument appearing before a flag stopped flag parsing and silently fell through to flag defaults. For commands that take a `--db` flag this is a production hazard:

```
sluice binding remove 2 --db /custom/path
```

stopped at `2`, never saw `--db`, and operated on the default `data/sluice.db` instead of the requested path. The same shape applies to `cred remove`, `policy remove`, `policy import`, `mcp remove`, `channel update`, and `channel remove`. Each command gets the same one-line wrap that all the other working subcommands already use. A regression test per command exercises the positional-before-flags ordering so this cannot recur silently.

Also fixes a small cosmetic bug from v0.8.0: the `sluice cred` dispatcher's usage line still listed `[add|list|remove]` and never mentioned the `update` subcommand.

Test plan

  • `go build ./...`
  • `go test ./... -timeout 120s` clean
  • `gofumpt -l cmd/sluice/ internal/` empty
  • `golangci-lint run ./...` 0 issues
  • 6 new regression tests covering positional-before-flags ordering for binding/cred/policy/mcp/channel

…g commands

Seven CLI subcommands called fs.Parse(args) directly instead of going
through reorderFlagsBeforePositional, so any positional argument before
a flag stopped flag parsing and silently fell through to flag defaults.

For commands that take a --db flag this is a production hazard. Running

    sluice binding remove 2 --db /custom/path

stopped at "2", missed --db, and operated on the default
data/sluice.db instead of the requested path. The same shape applies to
cred remove, policy remove, policy import, mcp remove, channel update,
and channel remove.

Wrap each fs.Parse call with reorderFlagsBeforePositional like the other
sibling subcommands already do, and add a regression test per command
that exercises the positional-before-flags ordering.

Also fix the cred dispatcher's usage line to mention the "update"
subcommand that was added in v0.8.0.
@nnemirovsky nnemirovsky merged commit 7478657 into main Apr 11, 2026
6 checks passed
@nnemirovsky nnemirovsky deleted the fix-cli-flag-ordering branch April 11, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant