Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions internal/ghcmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func Main() exitCode {
return exitCode(extError.ExitCode())
}

printError(stderr, err, cmd, hasDebug)
printError(stderr, ioStreams.ColorScheme(), err, cmd, hasDebug, invokingAgent != "")

if strings.Contains(err.Error(), "Incorrect function") {
fmt.Fprintln(stderr, "You appear to be running in MinTTY without pseudo terminal support.")
Expand Down Expand Up @@ -279,7 +279,12 @@ func isExtensionCommand(rootCmd *cobra.Command, args []string) bool {
return err == nil && c != nil && c.GroupID == "extension"
}

func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) {
// printError writes err to out, followed by usage information when the error
// is the result of command misuse. When fullHelp is set the complete help text
// is written instead of the terse usage string, giving AI agents the examples,
// JSON fields and environment variables they need to correct themselves without
// a second round trip.
func printError(out io.Writer, cs *iostreams.ColorScheme, err error, cmd *cobra.Command, debug, fullHelp bool) {
var dnsError *net.DNSError
if errors.As(err, &dnsError) {
fmt.Fprintf(out, "error connecting to %s\n", dnsError.Name)
Expand All @@ -297,6 +302,13 @@ func printError(out io.Writer, err error, cmd *cobra.Command, debug bool) {
if !strings.HasSuffix(err.Error(), "\n") {
fmt.Fprintln(out)
}
if fullHelp {

Copy link
Copy Markdown
Member

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):

go run ./cmd/gh xyz
AI_AGENT=foo go run ./cmd/gh xyz

The difference: the first (human) prints the terse Usage: ... Available commands: ..., while the second (agent) prints WriteHelp's full grouped help instead, so the flat Available commands: block is gone rather than just having extra help added to it.


idea: In fullHelp mode we should append the terse usage so agents keep everything a human sees. Right now WriteHelp replaces cmd.UsageString(), so the flat Available commands: block (and the flag list on leaf commands) is dropped for agents on root-level misuse.

Verified behavior:

  • gh zzqqxx (root unknown command) reaches printError. Human mode shows the terse Usage: ... Available commands: ...; agent mode shows WriteHelp's grouped rendering instead, so the literal Available commands: block is gone.
  • gh pr zzqqxx (nested unknown subcommand) never reaches printError at all - it's handled earlier by nestedSuggestFunc, so it stays terse in both modes.

Heads-up on the "capture cmd.UsageString() first" idea: for real gh commands cmd.UsageString() returns an empty string and writes the usage to IOStreams.ErrOut as a side effect, because gh's SetUsageFunc (root.go:115) writes straight to the captured ErrOut and 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; UsageString returns the same thing before and after WriteHelp. 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:

// WriteUsage renders the terse usage string for command to w. cmd.UsageString()
// cannot be used for this because gh sets a usage function that writes straight
// to IOStreams.ErrOut, so UsageString returns an empty string and leaks the
// usage onto ErrOut as a side effect.
func WriteUsage(w io.Writer, command *cobra.Command) error {
	return rootUsageFunc(w, command)
}

In internal/ghcmd/cmd.go, append it in the fullHelp branch:

if fullHelp {
	// Render into out rather than calling cmd.Help(), which would send
	// the help text to stdout and split a single failure across two streams.
	root.WriteHelp(out, cs, cmd)
	// Also append the terse usage so agents keep everything a human sees,
	// including the "Available commands" list that WriteHelp replaces with
	// its grouped rendering.
	fmt.Fprintln(out)
	_ = root.WriteUsage(out, cmd)
	fmt.Fprintln(out)
	return
}

Verified output for AI_AGENT=foo gh zzqqxx: full help, then Usage: gh <command> <subcommand> [flags] + Available commands: list appended at the end.

Note: this will change the expected output in the newly added Test_printError full-help cases, so those wantOut values need updating to include the appended usage.


Copy link
Copy Markdown
Contributor Author

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 captured ErrOut, so the usage text you see is a side effect and the fmt.Fprintln(out, cmd.UsageString()) contributes only a blank line. Confirmed on gh pr list --badFlag: 972 bytes to stderr, exactly one Usage: line and one --web line, 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 because out and ErrOut are the same stream.

And gh pr zzqqxx does bypass printError entirely, 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:

  • human: 36 bare command names (429 bytes)
  • agent: the same 36 names, each with a description, plus group structure and the 8 help topics (2,837 bytes)

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 --help and --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 --badFlag where 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 WriteUsage specifically: calling rootUsageFunc directly would bypass the alias resolution added in 3a73af2, re-introducing the alias-stub problem for gh <alias> --badFlag that the other review thread caught.

That said, I think you're onto something real with the nested case. Because nestedSuggestFunc handles gh pr zzqqxx before printError, 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.

Copy link
Copy Markdown
Member

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.

// Render into out rather than calling cmd.Help(), which would send
// the help text to stdout and split a single failure across two
// streams.
root.WriteHelp(out, cs, cmd)
Comment thread
niik marked this conversation as resolved.
Resolved
return
}
fmt.Fprintln(out, cmd.UsageString())
}
}
Expand Down
101 changes: 91 additions & 10 deletions internal/ghcmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,37 @@ import (
"os"
"testing"

"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/agents"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
ghmock "github.com/cli/cli/v2/internal/gh/mock"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
ghAPI "github.com/cli/go-gh/v2/pkg/api"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func Test_printError(t *testing.T) {
cmd := &cobra.Command{}
rootCmd := &cobra.Command{Use: "gh"}
cmd := &cobra.Command{
Use: "spend",
Short: "Spend money",
Example: heredoc.Doc(`
$ gh spend --amount 1
`),
}
cmd.Flags().Int("amount", 0, "How much to spend")
rootCmd.AddCommand(cmd)

type args struct {
err error
cmd *cobra.Command
debug bool
err error
cmd *cobra.Command
debug bool
fullHelp bool
}
tests := []struct {
name string
Expand Down Expand Up @@ -63,7 +75,7 @@ check your internet connection or https://githubstatus.com
cmd: cmd,
debug: false,
},
wantOut: "unknown flag --foo\n\nUsage:\n\n",
wantOut: "unknown flag --foo\n\n" + cmd.UsageString() + "\n",
},
{
name: "unknown Cobra command error",
Expand All @@ -72,17 +84,86 @@ check your internet connection or https://githubstatus.com
cmd: cmd,
debug: false,
},
wantOut: "unknown command foo\n\nUsage:\n\n",
wantOut: "unknown command foo\n\n" + cmd.UsageString() + "\n",
},
{
name: "Cobra flag error with full help",
args: args{
err: cmdutil.FlagErrorf("unknown flag --foo"),
cmd: cmd,
debug: false,
fullHelp: true,
},
wantOut: heredoc.Doc(`
unknown flag --foo

Spend money

USAGE
gh spend [flags]

FLAGS
--amount int How much to spend

EXAMPLES
$ gh spend --amount 1

LEARN MORE
Use ` + "`gh <command> <subcommand> --help`" + ` for more information about a command.
Read the manual at https://cli.github.com/manual
Learn about exit codes using ` + "`gh help exit-codes`" + `
Learn about accessibility experiences using ` + "`gh help accessibility`" + `

`),
},
{
name: "unknown Cobra command error with full help",
args: args{
err: errors.New("unknown command foo"),
cmd: cmd,
debug: false,
fullHelp: true,
},
wantOut: heredoc.Doc(`
unknown command foo

Spend money

USAGE
gh spend [flags]

FLAGS
--amount int How much to spend

EXAMPLES
$ gh spend --amount 1

LEARN MORE
Use ` + "`gh <command> <subcommand> --help`" + ` for more information about a command.
Read the manual at https://cli.github.com/manual
Learn about exit codes using ` + "`gh help exit-codes`" + `
Learn about accessibility experiences using ` + "`gh help accessibility`" + `

`),
},
{
name: "generic error is unaffected by full help",
args: args{
err: errors.New("the app exploded"),
cmd: cmd,
debug: false,
fullHelp: true,
},
wantOut: "the app exploded\n",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
out := &bytes.Buffer{}
printError(out, tt.args.err, tt.args.cmd, tt.args.debug)
if gotOut := out.String(); gotOut != tt.wantOut {
t.Errorf("printError() = %q, want %q", gotOut, tt.wantOut)
}
printError(out, ios.ColorScheme(), tt.args.err, tt.args.cmd, tt.args.debug, tt.args.fullHelp)
assert.Equal(t, tt.wantOut, out.String())
})
}
}
Expand Down
18 changes: 13 additions & 5 deletions pkg/cmd/root/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)
Expand Down Expand Up @@ -109,6 +110,14 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) {
return
}

WriteHelp(f.IOStreams.Out, cs, command)
}

// WriteHelp renders the full help text for command to w. This is the same
// output produced by `gh <command> --help`, exposed separately so that callers
// such as error reporting can render help to a stream of their choosing rather
// than always writing to stdout.
func WriteHelp(w io.Writer, cs *iostreams.ColorScheme, command *cobra.Command) {
type helpEntry struct {
Title string
Body string
Expand Down Expand Up @@ -193,17 +202,16 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) {
Learn about accessibility experiences using %[1]sgh help accessibility%[1]s
`, "`")})

out := f.IOStreams.Out
for _, e := range helpEntries {
if e.Title != "" {
// If there is a title, add indentation to each line in the body
fmt.Fprintln(out, cs.Bold(e.Title))
fmt.Fprintln(out, text.Indent(strings.Trim(e.Body, "\r\n"), " "))
fmt.Fprintln(w, cs.Bold(e.Title))
fmt.Fprintln(w, text.Indent(strings.Trim(e.Body, "\r\n"), " "))
} else {
// If there is no title print the body as is
fmt.Fprintln(out, e.Body)
fmt.Fprintln(w, e.Body)
}
fmt.Fprintln(out)
fmt.Fprintln(w)
}
}

Expand Down