Repository navigation
fix(slack): Convert canvas HTML to Markdown on read - #2647
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This comment has been minimized.
This comment has been minimized.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
|
Contributor
|
I think this is generally ready to go! Just need to rebase to pull in fixes for CI. A small nit is that instead of checking |
Slack serves canvas downloads as Quip HTML. slackCanvasEdit wrote that HTML back as Markdown, so Slack rendered the whole canvas as escaped HTML text. Convert the body to Markdown in readCanvas so reads and edits both work on Markdown.
Slack keeps the title h1 across a body replace, so an edit that wrote it back would show the title twice.
sentry-junior
Bot
force-pushed
the
fix/canvas-read-html-to-markdown
branch
from
November 4, 2026 12:31
a9e8291 to
06a6efd
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 06a6efd. Configure here.
Contributor
|
6 screenshot changes — 6 changed · 0 added · 0 removed Review screenshots in Frameshift
|
thomasjiangcy
approved these changes
Nov 4, 2026
thomasjiangcy
added a commit
that referenced
this pull request
Nov 5, 2026
The `prepare-commit-msg` hook removes every `Co-authored-by` line, then adds only the human actors of the current run. A run with no human actor (PR event follow-ups, automations, child agents) adds nobody. So when one of those runs amends or rebases, earlier credit is gone. That's how #2647 lost dingsdax's credit: a `pull_request.comment.created` follow-up rebased the branch and force-pushed, and none of the new commits had his trailer. The hook now builds co-authors from the commits being replaced, then appends the current run's human actors. It finds those commits from Git state: - `--amend`, `-c`/`-C`, `reword`: the source commit. `--amend -m`/`-F` give no commit arg, so the hook reads `--amend` from the parent `git commit` cmdline. - rebase `pick` and `cherry-pick`: `CHERRY_PICK_HEAD`. A conflicted rebase step: `REBASE_HEAD`. An `edit` stop (`rebase-merge/amend`) that makes a new commit gets no inherited credit. - `fixup`/`squash`: HEAD plus the folded commit. Rebased merges: the `merge -C` commit. `merge --squash`: the commits listed in `SQUASH_MSG`. Model-written message text still can't add co-authors. Trailers are deduped by email, and the bot is excluded. Other bugs found while reproducing: - The author check failed on an amend or `rebase --continue` of a commit a person authored, because Git passes the original author to the hook. It now accepts the author of a replaced commit only. A new commit with a spoofed author still fails. - A one-line unscoped subject like `chore: base` was read as a trailer block, so `Co-Authored-By` got stuck right under the subject. The subject paragraph is never trailers now, and a trailer key has to start with a capital letter (or be `co-authored-by`), so `feat: x` lines in squash messages stay body text. - The comment block Git adds for an editor (conflicts, squash) broke trailer detection. Trailers now go before it. - The hook is POSIX `sh` now (passes shellcheck, tested under dash). `-ef` is replaced with a `pwd -P` comparison. Known limits: - `rebase --apply` and `git am` don't run `prepare-commit-msg`. - `-C <any sha>` reuses that commit's co-authors and author. - The `--amend -m` detection depends on `/proc`, so it only works on Linux (the sandbox). - `core.hooksPath` overrides or `commit-tree` skip the hook. This is a guardrail, not a security boundary. Follow-up, not in this PR: runs with no human actor on a PR could credit the PR's stored requester, so brand-new fixup commits get credit too. <!-- junior-request-attribution:start --> via @thomasjiangcy. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent-dispatch%3Adispatch_2f266e56e30de8bcdda52e3d36433743 --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent-dispatch%3Adispatch_2f266e56e30de8bcdda52e3d36433743) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent-dispatch%3Adispatch_2f266e56e30de8bcdda52e3d36433743/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Thomas Jiang <20501235+thomasjiangcy@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Canvases that Junior edits end up showing raw HTML. The TPM digest canvas
F0CE40T1DUHis one example. Its body is now one paragraph of escaped<div class="quip-canvas-content"><h1 …>…text. The original document is nested inside that paragraph, and its heading ids match the outer heading.Root cause:
readCanvasdownloads the canvas fromurl_privateand returns the bytes unchanged. Slack serves that download as Quip HTML, not Markdown.slackCanvasReadsays it returns Markdown, but it returns this HTML.slackCanvasEdittakes the HTML, applies the edit, and writes it back withdocument_content.type: "markdown". Slack then escapes all of it as literal text. So every edit on a real canvas breaks it. AslackCanvasWriteof content fromslackCanvasReadbreaks it the same way. The tests did not catch this because their fake download returned Markdown.Fix:
readCanvasconverts an HTML body to Markdown withnode-html-markdown, which is already a dependency. It also maps Quip's<lnk href>to a Markdown link. A body that is already Markdown does not change. Read and edit both go throughreadCanvas, so both now work on Markdown.The Slack export also starts with the canvas title as an
h1. Slack keeps that heading when the body is replaced. I checked this onF0CE40T1DUH, where the title heading kept the same id. So the read now removes a leading# <title>. Without this, every edit would show the title twice. The edit integration test now covers a Quip HTML body. It fails without this change.Remaining risk:
node-html-markdownescapes characters such as[and.. I expect Slack's Markdown parser to read those escapes as plain characters, but I have not checked this against live Slack.via @dingsdax.
--
View Session [Sentry]