OUT-3763 (1/2) | Fall back to comment-scoped path for stale attachment URLs

[arpandhakal]

Changes

Patches the immediate user-visible symptom from OUT-3763: ~26 comments have Comments.content URLs pointing at the pre-move task-scoped path ({ws}/{taskId}/{file}), while the actual file lives at the comment-scoped path ({ws}/{taskId}/comments/{commentId}/{file}). On read, rewrite their srcs so the broken comments render until the backfill makes the stored content correct.

In src/utils/signedUrlReplacer.ts:

• signMediaForComments routes through a new comment-scoped signer (signCommentMediaPath) that tries the stored path first, and on failure retries against {prefix}/comments/{commentId}/{fileName}.
• <img> srcs always go through the signer (images need a fresh token to render).
• <div data-type="attachment"> srcs are only rewritten when their stored path lacks /comments/{commentId}/. Downloads (useDownloadFile) use the path, not the token, so healthy attachment tags don't need a fresh signed URL — and we skip the signing call entirely.
• Original replaceImageSrc is untouched and still used by task body and template rewrites — no behavior change for those paths.

Marked for removal in the inline comment once the backfill ticket rewrites the affected Comments.content rows.

Testing Criteria

• Proxy into an OUT-3763-affected workspace, open one of the 26 affected comments → image renders; if the comment has a file attachment, the download works.
• Healthy (non-affected) comments still render normally; check the network tab for <div data-type="attachment"> srcs — no new signing calls should fire compared to before this PR.
• Comment that has the same image embedded twice still renders both copies (dedup via seen set).
• Task body and template body rendering unchanged (they still use replaceImageSrc).

Notes

• This is the immediate-mitigation half of OUT-3763. The underlying-cause fix (write-side) is in a sibling PR — they're independent, either can merge first.
• The 26 stale Comments.content rows still need a backfill (separate ticket) to permanently rewrite their URLs. Once that lands, the OUT-3763 block in signedUrlReplacer.ts can be removed.

Impact & Surface Area of Change

• Every comment read (task detail page activity log, replies fetch) now routes through rewriteCommentMediaSrcs instead of replaceImageSrc. For healthy comments: one signing call per <img> (unchanged), zero new calls for <div data-type="attachment"> tags. For the 26 affected comments: one extra failing signing call per stale src, then a successful fallback call.
• replaceImageSrc is unchanged and still consumed by tasks and templates.

🤖 Generated with Claude Code

[linear-code]

OUT-3763

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
tasks-app	Ready	Preview, Comment	May 22, 2026 10:38am

[greptile-apps]

Greptile Summary

This PR introduces a read-side mitigation for OUT-3763, where ~26 comments have content HTML pointing at stale pre-move storage paths. The new rewriteCommentMediaSrcs function rewrites <img> srcs unconditionally and <div data-type=\"attachment\"> srcs only when the stored path lacks the comment-scoped segment, with a signCommentMediaPath fallback that retries at {prefix}/comments/{commentId}/{fileName} when the primary path returns no signed URL.

• signMediaForComments is updated to route all comment content through rewriteCommentMediaSrcs instead of the existing replaceImageSrc; replaceImageSrc is left unchanged for task/template paths.
• Two bugs are present: (1) IMG_TAG_REGEX and ATTACHMENT_TAG_REGEX are module-level singletons with /g, and concurrent Promise.all invocations interleave at await points, corrupting each other's lastIndex and potentially causing exec to exit the loop early — silently dropping unsigned images; (2) the replacement loop uses String.replace (first-occurrence only), so duplicate identical srcs within one comment only get the first occurrence rewritten, directly contradicting the stated test criterion.

Confidence Score: 3/5

Merging as-is will leave every comment fetch subject to silent image-signing failures under concurrent load, and will fail to fully rewrite duplicate srcs within a single comment.

The shared module-level regex instances with the /g flag are mutated across concurrent async calls spawned by Promise.all. At every await signCommentMediaPath(...) suspension, a sibling coroutine can advance lastIndex past the remaining matches in the current comment's string, causing exec to return null early and silently leaving images unsigned. Separately, String.replace (string overload) only patches the first occurrence of each src, so a comment containing the same image twice will only have one of the two occurrences rewritten — exactly the scenario the PR's own test criteria call out.

