Repository navigation
feat(desktop): automatically render and archive chat images - #5969
Sun-GLiang wants to merge 36 commits into
Conversation
79339f8 to
ad7dbf1
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Combined review of head bcf5a858. Both passes reviewed 447d8c88; the lead reviewed the three commits since then from two independent review passes (correctness/product, and security/data safety), synthesized and graded by the review lead. The review lead re-checked the two P1s and the retry P2 in the code. Verdict: COMMENT. There are two P1s that need a maintainer decision, plus P2s.
Status. On current main, mergeable, CI green (one eval job skipped). Locally, the Host artifact, composition and protocol suites, the runtime image and file-worker suites, the core Markdown parser, and the UI Markdown and attachment-image suites pass. chat-image-delivery.test.ts cancels 26 of 28 tests on Node 22 (see the P3), while CI runs Node 24. The browser and Electron E2E suites were not run.
What holds up. Archived bytes must be PNG, JPEG, GIF or WebP, so there is no SVG or script path. file:// URLs and local paths go through the same read boundary as the Read tool. For remote fetches, DNS is resolved once and pinned, every resolved address must be public, no cookies, auth or referer are sent, and downloads are capped at 2 MiB and 10 s. Archive writes refuse to overwrite and only hard-link files the store owns. Deleting a session purges its images, and an in-flight capture can't bring them back. A client can only trigger capture for a source that is already in the assistant's text. All 9 new strings exist in all three locales.
P1 (product/security decision needed)
- Zero-click remote fetch by the Host (
chat-image-delivery.ts:117,chat-image-source.ts:88, prompt atmain-session-prompt.ts:55). Every http(s) image URL in streaming model text is fetched by the Host about 250 ms after it appears, with no user action and outside the session's network mode, including network-restricted explore and ask modes. A prompt-injectedexfiltrates data through the URL. (Both passes.) - Renderer CSP opened to all web images (
index.html:28,markdown-image.tsx:97).img-src http: https:applies to every Markdown surface, including previews of untrusted.mdartifacts, the recent-turn overlay and daily review, so tracking pixels load automatically, including plain http. (Both passes.)
Delta since 447d8c88. The new markdown-image-redaction.ts (628f3925) deliberately keeps image destinations that redactSecrets would have altered, by swapping them for opaque aliases and resolving the original URL for rendering. A secret-bearing image URL is therefore no longer neutralized by display redaction, and it is loaded or fetched as written. This widens P1.1 and P1.2. edbd1e02 only adds tests. bcf5a858 bumps @modelcontextprotocol/client 2.1.0 → 2.2.0 for an OAuth advisory, which is unrelated to images and is better as its own PR, though the patch and notices were updated consistently. Line numbers below are for the new head.
A user opt-in, or an allowlist, for remote images would resolve both.
P2
chat-image-source.ts:88/:111: literal 127.0.0.1/::1 is allowed on any port, and the exemption is recomputed per redirect, so a public URL can 302 into a local service (blind GET). (Both passes.)chat-image-delivery.ts:155(viamarkdown-image.tsx:104): Retry on areadyimage deletes the archived copy before re-reading the source, so if the original is gone, the image is lost. (Re-checked by the lead.)artifact-image-storage.ts:60: the 1 GiB per workspace and 100 MiB per session budgets count archived sessions and nothing is evicted. Once the limit is hit, every new capture fails in every session. (Both passes.)
P3
protocol/index.ts:107: the epoch bump is needed becauseartifact.image.resolveis a new wire operation, but 206 is already claimed by #4138 and #5902 (+#5961), and 207–209 are taken too. Please move to 210 and add the floor test. The PR body still says 204→205.chat-image-delivery.ts:234: image records, including pending and failed placeholders, use thetool_result_projectionsource, which is included in child-result outputs and the plugin attachment list. (One pass.)image-file.ts:123: there is no pixel-dimension cap for chat images, so a small file can declare huge dimensions (decode bomb).runtime-host-client.ts:865:{ sessionId, ...request }lets a request field override the routed session id. Swap the order.execution-composition.ts:589: a non-storage capture error, such as a parser exception, drains the whole Host.chat-image-delivery.test.ts:144: the test waits on an unref'd timer and cancels on Node 22, which the CLI still supports.core/package.json:182: pinmarkedexactly, like the sibling dependency.shared-ui-copy.ts:318: the fallback error copy is wrong for missing files, and the attachment hint shows outside chat.- Full-access mode: a local path that appears only in model text gets archived with no tool call or audit event.
- Streaming capture can fetch half-written link targets.
- The browser suite isn't in CI, and there are no stories for the new image states.
Size. At 92 files and about 4.2k lines, including an unrelated filesystem-worker .js→.mjs rename and packaging scripts, this is hard to review safely in one pass. A split would help: (1) the rename, (2) storage and protocol, (3) local capture and PublishImage, (4) remote fetch plus the CSP change behind an opt-in, (5) UI.
| // Increment when the same protocol version no longer guarantees safe Client-Host | ||
| // interoperability. Mismatches are rejected before domain commands are admitted. | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 205 as const; | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 206 as const; |
There was a problem hiding this comment.
P3 (epoch ordering, same grading as the other 206 claimants): Epoch 206 is already claimed by #4138 and #5902/#5961, and 207/208/209 are also claimed. Please renumber to 210.
The bump is needed. artifact.image.resolve is a new operation, and per the 199/29 entries below, an older Host decodes it as unknown and drops the connection. Please make the comment say that, in the house style ("Epoch-N peers ..."), and add a floor assertion in protocol.test.ts. The PR body still says 204→205.
There was a problem hiding this comment.
Automated follow-up; not an independent human review.
Fixed in bd3147e. The compatibility epoch is 210; its comment now explains that Epoch-209 Hosts reject artifact.image.resolve and close the connection, so mixed peers must fail admission. protocol.test.ts asserts the epoch is greater than 209. The PR description was corrected in the latest follow-up.
| const host = url.hostname.replace(/^\[|\]$/g, ''); | ||
| // Literal loopback supports images served by this execution Host. Other private | ||
| // networks and DNS names resolving to them are never automatically fetched. | ||
| const literalLoopback = host === '127.0.0.1' || host === '::1'; |
There was a problem hiding this comment.
P1 (needs maintainer sign-off): The Host fetches every http(s) image URL the assistant emits, during streaming and with no user action.
Under prompt injection,  exfiltrates data through the URL itself, so stripping credentials/referrer doesn't help. The literal-loopback exemption also lets model-chosen GETs reach any local port.
Please consider click-to-load for remote sources, a setting, or limiting loopback to Host-served origins.
Also (P2, both passes): Literal 127.0.0.1 / ::1 is allowed on any port and path, so model output can make the Host GET arbitrary local services (blind SSRF, side-effecting GETs, port probing). If the intent is "images served by this execution Host", please allow only that exact origin (or a tokenised path).
There was a problem hiding this comment.
Automated follow-up; not an independent human review.
Agreed. bd3147e removes automatic remote capture: observing assistant text skips every HTTP(S) source, and remote capture requires explicit Load image consent plus current session network authority. Production callers do not grant loopback access. The exact-origin exception is only for downloader test fixtures, now explicitly documented in e27c725; it cannot be acquired through a redirect or transferred to a different local port. Tests assert denied sources produce zero requests.
| for (const source of chatImageSources(stream!.text)) { | ||
| if (stream!.seen.has(source)) continue; | ||
| stream!.seen.add(source); | ||
| this.#enqueue({ sessionId, turnId: event.turnId, messageId: event.messageId, source }); |
There was a problem hiding this comment.
P1 Every Markdown image in streaming model text is enqueued for capture 250 ms after it appears, with no user action. For http(s) sources this makes the Host issue a GET to a model-chosen URL. A prompt injection can emit  and exfiltrate data with zero clicks. This runs in the Host, so it bypasses the session's network: restricted profile in explore/ask modes. Please gate remote capture behind an opt-in setting or click-to-load, and skip it when the session's network is restricted. The second attempt at L282-289 also doubles the hit.
There was a problem hiding this comment.
Automated follow-up; not an independent human review.
Agreed; fixed in bd3147e and refined in e27c725. Capture observation runs only on text_complete and skips HTTP(S). Resolve validates that the source belongs to canonical assistant text, checks current network authority, and requires loadRemote consent. The capture job checks authority again before downloading. The automatic second download attempt was removed. Explore/ask sessions now receive not_allowed before a consent button is offered; production composition tests assert zero requests.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review of bcf5a858..4490eb6a (2 commits: bd3147ed "require remote consent and preserve saved archives", 4490eb6a "share validation and remove obsolete preview states"). The merge-base is unchanged at current main (3597abe8), so this is a clean delta. Verdict: COMMENT. Both P1s and most of the P2/P3 items are resolved. What remains is small.
Status. Mergeable. All CI checks pass except "Validate installed CLI Eval", which is skipped. The new "Markdown image security and layout" browser step ran in the test job and passed. The only blocker is a required review. I did not run any tests locally for this delta.
Prior findings
- P1.1 Zero-click Host fetch: fixed.
observe()now captures only ontext_completeand skips every http(s) source. A remote fetch needsloadRemote: truefrom a user click, andcanLoadRemoteis checked both atresolveand inside the capture job.canLoadRemoteallows onlybypassor a managednetwork.kind === 'enabled'boundary. The second-attempt download is gone. The system prompt no longer encourages remote embeds. A new composition test shows that explore and ask modes returnrequires_confirmationand thennot_allowed, with zero requests. - P1.2 CSP: fixed.
img-srcis back to'self' data: blob:, the entry contract and architecture test are updated, andImageResourceonly renders the archived artifact. - P2 Loopback on any port, recomputed on redirect: fixed. The exemption now needs an exact
loopbackOriginmatch that is pinned from the original URL, and production never passes one (see inline). - P2 Retry deletes the archive: fixed. A
readyrecord is always returned as-is, the UI no longer callsdelivery.retry()for a saved artifact, and the provider cache ignoresretry. - P2 Quota never evicted: partly fixed. The limits and the remedy are now spelled out in the
quota_exceededcopy. There is still no eviction, and archived sessions still count. I'd accept this as a documented follow-up (now P3). - P3 Epoch: fixed. It is now 210 with a house-style comment and a
> 209floor test. No other open PR claims 210. The PR body still says "204 to 205". - P3
tool_result_projectionleak: fixed.isArtifactChildResultOutputexcludes pending and failed placeholders, and the pluginlistfilters them. - P3 Pixel cap: fixed. Chat images are capped at 16384 px per edge and 32 MP, with a test.
- P3
{sessionId, ...request}: fixed. The order is swapped, and the IPC handler now validates withisImageDeliveryRequest. Both have tests. - P3 Host drains on a capture error: fixed. Only
ImagePersistenceError, a wrapped store write failure, drains. Other errors go topresentationFailed, which only logs. - P3 Node 22 timer: fixed in code. The deadline is a ref'd
setTimeout, andsourceReadTimeoutMsis injectable. Not re-run locally on Node 22. - P3
markedpin: fixed. It is pinned to18.0.11. - P3 Copy: fixed. There is a message per failure reason in all three locales, and
imageUnsupportedno longer suggests sending an attachment. - P3 Streaming capture of half-written targets: fixed, because capture now runs only on settled text.
- P3 Browser suite and stories: fixed. It is now a CI step, and stories were added.
- P3 Full-access local path archived with no audit event: open (unchanged).
- P3 MCP client 2.1.0 to 2.2.0 bump bundled: open (still in the PR).
- Delta note, redaction preserves secret-bearing URLs: partly fixed. It is no longer fetched or loaded automatically, but the consent placeholder still exposes the original URL (see inline).
New (P3 only)
markdown-image.tsx:141: the "Open in browser" link and the "Load image" request use the de-aliased original URL. WhenredactMarkdownImagesaliased it because it contained a secret, the user gets a one-click path to send that secret without seeing it.shared-ui-copy.ts:337: the consent text ("Load this remote image only if you trust...") is also shown where there is no Load button (!delivery.available, i.e. non-chat Markdown). In explore and ask sessions, Load always ends innot_allowed.chat-image-source.ts:89:loopbackOriginis only set by tests, so in production the loopback branch is dead code.- The PR description is stale. It still says "epoch 204 to 205", and it says over-limit images "may still display from their web URL", which the CSP now prevents.
Size. The PR is now 105 files against main, including the MCP bump, third-party notices and the worker rename. The split suggested in the previous review still applies, but the security-relevant surface is now much smaller.
| invalidInternalLink: 'Invalid internal link', | ||
| unsafeLink: 'Unsafe link', | ||
| imageLoading: 'Loading image…', | ||
| imageRemoteConsent: 'Load this remote image only if you trust its source. Loading sends a request to that source.', |
There was a problem hiding this comment.
P3 (copy/UX): needsConsent is also true when !delivery.available, which covers non-chat Markdown surfaces. There this text tells the user to "load" the image, but no Load button is rendered (markdown-image.tsx:140). Likewise, in explore and ask sessions, Load always ends in not_allowed. Consider separate copy for "remote images can't be shown here; open in browser" and, if cheap, letting resolve report the network restriction before the click.
There was a problem hiding this comment.
Automated follow-up; not an independent human review.
Agreed; fixed in e27c725. Non-chat Markdown now says remote images cannot be loaded here and offers only Open in browser for a visible destination. The Host checks current network authority before returning requires_confirmation, so explore/ask sessions show the existing permission-denied explanation without a Load button. Retry can recheck permissions after a session-mode change without granting remote consent. Saved replay remains available regardless of current network authority. Production composition and Chromium regressions pass.
There was a problem hiding this comment.
Verified against the current head, ef91354f5. The non-chat/no-delivery-bridge case now has its own remote-unavailable message and only offers Open in browser for a visible destination, rather than advertising a nonexistent Load button. Hidden destinations expose no network action. One correction to the earlier reply: the current implementation uses application privacy/proxy policy rather than Session explore/ask networking as its remote-media gate. That broader policy question remains open in the current P2 thread; only the misleading no-bridge copy is addressed here.
Follow-up to both the original review and the incremental review: every one of the 20 inline comments now has a direct reply explaining its fix or design tradeoff. The final follow-up is e27c725; the earlier fixes are in bd3147e and 4490eb6. The PR description now reflects epoch 210, explicit remote consent, no live-URL fallback, and current verification. The MCP patch's stale LICENSE source/version labels were also corrected to 2.2.0. The original review's additional body-only observations are covered too: capture only observes settled text_complete events, so partial streaming destinations are not opened. The Chromium image-security/layout suite is wired into .github/workflows/ci.yml, and packages/ui/stories/attachment.stories.tsx includes consent, saving, saved and failed image states. On the remaining observations:
Validation: root build/typecheck/lint/format, both Knip workspaces, 727 UI tests, 81 Host tests, 166 architecture tests, 48 Chromium tests plus 20 affected cases rerun after the final adjustment, Astryx inventory, desktop/CLI notices and 5 source-legal tests passed. Full monorepo and cross-platform Electron E2E suites were not rerun. |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review of 4490eb6a..d46b437b (2 commits: e27c725f "honor redaction and clarify remote consent", d46b437b "exercise production WorkHub browser click path"). Merge-base is unchanged (3597abe8), so all of the delta is from the PR. The delta is 9 files, +137/-42.
No P0/P1/P2 issues. I re-verified the following:
- The CSP is still
img-src 'self' data: blob:. - Remote sources are never auto-captured.
resolve()now checks network authority before offering consent, and capture re-checks it.- Saved
readyandfailedrecords are returned before the gate, so restricted sessions still replay saved bytes. - Retry deletion is still behind the remote gate and
loadRemote. - Production never passes
loopbackOrigin. - The compatibility epoch is 205 -> 210.
Fixed since the last review:
- Redacted remote destinations no longer expose Load, Open, or source-retry actions. Saved bytes still replay.
- The "no bridge" case now has its own copy instead of consent text without a Load button.
- The PR body now matches the epoch and CSP.
- The LICENSE notice now says MCP client 2.2.0.
Still open (P3, disclosed in the PR body):
- The loopback grant is a test-only seam compiled into the production fetcher.
- There is no quota eviction.
- There is no separate audit event.
- The MCP bump is bundled.
New (P3): d46b437b adds an unrelated WorkHub browser smoke-test change to this PR (see inline).
CI is green (18 pass, 1 skipped). The PR is mergeable and awaiting review.
Addressed both inline suggestions: the production loopback fixture grant is removed in 450291e, and the unrelated WorkHub smoke change is removed from this PR in a3b53a5 and split into draft #5979. Each inline thread has the implementation and validation details; the PR body now distinguishes the latest validation from earlier UI/Chromium results. For the remaining disclosed P3 observations, I would keep them as follow-ups rather than expand this image PR:
Latest local validation: root build/typecheck/lint/format, desktop/UI Knip, and 84 delivery/production-composition tests pass on Node 24. The new loopback regression fails against the old downloader. The split WorkHub native smoke has an identical initial screen-pixel failure on unchanged main and its split branch on this Mac, before the changed click path; Linux/Xvfb validation is pending in #5979. Full monorepo and cross-platform Electron E2E were not rerun for this follow-up. |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review: d46b437b → 450291ea. Two commits; the merge-base is unchanged (3597abe8).
a3b53a56drops the unrelated WorkHub smoke-test change.workhub-browser-presentation-smoke.mjsno longer differs from main, so the P3 about the bundled smoke change is fixed.450291earemoves the test-onlyloopbackOriginoption fromdownloadChatImage(chat-image-source.ts). Every resolved address, including literal127.0.0.1/::1and every redirect hop, must now passpublicAddress. This removes the dead loopback branch entirely, so the P3 about it is fixed. The tests inchat-image-delivery.test.tsnow stub the download port instead of granting loopback.
No new findings. The remaining earlier P3s (no quota eviction, no audit event for full-access archiving, bundled MCP client 2.2.0 bump) are unchanged and acknowledged in the PR body. CI is green, and the PR is mergeable.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review: 450291ea → 5d49612c (1342030, 5d49612). The merge-base is unchanged (3597abe8).
Lead ruling on severity. This re-opens the zero-click fetch that both earlier review passes graded P1 at bcf5a858, and that was fixed at 4490eb6a. For consistency it stays P1 here. The WebFetch comparison is fair: WebFetch is a visible tool call that can be turned off, whereas images fire from plain text and again whenever an old transcript is viewed. If the maintainers decide the app-wide network policy, not the per-session network mode, is the intended boundary for agent-initiated traffic, this can be downgraded to P2 with documentation. That is a maintainer decision.
Summary
- Remote images are fetched with zero clicks again. Any visible, non-redacted http(s) image URL in settled assistant text is downloaded by the Host. The gate moved from the session's network permission to app-level outbound policy, which checks only privacy mode and the proxy credential. Explore and ask sessions now fetch; the old restricted-mode test was inverted. This reverses the earlier fix for model-text exfiltration such as
. I grade it P2 rather than P1, because WebFetch already performs zero-approval GETs in explore and ask under the same app policy. The image path still widens it: it is triggered by text alone, it works where WebFetch is disabled, it is less visible, and it re-fires when old transcripts are viewed. Maintainers need to decide whether session network restriction should govern transcript media (and WebFetch). If it should, this is P1 and WebFetch has the same gap. - Remaining mitigations are sound:
- privacy mode blocks it
- public-address checks run on every redirect (max 3), and https to http is refused
- the direct path pins DNS
- 2 MiB / 10 s caps
- no cookies, auth or referer
- only non-partial (settled) text is eligible
- redacted-secret URLs are skipped, but only on the client
- The CSP is unchanged (
img-src 'self' data: blob:).usesFetchProxyis a pure predicate and does not widen fetch capability elsewhere. - CI
testfails because of this PR. The "Astryx surface inventory" step rejects the hand-written<span role=\"button\">added for click-to-enlarge inpackages/ui/src/markdown-image.tsx.
Findings
- P2: zero-click remote fetch, independent of session network mode (product decision)
- P2: CI blocker, hand-written interactive span
- P3: on the proxy path, the local DNS check is not bound to the proxy's own resolution
- P3: the fake-DNS range is accepted for any name on the direct path
- P3: the secret-URL skip is client-side and pattern-based only
- P3: clicking a linked image now opens the lightbox instead of following the link (untested)
| }, | ||
| // Transcript media belongs to the application's outbound policy, rather | ||
| // than the agent's subprocess sandbox. Privacy mode still blocks capture. | ||
| canLoadRemote: async () => { |
There was a problem hiding this comment.
P1 (regression of the earlier P1; maintainers may downgrade, see body): canLoadRemote now checks only resolveHostOutboundExecution(), i.e. privacy mode and the proxy credential. The per-session boundary.kind==='bypass' || profile.network.kind==='enabled' check is gone. Combined with the UI always sending loadRemote, model-chosen image URLs are fetched with zero clicks in explore and ask sessions. A URL like  becomes an exfil or beacon channel. WebFetch already has the same zero-approval behavior under this policy, so this widens an existing channel rather than opening a new class. Still, it fires from text alone, works where WebFetch is disabled, and re-fires when old transcripts are viewed. Maintainers should decide whether session network restriction governs transcript media.
| const context = useContext(DeliveryContext); | ||
| const [attempt, setAttempt] = useState(0); | ||
| // Saved images can resolve even when display redaction forbids a source fetch. | ||
| const loadRemote = allowRemote && isRemoteImageSource(source); |
There was a problem hiding this comment.
P3: the only remaining client-side gate is !redacted, which means redactSecrets() matched a known secret pattern. The Host trusts loadRemote and does not re-check. Encoded payloads (base64 or hex in the query or path) are not redacted and will be fetched. That is fine as UX, but it is not an exfiltration control.
| options.fetch && usesFetchProxy(options.fetch, url) ? options.fetch : undefined; | ||
| const response = proxyFetch | ||
| ? proxyImageResponse( | ||
| await proxyFetch(url, { signal, headers, redirect: 'manual', credentials: 'omit' }), |
There was a problem hiding this comment.
P3: on the proxy path, the public-address check at :97-104 runs on the Host's local DNS answer, but the proxy resolves the name again. Split-horizon names or DNS rebinding can make a corporate proxy reach intranet hosts. These are blind image GETs, capped at 2 MiB. Consider documenting the proxy as the egress authority, or rejecting names whose local answers are fake or non-public before proxying.
| : await abortable(() => lookup(host, { all: true }), signal); | ||
| if ( | ||
| !addresses.length || | ||
| addresses.some((a) => !publicAddress(a.address) && !(namedHost && fakeAddress(a.address))) |
There was a problem hiding this comment.
P3: any attacker-controlled name that resolves to 198.18.0.0/15 or 2001:2::/48 is accepted on the direct path, and the connection is pinned to that address. Where those ranges are routed (labs, benchmark networks, or TUN fake-IP VPNs whose mapping may alias another name), this sidesteps the private-address guard. Consider honoring fake ranges only via a proxy or when a VPN or fake-DNS setting is known.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Incremental review: 5d49612c → b535d57b (one commit, "fix(images): enforce copy quotas and restore UI checks"). The merge-base is unchanged (3597abe8).
- P2 (CI) fixed. The click-to-enlarge trigger is now the design-system
Buttoninstead of a hand-written<span role="button">, and CI is green. - Quota.
assertImageArchiveQuotais split out and now also runs on the copy path under the writer lock. This is an improvement; eviction is still absent (earlier P3, unchanged). - P1 (zero-click remote fetch) still open. This commit does not touch the gating (
execution-composition.ts:598,image-delivery.tsx:54). It only adds a comment acknowledging that encoded values in model-authored URLs can still leave. The lead ruling from the previous review stands. Model-written remote image URLs are fetched with no user action in every non-privacy-mode session, including explore and ask. This needs an explicit maintainer decision: either restore the click gate or the per-session network gate, or confirm the app-wide policy is the intended boundary, in which case it drops to P2 with documentation. - The linked-image click P3 (
[](href)opens the lightbox instead of following the link) still applies to the newButton, which also callspreventDefault/stopPropagation. The proxy and fake-DNS P3s are unchanged.
No new findings in this commit.
|
Hi @Sun-GLiang, W0 ( 中文对照：你好 @Sun-GLiang， |
The worker bundle was emitted as dist/workers/filesystem-worker.js, so Node walked up to the nearest package.json to decide the module type. Inside the Windows AppContainer sandbox that read is denied, and the worker crashed at launch with ERR_INVALID_PACKAGE_CONFIG. The windows_sandbox_w0_protocol job has failed on main because of it. An .mjs bundle is ESM by extension, so Node never performs the lookup. The build and desktop copy scripts now take the file name from FILESYSTEM_WORKER_BUNDLE_NAME so the producer cannot drift from the resolver; packaging verifiers keep literal paths because they check the shipped layout independently. Split out of #5969, which carries the same rename. Generated-by: Claude Code
|
Thanks @Astro-Han for pulling this fix forward and crediting #5969! It makes sense to unblock W0 independently of the image feature. Once #6016 lands, I’ll rebase this PR onto main, remove the overlapping worker rename and packaging changes, and rerun the relevant checks. |
…lane (#6016) Eight workflows with a cron trigger had no repository guard, so every fork with Actions enabled ran them nightly against its own, usually stale, main. On a fork they fail for reasons unrelated to the change under test: the dependency audit flags advisories already fixed upstream, and the Windows release check downloads its upgrade baseline from the fork's own releases, which do not exist. Gate each job on `github.event_name != 'schedule' || github.repository == 'apache/maka'`. Pull request, push and manual dispatch runs keep their current behavior, including on forks; only cron runs outside apache/maka are skipped. issue-pr-lifecycle and model-metadata-upkeep already guard on the repository, and npm-publication is gated by NPM_NIGHTLY_ENABLED. Generated-by: Claude Code fix(runtime): emit the filesystem worker bundle as .mjs The worker bundle was emitted as dist/workers/filesystem-worker.js, so Node walked up to the nearest package.json to decide the module type. Inside the Windows AppContainer sandbox that read is denied, and the worker crashed at launch with ERR_INVALID_PACKAGE_CONFIG. The windows_sandbox_w0_protocol job has failed on main because of it. An .mjs bundle is ESM by extension, so Node never performs the lookup. The build and desktop copy scripts now take the file name from FILESYSTEM_WORKER_BUNDLE_NAME so the producer cannot drift from the resolver; packaging verifiers keep literal paths because they check the shipped layout independently. Split out of #5969, which carries the same rename. Generated-by: Claude Code fix(release): accept the legacy worker name in Windows upgrade baselines The pinned-version upgrade check verifies the previously released installer with the current resource list, which now demands `workers/filesystem-worker.mjs`. Releases built before the rename ship `filesystem-worker.js`, so the check failed on bytes that were correct when they shipped. Relax the requirement for the upgrade-baseline contract, like the other resources that newer builds added. Generated-by: Claude Code
Resolve assistant Markdown images through a session-scoped Host service, save bounded replay copies without rewriting message text, and provide lazy previews, zoom and retry through independent UI capabilities. Share archived image payloads across deliveries and conversation copies, reuse Read filesystem boundaries, and retain PublishImage for explicit publication before temporary sources are removed. Generated-by: Codex (GPT-6)
Generated-by: Codex (GPT-6)
Reuse existing image controls and filesystem permissions, separate chat image validation from model input limits, and honor capture deadlines and Host drain. Add regression coverage for streaming, wide images, FIFOs and cancellation. Generated-by: Codex (GPT-6)
Avoid parent package.json reads outside the worker sandbox grant by shipping the bundled entry as .mjs. Update desktop and CLI packaging and add regression tests for invalid and CommonJS parent package configurations. Generated-by: Codex (GPT-6)
Reuse shared image validation, preserve structured filesystem failures, recapture invalid saved images on retry, and clean up pending artifacts after terminal persistence. Add regression coverage for validation, retry, recovery, and browser image failures. Generated-by: Codex (GPT-6)
Move the background-click smoke fix to a separate change so chat image delivery can be reviewed independently. Generated-by: Codex (GPT-6)
Keep network fixture routing in tests at the Node DNS and HTTP boundary. Exercise the production downloader without an address-policy exception, including private redirects, mixed DNS, byte limits, downgrade prevention and cancellation. Assert that the former runtime option no longer grants loopback access. Generated-by: Codex (GPT-6)
Remove the separate expand control and hover affordance. Keep keyboard activation and restore focus after closing the image preview. Generated-by: Codex (GPT-6)
Use application outbound policy and proxy routes for transcript media, permit named VPN fake DNS destinations, and retain private-source checks and archived replay. Generated-by: Codex (GPT-6)
Apply archive budgets under the writer lock to conversation copies, including linked child images and configured Host limits. Preserve content deduplication and verified copy retries. Use Astryx Button for image enlargement and update the legacy Host-unavailable Storybook assertion. Document the retained application-media network boundary, proxy DNS authority and trusted fake-DNS routing assumptions. Generated-by: Codex (GPT-6)
Require an epoch beyond main 212 and remove the obsolete epoch-210 compatibility declaration.
b535d57 to
a449346
Compare
Record the POSIX FIFO rejection test in the generated Windows skip inventory so the CI consistency gate agrees with the existing tests. Generated-by: Codex (GPT-6)
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Reviewed c6b0ec353ff3501ec41ff418824651a226336f29, incrementally from b535d57b364aace0b62f13bcaea72660563e0642. The old merge-base is 3597abe84356409ccc9fb649c761c1649abe941d; the new one is 560a27c1f6aa3d2bad6b3fb2a4c05434d569a2c9. The current complete PR is 94 files, +7213/-207. I compared the two PR patch series with git range-diff, not just the raw head-to-head diff that includes unrelated main changes.
The image implementation is substantially patch-equivalent. Main now supplies the worker .mjs implementation and MCP client upgrade; this PR retains additional worker regression tests. New follow-ups remove the obsolete epoch-210 compatibility declaration, assert the current protocol floor and synchronize the generated Windows skip inventory.
Prior remote-image concern remains; my severity recommendation is P2, not P1. At packages/ui/src/image-delivery.tsx:56, being display-eligible automatically grants loadRemote: true. The effect calls the resolver without a click. packages/ui/src/markdown-image.tsx:94-102 makes an image eligible when it enters a 300 px viewport margin; without IntersectionObserver it is eligible immediately. This can happen during a new response or while viewing an unfetched historical image, before clicking the image, Retry, or Open in browser.
The request goes through Desktop preload/main IPC to Host artifact.image.resolve. The Host requires a present session and a source found in canonical non-partial assistant text, rejects session guests, then uses application outbound policy (packages/runtime-host/src/server/execution-composition.ts:603-621). It does not use Session subprocess network permission or WebFetch tool availability. resolveHostOutboundExecution checks privacy mode and proxy credential readiness; it is not per-image consent. After those checks, the Host issues the GET, including the model-authored path/query, then validates and archives the response. The renderer itself receives archived bytes under the unchanged restrictive CSP.
I reproduced a fabricated encoded value surviving display redaction and reaching a local fixture's request query through the real delivery service, artifact store and downloader. DNS/dial were redirected to that fixture; no public endpoint or real credential was used. Separate production-composition tests prove media loads in explore/ask sessions and is blocked by application privacy mode. Actual Chromium tests prove the automatic UI trigger and offscreen gating. This proves the outbound channel, not successful live-model prompt injection or real confidential-data loss.
The remaining risk is a plain-text tracking/exfiltration channel that still works with WebFetch unavailable and lacks a visible tool call. Removing cookies, auth and referer, validating public addresses, and capping returned image bytes do not remove data already embedded in a request URL. OWASP describes this Markdown-image exfiltration threat. However, I have not established that the product's existing subprocess network restriction promises to constrain all application media traffic, nor reproduced a higher-impact real-data attack. Keeping P1 merely to match an earlier grade overstates that evidence. I recommend P2 for the verified widening and a maintainer decision on suitable application-level remote-media opt-in/trust control; author documentation alone is not maintainer risk acceptance. This concern is not fixed by the rebase.
Mitigations verified: model completion alone does not fetch remote sources; canonical settled text is required; privacy mode blocks initial remote downloads; recognized hidden-secret destinations do not auto-load; saved bytes replay without contacting the original source; literal private/local destinations and unsafe redirects remain rejected; downloads are bounded and failed downloads require retry. I did not demonstrate a renderer-CSP bypass or subprocess-sandbox escape.
Other earlier findings: the hand-written enlarge control is replaced by Astryx Button and the CI failure is resolved. The suspected linked-image click interception is not reproduced: the installed Astryx parser produces no nested image node for either inline or reference-style linked-image syntax, so I would withdraw that specific P3 rather than claim the click handler hijacks a rendered linked image. Proxy final-DNS authority, trusted fake-DNS routing, deferred eviction and the lack of a separate capture-audit stream remain disclosed limitations; I found no new regression in them in this delta.
P3 — epoch collision, packages/runtime-host/src/protocol/index.ts:107: this PR declares 213, correctly above current main 212, but open PR #5394 at 2fc0e4c2efcc38c4df7fda473e6a36bfb9b282de also declares 213 for a different incompatible change. Coordinate merge order and use the next free epoch after the first change lands. This is an allocation conflict, not a demonstrated current-main interoperability failure.
Local verification on Node 22/Linux: Core, Storage, MCP, Runtime, Runtime Host and UI builds passed. Focused suites passed 259 tests with 2 platform skips: 42 delivery tests, 3 image-related production-composition tests, 119 protocol/parser/worker/UI-redaction tests, 38 artifact/transport-group tests, 3 inventory tests and 54 Chromium tests. Scoped Biome and diff whitespace checks passed. I also ran the encoded-URL probe and the installed-parser linked-image probe. I did not rerun full-monorepo tests, actual Electron application journeys, packaged releases or live-provider prompt injection.
The exact-head CI run succeeded, including the Windows inventory, protocol guard, Runtime Host tests and Markdown image security/layout steps. GitHub reports 18 successful checks and one skipped CLI Eval check. The branch is now CONFLICTING/DIRTY against main c11a5336b; a read-only merge finds only docs/windows-test-inventory.md conflicting. Rebase/regenerate that inventory and rerun checks; green checks on this head do not validate a future merged tree.
| // Saved images can resolve even when display redaction forbids a source fetch. | ||
| // Redaction protects display/retry UX for recognized secrets, not outbound data: | ||
| // an encoded value in a model-authored URL may still pass this presentation check. | ||
| const loadRemote = allowRemote && isRemoteImageSource(source); |
There was a problem hiding this comment.
[P2] Keep an application-level trust decision before automatic remote-media egress. This flag becomes true when ordinary model-authored Markdown enters the viewport margin, not after image-specific consent; the effect sends it before a click or tool call. Host then uses app privacy/proxy policy, so this also works with Session subprocess networking restricted or WebFetch unavailable. A fabricated encoded query value reaches the downloader unredacted in a local fixture. Public-address and response-size checks do not stop URL-carried data or tracking. The channel remains, although I have not demonstrated live-model confidential-data loss or a subprocess-sandbox escape; I recommend P2 rather than carrying the earlier P1 by consistency alone. Preserve local/saved replay while adding suitable opt-in/trust control, or obtain explicit maintainer acceptance of this default.
| // Increment when the same protocol version no longer guarantees safe Client-Host | ||
| // interoperability. Mismatches are rejected before domain commands are admitted. | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 212 as const; | ||
| export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 213 as const; |
There was a problem hiding this comment.
[P3] Coordinate this epoch with #5394. Current main is 212, and both this PR and #5394 at 2fc0e4c declare 213 for different incompatible protocol changes. 213 clears the current-main guard, but whichever lands second must advance beyond the first. This is an in-flight allocation conflict, not a demonstrated interoperability failure with current main.
Generated-by: Codex (GPT-6)
Store pending and failed attempts independently from archived artifacts, migrate existing state, and publish saved results atomically. Centralize canonical image ranges and UI resource transitions while preserving archive identities, redaction, and source authority. Generated-by: Codex (GPT-6)
aa614a6 to
ef91354
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
I checked the increment from c6b0ec353ff3501ec41ff418824651a226336f29 to ef91354f537f47b722332805d9a877ec421c8d91, separating the merge of main from the subsequent image-delivery refactor. The old merge base is 560a27c1f6aa3d2bad6b3fb2a4c05434d569a2c9; the new one is 3ab4ef76de93195df15af2acd4f4f589a39064a2. The existing 26 non-merge commits match in range-diff. The new implementation separates capture attempts from saved artifacts, moves canonical-message lookup into the transcript reader, and consolidates image presentation state and Markdown source positions.
Previous P2 remains: automatic remote requests do not require a user trust decision. At packages/ui/src/image-delivery.tsx:56–66, displaying an eligible remote image still supplies loadRemote: true automatically. The refactored resource hook preserves that behavior. Host composition still uses application outbound/privacy policy, independently of Session subprocess networking or WebFetch availability. No opt-in, source-domain approval, or equivalent trust control was added.
The independent probe using the real service, artifact store and downloader still sends the original fabricated encoded query value to a local fixture on first capture; DNS and connection routing alone are adapted to keep the request local. No cookies, authorization or referer are sent, and saved replay does not request the source again. Four production-Host composition tests pass, including sandbox-independent remote loading and application privacy; all 54 Chromium image tests pass, including automatic display, hidden destinations, saved-byte retry and viewport geometry. These establish the request path, not a live-provider prompt-injection attack or real confidential-data loss. Display redaction is not a substitute for outbound trust control. The PR description now explicitly acknowledges the unresolved concern.
Previous P3 remains: epoch 213 is also claimed by #5394. This branch remains at 213 for artifact.image.resolve, while the currently OPEN #5394 at 2fc0e4c2efcc38c4df7fda473e6a36bfb9b282de claims 213 for Artifact invalidation feeds/guest routing. Main is still 212. Neither collision is a current main-versus-PR handshake failure, but whichever change merges second needs a distinct epoch so incompatible builds cannot advertise the same compatibility identity.
The Windows inventory conflict is resolved. The merge incorporated main's six additional declarations while retaining the image FIFO test, producing 127 declarations. The inventory checker passes. A merge-tree check against current main 40b3d17df3f60d9e0948397c567354bed52e2e51 is conflict-free.
I also checked the new artifact-schema migration (4 to 5), attempt persistence, copy/export/import, successful publication and default archive quotas. The focused Node suites pass 168 tests, with two platform skips; the four composition tests and 54 browser tests are additional. A separate disk-store probe verifies transaction rollback on a precommit fault, terminal-failure precedence during upgrade, unchanged ready-image bytes, placeholder reclamation through the existing maintenance API, and idempotent reopen. Core, Storage, MCP, Runtime, Runtime Host and UI build; CLI dependency notices and the epoch guard pass. I found no additional verified P0–P2 issue in this increment.
The first exact-head CI attempt failed two Usage Storybook width assertions (168.03125 <= 168). The PR does not change those Usage components or stories. This is not enough to attribute the failure to the image changes: the green main run I checked skipped Storybook smoke. Attempt 2 has been started and is still running at this handoff; I am not treating CI as green.
I did not rerun real Electron, packaged-release, Windows/macOS execution, the full monorepo suite, or a live-provider attack. The author's screenshots and Electron runs are not my execution evidence. This feature still needs a human decision on the retained default remote-media policy.
The usage request time column was 168px wide, while Linux system-font text plus padding measures 168.03125px. Increase the column to 176px and retain the strict Storybook fit assertion. Generated-by: Codex (GPT-6)
The packaged Runtime Host can still write into its temporary profile after Desktop exits. Match Windows verification's bounded fs.rm retry policy so transient ENOTEMPTY and EBUSY errors do not fail Linux package validation. Generated-by: Codex (GPT-6)
Preserve the upstream measured and unavailable tooltip copy in the post-compaction component assertions. The branch compatibility bump was already reconciled while replaying the protocol changes: epoch 214 is ahead of main at 212 and the 213 claims in open PRs apache#5394 and apache#5969. Generated-by: Codex
Describe remote image loading under application outbound/privacy and proxy policy, independently of session subprocess network restrictions. Generated-by: Codex (GPT-6)
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
I checked the increment from ef91354f537f47b722332805d9a877ec421c8d91 through a73ab9b0427eb239d496aba050d486db7a401b81, including the current PR diff and the unchanged remote-image authority. The initially requested 9ed0e5dc advanced by one prompt-only commit. Three added commits change three files, without a rebase or new main integration.
- Previous remote-request P2 remains.
packages/ui/src/image-delivery.tsx:56–66still sendsloadRemoteautomatically for eligible images.packages/runtime-host/src/server/execution-composition.ts:606–610still authorizes capture through application outbound/privacy policy, independently of Session subprocess networking or WebFetch. The real service/store/downloader probe again transmitted a fabricated encoded query value to a local HTTP fixture, with only DNS/dial routing adapted; it sent no cookies, authorization or referer, and saved replay made no second request. This is not a live-provider attack or demonstrated credential theft. The new system prompt accurately documents automatic loading but does not add consent or change this authority. - Previous epoch P3 remains. This head declares 213, current main declares 212, and open PR #5394 at
2fc0e4c2efcc38c4df7fda473e6a36bfb9b282dealso declares 213 for different protocol changes. Coordinate the merge order and advance the later change's epoch; this is not a present main/branch handshake failure. - Previous Usage CI failure is cleared on
9ed0e5dc. The time column increased from 168 to 176 pixels; the strict width assertion remains intact. Exact-head CI run38028206944succeeded, and its log confirms Storybook smoke passed 459 stories / 499 theme renders. The subsequent prompt-only head has its own CI run38033703469, still running at this check; the earlier green result is not a current-head all-green claim. - Linux release verification now retries temporary-directory removal with the bounded policy already used on Windows. No release assertions were removed. The migration and image-attempt persistence changes from the prior revision are unchanged; their earlier migration evidence was not rerun here.
No new verified P0–P2 was found in this increment. Six workspace builds pass. Locally, 44 prompt/image-delivery tests, four production-Host composition tests, 12 Linux verifier harness tests, 32 selected release-contract tests and 54 Chromium image tests pass. An initial broader release-test invocation reached one unbuilt Eval dist prerequisite; that unrelated assertion was excluded from the selected rerun, not represented as a pass. The independent egress probe and epoch guard pass. A merge check against current main has no source conflicts.
Outstanding remote-media risk still requires an explicit human decision. I did not run real Electron, actual packaged-release cleanup races, Windows/macOS, the complete monorepo suite or live-provider attacks. I did not independently rerun the Usage Storybook journey on this new prompt-only head.
Move destination validation and direct DNS pinning into the existing fetch transport. Image downloads opt in; ordinary callers retain their defaults. Let configured proxies resolve original hostnames and remove VPN fake-DNS exceptions. Generated-by: Codex (GPT-6)
Generated-by: Codex (GPT-6)
Generated-by: Codex (GPT-6)
Share image AST traversal and canonical destination handling across image and link modes. Move message identity scope into assistant bubbles and reuse narrow chat image service contracts across desktop surfaces. Cover canonical image links and the renamed WorkHub preload capability. Validate 164 related Node tests, 57 browser scenarios, 166 architecture fixtures, desktop typechecks and Biome checks. Generated-by: Codex (GPT-6)
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
At c19d60ecb72f7cce0eb90ff56e65049aff263fb1, the previous remote-request P2 and compatibility-epoch P3 remain. No new verified P0–P2 was found in the changes after a73ab9b0427eb239d496aba050d486db7a401b81.
- P2 — automatic final-answer media still bypasses Session network authority. Reasoning, intermediate commentary and provisional live replies now render images as links without image-delivery or attachment reads. This narrows exposure, but completed final replies still automatically send
loadRemoteinpackages/ui/src/image-delivery.tsx; production Host composition still authorizes the download through application outbound/privacy policy, independently of Session subprocess networking or WebFetch. A real service/store/downloader probe on this head transmitted a fabricated encoded query value once to a local fixture, without cookies, authorization or referer; saved replay made no second request. Only DNS/dial routing was adapted. This is not a live-provider attack or demonstrated credential theft. The documented unresolved concern remains a human risk decision, not a consent control. - P3 — epoch 213 is still shared with a different pending protocol change. Current main
2463d86c386c6bafc483b3d838f3014f4fc9b664declares 212; open PR #5394 at2fc0e4c2efcc38c4df7fda473e6a36bfb9b282dealso declares 213. Coordinate the merge order and advance the later change's epoch. This is not a present main/branch handshake failure.
Image downloads now opt into the shared transport's per-request public-destination policy. Direct DNS answers are checked and pinned, unsafe redirects and benchmark addresses are rejected, ordinary fetch defaults remain unchanged, and configured HTTP/SOCKS proxies own remote DNS resolution. The service-contract rename is wired through preload, main conversation, side chat and WorkHub. Natural-size previews now intentionally expand from a compact placeholder after decoding; fixed loading/ready geometry is no longer promised. The browser tests retain proportional scaling, narrow-column containment, no upscaling and click/keyboard enlargement checks.
On this exact head, 109 network/prompt/image-delivery tests, four production-Host composition tests, 96 UI tests and 64 Chromium image scenarios pass. UI build and all four Desktop TypeScript configurations pass; the initial local typecheck needed the missing computer-use build prerequisite before the successful rerun. The egress probe and protocol guard pass, and merging against current main has no source conflicts.
CI run 38039032678 passed on the preceding 03f7ed6a head, including 459 Storybook stories / 499 theme renders. Current-head CI run 38040817332 was still running at this check; the earlier green run is not an all-green claim for this head. The reported Usage width failure was attributed to main's existing test by the review coordinator, not to this PR. Its strict geometry assertion is unchanged in this increment.
Real Electron, live-provider attacks, Windows/macOS, the complete monorepo suite and the unchanged migration paths were not rerun locally.
Epoch 214 is also declared by open PR apache#5548. Verified all 188 current open PR heads: apache#5394 and apache#5969 use 213, apache#5548 and this PR use 214, and 215 is unused. Move WorkHub to 215; allocation must be rechecked at integration because the base-only CI guard cannot reserve numbers across open branches. Generated-by: Codex
…6051) * refactor(workhub): simplify coordination and execution configuration Use the coordinating agent directly, persist WorkHub model settings, scope delegated permissions to executions, and separate steering from stop and delegation. Preserve historical recovery records and return worker missing information through ordinary follow-up messages. Generated-by: Codex * fix(workhub): propagate delegated execution policy and steering questions Preserve delegated permissions across child and graph execution lineage, restore scoped policy from durable Host facts, and update question handling when steering an existing backend. Generated-by: Codex * fix(runtime): support frozen stores in delegated execution policy Use an independent proxy facade so bound methods and permission read overrides do not violate invariants on frozen Host stores. Exercise the production frozen facade shape in the scoped-policy regression test. Generated-by: Codex * feat(desktop): route remote chats through shared WorkHub Add remote message handling settings, durable source-aware replies, and compact interaction choice answers. Generated-by: Codex * fix(core): simplify remote message source labels Generated-by: Codex * fix(desktop): keep message copy actions visible Generated-by: Codex * fix(desktop): size WorkHub rails to message bubbles Generated-by: Codex * feat(workhub): preserve intent and show clarification handoffs Generated-by: Codex * fix(workhub): link result replies to their source task rails Result notification turns had no links unless they delegated again. Derive their Host-scoped task identity from the durable result origin so rails and filters also cover tool-free result summaries. Generated-by: Codex * feat(workhub): add wn current-work and next-prompt shortcut Generated-by: Codex * fix(workhub): show progress and next steps for three recent works Generated-by: Codex * feat(workhub): place concise wn suggestions in composer draft Generated-by: Codex * fix(workhub): simplify answer summaries and balance result spacing Generated-by: Codex * style(workhub): reduce spacing before result notices to 4px Generated-by: Codex * refactor(ui): share transcript disclosure for selected answers and completion Generated-by: Codex * style(workhub): remove handoff content disclosure Generated-by: Codex * style(workhub): remove gap before result notices Generated-by: Codex * style(ui): highlight focused choice instead of picker outline Generated-by: Codex * fix(ui): confirm question answers on option click Generated-by: Codex * fix(desktop): localize WorkHub remote handling settings through catalog Generated-by: Codex * fix(ci): refresh surface inventory for shared transcript disclosure Generated-by: Codex * test(desktop): retain pending choices with keyboard selection Generated-by: Codex * fix(desktop): align story geometry with rails and timestamp sizing Check WorkHub rails against bubble bounds after the shorter-rail design. Give localized usage timestamps 8px more room so Linux font metrics fit without weakening the overflow assertion. Generated-by: Codex * fix(workhub): preserve steering policy across physical successors Resolve the latest durable policy for each physical continuation, including steered ordinary admissions with no sourceMessages. Add production composition coverage proving successor inheritance and restoration on a later user Turn. Use compatibility epoch 214 to distinguish this contract from the open epoch-213 changes. Generated-by: Codex * test(desktop): reconcile WorkHub fixtures after main rebase Provide the shared WorkHub enablement context to the new dock backdrop fixture and refresh the merged UI inventory totals. Generated-by: Codex * fix(protocol): assign WorkHub an unused compatibility epoch Epoch 214 is also declared by open PR #5548. Verified all 188 current open PR heads: #5394 and #5969 use 213, #5548 and this PR use 214, and 215 is unused. Move WorkHub to 215; allocation must be rechecked at integration because the base-only CI guard cannot reserve numbers across open branches. Generated-by: Codex
Preserve the upstream measured and unavailable tooltip copy in the post-compaction component assertions. The branch compatibility bump was already reconciled while replaying the protocol changes: epoch 214 is ahead of main at 212 and the 213 claims in open PRs apache#5394 and apache#5969. Generated-by: Codex
This branch has not been deployed
2 participants
Summary
Closes #6030.
Assistant Markdown images previously showed only alt text or were blocked by desktop CSP. Supported images now become session attachments and render from saved bytes, so replay survives source deletion or an expired URL.
Read. Remote images load near the viewport using application outbound/privacy policy and HTTP/HTTPS/SOCKS proxy settings. Clicking the image or pressing Enter/Space opens its preview; Escape restores focus.img-src 'self' data: blob:. Inline/block geometry and offscreen gating are preserved.PublishImageremains available before deleting temporary sources.UI before / after
Before this PR: main
3ab4ef76de93195df15af2acd4f4f589a39064a2. After this PR: recorded revisionef91354f537f47b722332805d9a877ec421c8d91.These are unedited screenshots from two independently built, fully launched Electron applications on macOS arm64, using the same 1280 × 900 window, image input and chat messages. Messages are submitted through the real composer. Model generation uses the repository's deterministic
FakeBackendecho; rendering, preload IPC, main/Host capture, HTTPS download, SQLite persistence and attachment reads use production code.Remote HTTPS image
The completed assistant response shows a broken image on main; the PR automatically captures and displays the image.
Local image
Main shows the image label; the PR captures the local file and displays its saved bytes. Both isolated sessions use the same bypass permission mode.
Delete the local source, then restart Electron
Main still has no image to replay. The PR displays the saved image after a full application restart, with the original file deleted. Application privacy mode is also enabled before restarting the PR version.
Additional PR behavior: enlarged preview and remote replay in privacy mode
Clicking the rendered image opens an enlarged preview; Escape closes it. After a full restart with application privacy mode enabled, the remote image still renders from its saved attachment. The test verifies stable artifact identity and matching image-byte hashes.
Screenshots, test results, Playwright traces and the reproducible runner are GitHub Release attachments. Images are not stored in Git commits; the evidence tag points to the recorded PR revision.
Verification
Recorded UI validation revision
ef91354f537f47b722332805d9a877ec421c8d91, compared with main3ab4ef76de93195df15af2acd4f4f589a39064a2(2026-10-10):Unit/browser validation record.
The Electron journey above was run on the recorded revision; it was not rerun for the subsequent transport refactor. Full monorepo, live-provider, packaged-release and cross-platform Electron suites were not rerun locally. Current CI results remain authoritative for CI coverage.
Transport refactor validation
On the follow-up transport refactor (2026-10-10), runtime and runtime-host TypeScript builds pass. All 157 tests in the runtime network suites, chat image delivery and production execution composition pass, covering per-request policy isolation, DNS pinning/rebinding, mutable URL isolation, HTTP/SOCKS remote DNS, proxy bypass, real TLS handshake cancellation, and image redirects/limits. Biome checks for the changed files, ASF headers, Windows skip inventory and
git diff --checkpass. The Electron screenshots above remain evidence for their recorded revision.Reasoning image link validation
For the reasoning image display update (2026-10-10):
git diff --checkpass.Execution process image link validation
The link-only policy also covers intermediate commentary in the execution disclosure, in addition to the dedicated reasoning field. Live text stays link-only until the turn settles, so a provisional reply cannot load an image before a subsequent tool moves it into the process.
git diff --checkpass.Compatibility and migration
Host/Client compatibility epoch is 213, versus current main's 212.
loadRemoteexpresses client display authority, andrequires_confirmationretains its legacy wire spelling.Artifact metadata schema is 5 (main: 3; previous PR revision: 4). Migration moves old pending/failed placeholders into the attempt table and safely reclaims their files through existing orphan cleanup. Ready artifacts and source identities remain intact. Attempts survive restart, conversation copy and session bundle export/import, and are purged on session deletion.
Capture accepts PNG/JPEG/GIF/WebP up to 2 MiB, 16,384 pixels per edge and 32 MP, with two concurrent jobs and a 10-second source-read budget. Storage enforces default quotas of 100 MiB/session and 1 GiB/workspace, counting unique content, even when callers omit limits. Copies use the same limits; archived sessions retain bytes and consume quota, and deletion frees space. Automatic eviction and a separate capture audit stream are deferred.
Network trust boundary
Reasoning and intermediate commentary images in the execution disclosure are displayed as links. Live model text also remains link-only until the turn settles. These renderers do not initiate automatic image delivery requests; the final reply retains automatic image display.
The existing review concern remains unresolved: automatic transcript media uses application outbound/privacy policy even when Session subprocess networking is restricted or WebFetch is unavailable. A model-authored image URL can encode data in its destination; display redaction is not a DLP control. This PR retains that application-level authority and does not resolve the concern.
Image requests set
targetPolicy: publicon the existing scoped fetch transport and check each redirect through a new request. Direct requests validate every DNS answer and pin an admitted address in a separate request dispatcher while preserving Host, TLS SNI and certificate validation. Private/benchmark IPs, local/metadata names, mixed private DNS answers and unsafe redirects remain blocked. There is no VPN fake-DNS exception. Configured proxies receive the original hostname without a prerequisite local lookup and are trusted to enforce final egress restrictions; bypassed destinations use the checked direct route. Requests send no cookies, origin authorization or referer. Calls without the opt-in policy retain their existing routing and destination behavior.AI use
Tool(s) and scope: Codex (GPT-6) authored implementation, refactoring, regression tests, browser and Electron E2E evidence, and PR description. Retain
Generated-by: Codex (GPT-6)in the final squash commit.Checklist
Does this PR entail a change in behavior?