Multiple fixes

[Munawwar]

Fixes:

• Gracefully skip the Computer Use plugin gate patch when the upstream gate drifts
• Fix crash: Avoid passing an empty --cli-path to codex-update-manager by only adding the flag when CODEX_CLI_PATH is non-empty.

Warning suppressions:

• Treat the current screenshot marker implementation as already-applied for newer codex desktop versions instead of warning.

[ilysenko]

Review Mate: merge not recommended yet

🔴 Recommendation: merge not recommended yet
Risk level: high

Do not merge until the flagged risks are addressed.

This PR combines three runtime-facing fixes: launcher CLI preflight argument handling, a Computer Use plugin-gate fallback, and screenshot-marker warning suppression in the webview patcher.

What changed

• launcher/start.sh.template now omits --cli-path when CODEX_CLI_PATH is empty in foreground preflight, background preflight, and GUI install prompt paths.
• scripts/patches/computer-use.js downgrades an unpatchable but recognizable Computer Use gate from a hard failure to a warning.
• scripts/patches/webview-assets.js adds regex recognition for the current upstream screenshot marker implementation, with matching unit tests in scripts/patch-linux-window-ui.test.js.

Why it matters
This is not a generated-artifact problem: the launcher change is in the source template, and the tests were updated for the JS patcher behavior. The risk comes from scope and blast radius. The PR touches the launcher plus two hot patcher modules in one change, and one of those changes weakens a previously required Linux compatibility failure into a soft skip. That deserves human review before merge because upstream drift could now produce a build where Computer Use remains gated off on Linux without failing loudly.

Notes for the reviewer
I could not run local git/rg commands in this checkout because the execution sandbox failed before command startup with a bwrap namespace error. This review is based on the supplied PR diff and the project-specific review rules.

Risks

• Touches multiple hot patcher files in one PR (scripts/patches/computer-use.js and scripts/patches/webview-assets.js), which this repo treats as high-risk because patcher changes are fragile across upstream rolls.
• Combines unrelated fixes under a broad Multiple fixes title: launcher CLI argument handling, Computer Use gate behavior, and screenshot marker warning suppression. That scope makes it harder to validate and bisect.
• Downgrades the Computer Use plugin gate from a required hard failure to a warning when the gate is recognizable but unpatchable. If upstream drifts, the build can continue while the Linux Computer Use enablement patch is skipped.
• No tests/scripts_smoke.sh update is shown even though files under scripts/patches/ changed. For this repo, patcher changes without smoke-fixture consideration require human review.

Questions for the author

• Was bash tests/scripts_smoke.sh run against this diff, and is the missing smoke-fixture update intentional because neither patch changes the emitted bundle expression?
• For the Computer Use gate change, should the patch report still mark this as a required-upstream failure when the recognizable gate cannot be patched, rather than only logging a warning?

References

• launcher/start.sh.template:723
• launcher/start.sh.template:753
• launcher/start.sh.template:773
• scripts/patches/computer-use.js:135
• scripts/patches/webview-assets.js:117
• scripts/patch-linux-window-ui.test.js:1039
• scripts/patch-linux-window-ui.test.js:1249

Second opinion (Claude)

Risk level: high

Three defensive fixes in one PR: (1) only pass --cli-path to the updater when CODEX_CLI_PATH is non-empty, (2) downgrade the Computer Use plugin-gate patch from throw to warn-and-skip when upstream becomes unpatchable, (3) recognize the current upstream screenshot-marker shape as already-applied instead of warning.

What changed

• launcher/start.sh.template:723-786 — three updater call sites (run_cli_preflight, run_cli_preflight_background, run_gui_cli_prompt) now build preflight_args as an array and only append --cli-path "$CODEX_CLI_PATH" when the variable is non-empty. Edits the correct source-of-truth (not the generated codex-app/start.sh).
• scripts/patches/computer-use.js:135-141 — when both the computer-use literal and computerUse token are present but the gate pattern doesn't match any known shape, applyLinuxComputerUsePluginGatePatch now console.warns and returns the source unchanged instead of throwing Required Linux Computer Use plugin gate patch failed.
• scripts/patches/webview-assets.js:120-128 — applyBrowserAnnotationScreenshotPatch gains a pair of regexes (currentScreenshotMarkersRegex, currentDraftMarkerRegex) that detect the newer upstream shape (ye/_e/be/Me/Ne minified names) and treat it as a no-op rather than warning.
• scripts/patch-linux-window-ui.test.js — new fixtures and tests: shows only the selected marker in legacy screenshot mode, recognizes the current screenshot marker implementation without warning, and the existing fails hard… gate test rewritten as warns and skips when the Computer Use gate is recognizable but unpatchable.