src/utils/signedUrlReplacer.ts — the two new private functions and their interaction with the module-level regex state.

Important Files Changed

Filename	Overview
src/utils/signedUrlReplacer.ts	Adds rewriteCommentMediaSrcs and signCommentMediaPath for OUT-3763 fallback; two bugs: shared module-level /g regexes race under Promise.all concurrency (can silently drop images), and String.replace only fixes the first occurrence of a duplicate src.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant signMediaForComments
    participant rewriteCommentMediaSrcs
    participant signCommentMediaPath
    participant getSignedUrl as getSignedUrl (Supabase)

    Caller->>signMediaForComments: comments[]
    signMediaForComments->>rewriteCommentMediaSrcs: Promise.all per comment (concurrent)

    Note over rewriteCommentMediaSrcs: Reset IMG_TAG_REGEX.lastIndex = 0
    loop each img src
        rewriteCommentMediaSrcs->>signCommentMediaPath: filePath, commentId
        signCommentMediaPath->>getSignedUrl: primary filePath
        alt file exists at primary path
            getSignedUrl-->>signCommentMediaPath: signedUrl
        else stale pre-move path
            getSignedUrl-->>signCommentMediaPath: undefined
            signCommentMediaPath->>getSignedUrl: prefix/comments/commentId/fileName
            getSignedUrl-->>signCommentMediaPath: signedUrl (fallback)
        end
        signCommentMediaPath-->>rewriteCommentMediaSrcs: signedUrl
    end

    Note over rewriteCommentMediaSrcs: Reset ATTACHMENT_TAG_REGEX.lastIndex = 0
    loop stale attachment srcs only
        rewriteCommentMediaSrcs->>signCommentMediaPath: filePath, commentId
        signCommentMediaPath-->>rewriteCommentMediaSrcs: signedUrl
    end

    rewriteCommentMediaSrcs-->>signMediaForComments: rewritten HTML
    signMediaForComments-->>Caller: comments with updated content

Loading

_{Reviews (1): Last reviewed commit: "fix(OUT-3763): fall back to comment-scop..." | Re-trigger Greptile}

[greptile-apps]

Shared regex state causes concurrent calls to corrupt each other's lastIndex

IMG_TAG_REGEX and ATTACHMENT_TAG_REGEX are module-level singletons with the /g flag. signMediaForComments calls rewriteCommentMediaSrcs for all comments simultaneously via Promise.all, and each call performs await inside the while loop. At every await suspension point a sibling coroutine can start and reset lastIndex = 0 for its own string — when the first coroutine resumes, lastIndex is whatever the sibling left it at. If the sibling's string is longer and its last match advanced lastIndex past all remaining matches in the first comment's string, exec returns null immediately and the loop exits, silently dropping those images. Move both regex literals inside rewriteCommentMediaSrcs so each invocation owns a fresh instance.

[arpandhakal]

@greptileai done

[greptile-apps]

Thanks for the fix! Moving the regex literals inside rewriteCommentMediaSrcs ensures each invocation gets a fresh instance with lastIndex = 0, eliminating the shared state issue across concurrent Promise.all calls.

[greptile-apps]

String.replace (string overload) only replaces the first occurrence of originalSrc. The seen set correctly deduplicates signing calls, so only one replacements entry exists per unique URL — but if the same src appears twice in the HTML (the PR explicitly tests this case), only the first <img> gets the fresh signed URL; the second keeps the original stale/expired path. Use replaceAll to fix every occurrence.

Suggested change

	for (const { originalSrc, newUrl } of replacements) {
	htmlString = htmlString.replace(originalSrc, newUrl)
	}
	return htmlString
	}
	for (const { originalSrc, newUrl } of replacements) {
	htmlString = htmlString.replaceAll(originalSrc, newUrl)
	}
	return htmlString
	}

[arpandhakal]

@greptileai done

[greptile-apps]

Great, that takes care of it. Thanks for the fix!

[vercel]

Deployment failed with the following error:

Deploying Serverless Functions to multiple regions is restricted to the Pro and Enterprise plans.

Learn More: https://vercel.link/multiple-function-regions

[priosshrsth]

lgtm