[codex] Structure preview asset URL failures#3399
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
ApprovabilityVerdict: Approved Straightforward error handling improvement that converts an unstructured panic into a structured error class. The new error intentionally captures URL lengths rather than actual values to prevent leaking sensitive tokens, and includes comprehensive test coverage. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
6989347 to
199dc14
Compare
Dismissing prior approval to re-evaluate 199dc14
199dc14 to
c048ffe
Compare
Dismissing prior approval to re-evaluate c048ffe
e27816d to
118514f
Compare
c048ffe to
215b085
Compare
118514f to
c3688f1
Compare
215b085 to
8751fdb
Compare
Co-authored-by: codex <codex@users.noreply.github.com>
c3688f1 to
ca92705
Compare
8751fdb to
087f4d1
Compare
Summary
BrowserPreviewAssetUrlInvalidErrorScope
apps/web/src/browser/openFileInPreview.tsand its focused testChatMarkdownreporting are intentionally deferred until [codex] Preview workspace image files #3259 and [codex] Report markdown interaction failures #3355 are integratedStack dependency
codex/stack-pr-3259mirrors the exact fork head of draft [codex] Preview workspace image files #3259 because GitHub cannot directly target a fork branch as this PR basemainand retarget the PR tomainValidation
pnpm vp test apps/web/src/browser/openFileInPreview.test.tspnpm vp checkpnpm vp run typecheckNote
Low Risk
Localized change to browser preview URL resolution and error typing; improves observability without altering happy-path preview behavior.
Overview
openFileInPreviewno longer treats a bad preview asset URL as an unstructured defect. Whennew URL(relativeUrl, httpBaseUrl)throws, the flow returnsBrowserPreviewAssetUrlInvalidErrorviaCause.fail, with environment/thread/file context, URL length and expiry metadata, and the original parser error ascause—without putting the raw base URL or signed relative path on the error object.The return type and a
Schema.isguard are added for this error. A new unit test asserts cause identity, safe fields, and that serialized errors do not leak signed URL tokens; preview is not opened on failure.Reviewed by Cursor Bugbot for commit 087f4d1. Bugbot is set up for automated code reviews on this repo. Configure here.