Repository navigation
feat(cli): include fish picker shortcuts in Linux packages - #361
Conversation
2902f6a to
a9b8ba4
Compare
0ba000c to
7a38028
Compare
saioai
changed the title
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 6 mixed files remain; 0 existing customizations changed. Compared 6 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 37170209978 --repo openai/openai-cli \
--name castiron-custom-code-37170209978-1 --dir /tmp/castiron-custom-code-37170209978-1
git apply --stat /tmp/castiron-custom-code-37170209978-1/custom-code.patch
cat /tmp/castiron-custom-code-37170209978-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 9e1d6e2971f00d65724c308d176ee2dffc702008 1f13f2d6e0645e00c693a5e5df09feffe8e01a6a
python3 scripts/castiron/custom_code_report.py report \
--base 9e1d6e2971f00d65724c308d176ee2dffc702008 \
--head 1f13f2d6e0645e00c693a5e5df09feffe8e01a6a --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-1f13f2d6e064
cat /tmp/castiron-custom-code-1f13f2d6e064/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a380282fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Please review the updated Linux-only head 9b120b5. Package Tab now rechecks executable identity, package startup leaves ordinary completion untouched, and identity checks use external test. Focused regressions fail against the previous commit and pass after the fixes; personal shell scripts remain byte-identical. CI also exposed a pre-existing stopped-worker test race, now fixed by confirming SIGSTOP before testing forced cleanup. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b120b5304
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review the final follow-up at 45df354. The Linux package uses the system identity-check utility and preserves normal completion when it is unavailable. Focused regression checks and independent native shell review pass; the package-only scope and personal shell output are preserved. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45df3544f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review Please review head 1f13f2d. The latest test-only correction skips the native fish package probe when Expect is unavailable unless fish testing is explicitly required. Independent process probes reproduce the previous failure and verify both optional and required behavior. All 17 package scenarios pass with dependencies present; production code is unchanged from the preceding reviewed head. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 1f13f2d6e0645e00c693a5e5df09feffe8e01a6a. No blocking findings.
The Linux package reuses the existing first-prompt activation and disable handlers. It preserves ordinary completion and opt-outs, and checks the selected executable again on each Tab press. The release job creates and checks the extra file before receiving publishing credentials. Older tags keep their original four inputs. The stopped-worker test now waits for the stop notification before testing forced cleanup.
I reviewed all 17 changed files and relevant surrounding code. Hosted checks on this commit passed: 22 successful, three skipped. I did not run tests, builds, or native fish. Real Linux installs, upgrades, and older fish versions remain unverified here.
For people previewing images in Kitty or Ghostty, Ctrl-C during an image could dump encoded text into the terminal and break the next preview. This stops interrupted output safely, preserves saved images, and keeps repeated picker generations working when an update replaces the CLI. ### What changes - Own the terminal writer directly so helper crashes cannot leave an orphan writing after cancellation. - Prepare two workers before input or API waits. Normal images reuse one; the idle reserve can reset an interrupted transfer even after executable replacement. - Wait for active output before returning to text or the picker. Terminate and reap failed writers before recovery, using two-second reset and shutdown deadlines. Preserve ordinary Ctrl-C flushing and terminal settings. - Ignore failures from an unused reserve after successful output. Report failures when a worker is actually needed. - Keep existing platform support. System `cat` is no longer required. - Honor cancellation before fallback cleanup writes, including partial-frame cleanup, so cancellation cannot start another uncontrolled write. - Make CodeQL validate the current PR merge and its parents; its trusted publisher is unchanged. ### Commands No new public commands or flags. ```sh openai images preview --inline on image.png # preview an existing image openai images generate # generate and return to the picker ``` Inline settings, progress previews, explicit formats, extraction and piped output retain their behavior. Neither openai#360 nor openai#361 is required. ### Code `pkg/custom` owns command and picker lifetime. The existing `internal/terminalimage` package owns writer processes, cancellation and protocol recovery. Only those isolated workers change their SIGINT disposition. `main.go` adds early helper dispatch; generated commands and transformers are unchanged. No new packages or module dependencies. On macOS and Linux, two helpers remain resident during eligible command input and API waits. A failed worker is retired; the command reports the failure and exits after cleanup. Other platforms and custom writers use direct output: cancellation stops additional writes, but cannot interrupt an already blocked writer or guarantee protocol repair. Normal output has no new timeout or payload limit. ### Tested - Two independent adversarial reviews pass on `ee7b882`. The fallback hang reproduces before the fix; six independent cancellation cases pass after it. Uncanceled cleanup errors and retry behavior remain covered. - 24 focused race-test groups pass on Mac arm64; the same 24 groups pass in native Linux amd64 and 386 processes. Fresh public CLI checks cover helper death, cancellation, saved originals, executable replacement and repeated picker generations. - Windows fallback tests compile; hosted shell checks execute on Windows, macOS and Linux. Other architecture checks are compilation only. Earlier Linux amd64 typed Ctrl-C trials passed 20/20 on the unchanged native worker implementation. - [CI](https://github.com/openai/openai-cli/actions/runs/37154504298), [native shell help](https://github.com/openai/openai-cli/actions/runs/37154504215) and [CodeQL](https://github.com/openai/openai-cli/actions/runs/37154504325) all pass on `ee7b882`, including artifact builds, tests and native shell checks. The [trusted main budget](https://github.com/openai/openai-cli/actions/runs/37154525348) passes at 435/1000. The scoped native Ghostty capture and two independent visual reviews pass. Mac Kitty and Windows native graphics remain unverified; the full platform campaign remains deferred. Required SDK CODEOWNER approval remains outstanding. ### Demo Ghostty 1.3.1 on macOS 26.6.2 arm64, using live CLI output and synthetic local PNGs. Before is current main `759464e`; after is final commit `ee7b882`. Two independent reviewers checked all eight original screenshots, raw output, unchanged image pixels and process cleanup. Picker return has separate PTY coverage. No GIF was recorded. These are unmodified native screenshots. Before: Ctrl-C spills encoded text; the next preview fails to render.  After: cancellation leaves readable text and preserves the earlier image.  After retry: the new image renders and the earlier image remains.  [Native evidence and reproducible recipe](https://github.com/user-attachments/files/33012233/pr342-ghostty-ee7b882-proof.zip), [capture run](https://github.com/openai/openai-cli/actions/runs/37154549218), [helper lifecycle regression](https://github.com/openai/openai-cli/blob/ee7b882fb738589c5ecaf354071c72d8993ab941/scripts/check-image-output-lifecycle.py), [picker and executable replacement recipe](https://github.com/openai/openai-cli/blob/ee7b882fb738589c5ecaf354071c72d8993ab941/scripts/check-image-picker-native.py).
Automated Release PR --- ## [1.37.0](openai/openai-cli@v1.36.0...v1.37.0) (2026-10-04) ### Features * **cli:** include fish picker shortcuts in Linux packages ([openai#361](openai#361)) ([bae10cc](openai@bae10cc)) ### Documentation * **cli:** keep Homebrew picker setup on first run ([openai#367](openai#367)) ([fbe7e46](openai@fbe7e46)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: openai-sdks[bot] <284451331+openai-sdks[bot]@users.noreply.github.com>
…openai#438) Existing image compatibility checks still expected cleared prompts and one Kitty frame per image. This updates those checks for retained drafts and chunked image output. ### What changes - Check retained prompts after generation, printing, cancellation, and reopening. - Keep legacy draft migration, exact requests, saved bytes, and terminal cleanup assertions. - Check authentication recovery without automatically retrying generation. - Reassemble Kitty chunks before checking image counts and pixels. - Reject incomplete frames, invalid chunk sizes, malformed headers, and invalid base64. - Preserve separate capture files for case names containing periods. - Require exact shell-setup receipts on stderr while keeping stdout empty. ### Commands No CLI commands or flags change. Existing checks use synthetic local responses and temporary settings. ```sh python3 -I -B -m unittest discover -s scripts -p test_terminal_images.py python3 -I -B scripts/check-image-picker-integration.py /path/to/openai /tmp/image-integration python3 -I -B scripts/check-image-picker-reuse.py /path/to/openai /tmp/image-reuse python3 -I -B scripts/check-image-picker-native.py /path/to/openai /tmp/image-protocol ``` ### Code The existing scripts share Kitty parsing and picker synchronization helpers. Runtime packages, generated commands, dependencies, and startup wiring stay unchanged. ### Tested macOS ARM64, Go 1.26.9: 413 focused test and subtest results passed. These cover partial saves, cancellation, large images, model discovery, preferences, loading, snapshots, and font capacity. The corrected checkers passed 162 synthetic process and PTY cases. The old checkers failed on retained drafts and valid Kitty chunks from the same main binary. The corrected parser accepted all 12 captured protocol cases and passed 35 independent adversarial probes. Two Python unit tests passed. Two additional CLI sessions verified separate capture files for dotted case names. Temporary-profile setup passed in Bash 3.2 and zsh. Fish was unavailable. Bash 3.2 used ordinary-completion fallback; zsh registered its picker hook. Independent review, CI, and automated code and security reviews passed at `b8c0d09`. CI includes native setup checks on macOS and Windows, with skipped cases recorded separately. Native Windows, Linux, WSL, SSH/tmux, and named graphical-terminal coverage remain incomplete. Private PTYs establish process and protocol behavior, not native graphical appearance. ### Demo No product appearance changes. This PR repairs synthetic compatibility checks, so it does not add a visual demo. <!-- compatibility-parent: openai#277 --> <!-- compatibility-parent: openai#291 --> <!-- compatibility-parent: openai#293 --> <!-- compatibility-parent: openai#294 --> <!-- compatibility-parent: openai#296 --> <!-- compatibility-parent: openai#297 --> <!-- compatibility-parent: openai#302 --> <!-- compatibility-parent: openai#318 --> <!-- compatibility-parent: openai#325 --> <!-- compatibility-parent: openai#342 --> <!-- compatibility-parent: openai#357 --> <!-- compatibility-parent: openai#358 --> <!-- compatibility-parent: openai#359 --> <!-- compatibility-parent: openai#360 --> <!-- compatibility-parent: openai#361 --> <!-- compatibility-parent: openai#367 --> <!-- compatibility-parent: openai#368 --> <!-- compatibility-parent: openai#369 --> <!-- compatibility-parent: openai#370 --> <!-- compatibility-parent: openai#377 --> <!-- compatibility-parent: openai#381 --> <!-- compatibility-parent: openai#383 --> <!-- compatibility-parent: openai#394 --> <!-- codex-thread: 01a131fd-9aab-7e73-8988-52a0bbd0412d -->
In Ubuntu's Fish 3, typing `openai images gen` and pressing Tab could print an option error instead of completing `generate`. File completion also returned unrelated matches such as `assets/a=assets/` when the intended file was `assets/a=b.png`. This fixes those completion paths and checks them on Linux x64 and ARM64. ### What changes - Restore Fish 3 completion through its supported tokenization command while preserving Fish 4 token expansion. - Keep literal `=` and `:` filename prefixes from producing unrelated Fish 3 matches. - Check restored Tab bindings against each shell's original binding, so equivalent Fish 3 and Fish 4 output both pass. - Require zsh, Fish, and quoting checks on both existing Linux runners; missing or skipped tests fail. - Install zsh, Fish, and Expect from signed Ubuntu sources and record their versions, architectures, and executable paths. ### Commands No CLI commands or flags change. These examples show the prefixes to type before pressing Tab: ```sh # Press Tab after gen to complete generate. openai images gen # With assets/a=b.png present, press Tab after a= to complete that filename. openai files create --file=assets/a= ``` ### Code The Fish adapter in `internal/autocomplete/shellscripts/fish_autocomplete.fish` retains `commandline -xpc` and falls back to `-opc`. The fallback does not evaluate command substitutions. Fish 3 retains literal-token behavior; Fish 4 retains variable expansion. For legacy literal file prefixes, Fish's tokenizer removes quoting before the adapter encodes separators for the existing path helper. Expansion-bearing paths keep Fish's native behavior. This does not fix Fish 3 separator matches after variables or substitutions. The adjacent binding tests compare exact before/after bytes and preserve Tab execution, fallback, resourcing, and repeated disable checks. The workflow preserves all six job names, action pins, permissions, prepared-source handling, and native architecture guards. The existing tests use isolated settings, synthetic executables, and private PTYs. This adds no Go package or test framework. The [workflow recipe](https://github.com/openai/openai-cli/blob/ad584b96be03d19bffdc82f2fcb98edfd550b8e0/.github/workflows/help-compatibility.yml) contains the exact test selections and required-shell settings. ### Tested Local validation passed: YAML, Bash syntax, preserved workflow fields, ten execution-gate probes, six pipeline probes, and the existing source-boundary test. Six executable-identity probes also passed. They cover Ubuntu path aliases, unrelated executables, empty metadata, failed metadata commands, and architecture mismatches. Fish 4.9.3 passed all eight Fish test groups and 49 subtests on macOS ARM64, with no skips. The added cases cover literal separators, balanced and unfinished quotes, spaces, backslashes, variable paths, and inert command substitutions. Independent source review accepted the Fish correction. Corrected Linux execution remains pending. SDK CODEOWNER review remains required for this workflow change. The Fish package tests explicitly source rendered scripts. They do not establish automatic `vendor_conf.d` discovery or production package installation. ### Demo Fish 3 before: Tab prints `commandline: -xpc: unknown option` instead of completing the command. Expected Fish 3 result, pending hosted verification: `openai images gen<Tab>` completes to `openai images generate`. Hosted shell execution does not establish graphical terminal appearance or interactive zsh Tab behavior. <!-- compatibility-parent: openai#246 --> <!-- compatibility-parent: openai#291 --> <!-- compatibility-parent: openai#297 --> <!-- compatibility-parent: openai#315 --> <!-- compatibility-parent: openai#316 --> <!-- compatibility-parent: openai#347 --> <!-- compatibility-parent: openai#348 --> <!-- compatibility-parent: openai#357 --> <!-- compatibility-parent: openai#359 --> <!-- compatibility-parent: openai#360 --> <!-- compatibility-parent: openai#361 --> <!-- compatibility-parent: openai#379 --> <!-- compatibility-parent: openai#384 --> <!-- codex-thread: 01a131fd-7e05-7622-8102-817940a4b4ee -->
2 participants
Linux fish users currently need explicit setup or an eligible first CLI run before
Tab opens the image picker in a future shell. This adds a package-owned startup
file so the shortcut is available at the first prompt after installation.
What changes
and legacy opt-outs, custom bindings and existing ordinary completion.
after PATH changes, using the system file-comparison utility.
preserve release inputs for older tags.
Commands
No new public commands or flags. Existing commands remain:
Maintainers generate the package asset with
go run ./scripts/render-linux-picker.The existing local release script runs this automatically.
Code
internal/autocompleteowns rendering and the shared activation handler.scripts/render-linux-pickeris a build-only entrypoint; it does not initializethe CLI or API configuration. nFPM installs its output directly into
vendor_conf.d. Runtime command registration, main.go, generated commands,dependencies and response handling are unchanged.
Tested
Native fish tests pass on macOS with 17 package scenarios and the existing
installed-fish checks. Regression overlays fail for the previous first-prompt
and stable-opt-out bugs; the new implementation passes. Release-input validation,
historical-tag preflight, module verification, focused vet, compilation and the
trusted-main custom-code budget pass.
Hosted CI exposed a race in an existing stopped-worker test. A test-only follow-up
waits for the stop notification before exercising forced cleanup; the error,
timeout and process-reaping assertions remain intact.
Independent adversarial reviews found no remaining supported defects. Eleven additional
native fish probes cover executable replacement/removal, PATH changes, opt-outs
and preserved ordinary completion. Regression probes fail against earlier
commits and pass after the fixes. Personal Bash, zsh and fish scripts are
byte-identical; generator output checks pass. Older native fish was not run.
Synthetic GoReleaser builds produce APK, DEB, RPM and Arch packages. APK contents,
permissions and ownership are checked. These are package builds, not real Linux
installations. Hosted CI
passes on
1f13f2d. The final code and security reviews reported no new findings.Optional native-test dependencies are skipped unless fish testing is explicitly
required. The broader platform campaign remains deferred.
Demo
No image rendering changes. Native fish tests exercise first-prompt Tab,
unrelated bindings, opt-out, executable selection and disabling the shortcut.
Package fixtures verify the single installed file. Real Linux package-manager
installation and upgrade demonstrations remain pending.