fix: OSC 8 hyperlink bare URLs instead of letting them wrap-split

[than]

Overview

A bare URL is the board's own convention for something a person should click straight from the pane. That broke the moment a URL outgrew the render width — glamour's word-wrap force-split it mid-URL, leaving the terminal's link matcher with two dead fragments.

Approach

Two commits, because the first one wasn't the fix:

1. Stash/restore — keep glamour from ever seeing a bare-URL line as a wrappable token, by swapping it for a short placeholder before rendering and restoring the real URL after. Fixed the split. Verified against renderMarkdown's return value only.
2. OSC 8 hyperlink — turns out (1) wasn't the fix in the real app: bubbles' viewport.Model applies lipgloss MaxWidth to every frame, which silently truncated any line wider than the pane. Same bug, different failure mode, still broken — confirmed live with a screenshot.

Before building further on (2), checked the one thing a prior attempt at this (#18, closed unmerged after 4 review rounds) flagged as unverified and couldn't check from its environment: does bubbletea's renderer re-truncate through a non-OSC-aware truncator? Read x/ansi's truncate.go — escape bytes always pass through regardless of the visible-cell cutoff. Gate passes. OSC 8 survives the real pipeline.

Reimplemented as #18 was heading, using its four review rounds as the requirements list:

• Each URL keeps its own index-keyed placeholder end to end — no global text search, so two URLs that elide to identical visible text can't cross-link (Keep wrapped URLs clickable (#15) #18's confirmed collision bug).
• Fenced code blocks are skipped during stashing (line-by-line fence tracking) — a URL typed verbatim in a code block is never truncated or hyperlinked.
• The elision budget is measured from the actual rendered line prefix at restore time, not guessed — fixes Keep wrapped URLs clickable (#15) #18's nested-bullet-indent bug structurally instead of special-casing it.
• The OSC 8 target is percent-encoded for control/non-ASCII bytes instead of rejected — a pathological or non-ASCII URL still gets a complete, safe hyperlink.
• visibleWidth switched from muesli/reflow (CSI-only) to charmbracelet/x/ansi.StringWidth (OSC-aware) — width/padding math and "never wider than the pane" hold for hyperlinked lines too, no exemption needed.
• changedLines needed no change — its existing SGR-only strip never touched OSC 8, so the diff key already retains the link target.

Testing

• New tests in render_test.go: intact single-line hyperlinks, collision-safety, fenced-block untouched, control-byte/non-ASCII encoding, nested-indent budget, diff detection on target-only changes, and TestHyperlinkSurvivesDownstreamTruncation — the exact gate Keep wrapped URLs clickable (#15) #18 couldn't check, pinned directly against x/ansi.Truncate.
• Full suite + go vet clean.
• Manually verified in Ghostty at ~40 columns: link is clickable and opens the right place.

Closes #15

🤖 Generated with Claude Code

[claude]

Review

The core design is right. Index-keyed placeholders kill the collision class structurally, oscTarget encoding is correct and non-lossy (0x1b, BEL, DEL, non-ASCII all covered), the visibleWidth swap to xansi.StringWidth is the right call and pays off across tidy, applyLineBg, and the status bar for free, and TestHyperlinkSurvivesDownstreamTruncation pins exactly the assumption the whole approach rests on. Watcher, reload, and scroll-clamp paths are untouched — no regression there. Truecolor is preserved (termenv.TrueColor.Color(colorLink)), matching glamour's own Link style.

Four things, one of them real.

a. restoreBareURLs discards the rest of the line

style.go:263:

prefix := line[:idx]
...
lines[li] = prefix + hyperlink(url, styled)

Everything at line[idx+len(ph):] is dropped. That is safe for the board convention (- <url> as its own item — nothing follows the placeholder), which is why the suite is green. It is not safe when the URL is a soft-break line inside a multi-line paragraph:

See the docs at
https://example.test/very/long/url
for more information.

That is one paragraph. Glamour reflows soft breaks and word-wraps — the same reflow that force-split the URL in the first place — so the rendered line can be See the docs at ␟U0␟ for more, and for more is silently deleted. The elision budget has the same blind spot: it is width - visibleWidth(prefix) with nothing subtracted for what follows.

bareURLLine deliberately matches the un-bulleted indented-continuation form, so this input class is in scope by design. Fix:

rest := line[idx+len(ph):]
budget := width - visibleWidth(prefix) - visibleWidth(rest)
...
lines[li] = prefix + hyperlink(url, styled) + rest

I could not run a probe render in this environment — worth a 30-second check with a three-line paragraph before merging.

b. budget <= 0 breaks the never-wider-than-the-pane invariant

elideURL returns "…" (one cell) when budget <= 0, so the line comes out at visibleWidth(prefix) + 1 > width. Reachable at the width < 10 clamp floor with a nested bullet whose indent already fills the pane. Cheapest fix is to bail in restoreBareURLs when budget < 1 and leave prefix as-is; the URL disappearing beats the pane wrapping. TestNeverWiderThanWidth only exercises 40/60/78 against the fixture, so nothing catches this today.

c. CRLF input silently bypasses the fix

bareURLLine ends in ([ \t]*)$, and \r is in neither class — so on a CRLF board no bare URL is ever stashed and the original wrap-split bug is fully intact. tidy normalizes \r\n explicitly, so the codebase already treats CRLF as a supported input. One-character fix: ([ \t\r]*)$, or normalize at the top of stashBareURLs.

d. Indented code blocks aren't excluded, unlike fenced ones

The fence tracking is careful, but a 4-space indented code block matches bareURLLine (leading [ \t]*), gets stashed, and comes back as a styled, clickable hyperlink inside what is supposed to be verbatim text. Same decision as the fence case, opposite outcome. Minor, but it makes TestBareURLInFenceUntouched narrower than it reads.

Smaller notes

• No fallback if a placeholder fails to survive glamour — the raw \x1f-delimited token lands on screen as-is. A residual-placeholder sweep in tidy would turn a garbled failure into an invisible one.
• README drift: "Bare URLs stay on their own line, so Ghostty can detect them and make them clickable" no longer describes what happens. Worth stating the tradeoff explicitly, because it is a real one — on a terminal or multiplexer without OSC 8 support, a URL longer than the pane is now unrecoverable from the visible output, where before it was ugly but complete and selectable. Same applies to --static, whose doc comment advertises piping and CI.
• Test gaps matching the above: URL with trailing text in the same paragraph, CRLF, indented code block, budget <= 0.

go.mod movement (x/ansi to direct, reflow to indirect) is correct.

[claude]

Review

Solid PR. The approach is right, the reasoning in the description matches what the code actually does, and the hard invariants survive. A few notes, none blocking.

Verified intact

• Watcher — untouched. Still parent-directory watch plus 100ms debounce, still survives rename-swaps, deletes, recreates.
• Scroll preservation — untouched. reload still captures YOffset before SetContent and restores after; nothing in the render path changed line counts in a way that bypasses that.
• Never wider than the pane — holds structurally, not by luck. restoreBareURLs derives budget from visibleWidth(prefix) and visibleWidth(rest) measured on the real rendered line, and drops the URL rather than overflowing when budget < 1. Measuring the suffix too is what makes the soft-wrapped-paragraph case safe.
• Compact spacing, no trailing padding, hex colors — tidy and styleConfig unchanged.
• Concurrency — no new surface. Render stays synchronous inside Update.
• oscTarget percent-encoding everything below 0x20, DEL, and 0x80+ is the right call and easy to leave out. It also means the OSC target can never contain a literal ESC[0m, so applyLineBg's reset-reapply pass cannot corrupt a hyperlinked line. Worth noting that coupling somewhere, since it is load-bearing and non-obvious.
• Index-keyed placeholders instead of a global text search is the correct structural fix for the collision bug, and pinning x/ansi.Truncate behavior in a test is a good way to hold the downstream assumption.
• Switching visibleWidth to xansi.StringWidth is right for its own reason: it is the same measurement the downstream truncator uses, so the invariant and the thing enforcing it now agree.

Worth changing

a. Width-metric drift in elideURL

The pane-width invariant is now enforced with xansi.StringWidth, but elideURL builds its result with runewidth.StringWidth and per-rune runewidth.RuneWidth. Those are different measurements — x/ansi measures grapheme clusters, runewidth sums runes — and they disagree on VS16 presentation sequences (U+2764 U+FE0F: one cell by rune sum, two by cluster). A URL containing one lets elideURL return a display string that visibleWidth then measures as wider than the budget it was built against, which is exactly the over-width line the PR exists to prevent. Per-rune iteration can also cut inside a cluster.

One line fixes both, using the same package the invariant is measured with:

display := xansi.Truncate(url, budget, "…")

That is grapheme-aware, counts the tail toward the width, returns the URL unchanged when it fits — and it drops the mattn/go-runewidth direct dependency this PR promoted.

b. The 4-space skip reopens the original bug for list continuations

The len(indent) >= 4 guard treats a 4-space line as an indented code block. Under CommonMark that is only true outside a list; inside one it is a lazy continuation of the item paragraph. So this shape:

- Parent
  - Child item
    https://example.test/some-long-url

falls through to glamour and gets the pre-fix wrap-split. The comment acknowledges the tradeoff and "fail safe rather than hyperlink real code" is the right default, so this is fine to ship — but it is a real board shape, not a corner case, and the indented-code-block test only covers the non-list form. A cheap tightening: only apply the guard when the previous non-blank line is blank, which is the actual CommonMark precondition for an indented code block.

Minor

• fenceLine toggling on any 3+ run of the same char means a ~~~ line inside a ~~~~ block closes the fence early, and URLs after it in the same block get hyperlinked. Obscure, but the length check is a one-liner if you want it.
• README recommends tmux, and tmux only gained OSC 8 support in 3.4. On older tmux a long URL now renders as elided plain text with no way to recover the full address — previously it was split across two lines but fully copyable. The README wording covers this honestly; flagging it only so the tradeoff is a decision rather than a surprise. Same for copy-paste out of the pane in any terminal.
• --static piped to a file or grep now loses the full URL. Documented in the runStatic comment, and consistent with the existing decision to force truecolor when piped, so I would leave it — just noting it is now two behaviors that assume a terminal on the other end.
• residualPlaceholder.ReplaceAllString runs on every render even when no URL was stashed. Guarding on len(urls) > 0 costs nothing and makes the defensive intent clearer.
• TestNeverWiderThanWidth still only checks widths 40, 60, 78. Since the new code path is width-sensitive in a way the old one was not, a sweep over 10 through 100 on the fixture would be cheap insurance.

Test coverage for the new logic is genuinely good — the collision, nested-indent, zero-budget, CRLF, and downstream-truncation cases are the ones that matter, and testing restoreBareURLs directly for the zero-budget boundary rather than trying to coax glamour into producing it is the right instinct.

[claude]

Review

Verdict: solid. Right approach, the hard invariants hold, and the test suite is unusually good for a change of this shape — TestHyperlinkSurvivesDownstreamTruncation and TestHyperlinkedLineNeverWiderThanWidth pin exactly the two things that sank the previous attempt. go build ./..., go vet ./..., go test ./... all clean here.

What I verified

• Width invariant holds, including the hard case. The budget is measured from the actual rendered prefix and suffix, and re-measured per URL. So when two stashed URLs reflow onto one physical line, the second one's prefix already contains the first one's expanded hyperlink, and the total still sums to ≤ width. budget < 1 drops the URL instead of overflowing. That is the correct call — an over-width line is the one thing this renderer cannot do.
• No cross-linking. Index-keyed placeholders delimited by \x1f on both ends means \x1fU1\x1f cannot prefix-match inside \x1fU11\x1f. The collision bug from the earlier attempt is structurally gone, not patched.
• The visibleWidth swap is load-bearing, not incidental. applyLineBg's pad math and tidy's blank-line test both call it; with the reflow counter they would have counted the OSC 8 target as visible text and mispadded every hyperlinked line. Good catch to do this rather than exempt the URL path.
• No regressions in the areas I was asked to guard: watcher, reload, and scroll-preservation code is untouched; colors stay truecolor (termenv.TrueColor.Color(colorLink)); compact-spacing path unchanged; oscTarget percent-encodes control bytes without touching %, so an already-encoded URL is not double-encoded.

Worth addressing

a. Elide from the middle, not the tail. xansi.Truncate(url, budget, "…") keeps the head, so at a narrow pane the visible remnant is https://gi… — no information at all. Middle-elision (https://github.com/…/pull/21) stays identifiable and hand-typeable at every width. This matters more than it looks, because the visible text is now the only thing a terminal without OSC 8 gives the user, and the README recommends tmux for the split-pane setup — tmux's OSC 8 passthrough is version-dependent. Before this PR those users got a mangled-but-complete URL; after it they get a truncated one with no way to recover the tail. The change is contained entirely in restoreBareURLs and improves the OSC 8 case too.

b. Ordered-list and blockquoted URLs still hit the original bug. bareURLLine's marker group is (?:[-*+]\s+)?, so 1. https://… never matches, is never stashed, and glamour still hard-splits it mid-URL. Same for > https://…. An ordered list is plausible on a board. Widening group 2 to (?:(?:[-*+]|\d+[.)])\s+)? is a one-line fix and everything downstream already handles it.

c. runStatic already has the signal to fix the caveat you documented. term.GetSize returning an error is the non-TTY check. Rather than documenting that grep/CI readers lose the URL, thread a bool into renderMarkdown and skip the stash in the non-TTY branch — piped output then keeps the full URL as plain text. Two lines, and it removes the caveat instead of explaining it.

Notes, not blockers

• visibleWidth's swap changes width semantics for every caller, not just hyperlinked lines: go-runewidth per-rune vs. grapheme-cluster width. They disagree on ZWJ sequences and VS16 (❤️), and glamour's internal wrapper still uses reflow — so two metrics now coexist in one pipeline. The board's own markers (🧠🚧🚘✅📦) are single-codepoint and agree, and matching x/ansi is what bubbletea and lipgloss do downstream, so I think this is the right direction. Just flagging that the blast radius is wider than the URL path, and the non-hyperlink width coverage is still only 40/60/78 while the new sweep deliberately skips those lines.
• The indented-code-block guard classifies a loose list item's second paragraph (blank line, then a 4-space-indented URL under a bullet) as code, so that case keeps the old split. Narrow enough not to hold the PR for; worth a line at the guard if that is the intended tradeoff.
• If glamour ever splits a placeholder across a wrap, residualPlaceholder will not match the halves and raw \x1f bytes reach the screen. The placeholder is short enough that this should not happen at the width-10 floor, so the current defensive sweep is proportionate — just not airtight, contrary to what the comment implies.

[claude]

Review

The core approach is right, and the parts that mattered most are handled well.

What holds up:

• Switching visibleWidth to x/ansi.StringWidth was necessary, not cosmetic. applyLineBg pads to width - visibleWidth(ln) and tidy uses it for blank detection — reflow's CSI-only counter would have measured the OSC 8 target as visible text and blown the padding math on exactly the lines this PR adds. Good that it was fixed at the helper rather than exempted.
• Measuring the elision budget from the actual rendered prefix and suffix (width - visibleWidth(prefix) - visibleWidth(rest)) fixes PR 18's nested-indent bug structurally instead of special-casing it. Index-keyed placeholders kill the cross-link collision by construction.
• budget < 1 dropping the URL rather than drawing it is the correct call — the never-wider-than-the-pane invariant wins over showing something.
• changedLines genuinely needed no change: ansiRE is ESC [ ... m, so the OSC 8 target stays in the diff key. Nice that it is pinned by a test rather than asserted in prose.
• Watcher, reload, scroll clamp, and the p.Send path are untouched — no new concurrency surface, nothing to regress there. go.mod direct/indirect reclassification is correct.
• TestHyperlinkSurvivesDownstreamTruncation is the right gate to pin, and pinning it against x/ansi.Truncate directly is better than asserting it in a comment.

---

The display text is not sanitized the way the target is.

style.go:275 builds the visible text straight from the raw URL:

display := xansi.Truncate(url, budget, "…")
styled := termenv.String(display)...
lines[li] = prefix + hyperlink(url, styled) + rest

oscTarget carefully percent-encodes control bytes for the target — but display gets no such treatment, and neither xansi.Truncate nor termenv.String strips escapes (Truncate passing escapes through unconditionally is the very premise the PR verified). Critically, the URL is stashed before glamour runs, so glamour never sees these bytes and never neutralizes them; they go from the file to the terminal verbatim.

So a board line whose URL contains a literal ESC byte — say - https://x/<ESC>]8;;https://evil<BEL>pwned, no spaces so \S+ swallows it whole — emits a nested OSC 8 inside the outer one, and the visible text points somewhere the board never named. A <ESC>[2J does worse. Note the pane-width invariant does not catch this: visibleWidth scores injected escapes as zero cells, so the line measures fine while the screen does not.

TestBareURLUnsafeBytesEncoded only asserts on m[1]; m[2] is unchecked, which is why this slipped through.

Fix is small — sanitize before truncating, and assert on the display group too:

display := xansi.Truncate(stripControlBytes(url), budget, "…")

Given board files are written by an agent that routinely pastes tool output, this is worth closing.

---

Smaller things:

• Residual sweep cannot catch a split placeholder. residualPlaceholder is \x1fU\d+\x1f. If glamour ever force-splits the placeholder across two lines — reachable only at the width-10 floor with a deep indent, where limit - indent drops under the placeholder's ~2 cells — then neither strings.Index(line, ph) nor the sweep matches, and raw \x1f bytes reach the pane. That is precisely the failure the sweep exists to prevent. Adding out = strings.ReplaceAll(out, "\x1f", "") after the token sweep closes it for free.

• Mid-paragraph URLs elide much harder than own-paragraph ones. Because the placeholder is ~2 cells and the URL is not, glamour packs neighbouring words onto the same physical line, and the restore budget collapses. TestBareURLKeepsTrailingContent's own shape at width 40 leaves roughly 15 cells for a 42-char URL. Clickable in Ghostty, fine; but in a terminal without OSC 8 that URL is now unreadable and uncopyable, where pre-PR it was at least fully present across two lines. The README covers the general tradeoff, so this is a judgment call, not a blocker — just flagging that it bites the reflowed case far harder than the on-its-own-paragraph case the convention actually describes.

• Loose-list continuation reads as indented code. A blank line before a 4-space-indented continuation inside a nested list (- Parent / - Child / blank / https://…) satisfies marker == "" && len(indent) >= 4 && wasPrevBlank, so it is skipped and keeps the old wrap-split. Conservative failure, and the sibling case is tested — worth a line in the comment so the next reader knows it is a known boundary rather than an oversight.

• bareURLLine's doc comment cites CLAUDE.md for the bare-URL convention; there is no CLAUDE.md in the repo. The README rendering-style section is the live reference.

Fix the display-text sanitization; the rest is your call.

[claude]

Reviewed the full diff against style.go, diff.go, ui.go, and main.go. Built, vetted, and ran the suite locally — all clean.

The core approach is right, and the hard invariants hold. Stashing to an index-keyed placeholder is the correct shape: it structurally rules out the cross-linking collision, and measuring the elision budget from the actual rendered prefix/suffix is a real fix rather than a special-case patch. Two calls in particular are worth naming as correct:

• Swapping visibleWidth to x/ansi.StringWidth is load-bearing, not cosmetic. applyLineBg (diff.go:183) pads to width - visibleWidth(ln); with the old CSI-only counter an OSC 8 line would have measured the link target as visible cells and under-padded, and composeMarked would have mismeasured too. Getting the metric OSC-aware before emitting OSC is the right order of operations.
• Leaving changedLines' SGR-only strip alone so the diff key retains the target. Two URLs that elide to identical visible text must still diff — good that it's pinned by a test.

Watcher, reload, and scroll paths are untouched, and reload/rerenderCollapse still bracket SetContent with the YOffset save/restore. No concurrency surface changed. Nothing to flag there.

Three things:

a. The link styling is the only color path that depends on termenv's auto-detected profile

style.go:306:

styled := termenv.String(display).Foreground(termenv.TrueColor.Color(colorLink)).Underline().String()

termenv.String binds to termenv's package-global default Output, whose profile is auto-detected. Everywhere else this codebase deliberately refuses that: glamour.WithColorProfile(termenv.TrueColor) at style.go:340 ("Force truecolor so hex colors survive; piped/degraded profiles were how glow washed out"), and diff.go writes raw 38;2;r;g;b by hand. When termenv resolves the default output to Ascii, Style.Styled short-circuits and returns the bare string — so under a degraded profile the link text renders unstyled while every other color on screen stays truecolor.

It also means the styled path is never exercised by the tests: they run with stdout not a terminal, so "teal, underlined" is asserted nowhere. Matching diff.go's existing idiom fixes both:

r, g, b := hexToRGB(colorLink)
styled := fmt.Sprintf("\x1b[4;38;2;%d;%d;%dm%s\x1b[0m", r, g, b, display)

b. The residual-placeholder fallback is garbled, not invisible

style.go:351-361 says "better an invisible failure than a garbled one," but the second sweep doesn't deliver that. If glamour ever hard-breaks a placeholder across a wrap, residualPlaceholder can't match the halves, and strings.ReplaceAll(out, "\x1f", "") then strips only the delimiters — leaving a literal U and 0 on screen as visible text.

Narrow to reach (the placeholder is a single 2-cell word, so it needs a deep indent at a near-floor width), so this is a nit rather than a blocker. But the fallback should strip the token's letters and digits too — a pattern like \x1fU?[0-9]* / U?[0-9]*\x1f in place of the bare \x1f pass — otherwise the comment promises something the code doesn't do.

c. A URL reflowed into a paragraph can be elided down to nothing legible

budget = width - visibleWidth(prefix) - visibleWidth(rest) is correct for the board convention (URL as its own bullet: budget is width - 2, and testdata/REVIEW.md confirms that's the real shape). But when the bare-URL source line is a soft-wrap continuation inside a paragraph, the substitution changes the reflow itself: glamour now wraps around a 2-cell placeholder instead of the full URL, so it packs more prose onto that physical line, and rest eats most of the budget. The URL can land at a couple of cells — h… — where pre-fix it was fully readable, just split.

That's the exact fixture in TestBareURLKeepsTrailingContent, and the test only asserts the trailing prose survives and the width invariant holds — not that the URL is still legible. I'd either assert a floor on the visible display width there, or skip the stash when the URL isn't alone on its rendered line, so a paragraph-embedded URL keeps its old full-text behavior. (I derived the budget arithmetic by reading restoreBareURLs; I couldn't add a probe test in this environment to print the rendered line, so treat the exact cell count as illustrative.)

Noted, not asking for changes

• Blockquoted bare URLs (> https://…) don't match bareURLLine, so they keep the original wrap-split. Outside the board convention, but the PR says "Closes Viewer: wrapped URLs break clickable links #15" — worth knowing the gap exists.
• On a terminal without OSC 8, a long URL's full text is no longer recoverable from the screen (elided plain text, not a two-line split). A real tradeoff, but a deliberate one and the README states it plainly, so it's the author's call.
• TestHyperlinkSurvivesDownstreamTruncation pinning the x/ansi.Truncate assumption directly is the right instinct — that's the load-bearing premise of the whole approach, and it now fails loudly if it ever regresses.

Solid work overall. Item (a) is the only one I'd want fixed before merge; (b) and (c) are small.

[claude]

Review

Solid PR overall. The core design is right: index-keyed placeholders (no global text search) structurally kills the collision bug from the earlier attempt, measuring the elision budget from the actual rendered prefix/suffix beats guessing, and stripControlBytes + oscTarget is a genuinely careful pair — percent-encode where the bytes must stay meaningful, delete where they only need to be inert. Swapping visibleWidth from muesli/reflow to x/ansi.StringWidth is the correct call, not a drive-by: reflow's CSI-only scanner would have counted the OSC 8 target as visible text and quietly broken the pane-width invariant. Raw 38;2;r;g;b instead of termenv.String matches what diff.go already does and for the same reason. changedLines retaining the OSC 8 target in the diff key is the right answer, and it's pinned by a test.

Watcher, reload, and scroll-restore paths are untouched — renderMarkdown's new third arg is the only edit in ui.go, and the YOffset save / SetContent / SetYOffset sequence is unchanged in all three call sites. No new goroutines, no new shared state. Nothing to flag there.

Two things worth fixing before merge, then some nits.

a) Multi-line indented code blocks are only protected on their first line

stashBareURLs guards indented code with a per-line wasPrevBlank check (style.go:203), but an indented code block is a block — once it opens, every subsequent indented line belongs to it, blank-line-preceded or not. So:

Run these:

    https://example.test/first
    https://example.test/second

first has wasPrevBlank == true and is correctly skipped. second has wasPrevBlank == false, falls straight through the guard, gets stashed, and comes back as an elided OSC 8 hyperlink — inside verbatim content, truncated with an ellipsis. Two URLs in the same code block render differently from each other. TestBareURLInIndentedCodeBlockUntouched only covers the single-line case, so it does not catch this.

The fix is symmetric with what the fence tracker already does — carry the state instead of re-deriving it per line:

inIndentedCode := false
// ...
blank := strings.TrimSpace(line) == ""
if inIndentedCode {
    if !blank && !hasIndent4OrTab(line) {
        inIndentedCode = false
    }
} else if wasPrevBlank && !blank && hasIndent4OrTab(line) && noListMarker {
    inIndentedCode = true
}
if inIndentedCode { continue }

(Blank lines do not close an indented block — only a non-blank line indented under 4.) That also makes the existing wasPrevBlank guard redundant, so it collapses rather than adds.

b) Two bare URLs on the same reflowed line starve the second one

restoreBareURLs iterates URLs in order and computes each budget from the line's current state — correct per-URL, but order-dependent when two placeholders share a physical line. Glamour joins soft breaks into one paragraph (which is exactly what TestBareURLKeepsTrailingContent relies on), so:

Docs:
https://a.example.test/some/long/path
https://b.example.test/some/long/path

collapses to one rendered line holding both U0 and U1 placeholders — each about 2 visible cells, so glamour has no reason to wrap. URL 0 then expands to fill nearly the whole line, and by the time URL 1 is processed its budget is whatever is left: a cell or three, i.e. h… or a bare ellipsis.

The width invariant holds and both links are still clickable, so this is not a wrap-split regression. But a 1-cell link is effectively invisible, and the failure is silent. The budget < 1 rule already establishes "drop rather than render something broken" as the policy here — extending it to a small floor (drop, or do not elide below roughly 8 cells) would be consistent, or split the line's remaining width evenly across the placeholders on it. Either way this needs a test; nothing currently pins the multi-placeholder-per-line case at all.

c) Please confirm the width-15 overflow predates this PR

TestHyperlinkedLineNeverWiderThanWidth is scoped to hyperlink-bearing lines, and the comment justifies that by citing a plain blockquote line measuring one cell over at width 15. That may well be pre-existing — but this PR is also what changed the measuring function, from reflow's per-rune PrintableRuneWidth to x/ansi.StringWidth's grapheme-cluster width. Those disagree on exactly the content this fixture is full of (emoji, VS16, combining marks). Worth running the same sweep against main's reflow-based visibleWidth to establish which it is, and filing an issue either way — "never wider than the pane" is the invariant the tool exists for, and a scoped test is the right call for this PR but should not be where the finding stops.

Nits

• main.go:123 infers TTY-ness from term.GetSize returning no error. term.IsTerminal(int(os.Stdout.Fd())) is in the same already-imported package and says what it means; the width fallback can stay keyed off GetSize independently. The comment currently has to explain the conflation, which is a sign the code should just not conflate them.
• hexToRGB(colorLink) is called inside the per-URL loop in restoreBareURLs (style.go:315). Hoist it — loop-invariant.
• restoreBareURLs silently does nothing for a URL whose placeholder glamour dropped entirely (the strings.Index miss just falls off the end of the inner loop). The residual sweep cannot help there — there is nothing left to sweep. Not worth guarding against, but the doc comment implies the placeholder is always found, and it is the one path where a URL vanishes without a trace.

Comment density is heavy in places, and a couple of the longest comments are load-bearing for logic that should not need them — the wasPrevBlank guard in (a) and the TTY inference above are both cases where restructuring the code removes the need to explain it. Not blocking, just worth watching.

[claude]

Review

Solid PR, and the right call on the approach. The stash/restore + OSC 8 combination is the only way to keep a long URL clickable and keep the never-wider-than-the-pane invariant, and the reasoning about x/ansi passing escape bytes through truncation is correct — pinning that assumption in a test is exactly right, since the whole design rests on it.

Verified locally on the branch: go build, go vet, and the full suite pass. Specifically checked and happy with:

• Width invariant holds. The elision budget is measured from the real rendered prefix/suffix, and visibleWidth moving to x/ansi.StringWidth means the budget and the enforcement use the same OSC-aware metric. The 10..100 sweep over hyperlinked lines is the right shape of test. Bonus: applyLineBg's pad math in diff.go gets more correct from the same swap — muesli/reflow was counting the link target as visible cells.
• Colors. Hand-written 38;2;r;g;b rather than termenv.String is correct here, for the reason the comment gives — the codebase forces TrueColor everywhere else and termenv.String would silently degrade under a non-TTY profile.
• Diff marking. changedLines strips SGR only, so the OSC 8 target stays in the LCS key and a target-only change is still detected. Good catch that this needed no change, and better that there is a test pinning it.
• Watcher, scroll, concurrency. Untouched — watcher.go unchanged, and the ui.go diff is only the three call-site updates. Reload still does the YOffset save/restore around SetContent. No new shared state, nothing crossing the p.Send boundary.
• Compact spacing / trailing padding. tidy still runs last, after restore.
• Escape injection. Stripping control bytes from the display text is a real hazard closed, not a theoretical one, since glamour never sees the stashed URL. Right to handle both halves differently — percent-encode the target, strip the display.

One real bug: the budget math is wrong when two placeholders share a rendered line

restoreBareURLs computes budget as width - visibleWidth(prefix) - visibleWidth(rest), but when it processes URL i, rest still contains the unexpanded placeholders for URLs i+1..n. A placeholder measures ~2-3 cells; the hyperlink it becomes measures up to the full pane. So the first URL on the line is handed a budget that assumes the later ones need almost no room, and it takes nearly the whole width. The later ones then compute their own (correct) budget against an already-full line and come out at 1-2 cells — h… — or hit budget < 1 and get dropped outright, link and all.

This is reachable with a shape the board convention actually produces: two bare URLs as consecutive continuation lines of the same list item or paragraph, e.g.

- Working on it
  https://github.com/example/app/pull/411
  https://github.com/example/app/pull/412

Those are one paragraph, so glamour reflows both placeholders onto a single rendered line — the same reflow that TestBareURLKeepsTrailingContent already demonstrates for trailing prose. The existing collision test does not cover this because it uses two separate list items, which land on separate lines.

The pane-width invariant itself survives (each later URL measures against the real expanded prefix, and budget < 1 drops rather than overflows), so this is not the wrap-split bug coming back — but the second URL is effectively unusable or invisible, which is worse than the pre-PR wrap-split for that line.

Fix: make the unit of work the rendered line rather than the individual URL. Find every placeholder on the line, compute the available width once as width minus the visible width of the line with all placeholders removed, then divide that across the placeholders on the line before truncating each. That also removes the current O(len(urls) x len(lines)) rescan-from-line-zero, since one pass over the lines handles everything.

Worth a test with two bare URLs in one paragraph asserting both targets are intact and both display texts are non-trivial.

Minor

• Fence state can be opened from inside an indented code block. The fenceLine check runs before the indented-code tracking and ignores inIndentedCode, so a stray ``` or ~~~ line inside a 4-space-indented block (or a line that merely starts with ~~~) flips fenceChar on. If it never finds a matching close, every bare URL for the rest of the document silently reverts to the old wrap-split. Cheap guard: skip the fence check while inIndentedCode is set, which means hoisting the indented-code tracking above it.
• Trailing whitespace is double-counted. The stash preserves the source line's trailing whitespace into rest, so the budget is reduced by spaces that tidy strips a moment later. Harmless direction, just a cell or two of unused budget — dropping group 4 rather than reinserting it would be simpler.
• Comments citing prior PR numbers and review rounds ("the collision bug from the closed PR", "Round-7 review's confirmed bug") read as review history rather than code documentation and will age badly once that context is gone. The behavioural half of each comment is genuinely valuable — keep that, drop the provenance. The commit message and this PR body are the right home for it. Related: style.go is now ~65% comment by line in the new section; a few of these explain the same decision twice.

Nothing blocking except the shared-line budget. Fix that and I think this ships.

[claude]

Review

Solid overall. The width math holds, the OSC 8 handling is careful where it needs to be (percent-encoded target, control bytes stripped from the display text, index-keyed placeholders so two URLs cannot cross-link), and the visibleWidth switch to x/ansi is the right call — applyLineBg’s pad-to-width and the status bar both need an OSC-aware measure now. Watcher, reload, and scroll preserve/clamp are untouched. Full suite passes locally.

One real bug.

Fence tracking breaks when a fence follows an indented code block

In stashBareURLs, the fence-open check lives in the else branch of the inIndentedCode state machine. The line that closes an indented code block takes the if branch, clears the flag, and falls straight through to the bare-URL match — it never gets fence-checked.

Repro:

intro

    indented code

```
https://example.test/long-url-inside-a-fence
```

Trace: indented code sets inIndentedCode; the blank line hits continue; the opening clears `inIndentedCode` and skips the fence check, so **no fence opens**. The URL line is then stashed, hyperlinked, and elided — exactly what the PR states fenced blocks are protected from. Second-order effect: the *closing* does reach the else branch and opens a fence, so from there to the end of the document (or the next matching run) every real bare URL is silently skipped and the fix stops applying.

Fix is to hoist the fence check out of else. After the inIndentedCode block clears the flag, indented is false for that line by construction, so the indented-code-open check cannot misfire:

if inIndentedCode {
	if blank || indented {
		continue
	}
	inIndentedCode = false
}
if m := fenceLine.FindStringSubmatch(line); m != nil {
	fenceChar, fenceLen = m[1][0], len(m[1])
	continue
}
if wasPrevBlank && !blank && indented && !listMarkerPrefix.MatchString(line) {
	inIndentedCode = true
	continue
}

TestBareURLInFenceUntouched does not catch this because its fence starts at document position 0. Worth a case with the full sequence: indented block, blank, fence, URL, fence, then a real bare URL that must still be hyperlinked.

Smaller notes

a. The non-TTY path still splits URLs mid-token. linkify=false skips stashing entirely, so sidecar --static | grep gets glamour’s original hard break — the URL is un-elided but still broken across two lines, so it is neither greppable nor copy-pasteable. renderMarkdown’s doc comment ("plain, complete, un-elided text") and runStatic’s ("a full, plain, un-elided URL serves it better") oversell what that reader actually gets. Either stash-and-restore the plain full URL on that path too, or soften the comments to describe what happens. Reasonable to leave the behavior as out of scope for issue #15 — but the comment should not claim the stronger property.

b. Even split of the shared line budget. Two placeholders on one physical line each get available/2, even when the first needs 8 cells and the second needs 60. The comment correctly explains why the shared budget beats the sequential version; a "give each what it needs, split the surplus" pass would elide less. Fine to leave.

c. visibleWidth is now grapheme-cluster-aware everywhere, not just on hyperlinked lines — x/ansi.StringWidth clusters where reflow.PrintableRuneWidth counted runes. Right direction, and it is what makes the OSC-aware padding correct, but it also quietly changes status-bar and applyLineBg padding for the board’s emoji headings. Related: truncateTo (ui.go:497) still measures per-rune, which now disagrees with the cluster-aware metric for any multi-rune cluster. It only sees the ASCII "updated Ns ago" string today, so it is harmless — flagging it because the metric moved underneath it.

Nothing else of substance: colors stay truecolor hex (raw 38;2;r;g;b, and the reasoning for avoiding termenv.String is right), the compact-spacing/tidy path is unchanged, and neither residual-placeholder sweep can corrupt a link target since oscTarget percent-encodes \x1f.

[claude]

Review

This is solid work, and the approach is right. Verified locally: go vet clean, go test -race ./... passes, and watcher.go / the reload+scroll path in ui.go are untouched apart from the three renderMarkdown call sites, so the rename-swap/delete/recreate behaviour and SetYOffset preservation carry over unchanged.

On the hard requirements:

• Never wider than the pane — holds by construction, not just by test. In restoreBareURLs, available is measured against the line with placeholders removed, and each display text is cut to available / len(matches), so the reassembled line is bounded by width. available < len(matches) falls into the share < 1 drop, so it can't go negative-then-overflow. The padding-strip (TrimRight + xansi.Truncate to realWidth) is the right fix for the starved-budget problem — measuring glamour's MarginWriter filler as content would have been silent forever otherwise.
• Colors — good call using a hand-written \x1b[4;38;2;r;g;bm instead of termenv.String; that's consistent with diff.go and survives the degraded-profile case the tests actually run under.
• Compact spacing — tidy still runs last and is unaffected; the OSC 8 terminator is BEL, so TrimRight(line, " ") can't eat into it.
• Concurrency — no change, and none needed.
• The visibleWidth swap to x/ansi.StringWidth is correct and arguably overdue: it's the same metric lipgloss/bubbletea use downstream, so the width the invariant asserts is now the width the truncator applies.

Things worth changing:

a. \x1f in the source isn't neutralized before stashing. restoreBareURLs searches the rendered output for \x1fU<n>\x1f, but nothing strips \x1f from raw on the way in. A board line that already contains those bytes — from an agent pasting raw tool output, which is the exact threat model stripControlBytes cites — reaches the output intact and gets substituted with urls[0]'s hyperlink. The comment claiming "nothing else in this pipeline emits it" is true of the pipeline but not of the input. One line at the top of stashBareURLs closes it:

raw = strings.ReplaceAll(raw, "\x1f", "")

b. A dropped URL takes the whole line with it. When share < 1 the placeholder is replaced with nothing, leaving only glamour's indent — which tidy then trims to empty and collapses into the neighbouring blank run. So the line doesn't render as "URL elided", it vanishes. The comment frames this as "dropped rather than drawn", which undersells it. It needs a genuinely pathological indent to trigger, so this is a note rather than a blocker, but emitting a single … at share >= 1-equivalent cost would keep the line visible for the same width guarantee.

c. Leftover budget isn't reclaimed. share := available / len(matches) splits evenly, so a short first URL on a shared line leaves its unused cells on the floor and the integer remainder is discarded. Cosmetic, and only on multi-URL lines — mentioning it so the even split reads as a deliberate simplification rather than an oversight.

d. Trim the review archaeology out of the comments. Several of the new comments narrate the PR's own review history rather than the code: "Round-7 review's confirmed bug", "Round-9 review's confirmed bug", "PR #18's confirmed bug", "the 'keep trailing content' fix this comment replaces". A few of these run 15+ lines against 3 lines of code. That history belongs in the commit messages and this thread — six months from now a reader needs to know what the code does and why, not which review round found what. The invariant statements inside them are worth keeping; the round numbers and the references to superseded implementations aren't. Same for the test comments. This codebase's existing comments (diff.go, ui.go) are explanatory without being historical — match that.

Nothing here blocks merge except your own call on (a), which I'd take.