fix(reliability): fail closed for stale toolbar image uploads - #158
fix(reliability): fail closed for stale toolbar image uploads#158seonghobae wants to merge 25 commits into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
|
Validation for exact current head 1f5b6f5:\n\n- Updated the two Toolbar keyboard tests to match the PR's intentional customer-facing title change from Insert inline (base64) image to Insert inline image.\n- TypeScript typecheck passed.\n- Toolbar.test.tsx and ToolbarCustomerCopy.test.tsx passed: 2 files, 15 tests.\n- A full local coverage run reached 146 passed files / 829 passed tests and one existing SafeClipboard.wordCapacity test timed out at the default 5 seconds; this is not claimed as full local coverage proof. Hosted checks are re-running on this head.\n\nThe prior build-and-test annotations for this PR were the two stale-label assertions; no implementation behavior was reverted. |
|
Hosted validation update for exact current head 1f5b6f5: build-and-test and Cross-engine Clipboard / Playwright 1.62.0 both passed in run 33148143891. No completed required check is failing at this head; remaining checks are queued or in progress. Current-head formal reviews and approvals remain 0, and the PR remains Draft/blocked pending the remaining gates. |
|
Hosted gate update for exact current head 1f5b6f5: coverage-evidence completed successfully in run 33148142331 (job 98774679606). There are no completed required-check failures at this head; Strix remains the only substantive gate in progress. The PR remains Draft/blocked with 0 current-head approvals. |
|
Terminal current-head OpenCode update for 1f5b6f5: the required opencode-review job failed closed at Fail closed without a current-head OpenCode verdict, job https://github.com/ContextualWisdomLab/inkspan/actions/runs/33148142331/job/98775377775. This is not a source finding or a review approval. Build-and-test, cross-engine, and coverage-evidence passed at this head; current-head formal reviews and approvals remain 0, so the Draft PR remains blocked. |
|
Terminal current-head Strix update for 1f5b6f5: required job https://github.com/ContextualWisdomLab/inkspan/actions/runs/33148142409/job/98774461877 failed during Run Strix (quick). The uploaded artifact records provider-side failures: contextual-orchestrator HTTP 400 invalid_tools (tool.function.description over 1024 characters), followed by HTTP 500 internal_error and STRIX_PROVIDER_UNAVAILABLE. No usable scan verdict was emitted; the displayed Vulnerabilities 0 is not vulnerability-clear evidence. Build-and-test, cross-engine, and coverage-evidence passed at this head. Current-head approvals remain 0 and the Draft PR stays blocked. |
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Draft head |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Remove the duplicated event-matrix test repair from the Toolbar lane. PR #405 remains the single writer for that shared CI contract. Signed-off-by: Seongho Bae <me@seonghobae.me> Commit-Message-Assisted-by: Claude (via Claude Code)
|
Current head The two preceding commits had copied the event-dependent Office matrix test repair already owned by #405. This head removes that duplicate with an additive revert; history was not rewritten. The effective diff against protected main is again exactly the five intended Toolbar paths. Local current-head evidence:
Office/Python CI may remain inherited RED until #405 integrates. Do not re-add that shared test repair here; revalidate #158 on the protected descendant instead. |
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
Current post-prompt lifecycle repair — 2026-09-07
Current Draft head is
f83d1d9b2b346e4e8c1ff3897e6f54e489f22cf2, normally pushed and independently remote-verified. This is the existing Toolbar owner; no duplicate paste/drop implementation was introduced.The previous async guard checked editor state after image conversion but before host alternative-text prompting. A host prompt can change editability or destroy the editor before returning. Read-only document mutation reproduced on test-only
00d628ae; its destruction oracle also incorrectly queried a destroyed TipTap 3 editor. Revised test-only01826ffauses a command-chain spy after prompting and proves both stale command attempts (2 failed / 4 passed), retaining the read-only document-content assertion. The invalid first oracle and its raw failure are preserved.Repair
f83d1d9badds one post-prompt destroyed/read-only guard. All 31 tests in four existing Toolbar files pass, and TypeScript passes. Existing conversion error containment, decoration/cancel behavior, source-size guard, visible Image label and ownership remain unchanged. No new public contract, dependency, timeout, threshold or authority is introduced.Current controlled native-dialog source-browser verification passes all three Chromium/Firefox/WebKit cases (terminal exit 0): exactly one native prompt, readonly content unchanged, no image insertion. All three screenshots were directly inspected; bilingual text and Image label remain readable with no visible clipping at the 800px diagnostic viewport. This isolated Toolbar fixture is not the full composed editor, a physical-device/IME test, a packaged-browser matrix or WCAG certification. Initial fixture import failures and an interrupted stale-cache acquisition are retained; correcting and restarting only the diagnostic server enabled the completed run. Prior complete package/browser/Office/coverage results below belong to predecessor heads and do not transfer. Current full-suite/package/browser/Office/hosted integration evidence remains required; this PR is not protected or published.
This sibling defect is distinct from PR155's full-suite duplicate-prompt assertion. PR155's unchanged five-case focused suite passes, while its earlier full run remains failed. Existing PR158 pre-prompt lifecycle tests cover the plausible late-conversion-after-destruction mechanism, but do not establish the historical full-suite timing cause.
Evidence:
/private/tmp/inkspan-pr155-prompt-rca.z6dvXZ. Original untracked release archive is preserved unchanged and is not current-head evidence.Current package acquisition — 2026-09-07
Independent internal source review of
a4e4583..f83d1d9found no new actionable defect in the post-prompt guard or the revised lifecycle oracle. Installed TipTap 3.30.4 source was inspected after Context7 quota blocked documentation lookup. No tests or mutations were performed by the reviewer. This is not a qualifying GitHub approval or protected-main evidence. Current hosted build/browser/Office jobs are skipped, not passing acceptance.The unchanged single-worker diagnostic terminated with exit 1: 1022 passed / 19 failed / 1041 tests, 180 files (169 passed / 11 failed), 2247.60 seconds. Fourteen failures were existing 5000ms timeouts; five were missing-DOM change-event failures following the first Toolbar lifecycle timeout. This sequence is preserved without claiming causation. No timeout, assertion, test selection, retry, isolation, threshold or committed runner configuration was relaxed. The failed default acquisition remains authoritative for its own scope; different host-load intervals prevent a controlled comparison or a speedup claim.
At unchanged
f83d1d9b2b346e4e8c1ff3897e6f54e489f22cf2, completepnpm build,pnpm verify:packageand fresh externalpnpm packpassed as one sequential chain (terminal exit 0). All 13 package-configuration tests and existing root ESM/CommonJS/SSR-safe import/TypeScript checks passed, as did independently staged and framework-free consumers for revision evidence, document identity, autosave, text-position selectors and Markdown.Current local
contextualwisdomlab-cwl-editor-0.7.0.tgzSHA-256:895b7f8a026c132b594e44f4bc46d1525dd18c6e4b0d454e7d44215713f5bffd. Tarball editor JS and source map match current built bytes; the map contains exact current Toolbar source. Local and independently resolved remote head remain unchanged; tracked worktree is clean. This completes current package acquisition, superseding earlier pending-package statements only. It is not publication, protected integration, full-browser acceptance or default full-suite success.The completed concurrency diagnostic used CLI
--maxWorkers=1 --minWorkers=1, changing only execution concurrency and output directory. It does not replace the failed default acquisition below or establish a controlled performance improvement. A subsequent unchanged built-module diagnostic passed the original 3000-paragraph capacity assertions: source parsing/counting 246.04ms, sanitizer 484.31ms, output parsing/assertions 230.21ms. A separate inspector-profile run also passed but recorded sanitizer wall time 5171.16ms, demonstrating that instrumentation/execution context cannot be silently mixed into an ordinary latency result. Neither is a real-corpus/browser p95 or full-suite acceptance claim. Evidence and retained archive:/private/tmp/inkspan-pr158-postprompt-full.4yyRpG.Current full-suite acquisition — 2026-09-07
Current archive
895b7f8aalso passed the isolated native-prompt readonly case in Chromium, Firefox and WebKit (terminal exit0). The existing package-entry alias loaded extracted current editor JS/CSS; the JS hash matched current built bytes. All three 800px screenshots were directly opened and inspected: readable Image label and bilingual document text, no visible clipping, no image insertion after the host made the editor readonly during prompting. This is isolated Toolbar evidence, not full composed-editor readonly presentation, a full browser matrix, physical clipboard/IME validation, or WCAG certification.The complete canonical packed-browser suite then terminated exit1: 137 passed / 2 failed / 15 did not run / 154 total, 232.440474 seconds, zero flaky results. Existing projects, three workers, zero retries, build/server startup and all deadlines remained unchanged. Both failures were Firefox
afterEachnetwork-idle timeouts: unsafe-javascript-link corpus handling and native-form serialization when editability is revoked during composition. The former also recorded an incomplete-corpus receipt error; Firefox/consensus evidence is missing, not synthesized. All six 320px normal/forced-color Image-focus frames were directly inspected across three engines: readable Image text and visible focus, without clipping. These successful visual checks do not establish full browser acceptance or the timeout cause. Current receipts and the temporary release archive copy were moved intact to the evidence directory after acquisition; the checkout has no release directory, tracked files remain clean, exact head and archive digest are unchanged, and own port4173 is no longer listening.At unchanged
f83d1d9b2b346e4e8c1ff3897e6f54e489f22cf2, the complete existing coverage suite failed: 1015 passed / 26 failed / 1041 tests, 180 files (161 passed / 19 failed), 410.50 seconds, exit 1. Twenty-five failures report the existing 5000ms timeout; one drop-image presence assertion exhausted its existingwaitFor. The latter already polls observable insertion and must not be described as a fixed-sleep test. No deadline, assertion, test selection, worker, retry, threshold, dependency or runtime change was made.The unchanged 22-case image test file then passed in isolation; that does not erase the failed full acquisition or prove its cause. Current full-suite/coverage acceptance remains missing. High host load was observed, but it is context rather than causal proof. Raw logs and protocol:
/private/tmp/inkspan-pr158-postprompt-full.4yyRpG.The predecessor archive was moved intact from the checkout's
releasedirectory topredecessor-releaseinside that evidence directory, SHA-256e0df2cb8ced8e98d14f3f74fa9b4c78f7c73d5ea81007fb689809a226e81cdf6. It remains recoverable and is not a new-head artifact.Historical integration evidence — 2026-09-06
All sections below retain predecessor evidence and acquisition-time wording. They are not current-head or current-release authority.
Current candidate
a4e4583c8bf0e8100e3d332ecd45439f627b6566normally mergesparent #151
59d32209159f74ce17fb71eb4595a3ccba92bf8d, inheriting #380'sexpected-archive evidence guard without copying its implementation. Twenty-six
focused release-evidence/Toolbar tests and TypeScript pass; shared helper and
regression exactly match the parent. Image label and upload safeguards remain.
Full configured coverage passed 1,039 tests across 180 files in 97.85 seconds,
all configured source coverage 100%. New-head build, complete independent
package consumers and fresh pack pass. Archive SHA-256 is
e0df2cb8ced8e98d14f3f74fa9b4c78f7c73d5ea81007fb689809a226e81cdf6.The canonical full packed browser run failed: 134 passed, 3 Firefox failures,
17 did not run, 229.465263 seconds, zero flaky/global errors. Two cases timed
out in network-idle teardown; native-form composition also timed out during
baseline-text polling. Full browser acceptance remains incomplete. All six
three-engine normal/forced 320px Image-focus screenshots were directly inspected
and show readable, unclipped controls and focus, without claiming full-suite
acceptance. Protocol, raw failures, receipts and screenshots are retained at
/private/tmp/inkspan-pr158-current-package.WuCZko. Older fullpasses and visual observations below are predecessor evidence. This PR remains
Draft with unchanged ownership and protected release gates.
Current scope and prerequisite — 2026-09-06
Refs #157. Draft/Proposed candidate
7c405f9555725b84ee4cb4754ac3f878bed74e7cinherits canonical presentation owner #151 atcc766fd6faf80aacea114ce7702e7f84190632bb, which inherits #380, #201, #176 and #402. Ordinary mergee70071a0preserves predecessora8fa0555061cd31e002f64d8f72c97f69493e7b0and every existing image lifecycle, host-error containment, link-policy and accessible-name repair. No force push, closure, gate change or duplicate source writer.Actual Firefox inspection exposed a missing glyph for the image action emoji. This lane replaces that single visible label with
Image, preserving the accessible nameInsert inline image, native file selection, roving focus and all upload guards. No new font, icon dependency or layout is introduced. Research/design contract: readable image action.Current evidence and remaining work
697868a3: one RED failure because the label was still the emoji.3217f4a4: 20 tests in four Toolbar files passed, 9.14 seconds.7c405f95: nine browser tests passed in 1.3 minutes. They cover Chromium, Firefox and WebKit, forced-color cues, 320px toolbar bounds, readable image text and the native file chooser.5855d520-f4b3-47f5-9d0d-68d5cf873e01, package digest null. This does not erase the failed run or prove the timeout cause; keep our own heavy validations serialized as a precaution. No timeout, assertion, fixture or denominator changed.183015bd-e708-4f13-b00a-03f5474906eeidentifies archive SHA-25635890e5fe3e16e705491566082bd5376c1747c31b8fcdc5e3535f449c25ee334, independently recomputed before and after acquisition. Extracted editor JS and CSS matched the built bytes. Six packed normal/forced 320px image-button screenshots were inspected directly across Chromium, Firefox and WebKit; readable text, complete bounds and visible focus remained intact. This is local artifact evidence, not a published release./private/tmp/inkspan-image-label-evidence.piuJpf.Only the product-specific expectation/file-picker assertions in the inherited forced-color spec changed; shared harness/config/style implementations remain owned by their prerequisites. Readiness, protected integration and release remain unproven. Refetch exact head/base, reviews, threads, terminal checks and governing rules before lifecycle decisions. No self-approval or bypass.
Retained historical evidence — not current-head authority
The record below is retained for source/failure lineage. Its heads, protected-main snapshots, review counts, release blockers and checks are historical and must not be used for current lifecycle decisions.
Current boundary
Protected shipped truth is independently resolved as
main@128a239f8b71ca16add4b9e15e21752d1ad63ff0. Current exact Draft head is4f4f7db37f2ff488e061f9b69d6a0c4b019e0c0donfix/toolbar-image-lifecycle-20260811.The branch is non-destructively synchronized with that exact protected tip. The synchronization commit keeps the previous #158 head as first parent and protected main as the second parent; no force push or destructive rebase was used. Fresh comparison resolves the merge base exactly to protected main, reports
ahead_by=13,behind_by=0, and exactly five intended changed paths:src/components/Toolbar.tsx;src/components/Toolbar.test.tsx;src/components/ToolbarCustomerCopy.test.tsx;src/components/ToolbarImageLifecycle.test.tsx; andsrc/components/ToolbarLinkPolicy.test.tsx.The customer-facing image-action label/accessibility contract formerly duplicated by #391 is now represented only by this earlier canonical Toolbar writer; #391 is closed unmerged. The synchronization inherited current protected-main release controls rather than reviving historical release-workflow snapshots.
This lane owns adjacent
Toolbar.tsxtrust/lifecycle boundaries: stale asynchronous image upload continuations, hostile conversion-value redaction, host image-error observer isolation, toolbar SafeLink preflight, and the customer-facing image-action accessible name. It adds no transport, persistence, credentials, tenancy, collaboration-provider, durable audit, model, database, network, or deployment authority.Stale image-upload lifecycle lineage (#157)
RED —
3f3afc373c4a23951e40963bb5d7d4bdfea53fcd. Hosted CI failed the new lifecycle regressions because the alternative-text prompt still ran after the editor became read-only or was destroyed while conversion was pending.GREEN —
6da5eab43f4a05918df5ce520b5f3ce90d9144cf. Production recheckseditor.isDestroyed || !editor.isEditableafter asynchronous conversion and before prompt/focus/image insertion.Hostile conversion-value redaction lineage
RED —
6e767611315e7427f9c7b1c5018cc81aab200413. A hostile thrown Proxy from image conversion could cross the hostonImageErrorboundary by identity.GREEN —
84cb2b1e5fb94b587bf29dd9024c5288f54c7636. Conversion failures are replaced with stableError('Image processing failed.'); hostile thrown values are neither reflected nor propagated. Genuine Blob size is read intrinsically so typed oversize rejection does not trust a caller-owned accessor.Toolbar SafeLink preflight lineage
RED —
98ee151ee52d5792765f71f35408dee8550e3505. CI31933462011failed the new toolbar-link regression while Security31933462001and SAST31933462012succeeded. The test requiresjavascript:alert(1)to be rejected beforesetLink()or editor command execution.GREEN —
0693ba351c2b325e59e45a41e36c861800b1f732. The toolbar calls the existingisSafeLinkHref(url)policy before any link editor command; safe-link behavior and empty-input unlink behavior remain intact.Host observer isolation lineage
A later reliability sweep found another defect on the same owned image-failure boundary: both oversize rejection and conversion rejection invoked host
onImageErrordirectly. Because image conversion is asynchronous, a throwing host observer could turn an otherwise-contained rejection into an unhandled Promise rejection and alter Inkspan control flow.RED — exact test-only head
041e726b010e96cc8b9b4946de52012d719c3141. CI31949613767checked out that exact SHA, passed immutable install and typecheck, and ran all 143 test files / 823 tests successfully, but Vitest correctly failed the run because it caught one unhandled rejection:Error: private toolbar observer sentinel, originating fromcontains host image-error observer failures after conversion rejection. The production coverage table still reached 100% statements/branches/functions/lines; Office Python 3.11–3.14 succeeded on the same RED generation. Those parallel successes do not convert the intentional RED to passing evidence.GREEN source repair — predecessor
d448a9dd8296fca5f905a7371d22abf75f647f75. A narrowreportImageError()helper invokes the host observer inside a fail-closed best-effort boundary. Both typed oversize rejection and stable conversion-error delivery use the helper. No image conversion, size, lifecycle, prompt, editor-command, transport, persistence, or policy semantics changed.Exact-current-head evidence
The synchronization moved the exact contributor head to
4f4f7db37f2ff488e061f9b69d6a0c4b019e0c0d; all predecessor workflow/review results are historical and are not transferred. Current-head required workflows must become terminal and must be inspected down to required steps and checkout/source SHA before they count. Pending, queued, skipped-required, cancelled, absent, neutral, failed, stale, predecessor, status-only, model-only, wrong-checkout, synthetic-source-only, or otherwise vacuous evidence is non-passing.Formal qualifying approvals on this exact head: 0. Unresolved inline review threads: 0.
Integration boundary
Keep this PR Draft and unmerged while #118 owns exact protected
v0.6.0publication/provenance/digest operational acceptance and qualifying independent approval remains absent. Live governance requires a qualifying approval of the latest push by someone other than the pusher, stale-review dismissal after pushes, review-thread resolution, and the applicable central required workflows. Revalidate exact head, protected main, reviews/threads, rules and every required workflow before any later Ready/merge transition. Do not self-approve, weaken gates, transfer predecessor evidence, or fabricate tag/release identity.Latest same-head full trace acquisition
Current-head local Office verification on Python 3.14.6 also passed all
173 tests in 3.98 seconds; configured 598 statements/244 branches and public
docstrings 100%. This does not establish the hosted supported-version matrix.
At
a4e4583c8bf0e8100e3d332ecd45439f627b6566, the canonical packed suitewith
--trace onpassed all 154 cases in 79.402933 seconds, zero skipped,unexpected, flaky or global errors. Run
d0a09254-8985-45e0-9da7-6c8490175efabinds archive
e0df2cb8ced8e98d14f3f74fa9b4c78f7c73d5ea81007fb689809a226e81cdf6;digest independently rechecked afterward. Six fresh 320px normal/forced-color
image-focus screenshots were directly inspected across all three engines:
the Image label and focus remained visible without clipping. Canonical
timeouts, retries, workers and request guards were unchanged. No intermittent
timeout reproduced, so this does not establish a fix or erase the failed full
run below. Trace overhead is not a speed improvement. Evidence:
/private/tmp/inkspan-pr158-full-trace.WQOfWc. Draft, not protected or released.