Repository navigation
Preserve image drafts and improve picker feedback - #394
Conversation
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 2 mixed files remain; 0 existing customizations changed. Compared 2 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 37867449820 --repo openai/openai-cli \
--name castiron-custom-code-37867449820-1 --dir /tmp/castiron-custom-code-37867449820-1
git apply --stat /tmp/castiron-custom-code-37867449820-1/custom-code.patch
cat /tmp/castiron-custom-code-37867449820-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin e32158e57f05eae260ebb7a0bd0cacc6b2961ce1 bc8203c2eb767b3cc764fcd8dd9b553f8b301573
python3 scripts/castiron/custom_code_report.py report \
--base e32158e57f05eae260ebb7a0bd0cacc6b2961ce1 \
--head bc8203c2eb767b3cc764fcd8dd9b553f8b301573 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-bc8203c2eb76
cat /tmp/castiron-custom-code-bc8203c2eb76/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 Please review commit |
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. |
![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: d7cb5cbcd8
ℹ️ 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 commit This addresses discussion_r4221652116. Loading feedback receives only the current prompt already displayed in the interactive picker. Direct request bodies no longer supply loading text. Independent native checks covered file, JSON/YAML stdin, literal flags, multipart edit, and redirected stderr. Request values, uploaded bytes, and exit statuses remain unchanged. Picker retry and cancellation preserve the current quoted prompt, spinner, elapsed time, saving state, and cleanup. Please check the prompt-source boundary, per-request context isolation, and preserved request/error/cancellation behavior. |
|
Codex Review: Didn't find any major issues. Nice work! 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43c71b513c
ℹ️ 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".
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?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: fb00753ab9
ℹ️ 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".
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 0024d0bd25beedfde1ae8762488e2610dfd3bd3c. No blocking findings.
The versioned draft uses the existing atomic state writer. Controlled exits save edited drafts; reopening does not submit a request, and explicit flags or redirected input retain the direct-command path. Recoverable request errors keep the draft editable, while cancellation and save/preview failures exit without another generation. Picker prompt feedback uses stdout; direct commands keep generic diagnostic labels. Saving status follows the save operation, and feedback stops before results and previews are written.
Validation: source review of the implementation, tests, and check scripts, including the previously reported prompt-output and redirected-stderr issues. No repository workloads ran here. Current-head hosted tests, build, lint, help checks, CodeQL, and Castiron checks passed; conditional jobs were skipped. The detailed macOS scenarios and recording results remain author-reported. Native Linux/Windows picker behavior and graphical rendering remain unverified.
…#391) Large model lists currently fill the terminal with metadata. This shows each model's exact ID and owner, with clear navigation for large results. Slow interactive requests show loading feedback instead of an unexplained blank screen. ### What changes - Show sorted ID and OWNER columns in automatic interactive output. - Preserve both complete values in labeled output when the terminal is narrow. - Keep all accessible models, including fine-tuned IDs; unavailable owners show `(unknown)`. - Finish short results directly and show empty results clearly. - Use the existing viewer for long lists: Space advances, b goes back, and q exits. - Explain that p prints all selected complete records, including metadata. - Add Models-only `--filter` and `--sort-by` controls. - Show delayed loading feedback through slow response headers, body reads, and retries. - Stop feedback before rendering, preserve Ctrl+C status 130, and restore the terminal. - Preserve explicit formats, field extraction, pipes, and legacy limit ordering without selection controls. ### Commands `models list` remains the same command. Only `--filter` and `--sort-by` are new flags. ```sh openai models list # IDs and owners in an interactive terminal openai models list --format json # complete records openai models list --transform id --raw-output # IDs for a pipeline # Match one exact ID. openai models list --filter 'id = "demo-text-alpha"' # Filter, sort descending, then keep three results. openai models list --filter 'id~^demo-text-' --sort-by '~id' --max-items 3 ``` Filters support exact `id=VALUE` and RE2 `id~REGEX` matching. Matches are case-sensitive unless the regex changes that behavior. Boolean expressions and operand lists are unsupported. Quote the whole expression for your shell. Operand quotes and backslash escapes are decoded before matching. Selection sorts before `--max-items`; omitted selection retains response-order limiting before terminal display sorting. The limit defaults to unlimited; `-1` remains supported and `0` returns no items. Selection rejects `--format raw` before HTTP. Raw output without selection retains the full response envelope. ### Code This PR targets main `c65e03e`, after Navigation openai#389 and Tables openai#390 merged. It preserves the reviewed Navigation completion and terminal-boundary corrections. Models uses the SDK's single-response pager; navigating makes no additional API request. `pkg/custom` owns command options, loading feedback, presentation, and the Models viewer adapter. `pkg/transformers` selects complete records and projects exact ID/owner pairs. Shared readable tables and Bubble Tea/Bubbles handle layout and navigation. The existing delayed-feedback helper supplies loading feedback. Two presentation hooks stop and join its worker before results write. Generated sources, module dependencies, entrypoint wiring, and payload limits remain unchanged. The one-line `.go-version` pin selects Go 1.26.9 while the Actions version index catches up. This security patch passes the existing vulnerability scan. Future Go patches require updating the pin. Universal Output composition still needs its shared policy for the optional Details hint. The local follow-up patch remains separate; this branch does not add universal `--quiet`. Images openai#394 changes the shared loading-helper signature. Combining these PRs requires the separate verified Models caller adaptation. That integration patch remains local until the revised Image helper is available in the base. ### Tested Candidate `996def6` adds the Go 1.26.9 security pin above reviewed runtime `70282f3`. All five reported review findings are fixed, with regression coverage and resolved threads. Standard-record printing retains complete fields; Models viewer Ctrl+C returns 130; q, Escape, and p retain status 0. At `70282f3`, 46 focused race tests and 206 subcases passed without failures or skips. Independent native macOS checks reproduced both original defects and verified the fixes, terminal restoration, and cleanup. Additional probes preserve joined errors, writer failures, and existing typed exit-status precedence. The earlier `26e23a3` native checks cover narrow IDs, selection, API errors, and the corrected single-response checker. Normal and optimized Python checks verify key delivery and failed-write behavior. On Go 1.26.9, the unchanged vulnerability scan passed all nine release targets. Four focused Models race tests and seven subcases also passed, with no failures or skips. The exact native build passes. Module dependencies and counted budget inputs remain unchanged. CI passed for `996def6`: 21 checks passed, with two conditional skips. The build-artifact scan used Go 1.26.9 and found no vulnerabilities across all nine release targets. [CI run](https://github.com/openai/openai-cli/actions/runs/37860812082) and the trusted budget/isolation checks passed. Merge still uses the required normal merge queue. Earlier request-loading evidence remains tied to runtime `f739395` and its 25 public scenarios. The demos remain tied to `d4a2d744`; their table, navigation, and loading paths use q and remain unchanged. Fresh native checks cover the corrected p and Ctrl+C behavior separately. Justin's original hang remains unreproduced; controlled wait handling does not establish its historical cause. Broader generated-list activation remains separate. No live API calls ran. Native Linux/Windows Models behavior remains unverified. ### Demo The recordings compare ID-only `d16c92b` with ID/owner `d4a2d744` using 48 identical synthetic records. Both use `openai models list` on macOS arm64 with Menlo terminal replay. The wide recording includes a controlled slow response and visible loading feedback. The narrow recording preserves complete ID/owner labels. All five commands exited successfully after one identical-response request each. The recorder checks navigation, terminal restoration, capture bytes, and source/binary hashes. Author and independent reviews passed all 11 screenshots and 46 GIF frames. The media shows terminal replay rather than native graphical terminal appearance.  Before:  After:  [Narrow comparison](https://github.com/user-attachments/assets/f2095fb5-c6d5-473c-b20d-9d29a093f55f). [Recording recipe](https://github.com/openai/openai-cli/blob/cbe90a336390688c7723537cec2c2eeb8c812868/scripts/demos/models-list-viewer/README.md).
2 participants
The image picker discarded edits after exiting and closed when an image request failed. This preserves the draft and lets you edit before retrying.
What changes
Prompts now persist in local picker state. Explicit flags, piped input, and structured output retain their request and output-mode behavior.
Forced termination or power loss can discard unsaved edits. Concurrent edited sessions retain the last successful complete draft.
Generation feedback adds no API calls.
Commands
No new commands or flags.
Exit with Ctrl+C and reopen the picker to continue editing. Generation still requires a deliberate Enter.
Code
pkg/customowns draft persistence, request recovery, loading feedback, picker layout, and preview spacing.The existing atomic state writer stores a versioned prompt/settings record. Legacy settings remain readable.
Recording recipes reuse the shared lifecycle in
scripts/demos.The Models caller passes an empty prompt and terminal dimensions to the shared loading helper.
It retains only the stop callback; Models never advances image-saving stages.
No new package, dependency, generated command, or startup change is included.
The terminal-image test checks its existing final-newline contract; production renderer code is unchanged.
Tested
Initial macOS checks at
37f0ce01passed 454 focused race test/subtest events and 94 process dispatch events.One font producer contract test passed.
Four shell/helper tests skipped; fish and PowerShell were unavailable.
Six bash/zsh setup-copy checks passed, including executable paths containing spaces.
Eight native terminal scenarios and 134 navigation replay assertions passed.
They cover retries, cancellation, partial saves, preview spacing, restored drafts, queued input, and the installed launcher.
Dispatch checks also cover explicit formats, closed output, multipart scalars, and large synthetic image responses.
The direct-input correction at
07551104passed eight focused race events and six public input checks.File, JSON/YAML stdin, literal flag, and multipart prompts retain their request values; uploaded bytes remain unchanged.
The final loading correction at
0024d0bpassed 59 focused race events and seven redirected-output/input events.Separate-stream checks reproduced both earlier issues, then passed five picker and direct-command cases.
These checks cover redirected diagnostics, current prompts, terminal width, unchanged requests, saving order, cleanup, and cancellation.
The shared process helper also passed its existing combined-stream retry check at
7b336e1.The spacing update passed its writer checks. Main alignment at
fb00753passed 18 focused compatibility events.The integration at
2da2da3preserves the original Images patch and inherits Go 1.26.9 from merged main.Seven focused race tests and seven subcases passed, plus seven image public-mode test events.
Six Models terminal cases verify Ctrl+C 130, q/Escape/p 0, complete records, clean redirects, and terminal cleanup.
Three image terminal controls also passed. The unchanged vulnerability checker passed all nine shipped targets.
Main’s SDK alignment at
bc8203cpassed 38 fresh race/public test events, including image stream completion, failure, and cancellation.Module verification and all nine vulnerability targets passed again.
The nine native controls remain attributed to
2da2da3; independent review confirmed their paths are unchanged.Independent source and capture review passed. CI and fresh code/security reviews passed at
bc8203c.No live API requests or paid image generation were used.
Native Windows/Linux picker behavior and terminal graphics appearance remain unverified.
Demo
Real CLI binaries on macOS, recorded as a terminal replay with synthetic drafts and zero API requests.
Before is main
d32b3ae; after was recorded at runtime37f0ce01.The later loading corrections and Models integration do not change this draft-reopen scene.
Both scenes edit the prompt and quality, exit, then reopen the picker.
Before:
After:
Recording recipe · Process checks