Skip to content

Fix Fable review findings from the be1d066732 sync - #255

Merged
henrychu04 merged 4 commits into
coder-onlyfrom
t3code/fable-review-fixes-be1d066732
Oct 27, 2026
Merged

henrychu04 merged 4 commits into
coder-onlyfrom
t3code/fable-review-fixes-be1d066732

Conversation

@henrychu04

@henrychu04 henrychu04 commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

Follow-ups from the Fable review of #253 and #254. All of these predate those PRs.

  • GitLab uploads resolve below the project. A description's /uploads/<secret>/<file> was resolved against the host root (https://host/uploads/…). GitLab serves it under the project (https://host/group/project/uploads/…), so attachments in merge request descriptions linked to the wrong place. splitPullRequestBody now joins the path onto the repository URL. The test-only hostUrl prop on PullRequestMarkdown is gone, and both tests now expect the project URL.
  • GitLab uploads inside Markdown resolve too. GitLab writes a pasted screenshot as ![image](/uploads/…) and other files as [name](/uploads/…). Those kept a relative address, so they showed as broken images or links. PullRequestMarkdown's new remarkGitLabUploads plugin joins image, link, and reference-definition URLs onto the project with the same resolveGitLabUploadUrl. It works on the parsed tree, so code blocks are untouched.
  • The link dialog accepts !42. "Copy MR number" copies !42, which the dialog rejected. pullRequestLinkInput.ts now takes #, !, or a bare number, and the placeholder and validation text say !42.
  • Upstream's snapshot-status fallback. ThreadStatusIndicators.tsx uses upstream's two lines again. They read GitLab for every link the registry accepts.
  • coder-only.md drift:
    • Dropped the allowStale note, which describes upstream behavior.
    • Documented the hover-preview read and why gitLabMergeRequestBrowserUrl stays.
    • Noted that an unknown provider gets upstream's generic wording.

Verification

  • pnpm typecheck:web, client-runtime typecheck, vp fmt --check, and pnpm test:knip pass.
  • pnpm test:client-runtime passes (751 tests).
  • pnpm test:web:coder passes (277 files, 3420 tests).
  • pnpm build passes.
  • A scratch render of a merge request body checked the upload forms: inline images, links, standalone images, .mp4 videos, and reference-style definitions all became https://gitlab.example/group/project/uploads/…. A fenced code block kept its raw text, and external images were unchanged. It was deleted afterwards.
  • A scratch test checked that !42, #42, and 42 parse, that !!42 is rejected, and that !42 resolves to the project's merge request URL. It was deleted afterwards.

Not checked live: how an upload renders against a real GitLab instance. Images load straight from GitLab in the browser, so on a private project they need the browser's GitLab session, the same as other GitLab media on main.

🤖 Generated with Claude Code

henrychu04 and others added 4 commits October 26, 2026 08:49
GitLab serves a description's `/uploads/<secret>/<file>` at
`https://host/group/project/uploads/...`, but the body splitter resolved it
against the origin. Join it onto the repository URL instead, and drop the
test-only `hostUrl` override so the tests go through the markdown context.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"Copy MR number" copies `!42`, which the dialog rejected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fallback reads GitLab for every link the registry accepts, so the
fork-only null branch is gone. The doc drops the `allowStale` note (upstream
behavior now), documents the hover preview read and why
`gitLabMergeRequestBrowserUrl` stays, and notes that an `unknown` provider gets
generic wording.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitLab writes a pasted screenshot as `![image](/uploads/<secret>/<file>)` and
other files as `[name](/uploads/...)`. Only bare lines were resolved, so these
kept a relative address. A remark plugin now joins image, link, and reference
definition URLs onto the project with the same resolver; code is untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@henrychu04
henrychu04 merged commit d36c324 into coder-only Oct 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant