fix(chatgpt): drop unused second arg from getChatGPTImageAssets evaluate#1556
Open
ele-yufo wants to merge 1 commit into
Open
fix(chatgpt): drop unused second arg from getChatGPTImageAssets evaluate#1556ele-yufo wants to merge 1 commit into
ele-yufo wants to merge 1 commit into
Conversation
The IIFE inside the evaluate string already receives the URL list via
the embedded `${urlsJson}` template substitution, so the trailing
`, urls` argument to `page.evaluate` is redundant.
It also fails hard against the post-1.7.x browser bridge guard:
page.evaluate string input does not accept args;
use page.evaluate(fn, ...args) instead
This breaks every `opencli chatgpt image` invocation that downloads
the generated asset (i.e. anything without `--sd true`).
Fix: drop the trailing argument. The function already has the URLs
inlined in the evaluated script, so no behaviour change beyond
unblocking the download path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getChatGPTImageAssetspasses the URL list both ways:Browser bridge 1.7.x rejects the second form for string scripts:
So any
opencli chatgpt image <prompt>that actually downloads the asset (i.e. without--sd true) fails withcode: UNKNOWN.Fix
Drop the trailing
, urlsargument. The URLs are already inlined via the${urlsJson}template substitution, so no semantic change — only the redundant + now-invalid CDP arg is removed.Root cause
The redundant arg was harmless in older bridge versions; the strict-mode guard was added in
browser/utils.ts:Validation
npx tsc --noEmitcleannpx vitest run --project adapter clis/chatgpt/— 25/25 pass (3 files)opencli chatgpt image "a fluffy orange cat" --op /tmp/xsucceeds end-to-end after the fix (fails before with the error above)Scope
Single-line change in
clis/chatgpt/utils.js. No new tests — the existingimage.test.jsmocksgetChatGPTImageAssets, so this codepath is only exercised against a live browser. Happy to add an integration-style assertion if the reviewer prefers.