Skip to content

fix(ui-web): redraw a picture the agent rewrote under the same path - #907

Open
LivXue wants to merge 1 commit into
mainfrom
fix/stale_image_after_redelivery
Open

LivXue wants to merge 1 commit into
mainfrom
fix/stale_image_after_redelivery

Conversation

@LivXue

@LivXue LivXue commented Oct 11, 2026

Copy link
Copy Markdown
Member

Summary

Open a picture in the web viewer, let the agent rewrite that file in
place and deliver it again, then open the new delivery card: the viewer
showed the picture as it was before the rewrite. The file on disk held
the new bytes and the gateway was never asked for them.

The cause is the document's own memory of what it has drawn, not the
HTTP cache. A document that has drawn a picture from one URL draws it
again for the same URL without a request -- the HTML list of available
images -- and no Cache-Control directive reaches it. The viewer
named a picture by its path alone (/file?path=...&session=...), so a
re-open after a rewrite asked for a URL the document had already drawn.
#838 added no-store to the download routes. That directive governs
the HTTP cache, which is what a reload reads; it never reaches this
memory, so the same report came back.

Measured against a stand-in route sending the real route's exact
headers, no-store included, and the file rewritten between two loads
of one URL:

second <img> for the same URL          Chromium 151      Firefox 153
old <img> kept in the document         old, no request   old, no request
old <img> removed, not yet collected   old, no request   old, no request
old <img> removed and collected (gc)   new               not measured
a fetch() to the same URL in between   new               old
the same path under another query      new               new

Which is why, before this change:

  • the viewer was stale on every re-open: React unmounts the old picture
    and mounts the new one in one commit, so the old one is still alive
    when the new one asks;
  • a delivery card's tile runs the fetch-in-between sequence -- its
    existence probe is a HEAD to the same URL, then the <img> -- so
    it drew the old picture in Firefox, and looked fresh in Chromium only
    because the probe happens to evict the entry there;
  • a deck or PDF card's thumb (render=thumb, named by its path) was
    stale in Chromium too, since the probe asks a different URL.

Every picture URL now carries a version that moves when the bytes may
have, through one helper, versioned(url, version):

  • the viewer's picture is versioned by the open, the file record's
    seq. A re-open reads the file as it stands now, which is what the
    text views already get from their own fetch, and a repaint of the
    same open asks for nothing;
  • a delivery card's picture is versioned by that delivery's own stamp,
    delivered_at. A path delivered again keeps its token and a thumb is
    named by its path, so the stamp is what tells two cards apart. The
    stamp and not the turn, because a delegated stream numbers its turns
    from one as well;
  • pageURL builds its version through the same helper, and
    DeliveryRow declares the when stamp the registry already carried,
    which retires the cast DeckBody used to read it.

Both routes ignore the extra v query, so the gateway is unchanged.
The no-store from #838 stays: it is still what keeps a reload honest
and an agent's output out of the reader's disk cache.

Deliberately unchanged:

  • a pane that is open when its file is delivered again does not redraw
    itself; opening it again from the card or the shelf does. Text panes
    behave the same way;
  • a deck rebuilt without a new delivery is untouched by this change:
    its pages are still versioned by the delivery stamp only;
  • inline pictures from image generation keep their bare URL. That tool
    never writes a path twice (a uuid, or a numbered name claimed with
    O_EXCL), so there is no second drawing to go stale.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

The reported flow, in a real browser against this change's own gateway
and web UI: an isolated raven web with its own RAVEN_HOME, two real
turns on glm-5.3-flash, Playwright Chromium 151. Turn one copies a red
PNG to A and delivers it, and the card is opened; turn two copies a
blue PNG over A and delivers it again, and the new card is opened.
Pixels are read off the rendered elements, and "asked" is the gateway's
own access log after the rewrite.

                         origin/main            this change
open card 1 -> viewer    red                    red
A on disk after turn 2   blue                   blue
open card 2 -> viewer    red, /file asked +0    blue, /file asked +1
card 2 tile (PNG)        blue (probe eviction)  blue, &v=<stamp>
card 2 thumb (PDF)       red                    blue, &v=<stamp>

The bundle built from the final tree is byte-identical (md5) to the
one that run used.

The tests were written first and confirmed red for the reason they
name, then green:

4 failed
AssertionError: expected '/file?path=%2Fw%2Fout%2Fshot.png' not to be
'/file?path=%2Fw%2Fout%2Fshot.png' // Object.is equality
(and the same for the SVG, the PNG tile and the PDF thumb)

Each new construct was then removed on its own and the three test files
that reach it were run (265 tests):

viewer drops its version            killed, 2 red
tile drops its version              killed, 2 red
helper ignores its version          killed, 5 red (with the existing
                                    deck re-delivery test)
viewer versions per paint           killed, 2 red
tile versions by turn, not stamp    killed, 2 red

Commands and results at the pushed head:

cd ui-web
node node_modules/vitest/vitest.mjs run --no-file-parallelism
  Test Files 214 passed (168 under src, 46 under scripts/gates)
  Tests 3236 passed, 0 failed, 0 unhandled errors
node node_modules/typescript/bin/tsc --noEmit -p tsconfig.json   exit 0
node node_modules/eslint/bin/eslint.js <the changed files>       exit 0
node node_modules/vite/bin/vite.js build, then python build.py
  boot-snapshot: OK (343 and 342 nodes match golden)
cd ..
python scripts/check_commit_messages.py origin/main..HEAD        exit 0
python scripts/check_large_files.py origin/main..HEAD            exit 0
python scripts/check_source_language.py origin/main..HEAD        exit 0
commitlint --from origin/main --to HEAD                          exit 0
pre-commit run --from-ref origin/main --to-ref HEAD              exit 0

