fix: render images referenced from the local filesystem by blimmer · Pull Request #165 · contextbridge/planbridge

blimmer · 2026-05-18T16:09:39Z

Summary

Plan markdown containing local image references rendered as broken images because the browser tried to resolve filesystem paths against the localhost annotation server. This PR extracts local, allowlisted image files at annotation startup, embeds their bytes as JSON-safe base64 assets on AnnotationPayload, and serves those embedded assets through /assets/:id for the current browser UI. The payload is now portable enough for future shareable plan links while the markdown content remains byte-identical end-to-end. Closes #148.

Review focus

Please scrutinize the asset contract and security boundary: image bytes now live in the payload as dataBase64 plus mimeType, and the server only serves assets by opaque ID from the in-memory payload map. There are no request-time filesystem reads and no network/data-URL fetching during extraction.

The extraction limits are also deliberate: local images are capped at 10 MiB each and 25 MiB total, SVG remains excluded, and unsupported/empty/oversized images are skipped with logs instead of failing the review session. That keeps accidental huge/private payloads bounded, but it also means a skipped local image can still render as the original unresolved markdown path.

Commits

Stack is ordered by layer for commit-by-commit review: shared payload contract → server serving → browser rendering → CLI extraction/inlining.

2038ab1 — feat(shared): add portable annotation assets
d80053a — feat(server): serve embedded annotation assets
6069173 — feat(annotation): render payload image assets
cf16ccf — fix(cli): inline local markdown images as assets

blimmer · 2026-05-18T17:28:51Z

I've gotta run to my lunch meeting w/ Ron so didn't have time to rebase the commits. Feel free to look now, or wait until I'm back to have an agent rebase

blimmer · 2026-05-18T20:55:24Z

