fix(cascade-tools): accept --boolFlag value + wrap oclif parse errors in envelope#1281
Conversation
… in envelope
Prod 2026-05-09: 9/14 codex runs that called pm read-work-item hit
`--includeComments true` with `exit=2` and empty stdout — silent failures
that triggered help-read storms and wasted iterations. Two coupled regressions
against spec 014:
1. nativeToolPrompts.formatParam emitted `# example: --includeComments 'true'`
for boolean flags whose example was a literal `true|false`. Oclif's
`Flags.boolean({ allowNo: true })` rejects that form — the prompt taught a
syntax the CLI rejected ("no prompt lying" rule violation).
2. Oclif's `FailedFlagValidationError`, `FlagInvalidOptionError`, and
`UnexpectedArgsError` escaped the existing unknown-flag catch in
cliCommandFactory. Parse-time failures bypassed emitCliError, so the agent
got exit 2 with no envelope to self-correct from.
Fixes:
- formatParam now renders boolean examples as the canonical toggle (`--key`
or `--no-key`), never `--key 'true'`.
- cliCommandFactory.execute pre-processes argv so boolean flags accept
`--key true|false|yes|no|1|0` (space- or equals-separated, case-insensitive),
rewriting them into oclif's canonical toggle. Malformed values surface
inline through emitCliError as a `flag-parse` envelope with `got`,
`expected`, and `hint` populated.
- classifyParseError wraps the three oclif parse-time error classes into
structured envelopes (missing-required, enum-mismatch, flag-parse fallback).
- New static guard at tests/unit/gadgets/shared/promptExamplesGrammar.test.ts
iterates all 23 production gadget definitions and asserts the rendered
prompt never contains `--<bool-flag> 'true'` / `'false'` for any boolean
param. Verified to fail loudly when the renderer fix is reverted.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…taxonomies Mirrors the existing pattern at formatParam / formatExampleLine — both flagged for the same noExcessiveCognitiveComplexity rule with the same "-taxonomy" rationale. The two new functions added in the previous commit (massageBooleanFlagValues, FactoryCommand.execute's catch chain) are also shape-driven taxonomies that read more clearly inline than split. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Requesting changes because boolean value-form parsing still rejects false for a shipped boolean flag that does not declare allowNo.
Code Issues
Should Fix
- src/gadgets/shared/cliCommandFactory.ts:396 —
massageBooleanFlagValuesrewrites every recognized false value to--no-${name}. That only parses for booleans declared withallowNo.scm create-prhas a real boolean flag,draft, withoutallowNo;--draft falseand--draft=falseare rewritten to--no-draft, which oclif rejects as an unknown flag. I verified this with a local factory smoke:--draft truereaches the core with{ draft: true }, but both false forms emit an unknown-flag envelope for--no-draft. This leaves the PR's promised--boolFlag true|falsesupport incomplete for one of the production commands.
🕵️ codex · gpt-5.5 · run details
| continue; | ||
| } | ||
| if (normalized === false) { | ||
| result.push(`--no-${name}`); |
There was a problem hiding this comment.
This rewrite is only valid for flags whose ParameterDefinition has allowNo. For non-negatable booleans like scm create-pr's draft, --draft false / --draft=false becomes --no-draft and fails as an unknown flag instead of accepting the explicit false value. The preprocessor needs to track whether each boolean supports negation, or otherwise consume false values without producing an unsupported --no-* flag.
There was a problem hiding this comment.
Fixed. The root issue was that collectBooleanFlagNames returned a plain Set<string> with no information about whether each flag supports --no-<name> negation. The preprocessor then blindly emitted --no-draft for any boolean flag receiving a false value.
Change made: collectBooleanFlagNames now returns Map<string, boolean> where the value is the allowNo property from the flag's BooleanParameterDefinition. massageBooleanFlagValues reads that map and:
normalized === false+allowNo === true→ emit--no-${name}(existing behaviour forcommit,push,includeComments)normalized === false+allowNo === false→ consume the token and emit nothing (flag absence = false, which is the correct oclif result for non-negatable booleans likedraft)
The error hint for malformed values is also narrowed — non-negatable flags no longer suggest --no-${name} as a valid alternative.
Three new tests cover the draft-style case: --draft false (space-separated), --draft=false (equals-separated), and --draft true (still works).
`massageBooleanFlagValues` was rewriting every recognised false value to `--no-<name>`, but oclif only accepts that form for flags declared with `allowNo: true`. `scm create-pr`'s `draft` flag has no `allowNo`, so `--draft false` / `--draft=false` was emitted as `--no-draft` and oclif rejected it as an unknown flag — leaving the PR's promised `--boolFlag true|false` support incomplete for one production command. Fix: `collectBooleanFlagNames` now returns a `Map<string, boolean>` where the value tracks `allowNo`. The preprocessor emits `--no-<name>` only when `allowNo` is true; for non-negatable booleans a `false` value simply consumes the token without pushing anything (flag absence = false, which is the correct oclif parse result). Malformed-value error hints are updated to not suggest `--no-<name>` for non-negatable flags. Three new tests added covering the `draft`-style case: - `--draft false` (space-separated) → no unknown-flag error - `--draft=false` (equals-separated) → no unknown-flag error - `--draft true` → still works Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
nhopeatall
left a comment
There was a problem hiding this comment.
LGTM - The boolean value preprocessor preserves existing toggle behavior, handles non-allowNo booleans like draft, and parse-time oclif errors now emit structured envelopes. Focused unit tests passed locally.
🕵️ codex · gpt-5.5 · run details
Summary
Closes the dominant cascade-tools failure mode from the 2026-05-09 prod log analysis: 9/14 codex runs that called
pm read-work-itemfailed with--includeComments true, exit 2, and empty stdout (no diagnostic envelope), then looped on help-reads to recover.Two coupled regressions against spec 014:
nativeToolPrompts.formatParamrendered# example: --includeComments 'true'for any boolean param whoseexamplesarray carried a literaltrue|false. Oclif'sFlags.boolean({ allowNo: true })rejects that form at parse time, contradicting the synopsis it had just rendered ([--no-includeComments]). Codex agents followed the example. This is the "no prompt lying" rule — fixed once for arrays in spec 014, never closed for booleans+examples.FailedFlagValidationError,FlagInvalidOptionError, andUnexpectedArgsErrorescapedcliCommandFactory's existing unknown-flag catch and exited with code 2 + empty stdout — the agent had nothing to self-correct from. Spec 014 promised everyflag-parsefailure would emit{ error: { type, flag, got, expected, hint } }; for the three classes oclif throws at parse time, that promise was silently broken.Changes
src/backends/shared/nativeToolPrompts.ts—formatParamspecial-casestype === 'boolean'for the per-flag example line: emits the canonical toggle (# example: --keyfor true,# example: --no-keyfor false) instead of the rejected value form.src/gadgets/shared/cliCommandFactory.ts—massageBooleanFlagValuesargv preprocessor: accepts--key true|false|yes|no|1|0(space- or equals-separated, case-insensitive), rewrites to oclif's canonical--key/--no-keybefore parsing. Malformed values surface inline throughemitCliErroras aflag-parseenvelope withgot,expected, andhintpopulated — agents get exactly what they need to retry. Pass-through forundefinedargv keeps existing tests green.classifyParseErrorwraps the three oclif parse-time error classes:FailedFlagValidationError→{ type: 'missing-required', flag, hint }FlagInvalidOptionError→{ type: 'enum-mismatch', flag, got, expected }UnexpectedArgsError→{ type: 'flag-parse', got }(catches any boolean-value miss the preprocessor doesn't see)flag-parse. Existing unknown-flag + Levenshtein "did you mean" path is preserved.Static guard at
tests/unit/gadgets/shared/promptExamplesGrammar.test.ts— iterates all 23 production gadget definitions (pm + github + sentry + session) and asserts the rendered prompt never contains--<bool-flag> 'true'/'false'for any boolean param. Verified to fail loudly when the renderer fix is reverted.End-to-end smoke (built CLI, no creds)
Test plan
tests/unit/backends/shared-nativeToolPrompts.test.ts— 38 / 38 (2 new boolean-example cases)tests/unit/cli/cli-command-factory.test.ts— 35 / 35 (14 new: 10 boolean-value-form parametrized + bare toggle + envelope on malformed + enum-not-affected guard + 3 parse-error wraps)tests/unit/gadgets/shared/promptExamplesGrammar.test.ts— 3 / 3 (new static guard; verified fails on revert)npm run lintclean (the 2 remainingnoExcessiveCognitiveComplexitywarnings are pre-existing inmaterialize.ts+sentry/webhook-handler.ts)npm run typecheckcleanOut of scope
The DEBUG/ANSI noise leaking into
cascade-toolsstdout (issue #2 inanalyze-recent-25-runs-swirling-pearl.md— 75/120 calls polluted) and the misleadingcascade-tools --helptopic descriptions (issue #3) are independent fixes; will land in follow-up PRs.🤖 Generated with Claude Code