Why it matters

The Computer Use plugin-gate change has a subtle CI vs. user split worth raising. The patch is still registered as ciPolicy: REQUIRED_UPSTREAM in scripts/patches/registry.js:153. When the gate becomes unpatchable, the new path emits a warning and returns currentSource unchanged → patchStatusFromChange(false, [warning]) in scripts/lib/patch-report.js:52-60 returns "skipped-optional" → scripts/ci/validate-patch-report.js:62 will still fail the upstream-build CI profile because SUCCESS_STATUSES only allows applied/already-applied. So the fail-soft is really for end users running install.sh on a drifted upstream; maintainer-side CI still loudly fails (good), it just emits skipped-optional instead of an uncaught throw. That is probably the desired behavior but the PR body doesn't say so, and the practical user-visible effect of a fail-soft gate is a build where bundled Computer Use silently stays Mac-only — i.e. a degraded install rather than a broken one.

Notes for the reviewer

• PR title is literally "Multiple fixes" and packs three unrelated fixes spanning launcher + two hot patchers (computer-use.js, webview-assets.js). Per the project's standing rules this hits the multi-patcher-PR / scope-creep heuristic. Splitting into three PRs would let each fix bisect cleanly and lets the gate-fail-soft trade-off be discussed on its own thread.
• tests/scripts_smoke.sh is intentionally not updated; both patcher changes only add already-applied / fail-soft branches that don't alter the produced output, so the existing fixtures still exercise the success path. The new node --test cases cover the new branches. This is reasonable, but risks notes the spot to double-check.
• The new currentScreenshotMarkersRegex / currentDraftMarkerRegex rely on the minified symbol names ye, _e, be, Me, Ne, ge. These are exactly the kind of identifiers that reroll on each upstream minify; the regex will silently stop matching one upstream from now. Because it's only a no-op-when-matched branch, drift here is harmless (it'll just re-emit the WARN), but expect this to need refreshing alongside the existing patch needles.

Risks

• Touches two hot patcher files (scripts/patches/computer-use.js, scripts/patches/webview-assets.js) plus launcher/start.sh.template in a single PR titled "Multiple fixes" — repeated scope-creep pattern the maintainer has rejected before; recommend splitting into three PRs (launcher, gate, screenshot-marker).
• linux-computer-use-plugin-gate is still ciPolicy: REQUIRED_UPSTREAM (scripts/patches/registry.js:153), so when the new fail-soft branch fires the status becomes skipped-optional (scripts/lib/patch-report.js:52-60) and scripts/ci/validate-patch-report.js:62 still fails the upstream-build profile. Worth confirming this is the intended behavior — if the goal is to also let CI tolerate drift, the policy needs to change too; if the goal is only user-install resilience, that should be called out in the PR body and/or a code comment.
• Fail-soft on a REQUIRED_UPSTREAM patch means a user-side install can succeed with Computer Use silently left as Mac-only (gate stays isMac(t)). That's a degraded but functional app; users will not know why bundled Computer Use isn't available unless they read the install log for the WARN line. Consider routing this through notify_error or surfacing it in patch-report.json more prominently.
• New currentScreenshotMarkersRegex / currentDraftMarkerRegex anchor on minified identifiers (ye, _e, be, Me, Ne, ge, _l, bl) which will not survive the next upstream minifier reroll. Low-impact since both regex hits are no-ops, but be prepared to refresh this alongside the rest of the screenshot-marker needles.
• tests/scripts_smoke.sh is untouched. That's defensible here because neither patcher change alters the output expression of an applied patch, but per the maintainer's standing rule it's worth confirming explicitly in the PR body.
• Launcher fix correctly edits the source-of-truth launcher/start.sh.template and not the generated codex-app/start.sh — no source/generated mismatch concern, but worth a sanity check that the ${VAR} survives the install.sh::create_start_script prelude concatenation (the new locals preflight_args/args are template-scope, so this should be fine).

---

_{🤖 Reviewed by codex + claude.}

[Munawwar]

lol AI assisted review on an AI assisted PR. So breaking out of that cycle, this is me (human) writing this comment:

Sorry, I should have retested this changes on the latest main because fix 1 isn't needed anymore. So either 4 days ago I noticed fix 1 was needed OR my Codex.dmg was outdated (I can't trace back what happened).

And the warning that I suppressed, I can't replicate anymore.

I reverted the two. changes.

---

