fix(media/image): surface sharp-install hint when optimizer exhausts every resize attempt#77197
fix(media/image): surface sharp-install hint when optimizer exhausts every resize attempt#77197lonexreb wants to merge 2 commits intoopenclaw:mainfrom
Conversation
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: yes. Source inspection shows current main catches each Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Land a diagnostic-only fix after the message names a global repair that current package resolution/dependency policy actually supports and after the contributor supplies real missing- Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main catches each Is this the best way to solve the issue? No, not as written. The detector branch is the narrow maintainable fix, but the global install guidance should either use a command that actually places Full review comments:
Overall correctness: patch is incorrect What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 7d5ca3064a51. |
![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: b561df525a
ℹ️ 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".
| if (isOptionalImageOptimizerUnavailable(firstResizeError)) { | ||
| throw new Error( | ||
| "Image optimization requires the optional `sharp` native dependency, which is not installed. " + | ||
| "Install it with `pnpm add sharp` (or `npm install sharp` / `bun add sharp`) and restart the gateway.", |
There was a problem hiding this comment.
This hint is shown specifically for hosts where a global OpenClaw install is missing sharp, but the suggested commands (pnpm add sharp / npm install sharp / bun add sharp) are local installs, so operators can follow the message and still hit the same error on restart because the global runtime import path is unchanged. The remediation should include a global/path-aware command (for example the docs’ global reinstall flow) so the advice actually resolves the reported scenario.
Useful? React with 👍 / 👎.
lonexreb
force-pushed
the
fix/73148-image-sharp-missing-hint
branch
from
b561df5 to
fd35b2b
Compare
Codex review on PR openclaw#77197 noted: the previous hint suggested local install commands (pnpm add sharp / npm install sharp / bun add sharp) but the scenario operators most often hit is a global `npm install -g openclaw` where the global runtime resolves `sharp` from openclaw's own global node_modules, so a workspace install does not actually fix the failure on restart. Expand the thrown error to include both repair postures: the existing local commands plus an explicit global path (`npm install -g openclaw@latest --include=optional` or `npm install -g sharp`) so operators on either install posture get a working remediation. Update the regression test to assert both surfaces appear in the message.
lonexreb
force-pushed
the
fix/73148-image-sharp-missing-hint
branch
from
fd35b2b to
b969371
Compare
|
Addressed [P2] from codex review (
Updated the existing regression test to assert both surfaces appear in the message. Targeted tests still green: |
…every resize attempt (openclaw#73148) When optimizeImageToJpeg's resize loop fails on every (size, quality) combination because the optional native dep "sharp" is missing, the throw used to read 'Failed to optimize image: Cannot find module sharp...' with no install hint, leaving operators on globally-installed hosts (where pnpm/npm skipped the native build) guessing. isOptionalImageOptimizerUnavailable already detects the cause-chain shape; reuse it. When it matches, throw a clear 'Image optimization requires the optional sharp native dependency' message that names the install command. Cause is preserved. Tests: new direct-call regression in web-media.test.ts asserts the new hint text. Two existing 'sharp unavailable' tests relaxed from the literal old prefix to /sharp/i so both old (cause-chain) and new (top-level) wording are accepted.
Codex review on PR openclaw#77197 noted: the previous hint suggested local install commands (pnpm add sharp / npm install sharp / bun add sharp) but the scenario operators most often hit is a global `npm install -g openclaw` where the global runtime resolves `sharp` from openclaw's own global node_modules, so a workspace install does not actually fix the failure on restart. Expand the thrown error to include both repair postures: the existing local commands plus an explicit global path (`npm install -g openclaw@latest --include=optional` or `npm install -g sharp`) so operators on either install posture get a working remediation. Update the regression test to assert both surfaces appear in the message.
lonexreb
force-pushed
the
fix/73148-image-sharp-missing-hint
branch
from
b969371 to
bc4c1c5
Compare
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
1 participant
Summary
When
optimizeImageToJpegexhausts every (size, quality) attempt because the optionalsharpnative dep is missing, the throw used to readwith no install hint. On hosts where the global install skipped the native build (e.g. the reporter's
npm i -g openclawon Ubuntu), every image-tool call returned the opaqueFailed to optimize imageand operators had to guess what to install.isOptionalImageOptimizerUnavailablealready walks the cause chain and detects this exact failure shape — it's the existing safety net that letsoptimizeAndClampImagefall back to the original buffer when it's already under cap. Reuse it at the bottom of the resize loop: when the loop'sfirstResizeErrormatches that detector, throw a clear, actionable message that names the missing dep and the install command.Why this is the right place
There are two existing graceful-fallback paths above this throw:
optimizeAndClampImagereturns the original buffer when sharp is missing and the buffer is already under cap and the source isn't HEIC.optimizeImageToJpeg's HEIC branch wraps withHEIC image conversion failed:and includes the inner message viaString(err).Neither catches the case the issue reports: the agent
imagetool calls intooptimizeImageToJpeg(viatool-images.ts:resizeToJpeg) for over-cap or non-HEIC images that need actual resizing — those exhaust the resize loop with sharp module-resolution errors and hit the bottom throw. That's exactly where this fix sits.Changes
src/media/web-media.ts— At the bottom ofoptimizeImageToJpeg, ifisOptionalImageOptimizerUnavailable(firstResizeError)matches, throw a sharp-install-hint message that names the install command. Cause is preserved so log inspectors can still see the underlyingCannot find module 'sharp'.src/media/web-media.test.ts— Add a regression test (#73148) that callsoptimizeImageToJpegdirectly with the existingwithUnavailableImageOptimizermock and asserts the new hint text. Relax two existing tests that asserted on the old literal/Optional dependency sharp is required/to/sharp/iso both the old (cause-chain) and new (top-level) wording are accepted — the underlying behavior they cover hasn't changed.CHANGELOG.md— Unreleased > Fixes entry credited to me.Test plan
pnpm test src/media/web-media.test.ts— 39/39 pass (1 new + 38 existing)pnpm exec oxfmt --checkon touched files — cleanFixes #73148.
Real behavior proof
Test:
pnpm test src/media/web-media.test.ts— 39 passed.Calls
optimizeImageToJpegagainst a missing-sharp scenario and asserts the thrown error contains both the local repo (pnpm add sharp) and global install (npm install -g openclaw@latest --include=optional) repair commands. Same code path that fires when an embedded reply tries to resize a JPEG on a host without the optional native dep.