+
+  const components = {
+    ...markdownComponents,
+    img: ({ src, alt, ...rest }: ComponentPropsWithoutRef<'img'>) => {


Snags images defined in our assets metadata and serves them from the purpose-built CLI endpoint.

blimmer · 2026-05-18T21:38:32Z

        setTimeout(() => resolveResult(parsed.data), 0);
        return new Response(null, { status: 204, headers: { connection: 'close' } });
      }
+      if (req.method === 'GET' && url.pathname.startsWith('/assets/')) {


This method is getting gnarly AF - see #170 for a refactor

Represent annotation assets as JSON-safe base64 payload records and add shared test factory coverage.

Serve only payload-provided assets by opaque ID and decode their base64 content for the local browser UI.

Thread payload assets into markdown rendering and rewrite matching local image references to the server asset route.

Extract only local allowlisted images, support file URLs and path-relative refs, encode bytes as base64, and enforce per-asset/total size limits before starting the annotation server.

jcarver989

Left some thoughts on cleaning up the big function you mentioned, but nothing blocking.

jcarver989 · 2026-05-19T22:26:49Z

+import { visit } from 'unist-util-visit';
+import type { CliContext } from '#src/context.ts';
+
+export interface ExtractAssetsInput {


Ah yea I see what you were talking about know in our earlier discussion. Some observations/thoughts here:

1.

I'm not sure if you'd find this nicer, but the current code is doing a lot of check x and log y + return null for things that are essentially errors (or perhaps more accurately rejections), so you might consider creating a type like:

export type AssetExtractionIssue = IssueA | IssueB | IssueC...

That would allow several of these functions to return a Result and you could log only at the top level.

2.

You might consider a AssetFilter type that looks something like: `type AssetFilter = (candidate) => Result<void, AssetExtractionIssue>

That'd allow you to compose different filter rules together more easily,

I like it - thanks

Replaces null/false rejection signals across extractAssets with an AssetExtractionIssue discriminated union returned via Result. Logging moves out of the individual checks into a single logIssue switch at the collection/load boundary, and the per-step checks are typed as AssetFilter<T> so future filters compose the same way. Addresses Josh's PR feedback on #165.

blimmer · 2026-05-19T14:18:25Z

+
+const MIB = 1024 * 1024;
+export const MAX_ASSET_BYTES = 10 * MIB;
+export const MAX_TOTAL_ASSET_BYTES = 25 * MIB;


I don't expect this large of images, but I figured I'd have some safety mechanism since we base64 encode the data.

blimmer · 2026-05-19T23:04:35Z

+import { visit } from 'unist-util-visit';
+import type { CliContext } from '#src/context.ts';
+
+export interface ExtractAssetsInput {


I like it - thanks

🤖 I have created a release *beep* *boop* --- ## [0.8.0](v0.7.2...v0.8.0) (2026-05-20) ### Features * add planbridge-last skill to open the last agent message ([#189](#189)) ([dbfc615](dbfc615)) ### Bug Fixes * prevent data loss with onbeforeunload warning ([#188](#188)) ([ea1f5d5](ea1f5d5)) * render images referenced from the local filesystem ([#165](#165)) ([bdf98ff](bdf98ff)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: contextbridge-pr-automation[bot] <259134118+contextbridge-pr-automation[bot]@users.noreply.github.com>

blimmer marked this pull request as ready for review May 18, 2026 17:28

blimmer requested a review from jcarver989 as a code owner May 18, 2026 17:28

blimmer mentioned this pull request May 18, 2026

fix: allow displaying images in plans from local filesystem #151

Closed

blimmer marked this pull request as draft May 18, 2026 20:13

blimmer force-pushed the fix/plan-image-inlining-148 branch from a82cdc7 to b73e73b Compare May 18, 2026 20:36

blimmer changed the title ~~fix: render local-path plan images via in-memory asset map~~ fix: render local markdown images from portable assets May 18, 2026

blimmer force-pushed the fix/plan-image-inlining-148 branch 2 times, most recently from 0b36af4 to 8cd622f Compare May 18, 2026 20:53

blimmer commented May 18, 2026

View reviewed changes

blimmer marked this pull request as ready for review May 18, 2026 20:59

blimmer commented May 18, 2026

View reviewed changes

blimmer changed the title ~~fix: render local markdown images from portable assets~~ fix: render images in plans May 18, 2026

blimmer changed the title ~~fix: render images in plans~~ fix: render images referenced from the local filesystem May 18, 2026

blimmer added 3 commits May 19, 2026 07:42

feat(shared): add portable annotation assets

2038ab1

Represent annotation assets as JSON-safe base64 payload records and add shared test factory coverage.

feat(server): serve embedded annotation assets

d80053a

Serve only payload-provided assets by opaque ID and decode their base64 content for the local browser UI.

feat(annotation): render payload image assets

6069173

Thread payload assets into markdown rendering and rewrite matching local image references to the server asset route.

blimmer force-pushed the fix/plan-image-inlining-148 branch from 8cd622f to cf16ccf Compare May 19, 2026 13:48

fix(cli): inline local markdown images as assets

2460134

Extract only local allowlisted images, support file URLs and path-relative refs, encode bytes as base64, and enforce per-asset/total size limits before starting the annotation server.

blimmer force-pushed the fix/plan-image-inlining-148 branch from cf16ccf to 2460134 Compare May 19, 2026 14:32

jcarver989 approved these changes May 19, 2026

View reviewed changes

blimmer and others added 2 commits May 19, 2026 16:59

Merge branch 'main' into fix/plan-image-inlining-148

69ddf24

blimmer commented May 19, 2026

View reviewed changes

blimmer enabled auto-merge (squash) May 19, 2026 23:05

blimmer merged commit bdf98ff into main May 19, 2026
14 checks passed

blimmer deleted the fix/plan-image-inlining-148 branch May 19, 2026 23:06

contextbridge-pr-automation Bot mentioned this pull request May 19, 2026

chore(main): release 0.8.0 #183

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: render images referenced from the local filesystem#165

fix: render images referenced from the local filesystem#165
blimmer merged 6 commits into
mainfrom
fix/plan-image-inlining-148

blimmer commented May 18, 2026 •

edited

Loading

Uh oh!

blimmer commented May 18, 2026

Uh oh!

blimmer May 18, 2026

Uh oh!

blimmer May 18, 2026

Uh oh!

jcarver989 left a comment

Uh oh!

jcarver989 May 19, 2026

Uh oh!

blimmer May 19, 2026

Uh oh!

blimmer May 19, 2026

Uh oh!

blimmer May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

blimmer commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review focus

Commits

Uh oh!

blimmer commented May 18, 2026

Uh oh!

blimmer May 18, 2026

Choose a reason for hiding this comment

Uh oh!

blimmer May 18, 2026

Choose a reason for hiding this comment

Uh oh!

jcarver989 left a comment

Choose a reason for hiding this comment

Uh oh!

jcarver989 May 19, 2026

Choose a reason for hiding this comment

1.

2.

Uh oh!

blimmer May 19, 2026

Choose a reason for hiding this comment

Uh oh!

blimmer May 19, 2026

Choose a reason for hiding this comment

Uh oh!

blimmer May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

blimmer commented May 18, 2026 •

edited

Loading