fix(viewer): suppress stale PDF render publication - #323
Conversation
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@opencode-agent @cwl-noema-review Please independently review exact current head |
|
@coderabbitai review |
|
@coderabbitai review Please review exact current head |
|
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha="$(git rev-parse HEAD)"
base_sha="83ec6f7fe2b04bdcd28bf98ec350e41e55730a18"
expected_head="79689f928d8552c65dbdecf85ed45e2e9a6f1109"
printf 'HEAD=%s\nEXPECTED_HEAD=%s\nBASE=%s\n' "$head_sha" "$expected_head" "$base_sha"
git diff --check "${base_sha}..${expected_head}"
git diff --stat "${base_sha}..${expected_head}"
git diff --name-status "${base_sha}..${expected_head}"
printf '\n== Locate candidate files ==\n'
fd -HI '^(viewer\.js|test_viewer_render_cancellation\.py)$' . \
-E .git -E node_modules -E dist -E build || true
printf '\n== JavaScript structure ==\n'
viewer_file="$(fd -HI '^viewer\.js$' . -E .git -E node_modules -E dist -E build | head -n 1)"
if [ -n "$viewer_file" ]; then
ast-grep outline "$viewer_file" --items all --type function --match 'renderPdfInline|poll' --view expanded || true
printf '\n== Relevant source occurrences ==\n'
rg -n -C 16 -P '\b(renderPdfInline|poll)\s*\(|\.render\s*\(|appendChild\s*\(|textContent\s*=|innerHTML\s*=|ready|preview|abortSignal|aborted)\b' "$viewer_file"
fi
printf '\n== Exact source diff ==\n'
git diff --find-renames --find-copies --unified=80 "${base_sha}..${expected_head}" -- \
':(glob)**/viewer.js' \
':(glob)**/test_viewer_render_cancellation.py'Length of output: 2440 🏁 Script executed: #!/bin/bash
set -euo pipefail
base_sha="83ec6f7fe2b04bdcd28bf98ec350e41e55730a18"
head_sha="79689f928d8552c65dbdecf85ed45e2e9a6f1109"
viewer="src/main/resources/static/assets/viewer/viewer.js"
test_file="scripts/test_viewer_render_cancellation.py"
printf '%s\n' '== viewer.js: imports, state, and renderPdfInline =='
sed -n '1,235p' "$viewer" | cat -n
printf '%s\n' '== viewer.js: poll and subsequent state publication =='
sed -n '236,430p' "$viewer" | cat -n
printf '%s\n' '== Regression test =='
cat -n "$test_file"
printf '%s\n' '== Exact diff =='
git diff --check "${base_sha}..${head_sha}"
git diff --unified=60 "${base_sha}..${head_sha}" -- "$viewer" "$test_file"
printf '%s\n' '== All DOM/state publication calls in viewer.js =='
rg -n -C 5 -e 'appendChild\s*\(' -e 'replaceChildren\s*\(' -e '\.textContent\s*=' \
-e '\.innerHTML\s*=' -e 'setAttribute\s*\(' -e 'updateStatus\s*\(' \
-e 'renderPdfInline\s*\(' -e 'await\s+' "$viewer"Length of output: 42342
No findings.
The Node regression holds the PDF.js render promise, aborts the operation, completes the old render, and verifies zero preview children. This test covers stale canvas and metadata publication. The change does not modify the separate signed external-artifact path or
|
d7a3342 to
4ea2cce
Compare
|
@opencode-agent @cwl-noema-review Please independently review exact current head |
Pull request was converted to draft
Objective
Advance issue #322 with a bounded viewer-runtime race fix. A superseded PDF.js render may finish internally, but it must not append stale canvas/metadata or publish later preview/link/
Ready.state after a newer viewer operation has started.Fresh exact state — 2026-08-10
4ea2cce1a07462185ada525513de39c90cb9e085;main:55d7ae8647208e301f282350f076eeddaba61d11after protected merge of fix(security): harden audit pseudonymization and refresh Netty evidence #270;viewer.jswas byte-identical to the predecessor fix(viewer): suppress stale PDF render publication #323 base before applying the delta, proving fix(security): harden audit pseudonymization and refresh Netty evidence #270 introduced no path-local conflict;79689f928d8552c65dbdecf85ed45e2e9a6f1109;31390258726: success;31390258088: success;31390257890: success;31390257815: success;Test-first evidence
RED
Historical test-only head
45b1f9add3424228bf690b8009a92dff553c74e7addedscripts/test_viewer_render_cancellation.pywhile productionrenderPdfInlinestill ignored the operation abort signal. The behavioral Node harness held an oldpage.render().promise, aborted its operation, then resolved the old render. The required contract was zero stale canvas/metadata publication.GREEN and clean-base proof
renderPdfInline(path, abortSignal)now checks cancellation around PDF.js loading, page acquisition, render completion, and every subsequent DOM publication boundary.poll(...)passes the operation signal and stops later preview link and terminal ready-state publication after supersession.Clean exact output identity:
src/main/resources/static/assets/viewer/viewer.js:33ec05397abf8cb90aba312219e5e7a1af7da6a5;scripts/test_viewer_render_cancellation.py:307f8a8f21c5e6e25dc32e938d8ec650dcd26773.Scope / remaining issue contract
This slice is path-disjoint from #264
demo.js/dom-utils.js, #318 Java controller branding, #306 Office conversion, #313 HMAC readiness, and #316/#337 OpenAPI contracts. Issue #317 separately owns viewer/session authorization semantics.Do not close issue #322 when this PR integrates. Remaining acceptance includes active
RenderTask.cancel()and loading-task destruction where supported, rapid multi-generation/error-state coverage, signed-token-mode lifecycle parity, terminal status/focus ownership, and no unhandled cancellation rejection.Merge gate
Keep this exact head unchanged. Auto-merge may complete only after live repository protection still sees all required checks passing, zero valid unresolved findings, and the counted approving review from a qualifying independent reviewer with write access. Automated evidence is not approval.