Repository navigation
Print full help on command misuse for invoking agents #14198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+223
−17
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
internal/ghcmd/cmd.go—printError, fullHelp branch (~L306-L313)Heads-up: I noticed this discrepancy while reviewing and asked Copilot to dig into the case, so @niik please sanity check this one more time before acting on it.
Repro (compare the two):
The difference: the first (human) prints the terse
Usage: ... Available commands: ..., while the second (agent) printsWriteHelp's full grouped help instead, so the flatAvailable commands:block is gone rather than just having extra help added to it.idea: In
fullHelpmode we should append the terse usage so agents keep everything a human sees. Right nowWriteHelpreplacescmd.UsageString(), so the flatAvailable commands:block (and the flag list on leaf commands) is dropped for agents on root-level misuse.Verified behavior:
gh zzqqxx(root unknown command) reachesprintError. Human mode shows the terseUsage: ... Available commands: ...; agent mode showsWriteHelp's grouped rendering instead, so the literalAvailable commands:block is gone.gh pr zzqqxx(nested unknown subcommand) never reachesprintErrorat all - it's handled earlier bynestedSuggestFunc, so it stays terse in both modes.Heads-up on the "capture
cmd.UsageString()first" idea: for realghcommandscmd.UsageString()returns an empty string and writes the usage toIOStreams.ErrOutas a side effect, because gh'sSetUsageFunc(root.go:115) writes straight to the capturedErrOutand ignores the buffer Cobra swaps in. Empirically it returns len 0 and leaks ~946 bytes to ErrOut. So capturing it won't capture anything. Instead expose the terse renderer and call it directly.I also checked the "WriteHelp mutates Cobra state that UsageString reads" theory - it does not;
UsageStringreturns the same thing before and afterWriteHelp. So no capture/reorder is needed; we can just append.Suggested approach (spans two files, so not a single suggestion block):
In
pkg/cmd/root/help.go, expose the terse renderer:In
internal/ghcmd/cmd.go, append it in thefullHelpbranch:Verified output for
AI_AGENT=foo gh zzqqxx: full help, thenUsage: gh <command> <subcommand> [flags]+Available commands:list appended at the end.Note: this will change the expected output in the newly added
Test_printErrorfull-help cases, so thosewantOutvalues need updating to include the appended usage.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for flagging this, and for the nudge to sanity check — I dug in properly and had the findings independently verified before acting.
Your mechanism is right on both counts.
cmd.UsageString()does return""for real gh commands:SetUsageFunc(root.go:115) writes straight to the capturedErrOut, so the usage text you see is a side effect and thefmt.Fprintln(out, cmd.UsageString())contributes only a blank line. Confirmed ongh pr list --badFlag: 972 bytes to stderr, exactly oneUsage:line and one--webline, terminated by a lone\n— if the string were actually being returned we'd see it twice. Worth noting the existing human path only works becauseoutandErrOutare the same stream.And
gh pr zzqqxxdoes bypassprintErrorentirely, byte-identical in both modes, as you said.Where I land differently is on the consequence, so let me show the numbers rather than just assert it. Comparing information content rather than the literal string, for
gh zzqqxx:The set difference human-minus-agent is empty. The same holds on leaf commands, which I think is the part worth double-checking against the "(and the flag list on leaf commands) is dropped" note: for
gh pr list --badFlag, agent mode contains all 14 flags the human sees, plus--helpand--repo, plus the JSON fields and examples. Nothing is dropped in either case — the grouped rendering is a strict superset, not a replacement that loses something.So appending the terse usage would reprint the same command names and flags in the less informative form: roughly +14% at root, and about +38% on
gh pr create --badFlagwhere it would duplicate the entire FLAGS block. Given the token-cost pressure this epic is tracking, I'd rather not pay that for content already present.One further reason to avoid
WriteUsagespecifically: callingrootUsageFuncdirectly would bypass the alias resolution added in 3a73af2, re-introducing the alias-stub problem forgh <alias> --badFlagthat the other review thread caught.That said, I think you're onto something real with the nested case. Because
nestedSuggestFunchandlesgh pr zzqqxxbeforeprintError, agents get the terse list and no full help there — which is arguably the more valuable gap, since that's a much more common agent mistake than root-level misuse. It's outside this PR's changed path, but I'd be happy to open a follow-up issue for it if you agree.Happy to be overruled on the append if you still see a case I'm missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking deeper, @niik. I agree that this is so hacky already and touching it is a scope creep. Let's leave it as is for now.