From bb683d70faa6ceddf9e13667fce0ffc96df889d6 Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 16:00:07 -0700 Subject: [PATCH 1/7] Add inline image generation picker --- cmd/openai/main_help_examples_test.go | 16 + docs/image-generation-saving.md | 12 + pkg/custom/image_picker.go | 599 ++++++++++++ pkg/custom/image_picker_command.go | 75 ++ pkg/custom/image_picker_generate.go | 125 +++ pkg/custom/image_picker_generate_test.go | 52 ++ pkg/custom/image_picker_inline.go | 216 +++++ pkg/custom/image_picker_parent_shell.go | 32 + .../image_picker_parent_shell_darwin.go | 15 + pkg/custom/image_picker_parent_shell_linux.go | 10 + pkg/custom/image_picker_parent_shell_other.go | 9 + pkg/custom/image_picker_parent_shell_test.go | 20 + .../image_picker_parent_shell_windows.go | 21 + pkg/custom/image_picker_repeat_test.go | 102 ++ pkg/custom/image_picker_session.go | 39 + pkg/custom/image_picker_session_test.go | 30 + pkg/custom/image_picker_shell_quote.go | 67 ++ pkg/custom/image_picker_shell_quote_test.go | 60 ++ pkg/custom/image_picker_test.go | 869 ++++++++++++++++++ pkg/custom/image_picker_view.go | 276 ++++++ pkg/custom/image_saving_commands.go | 3 + pkg/custom/save_generated_images.go | 7 +- scripts/check-image-picker-integration.py | 315 +++++++ scripts/image_picker_harness.py | 196 ++++ 24 files changed, 3164 insertions(+), 2 deletions(-) create mode 100644 pkg/custom/image_picker.go create mode 100644 pkg/custom/image_picker_command.go create mode 100644 pkg/custom/image_picker_generate.go create mode 100644 pkg/custom/image_picker_generate_test.go create mode 100644 pkg/custom/image_picker_inline.go create mode 100644 pkg/custom/image_picker_parent_shell.go create mode 100644 pkg/custom/image_picker_parent_shell_darwin.go create mode 100644 pkg/custom/image_picker_parent_shell_linux.go create mode 100644 pkg/custom/image_picker_parent_shell_other.go create mode 100644 pkg/custom/image_picker_parent_shell_test.go create mode 100644 pkg/custom/image_picker_parent_shell_windows.go create mode 100644 pkg/custom/image_picker_repeat_test.go create mode 100644 pkg/custom/image_picker_session.go create mode 100644 pkg/custom/image_picker_session_test.go create mode 100644 pkg/custom/image_picker_shell_quote.go create mode 100644 pkg/custom/image_picker_shell_quote_test.go create mode 100644 pkg/custom/image_picker_test.go create mode 100644 pkg/custom/image_picker_view.go create mode 100644 scripts/check-image-picker-integration.py create mode 100644 scripts/image_picker_harness.py diff --git a/cmd/openai/main_help_examples_test.go b/cmd/openai/main_help_examples_test.go index 67f7a03a..03893a35 100644 --- a/cmd/openai/main_help_examples_test.go +++ b/cmd/openai/main_help_examples_test.go @@ -143,3 +143,19 @@ func TestMainVariationHelpRetirementGuidance(t *testing.T) { }) } } + +func TestMainHelpImagePicker(t *testing.T) { + for _, args := range [][]string{{"openai", "images", "generate", "--help"}, {"openai", "help", "--all", "images", "generate"}} { + got := runMainDispatch(t, "bash", args...) + if got.code != 0 || got.stderr != "" { + t.Fatalf("picker help failed: %+v", got) + } + text := strings.Join(strings.Fields(got.stdout), " ") + if !strings.Contains(text, "Run without flags to choose settings") || !strings.Contains(text, "Ctrl+C exits.") { + t.Fatalf("missing picker guidance: %s", text) + } + if strings.Contains(got.stdout, "\x1b") { + t.Fatalf("help should stay plain: %q", got.stdout) + } + } +} diff --git a/docs/image-generation-saving.md b/docs/image-generation-saving.md index dcc57780..559b1934 100644 --- a/docs/image-generation-saving.md +++ b/docs/image-generation-saving.md @@ -9,6 +9,18 @@ openai images generate --prompt "A tiny orange robot" --output-format webp openai --format json images generate --prompt "A tiny orange robot" --model gpt-image-2.5-sunburst ``` +Run `openai images generate` without flags in a terminal to choose a prompt, +model, size, quality, background, file type and image count. The picker starts +with the CLI's default model, a 1024 × 1024 image, automatic quality and +background, PNG and one image. Enter generates from the prompt; use the arrow +keys or Tab to move through settings. Ctrl+P prints the command without +requesting an image, and Ctrl+C exits. + +After a successful generation, the picker reopens below the saved result with +the active prompt and settings. Choices last until this invocation exits. +Explicit image flags, output formats, piped input and redirected output retain +the direct command behavior described below. + Generation uses API credits and requires access to the chosen model. The CLI's default for saving is the exact ID `gpt-image-2.5-sunburst`. This is a preset, not a guarantee of access or quota. When neither `model` nor `response_format` diff --git a/pkg/custom/image_picker.go b/pkg/custom/image_picker.go new file mode 100644 index 00000000..0cdb9dbb --- /dev/null +++ b/pkg/custom/image_picker.go @@ -0,0 +1,599 @@ +package custom + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "os/signal" + "strings" + "sync" + "syscall" + "time" + "unicode" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/colorprofile" + uv "github.com/charmbracelet/ultraviolet" + "github.com/charmbracelet/x/term" + "github.com/openai/openai-go/v3" +) + +// imagePickerOptions configures the interactive image settings. +type imagePickerOptions struct { + Prompt string + // PromptSet lets an explicit empty prompt override the active draft. + PromptSet bool + // Shell selects quoting for the displayed command, never execution. + Shell string + initial *imagePickerSettings + resuming bool +} + +// imagePickerResult contains arguments for the existing images generate command. +// The picker itself never makes API requests or writes image files. +type imagePickerResult struct { + Args []string + Canceled bool + PrintOnly bool + // ExitCode preserves external SIGINT/SIGTERM status after terminal cleanup. + ExitCode int + settings imagePickerSettings + shell string +} + +func runImagePicker(parent context.Context, input, output *os.File, options imagePickerOptions) (result imagePickerResult, err error) { + if err := parent.Err(); err != nil { + return imagePickerResult{}, err + } + model, err := newImagePicker(options) + if err != nil { + return imagePickerResult{}, err + } + if input == nil || output == nil || !term.IsTerminal(input.Fd()) || !term.IsTerminal(output.Fd()) { + return imagePickerResult{}, errors.New("the image picker needs terminal input and output") + } + if os.Getenv("TERM") == "dumb" { + return imagePickerResult{}, errors.New("the image picker needs a terminal with cursor support") + } + width, height, err := term.GetSize(output.Fd()) + if err != nil { + return imagePickerResult{}, errors.New("could not read the terminal size") + } + model.width, model.height = width, height + // WithoutRenderer also disables Tea's terminal initialization. Own the input + // state explicitly and restore it after the painter has closed its modes. + inputState, err := term.GetState(input.Fd()) + if err != nil { + return imagePickerResult{}, errors.New("could not read terminal input state") + } + outputState, err := term.GetState(output.Fd()) + if err != nil { + return imagePickerResult{}, errors.New("could not read terminal output state") + } + // Capture both states before setup: a platform setup can change input and + // then fail while enabling output. Restore both even after partial failure. + defer func() { + err = errors.Join(err, term.Restore(input.Fd(), inputState), term.Restore(output.Fd(), outputState)) + }() + console := uv.NewConsole(input, output, os.Environ()) + _, err = console.MakeRaw() + if err != nil { + return imagePickerResult{}, fmt.Errorf("could not enable terminal input: %w", err) + } + // Parent cancellation goes through the model so the inline painter clears + // its final frame before closing. Write failures stop the program at once. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + tracked := &imagePickerOutput{File: output, cancel: cancel} + inline := &imagePickerInline{model: model, output: tracked, profile: colorprofile.Detect(output, os.Environ()), resuming: options.resuming} + programOptions := []tea.ProgramOption{tea.WithInput(input), tea.WithOutput(tracked), tea.WithContext(ctx), tea.WithWindowSize(width, height), tea.WithoutSignalHandler(), tea.WithoutRenderer()} + if os.Getenv("NO_COLOR") != "" { + model.color = false + programOptions = append(programOptions, tea.WithColorProfile(colorprofile.NoTTY)) + } + program := tea.NewProgram(inline, programOptions...) + // Route OS signals through the model so cleanup and exit status match + // keyboard cancellation, without treating a write failure as cancellation. + signals := make(chan os.Signal, 1) + signal.Notify(signals, os.Interrupt, syscall.SIGTERM) + listenerDone := make(chan struct{}) + defer func() { + signal.Stop(signals) + cancel() + <-listenerDone + }() + go func() { + defer close(listenerDone) + // Tea's resize handler is inactive without its renderer. Polling also + // catches resizes on terminals that do not deliver SIGWINCH. + resize := time.NewTicker(100 * time.Millisecond) + defer resize.Stop() + for { + select { + case received := <-signals: + code := 130 + if received == syscall.SIGTERM { + code = 143 + } + program.Send(imagePickerStopMsg{code: code}) + return + case <-ctx.Done(): + return + case <-parent.Done(): + program.Send(imagePickerStopMsg{code: 130}) + return + case <-resize.C: + w, h, err := term.GetSize(output.Fd()) + if err != nil { + program.Send(imagePickerSizeErrorMsg{}) + return + } + if w != width || h != height { + width, height = w, h + program.Send(tea.WindowSizeMsg{Width: w, Height: h}) + } + } + } + }() + _, runErr := program.Run() + inline.close() + return model.result, errors.Join(runErr, tracked.Err(), inline.err, parent.Err()) +} + +type imagePickerStopMsg struct{ code int } + +// Stop on the first failed terminal write and retain that failure while the +// input loop exits and the painter attempts terminal cleanup. +type imagePickerOutput struct { + *os.File + mu sync.Mutex + err error + cancel context.CancelFunc +} + +// io.WriteString must use the same error tracking as Write, rather than the +// embedded file's method, so failed repaints stop the input loop too. +func (w *imagePickerOutput) WriteString(s string) (int, error) { + return w.Write([]byte(s)) +} + +func (w *imagePickerOutput) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + n, err := w.File.Write(p) + if err == nil && n != len(p) { + err = io.ErrShortWrite + } + if w.err == nil && err != nil { + w.err = err + w.cancel() + } + return n, err +} + +func (w *imagePickerOutput) Err() error { + w.mu.Lock() + defer w.mu.Unlock() + return w.err +} + +type imagePickerSettings struct { + prompt, model, size, quality, background, format, count string +} + +type imagePicker struct { + settings imagePickerSettings + page, field string + shell string + focus string + returnPage string + returnSelected int + draft []rune + cursor int + selected int + commandOffset int + width, height int + color bool + dark bool + note string + result imagePickerResult +} + +type imagePickerRow struct { + id, label, value, detail string +} + +func newImagePicker(options imagePickerOptions) (*imagePicker, error) { + m := &imagePicker{ + settings: imagePickerSettings{prompt: options.Prompt, model: defaultSavedImageModel, size: "1024x1024", quality: "auto", background: "auto", format: "png", count: "1"}, + page: "settings", focus: "prompt", color: true, dark: true, + draft: []rune(options.Prompt), cursor: len([]rune(options.Prompt)), + } + if options.initial != nil { + m.settings = *options.initial + if options.PromptSet || options.Prompt != "" { + m.settings.prompt = options.Prompt + } + m.draft, m.cursor = []rune(m.settings.prompt), len([]rune(m.settings.prompt)) + } + m.shell = options.Shell + return m, nil +} + +func (m *imagePicker) Init() tea.Cmd { + // The inline wrapper owns terminal initialization. + return nil +} + +func (m *imagePicker) Update(message tea.Msg) (tea.Model, tea.Cmd) { + if m.result.Canceled { + return m, nil + } + // Cancellation can still stop a submitted action before the picker exits. + switch msg := message.(type) { + case imagePickerStopMsg: + m.result = imagePickerResult{Canceled: true, ExitCode: msg.code} + return m, tea.Quit + case tea.KeyPressMsg: + if msg.String() == "ctrl+c" { + m.result = imagePickerResult{Canceled: true} + return m, tea.Quit + } + } + // Ignore queued keys and paste after submission; run the captured command once. + if len(m.result.Args) != 0 { + return m, nil + } + switch msg := message.(type) { + + case tea.BackgroundColorMsg: + m.dark = msg.IsDark() + case tea.WindowSizeMsg: + m.width, m.height = msg.Width, msg.Height + m.clampCommandOffset() + case tea.PasteMsg: + if m.focus == "prompt" { + m.insertPrompt(msg.Content) + } else { + m.note = "Tab to Prompt to paste text." + } + case tea.KeyPressMsg: + key := msg.String() + if key == "esc" { + m.focusPrompt() + return m, nil + } + // Esc immediately followed by another key can arrive as an Alt key. + // Leave menus first; no Alt shortcuts are assigned + // there. In the prompt itself, Alt+Enter still inserts a newline. + if msg.Mod&tea.ModAlt != 0 { + printable := (msg.Mod == tea.ModAlt || msg.Mod == tea.ModAlt|tea.ModShift) && unicode.IsPrint(msg.Code) + if m.focus != "prompt" || printable { + m.focusPrompt() + if printable { + letter := msg.Code + if msg.Mod&tea.ModShift != 0 { + letter = unicode.ToUpper(letter) + } + m.insertPrompt(string(letter)) + } + return m, nil + } + } + if key == "q" && m.focus != "prompt" { + m.result = imagePickerResult{Canceled: true} + return m, tea.Quit + } + // At very small sizes, only returning to the prompt or quitting is safe. + if m.width < 40 || m.height < 12 { + return m, nil + } + switch key { + case "ctrl+g": + return m, m.activate(imagePickerRow{id: "generate"}) + case "ctrl+p": + return m, m.activate(imagePickerRow{id: "print"}) + case "tab", "shift+tab": + m.cycleFocus(key == "shift+tab") + return m, nil + } + if m.focus == "prompt" { + switch key { + case "enter": + return m, m.activate(imagePickerRow{id: "generate"}) + case "down": + m.focus, m.selected = "options", 0 + default: + m.editPrompt(msg) + } + return m, nil + } + if m.focus == "command" { + if key == "up" && m.commandOffset == 0 { + m.focus, m.selected = "options", len(m.rows())-1 + return m, nil + } + if m.scrollCommand(key) { + return m, nil + } + switch key { + case "enter": + return m, m.activate(imagePickerRow{id: "generate"}) + } + return m, nil + } + rows := m.rows() + switch key { + case "up": + if m.selected == 0 { + m.focus = "prompt" + } else { + m.selected-- + } + case "down": + if m.selected == len(rows)-1 { + m.focus, m.commandOffset = "command", 0 + } else { + m.selected++ + } + case "home": + m.selected = 0 + case "end": + m.selected = len(rows) - 1 + case "left": + return m, m.back() + case "enter": + return m, m.activate(rows[m.selected]) + } + } + return m, nil +} + +// Esc closes any menu without applying its highlighted choice. Committed +// settings and prompt text remain intact. +func (m *imagePicker) focusPrompt() { + m.page, m.focus, m.selected, m.note = "settings", "prompt", 0, "" +} + +func (m *imagePicker) rows() []imagePickerRow { + s := m.settings + switch m.page { + case "choose": + return m.choices(m.field) + case "more": + return []imagePickerRow{{id: "background", label: "Background", value: s.background}, {id: "format", label: "File type", value: strings.ToUpper(s.format)}, {id: "count", label: "Images", value: s.count}, {id: "back", label: "Back to settings"}} + } + rows := []imagePickerRow{ + {id: "model", label: "Model", value: s.model}, + {id: "size", label: "Size", value: imagePickerSizeLabel(s.size)}, + {id: "quality", label: "Quality", value: imagePickerTitle(s.quality)}, + } + return append(rows, imagePickerRow{id: "more", label: "More options", value: strings.ToUpper(s.format) + " / " + s.count + " image(s)"}) +} + +func (m *imagePicker) cycleFocus(backwards bool) { + focuses := []string{"prompt", "options", "command"} + for i, focus := range focuses { + if focus == m.focus { + delta := 1 + if backwards { + delta = -1 + } + m.focus = focuses[(i+len(focuses)+delta)%len(focuses)] + return + } + } +} + +func (m *imagePicker) choices(field string) []imagePickerRow { + var values []string + switch field { + case "model": + values = []string{openai.ImageModelGPTImage2_5Sunburst, openai.ImageModelGPTImage2_5Flare, openai.ImageModelGPTImage2, openai.ImageModelGPTImage1_5, openai.ImageModelGPTImage1Mini} + case "size": + values = []string{"auto", "1024x1024", "1536x1024", "1024x1536"} + case "quality": + values = []string{"auto", "low", "medium", "high"} + if m.settings.model == openai.ImageModelGPTImage2_5Sunburst || m.settings.model == openai.ImageModelGPTImage2_5Flare { + values = append(values, "xhigh", "max") + } + case "background": + values = []string{"auto", "opaque", "transparent"} + case "format": + values = []string{"png", "jpeg", "webp"} + case "count": + for n := 1; n <= 10; n++ { + values = append(values, fmt.Sprint(n)) + } + } + rows := make([]imagePickerRow, 0, len(values)) + for _, value := range values { + row := imagePickerRow{id: "choice", value: value, label: imagePickerTitle(value)} + switch field { + case "model": + row.label = value + if value == openai.ImageModelGPTImage2_5Sunburst { + row.detail = "CLI default" + } + case "size": + row.label = imagePickerSizeLabel(value) + case "format": + row.label = strings.ToUpper(value) + if value == "jpeg" { + row.detail = "No transparency" + } + } + if value == "auto" { + row.detail = "Model chooses" + } + rows = append(rows, row) + } + return rows +} + +func (m *imagePicker) value(field string) string { + s := m.settings + switch field { + case "model": + return s.model + case "size": + return s.size + case "quality": + return s.quality + case "background": + return s.background + case "format": + return s.format + case "count": + return s.count + } + return "" +} + +func (m *imagePicker) choose(field string) { + m.page, m.field = "choose", field + m.selectCurrent(field) +} + +func (m *imagePicker) apply(value string) { + if m.value(m.field) != value { + m.commandOffset = 0 + } + m.note = "" + s := &m.settings + switch m.field { + case "model": + s.model = value + if (s.quality == "xhigh" || s.quality == "max") && value != openai.ImageModelGPTImage2_5Sunburst && value != openai.ImageModelGPTImage2_5Flare { + s.quality = "auto" + m.note = "Quality changed to Auto for this model." + } + case "size": + s.size = value + case "quality": + s.quality = value + case "background": + s.background = value + if value == "transparent" && s.format == "jpeg" { + s.format = "png" + m.note = "File type changed to PNG to keep transparency." + } + case "format": + s.format = value + if value == "jpeg" && s.background == "transparent" { + s.background = "opaque" + m.note = "Background changed to Opaque for JPEG." + } + case "count": + s.count = value + } +} + +func (m *imagePicker) activate(row imagePickerRow) tea.Cmd { + switch row.id { + case "choice": + m.apply(row.value) + m.page, m.selected = m.returnPage, m.returnSelected + case "model", "size", "quality", "background", "format", "count": + m.returnPage, m.returnSelected = m.page, m.selected + m.choose(row.id) + + case "more": + m.page, m.selected = "more", 0 + case "generate", "print": + if m.validPrompt() { + m.result = imagePickerResult{Args: m.settings.args(), PrintOnly: row.id == "print", settings: m.settings, shell: m.shell} + return tea.Quit + } + case "back": + return m.back() + } + return nil +} + +func (m *imagePicker) selectCurrent(field string) { + m.selected = 0 + for i, row := range m.choices(field) { + if row.value == m.value(field) { + m.selected = i + } + } +} + +func (m *imagePicker) validPrompt() bool { + switch { + case strings.TrimSpace(m.settings.prompt) == "": + m.note = "Add a prompt first." + case strings.ContainsRune(m.settings.prompt, 0): + m.note = "Remove the NUL character; shell arguments cannot contain it." + case strings.HasPrefix(m.settings.prompt, `\@`): + m.note = `Prompts beginning with \@ cannot be represented by the request parser.` + default: + return true + } + return false +} + +func (m *imagePicker) back() tea.Cmd { + m.note = "" + switch m.page { + case "more": + m.page = "settings" + m.selected = len(m.rows()) - 1 + case "choose": + m.page, m.selected = m.returnPage, m.returnSelected + default: + m.focus = "prompt" + } + return nil +} + +func (m *imagePicker) insertPrompt(text string) { + insert := []rune(text) + tail := append([]rune(nil), m.draft[m.cursor:]...) + m.draft = append(m.draft[:m.cursor], insert...) + m.draft = append(m.draft, tail...) + m.cursor += len(insert) + m.settings.prompt = string(m.draft) + if text != "" { + m.commandOffset = 0 + } + m.note = "" +} + +func (m *imagePicker) editPrompt(key tea.KeyPressMsg) { + previous := m.settings.prompt + switch key.String() { + case "left": + m.cursor = max(0, m.cursor-1) + case "right": + m.cursor = min(len(m.draft), m.cursor+1) + case "home", "ctrl+a": + m.cursor = 0 + case "end", "ctrl+e": + m.cursor = len(m.draft) + case "ctrl+u": + m.draft, m.cursor = m.draft[m.cursor:], 0 + case "backspace": + if m.cursor > 0 { + m.draft = append(m.draft[:m.cursor-1], m.draft[m.cursor:]...) + m.cursor-- + } + case "delete": + if m.cursor < len(m.draft) { + m.draft = append(m.draft[:m.cursor], m.draft[m.cursor+1:]...) + } + case "alt+enter": + m.insertPrompt("\n") + default: + if key.Text != "" { + m.insertPrompt(key.Text) + } + } + m.settings.prompt = string(m.draft) + if m.settings.prompt != previous { + m.commandOffset = 0 + } +} diff --git a/pkg/custom/image_picker_command.go b/pkg/custom/image_picker_command.go new file mode 100644 index 00000000..70b34f25 --- /dev/null +++ b/pkg/custom/image_picker_command.go @@ -0,0 +1,75 @@ +package custom + +import ( + "fmt" + "strings" + "unicode" +) + +func (s imagePickerSettings) args() []string { + prompt := s.prompt + if strings.HasPrefix(prompt, "@") { + // The existing request parser expands @file arguments. User text entered + // into the picker is always literal, including file-looking strings. + prompt = `\` + prompt + } + // Keep settings ahead of potentially long prompt text so live previews show + // setting changes even when the prompt tail needs to be truncated. + args := []string{"images", "generate", "--model", s.model, "--size", s.size, + "--quality", s.quality, "--output-format", s.format, "--background", s.background, "--count", s.count} + return append(args, "--prompt", prompt) +} + +func formatImagePickerCommand(args []string, shell string) string { + words := make([]string, 0, len(args)+1) + words = append(words, "openai") + for _, arg := range args { + quote := imagePickerShellQuote + switch shell { + case "fish": + quote = imagePickerFishQuote + case "pwsh": + quote = imagePickerPowerShellQuote + } + words = append(words, quote(arg)) + } + return strings.Join(words, " ") +} + +func imagePickerShellQuote(value string) string { + plain := value != "" + escaped := false + for _, r := range value { + if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || strings.ContainsRune("_./:-", r)) { + plain = false + } + if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || r == '\u2028' || r == '\u2029' { + escaped = true + } + } + if plain { + return value + } + if !escaped { + return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'" + } + var b strings.Builder + b.WriteString("$'") + for _, r := range value { + switch { + case r == '\\' || r == '\'': + b.WriteByte('\\') + b.WriteRune(r) + case unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || r == '\u2028' || r == '\u2029': + // Encode UTF-8 bytes, making C1 controls exact in both zsh and bash + // regardless of each shell's treatment of Unicode escape syntax. + for _, v := range []byte(string(r)) { + fmt.Fprintf(&b, "\\x%02x", v) + } + default: + b.WriteRune(r) + } + } + b.WriteByte('\'') + return b.String() +} diff --git a/pkg/custom/image_picker_generate.go b/pkg/custom/image_picker_generate.go new file mode 100644 index 00000000..aef852ce --- /dev/null +++ b/pkg/custom/image_picker_generate.go @@ -0,0 +1,125 @@ +package custom + +import ( + "context" + "fmt" + "os" + "slices" + "strings" + + "github.com/charmbracelet/x/term" + "github.com/openai/openai-cli/internal/requestflag" + "github.com/urfave/cli/v3" +) + +// Only a bare interactive command enters the picker. Explicit flags and piped +// bodies retain their existing validation, defaults and output behavior. +func imagePickerWorkflow(next cli.ActionFunc) cli.ActionFunc { + return func(ctx context.Context, command *cli.Command) error { + out, ok := command.Root().Writer.(*os.File) + if !ok || out != os.Stdout || !term.IsTerminal(os.Stdin.Fd()) || !term.IsTerminal(out.Fd()) || + strings.EqualFold(os.Getenv("TERM"), "dumb") || !imagePickerRequested(command) { + return next(ctx, command) + } + options := imagePickerOptions{} + for { + result, err := runImagePickerSession(ctx, os.Stdin, out, options) + if err != nil { + return err + } + if result.Canceled { + code := result.ExitCode + if code == 0 { + code = 130 + } + return cli.Exit("", code) + } + if err := ctx.Err(); err != nil { + return err + } + if result.PrintOnly { + return nil + } + if err := runImagePickerAction(ctx, command, result.Args, next); err != nil { + return err + } + // Reopen below the saved result, retaining this session's choices + // until the user exits. + options.initial, options.resuming = &result.settings, true + } + } +} + +func runImagePickerAction(ctx context.Context, command *cli.Command, args []string, next cli.ActionFunc) error { + for i := 2; i < len(args); i += 2 { + restore, err := setImagePickerFlag(command, strings.TrimPrefix(args[i], "--"), args[i+1]) + if err != nil { + return err + } + defer restore() + } + // Each request uses the existing saving workflow and generated SDK action. + // Restore flags before the next draft, including after failed requests. + return next(ctx, command) +} + +// Isolate selections from the shared command tree, including after a failed +// action. A struct copy alone would still share the flag's parsed value. +func setImagePickerFlag(command *cli.Command, name, value string) (func(), error) { + for _, candidate := range command.Flags { + if !slices.Contains(candidate.Names(), name) { + continue + } + switch flag := candidate.(type) { + case *requestflag.Flag[string]: + return setTemporaryImagePickerFlag(flag, name, value) + case *requestflag.Flag[*string]: + return setTemporaryImagePickerFlag(flag, name, value) + case *requestflag.Flag[*int64]: + return setTemporaryImagePickerFlag(flag, name, value) + case *cli.StringFlag: + original, replacement := *flag, *flag + // Do not mutate an existing parsed value or external destination. + replacement.Destination = nil + if err := replacement.PreParse(); err != nil { + return nil, err + } + if err := replacement.Set(name, value); err != nil { + return nil, err + } + *flag = replacement + return func() { *flag = original }, nil + } + break + } + return nil, fmt.Errorf("unsupported image picker setting: %s", name) +} + +func setTemporaryImagePickerFlag[T string | *string | *int64](flag *requestflag.Flag[T], name, value string) (func(), error) { + original, replacement := *flag, *flag + if err := replacement.PreParse(); err != nil { + return nil, err + } + if err := replacement.Set(name, value); err != nil { + return nil, err + } + *flag = replacement + return func() { *flag = original }, nil +} + +func imagePickerRequested(command *cli.Command) bool { + if command.Args().Len() != 0 { + return false + } + for _, flag := range command.Flags { + if flag.IsSet() { + return false + } + } + for _, name := range []string{"format", "format-error", "transform", "transform-error", "raw-output"} { + if command.Root().IsSet(name) { + return false + } + } + return true +} diff --git a/pkg/custom/image_picker_generate_test.go b/pkg/custom/image_picker_generate_test.go new file mode 100644 index 00000000..152fb4bf --- /dev/null +++ b/pkg/custom/image_picker_generate_test.go @@ -0,0 +1,52 @@ +package custom + +import ( + "context" + "errors" + "testing" + + "github.com/openai/openai-cli/internal/requestflag" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +func TestImagePickerSelectionsRestoreRequestFlagState(t *testing.T) { + app := &cli.Command{Name: "generate", Flags: []cli.Flag{ + &requestflag.Flag[string]{Name: "prompt", BodyPath: "prompt"}, + &requestflag.Flag[*string]{Name: "model", BodyPath: "model"}, + &requestflag.Flag[*int64]{Name: "n", Aliases: []string{"count"}, BodyPath: "n", Default: requestflag.Ptr[int64](1)}, + }} + app.Action = func(ctx context.Context, command *cli.Command) error { + before := requestflag.ExtractRequestContents(command) + for range 2 { + func() { + for _, pair := range [][2]string{{"prompt", "a blue cat"}, {"model", "gpt-image-2"}, {"count", "2"}} { + restore, err := setImagePickerFlag(command, pair[0], pair[1]) + require.NoError(t, err) + defer restore() + } + require.Equal(t, map[string]any{"prompt": "a blue cat", "model": requestflag.Ptr("gpt-image-2"), "n": requestflag.Ptr[int64](2)}, requestflag.ExtractRequestContents(command).Body) + }() + require.Equal(t, before, requestflag.ExtractRequestContents(command), "a later action must not inherit picker selections") + require.False(t, command.IsSet("model")) + require.False(t, command.IsSet("n")) + require.True(t, command.IsSet("prompt")) + require.Equal(t, "original", command.String("prompt")) + } + _, err := setImagePickerFlag(command, "count", "invalid") + require.Error(t, err) + require.Equal(t, before, requestflag.ExtractRequestContents(command)) + _, err = setImagePickerFlag(command, "missing", "value") + require.Error(t, err) + return nil + } + require.NoError(t, app.Run(context.Background(), []string{"openai", "--prompt", "original"})) +} + +func TestImagePickerCanceledContextDoesNotOpenTerminal(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + result, err := runImagePicker(ctx, nil, nil, imagePickerOptions{}) + require.True(t, errors.Is(err, context.Canceled)) + require.Empty(t, result.Args) +} diff --git a/pkg/custom/image_picker_inline.go b/pkg/custom/image_picker_inline.go new file mode 100644 index 00000000..01b6d180 --- /dev/null +++ b/pkg/custom/image_picker_inline.go @@ -0,0 +1,216 @@ +package custom + +import ( + "errors" + "io" + "strings" + "time" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/colorprofile" + "github.com/charmbracelet/x/ansi" + "github.com/charmbracelet/x/term" +) + +// imagePickerInline keeps one transient logical line below the shell transcript. +// Hard line breaks let terminal reflow move old UI rows above a saved cursor on +// resize, where clearing them would risk shell history. Soft-wrapping a single +// block keeps the cursor inside that same logical line while its width changes. +// Bubble Tea continues to own key decoding and event ordering; runImagePicker +// owns console setup, restoration and resize notifications. +type imagePickerInline struct { + model *imagePicker + output *imagePickerOutput + profile colorprofile.Profile + started bool + modesActive bool + restoreWrap bool + content string + width, height int + err error + resuming bool + submitAfter time.Time +} + +// Legacy terminal input has no key-release event. A quiet interval absorbs +// queued submit keys and ordinary auto-repeat when reopening after a request. +const imagePickerResumeQuiet = 750 * time.Millisecond + +type imagePickerWrapTimeout struct{} +type imagePickerSizeErrorMsg struct{} + +func (p *imagePickerInline) Init() tea.Cmd { + p.openModes() + query := ansi.RequestModeAutoWrap + if p.model.color { + query += ansi.RequestBackgroundColor + } + _, _ = io.WriteString(p.output, query) + // Older terminals do not report modes. They retain the normal shell wrap + // assumption; terminals that report disabled wrapping get it restored below. + return tea.Tick(150*time.Millisecond, func(time.Time) tea.Msg { return imagePickerWrapTimeout{} }) +} + +func (p *imagePickerInline) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case imagePickerSizeErrorMsg: + p.err = errors.New("could not read the terminal size") + return p, tea.Quit + case tea.ModeReportMsg: + if msg.Mode == ansi.ModeAutoWrap { + if msg.Value == ansi.ModePermanentlyReset { + p.err = errors.New("the image picker needs terminal line wrapping") + return p, tea.Quit + } + p.restoreWrap = msg.Value == ansi.ModeReset + p.start() + } + case imagePickerWrapTimeout: + p.start() + case tea.ColorProfileMsg: + p.profile = msg.Profile + } + if !p.started { + // Ignore input queued during generation before the new draft is visible. + if p.resuming { + switch msg := msg.(type) { + case tea.KeyPressMsg: + if msg.String() != "ctrl+c" { + return p, nil + } + case tea.PasteMsg: + return p, nil + } + } + if key, ok := msg.(tea.KeyPressMsg); ok { + switch key.String() { + case "enter", "ctrl+g", "ctrl+p": + return p, nil + } + } + } + if key, ok := msg.(tea.KeyPressMsg); ok && !p.submitAfter.IsZero() { + switch key.String() { + case "enter", "ctrl+g", "ctrl+p": + now := time.Now() + if now.Before(p.submitAfter) { + p.submitAfter = now.Add(imagePickerResumeQuiet) + return p, nil + } + p.submitAfter = time.Time{} + } + } + _, cmd := p.model.Update(msg) + if p.started { + p.draw() + } + return p, cmd +} + +func (p *imagePickerInline) View() tea.View { return tea.NewView("") } + +func (p *imagePickerInline) start() { + if p.started || p.err != nil { + return + } + p.openModes() + p.started = true + _, _ = io.WriteString(p.output, ansi.SetModeAutoWrap) +} + +func (p *imagePickerInline) openModes() { + if !p.modesActive { + p.modesActive = true + _, _ = io.WriteString(p.output, ansi.ResetModeTextCursorEnable+ansi.SetModeBracketedPaste) + } +} + +func (p *imagePickerInline) draw() { + width, height, err := term.GetSize(p.output.Fd()) + if err != nil { + p.err = errors.New("could not read the terminal size") + p.output.cancel() + return + } + p.model.width, p.model.height = width, height + p.model.clampCommandOffset() + content := p.model.View().Content + var converted strings.Builder + _, _ = io.WriteString(&colorprofile.Writer{Forward: &converted, Profile: p.profile}, content) + content = converted.String() + if content == p.content && width == p.width && height == p.height { + return + } + _, _ = io.WriteString(p.output, imagePickerInlineFrame(content, width)) + p.content, p.width, p.height = content, width, height + if p.resuming { + p.submitAfter = time.Now().Add(imagePickerResumeQuiet) + p.resuming = false + } +} + +func imagePickerInlineFrame(content string, width int) string { + var frame strings.Builder + frame.WriteString("\r" + ansi.EraseScreenBelow) + if content == "" || width <= 0 { + return frame.String() + } + lines := strings.Split(content, "\n") + for _, line := range lines { + // A printable space completes the preceding row's pending wrap. CR + // then returns to column zero without breaking that logical line. + frame.WriteString(" \r") + used, clipped := 0, false + for len(line) > 0 { + sequence, cells, read := imagePickerSequence(line) + line = line[read:] + if cells == 0 { + frame.WriteString(sequence) + } else if !clipped && used+cells < width { + frame.WriteString(sequence) + used += cells + } else { + clipped = true + } + } + // Place the wrap boundary explicitly. Emoji clusters can occupy fewer + // cells than wcwidth reports; spaces alone would leave the cursor on + // the wrong row. The final cell is reserved from content above. + frame.WriteString(ansi.CursorHorizontalAbsolute(width)) + frame.WriteByte(' ') + } + // CR cancels the pending wrap after the final boundary cell. All preceding + // rows were reached by autowrap, so this remains one logical terminal line. + frame.WriteByte('\r') + if len(lines) > 1 { + frame.WriteString(ansi.CursorUp(len(lines) - 1)) + } + return frame.String() +} + +// Count complete printable clusters, including ASCII-led keycap emoji. ANSI's +// sequence decoder alone treats the leading digit as a separate byte. Taking +// the larger width covers both scalar and grapheme-aware terminal presentation. +func imagePickerSequence(text string) (sequence string, cells, read int) { + if text[0] == '\x1b' { + sequence, cells, read, _ = ansi.DecodeSequenceWc(text, 0, nil) + return + } + sequence, cells = ansi.FirstGraphemeCluster(text, ansi.WcWidth) + return sequence, max(cells, ansi.StringWidth(sequence)), len(sequence) +} + +func (p *imagePickerInline) close() { + if !p.modesActive { + return + } + cleanup := ansi.ResetModeBracketedPaste + ansi.SetModeTextCursorEnable + if p.started { + cleanup = "\r" + ansi.EraseScreenBelow + cleanup + } + if p.restoreWrap { + cleanup += ansi.ResetModeAutoWrap + } + _, _ = io.WriteString(p.output, cleanup) + p.started, p.modesActive = false, false +} diff --git a/pkg/custom/image_picker_parent_shell.go b/pkg/custom/image_picker_parent_shell.go new file mode 100644 index 00000000..a283f82d --- /dev/null +++ b/pkg/custom/image_picker_parent_shell.go @@ -0,0 +1,32 @@ +package custom + +import ( + "context" + "path/filepath" + "strings" +) + +// imagePickerParentShell identifies the immediate caller. The login shell in +// SHELL can differ from an interactive subshell, so it is not a fallback here. +func imagePickerParentShell(ctx context.Context) string { + if ctx.Err() != nil { + return "" + } + name, err := imagePickerParentProcessName() + if err != nil || ctx.Err() != nil { + return "" + } + return imagePickerShellName(name) +} + +func imagePickerShellName(name string) string { + name = filepath.Base(strings.ReplaceAll(name, `\`, "/")) + name = strings.TrimPrefix(strings.ToLower(name), "-") + name = strings.TrimSuffix(name, ".exe") + switch name { + case "bash", "zsh", "fish", "pwsh": + return name + default: + return "" + } +} diff --git a/pkg/custom/image_picker_parent_shell_darwin.go b/pkg/custom/image_picker_parent_shell_darwin.go new file mode 100644 index 00000000..6aeec1f2 --- /dev/null +++ b/pkg/custom/image_picker_parent_shell_darwin.go @@ -0,0 +1,15 @@ +package custom + +import ( + "os" + + "golang.org/x/sys/unix" +) + +func imagePickerParentProcessName() (string, error) { + info, err := unix.SysctlKinfoProc("kern.proc.pid", os.Getppid()) + if err != nil { + return "", err + } + return unix.ByteSliceToString(info.Proc.P_comm[:]), nil +} diff --git a/pkg/custom/image_picker_parent_shell_linux.go b/pkg/custom/image_picker_parent_shell_linux.go new file mode 100644 index 00000000..56079dd1 --- /dev/null +++ b/pkg/custom/image_picker_parent_shell_linux.go @@ -0,0 +1,10 @@ +package custom + +import ( + "os" + "strconv" +) + +func imagePickerParentProcessName() (string, error) { + return os.Readlink("/proc/" + strconv.Itoa(os.Getppid()) + "/exe") +} diff --git a/pkg/custom/image_picker_parent_shell_other.go b/pkg/custom/image_picker_parent_shell_other.go new file mode 100644 index 00000000..8d1804bf --- /dev/null +++ b/pkg/custom/image_picker_parent_shell_other.go @@ -0,0 +1,9 @@ +//go:build !darwin && !linux && !windows + +package custom + +import "errors" + +func imagePickerParentProcessName() (string, error) { + return "", errors.New("parent shell detection is unavailable on this platform") +} diff --git a/pkg/custom/image_picker_parent_shell_test.go b/pkg/custom/image_picker_parent_shell_test.go new file mode 100644 index 00000000..acbe57a8 --- /dev/null +++ b/pkg/custom/image_picker_parent_shell_test.go @@ -0,0 +1,20 @@ +package custom + +import ( + "context" + "github.com/stretchr/testify/require" + "testing" +) + +func TestImagePickerParentShellNames(t *testing.T) { + for _, tc := range []struct{ name, want string }{ + {"/opt/homebrew/bin/bash", "bash"}, {"-zsh", "zsh"}, {"fish", "fish"}, + {`C:\Program Files\PowerShell\7\pwsh.exe`, "pwsh"}, + {"/bin/sh", ""}, {"powershell.exe", ""}, {"python3", ""}, {"", ""}, + } { + require.Equal(t, tc.want, imagePickerShellName(tc.name), tc.name) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + require.Empty(t, imagePickerParentShell(ctx)) +} diff --git a/pkg/custom/image_picker_parent_shell_windows.go b/pkg/custom/image_picker_parent_shell_windows.go new file mode 100644 index 00000000..346cd649 --- /dev/null +++ b/pkg/custom/image_picker_parent_shell_windows.go @@ -0,0 +1,21 @@ +package custom + +import ( + "os" + + "golang.org/x/sys/windows" +) + +func imagePickerParentProcessName() (string, error) { + parent, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(os.Getppid())) + if err != nil { + return "", err + } + defer windows.CloseHandle(parent) + var path [32768]uint16 + size := uint32(len(path)) + if err := windows.QueryFullProcessImageName(parent, 0, &path[0], &size); err != nil { + return "", err + } + return windows.UTF16ToString(path[:size]), nil +} diff --git a/pkg/custom/image_picker_repeat_test.go b/pkg/custom/image_picker_repeat_test.go new file mode 100644 index 00000000..f4d29171 --- /dev/null +++ b/pkg/custom/image_picker_repeat_test.go @@ -0,0 +1,102 @@ +package custom + +import ( + "context" + "errors" + "os" + "testing" + "time" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" + "github.com/openai/openai-cli/internal/requestflag" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +func TestImagePickerResumesAtPreviousDraft(t *testing.T) { + original := pickerForTest(t).settings + original.prompt = "Keep this prompt" + m, err := newImagePicker(imagePickerOptions{initial: &original, resuming: true}) + require.NoError(t, err) + m.width, m.height = 80, 24 + require.Equal(t, "settings", m.page) + require.Equal(t, "prompt", m.focus) + require.Equal(t, original, m.settings) + require.Equal(t, original.prompt, string(m.draft)) + require.Contains(t, ansi.Strip(m.View().Content), original.prompt) + require.Contains(t, ansi.Strip(m.View().Content), "Ctrl+C") + require.Empty(t, m.result.Args, "reopening alone must never submit") +} + +func TestImagePickerResumeIgnoresQueuedAndRepeatingSubmit(t *testing.T) { + file, err := os.CreateTemp(t.TempDir(), "terminal") + require.NoError(t, err) + defer file.Close() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + m, err := newImagePicker(imagePickerOptions{Prompt: "previous", resuming: true}) + require.NoError(t, err) + p := &imagePickerInline{model: m, output: &imagePickerOutput{File: file, cancel: cancel}, resuming: true} + for _, msg := range []tea.Msg{ + tea.KeyPressMsg{Code: 'n'}, tea.KeyPressMsg{Code: tea.KeyEnter}, + tea.KeyPressMsg{Code: 'g', Mod: tea.ModCtrl}, tea.PasteMsg{Content: "new\n"}, + } { + _, cmd := p.Update(msg) + require.Nil(t, cmd) + require.Equal(t, "previous", string(m.draft)) + require.Empty(t, m.result.Args) + } + p.start() + for _, key := range []tea.KeyPressMsg{ + {Code: tea.KeyEnter}, {Code: 'g', Mod: tea.ModCtrl}, {Code: 'p', Mod: tea.ModCtrl}, + } { + p.submitAfter = time.Now().Add(time.Minute) + before := time.Now() + _, cmd := p.Update(key) + require.Nil(t, cmd, "blocked keys must not queue delayed submissions") + require.Empty(t, m.result.Args) + require.False(t, p.submitAfter.Before(before.Add(imagePickerResumeQuiet))) + } + // Cancellation remains available even before the first paint. + p.started = false + _, cmd := p.Update(tea.KeyPressMsg{Code: 'c', Mod: tea.ModCtrl}) + require.NotNil(t, cmd) + require.True(t, m.result.Canceled) + require.NoError(t, ctx.Err()) +} + +func TestImagePickerRepeatedActionsRestoreFlagsOnFailure(t *testing.T) { + failure := errors.New("second request failed") + app := &cli.Command{Flags: []cli.Flag{ + &requestflag.Flag[string]{Name: "prompt", BodyPath: "prompt"}, + &cli.StringFlag{Name: "output-dir"}, + }} + app.Action = func(ctx context.Context, command *cli.Command) error { + before := requestflag.ExtractRequestContents(command) + for i, pair := range [][2]string{{"first", "/first folder"}, {"second", "/second folder"}} { + calls := 0 + err := runImagePickerAction(ctx, command, []string{"images", "generate", "--prompt", pair[0], "--output-dir", pair[1]}, func(context.Context, *cli.Command) error { + calls++ + require.Equal(t, pair[0], command.String("prompt")) + require.Equal(t, pair[1], command.String("output-dir")) + if i == 1 { + return failure + } + return nil + }) + if i == 1 { + require.ErrorIs(t, err, failure) + } else { + require.NoError(t, err) + } + require.Equal(t, 1, calls) + require.Equal(t, before, requestflag.ExtractRequestContents(command)) + require.False(t, command.IsSet("prompt")) + require.False(t, command.IsSet("output-dir")) + require.Empty(t, command.String("output-dir")) + } + return nil + } + require.NoError(t, app.Run(context.Background(), []string{"openai"})) +} diff --git a/pkg/custom/image_picker_session.go b/pkg/custom/image_picker_session.go new file mode 100644 index 00000000..13862197 --- /dev/null +++ b/pkg/custom/image_picker_session.go @@ -0,0 +1,39 @@ +package custom + +import ( + "context" + "fmt" + "io" + "os" +) + +// Keep choices within the current invocation. Direct API commands never enter +// this session or acquire picker state. +func runImagePickerSession(ctx context.Context, input, output *os.File, options imagePickerOptions) (imagePickerResult, error) { + if options.Shell == "" { + options.Shell = os.Getenv("OPENAI_PICKER_SHELL") + } + if options.Shell == "" { + options.Shell = imagePickerParentShell(ctx) + } + result, err := runImagePicker(ctx, input, output, options) + if err != nil || result.Canceled { + return result, err + } + return result, finishImagePickerSelection(ctx, output, result) +} + +// Echo the selected command only after restoring the terminal and before +// starting the request. A failed echo must not silently start generation. +func finishImagePickerSelection(ctx context.Context, output io.Writer, result imagePickerResult) error { + if err := ctx.Err(); err != nil { + return err + } + if result.Canceled || len(result.Args) < 4 || (len(result.Args)-2)%2 != 0 { + return fmt.Errorf("no image request was selected") + } + if _, err := fmt.Fprintln(output, formatImagePickerCommand(result.Args, result.shell)); err != nil { + return imageSavingFailure("Could not print the command. No image request was started.", err) + } + return nil +} diff --git a/pkg/custom/image_picker_session_test.go b/pkg/custom/image_picker_session_test.go new file mode 100644 index 00000000..61bbfcb9 --- /dev/null +++ b/pkg/custom/image_picker_session_test.go @@ -0,0 +1,30 @@ +package custom + +import ( + "bytes" + "context" + "errors" + "github.com/stretchr/testify/require" + "io" + "testing" +) + +type failedImagePickerWriter struct{} + +func (failedImagePickerWriter) Write([]byte) (int, error) { return 0, io.ErrClosedPipe } + +func TestImagePickerSelectionEchoFailurePreventsGeneration(t *testing.T) { + m, err := newImagePicker(imagePickerOptions{Prompt: "Synthetic prompt"}) + require.NoError(t, err) + result := imagePickerResult{Args: m.settings.args(), settings: m.settings} + err = finishImagePickerSelection(context.Background(), failedImagePickerWriter{}, result) + require.ErrorIs(t, err, io.ErrClosedPipe) + var output bytes.Buffer + require.NoError(t, finishImagePickerSelection(context.Background(), &output, result)) + require.Equal(t, formatImagePickerCommand(result.Args, "")+"\n", output.String()) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + output.Reset() + require.True(t, errors.Is(finishImagePickerSelection(ctx, &output, result), context.Canceled)) + require.Empty(t, output.String()) +} diff --git a/pkg/custom/image_picker_shell_quote.go b/pkg/custom/image_picker_shell_quote.go new file mode 100644 index 00000000..8c7a1ad4 --- /dev/null +++ b/pkg/custom/image_picker_shell_quote.go @@ -0,0 +1,67 @@ +package custom + +import ( + "fmt" + "strings" + "unicode" +) + +func imagePickerQuoteProperties(value string) (plain, control bool) { + plain = value != "" + for _, r := range value { + if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || strings.ContainsRune("_./:-", r)) { + plain = false + } + control = control || unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || r == '\u2028' || r == '\u2029' + } + return +} + +func imagePickerFishQuote(value string) string { + if plain, _ := imagePickerQuoteProperties(value); plain { + return value + } + // Fish joins adjacent quoted and escaped fragments into one argument. Its + // single quotes escape backslashes and quotes, unlike POSIX single quotes. + var b strings.Builder + b.WriteByte('\'') + for _, r := range value { + switch { + case r == '\\' || r == '\'': + b.WriteByte('\\') + b.WriteRune(r) + case unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || r == '\u2028' || r == '\u2029': + fmt.Fprintf(&b, "'\\U%08x'", r) + default: + b.WriteRune(r) + } + } + b.WriteByte('\'') + return b.String() +} + +func imagePickerPowerShellQuote(value string) string { + plain, control := imagePickerQuoteProperties(value) + if plain { + return value + } + if !control { + return "'" + strings.ReplaceAll(value, "'", "''") + "'" + } + // PowerShell 7 Unicode escapes keep control bytes out of terminal output. + var b strings.Builder + b.WriteByte('"') + for _, r := range value { + switch { + case r == '`' || r == '$' || r == '"': + b.WriteByte('`') + b.WriteRune(r) + case unicode.IsControl(r) || unicode.Is(unicode.Cf, r) || r == '\u2028' || r == '\u2029': + fmt.Fprintf(&b, "`u{%x}", r) + default: + b.WriteRune(r) + } + } + b.WriteByte('"') + return b.String() +} diff --git a/pkg/custom/image_picker_shell_quote_test.go b/pkg/custom/image_picker_shell_quote_test.go new file mode 100644 index 00000000..313ea364 --- /dev/null +++ b/pkg/custom/image_picker_shell_quote_test.go @@ -0,0 +1,60 @@ +package custom + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// Parse the full printed command in each real shell; never invoke an API. +func TestImagePickerNativeShellQuoting(t *testing.T) { + values := []string{"", "ordinary", "two words", "a 'quote' and \"double\"", `C:\folder\name`, + "line one\nline two\tend\r", "雪 🐈", "a\u202eb\u0085c\u200bd", "$(touch NOT_RUN); & | > [x] {x}", + "$HOME `backtick`", "trailing ", "~", "\x1b]52;c;synthetic\x07", "a\u2028b\u2029c", "\\@literal"} + for _, shell := range []string{"bash", "zsh", "fish", "pwsh"} { + t.Run(shell, func(t *testing.T) { + binary, err := exec.LookPath(shell) + if err != nil { + if strings.Contains(","+os.Getenv("OPENAI_CLI_REQUIRE_NATIVE_SHELLS")+",", ","+shell+",") { + t.Fatalf("required shell %s unavailable", shell) + } + t.Skipf("%s is not installed", shell) + } + arguments := append([]string{"images", "generate", "--prompt"}, values...) + command := formatImagePickerCommand(arguments, shell) + require.NotContains(t, command, "\x1b") + require.NotContains(t, command, "\n") + require.NotContains(t, command, "\u202e") + prefix := "openai() { printf '%s\\0' \"$@\"; }\n" + var shellArgs []string + switch shell { + case "bash": + shellArgs = []string{"--noprofile", "--norc", "-c"} + case "zsh": + shellArgs = []string{"-f", "-c"} + case "fish": + shellArgs = []string{"--no-config", "-c"} + prefix = "function openai; printf '%s\\0' $argv; end\n" + case "pwsh": + shellArgs = []string{"-NoLogo", "-NoProfile", "-NonInteractive", "-Command"} + prefix = "function openai { foreach ($v in $args) { [Console]::Out.Write($v); [Console]::Out.Write([char]0) } }\n" + } + dir := t.TempDir() + process := exec.Command(binary, append(shellArgs, prefix+command)...) + process.Dir = dir + process.Env = append(os.Environ(), "HOME="+dir, "XDG_CONFIG_HOME="+dir, "OPENAI_API_KEY=synthetic-not-used", "OPENAI_BASE_URL=http://127.0.0.1:9") + var stdout, stderr bytes.Buffer + process.Stdout, process.Stderr = &stdout, &stderr + require.NoError(t, process.Run(), stderr.String()) + require.Empty(t, stderr.String()) + require.Equal(t, strings.Join(arguments, "\x00")+"\x00", stdout.String(), command) + _, err = os.Stat(filepath.Join(dir, "NOT_RUN")) + require.ErrorIs(t, err, os.ErrNotExist) + }) + } +} diff --git a/pkg/custom/image_picker_test.go b/pkg/custom/image_picker_test.go new file mode 100644 index 00000000..9f022c3c --- /dev/null +++ b/pkg/custom/image_picker_test.go @@ -0,0 +1,869 @@ +package custom + +import ( + "context" + "errors" + "io" + "os" + "os/exec" + "runtime" + "strconv" + "strings" + "testing" + "unicode" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" + "github.com/openai/openai-go/v3" + "github.com/stretchr/testify/require" +) + +func pickerForTest(t *testing.T) *imagePicker { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + m, err := newImagePicker(imagePickerOptions{Prompt: "A tiny orange robot watering a plant"}) + require.NoError(t, err) + m.width, m.height, m.color = 90, 24, false + return m +} + +func pickerKey(m *imagePicker, code rune) tea.Cmd { + _, cmd := m.Update(tea.KeyPressMsg{Code: code}) + return cmd +} + +func pickerArg(t *testing.T, args []string, flag string) string { + t.Helper() + for i, arg := range args { + if arg == flag && i+1 < len(args) { + return args[i+1] + } + } + t.Fatalf("missing flag %s", flag) + return "" +} + +func TestImagePickerPromptSubmitsExactlyOnce(t *testing.T) { + m := pickerForTest(t) + require.Equal(t, "prompt", m.focus) + cmd := pickerKey(m, tea.KeyEnter) + require.NotNil(t, cmd) + require.IsType(t, tea.QuitMsg{}, cmd()) + require.False(t, m.result.PrintOnly) + require.False(t, m.result.Canceled) + require.Equal(t, []string{"images", "generate"}, m.result.Args[:2]) + result := m.result + result.Args = append([]string(nil), m.result.Args...) + settings := m.settings + for range 12 { + require.Nil(t, pickerKey(m, tea.KeyEnter), "queued Enter must not resubmit") + } + for _, msg := range []tea.Msg{ + tea.KeyPressMsg{Code: tea.KeyEscape}, + tea.KeyPressMsg{Code: 'p', Mod: tea.ModCtrl}, + tea.KeyPressMsg{Code: 'g', Mod: tea.ModCtrl}, + tea.KeyPressMsg{Code: 'x', Text: "x"}, + tea.PasteMsg{Content: "queued text\n"}, + } { + _, cmd := m.Update(msg) + require.Nil(t, cmd) + require.Equal(t, result, m.result, "queued input must preserve the submitted action") + require.Equal(t, settings, m.settings) + } +} + +func TestImagePickerCancellationCanStopPendingSubmission(t *testing.T) { + for _, cancel := range []tea.Msg{tea.KeyPressMsg{Code: 'c', Mod: tea.ModCtrl}, imagePickerStopMsg{code: 143}} { + m := pickerForTest(t) + require.NotNil(t, pickerKey(m, tea.KeyEnter)) + require.NotEmpty(t, m.result.Args) + _, cmd := m.Update(cancel) + require.NotNil(t, cmd) + require.True(t, m.result.Canceled) + require.Empty(t, m.result.Args) + result, settings := m.result, m.settings + require.Nil(t, pickerKey(m, tea.KeyEnter)) + m.Update(tea.PasteMsg{Content: "after cancellation"}) + require.Equal(t, result, m.result) + require.Equal(t, settings, m.settings) + } +} + +func TestImagePickerSettingsConstraints(t *testing.T) { + m := pickerForTest(t) + require.Len(t, m.choices("quality"), 6) + m.settings.quality = "max" + m.field = "model" + m.apply(openai.ImageModelGPTImage1Mini) + require.Equal(t, "auto", m.settings.quality) + require.Len(t, m.choices("quality"), 4) + require.Contains(t, m.note, "Quality changed") + m.settings.format = "jpeg" + m.field = "background" + m.apply("transparent") + require.Equal(t, "png", m.settings.format) + require.Contains(t, m.note, "transparency") + m.field = "format" + m.apply("jpeg") + require.Equal(t, "opaque", m.settings.background) + require.Contains(t, m.note, "JPEG") + m.field = "count" + require.Len(t, m.choices("count"), 10) + m.apply("10") + require.Equal(t, "10", m.settings.count) +} + +func TestImagePickerLivePromptAndCommandUpdate(t *testing.T) { + m := pickerForTest(t) + m.Update(tea.KeyPressMsg{Code: 'u', Mod: tea.ModCtrl}) + m.Update(tea.PasteMsg{Content: "A 'purple' robot"}) + require.Equal(t, "A 'purple' robot", m.settings.prompt) + require.Equal(t, "prompt", m.focus) + preview, _, _ := m.commandPreview(1000, 1) + require.Contains(t, preview[0], "--prompt 'A '\\''purple'\\'' robot'") + m.field = "quality" + m.apply("max") + preview, _, _ = m.commandPreview(1000, 1) + require.Contains(t, preview[0], "--quality max") + m.field = "model" + m.apply(openai.ImageModelGPTImage1Mini) + preview, _, _ = m.commandPreview(1000, 1) + require.Contains(t, preview[0], "--quality auto") + require.Contains(t, preview[0], "--model gpt-image-1-mini") +} + +func TestImagePickerSubmissionFromEveryFocus(t *testing.T) { + for _, focus := range []string{"prompt", "command"} { + m := pickerForTest(t) + m.focus = focus + cmd := pickerKey(m, tea.KeyEnter) + require.NotNil(t, cmd) + require.IsType(t, tea.QuitMsg{}, cmd()) + require.Equal(t, m.settings.args(), m.result.Args) + require.False(t, m.result.PrintOnly) + } +} + +func TestImagePickerFocusCycleAndPrint(t *testing.T) { + m := pickerForTest(t) + for _, focus := range []string{"options", "command", "prompt"} { + pickerKey(m, tea.KeyTab) + require.Equal(t, focus, m.focus) + } + m.Update(tea.KeyPressMsg{Code: tea.KeyTab, Mod: tea.ModShift}) + require.Equal(t, "command", m.focus) + _, cmd := m.Update(tea.KeyPressMsg{Code: 'p', Mod: tea.ModCtrl}) + require.NotNil(t, cmd) + require.True(t, m.result.PrintOnly) + require.Equal(t, m.settings.prompt, pickerArg(t, m.result.Args, "--prompt")) +} + +func TestImagePickerArrowsReturnToPromptWithoutClosingList(t *testing.T) { + for _, page := range []string{"settings", "choose", "more"} { + m := pickerForTest(t) + m.page, m.field, m.focus = page, "model", "options" + m.settings.model = openai.ImageModelGPTImage2_5Flare + m.selected = 1 // Highlight a different row before returning to Prompt. + before := m.settings + pickerKey(m, tea.KeyUp) + pickerKey(m, tea.KeyUp) + require.Equal(t, "prompt", m.focus) + require.Equal(t, page, m.page, "arrow navigation keeps the visible list") + require.Equal(t, before, m.settings, "highlighting must not commit a value") + pickerKey(m, tea.KeyUp) // No wrapping back to the bottom. + m.Update(tea.KeyPressMsg{Code: 'q', Text: "q"}) + require.Equal(t, before.prompt+"q", m.settings.prompt) + require.False(t, m.result.Canceled) + pickerKey(m, tea.KeyDown) + require.Equal(t, "options", m.focus) + require.Equal(t, page, m.page) + require.Zero(t, m.selected) + require.Empty(t, m.result.Args) + } +} + +func TestImagePickerArrowsFollowScreenOrder(t *testing.T) { + m := pickerForTest(t) + m.insertPrompt(strings.Repeat(" long text", 50)) + pickerKey(m, tea.KeyDown) + pickerKey(m, tea.KeyEnd) + m.commandOffset = 10 // Re-entering from above must start at the first line. + pickerKey(m, tea.KeyDown) + require.Equal(t, "command", m.focus) + require.Zero(t, m.commandOffset) + require.Contains(t, ansi.Strip(m.View().Content), "› openai images generate") + pickerKey(m, tea.KeyDown) + require.Equal(t, 1, m.commandOffset) + pickerKey(m, tea.KeyUp) + require.Equal(t, "command", m.focus) + require.Zero(t, m.commandOffset) + pickerKey(m, tea.KeyUp) + require.Equal(t, "options", m.focus) + require.Equal(t, len(m.rows())-1, m.selected) + for range len(m.rows()) { + pickerKey(m, tea.KeyUp) + } + require.Equal(t, "prompt", m.focus) + require.Empty(t, m.result.Args) +} + +func TestImagePickerPrintShortcutKeepsCommittedChoices(t *testing.T) { + m := pickerForTest(t) + pickerKey(m, tea.KeyDown) + pickerKey(m, tea.KeyEnter) // Open model choices. + pickerKey(m, tea.KeyDown) // Highlight Flare without selecting it. + _, cmd := m.Update(tea.KeyPressMsg{Code: 'p', Mod: tea.ModCtrl}) + require.NotNil(t, cmd) + require.True(t, m.result.PrintOnly) + require.Equal(t, openai.ImageModelGPTImage2_5Sunburst, pickerArg(t, m.result.Args, "--model")) +} + +func TestImagePickerEscapeModelMenuToPrompt(t *testing.T) { + m := pickerForTest(t) + original := m.settings + pickerKey(m, tea.KeyTab) + pickerKey(m, tea.KeyEnter) + pickerKey(m, tea.KeyDown) // Highlight Flare without choosing it. + pickerKey(m, tea.KeyEscape) + require.Equal(t, "settings", m.page) + require.Equal(t, "prompt", m.focus) + require.Equal(t, original, m.settings, "Esc must not apply the highlighted model") + pickerKey(m, tea.KeyEscape) // Repeated Esc must not move focus away again. + m.Update(tea.KeyPressMsg{Code: 'x', Text: "x"}) + require.Equal(t, original.prompt+"x", m.settings.prompt) + require.Empty(t, m.result.Args) + require.False(t, m.result.Canceled) +} + +func TestImagePickerEscapeFromEveryFocusAndSmallWindow(t *testing.T) { + for _, page := range []string{"settings", "choose", "more"} { + for _, focus := range []string{"prompt", "options", "command"} { + m := pickerForTest(t) + m.page, m.focus, m.field = page, focus, "format" + m.settings.quality = "high" + original := m.settings + m.width, m.height = 25, 7 + _, cmd := m.Update(tea.KeyPressMsg{Code: tea.KeyEscape}) + require.Nil(t, cmd) + require.Equal(t, "settings", m.page) + require.Equal(t, "prompt", m.focus) + require.Equal(t, original, m.settings) + require.Empty(t, m.result.Args) + require.False(t, m.result.Canceled) + m.Update(tea.WindowSizeMsg{Width: 80, Height: 24}) + m.Update(tea.KeyPressMsg{Code: 'z', Text: "z"}) + require.Equal(t, original.prompt+"z", m.settings.prompt) + } + } +} + +func TestImagePickerFastEscapeAndTyping(t *testing.T) { + for _, page := range []string{"settings", "choose", "more"} { + for _, mod := range []tea.KeyMod{tea.ModAlt, tea.ModAlt | tea.ModShift} { + m := pickerForTest(t) + m.page, m.focus, m.field = page, "options", "model" + original := m.settings + // Legacy terminal input can combine Esc then a letter into Alt+letter. + m.Update(tea.KeyPressMsg{Code: 'x', Mod: mod}) + letter := "x" + if mod&tea.ModShift != 0 { + letter = "X" + } + require.Equal(t, "prompt", m.focus) + require.Equal(t, "settings", m.page) + require.Equal(t, original.prompt+letter, m.settings.prompt) + require.Equal(t, original.model, m.settings.model) + require.Empty(t, m.result.Args) + } + } +} + +func TestImagePickerFastEscapeWhileAlreadyAtPrompt(t *testing.T) { + m := pickerForTest(t) + original := m.settings.prompt + m.Update(tea.KeyPressMsg{Code: 'q', Mod: tea.ModAlt}) + for _, r := range "uiet" { + m.Update(tea.KeyPressMsg{Code: r, Text: string(r)}) + } + require.Equal(t, original+"quiet", m.settings.prompt) + require.Equal(t, "prompt", m.focus) + require.False(t, m.result.Canceled) + require.Empty(t, m.result.Args) +} + +func TestImagePickerFastEscapeEnterLeavesMenuWithoutSubmitting(t *testing.T) { + for _, key := range []tea.KeyPressMsg{ + {Code: tea.KeyEnter, Mod: tea.ModAlt}, + {Code: tea.KeyEscape, Mod: tea.ModAlt}, + {Code: 'g', Mod: tea.ModAlt | tea.ModCtrl}, + } { + m := pickerForTest(t) + m.page, m.focus, m.field, m.selected = "choose", "options", "model", 1 + _, cmd := m.Update(key) + require.Nil(t, cmd) + require.Equal(t, "prompt", m.focus) + require.Equal(t, "settings", m.page) + require.Empty(t, m.result.Args) + require.Equal(t, openai.ImageModelGPTImage2_5Sunburst, m.settings.model) + } +} + +func TestImagePickerCtrlGFromDropdown(t *testing.T) { + m := pickerForTest(t) + pickerKey(m, tea.KeyTab) + pickerKey(m, tea.KeyEnter) + require.Equal(t, "choose", m.page) + pickerKey(m, tea.KeyDown) + _, cmd := m.Update(tea.KeyPressMsg{Code: 'g', Mod: tea.ModCtrl}) + cmd = cmd + require.NotNil(t, cmd) + require.Equal(t, openai.ImageModelGPTImage2_5Sunburst, m.settings.model, "unconfirmed highlighted option must not apply") + require.Equal(t, openai.ImageModelGPTImage2_5Sunburst, pickerArg(t, m.result.Args, "--model")) + require.False(t, m.result.PrintOnly) +} + +func TestImagePickerMoreOptions(t *testing.T) { + m := pickerForTest(t) + pickerKey(m, tea.KeyTab) + pickerKey(m, tea.KeyEnd) + pickerKey(m, tea.KeyEnter) + require.Equal(t, "more", m.page) + pickerKey(m, tea.KeyEnter) // background + pickerKey(m, tea.KeyEnd) + pickerKey(m, tea.KeyEnter) // transparent + require.Equal(t, "transparent", m.settings.background) + pickerKey(m, tea.KeyDown) + pickerKey(m, tea.KeyEnter) // file type + pickerKey(m, tea.KeyDown) + pickerKey(m, tea.KeyEnter) // JPEG + require.Equal(t, "jpeg", m.settings.format) + require.Equal(t, "opaque", m.settings.background) + pickerKey(m, tea.KeyDown) + pickerKey(m, tea.KeyEnter) // image count + pickerKey(m, tea.KeyEnd) + pickerKey(m, tea.KeyEnter) // ten + require.Equal(t, "10", m.settings.count) + command := formatImagePickerCommandBash(m.settings.args()) + require.Contains(t, command, "--background opaque --count 10") + require.Empty(t, m.result.Args, "selecting options must not submit") + pickerKey(m, tea.KeyTab) + require.NotNil(t, pickerKey(m, tea.KeyEnter)) + require.Equal(t, "10", pickerArg(t, m.result.Args, "--count")) + require.False(t, m.result.PrintOnly) +} + +func TestImagePickerTinyWindowCannotSubmit(t *testing.T) { + for _, focus := range []string{"prompt", "options", "command"} { + m := pickerForTest(t) + m.focus = focus + m.Update(tea.WindowSizeMsg{Width: 25, Height: 7}) + require.Nil(t, pickerKey(m, tea.KeyEnter)) + _, cmd := m.Update(tea.KeyPressMsg{Code: 'g', Mod: tea.ModCtrl}) + require.Nil(t, cmd) + require.Empty(t, m.result.Args) + require.Contains(t, m.View().Content, "Resize") + } +} + +func TestImagePickerEmptyPromptPlaceholder(t *testing.T) { + m, err := newImagePicker(imagePickerOptions{}) + require.NoError(t, err) + m.width, m.height = 80, 24 + require.Contains(t, m.View().Content, "Describe your image") + pickerKey(m, tea.KeyEnter) + require.Equal(t, "settings", m.page) + require.Equal(t, "Add a prompt first.", m.note) + require.Empty(t, m.settings.prompt) +} + +func TestImagePickerBoundedCommandPreviewShowsRange(t *testing.T) { + m := pickerForTest(t) + m.settings.prompt = strings.Repeat("long prompt ", 1000) + lines, start, total := m.commandPreview(40, 2) + require.Greater(t, total, len(lines)) + require.Zero(t, start) + require.Len(t, lines, 2) + require.False(t, strings.HasSuffix(lines[1], "…")) + m.focus = "command" + require.Contains(t, m.View().Content, "↑↓ scroll 1-4/") + require.Contains(t, m.View().Content, "Ctrl+P print") + require.NotContains(t, m.View().Content, "Command") + for _, line := range lines { + require.LessOrEqual(t, ansi.StringWidth(line), 40) + } +} + +func TestImagePickerNavigationHintsFitNarrowWindows(t *testing.T) { + m := pickerForTest(t) + m.settings.prompt = strings.Repeat("long prompt ", 100) + for _, width := range []int{40, 44, 45, 80} { + m.width, m.height = width, 12 + for _, focus := range []string{"prompt", "command"} { + m.focus = focus + lines := strings.Split(ansi.Strip(m.View().Content), "\n") + footer := lines[len(lines)-1] + require.Contains(t, footer, "Ctrl+C exit", "layout=%s width=%d focus=%s", "compact", width, focus) + require.Contains(t, footer, "Enter ") + if focus == "prompt" { + require.Contains(t, footer, "↓ settings") + } else { + require.Contains(t, footer, "↑↓ scroll") + } + require.NotContains(t, footer, "…", "essential controls must fit without truncation") + } + } +} + +func TestImagePickerLongPromptKeepsSettingChangesVisible(t *testing.T) { + m := pickerForTest(t) + m.settings.prompt = strings.Repeat("long prompt ", 1000) + m.field = "quality" + m.apply("max") + m.field = "background" + m.apply("transparent") + lines, _, total := m.commandPreview(80, 4) + require.Greater(t, total, len(lines)) + preview := strings.Join(lines, "") + require.Contains(t, preview, "--quality max") + require.Contains(t, preview, "--background transparent") + require.Contains(t, preview, "--count 1") + fullCommand := formatImagePickerCommandBash(m.settings.args()) + require.Contains(t, fullCommand, imagePickerShellQuote(m.settings.prompt)) +} + +func TestImagePickerCommandScrollPreservesEveryCharacter(t *testing.T) { + for _, size := range [][2]int{{46, 12}, {86, 24}} { + m := pickerForTest(t) + prompt := strings.Repeat("'quoted' 雪 e\u0301 $HOME ", 30) + "\nend\x1b" + m.settings.prompt, m.draft, m.cursor = prompt, []rune(prompt), len([]rune(prompt)) + m.Update(tea.WindowSizeMsg{Width: size[0], Height: size[1]}) + pickerKey(m, tea.KeyTab) + pickerKey(m, tea.KeyTab) + width, height := m.commandDimensions() + visited := map[int]string{} + total := 0 + for { + lines, start, count := m.commandPreview(width, height) + total = count + for i, line := range lines { + visited[start+i] = line + } + require.Contains(t, m.View().Content, "↑↓ scroll") + require.NotContains(t, m.View().Content, "Generate image") + require.NotContains(t, m.View().Content, "Print command") + if start+len(lines) == count { + break + } + pickerKey(m, tea.KeyPgDown) + } + require.Len(t, visited, total) + var reconstructed strings.Builder + for i := range total { + reconstructed.WriteString(visited[i]) + } + require.Equal(t, formatImagePickerCommandBash(m.settings.args()), reconstructed.String()) + require.Equal(t, "command", m.focus) + require.NotNil(t, pickerKey(m, tea.KeyEnter)) + require.Equal(t, prompt, pickerArg(t, m.result.Args, "--prompt")) + } +} + +func TestImagePickerCommandScrollKeysResetAndResize(t *testing.T) { + m := pickerForTest(t) + m.insertPrompt(strings.Repeat(" long prompt", 100)) + m.focus = "command" + width, height := m.commandDimensions() + pickerKey(m, tea.KeyDown) + require.Equal(t, 1, m.commandOffset) + pickerKey(m, tea.KeyUp) + require.Zero(t, m.commandOffset) + pickerKey(m, tea.KeyPgDown) + require.Equal(t, height, m.commandOffset) + pickerKey(m, tea.KeyPgUp) + require.Zero(t, m.commandOffset) + pickerKey(m, tea.KeyEnd) + require.Equal(t, len(m.commandLines(width))-height, m.commandOffset) + m.Update(tea.WindowSizeMsg{Width: 140, Height: 30}) + width, height = m.commandDimensions() + require.Equal(t, len(m.commandLines(width))-height, m.commandOffset) + pickerKey(m, tea.KeyHome) + require.Zero(t, m.commandOffset) + pickerKey(m, tea.KeyEnd) + m.focus = "prompt" + before := m.commandOffset + pickerKey(m, tea.KeyLeft) + require.Equal(t, before, m.commandOffset, "cursor movement must preserve scrolling") + m.Update(tea.KeyPressMsg{Code: 'x', Text: "x"}) + require.Zero(t, m.commandOffset) + m.focus = "command" + pickerKey(m, tea.KeyEnd) + m.Update(tea.KeyPressMsg{Code: tea.KeyTab, Mod: tea.ModShift}) + pickerKey(m, tea.KeyEnter) + pickerKey(m, tea.KeyDown) + pickerKey(m, tea.KeyEnter) + require.Zero(t, m.commandOffset, "changing a setting must reveal command changes") +} + +func TestImagePickerPromptPasteIsText(t *testing.T) { + m := pickerForTest(t) + pickerKey(m, tea.KeyTab) + m.Update(tea.PasteMsg{Content: "q\n\x03\x1b[B\r"}) + require.Empty(t, m.result.Args) + require.False(t, m.result.Canceled) + require.Equal(t, "settings", m.page) + m.Update(tea.KeyPressMsg{Code: tea.KeyTab, Mod: tea.ModShift}) + m.Update(tea.KeyPressMsg{Code: 'u', Mod: tea.ModCtrl}) + paste := "'quoted' $HOME `touch /never` q\n\x03\x1b]52;c;evil\a\t雪" + m.Update(tea.PasteMsg{Content: paste}) + require.Equal(t, paste, string(m.draft)) + require.Equal(t, "prompt", m.focus) + require.False(t, m.result.Canceled) + m.Update(tea.KeyPressMsg{Code: 'q', Text: "q"}) + require.Equal(t, paste+"q", string(m.draft)) + require.False(t, m.result.Canceled) + require.Equal(t, paste+"q", m.settings.prompt) + require.Contains(t, formatImagePickerCommandBash(m.settings.args()), "\\x1b") + view := m.View().Content + require.NotContains(t, view, "\x1b]") + require.NotContains(t, view, "\x03") + m.Update(tea.PasteMsg{Content: "\n\x1b[B\n"}) + require.Empty(t, m.result.Args) + require.Equal(t, paste+"q\n\x1b[B\n", m.settings.prompt) +} + +func TestImagePickerPromptEditing(t *testing.T) { + m := pickerForTest(t) + m.settings.prompt, m.draft, m.cursor = "A雪B", []rune("A雪B"), 3 + pickerKey(m, tea.KeyLeft) + pickerKey(m, tea.KeyBackspace) + require.Equal(t, "AB", string(m.draft)) + m.Update(tea.KeyPressMsg{Code: 'q', Text: "q"}) + pickerKey(m, tea.KeyHome) + pickerKey(m, tea.KeyDelete) + pickerKey(m, tea.KeyEnd) + _, cmd := m.Update(tea.KeyPressMsg{Code: tea.KeyEnter, Mod: tea.ModAlt}) + require.Nil(t, cmd) + require.Empty(t, m.result.Args) + require.Equal(t, "qB\n", m.settings.prompt) +} + +func TestImagePickerCancellationEveryPage(t *testing.T) { + for _, page := range []string{"settings", "choose", "more"} { + t.Run(page, func(t *testing.T) { + m := pickerForTest(t) + m.page, m.field = page, "model" + _, cmd := m.Update(tea.KeyPressMsg{Code: 'c', Mod: tea.ModCtrl}) + require.NotNil(t, cmd) + require.True(t, m.result.Canceled) + require.Empty(t, m.result.Args) + }) + } +} + +func TestImagePickerAllRowsVisibleWhenSelected(t *testing.T) { + for _, page := range []string{"settings", "choose", "more"} { + m := pickerForTest(t) + m.page, m.field, m.width, m.height = page, "count", 48, 12 + m.focus = "options" + for i, row := range m.rows() { + m.selected = i + view := m.View() + require.False(t, view.AltScreen) + require.Contains(t, ansi.Strip(view.Content), "› "+row.label, "layout=%s page=%s row=%d", "compact", page, i) + } + } +} + +func TestImagePickerViewBoundsAndResize(t *testing.T) { + m := pickerForTest(t) + m.settings.prompt = strings.Repeat("雪 e\u0301 👨‍👩‍👧‍👦 \n\t\x1b[31m ", 1000) + for _, size := range [][2]int{{0, 0}, {1, 1}, {12, 3}, {39, 11}, {40, 12}, {48, 12}, {80, 24}, {240, 80}} { + m.Update(tea.WindowSizeMsg{Width: size[0], Height: size[1]}) + for _, page := range []string{"settings", "choose", "more"} { + m.page, m.field, m.selected = page, "quality", 0 + m.draft = []rune(m.settings.prompt) + m.cursor = len(m.draft) + content := m.View().Content + if size[1] == 0 { + require.Empty(t, content) + continue + } + lines := strings.Split(content, "\n") + require.LessOrEqual(t, len(lines), size[1], "page=%s", page) + require.LessOrEqual(t, len(lines), 18, "page=%s", page) + if size[1] >= 12 { + require.LessOrEqual(t, len(lines), size[1]-3, "leave prior shell context visible") + } + for _, line := range lines { + require.LessOrEqual(t, ansi.StringWidth(line), size[0], "page=%s size=%v", page, size) + } + require.NotContains(t, content, "\x1b[31m") + } + } +} + +func TestImagePickerInlineFinishedView(t *testing.T) { + for _, action := range []tea.Msg{ + tea.KeyPressMsg{Code: tea.KeyEnter}, + tea.KeyPressMsg{Code: 'p', Mod: tea.ModCtrl}, + tea.KeyPressMsg{Code: 'c', Mod: tea.ModCtrl}, + imagePickerStopMsg{code: 143}, + } { + m := pickerForTest(t) + view := m.View() + require.False(t, view.AltScreen) + require.NotEmpty(t, view.Content) + _, cmd := m.Update(action) + cmd = cmd + require.NotNil(t, cmd) + view = m.View() + require.False(t, view.AltScreen) + require.Empty(t, view.Content, "the final frame must erase the inline controls") + } +} + +func TestImagePickerInlineRestoresModes(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "output") + require.NoError(t, err) + defer f.Close() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + w := &imagePickerOutput{File: f, cancel: cancel} + p := &imagePickerInline{output: w, restoreWrap: true} + p.start() + p.start() // Repeated capability replies must not start another lifecycle. + p.close() + p.close() + require.NoError(t, w.Err()) + require.NoError(t, ctx.Err()) + _, err = f.Seek(0, io.SeekStart) + require.NoError(t, err) + output, err := io.ReadAll(f) + require.NoError(t, err) + require.Equal(t, ansi.ResetModeTextCursorEnable+ansi.SetModeBracketedPaste+ansi.SetModeAutoWrap+ + "\r"+ansi.EraseScreenBelow+ansi.ResetModeBracketedPaste+ansi.SetModeTextCursorEnable+ansi.ResetModeAutoWrap, string(output)) +} + +func TestImagePickerEarlyPasteCannotSubmitBeforeFirstPaint(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "output") + require.NoError(t, err) + defer f.Close() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + m, err := newImagePicker(imagePickerOptions{}) + require.NoError(t, err) + m.width, m.height = 80, 24 + p := &imagePickerInline{model: m, output: &imagePickerOutput{File: f, cancel: cancel}} + require.NotNil(t, p.Init()) + prompt := "Early paste\nq 'quoted'\rfinal line" + _, cmd := p.Update(tea.PasteMsg{Content: prompt}) + require.Nil(t, cmd) + for _, key := range []tea.KeyPressMsg{ + {Code: tea.KeyEnter}, {Code: 'g', Mod: tea.ModCtrl}, {Code: 'p', Mod: tea.ModCtrl}, + } { + _, cmd := p.Update(key) + require.Nil(t, cmd) + } + require.Equal(t, prompt, m.settings.prompt) + require.Equal(t, prompt, string(m.draft)) + require.Empty(t, m.result.Args) + require.False(t, m.result.Canceled) + _, cmd = p.Update(tea.KeyPressMsg{Code: 'c', Mod: tea.ModCtrl}) + require.NotNil(t, cmd) + p.close() + require.NoError(t, ctx.Err()) + _, err = f.Seek(0, io.SeekStart) + require.NoError(t, err) + output, err := io.ReadAll(f) + require.NoError(t, err) + require.True(t, strings.HasPrefix(string(output), ansi.ResetModeTextCursorEnable+ansi.SetModeBracketedPaste)) + require.Contains(t, string(output), ansi.ResetModeBracketedPaste+ansi.SetModeTextCursorEnable) + require.NotContains(t, string(output), ansi.EraseScreenBelow, "no frame was painted or owned") +} + +func TestImagePickerInlineFrameHasOneLogicalLine(t *testing.T) { + for _, width := range []int{1, 7, 40, 80, 120} { + content := strings.Repeat("❤️", 10) + "\n\x1b[31m" + strings.Repeat("1️⃣", 10) + "\x1b[m\n雪 e\u0301 👨‍👩‍👧‍👦" + frame := imagePickerInlineFrame(content, width) + require.NotContains(t, frame, "\n", "hard line breaks lose ownership during reflow") + require.True(t, strings.HasPrefix(frame, "\r"+ansi.EraseScreenBelow)) + require.True(t, strings.HasSuffix(frame, "\r"+ansi.CursorUp(2))) + paint := strings.TrimSuffix(strings.TrimPrefix(frame, "\r"+ansi.EraseScreenBelow), "\r"+ansi.CursorUp(2)) + rows := strings.Split(paint, ansi.CursorHorizontalAbsolute(width)+" ") + require.Len(t, rows, 4, "three explicit right-edge wrap boundaries") + require.Empty(t, rows[3]) + for _, row := range rows[:3] { + require.True(t, strings.HasPrefix(row, " \r")) + require.Less(t, ansi.WcWidth.StringWidth(strings.TrimPrefix(row, " \r")), width) + require.Less(t, ansi.StringWidth(strings.TrimPrefix(row, " \r")), width) + } + } + require.Equal(t, "\r"+ansi.EraseScreenBelow, imagePickerInlineFrame("", 80)) +} + +func TestImagePickerWidthUsesWholeEmojiClusters(t *testing.T) { + for _, test := range []struct { + text string + cells int + }{{"1️⃣", 2}, {"❤️", 2}, {"👨‍👩‍👧‍👦", 8}, {"雪", 2}, {"e\u0301", 1}} { + sequence, cells, read := imagePickerSequence(test.text + "next") + require.Equal(t, test.text, sequence) + require.Equal(t, test.cells, cells, test.text) + require.Equal(t, len(test.text), read) + } +} + +func TestImagePickerParentContextDuringInput(t *testing.T) { + if descriptor := os.Getenv("OPENAI_PICKER_CONTEXT_TEST_FD"); descriptor != "" { + fd, err := strconv.Atoi(descriptor) + require.NoError(t, err) + control := os.NewFile(uintptr(fd), "context-control") + defer control.Close() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { + var data [1]byte + _, _ = control.Read(data[:]) + cancel() + }() + result, err := runImagePicker(ctx, os.Stdin, os.Stdout, imagePickerOptions{Prompt: "Synthetic context cancellation"}) + require.ErrorIs(t, err, context.Canceled) + require.True(t, result.Canceled) + require.Empty(t, result.Args) + return + } + if runtime.GOOS == "windows" { + t.Skip("requires a Unix PTY") + } + python, err := exec.LookPath("python3") + if err != nil { + t.Skip("python3 is needed for the PTY regression") + } + const script = `import errno,fcntl,os,pty,select,struct,subprocess,sys,termios,time +master,slave=pty.openpty() +fcntl.ioctl(slave,termios.TIOCSWINSZ,struct.pack('HHHH',24,80,0,0)) +initial=termios.tcgetattr(slave) +control,writer=os.pipe() +env=dict(os.environ,TERM='xterm-256color',NO_COLOR='1',OPENAI_PICKER_CONTEXT_TEST_FD=str(control)) +child=subprocess.Popen([sys.argv[1],'-test.run=^TestImagePickerParentContextDuringInput$'],stdin=slave,stdout=slave,stderr=slave,env=env,pass_fds=(control,)) +os.close(control) +raw=bytearray() +deadline=time.monotonic()+12 +sent=False +try: + while child.poll() is None: + assert time.monotonic()= 16 + _, commandHeight := m.commandDimensions() + commandLines, commandStart, commandTotal := m.commandPreview(width-2, commandHeight) + prompt := imagePickerLine(m.settings.prompt) + if m.focus == "prompt" { + before := imagePickerLine(string(m.draft[:m.cursor])) + after := imagePickerLine(string(m.draft[m.cursor:])) + leftWidth := max(0, ansi.StringWidth(before)-(width-6)) + before = ansi.TruncateLeft(before, leftWidth, "…") + prompt = before + "▏" + after + } + if m.settings.prompt == "" { + prompt = "Describe your image…" + if m.focus == "prompt" { + prompt = "▏ " + prompt + } + } + lines := []string{strong.Render("openai") + " " + muted.Render("Images")} + if height >= 18 && m.width >= 50 { + prompt = ansi.Truncate(prompt, width-4, "…") + prompt += strings.Repeat(" ", max(0, width-4-ansi.StringWidth(prompt))) + caption := "╭─ Prompt " + lines = append(lines, "", + border.Render(caption+strings.Repeat("─", width-ansi.StringWidth(caption)-1)+"╮"), + border.Render("│")+input.Render(" "+prompt+" ")+border.Render("│"), + border.Render("╰"+strings.Repeat("─", width-2)+"╯")) + } else { + lines = append(lines, highlight("Prompt", m.focus == "prompt"), " "+prompt) + } + if roomy { + lines = append(lines, "") + } + lines = append(lines, strong.Render(m.heading())) + // Prompt, command and help remain visible. The option list scrolls + // independently, keeping every selected choice accessible in a short terminal. + reserved := len(lines) + 1 + len(commandLines) + if m.note != "" { + reserved++ + } + if roomy { + reserved++ + } + available := max(1, height-reserved) + rows := m.rows() + start := max(0, min(m.selected-available+1, len(rows)-available)) + end := min(len(rows), start+available) + for i := start; i < end; i++ { + row := rows[i] + text := row.label + if row.id == "choice" { + field := m.field + if row.value == m.value(field) { + text += " ✓" + } + if row.detail != "" { + text += " " + row.detail + } + } else if row.value != "" { + text = fmt.Sprintf("%-14s %s", row.label, imagePickerLine(row.value)) + } + active := i == m.selected && m.focus == "options" + lines = append(lines, highlight(ansi.Truncate(text, width-2, "…"), active)) + } + if roomy { + lines = append(lines, "") + } + for i, line := range commandLines { + line = highlight(line, m.focus == "command" && i == 0) + if m.focus != "command" { + line = muted.Render(line) + } + lines = append(lines, line) + } + footer := "Ctrl+C exit · Enter select · ↑↓" + if m.focus == "prompt" { + footer = "Ctrl+C exit · Enter generate · ↓ settings" + if ansi.StringWidth(footer) > width { + footer = "Ctrl+C exit · Enter run · ↓ settings" + } + } else if m.focus == "command" { + footer = "Ctrl+C exit · Enter generate" + if commandTotal > len(commandLines) { + footer = fmt.Sprintf("Ctrl+C exit · ↑↓ scroll %d-%d/%d · Enter generate", commandStart+1, commandStart+len(commandLines), commandTotal) + if ansi.StringWidth(footer) > width { + footer = "Ctrl+C exit · Enter run · ↑↓ scroll" + } + } + if ansi.StringWidth(footer+" · Ctrl+P print") <= width { + footer += " · Ctrl+P print" + } + + } + if len(rows) > available && m.focus == "options" { + footer = fmt.Sprintf("Ctrl+C exit · ↑↓ %d/%d · Enter select", m.selected+1, len(rows)) + } + if m.note != "" { + lines = append(lines, muted.Render(imagePickerLine(m.note))) + } + lines = append(lines, muted.Render(footer)) + view.Content = m.fit(lines, width, height) + return view +} + +// Leave room for previous shell output while keeping the active controls short. +// Tiny terminals retain a visible resize hint and keyboard cancellation. +func (m *imagePicker) viewHeight() int { + if m.height < 12 { + return max(0, min(m.height, 2)) + } + return min(18, m.height-3) +} + +func (m *imagePicker) commandPreview(width, height int) ([]string, int, int) { + lines := m.commandLines(width) + start := max(0, min(m.commandOffset, len(lines)-height)) + return lines[start:min(len(lines), start+height)], start, len(lines) +} + +func (m *imagePicker) commandLines(width int) []string { + command := formatImagePickerCommand(m.settings.args(), m.shell) + var lines []string + var line strings.Builder + used, width := 0, max(1, width) + for len(command) > 0 { + sequence, cells, read := imagePickerSequence(command) + command = command[read:] + if used > 0 && used+cells > width { + lines = append(lines, line.String()) + line.Reset() + used = 0 + } + line.WriteString(sequence) + used += cells + } + return append(lines, line.String()) +} + +func (m *imagePicker) commandDimensions() (width, height int) { + height = 1 + if m.viewHeight() >= 16 { + height = 3 + } + if m.viewHeight() >= 18 { + height = 4 + } + return max(1, min(m.width-4, 100)-2), height +} + +func (m *imagePicker) clampCommandOffset() { + width, height := m.commandDimensions() + m.commandOffset = max(0, min(m.commandOffset, len(m.commandLines(width))-height)) +} + +func (m *imagePicker) scrollCommand(key string) bool { + width, height := m.commandDimensions() + switch key { + case "up": + m.commandOffset-- + case "down": + m.commandOffset++ + case "pgup": + m.commandOffset -= height + case "pgdown": + m.commandOffset += height + case "home": + m.commandOffset = 0 + case "end": + m.commandOffset = len(m.commandLines(width)) - height + default: + return false + } + m.clampCommandOffset() + return true +} + +func (m *imagePicker) fit(lines []string, width, height int) string { + lines = lines[:min(len(lines), height)] + for i, line := range lines { + lines[i] = ansi.Truncate(line, width, "…") + if m.width >= 40 && m.height >= 12 { + lines[i] = " " + lines[i] + } + } + return strings.Join(lines, "\n") +} diff --git a/pkg/custom/image_saving_commands.go b/pkg/custom/image_saving_commands.go index eb72c65d..33c7cdd4 100644 --- a/pkg/custom/image_saving_commands.go +++ b/pkg/custom/image_saving_commands.go @@ -42,6 +42,9 @@ func configureImageSaving(root *cli.Command) { command.CustomHelpTemplate = imageUploadQuickHelp } command.Action = imageSavingWorkflow(command.Action) + if name == "generate" { + command.Action = imagePickerWorkflow(command.Action) + } } } diff --git a/pkg/custom/save_generated_images.go b/pkg/custom/save_generated_images.go index 800b792e..b4dc5254 100644 --- a/pkg/custom/save_generated_images.go +++ b/pkg/custom/save_generated_images.go @@ -17,10 +17,10 @@ import ( // latest availability. Explicit API output retains the API's own defaults. const defaultSavedImageModel = "gpt-image-2.5-sunburst" -const imageGenerationQuickHelp = `{{$bin := or (index .Root.Metadata "help-invocation") "openai"}}Make an image +const imageGenerationQuickHelp = `{{$bin := or (index .Root.Metadata "help-invocation") "openai"}}Generate an image {{$bin}} images generate --prompt "A tiny orange robot" -Replace the prompt with the image you want. +Run without flags to choose settings. Ctrl+C exits. Saves to ~/Downloads/gpt-images/. Full help: {{$bin}} help --all images generate @@ -31,6 +31,9 @@ const imageGenerationSavingHelp = `EXAMPLE openai images generate --prompt "A tiny cat" --name cat +Run without flags to choose settings in a terminal. +After saving, the picker reopens with your settings. Ctrl+C exits. + Saves to ~/Downloads/gpt-images/. Use --output-dir to choose an existing folder. For API data without saving, use --format json without --name or --output-dir. ` diff --git a/scripts/check-image-picker-integration.py b/scripts/check-image-picker-integration.py new file mode 100644 index 00000000..20018366 --- /dev/null +++ b/scripts/check-image-picker-integration.py @@ -0,0 +1,315 @@ +#!/usr/bin/env python3 +"""Check the integrated images generate trigger with PTYs and a loopback API. + +Run with python3 -I -B scripts/check-image-picker-integration.py dist/openai OUTPUT. +Uses synthetic prompts and credentials; never contacts a production API. +""" +import argparse +import importlib.util +import json +import os +import pathlib +import shutil +import signal +import subprocess +import sys +import tempfile +import threading +import time + + +spec = importlib.util.spec_from_file_location( + 'picker_harness', pathlib.Path(__file__).with_name('image_picker_harness.py')) +picker = importlib.util.module_from_spec(spec) +spec.loader.exec_module(picker) + + +class Fixture(picker.Fixture): + def do_POST(self): + self.server.request_headers.append({key.lower(): value for key, value in self.headers.items()}) + self.server.request_paths.append(self.path) + super().do_POST() + + +def check_body(actual, expected): + for key, value in expected.items(): + assert key in actual and actual[key] == value, (key, expected, actual) + + +def check_saved(home, directory=None, filename=None): + directory = directory or home/'Downloads'/'gpt-images' + files = list(directory.glob('*.png')) + assert len(files) == 1 and files[0].read_bytes() == picker.PNG, files + if filename is not None: + assert files[0].name == filename, files + + +def wait_resumed(terminal, mark): + terminal.wait('Saved image:', after=mark) + saved = terminal.raw.index(b'Saved image:', mark) + terminal.wait('Images', after=saved) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('binary') + parser.add_argument('output') + args = parser.parse_args() + binary = str(pathlib.Path(args.binary).resolve()) + output = pathlib.Path(args.output).resolve() + output.mkdir(parents=True, exist_ok=True) + results = [] + with tempfile.TemporaryDirectory(prefix='image-picker-integration-') as temporary: + root = pathlib.Path(temporary) + server = picker.http.server.ThreadingHTTPServer(('127.0.0.1', 0), Fixture) + server.requests, server.request_headers, server.request_paths = [], [], [] + server.mode = 'ok' + server.request_started, server.release = threading.Event(), threading.Event() + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + base_url = f'http://127.0.0.1:{server.server_port}/v1' + base_env = {'PATH': '/usr/bin:/bin', 'TERM': 'xterm-256color', 'LANG': 'en_US.UTF-8', + 'OPENAI_API_KEY': 'synthetic-picker-key', 'OPENAI_BASE_URL': base_url, 'CI': 'true'} + + def environment(name, extra=None): + home = root/name + home.mkdir() + return home, dict(base_env, HOME=str(home), **(extra or {})) + + def passed(name): + results.append({'case': name, 'result': 'pass'}) + print('PASS', name, flush=True) + + def direct_tty(name, arguments, expected_status=1, expected_body=None, extra_env=None, + input_file=None, output_file=None, save_directory=None, save_name=None): + home, env = environment(name, extra_env) + before = len(server.requests) + terminal = picker.Terminal(binary, arguments, env, input_file=input_file, output_file=output_file) + try: + terminal.finish(expected_status, expect_picker=False) + if expected_status == 1: + assert 'Missing required options: --prompt' in terminal.text(), terminal.text() + assert len(server.requests)-before == (1 if expected_body is not None else 0), name + if expected_body is not None: + check_body(server.requests[-1], expected_body) + check_saved(home, save_directory, save_name) + assert not (home/'Library'/'Application Support'/'openai'/'image-picker.json').exists() + assert not (home/'.config'/'openai'/'image-picker.json').exists() + passed(name) + finally: + terminal.save(output/name) + terminal.close() + + def direct_pipe(name, arguments, data=b'', expected_status=1, expected_body=None): + home, env = environment(name) + before = len(server.requests) + result = subprocess.run([binary, *arguments], input=data, capture_output=True, env=env, timeout=12) + (output/(name+'.stdout')).write_bytes(result.stdout) + (output/(name+'.stderr')).write_bytes(result.stderr) + assert result.returncode == expected_status, (name, result.returncode, result.stderr) + if expected_status == 1: + assert b'Missing required options: --prompt' in result.stderr, result.stderr + assert b'\x1b' not in result.stdout+result.stderr, (name, 'terminal sequences in redirected output') + assert len(server.requests)-before == (1 if expected_body is not None else 0), name + if expected_body is not None: + check_body(server.requests[-1], expected_body) + if '--format' in arguments: + assert json.loads(result.stdout)['data'][0]['b64_json'], result.stdout + else: + check_saved(home) + passed(name) + + try: + name = 'bare-tty-early-paste' + home, env = environment(name) + before = len(server.requests) + terminal = picker.Terminal(binary, ['images', 'generate'], env) + prompt = "First line\nSecond 'quoted' line" + try: + deadline = time.monotonic()+5 + while b'\x1b[?2004h' not in terminal.raw: + terminal.read() + assert terminal.child.poll() is None and time.monotonic() < deadline, 'paste mode not enabled' + assert 'Images' not in terminal.text(), 'early-paste probe missed the startup window' + terminal.send(b'\x1b[200~'+prompt.encode()+b'\x1b[201~\r\x07\x10') + picker.ready(terminal) + terminal.wait('Images') + assert len(server.requests) == before, 'startup input submitted an unseen request' + terminal.send(picker.PRINT) + terminal.finish(0) + printed = picker.ANSI.sub('', terminal.raw.rsplit(b'\x1b[?2004l', 1)[-1].decode()).strip() + assert printed.count('openai images generate ') == 1, printed + # Multiline prompts use ANSI-C shell quoting, which Python's + # shlex does not implement. Parse with the real target shell; + # this local function records arguments instead of calling CLI. + shell = shutil.which('zsh') or shutil.which('bash') + assert shell, 'zsh or bash is required to check printed prompt quoting' + parsed = subprocess.run([shell, '-f'], + input="openai() { printf '%s\\0' \"$@\"; }\n"+printed+'\n', + capture_output=True, text=True, env=env, timeout=5) + assert parsed.returncode == 0, parsed.stderr + arguments = parsed.stdout.split('\0')[:-1] + assert arguments[:2] == ['images', 'generate'], arguments + assert dict(zip(arguments[2::2], arguments[3::2]))['--prompt'] == prompt + assert len(server.requests) == before + assert not list(home.glob('Downloads/gpt-images/*')) + assert not (home/'Library'/'Application Support'/'openai'/'image-picker.json').exists() + assert not (home/'.config'/'openai'/'image-picker.json').exists() + passed(name) + finally: + terminal.save(output/name) + terminal.close() + + for action in ['cancel', 'print', 'generate', 'repeated-enter', 'root-connection-options', + 'changed-model', 'literal-at-prompt', 'api-error', 'cancel-request', 'two-generations']: + name = 'bare-tty-'+action + home, env = environment(name) + arguments = ['images', 'generate'] + if action == 'root-connection-options': + arguments = ['--api-key', 'synthetic-root-override-key', '--base-url', base_url+'/override', + '--organization', 'org_synthetic_picker', '--project', 'proj_synthetic_picker', + *arguments] + before = len(server.requests) + server.mode = 'error' if action == 'api-error' else 'slow' if action == 'cancel-request' else 'ok' + server.request_started.clear() + server.release.clear() + terminal = picker.Terminal(binary, arguments, env) + prompt = "Synthetic integrated 'blue cat'" + if action == 'literal-at-prompt': + prompt = "@not-a-local-file 'quoted'\n$(touch NOT_RUN)" + try: + picker.ready(terminal) + terminal.wait('Images') + if action == 'cancel': + terminal.send(b'\x03') + terminal.finish(130) + restored = terminal.raw.rsplit(b'\x1b[?2004l', 1)[-1].decode('utf-8', 'replace') + assert not picker.ANSI.sub('', restored).strip(), 'cancel printed a spurious command error' + else: + terminal.send(b'\x1b[200~'+prompt.encode()+b'\x1b[201~') + if action == 'changed-model': + terminal.send(picker.DOWN+b'\r'+picker.DOWN+b'\r') + if action == 'print': + terminal.send(picker.PRINT) + terminal.finish(0) + assert picker.printed_flags(terminal)['--prompt'] == prompt + else: + mark = len(terminal.raw) + if action == 'changed-model': + terminal.send(b'\x07') # Ctrl+G submits from the options region. + else: + terminal.send(b'\r'*(12 if action == 'repeated-enter' else 1)) + picker.wait_for_request(terminal, server.request_started) + if action == 'cancel-request': + terminal.wait('Generating image') + os.kill(terminal.child.pid, signal.SIGINT) + terminal.finish(130) + elif action == 'api-error': + terminal.finish(1) + assert '401 Unauthorized' in terminal.text(), terminal.text() + assert 'Authentication failed.' in terminal.text(), terminal.text() + else: + wait_resumed(terminal, mark) + if action == 'two-generations': + # Wait for the visible resumed prompt to accept a new submit. + until = time.monotonic()+1.0 + while time.monotonic() < until: + terminal.read(0.02) + server.request_started.clear() + second = len(terminal.raw) + terminal.send(b' appended\r') + picker.wait_for_request(terminal, server.request_started) + wait_resumed(terminal, second) + assert server.requests[-1]['prompt'] == prompt+' appended' + prompt += ' appended' + terminal.send(b'\x03') + terminal.finish(130) + model = 'gpt-image-2.5-flare' if action == 'changed-model' else 'gpt-image-2.5-sunburst' + check_body(server.requests[-1], {'prompt': prompt, 'model': model, + 'size': '1024x1024', 'quality': 'auto', 'output_format': 'png', + 'background': 'auto', 'n': 1}) + if action in {'api-error', 'cancel-request'}: + assert not list(home.glob('Downloads/gpt-images/*')), name + else: + if action == 'two-generations': + saved = list((home/'Downloads'/'gpt-images').glob('*.png')) + assert len(saved) == 2 and all(f.read_bytes() == picker.PNG for f in saved) + else: + check_saved(home) + assert not list(home.rglob('image-picker.json')), 'picker state was persisted' + generates = action not in {'cancel', 'print'} + assert len(server.requests)-before == int(generates)+int(action == 'two-generations'), name + if action == 'root-connection-options': + headers = server.request_headers[-1] + assert headers['authorization'] == 'Bearer synthetic-root-override-key', headers + assert headers['openai-organization'] == 'org_synthetic_picker', headers + assert headers['openai-project'] == 'proj_synthetic_picker', headers + assert server.request_paths[-1] == '/v1/override/images/generations', server.request_paths[-1] + passed(name) + finally: + server.release.set() + terminal.save(output/name) + terminal.close() + + server.mode = 'ok' + direct_tty('help-tty', ['images', 'generate', '--help'], expected_status=0) + direct_tty('full-help-tty', ['help', '--all', 'images', 'generate'], expected_status=0) + direct_tty('dumb-terminal', ['images', 'generate'], extra_env={'TERM': 'dumb'}) + direct_tty('positional-argument', ['images', 'generate', 'unexpected']) + for flag, value in [('model', 'future-image-model'), ('size', '1536x864'), ('quality', 'high'), + ('count', '2'), ('output-format', 'webp'), ('background', 'transparent'), + ('name', 'synthetic-name'), ('inline', 'off'), ('moderation', 'low')]: + direct_tty('local-'+flag+'-without-prompt', ['images', 'generate', '--'+flag, value]) + direct_tty('local-stream-false-without-prompt', ['images', 'generate', '--stream', 'false']) + direct_tty('local-count-zero-without-prompt', ['images', 'generate', '--count', '0']) + for flag, value in [('format', 'json'), ('format', 'text'), ('format-error', 'json'), + ('transform', 'data'), ('transform-error', 'message'), + ('raw-output', 'true'), ('raw-output', 'false')]: + direct_tty('root-'+flag+'-'+value, ['--'+flag+'='+value, 'images', 'generate']) + + direct_tty('explicit-prompt-tty', ['images', 'generate', '--prompt', "A direct 'quoted' robot"], + expected_status=0, expected_body={'prompt': "A direct 'quoted' robot"}) + direct_tty('explicit-empty-prompt-tty', ['images', 'generate', '--prompt', ''], + expected_status=0, expected_body={'prompt': ''}) + target = root/'explicit-save-target' + target.mkdir() + direct_tty('explicit-options-preserved', ['images', 'generate', '--prompt', 'Direct settings', + '--model', 'gpt-image-2.5-flare', '--size', '1536x864', '--quality', 'high', + '--background', 'transparent', '--output-format', 'webp', '--count', '2', + '--moderation', 'low', '--output-compression', '67', '--stream', 'false', + '--output-dir', str(target), '--name', 'preserved', '--inline', 'off'], + expected_status=0, expected_body={'prompt': 'Direct settings', 'model': 'gpt-image-2.5-flare', + 'size': '1536x864', 'quality': 'high', 'background': 'transparent', 'output_format': 'webp', + 'n': 2, 'moderation': 'low', 'output_compression': 67, 'stream': False}, + save_directory=target, save_name='preserved.png') + direct_tty('output-dir-without-prompt', ['images', 'generate', '--output-dir', str(target)]) + + direct_pipe('help-pipe', ['images', 'generate', '--help'], expected_status=0) + direct_pipe('missing-prompt-pipe', ['images', 'generate']) + direct_pipe('direct-prompt-pipe', ['images', 'generate', '--prompt', 'Pipe direct'], + expected_status=0, expected_body={'prompt': 'Pipe direct'}) + payload = {'prompt': "Piped 'JSON' prompt", 'model': 'gpt-image-2.5-flare', 'size': '1536x864', + 'quality': 'high', 'background': 'transparent', 'output_format': 'png', 'n': 2} + direct_pipe('stdin-json', ['images', 'generate'], json.dumps(payload).encode(), + expected_status=0, expected_body=payload) + direct_pipe('stdin-json-explicit-prompt-wins', ['images', 'generate', '--prompt', 'Flag wins'], + json.dumps(payload).encode(), expected_status=0, expected_body=dict(payload, prompt='Flag wins')) + direct_pipe('structured-json-direct', ['--format', 'json', 'images', 'generate', '--prompt', 'Structured'], + expected_status=0, expected_body={'prompt': 'Structured'}) + with tempfile.TemporaryFile() as source: + source.write(json.dumps(payload).encode()) + source.seek(0) + direct_tty('stdin-json-stdout-tty', ['images', 'generate'], expected_status=0, + expected_body=payload, input_file=source) + with (output/'tty-stdin-stdout-file.stdout').open('wb') as destination: + direct_tty('tty-stdin-stdout-file', ['images', 'generate'], output_file=destination) + assert b'\x1b' not in (output/'tty-stdin-stdout-file.stdout').read_bytes() + finally: + server.release.set() + server.shutdown() + server.server_close() + (output/'results.json').write_text(json.dumps(results, indent=2)+'\n') + + +if __name__ == '__main__': + main() diff --git a/scripts/image_picker_harness.py b/scripts/image_picker_harness.py new file mode 100644 index 00000000..ad7e1748 --- /dev/null +++ b/scripts/image_picker_harness.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 +"""Shared PTY and synthetic loopback fixtures for picker process checks.""" +import base64 +import errno +import fcntl +import http.server +import json +import os +import pathlib +import pty +import re +import select +import shlex +import signal +import struct +import subprocess +import sys +import tempfile +import termios +import threading +import time + +PNG = base64.b64decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP438AAAAQBAYDFKhhdAAAAAElFTkSuQmCC') +ANSI = re.compile(r'\x1b\][^\x07]*?(?:\x07|\x1b\\)|\x1b\[[0-?]*[ -/]*[@-~]') +DOWN, UP, HOME, END = b'\x1b[B', b'\x1b[A', b'\x1b[H', b'\x1b[F' +PRINT = b'\x10' # Ctrl+P prints the complete command without making a request. + + +class Terminal: + def __init__(self, binary, args, env, width=80, height=24, + input_file=None, output_file=None, controlling_terminal=False): + self.master, self.slave = pty.openpty() + self.initial = termios.tcgetattr(self.slave) + self.initial_size = (width, height) + self.resize(width, height, notify=False) + self.started = time.monotonic() + self.events, self.raw = [], bytearray() + self.child = subprocess.Popen([binary, *args], stdin=self.slave if input_file is None else input_file, + stdout=self.slave if output_file is None else output_file, stderr=self.slave, + env=env, start_new_session=True, + preexec_fn=(lambda: fcntl.ioctl(0, termios.TIOCSCTTY, 0)) + if controlling_terminal else None) + + def resize(self, width, height, notify=True): + fcntl.ioctl(self.slave, termios.TIOCSWINSZ, struct.pack('HHHH', height, width, 0, 0)) + if notify: + self.events.append([time.monotonic()-self.started, 'r', f'{width}x{height}']) + os.kill(self.child.pid, signal.SIGWINCH) + + def read(self, duration=0.05): + if select.select([self.master], [], [], duration)[0]: + try: + chunk = os.read(self.master, 65536) + except OSError as error: + if error.errno == errno.EIO: + return + raise + self.raw.extend(chunk) + self.events.append([time.monotonic()-self.started, 'o', chunk.decode('utf-8', 'replace')]) + # A PTY has no emulator to answer terminal capability queries. + for query, reply in [(b'\x1b[6n', b'\x1b[1;1R'), + (b'\x1b]11;?\x07', b'\x1b]11;rgb:1818/1818/1818\x07'), + (b'\x1b[c', b'\x1b[?1;2c'), + (b'\x1b[0c', b'\x1b[?1;2c')]: + if query in chunk: + os.write(self.master, reply) + + def text(self): + return ANSI.sub('', self.raw.decode('utf-8', 'replace')) + + def wait(self, marker, timeout=12, after=0): + deadline = time.monotonic()+timeout + while marker not in ANSI.sub('', self.raw[after:].decode('utf-8', 'replace')): + self.read() + if self.child.poll() is not None or time.monotonic() > deadline: + raise AssertionError(f'missing {marker!r}; exit={self.child.poll()}; tail={self.text()[-1800:]}') + + def send(self, value): + os.write(self.master, value) + + def finish(self, expected, timeout=12, expect_picker=True): + deadline = time.monotonic()+timeout + while self.child.poll() is None: + self.read() + if time.monotonic() > deadline: + raise AssertionError('picker did not exit: '+self.text()[-1200:]) + for _ in range(3): + self.read(0.03) + assert self.child.returncode == expected, (self.child.returncode, expected, self.text()[-2000:]) + restored = termios.tcgetattr(self.slave) + # Darwin sets transient PENDIN when restoring canonical input, even for + # a plain tty.setraw/tcsetattr round trip. Ignore only that kernel bit. + if sys.platform == 'darwin': + restored[3] &= ~termios.PENDIN + self.initial[3] &= ~termios.PENDIN + assert restored == self.initial, ('terminal input mode was not restored', self.initial, restored) + assert not re.search(rb'\x1b\[\?(?:47|1047|1049)[hl]', self.raw), 'command used an alternate screen' + assert not re.search(rb'\x1b\[[23]J', self.raw), 'command cleared the screen or scrollback' + if expect_picker: + for mode in (b'2004', b'25'): + enabled = self.raw.rfind(b'\x1b[?'+mode+b'h') + disabled = self.raw.rfind(b'\x1b[?'+mode+b'l') + if mode == b'2004': + assert enabled >= 0 and disabled > enabled, 'bracketed paste was not restored' + else: + assert enabled > disabled >= 0, 'cursor visibility was not restored' + else: + assert b'\x1b[?2004h' not in self.raw, 'direct command entered the picker' + + def close(self): + try: + if self.child.poll() is None: + try: + os.killpg(self.child.pid, signal.SIGKILL) + except ProcessLookupError: + pass + finally: + # Darwin can keep a session leader exiting until the controlling + # PTY's parent handles close. Close them before waiting to reap it. + os.close(self.master) + os.close(self.slave) + self.child.wait(timeout=5) + + def save(self, destination): + destination.with_suffix('.raw').write_bytes(self.raw) + header = dict(version=2, width=self.initial_size[0], height=self.initial_size[1], + title='Local image picker; synthetic API', env={'TERM':'xterm-256color'}) + destination.with_suffix('.cast').write_text('\n'.join(json.dumps(v) for v in [header, *self.events])+'\n') + + +class Fixture(http.server.BaseHTTPRequestHandler): + def log_message(self, *_): + pass + + def do_POST(self): + body = json.loads(self.rfile.read(int(self.headers['Content-Length']))) + self.server.requests.append(body) + self.server.request_started.set() + if self.server.mode == 'slow': + self.server.release.wait(10) + status = 401 if self.server.mode == 'error' else 200 + result = ({'error': {'message':'Synthetic authentication failure', 'type':'invalid_request_error'}} + if status == 401 else {'created':1700000000, 'data':[{'b64_json':base64.b64encode(PNG).decode()}]}) + data = json.dumps(result).encode() + self.send_response(status) + self.send_header('Content-Type', 'application/json') + self.send_header('Content-Length', str(len(data))) + self.end_headers() + try: + self.wfile.write(data) + except (BrokenPipeError, ConnectionResetError): + pass + + +def ready(terminal): + terminal.wait('Prompt') + terminal.wait('openai images generate') + + +def generate(terminal, focus='prompt'): + if focus == 'command': + terminal.send(b'\t\t') + if focus == 'shortcut': + terminal.send(b'\t\x07') # Ctrl+G from options. + else: + terminal.send(b'\r') + + +def wait_for_request(terminal, started, timeout=8): + # Keep draining the PTY while Tea renders its final frame and restores the + # terminal. Waiting only on the server event can fill the PTY output buffer + # and prevent the child from reaching the API request at all. + deadline = time.monotonic() + timeout + while not started.is_set(): + remaining = deadline - time.monotonic() + if remaining <= 0: + raise AssertionError('no request reached fixture; tail='+terminal.text()[-1800:]) + terminal.read(min(0.05, remaining)) + if terminal.child.poll() is not None and not started.is_set(): + raise AssertionError(f'child exited before request; exit={terminal.child.returncode}; tail={terminal.text()[-1800:]}') + + +def printed_flags(terminal): + """Read the full command printed after terminal restoration, without running it.""" + # Inline rendering stays in the main buffer. Bracketed-paste teardown marks + # terminal restoration; only subsequent output is the submitted command. + assert b'\x1b[?2004l' in terminal.raw, 'picker did not restore terminal input' + after_ui = terminal.raw.rsplit(b'\x1b[?2004l', 1)[-1].decode('utf-8', 'replace') + lines = ANSI.sub('', after_ui).splitlines() + commands = [line for line in lines if line.startswith('openai images generate ')] + assert len(commands) == 1, ('expected one printed command', lines) + arguments = shlex.split(commands[0]) + assert len(arguments[3:]) % 2 == 0, arguments + return dict(zip(arguments[3::2], arguments[4::2])) + + From 84d4282b78919558db5b7ec4ae4ea6b793238926 Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 16:04:31 -0700 Subject: [PATCH 2/7] Remove redundant picker test assignments --- pkg/custom/image_picker_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/custom/image_picker_test.go b/pkg/custom/image_picker_test.go index 9f022c3c..a22cf371 100644 --- a/pkg/custom/image_picker_test.go +++ b/pkg/custom/image_picker_test.go @@ -317,7 +317,6 @@ func TestImagePickerCtrlGFromDropdown(t *testing.T) { require.Equal(t, "choose", m.page) pickerKey(m, tea.KeyDown) _, cmd := m.Update(tea.KeyPressMsg{Code: 'g', Mod: tea.ModCtrl}) - cmd = cmd require.NotNil(t, cmd) require.Equal(t, openai.ImageModelGPTImage2_5Sunburst, m.settings.model, "unconfirmed highlighted option must not apply") require.Equal(t, openai.ImageModelGPTImage2_5Sunburst, pickerArg(t, m.result.Args, "--model")) @@ -616,7 +615,6 @@ func TestImagePickerInlineFinishedView(t *testing.T) { require.False(t, view.AltScreen) require.NotEmpty(t, view.Content) _, cmd := m.Update(action) - cmd = cmd require.NotNil(t, cmd) view = m.View() require.False(t, view.AltScreen) From 8fefbb87192235cf02efa552e90d41fcd3b767bb Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 16:53:01 -0700 Subject: [PATCH 3/7] Remove trailing blank lines from picker PTY helper --- scripts/image_picker_harness.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts/image_picker_harness.py b/scripts/image_picker_harness.py index ad7e1748..aec4f23b 100644 --- a/scripts/image_picker_harness.py +++ b/scripts/image_picker_harness.py @@ -192,5 +192,3 @@ def printed_flags(terminal): arguments = shlex.split(commands[0]) assert len(arguments[3:]) % 2 == 0, arguments return dict(zip(arguments[3::2], arguments[4::2])) - - From 8598361aad82551d75e6240298ce12d58cf95111 Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 17:03:05 -0700 Subject: [PATCH 4/7] Mark picker terminal library as a direct dependency --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 0464ab93..86c269a4 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( charm.land/bubbletea/v2 v2.0.8 charm.land/lipgloss/v2 v2.0.5 github.com/charmbracelet/colorprofile v0.4.3 + github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7 github.com/charmbracelet/x/ansi v0.11.8 github.com/charmbracelet/x/term v0.2.2 github.com/goccy/go-yaml v1.19.2 @@ -23,7 +24,6 @@ require ( ) require ( - github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7 // indirect github.com/charmbracelet/x/termios v0.1.1 // indirect github.com/charmbracelet/x/windows v0.2.2 // indirect github.com/clipperhouse/displaywidth v0.11.0 // indirect From 0cc82d810aa48434ceb6784fd35be381a4da173f Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 16:02:00 -0700 Subject: [PATCH 5/7] Add current-session image picker Tab shortcuts --- cmd/openai/image_picker_completion_test.go | 58 ++ docs/image-generation-saving.md | 3 + docs/image-picker-shortcuts.md | 35 ++ internal/autocomplete/autocomplete.go | 7 + internal/autocomplete/picker_bash_zsh_test.go | 554 ++++++++++++++++++ internal/autocomplete/picker_fish_test.go | 106 ++++ internal/autocomplete/picker_script.go | 38 ++ internal/autocomplete/picker_script_test.go | 68 +++ .../shellscripts/bash_autocomplete.bash | 21 +- .../shellscripts/bash_picker.bash | 120 ++++ .../shellscripts/fish_picker.fish | 120 ++++ .../autocomplete/shellscripts/zsh_picker.zsh | 97 +++ pkg/custom/command.go | 1 + pkg/custom/image_picker_completion.go | 23 + pkg/custom/local_errors.go | 1 + 15 files changed, 1247 insertions(+), 5 deletions(-) create mode 100644 cmd/openai/image_picker_completion_test.go create mode 100644 docs/image-picker-shortcuts.md create mode 100644 internal/autocomplete/picker_bash_zsh_test.go create mode 100644 internal/autocomplete/picker_fish_test.go create mode 100644 internal/autocomplete/picker_script.go create mode 100644 internal/autocomplete/picker_script_test.go create mode 100644 internal/autocomplete/shellscripts/bash_picker.bash create mode 100644 internal/autocomplete/shellscripts/fish_picker.fish create mode 100644 internal/autocomplete/shellscripts/zsh_picker.zsh create mode 100644 pkg/custom/image_picker_completion.go diff --git a/cmd/openai/image_picker_completion_test.go b/cmd/openai/image_picker_completion_test.go new file mode 100644 index 00000000..a24ab90b --- /dev/null +++ b/cmd/openai/image_picker_completion_test.go @@ -0,0 +1,58 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "os" + "strings" + "sync/atomic" + "testing" +) + +func TestMainPickerCompletion(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + requests.Add(1) + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + home := t.TempDir() + env := []string{"HOME=" + home, "USERPROFILE=" + home, "XDG_CONFIG_HOME=" + home, "OPENAI_BASE_URL=" + server.URL} + for _, shell := range []string{"bash", "zsh", "fish", "pwsh"} { + t.Run(shell, func(t *testing.T) { + standard := runMainDispatchWithEnv(t, shell, env, "openai", "@completion", shell) + if standard.code != 0 || standard.stderr != "" || standard.stdout == "" || strings.Contains(standard.stdout, "picker_disable") { + t.Fatalf("standard completion = %+v", standard) + } + disabled := runMainDispatchWithEnv(t, shell, env, "openai", "@completion", shell, "--picker=false") + if disabled != standard { + t.Fatalf("explicit false changed ordinary completion: %+v", disabled) + } + for _, format := range []string{"text", "json"} { + got := runMainDispatchWithEnv(t, shell, env, "openai", "--format-error", format, "@completion", shell, "--picker") + if shell == "pwsh" { + if got.code != 1 || got.stdout != "" { + t.Fatalf("unsupported hook = %+v", got) + } + message := got.stderr + if format == "json" { + payload := decodeMainStructuredError(t, format, got.stderr) + message, _ = payload["message"].(string) + } + if !strings.Contains(message, "press Enter") { + t.Fatalf("missing Enter fallback: %q", message) + } + } else if got.code != 0 || got.stderr != "" || !strings.HasPrefix(got.stdout, standard.stdout+"\n") || !strings.Contains(got.stdout, "openai_picker_disable") { + t.Fatalf("picker completion = %+v", got) + } + } + }) + } + if requests.Load() != 0 { + t.Fatalf("completion made %d API requests", requests.Load()) + } + entries, err := os.ReadDir(home) + if err != nil || len(entries) != 0 { + t.Fatalf("completion wrote to the home/config directory: %v, %v", entries, err) + } +} diff --git a/docs/image-generation-saving.md b/docs/image-generation-saving.md index 559b1934..40ca6ced 100644 --- a/docs/image-generation-saving.md +++ b/docs/image-generation-saving.md @@ -16,6 +16,9 @@ background, PNG and one image. Enter generates from the prompt; use the arrow keys or Tab to move through settings. Ctrl+P prints the command without requesting an image, and Ctrl+C exits. +An optional [Tab shortcut](image-picker-shortcuts.md) opens the same picker in +Bash 4.3+, zsh and fish. Other shells use Enter. + After a successful generation, the picker reopens below the saved result with the active prompt and settings. Choices last until this invocation exits. Explicit image flags, output formats, piped input and redirected output retain diff --git a/docs/image-picker-shortcuts.md b/docs/image-picker-shortcuts.md new file mode 100644 index 00000000..2f470724 --- /dev/null +++ b/docs/image-picker-shortcuts.md @@ -0,0 +1,35 @@ +# Image picker shortcuts + +`openai images generate` opens the image picker when you press Enter in an +interactive terminal. To also open it with Tab, load the optional hook in your +current shell session: + +```bash +# Bash 4.3 or newer +source <(openai @completion bash --picker) +``` + +```zsh +# zsh, after completion has been initialized +autoload -Uz compinit && compinit +source <(openai @completion zsh --picker) +``` + +```fish +# fish +openai @completion fish --picker | source +``` + +Type `openai images generate` and press Tab at the end of the line. Ctrl+C +leaves the picker and keeps the command editable. Other command lines retain +ordinary completion. The shortcut requires `openai` on PATH as an executable; +aliases, shell functions, extra arguments, redirections and compound commands +use ordinary completion. Existing custom Bash Tab bindings are preserved. + +Run `openai_picker_disable` to turn off the hook in this session. Closing the +shell also removes it. These commands do not edit startup files. Ordinary +completion scripts are still available without `--picker`. + +PowerShell and Bash older than 4.3 keep normal Tab completion. Press Enter after +`openai images generate` to open the picker. Hooks are inactive outside an +interactive terminal or when `TERM=dumb`. diff --git a/internal/autocomplete/autocomplete.go b/internal/autocomplete/autocomplete.go index 2cd852e0..5f2ddf5f 100644 --- a/internal/autocomplete/autocomplete.go +++ b/internal/autocomplete/autocomplete.go @@ -66,6 +66,13 @@ func OutputCompletionScript(ctx context.Context, cmd *cli.Command) error { if err != nil { return cli.Exit(err, 1) } + if cmd.Bool("picker") { + picker, err := renderPickerCompletion(s, cmd.Root().Name) + if err != nil { + return cli.Exit(err, 1) + } + completionScript += "\n" + picker + } _, err = cmd.Writer.Write([]byte(completionScript)) if err != nil { diff --git a/internal/autocomplete/picker_bash_zsh_test.go b/internal/autocomplete/picker_bash_zsh_test.go new file mode 100644 index 00000000..813c75a1 --- /dev/null +++ b/internal/autocomplete/picker_bash_zsh_test.go @@ -0,0 +1,554 @@ +package autocomplete + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +// These tests run real line editors. The synthetic executable records the +// argv/terminal boundary; the picker process suite exercises the real UI. +func TestPickerBashZshTabAndLifecycle(t *testing.T) { + for _, shell := range []string{"bash", "zsh"} { + t.Run(shell, func(t *testing.T) { + requirePickerBash(t, shell) + directory := t.TempDir() + setup := "" + if shell == "zsh" { + setup = ` +fallback_emacs() { print -r -- fallback-emacs >>"$PICKER_TEST_RESULT"; } +fallback_viins() { print -r -- fallback-viins >>"$PICKER_TEST_RESULT"; } +zle -N fallback_emacs +zle -N fallback_viins +bindkey -M emacs '^I' fallback_emacs +bindkey -M viins '^I' fallback_viins +bindkey -e +` + } + body := ` +send -- "openai images generate\t" +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +# The same command line remains editable after cancellation. Ctrl+U discards it. +send -- "\025printf 'AFTER_CANCEL\\n'\r" +expect -exact "AFTER_CANCEL\r\n" +expect -exact "PICKER_TEST> " +send -- "openai images gen\t" +` + if shell == "bash" { + body += ` +expect -exact "generate " +send -- "\025openai_picker_disable; complete -p openai >\"\$PICKER_TEST_BINDING\"\r" +` + } else { + body += ` +send -- "\025bindkey -v\r" +expect -exact "PICKER_TEST> " +send -- "openai images generate\t" +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +send -- "\025openai images gen\t" +send -- "\025openai_picker_disable; bindkey -M emacs '^I' >\"\$PICKER_TEST_BINDING\"; bindkey -M viins '^I' >>\"\$PICKER_TEST_BINDING\"\r" +` + } + body += ` +expect -exact "PICKER_TEST> " +send -- "exit\r" +expect eof +` + runPickerShellPTY(t, shell, directory, setup, "", body) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + text := string(result) + require.Contains(t, text, "launch "+shell+" 2\n\n\ntty:111\n") + binding, err := os.ReadFile(filepath.Join(directory, "binding")) + require.NoError(t, err) + if shell == "bash" { + require.Equal(t, 1, strings.Count(text, "launch ")) + require.Equal(t, "complete -o filenames -F __openai_bash_autocomplete openai\n", string(binding)) + } else { + require.Equal(t, 2, strings.Count(text, "launch ")) + require.Contains(t, text, "fallback-emacs\n") + require.Contains(t, text, "fallback-viins\n") + require.Equal(t, "\"^I\" fallback_emacs\n\"^I\" fallback_viins\n", string(binding)) + } + }) + } +} + +func TestPickerBashZshExactLines(t *testing.T) { + lines := []struct { + line string + want string + }{ + {"openai images generate", "yes"}, + {" \topenai\timages generate \t", "yes"}, + {"OPENAI IMAGES GENERATE", "no"}, + {"openai images generate --help", "no"}, + {"openai images generate >out", "no"}, + {"openai images generate;", "no"}, + {"openai images generate | cat", "no"}, + {"openai images generate && true", "no"}, + {"openai images generate\n", "no"}, + {"openai\nimages generate", "no"}, + {"'openai' images generate", "no"}, + {"command openai images generate", "no"}, + {"openai images generat", "no"}, + {"openai images generate $(touch unsafe)", "no"}, + {"openai\rimages generate", "no"}, + } + for _, shell := range []string{"bash", "zsh"} { + t.Run(shell, func(t *testing.T) { + requirePickerBash(t, shell) + var probe strings.Builder + if shell == "bash" { + probe.WriteString("shopt -s nocasematch\n") + } else { + probe.WriteString("setopt nocasematch\n") + } + var expected strings.Builder + for _, line := range lines { + variable, cursor := "COMP_LINE", "COMP_POINT" + if shell == "zsh" { + variable, cursor = "BUFFER", "CURSOR" + } + fmt.Fprintf(&probe, "%s=%s; %s=${#%s}\n", variable, pickerShellQuote(line.line), cursor, variable) + probe.WriteString("if __openai_picker_matches; then printf 'yes\\n'; else printf 'no\\n'; fi >>\"$PICKER_TEST_RESULT\"\n") + expected.WriteString(line.want + "\n") + } + probe.WriteString("COMP_LINE='openai images generate'; COMP_POINT=5; BUFFER=$COMP_LINE; CURSOR=5\n") + probe.WriteString("if __openai_picker_matches; then printf 'yes\\n'; else printf 'no\\n'; fi >>\"$PICKER_TEST_RESULT\"\n") + expected.WriteString("no\n") + directory := t.TempDir() + runPickerShellPTY(t, shell, directory, "", probe.String(), `send -- "exit\r"; expect eof`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, expected.String(), string(result)) + }) + } +} + +func TestPickerZshHintAndReplacement(t *testing.T) { + directory := t.TempDir() + probe := ` +BUFFER='openai images generate'; CURSOR=${#BUFFER}; POSTDISPLAY='existing suggestion' +__openai_picker_hint +printf '%s\n' "$POSTDISPLAY" >>"$PICKER_TEST_RESULT" +POSTDISPLAY=''; __openai_picker_hint +printf '%s\n' "$POSTDISPLAY" >>"$PICKER_TEST_RESULT" +BUFFER='openai images generate --help'; __openai_picker_hint +printf '<%s>\n' "$POSTDISPLAY" >>"$PICKER_TEST_RESULT" +POSTDISPLAY='new suggestion'; __openai_picker_hint +printf '%s\n' "$POSTDISPLAY" >>"$PICKER_TEST_RESULT" +new_tab() { :; }; zle -N new_tab +bindkey -M emacs '^I' new_tab +source "$PICKER_TEST_HOOK" +openai_picker_disable +bindkey -M emacs '^I' >>"$PICKER_TEST_RESULT" +openai_picker_disable +bindkey -M emacs '^I' >>"$PICKER_TEST_RESULT" +` + runPickerShellPTY(t, "zsh", directory, "", probe, `send -- "exit\r"; expect eof`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "existing suggestion\n [Tab: image options]\n<>\nnew suggestion\n\"^I\" new_tab\n\"^I\" new_tab\n", string(result)) +} + +func TestPickerBashReplacementAndDisable(t *testing.T) { + requirePickerBash(t, "bash") + directory := t.TempDir() + probe := ` +complete -F newer_completion openai +openai_picker_disable +complete -p openai >>"$PICKER_TEST_RESULT" +openai_picker_disable +complete -p openai >>"$PICKER_TEST_RESULT" +` + runPickerShellPTY(t, "bash", directory, "", probe, `send -- "exit\r"; expect eof`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, strings.Repeat("complete -F newer_completion openai\n", 2), string(result)) +} + +func TestPickerBashBindingsAndResourcing(t *testing.T) { + requirePickerBash(t, "bash") + for _, scenario := range []struct { + name, setup, probe, want string + }{ + { + name: "custom macro", + setup: `bind '"\C-i": "custom"'`, + probe: `source "$PICKER_TEST_HOOK"; openai_picker_disable +__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, + want: "\"\\C-i\": \"custom\"\n", + }, + { + name: "custom shell callback", + setup: `bind -x '"\C-i": printf custom'`, + probe: `source "$PICKER_TEST_HOOK"; openai_picker_disable +__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, + want: "\"\\C-i\": \"printf custom\"\n", + }, + { + name: "occupied private key", + setup: `bind -x '"\e[99;1~": printf owned'`, + probe: `__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT" +openai_picker_disable +__openai_picker_binding emacs-standard '\e[99;1~' >>"$PICKER_TEST_RESULT"`, + want: "\"\\C-i\": complete\n\"\\e[99;1~\": \"printf owned\"\n", + }, + { + name: "replacement after install", + probe: `bind '"\C-i": "new owner"' +source "$PICKER_TEST_HOOK"; openai_picker_disable +__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, + want: "\"\\C-i\": \"new owner\"\n", + }, + { + name: "replacement private callback", + probe: `bind -x '"\e[99;2~": printf newer' +COMP_LINE='openai images generate'; COMP_POINT=${#COMP_LINE}; COMP_TYPE=9; COMP_KEY=126 +COMP_WORDS=(openai images generate); COMP_CWORD=2 +__openai_picker_complete +openai_picker_disable +__openai_picker_binding emacs-standard '\e[99;2~' >>"$PICKER_TEST_RESULT"`, + want: "\"\\e[99;2~\": \"printf newer\"\n", + }, + { + name: "repeated install disable enable", + probe: `source "$PICKER_TEST_HOOK"; openai_picker_disable +__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT" +__openai_picker_binding emacs-standard '\e[99;1~' >>"$PICKER_TEST_RESULT" +__openai_picker_binding emacs-standard '\e[99;2~' >>"$PICKER_TEST_RESULT" +source "$PICKER_TEST_HOOK"; openai_picker_disable +__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, + want: strings.Repeat("\"\\C-i\": complete\n", 2), + }, + } { + t.Run(scenario.name, func(t *testing.T) { + directory := t.TempDir() + runPickerShellPTY(t, "bash", directory, scenario.setup, scenario.probe, `send -- "exit\r"; expect eof`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, scenario.want, string(result)) + }) + } +} + +func TestPickerBashRejectsWholeLineContext(t *testing.T) { + requirePickerBash(t, "bash") + var interaction strings.Builder + for index, line := range []string{ + "OPENAI_KEY=x openai images generate", + "true; openai images generate", + "true && openai images generate", + "true | openai images generate", + "(openai images generate", + } { + fmt.Fprintf(&interaction, "send -- {%s}\nsend -- \"\\t\\025printf 'CONTEXT_%d\\\\n'\\r\"\n", line, index) + fmt.Fprintf(&interaction, "expect -exact \"CONTEXT_%d\\r\\n\"\nexpect -exact \"PICKER_TEST> \"\n", index) + } + interaction.WriteString(`send -- "exit\r"; expect eof`) + directory := t.TempDir() + runPickerShellPTY(t, "bash", directory, "", "", interaction.String()) + _, err := os.Stat(filepath.Join(directory, "result")) + require.ErrorIs(t, err, os.ErrNotExist, "none of the completion attempts may run the picker") +} + +func TestPickerBashKeepsRepeatedCompletionAfterCancel(t *testing.T) { + requirePickerBash(t, "bash") + directory := t.TempDir() + runPickerShellPTY(t, "bash", directory, "", "", ` +send -- "openai images generate\t" +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +send -- "\025__openai_bash_autocomplete() { COMPREPLY=(general generate); }\r" +expect -exact "PICKER_TEST> " +send -- "openai gen\t\t\t" +expect -re {general +generate} +send -- "\025exit\r"; expect eof +`) +} + +func TestPickerBashPreservesWhitespaceAfterCancel(t *testing.T) { + requirePickerBash(t, "bash") + for _, line := range []string{" openai images generate", " openai\timages generate ", "openai images generate\t"} { + t.Run(fmt.Sprintf("%q", line), func(t *testing.T) { + directory := t.TempDir() + probe := ` +capture_buffer() { printf 'buffer:<%s>\n' "$READLINE_LINE" >>"$PICKER_TEST_RESULT"; } +bind -x '"\C-x\C-g": capture_buffer' +` + interaction := fmt.Sprintf("send -- \"%s\\t\"\n", strings.ReplaceAll(line, "\t", `\026\t`)) + ` +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +send -- "\030\007\025exit\r"; expect eof +` + runPickerShellPTY(t, "bash", directory, "", probe, interaction) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Contains(t, string(result), "buffer:<"+line+">\n") + }) + } +} + +func TestPickerBashLegacyKeepsNormalCompletion(t *testing.T) { + if _, err := os.Stat("/bin/bash"); err != nil { + t.Skip("system Bash is unavailable") + } + if err := exec.Command("/bin/bash", "-c", `(( BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 3) ))`).Run(); err != nil { + t.Skip("system Bash is new enough for the Tab hook") + } + t.Setenv("OPENAI_CLI_TEST_BASH", "/bin/bash") + directory := t.TempDir() + probe := ` +if type openai_picker_disable >/dev/null 2>&1; then printf 'installed\n'; else printf 'preserved\n'; fi >>"$PICKER_TEST_RESULT" +complete -p openai >>"$PICKER_TEST_RESULT" +` + runPickerShellPTY(t, "bash", directory, "", probe, ` +send -- "openai images gen\t" +expect -exact "generate " +send -- "\025exit\r"; expect eof +`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "preserved\ncomplete -o filenames -F __openai_bash_autocomplete openai\n", string(result)) +} + +func TestPickerBashZshRejectAliasesAndFunctions(t *testing.T) { + for _, shell := range []string{"bash", "zsh"} { + t.Run(shell, func(t *testing.T) { + requirePickerBash(t, shell) + directory := t.TempDir() + probe := ` +COMP_LINE='openai images generate'; COMP_POINT=${#COMP_LINE}; BUFFER=$COMP_LINE; CURSOR=${#BUFFER} +alias openai='printf alias' +if __openai_picker_matches; then printf 'launched\n'; else printf 'preserved\n'; fi >>"$PICKER_TEST_RESULT" +unalias openai +openai() { :; } +if __openai_picker_matches; then printf 'launched\n'; else printf 'preserved\n'; fi >>"$PICKER_TEST_RESULT" +` + runPickerShellPTY(t, shell, directory, "", probe, `send -- "exit\r"; expect eof`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "preserved\npreserved\n", string(result)) + }) + } +} + +func TestPickerZshChainedReplacementAfterReenable(t *testing.T) { + directory := t.TempDir() + setup := ` +old_tab() { print -r -- old >>"$PICKER_TEST_RESULT"; } +zle -N old_tab +bindkey -M emacs '^I' old_tab +bindkey -e +` + probe := ` +new_tab() { print -r -- new >>"$PICKER_TEST_RESULT"; zle __openai_picker_tab_emacs; } +zle -N new_tab +bindkey -M emacs '^I' new_tab +openai_picker_disable +source "$PICKER_TEST_HOOK" +` + runPickerShellPTY(t, "zsh", directory, setup, probe, ` +send -- "unrelated\t" +send -- "\025printf 'CHAINED_DONE\\n'\r" +expect -exact "CHAINED_DONE\r\n" +expect -exact "PICKER_TEST> " +send -- "exit\r"; expect eof +`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "new\nold\n", string(result)) +} + +func TestPickerBashZshDumbTerminalKeepsCompletion(t *testing.T) { + for _, shell := range []string{"bash", "zsh"} { + t.Run(shell, func(t *testing.T) { + directory := t.TempDir() + probe := ` +if type openai_picker_disable >/dev/null 2>&1; then printf 'installed\n'; else printf 'preserved\n'; fi >>"$PICKER_TEST_RESULT" +` + runPickerShellPTY(t, shell, directory, "TERM=dumb", probe, `send -- "exit\r"; expect eof`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "preserved\n", string(result)) + }) + } +} + +func TestPickerBashZshDoesNotInstallOutsideTerminal(t *testing.T) { + for _, shell := range []string{"bash", "zsh"} { + t.Run(shell, func(t *testing.T) { + binary, err := exec.LookPath(shell) + if err != nil { + t.Skip(shell + " is not available") + } + script, err := renderPickerCompletion(CompletionStyle(shell), "openai") + require.NoError(t, err) + cmd := exec.Command(binary, "-c", script+"\nif type openai_picker_disable >/dev/null 2>&1; then exit 1; fi") + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) + }) + } +} + +func TestBashCompletionWithoutMapfile(t *testing.T) { + bash, err := exec.LookPath("bash") + if err != nil { + t.Skip("bash is not available") + } + script, err := shellCompletions[CompletionStyleBash](&cli.Command{}, "openai") + require.NoError(t, err) + for _, scenario := range []string{"values", "files", "empty"} { + t.Run(scenario, func(t *testing.T) { + directory := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(directory, "fixture one.txt"), nil, 0o600)) + probe := ` +mapfile() { printf 'unexpected mapfile call' >&2; return 99; } +openai() { if [ "$PICKER_COMPLETION_CASE" = files ]; then return 10; fi; if [ "$PICKER_COMPLETION_CASE" != empty ]; then printf 'fixture one\nfixture two\n'; fi; } +` + script + ` +file='caller shell value' +COMP_WORDS=(openai fix); COMP_CWORD=1 +__openai_bash_autocomplete +if [[ "$file" != 'caller shell value' ]]; then + printf 'completion changed caller variable file to <%s>\n' "$file" >&2 + exit 1 +fi +for candidate in "${COMPREPLY[@]}"; do printf '<%s>\n' "$candidate"; done +` + cmd := exec.Command(bash, "-c", probe) + cmd.Dir = directory + cmd.Env = append(os.Environ(), "PICKER_COMPLETION_CASE="+scenario) + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) + want := "\n\n" + if scenario == "files" { + want = "\n" + } else if scenario == "empty" { + want = "" + } + require.Equal(t, want, string(out)) + }) + } +} + +func pickerShellQuote(value string) string { + return "'" + strings.ReplaceAll(value, "'", "'\\''") + "'" +} + +func runPickerShellPTY(t *testing.T, shell, directory, setup, probe, interaction string) { + t.Helper() + binary, err := exec.LookPath(pickerTestShell(shell)) + if err != nil { + t.Skip(shell + " is not available") + } + expect, err := exec.LookPath("expect") + if err != nil { + t.Skip("expect is required for real shell line editor tests") + } + completion, err := shellCompletions[CompletionStyle(shell)](&cli.Command{}, "openai") + require.NoError(t, err) + hook, err := renderPickerCompletion(CompletionStyle(shell), "openai") + require.NoError(t, err) + if shell == "zsh" { + completion = "autoload -Uz compinit; compinit -D\n" + completion + } + require.NoError(t, os.WriteFile(filepath.Join(directory, "hook"), []byte(hook), 0o600)) + startup := "PS1='PICKER_TEST> '; PS2='PICKER_MORE> '\n" + completion + "\n" + setup + "\n" + hook + "\n" + probe + "\nprintf '\\nPICKER_SHELL_READY\\n'\n" + if shell == "fish" { + startup = "function fish_prompt; printf 'PICKER_TEST> '; end\n" + completion + "\n" + setup + "\n" + hook + "\n" + probe + "\nprintf '\\nPICKER_SHELL_READY\\n'\n" + } + require.NoError(t, os.WriteFile(filepath.Join(directory, "startup"), []byte(startup), 0o600)) + fixture := `#!/bin/sh +if [ "$1" = __complete ]; then + for completion_word do :; done + case "$completion_word" in gen*) printf 'generate\n';; esac + exit 0 +fi +printf 'launch %s %s\n' "$OPENAI_PICKER_SHELL" "$#" >>"$PICKER_TEST_RESULT" +printf '<%s>\n' "$@" >>"$PICKER_TEST_RESULT" +terminal_fds='' +for fd in 0 1 2; do if [ -t "$fd" ]; then terminal_fds="${terminal_fds}1"; else terminal_fds="${terminal_fds}0"; fi; done +printf 'tty:%s\n' "$terminal_fds" >>"$PICKER_TEST_RESULT" +printf '\nPICKER_LAUNCHED\n' +read -r response +printf '\nPICKER_CANCELED\n' +exit 130 +` + require.NoError(t, os.WriteFile(filepath.Join(directory, "openai"), []byte(fixture), 0o700)) + arguments := "--noprofile --norc -i" + if shell == "zsh" { + arguments = "-f -i" + } + if shell == "fish" { + arguments = "--no-config --interactive" + } + driver := ` +set timeout 8 +match_max 100000 +proc fail {message} { puts stderr $message; exit 1 } +spawn -noecho $env(PICKER_TEST_SHELL) ` + arguments + ` +expect_before { + -exact "\033\1330c" {send -- "\033\133?1;2c"; exp_continue} + -exact "\033\1336n" {send -- "\033\1331;1R"; exp_continue} +} +expect_after timeout { fail "shell probe timed out" } +send -- "source \"\$PICKER_TEST_STARTUP\"\r" +expect -exact "\r\nPICKER_SHELL_READY\r\n" +expect -exact "PICKER_TEST> " +` + interaction + require.NoError(t, os.WriteFile(filepath.Join(directory, "driver.exp"), []byte(driver), 0o600)) + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, expect, filepath.Join(directory, "driver.exp")) + cmd.Dir = directory + cmd.Env = []string{ + "HOME=" + directory, "PATH=" + directory + ":" + os.Getenv("PATH"), "LC_ALL=C", "TERM=xterm-256color", + "BASH_SILENCE_DEPRECATION_WARNING=1", "PICKER_TEST_SHELL=" + binary, + "PICKER_TEST_STARTUP=" + filepath.Join(directory, "startup"), + "PICKER_TEST_HOOK=" + filepath.Join(directory, "hook"), + "PICKER_TEST_RESULT=" + filepath.Join(directory, "result"), + "PICKER_TEST_BINDING=" + filepath.Join(directory, "binding"), + } + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) +} + +func pickerTestShell(shell string) string { + if shell == "bash" && os.Getenv("OPENAI_CLI_TEST_BASH") != "" { + return os.Getenv("OPENAI_CLI_TEST_BASH") + } + return shell +} + +func requirePickerBash(t *testing.T, shell string) { + t.Helper() + if shell != "bash" { + return + } + binary, err := exec.LookPath(pickerTestShell(shell)) + if err != nil { + t.Skip("bash is not available") + } + if err := exec.Command(binary, "-c", `(( BASH_VERSINFO[0] > 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] >= 3) ))`).Run(); err != nil { + t.Skip("Tab hook requires Bash 4.3+; set OPENAI_CLI_TEST_BASH to test a supported runtime") + } +} diff --git a/internal/autocomplete/picker_fish_test.go b/internal/autocomplete/picker_fish_test.go new file mode 100644 index 00000000..477408aa --- /dev/null +++ b/internal/autocomplete/picker_fish_test.go @@ -0,0 +1,106 @@ +package autocomplete + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestPickerFishTabAndLifecycle(t *testing.T) { + directory := t.TempDir() + runPickerShellPTY(t, "fish", directory, "", "", ` +send -- "openai images generate\t" +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +send -- "\025printf 'AFTER_CANCEL\\n'\r" +expect -exact "AFTER_CANCEL\r\n" +expect -exact "PICKER_TEST> " +send -- "openai images gen\t" +expect -exact "generate" +send -- "\025openai_picker_disable; openai_picker_disable; bind --user \\t >\"\$PICKER_TEST_BINDING\" 2>/dev/null\r" +expect -exact "PICKER_TEST> " +send -- "exit\r"; expect eof +`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "launch fish 2\n\n\ntty:111\n", string(result)) + binding, err := os.ReadFile(filepath.Join(directory, "binding")) + require.NoError(t, err) + require.Empty(t, binding) +} + +func TestPickerFishBindingsAndResourcing(t *testing.T) { + for _, scenario := range []struct{ name, setup, probe, interaction, want string }{ + { + name: "custom binding", + setup: `fish_default_key_bindings +function custom_tab; printf 'fallback\n' >>"$PICKER_TEST_RESULT"; end +bind \t custom_tab`, + probe: `source "$PICKER_TEST_HOOK"`, + interaction: `send -- "other\t\025openai_picker_disable; openai_picker_disable; bind --user \\t >>\"\$PICKER_TEST_RESULT\"\r"`, + want: "fallback\nbind tab custom_tab\n", + }, + { + name: "replacement binding", + probe: `function new_tab; end +bind \t new_tab +source "$PICKER_TEST_HOOK" +openai_picker_disable +openai_picker_disable +bind --user \t >>"$PICKER_TEST_RESULT"`, + want: "bind tab new_tab\n", + }, + { + name: "disable then enable", + probe: `openai_picker_disable +source "$PICKER_TEST_HOOK"`, + interaction: `send -- "openai images generate\t" +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +send -- "\025printf 'DONE\\n'\r"`, + want: "launch fish 2\n\n\ntty:111\n", + }, + } { + t.Run(scenario.name, func(t *testing.T) { + directory := t.TempDir() + interaction := scenario.interaction + if interaction != "" { + interaction += "\nexpect -exact \"PICKER_TEST> \"\n" + } + interaction += `send -- "exit\r"; expect eof` + runPickerShellPTY(t, "fish", directory, scenario.setup, scenario.probe, interaction) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, scenario.want, string(result)) + }) + } +} + +func TestPickerFishRejectsOtherCommandLines(t *testing.T) { + directory := t.TempDir() + setup := `fish_default_key_bindings +function custom_tab; printf 'fallback\n' >>"$PICKER_TEST_RESULT"; end +bind \t custom_tab` + var interaction strings.Builder + for _, line := range []string{ + "openai images generate --help", "openai images generate >out", "openai images generate;", + "openai images generate | cat", "true; openai images generate", "'openai' images generate", + "OPENAI IMAGES GENERATE", "openai images generate $(touch unsafe)", + } { + interaction.WriteString("send -- {" + line + "}\nsend -- \"\\t\\025printf 'NEXT\\\\n'\\r\"\nexpect -exact \"NEXT\\r\\n\"\nexpect -exact \"PICKER_TEST> \"\n") + } + interaction.WriteString(`send -- "exit\r"; expect eof`) + runPickerShellPTY(t, "fish", directory, setup, "", interaction.String()) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, strings.Repeat("fallback\n", 8), string(result)) + _, err = os.Stat(filepath.Join(directory, "unsafe")) + require.ErrorIs(t, err, os.ErrNotExist) +} diff --git a/internal/autocomplete/picker_script.go b/internal/autocomplete/picker_script.go new file mode 100644 index 00000000..d5564f40 --- /dev/null +++ b/internal/autocomplete/picker_script.go @@ -0,0 +1,38 @@ +package autocomplete + +import ( + "fmt" + "strings" +) + +// Picker hooks share the completion script distribution, but are opt-in because +// they change an interactive key's behavior. They never edit startup files. +func renderPickerCompletion(shell CompletionStyle, appName string) (string, error) { + if appName == "" { + return "", fmt.Errorf("a command name is required for picker integration") + } + for i, r := range appName { + if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r == '_' || i > 0 && r >= '0' && r <= '9') { + return "", fmt.Errorf("picker integration requires a simple command name on PATH") + } + } + if shell == CompletionStylePowershell { + // PSReadLine can prefetch input before invoking a key handler. A child + // launched by that handler cannot consume it, so it can reach the shell + // after cancellation. Keep Tab in the line editor and use normal Enter. + return "", fmt.Errorf("PowerShell uses normal Tab completion. Type %s images generate and press Enter to open the image picker.", appName) + } + files := map[CompletionStyle]string{ + CompletionStyleBash: "bash_picker.bash", CompletionStyleZsh: "zsh_picker.zsh", + CompletionStyleFish: "fish_picker.fish", + } + name, ok := files[shell] + if !ok { + return "", fmt.Errorf("unsupported picker shell") + } + script, err := autoCompleteFS.ReadFile("shellscripts/" + name) + if err != nil { + return "", err + } + return strings.ReplaceAll(string(script), "__APPNAME__", appName), nil +} diff --git a/internal/autocomplete/picker_script_test.go b/internal/autocomplete/picker_script_test.go new file mode 100644 index 00000000..46308609 --- /dev/null +++ b/internal/autocomplete/picker_script_test.go @@ -0,0 +1,68 @@ +package autocomplete + +import ( + "bytes" + "context" + "testing" + + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +func TestPickerScriptRequiresSafeCommandName(t *testing.T) { + for _, name := range []string{"", "openai;echo bad", "../openai", "x'\n", "openai.exe", "$(id)", "雪", "1name"} { + script, err := renderPickerCompletion(CompletionStyleZsh, name) + require.Error(t, err, name) + require.Empty(t, script) + } + _, err := renderPickerCompletion("unknown", "openai") + require.Error(t, err) +} + +func TestPickerCompletionRequiresExplicitOptIn(t *testing.T) { + for _, shell := range []string{"bash", "zsh", "fish", "pwsh"} { + t.Run(shell, func(t *testing.T) { + for _, enabled := range []bool{false, true} { + var output bytes.Buffer + root := &cli.Command{Name: "openai", Writer: &output, Commands: []*cli.Command{{ + Name: "@completion", Flags: []cli.Flag{&cli.BoolFlag{Name: "picker"}}, Action: OutputCompletionScript, + }}, ExitErrHandler: func(context.Context, *cli.Command, error) {}} + args := []string{"openai", "@completion", shell} + if enabled { + args = append(args, "--picker") + } + err := root.Run(context.Background(), args) + if shell == "pwsh" && enabled { + require.EqualError(t, err, "PowerShell uses normal Tab completion. Type openai images generate and press Enter to open the image picker.") + var exit cli.ExitCoder + require.ErrorAs(t, err, &exit) + require.Equal(t, 1, exit.ExitCode()) + require.Empty(t, output.String(), "unsupported setup must not emit an executable partial script") + continue + } + require.NoError(t, err) + standard, err := shellCompletions[CompletionStyle(shell)](root, "openai") + require.NoError(t, err) + if enabled { + hook, err := renderPickerCompletion(CompletionStyle(shell), "openai") + require.NoError(t, err) + require.Equal(t, standard+"\n"+hook, output.String()) + require.Contains(t, output.String(), "openai_picker_disable") + require.NotContains(t, output.String(), "__APPNAME__") + } else { + require.Equal(t, standard, output.String()) + } + } + }) + } +} + +func TestPickerScriptFailureDoesNotEmitPartialSetup(t *testing.T) { + var output bytes.Buffer + root := &cli.Command{Name: "unsafe;name", Writer: &output, Commands: []*cli.Command{{ + Name: "@completion", Flags: []cli.Flag{&cli.BoolFlag{Name: "picker"}}, Action: OutputCompletionScript, + }}, ExitErrHandler: func(context.Context, *cli.Command, error) {}} + err := root.Run(context.Background(), []string{"unsafe;name", "@completion", "bash", "--picker"}) + require.Error(t, err) + require.Empty(t, output.String()) +} diff --git a/internal/autocomplete/shellscripts/bash_autocomplete.bash b/internal/autocomplete/shellscripts/bash_autocomplete.bash index 95183934..0f4504a5 100755 --- a/internal/autocomplete/shellscripts/bash_autocomplete.bash +++ b/internal/autocomplete/shellscripts/bash_autocomplete.bash @@ -2,7 +2,7 @@ ____APPNAME___bash_autocomplete() { if [[ "${COMP_WORDS[0]}" != "source" ]]; then - local cur completions exit_code + local cur completions exit_code file local IFS=$'\n' cur="${COMP_WORDS[COMP_CWORD]}" @@ -49,16 +49,27 @@ ____APPNAME___bash_autocomplete() { fi if [[ "$force_file_completion" == true ]]; then - local file COMPREPLY=() while IFS= read -r file; do COMPREPLY+=("$prefix$file") done < <(compgen -f -- "$file_part") else case $exit_code in - 10) mapfile -t COMPREPLY < <(compgen -f -- "$cur") ;; # file completion - 11) COMPREPLY=() ;; # no completion - 0) mapfile -t COMPREPLY <<<"$completions" ;; # use returned completions + 10) # File completion, including Bash 3.2 (which has no mapfile). + COMPREPLY=() + while IFS= read -r file; do + COMPREPLY+=("$file") + done < <(compgen -f -- "$cur") + ;; + 11) COMPREPLY=() ;; # no completion + 0) + COMPREPLY=() + if [[ -n "$completions" ]]; then + while IFS= read -r file; do + COMPREPLY+=("$file") + done <<<"$completions" + fi + ;; esac fi return 0 diff --git a/internal/autocomplete/shellscripts/bash_picker.bash b/internal/autocomplete/shellscripts/bash_picker.bash new file mode 100644 index 00000000..d97b009b --- /dev/null +++ b/internal/autocomplete/shellscripts/bash_picker.bash @@ -0,0 +1,120 @@ +# Optional image picker integration. Source the output of +# __APPNAME__ @completion bash --picker in an interactive terminal. +if [[ $- == *i* && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]]; then + if (( BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 3) )); then + : # Keep ordinary completion on older Bash without startup diagnostics. + else + ____APPNAME___picker_binding() { + local line prefix="\"$2\" " + while IFS= read -r line; do + if [ "${line%%: *}" = "\"$2\"" ]; then + printf '%s\n' "$line" + return + fi + # bind -X omits the colon used by bind -p and bind -s. + if [ "${line#"$prefix"}" != "$line" ]; then + printf '"%s": %s\n' "$2" "${line#"$prefix"}" + return + fi + done < <({ bind -m "$1" -p; bind -m "$1" -s; bind -m "$1" -X; } 2>/dev/null) + } + + ____APPNAME___picker_matches() { + local IFS=$' \t' + local -a words + [[ ${COMP_LINE-} != *$'\n'* && ${COMP_POINT-0} -eq ${#COMP_LINE} ]] || return 1 + read -r -a words <<<"$COMP_LINE" + # `test` keeps exact case even with the user's nocasematch option set. + [ ${#words[@]} -eq 3 ] && [ "${words[0]}" = '__APPNAME__' ] && + [ "${words[1]}" = images ] && [ "${words[2]}" = generate ] && + [ "$(type -t '__APPNAME__')" = file ] + } + + ____APPNAME___picker_redraw() { + local keymap=${____APPNAME___picker_active_keymap-} + if [ -n "$keymap" ] && + [ "$(____APPNAME___picker_binding "$keymap" '\e[99;2~')" = '"\e[99;2~": "____APPNAME___picker_redraw"' ]; then + bind -m "$keymap" '"\e[99;2~": ""' + fi + # COMP_LINE omits shell syntax before this command (assignments, pipes, + # and command separators). Only bind -x exposes the entire editor buffer. + local COMP_LINE=${READLINE_LINE-} COMP_POINT=${READLINE_POINT-0} + if [[ ${____APPNAME___picker_enabled-} == 1 && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]] && + ____APPNAME___picker_matches; then + # Normal completion may have added a suffix. Cancel restores the line + # that requested the picker, including its original cursor position. + local leading=${READLINE_LINE%%[!$' \t']*} + local candidate=$____APPNAME___picker_candidate_line + candidate=${candidate#"${candidate%%[!$' \t']*}"} + READLINE_LINE=$leading$candidate + READLINE_POINT=${#READLINE_LINE} + printf '\n' + OPENAI_PICKER_SHELL=bash command '__APPNAME__' images generate <&2 + fi + # Returning from bind -x asks Readline to redraw its own prompt/buffer. + } + + ____APPNAME___picker_complete() { + local keymap=vi-insertion + local binding + [[ -o emacs ]] && keymap=emacs-standard + if [[ ${____APPNAME___picker_enabled-} == 1 && ${COMP_TYPE-} == 9 && ${COMP_KEY-} == 126 && + -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]] && + [ "${____APPNAME___picker_keymaps[$keymap]-}" = 1 ] && + [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": "\e[99;1~\e[99;2~"' ] && + [ "$(____APPNAME___picker_binding "$keymap" '\e[99;1~')" = '"\e[99;1~": complete' ] && + ____APPNAME___picker_matches; then + binding=$(____APPNAME___picker_binding "$keymap" '\e[99;2~') + if [ "$binding" = '"\e[99;2~": ""' ] || [ "$binding" = '"\e[99;2~": "____APPNAME___picker_redraw"' ]; then + ____APPNAME___picker_active_keymap=$keymap + ____APPNAME___picker_candidate_line=$COMP_LINE + bind -m "$keymap" -x '"\e[99;2~": ____APPNAME___picker_redraw' + fi + fi + ____APPNAME___bash_autocomplete "$@" + } + + __APPNAME___picker_disable() { + ____APPNAME___picker_enabled=0 + local keymap binding + if [ "$(complete -p '__APPNAME__' 2>/dev/null)" = 'complete -o filenames -F ____APPNAME___picker_complete __APPNAME__' ]; then + complete -o filenames -F ____APPNAME___bash_autocomplete '__APPNAME__' + fi + for keymap in emacs-standard vi-insertion; do + [ "${____APPNAME___picker_keymaps[$keymap]-}" = 1 ] || continue + if [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": "\e[99;1~\e[99;2~"' ]; then + bind -m "$keymap" '"\C-i": complete' + if [ "$(____APPNAME___picker_binding "$keymap" '\e[99;1~')" = '"\e[99;1~": complete' ]; then + bind -m "$keymap" -r '\e[99;1~' + fi + binding=$(____APPNAME___picker_binding "$keymap" '\e[99;2~') + if [ "$binding" = '"\e[99;2~": ""' ] || [ "$binding" = '"\e[99;2~": "____APPNAME___picker_redraw"' ]; then + bind -m "$keymap" -r '\e[99;2~' + fi + unset '____APPNAME___picker_keymaps[$keymap]' + fi + done + } + + ____APPNAME___picker_install() { + local keymap + declare -gA ____APPNAME___picker_keymaps + for keymap in emacs-standard vi-insertion; do + [ "${____APPNAME___picker_keymaps[$keymap]-}" = 1 ] && continue + # Install only around ordinary completion and only into unused keys. + # Custom Tab bindings and later replacements remain owned by their user. + if [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": complete' ] && + [ -z "$(____APPNAME___picker_binding "$keymap" '\e[99;1~')" ] && + [ -z "$(____APPNAME___picker_binding "$keymap" '\e[99;2~')" ]; then + bind -m "$keymap" '"\e[99;1~": complete' + bind -m "$keymap" '"\e[99;2~": ""' + bind -m "$keymap" '"\C-i": "\e[99;1~\e[99;2~"' + ____APPNAME___picker_keymaps[$keymap]=1 + fi + done + ____APPNAME___picker_enabled=1 + complete -o filenames -F ____APPNAME___picker_complete '__APPNAME__' + } + ____APPNAME___picker_install + fi +fi diff --git a/internal/autocomplete/shellscripts/fish_picker.fish b/internal/autocomplete/shellscripts/fish_picker.fish new file mode 100644 index 00000000..96c065f5 --- /dev/null +++ b/internal/autocomplete/shellscripts/fish_picker.fish @@ -0,0 +1,120 @@ +# Optional image picker. Source after the ordinary fish completions. +# Keep custom bindings as shell-owned commands; never evaluate the input buffer. +if not status is-interactive; or not isatty stdin; or not isatty stdout; or not isatty stderr + return +end +if test "$TERM" = dumb; or test -z "$TERM" + return +end +if set -q __openai_picker_modes + return +end + +function openai_picker_disable + if not set -q __openai_picker_modes + return + end + for slot in (seq (count $__openai_picker_modes)) + set -l wrapper $__openai_picker_wrappers[$slot] + set -g $wrapper\_active 0 + set -l mode $__openai_picker_modes[$slot] + set -l owned_name __openai_picker_owned_$slot + set -l prior_name __openai_picker_prior_$slot + if test "$(bind --user --mode $mode \t 2>/dev/null | string collect)" = "$$owned_name" + bind --erase --user --mode $mode \t + if test -n "$$prior_name" + printf '%s\n' $$prior_name | source + end + functions --erase $wrapper + set --erase --global $wrapper\_active + end + set --erase --global __openai_picker_owned_$slot __openai_picker_prior_$slot + end + set --erase --global __openai_picker_modes __openai_picker_wrappers +end + +# fish initializes its default bindings lazily. Resolve them before saving Tab. +if not set -q fish_key_bindings + fish_default_key_bindings +end +set -g __openai_picker_modes +set -g __openai_picker_wrappers +if not set -q __openai_picker_serial + set -g __openai_picker_serial 0 +end +for mode in default insert + set -l prior (bind --user --mode $mode \t 2>/dev/null | string collect) + set -l binding "$prior" + if test -z "$binding" + set binding (bind --preset --mode $mode \t 2>/dev/null | string collect) + end + if test -z "$binding" + continue + end + # bind emits shell-escaped words. Tokenize them without executing anything. + set -l words + printf '%s\n' "$binding" | read --tokenize --array words + set -l position 2 + set -l next_mode '' + while string match --quiet -- '-*' "$words[$position]" + switch $words[$position] + case -M --mode + set position (math $position + 2) + case -m --sets-mode + set next_mode $words[(math $position + 1)] + set position (math $position + 2) + case --preset --user + set position (math $position + 1) + case '*' + break + end + end + set -l commands $words[(math $position + 1)..-1] + if test (count $commands) -eq 0 + continue + end + set -l input_functions (bind --function-names) + set -l all_input 1 + for original_command in $commands + if not contains -- "$original_command" $input_functions + set all_input 0 + end + end + set -g --append __openai_picker_modes $mode + set -l slot (count $__openai_picker_modes) + set -g __openai_picker_prior_$slot "$prior" + set -g __openai_picker_serial (math $__openai_picker_serial + 1) + set -l wrapper __openai_picker_tab_$__openai_picker_serial + set -l active_name $wrapper\_active + set -g $active_name 1 + set -g --append __openai_picker_wrappers $wrapper + function $wrapper --inherit-variable commands --inherit-variable all_input --inherit-variable next_mode --inherit-variable active_name + # Keep commandline's one output newline in the pattern. Removing it via + # command substitution would also trim newlines in the editor buffer. + set -l buffer (commandline --current-buffer | string collect --no-trim-newlines) + set -l app (string escape --style=regex -- '__APPNAME__') + if test "$$active_name" = 1; and isatty stdin; and isatty stdout; and isatty stderr; and test "$TERM" != dumb; and test (commandline --cursor) -eq (math (string length -- "$buffer") - 1); and string match --quiet --regex -- "\\A[ \\t]*"$app"[ \\t]+images[ \\t]+generate[ \\t]*\\n\\z" "$buffer"; and test "$(type --type __APPNAME__ 2>/dev/null)" = file + # The current command resolves to an external executable. + printf '\n' + env OPENAI_PICKER_SHELL=fish __APPNAME__ images generate + commandline --function repaint + return + end + + if test "$all_input" = 1 + commandline --function $commands + else + # These are the original, trusted fish binding commands. They are not + # commandline text, completions, or output from the CLI. + for original_command in $commands + eval $original_command + end + end + if test -n "$next_mode" + set -g fish_bind_mode $next_mode + end + end + + bind --user --mode $mode \t $wrapper + set -g __openai_picker_owned_$slot (bind --user --mode $mode \t | string collect) +end diff --git a/internal/autocomplete/shellscripts/zsh_picker.zsh b/internal/autocomplete/shellscripts/zsh_picker.zsh new file mode 100644 index 00000000..5dd6b8f8 --- /dev/null +++ b/internal/autocomplete/shellscripts/zsh_picker.zsh @@ -0,0 +1,97 @@ +# Optional image picker integration. Source after normal completion setup. +if [[ -o interactive && -o zle && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]]; then + ____APPNAME___picker_matches() { + emulate -L zsh + setopt casematch + local pattern=$'^[ \t]*__APPNAME__[ \t]+images[ \t]+generate[ \t]*$' + [[ $BUFFER =~ $pattern && $CURSOR -eq ${#BUFFER} && $(whence -w '__APPNAME__') == '__APPNAME__: command' ]] + } + + ____APPNAME___picker_owns_tab() { + emulate -L zsh + local binding=$(bindkey -M "${KEYMAP:-main}" '^I') + [[ $binding == '"^I" ____APPNAME___picker_tab_emacs' || $binding == '"^I" ____APPNAME___picker_tab_viins' ]] + } + + ____APPNAME___picker_clear_hint() { + emulate -L zsh + if [[ ${____APPNAME___picker_hint_visible-} == 1 && ${POSTDISPLAY-} == ' [Tab: image options]' ]]; then + POSTDISPLAY='' + fi + typeset -g ____APPNAME___picker_hint_visible=0 + } + + ____APPNAME___picker_hint() { + emulate -L zsh + ____APPNAME___picker_clear_hint + if [[ ${____APPNAME___picker_enabled-} == 1 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb && -z $POSTDISPLAY ]] && + ____APPNAME___picker_owns_tab && ____APPNAME___picker_matches; then + POSTDISPLAY=' [Tab: image options]' + typeset -g ____APPNAME___picker_hint_visible=1 + fi + } + + ____APPNAME___picker_tab() { + emulate -L zsh + if [[ ${____APPNAME___picker_enabled-} == 1 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]] && + ____APPNAME___picker_owns_tab && ____APPNAME___picker_matches; then + ____APPNAME___picker_clear_hint + # ZLE redirects stdin away from the terminal while running widgets. + # Stderr was verified above, so duplicate that terminal for the child. + zle -I + OPENAI_PICKER_SHELL=zsh command '__APPNAME__' images generate <&2 + zle reset-prompt + return 0 + fi + zle "____APPNAME___picker_previous_$1" -- "${@:2}" + } + + ____APPNAME___picker_tab_emacs() { ____APPNAME___picker_tab emacs "$@"; } + ____APPNAME___picker_tab_viins() { ____APPNAME___picker_tab viins "$@"; } + + __APPNAME___picker_disable() { + emulate -L zsh + typeset -g ____APPNAME___picker_enabled=0 + ____APPNAME___picker_clear_hint + local keymap binding + for keymap in emacs viins; do + binding=$(bindkey -M "$keymap" '^I') + if [[ $binding == '"^I" ____APPNAME___picker_tab_'$keymap ]]; then + bindkey -M "$keymap" '^I' "${____APPNAME___picker_bindings[$keymap]}" + fi + done + add-zle-hook-widget -d line-pre-redraw ____APPNAME___picker_hint + add-zle-hook-widget -d line-finish ____APPNAME___picker_clear_hint + # Leave private widget aliases available for plugins that chained them. + # They delegate to the saved widget while the picker is disabled. + } + + () { + emulate -L zsh + zmodload zsh/zleparameter || return + autoload -Uz add-zle-hook-widget + typeset -gA ____APPNAME___picker_bindings + local keymap original + local -a binding + for keymap in emacs viins; do + binding=(${(z)$(bindkey -M "$keymap" '^I')}) + original=${(Q)binding[2]} + # Capture a delegate once. Recapturing a newer plugin that calls our + # wrapper would introduce a cycle when integration is enabled again. + if [[ -n ${____APPNAME___picker_bindings[$keymap]-} ]]; then + if [[ $original == ${____APPNAME___picker_bindings[$keymap]} && ${____APPNAME___picker_enabled-} != 1 ]]; then + bindkey -M "$keymap" '^I' "____APPNAME___picker_tab_$keymap" + fi + continue + fi + [[ -n ${widgets[$original]-} && $original != ____APPNAME___picker_tab_* ]] || continue + zle -A "$original" "____APPNAME___picker_previous_$keymap" || continue + ____APPNAME___picker_bindings[$keymap]=$original + zle -N "____APPNAME___picker_tab_$keymap" + bindkey -M "$keymap" '^I' "____APPNAME___picker_tab_$keymap" + done + typeset -g ____APPNAME___picker_enabled=1 + add-zle-hook-widget line-pre-redraw ____APPNAME___picker_hint + add-zle-hook-widget line-finish ____APPNAME___picker_clear_hint + } +fi diff --git a/pkg/custom/command.go b/pkg/custom/command.go index 06fca1a7..10b18258 100644 --- a/pkg/custom/command.go +++ b/pkg/custom/command.go @@ -20,6 +20,7 @@ func ConfigureCommand(root *cli.Command) { registerImageModels(root) configureReadableOutput(root) configureImageSaving(root) + configureImagePickerCompletion(root) registerImagePreviewCommands(root) configureReadableAudio(root) configureReadableSpeech(root) diff --git a/pkg/custom/image_picker_completion.go b/pkg/custom/image_picker_completion.go new file mode 100644 index 00000000..b283e6a1 --- /dev/null +++ b/pkg/custom/image_picker_completion.go @@ -0,0 +1,23 @@ +package custom + +import ( + "slices" + + "github.com/urfave/cli/v3" +) + +// Keep the optional current-session hook on the existing completion command. +func configureImagePickerCompletion(root *cli.Command) { + completion := root.Command("@completion") + if completion == nil { + return + } + for _, flag := range completion.Flags { + if slices.Contains(flag.Names(), "picker") { + return + } + } + completion.Flags = append(completion.Flags, + &cli.BoolFlag{Name: "picker", Usage: "Include the optional Tab shortcut for the image picker"}, + ) +} diff --git a/pkg/custom/local_errors.go b/pkg/custom/local_errors.go index 7ba11978..89237b7f 100644 --- a/pkg/custom/local_errors.go +++ b/pkg/custom/local_errors.go @@ -92,6 +92,7 @@ func knownLocalError(command *cli.Command, message string) string { "cannot read from stdin: stdin is already being used for the request body", "cannot read from stdin: stdin was already consumed by piped YAML/JSON input", "Setup help takes no additional arguments.", + "PowerShell uses normal Tab completion. Type openai images generate and press Enter to open the image picker.", "COMPLETION_STYLE must be set to 'bash', 'zsh', 'pwsh', or 'fish'", "COMPLETION_STYLE must be set to 'bash', 'zsh', 'pwsh', 'fish'": return message From 4727819eb423377f48ae84fdbdc1d06d10b1100f Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 17:13:06 -0700 Subject: [PATCH 6/7] Clean up picker process test imports and cancellation comments --- scripts/check-image-picker-integration.py | 1 - scripts/image_picker_harness.py | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/check-image-picker-integration.py b/scripts/check-image-picker-integration.py index 20018366..56a6d943 100644 --- a/scripts/check-image-picker-integration.py +++ b/scripts/check-image-picker-integration.py @@ -12,7 +12,6 @@ import shutil import signal import subprocess -import sys import tempfile import threading import time diff --git a/scripts/image_picker_harness.py b/scripts/image_picker_harness.py index aec4f23b..ea952365 100644 --- a/scripts/image_picker_harness.py +++ b/scripts/image_picker_harness.py @@ -6,7 +6,6 @@ import http.server import json import os -import pathlib import pty import re import select @@ -15,9 +14,7 @@ import struct import subprocess import sys -import tempfile import termios -import threading import time PNG = base64.b64decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP438AAAAQBAYDFKhhdAAAAAElFTkSuQmCC') @@ -113,6 +110,7 @@ def close(self): try: os.killpg(self.child.pid, signal.SIGKILL) except ProcessLookupError: + # The process can exit between poll() and killpg(). pass finally: # Darwin can keep a session leader exiting until the controlling @@ -149,6 +147,7 @@ def do_POST(self): try: self.wfile.write(data) except (BrokenPipeError, ConnectionResetError): + # Cancellation tests intentionally disconnect before the reply. pass From 8f1358463f00becdf37928ca4e723be864a289c6 Mon Sep 17 00:00:00 2001 From: Sai Guvvala Date: Fri, 2 Oct 2026 21:21:13 -0700 Subject: [PATCH 7/7] Preserve Bash completion keys and restore picker bindings --- internal/autocomplete/picker_bash_zsh_test.go | 71 +++++++++++++++++-- .../shellscripts/bash_picker.bash | 46 +++++++----- 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/internal/autocomplete/picker_bash_zsh_test.go b/internal/autocomplete/picker_bash_zsh_test.go index 813c75a1..eee2050e 100644 --- a/internal/autocomplete/picker_bash_zsh_test.go +++ b/internal/autocomplete/picker_bash_zsh_test.go @@ -182,6 +182,67 @@ complete -p openai >>"$PICKER_TEST_RESULT" require.Equal(t, strings.Repeat("complete -F newer_completion openai\n", 2), string(result)) } +func TestPickerBashPreservesOtherCompletionKey(t *testing.T) { + requirePickerBash(t, "bash") + for _, mode := range []string{"emacs", "vi"} { + t.Run(mode, func(t *testing.T) { + directory := t.TempDir() + setup := "set -o " + mode + ` +other_completion() { + printf '%s %s\n' "$COMP_KEY" "$COMP_TYPE" >>"$PICKER_TEST_RESULT" + COMPREPLY=() + if [[ $COMP_KEY == 9 ]]; then COMPREPLY=(tab-result); fi +} +complete -F other_completion other +` + runPickerShellPTY(t, "bash", directory, setup, "", ` +send -- "other tab\t" +expect -exact "tab-result " +send -- "\025exit\r"; expect eof +`) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, "9 9\n", string(result)) + }) + } +} + +func TestPickerBashReenableAfterTemporaryReplacement(t *testing.T) { + requirePickerBash(t, "bash") + for _, mode := range []string{"emacs", "vi"} { + t.Run(mode, func(t *testing.T) { + directory := t.TempDir() + probe := ` +for keymap in emacs-standard vi-insert; do bind -m "$keymap" '"\C-i": "temporary"'; done +openai_picker_disable +for keymap in emacs-standard vi-insert; do + __openai_picker_binding "$keymap" '\C-i' >>"$PICKER_TEST_BINDING" + bind -m "$keymap" '"\C-i": complete' +done +source "$PICKER_TEST_HOOK" +` + runPickerShellPTY(t, "bash", directory, "set -o "+mode, probe, ` +send -- "openai images generate\t" +expect -exact "PICKER_LAUNCHED" +send -- "cancel\n" +expect -exact "PICKER_CANCELED" +expect -exact "PICKER_TEST> " +send -- "\025openai images gen\t" +expect -exact "generate " +send -- "\025openai_picker_disable\r" +expect -exact "PICKER_TEST> " +send -- "exit\r"; expect eof +`) + binding, err := os.ReadFile(filepath.Join(directory, "binding")) + require.NoError(t, err) + require.Equal(t, strings.Repeat("\"\\C-i\": \"temporary\"\n", 2), string(binding)) + result, err := os.ReadFile(filepath.Join(directory, "result")) + require.NoError(t, err) + require.Equal(t, 1, strings.Count(string(result), "launch ")) + }) + } +} + func TestPickerBashBindingsAndResourcing(t *testing.T) { requirePickerBash(t, "bash") for _, scenario := range []struct { @@ -203,11 +264,11 @@ __openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, }, { name: "occupied private key", - setup: `bind -x '"\e[99;1~": printf owned'`, + setup: `bind -x '"\e[99;1\C-i": printf owned'`, probe: `__openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT" openai_picker_disable -__openai_picker_binding emacs-standard '\e[99;1~' >>"$PICKER_TEST_RESULT"`, - want: "\"\\C-i\": complete\n\"\\e[99;1~\": \"printf owned\"\n", +__openai_picker_binding emacs-standard '\e[99;1\C-i' >>"$PICKER_TEST_RESULT"`, + want: "\"\\C-i\": complete\n\"\\e[99;1\\C-i\": \"printf owned\"\n", }, { name: "replacement after install", @@ -219,7 +280,7 @@ __openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, { name: "replacement private callback", probe: `bind -x '"\e[99;2~": printf newer' -COMP_LINE='openai images generate'; COMP_POINT=${#COMP_LINE}; COMP_TYPE=9; COMP_KEY=126 +COMP_LINE='openai images generate'; COMP_POINT=${#COMP_LINE}; COMP_TYPE=9; COMP_KEY=9 COMP_WORDS=(openai images generate); COMP_CWORD=2 __openai_picker_complete openai_picker_disable @@ -230,7 +291,7 @@ __openai_picker_binding emacs-standard '\e[99;2~' >>"$PICKER_TEST_RESULT"`, name: "repeated install disable enable", probe: `source "$PICKER_TEST_HOOK"; openai_picker_disable __openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT" -__openai_picker_binding emacs-standard '\e[99;1~' >>"$PICKER_TEST_RESULT" +__openai_picker_binding emacs-standard '\e[99;1\C-i' >>"$PICKER_TEST_RESULT" __openai_picker_binding emacs-standard '\e[99;2~' >>"$PICKER_TEST_RESULT" source "$PICKER_TEST_HOOK"; openai_picker_disable __openai_picker_binding emacs-standard '\C-i' >>"$PICKER_TEST_RESULT"`, diff --git a/internal/autocomplete/shellscripts/bash_picker.bash b/internal/autocomplete/shellscripts/bash_picker.bash index d97b009b..a6f8e7ff 100644 --- a/internal/autocomplete/shellscripts/bash_picker.bash +++ b/internal/autocomplete/shellscripts/bash_picker.bash @@ -55,14 +55,14 @@ if [[ $- == *i* && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]]; t } ____APPNAME___picker_complete() { - local keymap=vi-insertion + local keymap=vi-insert local binding [[ -o emacs ]] && keymap=emacs-standard - if [[ ${____APPNAME___picker_enabled-} == 1 && ${COMP_TYPE-} == 9 && ${COMP_KEY-} == 126 && + if [[ ${____APPNAME___picker_enabled-} == 1 && ${COMP_TYPE-} == 9 && ${COMP_KEY-} == 9 && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]] && [ "${____APPNAME___picker_keymaps[$keymap]-}" = 1 ] && - [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": "\e[99;1~\e[99;2~"' ] && - [ "$(____APPNAME___picker_binding "$keymap" '\e[99;1~')" = '"\e[99;1~": complete' ] && + [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": "\e[99;1\C-i\e[99;2~"' ] && + [ "$(____APPNAME___picker_binding "$keymap" '\e[99;1\C-i')" = '"\e[99;1\C-i": complete' ] && ____APPNAME___picker_matches; then binding=$(____APPNAME___picker_binding "$keymap" '\e[99;2~') if [ "$binding" = '"\e[99;2~": ""' ] || [ "$binding" = '"\e[99;2~": "____APPNAME___picker_redraw"' ]; then @@ -80,12 +80,12 @@ if [[ $- == *i* && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]]; t if [ "$(complete -p '__APPNAME__' 2>/dev/null)" = 'complete -o filenames -F ____APPNAME___picker_complete __APPNAME__' ]; then complete -o filenames -F ____APPNAME___bash_autocomplete '__APPNAME__' fi - for keymap in emacs-standard vi-insertion; do + for keymap in emacs-standard vi-insert; do [ "${____APPNAME___picker_keymaps[$keymap]-}" = 1 ] || continue - if [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": "\e[99;1~\e[99;2~"' ]; then + if [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": "\e[99;1\C-i\e[99;2~"' ]; then bind -m "$keymap" '"\C-i": complete' - if [ "$(____APPNAME___picker_binding "$keymap" '\e[99;1~')" = '"\e[99;1~": complete' ]; then - bind -m "$keymap" -r '\e[99;1~' + if [ "$(____APPNAME___picker_binding "$keymap" '\e[99;1\C-i')" = '"\e[99;1\C-i": complete' ]; then + bind -m "$keymap" -r '\e[99;1\C-i' fi binding=$(____APPNAME___picker_binding "$keymap" '\e[99;2~') if [ "$binding" = '"\e[99;2~": ""' ] || [ "$binding" = '"\e[99;2~": "____APPNAME___picker_redraw"' ]; then @@ -97,18 +97,28 @@ if [[ $- == *i* && -t 0 && -t 1 && -t 2 && -n ${TERM-} && ${TERM-} != dumb ]]; t } ____APPNAME___picker_install() { - local keymap + local keymap tab completion redraw owned declare -gA ____APPNAME___picker_keymaps - for keymap in emacs-standard vi-insertion; do - [ "${____APPNAME___picker_keymaps[$keymap]-}" = 1 ] && continue - # Install only around ordinary completion and only into unused keys. - # Custom Tab bindings and later replacements remain owned by their user. - if [ "$(____APPNAME___picker_binding "$keymap" '\C-i')" = '"\C-i": complete' ] && - [ -z "$(____APPNAME___picker_binding "$keymap" '\e[99;1~')" ] && - [ -z "$(____APPNAME___picker_binding "$keymap" '\e[99;2~')" ]; then - bind -m "$keymap" '"\e[99;1~": complete' + for keymap in emacs-standard vi-insert; do + tab=$(____APPNAME___picker_binding "$keymap" '\C-i') + completion=$(____APPNAME___picker_binding "$keymap" '\e[99;1\C-i') + redraw=$(____APPNAME___picker_binding "$keymap" '\e[99;2~') + owned=${____APPNAME___picker_keymaps[$keymap]-} + # A previous installation may have lost Tab to another owner. Only + # treat exact, still-owned bindings as available for reinstallation. + if [ "$owned" = 1 ]; then + [ "$tab" = '"\C-i": "\e[99;1\C-i\e[99;2~"' ] && tab='"\C-i": complete' + [ "$completion" = '"\e[99;1\C-i": complete' ] && completion='' + if [ "$redraw" = '"\e[99;2~": ""' ] || [ "$redraw" = '"\e[99;2~": "____APPNAME___picker_redraw"' ]; then + redraw='' + fi + fi + if [ "$tab" = '"\C-i": complete' ] && [ -z "$completion" ] && [ -z "$redraw" ]; then + # End the completion sequence in Tab so every command's completion + # receives the ordinary COMP_KEY=9, including unrelated commands. + bind -m "$keymap" '"\e[99;1\C-i": complete' bind -m "$keymap" '"\e[99;2~": ""' - bind -m "$keymap" '"\C-i": "\e[99;1~\e[99;2~"' + bind -m "$keymap" '"\C-i": "\e[99;1\C-i\e[99;2~"' ____APPNAME___picker_keymaps[$keymap]=1 fi done