Two of those were shown able to fail before their green was trusted:
the source-language gate exited 1 on a probe CJK line and named its
file and line, and commitlint exited 1 on a malformed header. The
vitest run set NODE_OPTIONS=--localstorage-file=<file>, which node 26
needs for happy-dom's localStorage; CI's node 22 does not.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

No user-facing doc describes how a delivered picture is fetched, so
none changes.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Security: no route, header, authorization or path fence changes; the
version is a query both routes ignore.

Behaviour and cost: a picture's URL gains &v=<n>. Nothing on main
reads a picture's URL back, and the open drag-and-drop change (#900)
parses dropped URLs with searchParams, so the extra query does not
reach it. A row with no stamp (a manifest from before delivered_at,
or a row recovered from the registry) keeps the bare URL. The viewer
now fetches a picture once per open, where a re-open used to be
answered from memory; a repaint still fetches nothing, and /file
keeps its view-size ceiling.

Rollback: revert the commit.

Related Issues

#838 -- the HTTP-cache half of the same report. Nothing closes here.

Open a picture, let the agent rewrite it in place and deliver it again,
then open the new delivery card: the viewer showed the picture as it
was before the rewrite. The file on disk held the new bytes and the
gateway was never asked for them.

A document that has drawn a picture from one URL draws it again from
memory for that URL without a request -- the HTML list of available
images -- and no Cache-Control directive reaches that layer. The viewer
named a picture by its path alone, so a re-open after a rewrite asked
for a URL the document had already drawn. The no-store directive on the
download routes covers a fresh document only.

Each picture URL now carries a version that moves when the bytes may
have. The viewer's picture is versioned by the open, its file record's
seq, so a re-open reads the file as it stands now and a repaint asks for
nothing. A delivery card's picture is versioned by the delivery's own
stamp: a path delivered again keeps its token, and a thumb is named by
its path. pageURL builds its version through the same helper, and
DeliveryRow declares the stamp it already carried.

Co-authored-by: Claude (claude-opus-5-5[1m]) <noreply@anthropic.com>
@LivXue
LivXue requested review from 0xKT and gloryfromca October 11, 2026 10:05

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

Reviewed github/main...HEAD and the relevant workspace, desk, transcript, delivery-registry, and gateway paths. The per-open seq and per-delivery monotonic delivered_at versions produce distinct image URLs while preserving resource resolution, authorization, repaint behavior, legacy unstamped rows, and existing deck versioning. The added tests cover both image/SVG pane reopens and PNG/PDF redeliveries without weakening existing assertions.

Coverage: AGENTS.md, CLAUDE.md, CONTEXT-MAP.md, ui-web/CONTEXT.md, and ui-web/CONTRIBUTING.md rules; the complete diff; callers and data provenance; relevant history; backward compatibility; test integrity; and UI architecture/import constraints.

Verification: full UI suite passed (214 files, 3,236 tests); focused affected suites passed (4 files, 291 tests); architecture gates passed (46 files, 205 tests); TypeScript and changed-file ESLint passed; git diff --check passed. The test process emitted Happy DOM iframe teardown diagnostics but completed with exit 0 and no failed tests.

@LivXue

LivXue commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

No blockers in the reviewed scope.

Scope: I reviewed the full PR diff (6 changed files), including tests and documentation, and traced affected callers and contracts at 5f482e037c50. This review covers all changed areas, not only raven/agent.

Checked per-open file.seq, FileBody remounting, the producer's monotonic delivered_at stamp, delivery parsing, the URL consumers, and legacy unstamped rows. A new open or delivery gets a fresh image URL while repainting the same open preserves its URL.

Verification environment: native Windows; isolated source snapshots.

Frontend verification: npm test -- --maxWorkers=2 src/features/desk/DeskSurface.test.tsx src/features/transcript/TranscriptPage.test.tsx src/features/workspace passed 356 tests; npm run type-check passed. The combined tree with #900 passed 592 related frontend tests. Happy DOM emitted teardown diagnostics but the runs completed with no failed tests. This is focused coverage, not a claim that the entire UI suite ran.

git diff --check over the complete merge-base-to-head range passed. No new inline finding is being filed.

@LivXue

LivXue commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

Windows compatibility review: no new issue found in the inspected scope.

Reviewed all 6 changed files at head 5f482e037c50a95ee17454dcd7d37fbc19a60e10 against base 612696546702411e63d22f4b87d742d5e78b146b, plus URL producers/consumers, per-open sequence numbers and delivery timestamps. The added version query leaves the encoded file path, session and download token intact.

Verification in an isolated detached checkout on native Windows, Node 25.9.0:

npm.cmd ci --ignore-scripts --no-audit --no-fund
node node_modules/vitest/vitest.mjs run --no-file-parallelism src/features/desk/DeskSurface.test.tsx src/features/transcript/TranscriptPage.test.tsx src/features/workspace/WorkspacePage.test.tsx
# 3 files / 265 tests passed, exit 0.
node ../../pr907/probe_urls.cjs
# 42 encoded Windows path/version checks passed; download token preserved.

The scratch probe extracts and transpiles the actual URL helpers from this head. It covers drive-letter and UNC paths, spaces, non-ASCII characters, literal query metacharacters, changing versions and the unstamped fallback. Happy DOM emitted iframe-abort diagnostics during the passing component tests.

Limits: mocked browser components and URL round trips only; no real Windows browser pixel/cache behavior, UNC filesystem access, rendering application or full UI suite was tested. This is not a full compatibility certification.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants