Skip to content

fix(ui): bounded negative cache + in-page fallback for dynamic parser loader - #252

Open
claudegoogl-sudo wants to merge 2 commits into
masterfrom
fix/adapter-ui-loader-worker-fallback
Open

fix(ui): bounded negative cache + in-page fallback for dynamic parser loader#252
claudegoogl-sudo wants to merge 2 commits into
masterfrom
fix/adapter-ui-loader-worker-fallback

Conversation

@claudegoogl-sudo

Copy link
Copy Markdown
Owner

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • External adapters can ship a custom run-transcript parser (ui-parser.js) that the dashboard loads dynamically, so their run pages render structured output instead of raw process text
  • For isolation, that parser is executed inside a locked-down Web Worker; if the worker initialises, everything is fine
  • But some devices can never initialise a Blob-URL worker (browser policy, hardening, embedded webviews) — and the loader cached such failures permanently (failedLoads.add with no expiry), so those devices showed raw fallback output forever, no matter how often the user reloaded
  • This pull request bounds the failure cache with a 60s negative-cache TTL (retries allowed afterwards) and adds an in-page synchronous fallback that evaluates the same board-served parser source when the worker path fails
  • The benefit is that run transcripts keep rendering structured entries on worker-hostile devices, and a transient failure no longer poisons the loader for the whole session

Linked Issues or Issue Description

No public GitHub issue exists. Problem description (bug-report shape):

  • What happened: On devices where the sandboxed parser Web Worker never initialises, run pages for external adapters rendered raw process-fallback output permanently. Reloads did not help because the loader cached the worker-init failure forever.
  • Expected behavior: Transcripts should render structured entries via the adapter's parser, and a failed load should be retried rather than cached permanently.
  • Steps to reproduce: Use an external adapter with a ui-parser.js on a device/browser configuration that blocks Blob-URL Workers; open a run page; observe raw output; reload — output stays raw.
  • Version/commit: reproduced at the current master of this repository (loader at ui/src/adapters/dynamic-loader.ts); fix previously validated as a patched dist bundle on a live deployment before being ported to source here.

What Changed

  • ui/src/adapters/dynamic-loader.ts
    • Replaced the permanent failedLoads Set with a TTL-bounded negative cache (failedLoadRetryAt map, FAILED_LOAD_RETRY_MS = 60_000): parser 404s, worker-init failures, and unusable fallbacks are all retried after ≤60s instead of never.
    • Added buildInPageFallbackParser(): when initSandboxedWorker rejects, the already-fetched parser source is evaluated in-page using the same semantics as the worker bootstrap (CJS exports/module shims, self/globalThis shadowed to undefined, strict-mode block wrap). Only parseStdoutLine is wired; parse errors degrade to [] exactly like the worker's own error handling. Trust level: the parser source is board-served, display-only code from the same origin as the UI bundle that evaluates it — strictly weaker isolation than the worker, but only reached after the worker path already failed.
    • Successful fallback modules are cached like normal ones; invalidateDynamicParser clears the negative cache immediately (unchanged semantics, new map).
  • ui/src/adapters/dynamic-loader.test.ts (new): focused unit tests — worker path still preferred; in-page fallback activation with synchronous results; fallback parse-error degradation to []; 404 TTL; worker+fallback-failure TTL; invalidate clears the negative cache.

Verification

Local commands (from repo root, clean checkout of this branch):

pnpm install --frozen-lockfile
pnpm --filter @paperclipai/ui exec vitest run src/adapters/
  -> Test Files 9 passed (9) | Tests 32 passed (32)   (includes 6 new dynamic-loader tests)
pnpm --filter @paperclipai/ui exec tsc -b
  -> clean (no type errors)
pnpm --filter @paperclipai/ui... build
  -> ui build succeeds
node scripts/check-no-internal-ids.mjs origin/master HEAD
  -> ✓ No internal ticket ids ... added in this diff
node scripts/check-no-git-push.mjs
  -> ✓ No unapproved `git push` invocations found

