Repository navigation
refactor(images): isolate image workflows and presentation - #560
Conversation
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. |
Castiron custom codeEvaluated main: Trusted main policy: ✅ No new custom-code files detected. 0 mixed files remain; 0 existing customizations changed. Compared 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 39339094038 --repo openai/openai-cli \
--name castiron-custom-code-39339094038-1 --dir /tmp/castiron-custom-code-39339094038-1
git apply --stat /tmp/castiron-custom-code-39339094038-1/custom-code.patch
cat /tmp/castiron-custom-code-39339094038-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin e539d9772a0db679dc2c5c1f77fec4df556c9980 57e95bc2fa5e857c47e28249f4a932d65fc66de3
python3 scripts/castiron/custom_code_report.py report \
--base e539d9772a0db679dc2c5c1f77fec4df556c9980 \
--head 57e95bc2fa5e857c47e28249f4a932d65fc66de3 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-57e95bc2fa5e
cat /tmp/castiron-custom-code-57e95bc2fa5e/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed faafe4c9575f94a1d49133452d0e35604977b6b9. The WSL correction builds and runs the moved image tests from their new package while preserving the complete existing inventory. I found no further issues in the image refactor or the four-file correction.
Source review only; I did not run repository code. Current-head CI was still running at the inspected snapshot.
3 participants
Image workflows still lived in
pkg/custom, mixed with application policy. Move model discovery, picker/editor state, request preparation, saving/stream handling, previews/preferences, and shell setup mechanics intointernal/commands/images.Assembly retains registration order, request/output/error policy, saved-image route selection, and first-run worker admission/timeout. Prepared multipart state and stream cleanup remain feature-owned. Shared loading feedback moves to
internal/output, terminal writer classification tointernal/terminal, and image command ordering tointernal/commands. Generated sources, provenance, dependencies, and the zero-budget policy are unchanged.Feature unit tests stay with the implementation. Integration tests call application entrypoints directly, with test-only accessors for private feature state; there is no callback-injection test harness or forwarding-test layer. Assembly registers the feature directly without compatibility wrappers. Original test names are preserved; native CI package selectors and strict persistence inventories are updated without dropping cases. WSL runs the moved image contracts in their own integrity-checked Linux test binary. Demo script paths and the comparison patch target the moved source; the comparison build succeeds.
Validation:
go test ./... -run '^$';go test ./pkg/custom -count=1.go vet ./internal/commands/images ./internal/output ./internal/terminal;go mod verify; formatting, diff, shell syntax, workflow selector/test-name inventory, and generation-boundary checks.python3 scripts/image-picker-layout/build_two_column.py /tmp/cli-layout-images-two-column.Local limitation: the complete image suite encounters 17 macOS shell/first-run test failures due to protected/unreadable extended metadata. Running those tests on an unchanged archive of base
f5558bereproduces exactly the same 17 failing test roots. Assertions and hosted native coverage are preserved; no production workaround or test weakening is included. Hosted CI will verify native execution and the authoritative custom-code budget.