About the 2nd fix, it still happens if codex CLI is not installed. Line 770 in main branch in launcher/start.sh.template

    if ! refreshed_path="$(run_update_manager prompt-install-cli --cli-path "$CODEX_CLI_PATH" --print-path)"; then
        return 1
    fi

If $CODEX_CLI_PATH is empty pdate_manager prompt-install-cli just exits with error: a value is required for '--cli-path <CLI_PATH>' but none was supplied

So my fix is to check if CODEX_CLI_PATH is not empty before adding --cli-path

[ilysenko]

Review Mate: maintainer review required

🟡 Recommendation: maintainer review required
Risk level: medium

Review this before merging; the risk is non-trivial.

This PR has been narrowed to one launcher fix: the update-manager CLI preflight and GUI install prompt no longer pass --cli-path when CODEX_CLI_PATH is empty.

What changed

• launcher/start.sh.template now builds updater arguments as Bash arrays in run_cli_preflight, run_cli_preflight_background, and run_gui_cli_prompt.
• The earlier hot-patcher changes described in the previous review are no longer present in the current diff.

Why it matters
The edit is in the correct source-of-truth launcher template, not a generated codex-app/start.sh artifact. It is still a launcher runtime change, so it affects source installs and the generated launcher shipped through the package/update-builder paths. Per this repo's risk bands, any launcher/start.sh.template behavior change is at least medium and should get human review.

Notes for the reviewer
The implementation is small and directionally right: prompt-install-cli and cli-preflight both model cli_path as optional on the Rust side, so omitting the flag when the shell variable is empty matches that contract. The remaining review concern is test coverage: the existing smoke checks only assert that the prompt path exists, not that an empty CODEX_CLI_PATH omits --cli-path.

Risks

• Runtime launcher behavior changed in launcher/start.sh.template; this entrypoint is shared by source installs and packaged builds, so it needs human validation even though the patch is small.
• No regression test was added for the empty CODEX_CLI_PATH case; current smoke coverage only checks for launcher prompt strings and would not catch reintroducing --cli-path "".

References

• launcher/start.sh.template:724
• launcher/start.sh.template:728
• launcher/start.sh.template:756
• launcher/start.sh.template:777
• launcher/start.sh.template:1702
• launcher/start.sh.template:1718
• updater/src/cli.rs:22
• updater/src/cli.rs:30
• tests/scripts_smoke.sh:644

Second opinion (Claude)

Risk level: medium

After the author's revert, this PR is a single contained fix to launcher/start.sh.template: only pass --cli-path to codex-update-manager when CODEX_CLI_PATH is non-empty. The other two changes (Computer Use plugin-gate fail-soft, screenshot-marker warning suppression) were dropped in response to earlier review feedback.

What changed

• launcher/start.sh.template:715-748 — run_cli_preflight builds preflight_args as an array and only appends --cli-path "$CODEX_CLI_PATH" when set.
• launcher/start.sh.template:750-764 — run_cli_preflight_background does the same in the background subshell.
• launcher/start.sh.template:770-790 — run_gui_cli_prompt does the same for prompt-install-cli.

Why it matters

The author confirmed the underlying crash: when CODEX_CLI_PATH is empty and the CLI isn't installed, prompt-install-cli aborts with error: a value is required for '--cli-path <CLI_PATH>' but none was supplied, blocking the first-run install prompt. The fix edits the correct source-of-truth template (not the generated codex-app/start.sh), is applied symmetrically to all three updater call sites, and is bash-safe (${VAR:-} guard, array append, quoted expansion). The clap CLI on the updater side already treats --cli-path as optional in its parser surface, so omitting the flag is the right shape.

Notes for the reviewer

• Worth a quick verification that the updater's cli-preflight / prompt-install-cli subcommands behave correctly when invoked without --cli-path (they should fall back to discovery via find_codex_cli); if any of them still requires the arg, that's the place to fix it rather than the launcher.
• tests/scripts_smoke.sh is untouched. That's fine here — the patcher diffs were reverted, so no smoke fixture needs updating; only the launcher template changed and the smoke suite doesn't anchor on these helper bodies.
• Lands as medium only because any launcher/start.sh.template edit is medium per the project's risk catalogue. The change itself is narrow and low-blast-radius.

Risks

• Launcher template change — per project rules, any edit to launcher/start.sh.template is at least medium because it affects every install path (deb/rpm/pacman/nix/source).
• Behavior depends on the updater treating missing --cli-path as 'discover via fallback'. If any of cli-preflight / prompt-install-cli paths in updater/src/cli.rs actually require a path argument for correctness (not just clap), the launcher will silently proceed with an under-specified call.

---

_{🤖 Reviewed by codex + claude.}