[soustruh]

What was wrong

• The spec did not say which error code a refusal gets when the target cannot do what the command line asks. Two forms exist: storage create-table --source-table-id on a project that is not BigQuery is INVALID_ARGUMENT (exit 2), and FEATURE_NOT_ENABLED and AUTH_NOT_SUPPORTED_ON_STACK are exit 5 (type configuration). A new option could take either form (feat(AJDA-3460): workspace create --governed #892).
• A7 said "Booleans are --x/--no-x". The commands do otherwise: about 270 flags with the default false are one-way (--dry-run, --ui), and the 13 flag pairs all have the default true or no default. A review of feat(AJDA-3460): workspace create --governed #892 asked for --no-governed because of the text of the rule.

What changed

• New rule E8: the error code of a refusal follows the fix. A fix on the command line is INVALID_ARGUMENT (exit 2). A fix in the project, the stack or the kbagent configuration is a code of type configuration (exit 5). Existing refusals keep their codes (C1). Checked by review.
• A7 now says what the commands do: a boolean is a flag, never an option that takes true or false. A flag with the default false can be one-way. A flag with the default true, or one whose omission keeps the current value, also has an off form.
• New blocking rule args.boolean-flag-shape in make cli-spec-check. It fails on an option that takes a boolean value and on a flag with the default true and no off form. The current commands have no violation, so the baseline has no entry. A unit test covers both cases.

make check passes.

[zajca]

Automated code review

Reviewed by Opus + Sonnet, full review of a79f589d45a9.

Opus: The PR changes three things. It adds rule E8 to docs/cli-interface-spec.md: a refusal that can be fixed on the command line gets INVALID_ARGUMENT (exit 2), and one that needs a fix in the project, stack or configuration gets a code of type configuration (exit 5). It rewrites the boolean part of A7 to describe the flag shapes the commands actually use. It also adds a blocking tree rule args.boolean-flag-shape to scripts/check_cli_spec.py, with a baseline entry and a unit test. The trusted base is 7ca1e06, and I read its CLAUDE.md and the spec under docs/ (there is no .agents/skills/test-audit skill). I checked E8's examples against src/keboola_agent_cli/errors.py. INVALID_ARGUMENT maps to validation/2, and FEATURE_NOT_ENABLED and AUTH_NOT_SUPPORTED_ON_STACK map to configuration/5. I also ran the head version of the new rule against the real Typer command tree. It reports no violations, which matches the empty baseline. All 10 default-true flags have an off form, and the only default-None flags (agent update --enabled/--disabled, --manual/--auto) are pairs. The ratchet accepts the new blocking rule because the base baseline does not know it yet. Tests: the new unit test checks the rule's actual output for a one-way, a paired, a default-true one-way and a value-taking boolean option. It would turn red if the secondary_opts check were dropped (the --pair case would be flagged) or if the boolean-type branch were removed. Compatibility: no CLI behavior, output, error code or config changes. E8 says existing refusals keep their codes, so nothing breaks for consumers. Reuse: the rule builds on the existing is_option, label and type_name helpers, and the base has no existing check that covers boolean flag shape. I found no blocking correctness, security, compatibility, test or maintainability issues. The one non-blocking note is that the automated check covers less than the A7 text. Running the rule required a virtualenv, so uv created an untracked, gitignored .venv in the review worktree, and removing it was not allowed. No tracked files were changed.

Sonnet: The effective change relative to the trusted base (7ca1e06) is one commit, a79f589. It adds the error-code rule E8 to docs/cli-interface-spec.md. It extends rule A7 with a boolean-flag shape check, args.boolean-flag-shape, implemented as r_boolean_flag_shape in scripts/check_cli_spec.py. It registers that rule as blocking in scripts/cli_spec_baseline.json. It also adds a unit test in tests/test_check_cli_spec.py. The three-dot diff against main is large (683 files) only because the local main ref is stale. I reviewed the 4-file base-to-head diff in full. I could not run the spec checker because typer is not installed here, so I checked the new rule against the CLI sources by grep. The only flags with default True in src/ are --enabled/--disabled, --stream/--no-stream, --workspace/--no-workspace, --git-public/--no-git-public, --auto-create/--no-auto-create, --create-buckets/--no-create-buckets and --read-only/--no-read-only. All of them have an off form, so the rule should pass on the current tree and the blocking baseline entry with no exception list is consistent. The hidden root-flag copies in root_command.py use default False, so they are not flagged either. No repository AGENTS.md or CLAUDE.md guidance applies to these paths that conflicts with the change. Tests: the new test feeds the rule one-way, paired, default-true-without-off-form and value-boolean options and asserts the exact violation map, so removing the secondary_opts check or the boolean-type check would turn it red. Compatibility: the change touches only the spec, the checker script and the baseline, so the CLI surface and error codes are unchanged. E8 is a rule for new refusals and keeps existing codes. Reuse: the rule follows the existing r_* checker pattern and reuses is_option, type_name and label. I found no duplicated functionality. I found no actionable correctness, security, compatibility, test or maintainability issues in the trusted review scope.

No blocking findings were identified.

1 non-blocking note(s): no action required in this pull request

O001 LOW — Boolean flag rule misses one-way flags whose omission keeps the current value (confidence: medium, Opus)
scripts/check_cli_spec.py:758

The new A7 text says a flag with the default true, "or one whose omission keeps the current value", must also have an off form. r_boolean_flag_shape only checks p.default is True. A new update-style command with a one-way tri-state flag would pass make cli-spec-check even though A7 forbids it. For example, a Typer Optional[bool] = typer.Option(None, "--enabled") becomes a flag with default None and no secondary_opts, so it can set the value to true but never back to false. Checking p.default is not False (true or None) would cover the whole A7 text, and the current tree would still pass: the only default-None flags, agent update --enabled/--disabled and --manual/--auto, are already pairs. A test case for a default-None one-way flag could then be added to test_boolean_flag_shape_finds_a_value_boolean_and_a_flag_that_stays_on. The spec's 'Checked by' line names only the default-true case and lists review as a backstop, so this is not blocking.

🤖 Written by AI: Hermes pr-review (posted from zajca's GitHub account, not by a human)

[keboola-pr-reviewer-bot]

Verdict: auto_approve (risk 1/5) · profile _default

Safe additive spec-docs plus lint-tooling change that strengthens a CI check and touches no runtime code.