You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Embed a URL that stands alone on its line in rich text - #927
An agent that writes a URL on a line by itself in a card, comment, message, document, to-do or schedule entry now gets an embed, the way a person does by pasting the link in the editor: a Dash chart, a YouTube video, anything Basecamp's oEmbed providers cover.
A URL alone on its line in Markdown content becomes an embed. No flag, no new syntax. That's the shape agents already write (the card's report was four Dash links, one per line), and it's what the editor does on paste. Writing [title](url) keeps a link, and it's already the form the skill asks for when a link is meant as a link. A URL inside a sentence, a list, a quote, a table, a heading or code is left alone, as is everything sent with --format html.
I considered and rejected:
An --embed <url> flag. It's one more thing to learn, and it can't say where in the body the embed goes.
Embedding every provider URL anywhere in the text. It would turn a link mentioned in a sentence into a player.
basecamp embeds create <url> and basecamp embeds show <sgid> cover the new SDK methods. They're also how to embed with --format html: create waits for the embed and prints its attachment tag.
How it works
richtext.MarkdownToHTMLWithEmbeds parses once and collects the URLs that stand alone on a line of a top-level paragraph. It asks an EmbedFunc for the ones it can embed, splits each paragraph around them into <bc-attachment sgid="…" content-type="…"> blocks, then renders. MarkdownToHTML is that with no EmbedFunc, so its output doesn't change.
richTextToHTML passes embedLinks(cmd), so every rich-text command gets this in one place. The EmbedFunc runs under one 10-second context that bounds every embed request. It creates the embeds, then polls the pending ones together once a second, as the editor does, and inserts only the active ones. Rich text that's already rendered is cached, so an embed inserted while pending could stay a link.
Nothing fails the post. A URL Basecamp can't embed (422), or a Basecamp without this endpoint (404), stays a link with no warning. A refusal, a 429, a provider with nothing to embed or too slow to answer, or a hung request also stays a link, with a one-line warning on stderr. The first failure that isn't a refusal ends the embed step, so a struggling server costs the post one request, not one per URL.
Attacked
Bad URLs: javascript:, ftp:, mail autolinks, a URL with markup, a URL with an escape or a character reference (&), two URLs on a line, and a URL after a <bc-attachment> are never sent. The server refuses non-provider URLs itself.
HTML passthrough: --format html makes no embed requests and sends the HTML byte for byte. Raw HTML blocks aren't paragraphs, and a paragraph holding inline HTML anywhere is left whole, since splitting it would break the HTML. EmbedToHTML escapes what it writes.
Person versus agent token: the CLI doesn't distinguish them, and the server allows both. A refusal (403) stays a link with a warning.
Rate limits: a 429 stays a link with a warning. The server's embed 429 is an hourly verdict with no Retry-After. The SDK models it as not retried, and its hook names the operation CreateEmbed, so isVerdict429 doesn't set the CLI-wide 60-second block that would otherwise hold back the post.
The same URL twice is created once and embedded at both places.
Also
--format help on every rich-text command says a URL alone on its line becomes an embed.
skills/basecamp/SKILL.md documents it with an example (11.4 KB of the 16 KB budget), and skill-evals/cases/create-card-embed.yml checks that an agent asked for an inline chart puts the bare URL on its own line.
The MCP server's catalog serves create_embed and get_embed in the files domain, from the vendored model synced at the same SDK commit.
TestGatingHooksHeaderless429BlocksUnlessTheOperationTreatsItAsAVerdict now pins that the embed limit's 429 sets no block. It fails against the SDK's first commit, before the fix.
Catalog, root, commands_test.go, .surface, e2e/smoke/smoke_embeds.bats and API-COVERAGE.md (209 of 209).
Not done, on purpose
Embed requests share the circuit breaker the post goes through. If the breaker already holds four failures, one failed embed request could open it before the post. The embed step makes at most one failed request per post, which is the same exposure as the mention lookups and image uploads that already run before a post. Isolating it would mean a second SDK client for one optional step.
Depends on
go.mod pins basecamp-sdk at its v0.27.0 release (make bump-sdk REF=v0.27.0, which resolves the go/v0.27.0 module tag), the release that carries basecamp/basecamp-sdk#1019. The MCP model is re-synced from that tag and the Nix vendorHash is updated.
The server side, basecamp/bc3#13962, is merged and has been deployed to production since 2026-10-21.
🤖 Pi review summary (openai-codex/gpt-6.1-sol, thinking high, read-only): four rounds, with 6, 4, 1 and 0 findings. Every finding was checked against the code before I acted on it.
The 10-second budget didn't bound the create requests or the polls. Verified: a hung create held the post for the SDK's full timeout, 30 seconds in a test. Fixed in 27fb024 with one context over every embed request. Test: TestCommentsCreatePostsWhenAnEmbedRequestHangs.
One slow provider used up the shared deadline, so later embeds were never polled. Verified. Fixed in 27fb024: pending embeds are polled together. Test: ...KeepsPollingTheOthersWhileOneProviderIsSlow.
A URL inside multi-line inline HTML was embedded, splitting the HTML. Verified. Fixed in 27fb024: a paragraph holding inline HTML is left whole.
& in a URL reached Basecamp undecoded. Verified. Fixed in 27fb024: a URL with a character reference stays a link, as one with a backslash escape already did.
Repeated embed 5xx could open the circuit breaker the post goes through. Verified. Fixed in 27fb024: the first failure that isn't a refusal ends the embed step. Test: ...StopsAskingForEmbedsAfterAFailure.
An embed 429 set a CLI-wide 60-second block that rejected the post. Verified (hooks.go). Fixed in the SDK: CreateEmbed doesn't retry on 429, so isVerdict429 treats it as a verdict. See round 2 for the rest.
A create failure still polled embeds created before it.Fixed in f2c94f4. Test: ...StopsPollingAfterAFailure.
isVerdict429 looks up the hook's operation name, and the SDK passed "Create", not "CreateEmbed", so finding 6 still blocked. Verified. Fixed in the SDK, which now passes full operation IDs. Bumped in 2a2323c, and a resilience test pins CreateEmbed as a verdict. It fails against the earlier SDK pin.
HTML nested inside emphasis wasn't seen.Fixed in f2c94f4: the whole paragraph is walked.
With four failures already on the breaker, one embed failure could open it before the post. True. Declined: the embed step now makes at most one failed request per post. That's the same exposure as the mention lookups and image uploads that already run before a post, and isolating it would take a second SDK client for one optional step. The PR description records this.
Adds automatic embeds for standalone URLs in Markdown rich text across the CLI, matching Basecamp’s editor behavior.
Changes:
Converts eligible standalone URLs into ready embeds, with fallback to links.
Adds embeds create and embeds show, plus MCP embed operations.
Updates SDK dependencies, documentation, and regression coverage.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
The URL pattern accepts control characters such as ESC and BEL. If an embed request fails or times out, resolveEmbeds prints the original URL directly to stderr, so untrusted Markdown/stdin can inject terminal escape sequences into the warning. Reject Unicode control characters when selecting embed candidates; this keeps those URLs out of the new request and warning path.
When the server has not deployed the embeds endpoint, each create request returns 404. Treating that like a URL-specific 422 sends another request to the same missing endpoint for every remaining URL, potentially spending the full ten-second embed budget before posting. Stop the create loop silently on 404; continue only for 422. Add a multiple-URL regression test asserting that a missing endpoint causes just one create request and leaves the content unchanged.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Endpoint fallback needs correction, and the unreleased SDK dependency and server rollout require final integration validation.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Stop embed creation loop after a 404 response
internal/commands/embeds.go:194
When Basecamp does not yet serve /embeds.json, this continue sends another failing POST for every remaining distinct URL. Four standalone URLs therefore cause four 404 responses before posting, potentially using the full 10-second embed budget. Stop the creation loop quietly on 404, since the endpoint is unavailable, while continuing past URL-specific 422 refusals. Add a regression test with multiple URLs that checks only one request is made after a 404.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The SDK release prerequisite and live server compatibility still require final validation, alongside an unresolved fallback issue.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Avoid retrying missing /embeds.json for every URL
internal/commands/embeds.go:194
When Basecamp does not serve /embeds.json yet, this continue retries the missing endpoint for every distinct standalone URL. The four-link example makes four failing requests, and larger bodies can spend the full 10-second embed budget before posting. Stop quietly on 404; continue only for URL-specific 422 refusals. Add a regression test with multiple URLs that verifies one create request, no warning, and a successful post with the URLs unchanged.
🤖 Copilot's review-body finding on c0939cb (Avoid retrying missing /embeds.json for every URL) is fixed in 264b493. A 404 from create means this Basecamp doesn't serve the endpoint yet, so the embed step now stops at the first one, quietly, and posts the URLs unchanged. TestCommentsCreateStopsQuietlyWhereBasecampHasNoEmbeds covers two URLs: one create request, no warning, and the post's content unchanged. It fails with the old continue.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
SDK release and server integration need human verification, and unresolved evaluation and smoke-runner issues weaken validation.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Include smoke_embeds.bats in the smoke test runner
e2e/smoke/smoke_embeds.bats:17
make smoke runs e2e/smoke/run_smoke.sh, whose three explicit file lists omit smoke_embeds.bats. Neither endpoint is exercised by that runner, while scripts/check-smoke-coverage.sh:108 still counts both commands through its *.bats glob. Register this file in the appropriate smoke-runner list so the create/show round trip actually runs.
Accept URL-only positional bodies in card CLI eval
skill-evals/cases/create-card-embed.yml:6
skill-evals/run:213–215 applies this regex to the raw CLI arguments, not the card body. A valid invocation such as cards create "Checkout latency" "https://dash.37signals.com/widgets/abc" --in 55555 therefore fails the eval, although its entire body is a standalone URL and the CLI embeds it. Accept URL-only positional bodies as well as heredoc input, or grade the parsed body so equivalent valid invocations pass.
🤖 Copilot's two review-body findings on 264b493 are fixed in f768395:
Include smoke_embeds.bats in the smoke test runner: it's now in run_smoke.sh's write-level list, beside smoke_communication_write.bats, since embeds create writes.
Accept URL-only positional bodies in card CLI eval: the accept pattern now takes the URL as a whole quoted argument, as well as on a line of its own. A titled link and a URL inside a sentence still don't match. I checked four sample invocations under Onigmo.
🤖 Converged on f768395: CI is green, Copilot reported on this head with no open issues, it merges cleanly, and no threads are open. Pi: 6, 4, 1, 0, plus clean rounds on the later commits. Copilot's three review-body findings are fixed. Nothing is merged or armed.
An agent writing a card, comment, message, document, to-do or schedule
entry had no way to embed a video or a Dash chart: a URL stayed a link,
where a person pasting it in the editor gets an embed. Now, in Markdown
content, a URL alone on its line becomes an embed when Basecamp can embed
it. A titled link, a URL inside a sentence, list, quote, table or code,
and anything sent with --format html are left as written.
The CLI creates every embed first, then waits for them together, polling
as the editor does, and inserts only the ones that are ready. A URL
Basecamp can't embed stays a link quietly; a refusal, a rate limit, or a
provider with nothing to embed or too slow to answer stays a link with a
warning. Nothing fails the post.
embeds create and embeds show cover the new SDK methods, and are how to
embed with --format html. The MCP catalog serves them in the files
domain. The skill, --format help, API coverage, surface snapshot and a
smoke test say so.
…nces alone
One ten-second budget now covers every embed request, not just the
waiting, so a hung request can't hold the post. Pending embeds are polled
together, so one slow provider no longer stops the others being seen. The
first failure that isn't a refusal ends the embed step, leaving the rest
as links, so a struggling server costs the post one request, not one per
URL, and can't trip the circuit breaker the post goes through.
A paragraph holding inline HTML is left as written, since splitting it
would break the HTML, and so is a URL with a character reference, which
would reach Basecamp undecoded.
… paragraph
A create failure stopped further creates but still polled the embeds
created before it; now it ends the step. Inline HTML inside emphasis or a
link also keeps its paragraph as written.
CreateEmbed no longer retries on 429 and its hooks name the operation
CreateEmbed, so a headerless 429 from the hourly new-embed limit sets no
CLI-wide block that would hold back the post it was for.
URLs and server error text in the embed warnings go through
SanitizeSingleLine, so a control character or a newline can't restyle
the terminal or forge a line. The styled renderer turns any HTML into a
placeholder, so the help says the attachment tag comes in --json's
attachment field.
make smoke lists its files, and smoke_embeds.bats wasn't among them. The
eval also takes the chart URL as the whole positional body, which the CLI
embeds the same way.
🤖 Rebased onto main (ccfc3c8) and repinned the SDK at #1019's rebased head, f951b827. This PR is now at 93b2e72.
Rebase: mechanical. range-diff shows all 12 commits unchanged. The old and new diffs are identical except two index lines, in go.mod and nix/package.nix, where main changed other hunks (the go1.26.10 toolchain and the 0.15.1 version).
Repin (2161546c, amended into 93b2e72): make bump-sdk REF=f951b827…, scripts/sync-mcp-model.sh and make update-nix-hash. The vendored model files are byte-identical, and only PROVENANCE.json moved, since #1019's Go content didn't change. Nix verified the new vendorHash with a real build.
CI: green, 36 pass and 4 skipped. go test ./... also passes locally.
Reviews: Copilot reviewed 93b2e72 and left one thread, the move to a released SDK. I answered it and left it open on purpose, so this PR can't merge on the development pin. That's the only open thread.
Against production, I built this head with -buildvcs=false and ran it as Rob, an API client, in an internal test project. I read each post back through the API:
A YouTube URL alone on its line became a <bc-attachment content-type="embed/youtube-video"> holding the player, in a document and in a comment. embed/youtube-video is the content type the server stores for this embed.
A URL no provider embeds stayed a link. The post succeeded with no warning, and a YouTube URL after it in the same document still embedded.
A titled link and a URL inside a sentence stayed links.
The comment reused a URL Basecamp already had, and embedded with the same sgid.
embeds create and embeds show worked, and a non-provider URL got usage: Basecamp can't embed … / Link to it instead.
Not run: the same cases as a personal agent. The agent profile's token had expired, and the server refused to mint a new one (invalid_client). That needs a fresh agent login by a person.
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
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.
github-actions
Bot
added
commands
An agent that writes a URL on a line by itself in a card, comment, message, document, to-do or schedule entry now gets an embed, the way a person does by pasting the link in the editor: a Dash chart, a YouTube video, anything Basecamp's oEmbed providers cover.
Card: Agents can't add embeds to rich text
Server: https://github.com/basecamp/bc3/pull/13962
SDK: basecamp/basecamp-sdk#1019
The design
A URL alone on its line in Markdown content becomes an embed. No flag, no new syntax. That's the shape agents already write (the card's report was four Dash links, one per line), and it's what the editor does on paste. Writing
[title](url)keeps a link, and it's already the form the skill asks for when a link is meant as a link. A URL inside a sentence, a list, a quote, a table, a heading or code is left alone, as is everything sent with--format html.I considered and rejected:
--embed <url>flag. It's one more thing to learn, and it can't say where in the body the embed goes.basecamp embeds create <url>andbasecamp embeds show <sgid>cover the new SDK methods. They're also how to embed with--format html:createwaits for the embed and prints itsattachmenttag.How it works
richtext.MarkdownToHTMLWithEmbedsparses once and collects the URLs that stand alone on a line of a top-level paragraph. It asks anEmbedFuncfor the ones it can embed, splits each paragraph around them into<bc-attachment sgid="…" content-type="…">blocks, then renders.MarkdownToHTMLis that with noEmbedFunc, so its output doesn't change.richTextToHTMLpassesembedLinks(cmd), so every rich-text command gets this in one place. TheEmbedFuncruns under one 10-second context that bounds every embed request. It creates the embeds, then polls the pending ones together once a second, as the editor does, and inserts only theactiveones. Rich text that's already rendered is cached, so an embed inserted while pending could stay a link.Attacked
javascript:,ftp:, mail autolinks, a URL with markup, a URL with an escape or a character reference (&), two URLs on a line, and a URL after a<bc-attachment>are never sent. The server refuses non-provider URLs itself.--format htmlmakes no embed requests and sends the HTML byte for byte. Raw HTML blocks aren't paragraphs, and a paragraph holding inline HTML anywhere is left whole, since splitting it would break the HTML.EmbedToHTMLescapes what it writes.Retry-After. The SDK models it as not retried, and its hook names the operationCreateEmbed, soisVerdict429doesn't set the CLI-wide 60-second block that would otherwise hold back the post.Also
--formathelp on every rich-text command says a URL alone on its line becomes an embed.skills/basecamp/SKILL.mddocuments it with an example (11.4 KB of the 16 KB budget), andskill-evals/cases/create-card-embed.ymlchecks that an agent asked for an inline chart puts the bare URL on its own line.create_embedandget_embedin the files domain, from the vendored model synced at the same SDK commit.TestGatingHooksHeaderless429BlocksUnlessTheOperationTreatsItAsAVerdictnow pins that the embed limit's 429 sets no block. It fails against the SDK's first commit, before the fix.commands_test.go,.surface,e2e/smoke/smoke_embeds.batsandAPI-COVERAGE.md(209 of 209).Not done, on purpose
Embed requests share the circuit breaker the post goes through. If the breaker already holds four failures, one failed embed request could open it before the post. The embed step makes at most one failed request per post, which is the same exposure as the mention lookups and image uploads that already run before a post. Isolating it would mean a second SDK client for one optional step.
Depends on
go.modpins basecamp-sdk at its v0.27.0 release (make bump-sdk REF=v0.27.0, which resolves thego/v0.27.0module tag), the release that carries basecamp/basecamp-sdk#1019. The MCP model is re-synced from that tag and the Nix vendorHash is updated.The server side, basecamp/bc3#13962, is merged and has been deployed to production since 2026-10-21.