MAI-43: fix subcommand-flag validation contract#13
Merged
Conversation
The global flag validator in maiass.mjs maintained a single hardcoded list
of valid flags and rejected anything else as "unrecognized option". This
meant legitimate subcommand flags like `version --current` were rejected
before reaching their handler. MAI-39 patched the config case with a
wholesale bypass, but typos on config flags then silently fell through.
This change introduces a per-subcommand FLAGS allow-list (Option B):
* Each subcommand handler exports `FLAGS` — the flags it accepts.
* `lib/flag-validator.js` validates argv against the union of always-
valid built-ins and the active subcommand's FLAGS.
* Unknown flags are still rejected with a useful error and a
Levenshtein-based "Did you mean…?" suggestion (no new deps).
Effects:
* `maiass version --current` now works (previously rejected).
* `maiass config --globl ai_mode=ask` is now rejected with a typo hint
(closes the MAI-39 MINOR).
* `maiass config <four MAI-39 invocations>` continues to work.
* Main pipeline (`maiass`, `maiass minor`, `maiass --auto`) unaffected.
Per-subcommand FLAGS:
* version: --current, --dry-run, --tag, --force
* config: --global, --project, --edit, --list, --show-sensitive,
--list-vars
* account-info: --json
* maiass (workflow): --auto/-a, --auto-commit/-ac, --commits-only/-c,
--auto-stage, --dry-run/-d, --force/-f, --silent/-s, --tag/-t
* hello, env, git-info: none currently — declared empty for the contract
Help text updated to document subcommand-specific flag conventions.
All 10 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Remove `--json` from ALWAYS_VALID_FLAGS (lib/flag-validator.js). It's already exported by account-info's FLAGS, so the global listing was a loose contract: `version --json` / `config --json` passed validation and were silently ignored by handlers that don't consume them. - Drop the redundant `|| validFlags.includes(arg)` clause in validateFlags(). The `=` split already normalises both forms; the second check could never match. - Move auto-mode env-var assignment (MAIASS_AUTO_*) to AFTER flag validation in maiass.mjs. Previously they were set before validation, so `account-info --auto` would write MAIASS_AUTO_STAGE_UNSTAGED etc. into process.env, then the validator would reject and exit. Contained (process exits 1) but order-of-operations was wrong. Verified: - `version --json` -> "Unrecognized option '--json' for command 'version'" - `account-info --json` -> still works (in account-info's FLAGS export) - `account-info --auto` -> rejected before env-var leak - 10/10 tests pass https://velvary.atlassian.net/browse/MAI-43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the broader class of subcommand-flag rejection bugs surfaced by MAI-39's narrow
config-only fix.The global flag validator at
maiass.mjswas rejecting any subcommand-specific flag (version --current,account-info --json, etc.) as "unrecognized option" before it reached the handler that would process it. MAI-39 patched this forconfigonly with a wholesale skip, which then created a new problem — typos likeconfig --globlsilently fell through.This PR generalises the fix with Option B (per-subcommand FLAGS allow-list) and gets useful typo-suggestion errors for free.
Architecture
Each subcommand handler now exports a
FLAGSarray of the flags it accepts.maiass.mjslooks up the active subcommand's flag list and validates the argv against the union ofALWAYS_VALID_FLAGS(globals like--help,--version) + the subcommand's FLAGS. Unknown flags are rejected with a"Did you mean '--foo'?"suggestion driven by a tiny inline Levenshtein implementation (no new deps).Files
lib/flag-validator.js—validateFlags(),ALWAYS_VALID_FLAGS,suggestFlag()+ inline Levenshtein.maiass.mjs— imports each handler's FLAGS; replaces the hardcodedvalidFlagsarray AND the MAI-39command === 'config'skip with a per-command lookup. Auto-mode env-var assignment moved BELOW validation (was a contained leak before —account-info --autowould setMAIASS_AUTO_*before rejection).lib/version-command.js,lib/config-command.js,lib/account-info.js,lib/maiass-command.js—export const FLAGS = [...]of each handler's flags.lib/git-info.js,lib/env-display.js—export const FLAGS = [](no CLI flags accepted today, but the contract is consistent).Bugs fixed
maiass version --current— was rejected; now works (root cause of MAI-43).config --globl ai_mode=ask→"Did you mean '--global'?"(closes the MAI-39 code-review MINOR).version --bogus,git-info --bogus, etc. → rejected with the valid-flags list.Code review (already done)
Reviewer flagged 3 MINORs, all fixed in follow-up commit
dfb6ab4:--jsonwas inALWAYS_VALID_FLAGSbut is account-info-specific — moved exclusively to account-info's FLAGS export soversion --jsonis now correctly rejected (was silently ignored).MAIASS_AUTO_*) was happening BEFORE validation; moved to AFTER so rejected commands don't leak.|| validFlags.includes(arg)clause invalidateFlags()removed (the=split makes both forms identical).Test plan
version --current→ prints version (previously rejected)version --json,account-info --auto→ rejected (loose contract tightened)config --globl ai_mode=ask→ "Did you mean '--global'?" — MAI-39 MINOR closedmaiass,maiass minor,maiass --auto) still runsnpm test→ 10/10 passIntentional semantic narrowing (NOT a regression)
--account-info --autois now rejected because--autoisn't a valid flag foraccount-info. Historically the global validator accepted any listed flag regardless of command —--autowas silently swallowed byhandleAccountInfoCommand(which only reads{ json }). Confirmed by code-reviewer as correct tightening, not a regression.Follow-ups (will file post-merge)
version --currentflag. Does exactly the same thing asmaiass versionwith no arguments — both callgetCurrentVersion()+displayVersionInfo(). Now reachable through the validator, but the redundancy is its own concern.bashmaiassparity — likely has analogous flag-rejection bugs. Worth abash-cli-specialistaudit.Ticket: https://velvary.atlassian.net/browse/MAI-43