Built-bundle evidence (production build, ui/dist/assets/index-DiDXuyG0.js at commit 9b0d178):

  • In-page fallback present: the bundle contains new Function("exports","module","self","globalThis",\"use strict";\n{\n`+e+`\n}`)and theparseStdoutLineresolution +[]` degradation wrapper.
  • Bounded negative cache present: SBt=6e4 (60 000 ms TTL), function NBt(e){const t=qD.get(e);return t===void 0?!1:Date.now()>=t?(qD.delete(e),!1):!0} (lazy-expiring negative-cache check) and function Zfe(e){qD.set(e,Date.now()+SBt)} (TTL write) — both failure paths (!a.ok and catch-after-fallback) route through Zfe; there is no unconditional permanent failure set.
  • Note on log markers: the source logs [adapter-ui-loader] sandboxed worker failed for "<type>"; trying in-page fallback: and [adapter-ui-loader] in-page fallback parser active for "<type>", but this repo's ui/vite.config.ts sets esbuild.drop: ["console"] for production builds, so no first-party console strings appear in any production bundle (this is pre-existing, repo-wide; the previously deployed hand-patched dist only contained the markers because it was patched after building). The markers do appear verbatim in a development-mode build of this branch (vite build --mode development, index-C8uWT6rm.js), and the unit tests assert the marker log lines fire.

Manual steps for a reviewer with a worker-hostile browser configuration: install an external adapter with a ui-parser, block Blob-URL Workers (or run in such a webview), open a run page — structured entries should render (fallback path), and after fixing the worker condition the sandboxed path returns on the next load/invalidation.

Risks

  • Reduced isolation on the fallback path (main risk). The in-page fallback evaluates parser code on the main thread without the worker's network/DOM lockdown. Mitigations: the source is same-origin, board-served, display-only code at the same trust level as the UI bundle itself; the fallback is only reached after worker init already failed; self/globalThis are shadowed; the eval happens with CJS shims and a strict-mode block wrap mirroring the worker bootstrap. This mirrors the trust argument already validated for the deployed hotfix this PR formalises.
  • Behavioral shift: transient failures are now retried every ≤60s per adapter type instead of once per page load — negligible fetch cost, bounded by the negative cache.
  • Production bundles intentionally drop console output (pre-existing esbuild.drop), so fallback activation is observable in dev builds/logs and unit tests but not in production console. No migration, no breaking API changes; loadDynamicParser/invalidateDynamicParser signatures unchanged.

For core feature work, check ROADMAP.md first and discuss it in #dev before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See CONTRIBUTING.md.

Model Used

  • Provider: Anthropic (via Prime Agent / Paperclip-managed Claude agent)
  • Model: Claude Sonnet 4.5 (claude-sonnet-4-5-20250929), 200K context window
  • Capabilities used: extended thinking (reasoning), tool use (shell, file editing), test execution and iteration
  • Role: produced the complete change (source port of a previously deployed dist-only hotfix, unit tests, build verification, PR body); reviewed against the repository's CONTRIBUTING.md and PR template

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes: # / Refs: # OR (b) described the issue in-PR following the relevant issue template
  • I have not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
  • My branch name describes the change (e.g. docs/..., fix/...) and contains no internal Paperclip ticket id or instance-derived details
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • All Paperclip CI gates are green
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups
  • I will address all Greptile and reviewer comments before requesting merge

Follows CONTRIBUTING.md PR template — sections present: Thinking Path (8 steps), Linked Issues or Issue Description (path B, bug-report shape), What Changed, Verification (commands + results), Risks, Model Used, Checklist. Path class: Path 1 (small, focused change — two files, one logical fix, no feature-roadmap overlap). The last three checklist boxes await CI/Greptile on the pushed branch and will be addressed before requesting merge.

PR target note: this PR targets this repository (claudegoogl-sudo/paperclip) master only. It is not directed at any upstream repository.

… loader

The dynamic parser loader permanently cached Web-Worker init failures,
so devices where the sandboxed worker never initialises rendered raw
process-fallback output on run pages forever, regardless of reloads.

- Failed loads (parser 404, worker-init failure, unusable fallback) are
  now negative-cached for 60s instead of permanently: retry is allowed
  after the TTL, and invalidateDynamicParser still clears it at once.
- When the sandboxed worker fails to init, the parser is evaluated
  in-page via the same CJS-shim semantics the worker bootstrap uses
  (exports/module shims, self/globalThis shadowed, strict-mode block
  wrap). The parser source is board-served, display-only code from the
  same origin as the UI bundle, so the fallback runs it at the bundle's
  own trust level; only parseStdoutLine is wired up and parse errors
  degrade to [] exactly like the worker path.
- Focused unit tests cover: worker path still preferred, in-page
  fallback activation + sync results, fallback parse-error degradation,
  404 and worker+fallback failure TTLs, and invalidate clearing the
  negative cache.

This formalises a hotfix that was previously only patched into built
dist assets (a source rebuild would silently regress it).

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@claudegoogl-sudo

Copy link
Copy Markdown
Owner Author

SE sign-off posted on PLA-4433: APPROVED — mergeable as-is. One non-blocking should-fix: amend the buildInPageFallbackParser "Trust argument" docblock to state the shadowing is namespace hygiene (not a sandbox) and that containment is the instance-admin install gate + same-origin/audience. Full evidence, residual risk, and follow-ups in the PLA-4433 thread.

Absorb post-sync master (455-file sync + #263/#264 follow-ups) into the open
UI-loader PR so its CI runs against the landed tree. Branch side preserved;
merge verified clean via merge-tree before pushing.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
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