Skip to content

UN-3185: Defer heavy chunks on login route + cache fingerprinted assets - #2114

Merged
jaseemjaskp merged 7 commits into
mainfrom
UN-3185-frontend-login-perf
Jun 26, 2026
Merged

UN-3185: Defer heavy chunks on login route + cache fingerprinted assets#2114
jaseemjaskp merged 7 commits into
mainfrom
UN-3185-frontend-login-perf

Conversation

@jaseemjaskp

Copy link
Copy Markdown
Contributor

What

  • Stop the unauthenticated /landing (login) page from eagerly downloading the full app, and cache fingerprinted assets so repeat visits don't re-fetch the bundle.
  • Fix 1 — frontend/nginx.conf: serve content-hashed /assets/* with Cache-Control: public, max-age=31536000, immutable; keep index.html and config/runtime-config.js no-cache.
  • Fix 2 — code-splitting: OSS route pages and enterprise plugin route elements → React.lazy behind a single <Suspense> in Router.jsx; new helper src/helpers/pluginRegistry.js (lazyPlugin) defers plugin chunks to navigation; app shell (PageLayout/FullPageLayout) made lazy.

Why

  • A DevTools trace of the cloud frontend showed /landing requesting 190+ chunks (PDF viewer, recharts, Monaco/ToolIde, every enterprise plugin) before the login screen paints, and content-hashed assets served with Cache-Control TTL 0 (~403 kB re-fetched every visit). Both hurt repeat visits and slow/mobile connections.

How

  • Static page imports → React.lazy(() => import(...)); one <Suspense fallback={<GenericLoader/>}> around <Routes>.
  • lazyPlugin(loader, exportName) wraps a plugin dynamic import as a lazy element; only referenced entrypoints enter the graph. In OSS the optionalPluginImports stub makes the import reject, and the element falls back to NotFound, so absent-plugin routes 404 harmlessly (same UX as before).
  • The two route-returning hooks (useVerticalsRoutes, useLlmWhispererRoutes) and PRODUCT_NAMES stay guarded await imports (consumed synchronously). Their heavy page/shell imports are lazy-loaded in the cloud plugin repo (companion PR).
  • PageLayout/FullPageLayout made lazy — they statically pulled SideNavBarlookup-studio → PDF/charts onto /landing.

Can this PR break any existing features?

  • Low risk. Route paths, auth guards (RequireAuth/RequireGuest/RequireAdmin), and behavior are unchanged. Lazy routes now show a brief GenericLoader on first load of each chunk. OSS-parity build (with src/plugins removed) verified green: missing plugins resolve to the stub and routes fall through to NotFound exactly as before.

Database Migrations

  • None.

Env Config

  • None.

Relevant Docs

  • None.

Related Issues or PRs

  • UN-3185. Companion PR in unstract-cloud lazy-loads the verticals / llm-whisperer route hooks' pages.

Dependencies Versions

  • None (uses React lazy/Suspense, already present).

Notes on Testing

  • npm run build green (with and without src/plugins). Biome clean.
  • Measured /landing script requests via Chrome DevTools: 201 → 93. pdf-vendor (548K), recharts CategoricalChart (284K), Monaco/ToolIde, and verticals/llm-whisperer pages no longer load until navigation.
  • Recommend a manual click-through of authenticated routes (dashboard, tools/:id, settings) to confirm the lazy shell renders.

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

Fix 1 (nginx.conf): serve content-hashed /assets/* with a 1y immutable
Cache-Control while keeping index.html and config/runtime-config.js
non-cached, so repeat visits stop re-fetching the whole bundle.

Fix 2: stop the unauthenticated /landing page from eagerly downloading
the full app. Convert OSS route pages and enterprise plugin route
elements to React.lazy behind a single <Suspense>, and lazy-load the
app shell (PageLayout/FullPageLayout) which transitively pulled the
PDF viewer, charts and lookup-studio onto /landing.

New helper src/helpers/pluginRegistry.js (lazyPlugin) defers plugin
chunks to navigation; OSS builds resolve the stub and fall back to
NotFound, so absent-plugin routes 404 harmlessly as before.

Measured /landing script requests: 201 -> 93; pdf-vendor, recharts and
Monaco no longer load until navigated to. OSS-parity build (no
src/plugins) verified.
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

The frontend now lazy-loads selected routes and plugin-backed pages through shared helpers and suspense/error boundaries, while nginx maps cache headers by URI and serves hashed assets with long-lived caching.

Changes

Frontend lazy loading and cache policy

Layer / File(s) Summary
Cache policy and asset serving
frontend/nginx.conf
The nginx server maps request URIs to cache-control values, applies the header at server scope, and restricts /assets/ to existing files served by GET and HEAD only.
Lazy module helpers
frontend/src/helpers/lazyNamed.js, frontend/src/helpers/pluginRegistry.js
lazyNamed adapts named exports for React.lazy, and lazyPlugin resolves plugin modules while turning the optional-plugin stub error into NotFound.
Route fallbacks and outlet recovery
frontend/src/components/widgets/error-boundary/ErrorBoundary.jsx, frontend/src/components/error/LazyOutlet/LazyOutlet.jsx, frontend/src/layouts/fullpage-payout/FullPageLayout.jsx, frontend/src/layouts/page-layout/PageLayout.jsx
ErrorBoundary gains reset-key recovery, LazyOutlet adds suspense and route-scoped error handling, and the page layouts render LazyOutlet instead of Outlet.
Lazy route registration
frontend/src/routes/useMainAppRoutes.js, frontend/src/routes/Router.jsx
Route and plugin entry points switch to lazy loading, guarded plugin route trees keep their module checks, and the router is wrapped in suspense and error boundaries while preserving the authenticated route structure.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the two main changes: lazy-loading heavy login-route chunks and caching fingerprinted assets.
Description check ✅ Passed The description follows the repository template and fills the required sections with clear details about what, why, how, risks, testing, and related work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch UN-3185-frontend-login-perf

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR implements code-splitting for the React frontend to prevent the unauthenticated /landing route from eagerly downloading the full application bundle, and adds long-term immutable caching for Vite's content-hashed assets via nginx. The nginx fix uses a map directive at http scope to drive Cache-Control from a single server-level add_header, cleanly avoiding the header-inheritance problem that would arise from per-location add_header calls.

  • nginx: a map $uri $cache_control selects immutable for /assets/* and no-cache everywhere else; a dedicated /assets/ location block returns hard 404 (not the SPA fallback) for missing chunks.
  • Code-splitting: all route pages and enterprise plugins are wrapped in React.lazy (via new helpers lazyNamed and lazyPlugin); layouts PageLayout/FullPageLayout replace <Outlet> with the new <LazyOutlet>, which scopes its own <Suspense> + <ErrorBoundary> to the content area; a top-level <ErrorBoundary> in Router.jsx covers the shell and routes outside layouts, with auto-reload-once logic for stale-chunk failures.
  • OSS parity: plugins absent from OSS builds resolve via the Vite stub to NotFound; the lazyPlugin helper maps only the "Optional plugin not available" sentinel to NotFound and re-throws all other errors to the boundary.

Confidence Score: 5/5

Safe to merge. Route paths, auth guards, and OSS/cloud parity are all preserved; the two previously-identified issues (security-header inheritance in nginx, missing ErrorBoundary around Suspense) were resolved in earlier commits and are correctly addressed in this version.

The nginx change is correct — the map + server-level add_header approach composes cleanly with existing security headers. The lazy-loading strategy is well-scoped: LandingPage stays eager, layouts and plugin pages are deferred, and the two hook-returning route plugins that must run synchronously are explicitly documented as remaining as guarded awaits. Error recovery through LazyOutlet's scoped boundary and the app-wide backstop in Router.jsx is sound. The only substantive observation is a silent fallback-to-default-export edge case in lazyPlugin that affects only misconfigured cloud plugins and not the OSS path.

No files require special attention. frontend/src/helpers/pluginRegistry.js has a minor edge case with the named-export fallback, but it only matters when a cloud plugin renames an export and has an unrelated default export simultaneously.

Important Files Changed

Filename Overview
frontend/nginx.conf Adds map $uri $cache_control at http scope and a server-level add_header Cache-Control so immutable caching for /assets/* composes cleanly with existing security headers instead of overriding them; dedicated /assets/ location block returns hard 404 for missing files.
frontend/src/helpers/pluginRegistry.js New lazyPlugin helper wraps enterprise plugin imports as React.lazy components; correctly maps only the 'Optional plugin not available' stub error to NotFound and re-throws all others, but m[exportName] ?? m.default silently falls back to the default export when a named export is absent.
frontend/src/components/error/LazyOutlet/LazyOutlet.jsx New component that scopes Suspense+ErrorBoundary to the content area; auto-reload-once logic for stale-chunk failures uses a 10-second sessionStorage guard to prevent reload loops; navigation resets the boundary via resetKeys.
frontend/src/routes/Router.jsx Wraps routes in a top-level ErrorBoundary+Suspense; converts eagerly-imported pages and plugin components to lazy equivalents; guarded await imports for hook-returning route plugins remain with improved error logging.
frontend/src/routes/useMainAppRoutes.js Replaces all eager imports with lazyNamed/lazyPlugin; layouts are now lazy-loaded so the SideNavBar/lookup-studio/PDF graph is no longer pulled onto /landing; PRODUCT_NAMES guard refined to check the specific unstract key.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Browser navigates to /landing"] --> B["nginx serves index.html\n(Cache-Control: no-cache)"]
    B --> C["React app boots\nEager: LandingPage only"]
    C --> D{"Authenticated?"}
    D -- No --> E["Render LandingPage\nNo heavy chunks fetched"]
    D -- Yes --> F["Navigate to authenticated route"]
    F --> G["Outer Suspense+ErrorBoundary\nin Router.jsx"]
    G --> H{"Layout type?"}
    H -- PageLayout / FullPageLayout --> I["lazyNamed loads layout chunk\n/assets/PageLayout-hash.js\n(Cache-Control: immutable)"]
    I --> J["Layout renders LazyOutlet\n(inner Suspense+ErrorBoundary)"]
    J --> K["lazyNamed / lazyPlugin\nloads page chunk on navigation"]
    K --> L["Page renders in content area\nShell stays mounted"]
    H -- No layout route --> M["lazyPlugin loads plugin chunk"]
    M --> N{"Plugin absent in OSS?"}
    N -- Yes --> O["Stub throws sentinel\nlazyPlugin returns NotFound"]
    N -- No --> P["Component renders normally"]
    K -- "Chunk load error" --> Q["Inner ErrorBoundary catches\nhandleRouteError: auto-reload once\n(sessionStorage guard)"]
    Q -- "Reloaded" --> R["Fresh index.html\nNew content-hashed chunk URLs"]
    Q -- "Within 10s window" --> S["Show RouteLoadError\n'Reload' button"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["Browser navigates to /landing"] --> B["nginx serves index.html\n(Cache-Control: no-cache)"]
    B --> C["React app boots\nEager: LandingPage only"]
    C --> D{"Authenticated?"}
    D -- No --> E["Render LandingPage\nNo heavy chunks fetched"]
    D -- Yes --> F["Navigate to authenticated route"]
    F --> G["Outer Suspense+ErrorBoundary\nin Router.jsx"]
    G --> H{"Layout type?"}
    H -- PageLayout / FullPageLayout --> I["lazyNamed loads layout chunk\n/assets/PageLayout-hash.js\n(Cache-Control: immutable)"]
    I --> J["Layout renders LazyOutlet\n(inner Suspense+ErrorBoundary)"]
    J --> K["lazyNamed / lazyPlugin\nloads page chunk on navigation"]
    K --> L["Page renders in content area\nShell stays mounted"]
    H -- No layout route --> M["lazyPlugin loads plugin chunk"]
    M --> N{"Plugin absent in OSS?"}
    N -- Yes --> O["Stub throws sentinel\nlazyPlugin returns NotFound"]
    N -- No --> P["Component renders normally"]
    K -- "Chunk load error" --> Q["Inner ErrorBoundary catches\nhandleRouteError: auto-reload once\n(sessionStorage guard)"]
    Q -- "Reloaded" --> R["Fresh index.html\nNew content-hashed chunk URLs"]
    Q -- "Within 10s window" --> S["Show RouteLoadError\n'Reload' button"]
Loading

Reviews (6): Last reviewed commit: "UN-3185: Auto-reload once on chunk-load ..." | Re-trigger Greptile

Comment thread frontend/nginx.conf Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/routes/useMainAppRoutes.js (1)

351-356: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate the product wrapper on the value actually used.

Object.keys(PRODUCT_NAMES).length can be true even when PRODUCT_NAMES.unstract is absent, but Line 355 passes that value as type. Use the unstract entry itself as the condition to avoid wrapping all app routes with an undefined product type.

Proposed fix
-  if (OnboardProduct && Object.keys(PRODUCT_NAMES)?.length) {
+  const unstractProduct = PRODUCT_NAMES?.unstract;
+
+  if (OnboardProduct && unstractProduct) {
     return (
       <Route
         path=""
-        element={<OnboardProduct type={PRODUCT_NAMES?.unstract} />}
+        element={<OnboardProduct type={unstractProduct} />}
       >
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/routes/useMainAppRoutes.js` around lines 351 - 356, The product
wrapper guard in useMainAppRoutes is checking Object.keys(PRODUCT_NAMES).length,
but the route element actually depends on PRODUCT_NAMES.unstract; update the
condition in the OnboardProduct/Route block to gate on the same unstract value
that is passed as type, so the wrapper is only rendered when that product type
exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/nginx.conf`:
- Around line 61-68: The location blocks that use add_header are overriding the
server-level security headers, so explicitly re-add the security headers in each
affected location stanza. Update the nginx.conf location handlers for /assets/,
/index.html, and /config/runtime-config.js so they preserve the same
X-Content-Type-Options, X-Frame-Options, Referrer-Policy, and
Content-Security-Policy-Report-Only values defined at the server level, while
keeping the existing cache-related headers in place.

In `@frontend/src/helpers/pluginRegistry.js`:
- Around line 26-28: Update the lazy import error handling in pluginRegistry.js
so chunk fetch failures are not treated as missing plugins. In lazyPlugin, the
catch path currently relies on isModuleMissing(err) and returns NotFound for
both the build-time stub and transient dynamic import/network failures; tighten
isModuleMissing to only match the optional-plugin stub error or change the
lazyPlugin catch logic to only map that specific case to NotFound and rethrow
all other errors.

In `@frontend/src/routes/Router.jsx`:
- Around line 113-127: The top-level dynamic imports in Router.jsx are still
running during module evaluation, which keeps useLlmWhispererRoutes and
useVerticalsRoutes on the startup critical path. Move these imports out of the
Router module scope and into lazy-loaded route/component wrappers so the plugins
load only when their routes are actually needed. Use the existing symbols
llmWhispererRouter, verticalsRouter, useLlmWhispererRoutes, and
useVerticalsRoutes to relocate the loading logic without blocking /landing
rendering.

---

Outside diff comments:
In `@frontend/src/routes/useMainAppRoutes.js`:
- Around line 351-356: The product wrapper guard in useMainAppRoutes is checking
Object.keys(PRODUCT_NAMES).length, but the route element actually depends on
PRODUCT_NAMES.unstract; update the condition in the OnboardProduct/Route block
to gate on the same unstract value that is passed as type, so the wrapper is
only rendered when that product type exists.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 92f99d45-c009-4324-8547-15d11fbbaa70

📥 Commits

Reviewing files that changed from the base of the PR and between 069a177 and 425feac.

📒 Files selected for processing (4)
  • frontend/nginx.conf
  • frontend/src/helpers/pluginRegistry.js
  • frontend/src/routes/Router.jsx
  • frontend/src/routes/useMainAppRoutes.js

Comment thread frontend/nginx.conf
Comment thread frontend/src/helpers/pluginRegistry.js
Comment thread frontend/src/routes/Router.jsx
…bsent check

- nginx: drive Cache-Control from a $uri map at server scope instead of
  per-location add_header. Location-level add_header replaces (not merges)
  the inherited server headers, which dropped X-Content-Type-Options/
  X-Frame-Options/Referrer-Policy/CSP from /assets, index.html and
  runtime-config.js. Now both locations carry no add_header and inherit all
  security + cache headers.
- pluginRegistry: only treat the build-time stub ('Optional plugin not
  available') as plugin-absent; rethrow transient chunk-load failures of a
  shipped plugin instead of masking them as NotFound.
- useMainAppRoutes: gate the OnboardProduct wrapper on PRODUCT_NAMES.unstract
  (the value passed as type) rather than the map being non-empty.
@jaseemjaskp

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews 🙏 Addressed in 020ef555b:

# Finding Action
1 nginx: location-level add_header drops inherited security headers (greptile P1 + CodeRabbit) Fixed — Cache-Control now driven by a map $uri $cache_control at server scope; both location blocks carry no add_header, so they inherit all security + cache headers.
2 pluginRegistry masks transient chunk-fetch failures as missing (CodeRabbit) FixedlazyPlugin matches only the build-stub 'Optional plugin not available' and rethrows all other errors.
3 Top-level await for the two route hooks still on /landing path (CodeRabbit) Won't fix (documented) — the hooks return a <Route> tree consumed synchronously; they can't be React.lazy without a route-config rewrite. Companion cloud PR Zipstack/unstract-cloud#1605 already split their pages/shell out, so each chunk is ~8 KB now (was 744 KB / 46 KB).
4 Gate OnboardProduct on PRODUCT_NAMES.unstract, not map length (CodeRabbit, outside diff) Fixed — now gates on unstractProduct = PRODUCT_NAMES?.unstract, the same value passed as type. More relevant now since OnboardProduct is always-truthy via lazyPlugin.

Build green, biome clean. One caveat: I couldn't run nginx -t locally (no Docker daemon up) — the map/inheritance pattern is standard nginx, but please confirm in the image build/CI.

Comment thread frontend/src/routes/Router.jsx Outdated
Pull the repeated 'lazy a named export' pattern into src/helpers/lazyNamed.js
and use it in Router.jsx and useMainAppRoutes.js instead of inline
.then((m) => ({ default: m.X })) / a per-file 'named' helper. Behaviour is
identical; /landing chunk count unchanged. Addresses cloud-PR review feedback
about the duplicated helper (the two route hooks import the same util).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/src/helpers/lazyNamed.js`:
- Around line 11-12: The lazyNamed helper currently maps any requested export to
{ default: m[exportName] }, which can silently return undefined and fail later
in React.lazy. Update lazyNamed(loader, exportName) to explicitly validate that
the loaded module contains exportName before resolving, and throw a descriptive
error during the loader promise chain when it is missing. Keep the fix localized
to lazyNamed and ensure the error makes it clear which missing named export
caused the load failure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 70d9876e-a614-4c86-9aa0-e5dbd86bb5de

📥 Commits

Reviewing files that changed from the base of the PR and between 020ef55 and 9864b3f.

📒 Files selected for processing (3)
  • frontend/src/helpers/lazyNamed.js
  • frontend/src/routes/Router.jsx
  • frontend/src/routes/useMainAppRoutes.js
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/src/routes/useMainAppRoutes.js
  • frontend/src/routes/Router.jsx

Comment thread frontend/src/helpers/lazyNamed.js Outdated
- Wrap the route <Suspense> in Router.jsx with the existing ErrorBoundary and
  a reload-prompt fallback. lazyPlugin/lazyNamed rethrow non-stub failures
  (e.g. a transient chunk-load blip), and App rendered <Router/> with no
  boundary — so such a failure would unmount the tree to a blank screen.
  The boundary now contains it and offers a reload (which re-fetches the chunk).
- lazyNamed: throw a descriptive error when the requested named export is
  missing instead of handing React.lazy { default: undefined } (opaque error).

Addresses greptile P1 (no ErrorBoundary) and CodeRabbit (lazyNamed validation).

@jaseemjaskp jaseemjaskp left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Automated follow-up review (PR Review Toolkit). I raised the bar to only correctness/contract/resilience issues that weren't already covered in earlier rounds. Three findings below; the rest (comment-accuracy nit on the FullPageLayout rationale, nginx no-cache wording, missing helper unit tests, a behavior-preserving dedup of the guarded-await import blocks) are minor and left out of inline comments.

Comment thread frontend/src/helpers/pluginRegistry.js Outdated
Comment thread frontend/src/routes/Router.jsx Outdated
Comment thread frontend/src/routes/useMainAppRoutes.js
… dead guard

Review round 3:
- lazyPlugin: validate the resolved export (mirror lazyNamed). A shipped plugin
  whose named export was renamed/removed now throws a descriptive error instead
  of handing React.lazy { default: undefined }; isPluginAbsent doesn't match it,
  so it re-throws to the ErrorBoundary rather than masking as NotFound.
- ErrorBoundary: support resetKeys — clear the error when a key changes (e.g.
  location), so navigation recovers without a full reload.
- New LazyOutlet (content-scoped <Suspense> + nav-resettable ErrorBoundary +
  <Outlet/>); PageLayout/FullPageLayout use it instead of a bare <Outlet/>.
  Per-page load spinners and chunk-load failures now stay in the content area
  with the shell mounted; the app-wide boundary in Router.jsx remains the
  backstop and is now also location-reset. Fixes the blast-radius / no-recovery
  and shell-blanks-on-first-nav issues from the single top-level boundary.
- useMainAppRoutes: remove the now-dead 'ReadOnlyReviewPage && !ReviewLayout'
  warning — with lazyPlugin both are always truthy so it could never fire; the
  route degrades to NotFound if manual-review is absent.

Build green (with and without src/plugins); biome clean.
…ud S7764)

Prefer globalThis over window for the reload handler, matching existing
globalThis.location usage in the codebase (e.g. SideNavBar). Clears the only
open SonarCloud issue on this PR (javascript:S7764, minor code smell).
…loy)

The route ErrorBoundary already catches a rejected dynamic import() (a
<Suspense> alone only handles the pending state). Add chunk-error handling so
the common production trigger — a stale hashed chunk after a redeploy (client
requests a filename the CDN no longer serves) — auto-recovers:

- isChunkLoadError() detects the failed-dynamic-import error across browsers.
- handleRouteError() (wired as onError on both the app-wide boundary in
  Router.jsx and the content-scoped one in LazyOutlet) reloads ONCE to pick up
  fresh chunk hashes, guarded by a sessionStorage timestamp so a genuinely-gone
  chunk falls through to the manual Reload fallback instead of looping.
  Non-chunk render errors are never auto-reloaded.

This covers the outermost lazy route elements too (e.g. FullPageLayout for
verticals, OnboardProduct for llm-whisperer), which render under the Router
boundary.
@github-actions

Copy link
Copy Markdown
Contributor

Frontend Lint Report (Biome)

All checks passed! No linting or formatting issues found.

@jaseemjaskp
jaseemjaskp merged commit b13e5bd into main Jun 26, 2026
9 checks passed
@jaseemjaskp
jaseemjaskp deleted the UN-3185-frontend-login-perf branch June 26, 2026 04:55
@sonarqubecloud

Copy link
Copy Markdown

kirtimanmishrazipstack pushed a commit that referenced this pull request Jun 30, 2026
…+ Edit LLM Profile modal layout (#2128)

* fix(frontend): bundle all CSS into one file to fix lazy-load cascade order

Code-splitting (#2114) injects per-route CSS at navigation time, so
equal-specificity rules across components resolve in load order — breaking
the Edit LLM Profile form and Execution Logs header layout (UN-3185).
cssCodeSplit:false restores a single deterministic stylesheet; JS splitting
is unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* perf(frontend): gzip static assets (css/js/fonts), not just html

nginx `gzip on` only compresses text/html by default, so JS/CSS shipped
uncompressed. Add gzip_types so the single CSS bundle (~341KB -> ~57KB) and
all JS chunks compress on the wire — offsets the cssCodeSplit:false upfront
cost and speeds every asset.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* fix(frontend): clean up Edit LLM Profile modal padding & drop dead scroll classes

Remove settings-body-pad-top + add-llm-profile-scroll-root from the form root
(the shared overflow-y:auto added a nested scroll context that #2119 only
patched over). The sticky footer now pins directly to .conn-modal-col. Replace
the dead override with a :has()-scoped padding so .conn-modal-form-pad-left is
adjusted for this panel alone — sibling settings panels and the connector
dialog are untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* perf(frontend): gzip_proxied any so assets compress behind the LB

nginx skips gzip for proxied requests by default (gzip_proxied off); the GKE
load balancer adds a Via header to every request, so static assets shipped
uncompressed despite gzip_types. Verified gzip works direct-to-pod.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* fix(frontend): remove inner scrollbar on Edit LLM Profile modal

The settings row is hard-fixed at 800px, forcing the form's column
(.conn-modal-col) to scroll once Advanced Settings expands. Let the row size to
the form so the modal grows instead. Scoped via :has to the LLM profile panel —
the other settings panels and the connector dialog share .conn-modal-row/-col
and are untouched (so the shared class stays; it is not unused).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* docs: tighten code comments in CSS/vite/nginx changes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* style: drop comments from AddLlmProfile.css changes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* fix: drop pre-compressed font types from gzip_types

WOFF2 (Brotli) and WOFF (zlib) are already compressed; re-gzipping wastes
CPU for no size gain. application/font-woff also never matches nginx's
mime.types (.woff -> font/woff), so it was a no-op.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
muhammad-ali-e added a commit that referenced this pull request Jul 15, 2026
…N-3735 .value fix (#2181)

* UN-3185: Fix squished route-loading spinner in the content area (#2118)

GenericLoader's .center uses width/height: inherit, which collapsed to a
narrow column when used as LazyOutlet's content-area Suspense fallback (the
loader text wrapped one word per line beside the sidebar). Wrap the fallback in
a full-width, flex-centered .lazyOutletFallback box and pin .center to width:100%
so it renders centered at its natural size. The top-level loader in Router.jsx
is unaffected (it doesn't use this wrapper).

* UN-3185: Pin LLM profile form submit button as a sticky footer (#2119)

* UN-3185: Pin LLM profile form submit button as a sticky footer

In the prompt-studio Settings modal, the LLM-profile Add/Edit form put its
submit button (Update/Add) as the last element in normal flow inside a fixed
800px scroll column. When the form is tall — e.g. Advanced Settings expanded —
the button flowed past the modal's visible area and appeared to overflow.

- Make the button a sticky footer (position: sticky; bottom: 0) with a solid
  themed background so it stays pinned at the bottom of the scroll column and
  fields scroll beneath it.
- The form root's .settings-body-pad-top declared overflow-y: auto with no
  bounded height — a dead nested scroll context that would stop the sticky
  footer from pinning to the real scroller (.conn-modal-col). Override it back
  to visible for this form (scoped via .add-llm-profile-scroll-root).

Verified the sticky pinning against the real CSS rules; build green, biome clean.

* Apply suggestion from @greptile-apps[bot]

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Jaseem Jas <89440144+jaseemjaskp@users.noreply.github.com>

---------

Signed-off-by: Jaseem Jas <89440144+jaseemjaskp@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

* [MISC] Decommission prompt-service, old tools, SDK1 prompt module (#1978)

* [MISC] Decommission prompt-service, old tools, and SDK1 prompt module (Phase 5)

Remove prompt-service source, Dockerfiles, and docker-compose entries.
Remove tools/classifier, tools/structure, tools/text_extractor directories.
Remove SDK1 prompt.py module and its tests.
Clean up PROMPT_HOST/PROMPT_PORT from backend settings, sample envs,
docker configs, and CI workflows. Remove prompt-service from uv-lock
scripts and production build workflow.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* [MISC] Remove prompt-service from tox.ini env_list

The prompt-service directory was deleted in the prior commit but tox.ini
still referenced it, which would break CI test runs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* UN-2888 [FIX] Add hook for setting default triad for invited users (#1877)

* [FIX] Add hook for setting default adapters for invited users

Add setup_default_adapters_for_user() hook to AuthenticationService
and call it from set_user_organization() when an invited user joins
an existing organization. This allows the cloud plugin to set up
default triad adapters (LLM, embedding, vector DB, x2text) for
invited users, fixing silent failures in API deployment creation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update backend/account_v2/authentication_controller.py

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Praveen Kumar <praveen@zipstack.com>

* [FIX] Improve log message for setup_default_adapters_for_user

Address review comment: log user email and explain that default
adapters will not be set when the method is not implemented.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* [MISC] Rename Default Triad to Default LLM Profile in UI

Update display label from "Default Triad" to "Default LLM Profile"
in the page heading and side navigation menu.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Signed-off-by: Praveen Kumar <praveen@zipstack.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: Deepak K <89829542+Deepak-Kesavan@users.noreply.github.com>

* UN-3465 [FIX] Wrap set_user_organization in transaction.atomic (#1954)

* [FIX] Wrap set_user_organization in transaction.atomic

The new-org branch creates the org row, then calls frictionless onboarding
and the initial platform key. Failures mid-flow leave an orphan org with no
adapters or key, and subsequent logins skip onboarding entirely (gated on
new_organization). Atomic ensures the org rolls back on any failure so
retries get a clean fresh-org path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [MISC] Worktree skill — use --no-track to prevent accidental main pushes

Without --no-track, a later `git push -u origin <branch>` can be reported
by the server as also fast-forwarding main, landing commits on main.

* [FIX] Use logger.exception in authorization_callback

Preserves the traceback when the OAuth callback hits the safety-net
catch. Behaviour unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Athul <89829560+athul-rs@users.noreply.github.com>
Co-authored-by: vishnuszipstack <117254672+vishnuszipstack@users.noreply.github.com>

* UN-3386 [FEAT] Add Prompt Studio HITL change indicator plugin slot (#1930)

* UN-3386 [FEAT] Add Prompt Studio HITL change indicator plugin slot

Wires up the host-side hooks for the prompt-change-indicator plugin
(implementation lives in unstract-cloud): a dynamic-import slot in
the prompt card Header for the indicator button, and a route at
:orgName/review/readonly/:documentId for the read-only audit view.
Both gates fall through gracefully when the plugin is absent (OSS).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* UN-3386 [FIX] Warn when ReadOnlyReviewPage loads without ReviewLayout

Addresses review feedback: the readonly route nests inside ReviewLayout
(manual-review plugin), so a deployment that ships prompt-change-indicator
without manual-review would silently fail to register the route. Log a
console.warn in that case to make the misconfiguration discoverable.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* UN-3386 [FIX] Surface real plugin import errors in route loader

Bare catch in the prompt-change-indicator dynamic import was swallowing
syntax/runtime errors in the plugin file alongside the expected
"plugin missing in OSS" case. Detect the missing-module messages
explicitly and console.error anything else so a broken cloud plugin
no longer disables the readonly route silently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Add a dedicated OpenAI-compatible LLM adapter (#1895)

* Add OpenAI-compatible LLM adapter

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address review feedback for custom OpenAI adapter

* Fix import formatting after rebase

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address follow-up review comments for OpenAI-compatible adapter

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Refine OpenAI compatible adapter schema naming

* Reject empty model string in OpenAICompatibleLLMParameters

validate_model previously produced "custom_openai/" for an empty model,
surfacing as a confusing LiteLLM error at call time. Match the existing
GeminiLLMParameters.validate_model pattern: strip whitespace, raise
ValueError on empty input.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Revert SCHEMA_PATH plumbing; rename schema to custom_openai.json

Addresses Ritwik's review feedback. The new BaseAdapter.SCHEMA_PATH
class variable and the conditional branch in get_json_schema() are
unnecessary: OpenAICompatibleLLMAdapter.get_provider() returns
"custom_openai", and the default path resolution already builds
…/llm1/static/{get_provider()}.json. Renaming the schema file lets
the default lookup find it and keeps the base class untouched, which
is the convention every other adapter follows.

- Rename openai_compatible.json -> custom_openai.json
- Drop SCHEMA_PATH class var and the if-None branch from BaseAdapter
- Drop SCHEMA_PATH override (and unused os/ClassVar imports) from
  OpenAICompatibleLLMAdapter
- Update test_openai_compatible_schema_is_loadable to read schema via
  get_json_schema() instead of touching SCHEMA_PATH directly

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Hari John Kuriakose <hari@zipstack.com>
Co-authored-by: Chandrasekharan M <chandrasekharan@zipstack.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Athul <athul@zipstack.com>
Co-authored-by: Athul <89829560+athul-rs@users.noreply.github.com>
Co-authored-by: vishnuszipstack <117254672+vishnuszipstack@users.noreply.github.com>

* ReverseMerge: V0.163.4 hotfix (#1980)

* [HOTFIX] Use importlib.util.find_spec for pluggable worker discovery (#1918)

* [FIX] Use importlib.util.find_spec for pluggable worker discovery

_verify_pluggable_worker_exists() previously checked for the literal file
`pluggable_worker/<name>/worker.py` on disk, which breaks when the plugin
has been compiled to a .so (Nuitka, Cython, or any C extension) — the
module is perfectly importable but the pre-check rejects it because only
the .py extension is considered.

Replace the filesystem check with importlib.util.find_spec(), which is
Python's standard way to ask "is this module resolvable by the import
system?". It honors every registered finder — source .py, compiled .so,
bytecode .pyc, namespace packages, zipimports — so the function now
matches what its docstring claims: verifying the module can be loaded,
not that a specific file extension is present.

Behavior is preserved for existing deployments:
- Images with no `pluggable_worker/<name>/` subpackage → find_spec
  raises ModuleNotFoundError (ImportError subclass) → returns False.
- Images with source .py → find_spec resolves the .py → returns True.
- Images with compiled .so → find_spec resolves the .so → returns True.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [FIX] Handle ValueError from find_spec in pluggable worker verification

Greptile-flagged edge case: importlib.util.find_spec() can raise
ValueError (not just ImportError) when sys.modules has a partially
initialised module entry with __spec__ = None from a prior failed import.
Broaden the except to catch both.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [FIX] Resolve api-deployment worker directory from enum import path

worker.py:452 did worker_type.value.replace("-", "_") to derive the
on-disk dir name. All WorkerType enum values already use underscores,
so the replace was a no-op; for API_DEPLOYMENT whose dir is
"api-deployment" (hyphen), it resolved to "api_deployment" and the
os.path.exists() check failed. Boot then logged a spurious
"❌ Worker directory not found: /app/api_deployment" at ERROR level.

The task registration path (builder + celery autodiscover via
to_import_path) is unaffected, so this was purely log noise — but
noise at ERROR level that masks real failures in log scans.

Fix: derive the directory from the authoritative to_import_path()
which already handles the hyphen case (api_deployment -> api-deployment).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [HOTFIX] Add IAM Role / Instance Profile auth mode to AWS Bedrock adapter (#1944)

* [FEAT] Allow Bedrock to fall through to boto3's default credential chain

Match the S3/MinIO connector pattern: when AWS access keys are left blank
on the Bedrock LLM and embedding adapter forms, drop them from the kwargs
dict so boto3's default credential chain handles authentication. This
unlocks IAM role / instance profile / IRSA / AWS Profile scenarios on
hosts that already have ambient AWS credentials (e.g. EKS workers with
IRSA, EC2 with an instance profile).

- llm1/static/bedrock.json: clarify access-key descriptions to mention
  IRSA and instance profile (already non-required at v0.163.2 base).
- embedding1/static/bedrock.json: drop aws_access_key_id and
  aws_secret_access_key from top-level required; same description fix;
  expose aws_profile_name for parity with the LLM form.
- base1.py: AWSBedrockLLMParameters and AWSBedrockEmbeddingParameters
  now strip empty access-key values from the validated kwargs before
  returning, so empty strings don't override boto3's default chain.
  AWSBedrockEmbeddingParameters fields gain explicit None defaults
  and an aws_profile_name field.

Backward-compatible: existing adapters with access keys filled in
continue to work unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [FEAT] Add Authentication Type selector to Bedrock adapter form

Add an explicit `auth_type` selector with two options, making the auth
choice clear to users:

- "Access Keys" (default): existing flow, keys required
- "IAM Role / Instance Profile (on-prem AWS only)": no fields; relies on
  boto3's default credential chain (IRSA on EKS, task role on ECS,
  instance profile on EC2). Description on the selector explicitly notes
  this option is only for AWS-hosted Unstract deployments.

The form-only auth_type field is stripped before LiteLLM validation in
both AWSBedrockLLMParameters.validate() and AWSBedrockEmbeddingParameters.
validate(). Empty access keys continue to be stripped so boto3 falls
through to the default chain even when the access_keys arm is selected
without values (matches the S3/MinIO connector pattern).

Backward-compatible: legacy adapters without auth_type behave as
"Access Keys" mode (the default), and existing keys are forwarded
unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [REVIEW] Address Bedrock auth_type review feedback

Fixes the P0/P1 issues raised by greptile-apps and jaseemjaskp on
PR #1944.

Behaviour fixes:
- Stale-key leak in IAM Role mode: switching an existing adapter from
  Access Keys to IAM Role would carry truthy stored access keys through
  the strip-empty-only loop, so boto3 silently authenticated with the
  old long-lived credentials instead of falling through to the host's
  IRSA / instance-profile identity. Both LLM and embedding paths were
  affected.
- Silent acceptance of unknown auth_type: a typo (e.g. "access_key") or
  a malformed payload from a non-UI client passed through the dict
  comprehension untouched, with no enum guard.
- Cross-field validation gap: explicit Access Keys mode with blank or
  whitespace-only values silently fell through to the default
  credential chain instead of surfacing the misconfiguration.

Implementation:
- Add a module-level _resolve_bedrock_aws_credentials helper used by
  both AWSBedrockLLMParameters.validate() and AWSBedrock
  EmbeddingParameters.validate(), so the auth-type contract is
  expressed once.
  - Validates auth_type against an allowlist (None | "access_keys" |
    "iam_role"); raises ValueError on anything else.
  - iam_role: unconditionally drops aws_access_key_id and
    aws_secret_access_key.
  - access_keys (explicit): requires non-blank values; raises ValueError
    if either is empty or whitespace-only.
  - Legacy (auth_type absent): retains the lenient strip behaviour so
    pre-PR adapter configurations continue to deserialise unchanged.
- Restore aws_region_name as required (no `= None` default) on
  AWSBedrockEmbeddingParameters; only credentials may legitimately be
  absent.
- Drop the orphan aws_profile_name field from
  embedding1/static/bedrock.json: it was added for parity with the LLM
  form but lives outside the auth_type oneOf and contradicts the
  selector's "no further input" semantics. The LLM form already had
  aws_profile_name pre-PR and is left alone for backwards compatibility.

Tests:
- New tests/test_bedrock_adapter.py covers 15 cases across LLM and
  embedding adapters: legacy-no-auth-type, explicit access_keys with
  valid/blank/whitespace keys, iam_role with stale/no keys, unknown
  auth_type rejection, cross-field validation, and preservation of
  unrelated params (model_id, aws_profile_name, region, thinking).

Skipped (P2 nice-to-have):
- Comment-scope clarification, MinIO reference rewording,
  validate-mutates-caller'\''s-dict, and the LLM form description nit
  about aws_profile_name visibility. These don'\''t change behaviour
  and can be addressed in a follow-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [HOTFIX] Bump litellm to 1.83.10 from PyPI to clear CVE-2026-42208 (#1976)

Hotfix for cloud v0.159.3 (OSS v0.163.4). Customer scanner flagged
litellm 1.82.3 for CVE-2026-42208 (SQL injection in litellm proxy auth
path, affects 1.81.16-1.83.6). We do not use litellm.proxy, but
vulnerability scanners flag the installed package regardless of which
code path is reachable.

Bump to 1.83.10 — the exact version recommended by the upstream advisory
(v1.83.10-stable) and the smallest jump that clears the CVE range while
keeping python-dotenv==1.0.1 compatible (1.83.14 would force bumping
python-dotenv across 7+ pyproject.toml files). Only tiktoken needed to
move 0.9 -> 0.12 to satisfy litellm's pin.

Switch source back to PyPI now that the PyPI quarantine is over,
reversing the temporary fork in #1873.

Cohere embed timeout patch: verified that
litellm/llms/cohere/embed/handler.py is byte-identical between v1.82.3,
v1.83.10-stable, and v1.83.14-stable (the timeout-not-forwarded bug
fixed in #1848 is still present upstream — BerriAI/litellm#14635 remains
OPEN). Version guard bumped 1.82.3 -> 1.83.10; 6/6 patch tests pass on
the new version, confirming the monkey-patch still binds correctly.

Other cleanup from #1873:
- Drop git apt-install from worker-unified and tool Dockerfiles (no
  git-sourced deps remain in any uv.lock)
- Bump tool versions: structure 0.0.100 -> 0.0.101,
  classifier 0.0.79 -> 0.0.80, text_extractor 0.0.75 -> 0.0.76

Note on root uv.lock churn: the v0.163.4 root uv.lock had a pre-existing
corruption (banks v2.4.1 entry pointing at banks-2.2.0 wheel) that
blocked incremental resolution. Regenerated from scratch.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* [FIX] Align cohere patch docstring with version-guard semantics

Reviewer flagged that the docstring claimed the patch is "confirmed in
every release between 1.82.3 and 1.83.14-stable", but the guard at
_PATCHED_LITELLM_VERSION activates only on the exact pinned version. A
future maintainer reading the old text could reasonably expect bumping
to e.g. 1.83.11 to keep the fix active; in reality it silently turns
off.

Rewritten to reference _PATCHED_LITELLM_VERSION as the single source of
truth and to drop the rot-prone "as of 2026-05-20" calendar date.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* UN-3476 [FIX] Revert atomic wrap on set_user_organization (#1977)

The atomic wrap from #1954 uncommits the new org row when
frictionless_onboarding HTTP-calls the LLMW portal mid-transaction.
The portal runs on a separate DB session and under READ COMMITTED
cannot see the uncommitted row, so the call returns 400 and the
caller silently persists an adapter with an empty unstract_key.
Every new signup since 2026-05-19 09:47 UTC ships a broken
free-trial X2Text adapter (401 on first OCR).

Hotfix only — Phase 2 (UN-3476) restructures the function so the
atomic guarantee is reapplied around just the pure-DB writes, with
HTTP and non-DB side effects moved outside the transaction.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Restore text_extractor tool removed in Phase 5 decommission

The Phase 5 decommission commit removed classifier, structure,
text_extractor, and prompt-service. However, text_extractor is still
in active use by customers. This surgically restores only the
text_extractor tool while keeping the other decommissions in place.

- Restore tools/text_extractor/ directory (14 files from origin/main)
- Add tool-text_extractor back to docker-compose.build.yaml
- Add tool-text-extractor back to docker-tools-build-push.yaml workflow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Restore classifier tool removed in Phase 5 decommission

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Remove unit-prompt-service group from test rig manifest

The prompt-service directory was deleted in the decommission PR, but
the test rig groups.yaml still referenced it, causing CI to fail with
"workdir does not exist" during validate and integration steps.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Remove deleted prompt-service and structure tool refs from bump script

prompt-service/ and tools/structure/ are deleted by this PR, so
remove their variables, reset_file calls, and the entire
update_structure_tool_version function from bump_sdk_v0_version.sh.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix stale references from decommissioned components

- Fix tool-text_extractor image name to tool-text-extractor in
  docker-compose.build.yaml to match CI, registry, and cloud naming
- Remove stale tool-structure from run-platform.sh ignore list
- Drop prompt-service from is_retryable_error docstring in retry_utils.py

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Trigger CI re-run

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Signed-off-by: Praveen Kumar <praveen@zipstack.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Praveen Kumar <praveen@zipstack.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: Deepak K <89829542+Deepak-Kesavan@users.noreply.github.com>
Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
Co-authored-by: Athul <89829560+athul-rs@users.noreply.github.com>
Co-authored-by: vishnuszipstack <117254672+vishnuszipstack@users.noreply.github.com>
Co-authored-by: jimmy <ziming_zhu2002@163.com>
Co-authored-by: Hari John Kuriakose <hari@zipstack.com>
Co-authored-by: Chandrasekharan M <chandrasekharan@zipstack.com>
Co-authored-by: Athul <athul@zipstack.com>

* UN-3185 [FIX] Deterministic CSS cascade (single bundle) + asset gzip + Edit LLM Profile modal layout (#2128)

* fix(frontend): bundle all CSS into one file to fix lazy-load cascade order

Code-splitting (#2114) injects per-route CSS at navigation time, so
equal-specificity rules across components resolve in load order — breaking
the Edit LLM Profile form and Execution Logs header layout (UN-3185).
cssCodeSplit:false restores a single deterministic stylesheet; JS splitting
is unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* perf(frontend): gzip static assets (css/js/fonts), not just html

nginx `gzip on` only compresses text/html by default, so JS/CSS shipped
uncompressed. Add gzip_types so the single CSS bundle (~341KB -> ~57KB) and
all JS chunks compress on the wire — offsets the cssCodeSplit:false upfront
cost and speeds every asset.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* fix(frontend): clean up Edit LLM Profile modal padding & drop dead scroll classes

Remove settings-body-pad-top + add-llm-profile-scroll-root from the form root
(the shared overflow-y:auto added a nested scroll context that #2119 only
patched over). The sticky footer now pins directly to .conn-modal-col. Replace
the dead override with a :has()-scoped padding so .conn-modal-form-pad-left is
adjusted for this panel alone — sibling settings panels and the connector
dialog are untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* perf(frontend): gzip_proxied any so assets compress behind the LB

nginx skips gzip for proxied requests by default (gzip_proxied off); the GKE
load balancer adds a Via header to every request, so static assets shipped
uncompressed despite gzip_types. Verified gzip works direct-to-pod.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* fix(frontend): remove inner scrollbar on Edit LLM Profile modal

The settings row is hard-fixed at 800px, forcing the form's column
(.conn-modal-col) to scroll once Advanced Settings expands. Let the row size to
the form so the modal grows instead. Scoped via :has to the LLM profile panel —
the other settings panels and the connector dialog share .conn-modal-row/-col
and are untouched (so the shared class stays; it is not unused).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* docs: tighten code comments in CSS/vite/nginx changes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* style: drop comments from AddLlmProfile.css changes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

* fix: drop pre-compressed font types from gzip_types

WOFF2 (Brotli) and WOFF (zlib) are already compressed; re-gzipping wastes
CPU for no size gain. application/font-woff also never matches nginx's
mime.types (.woff -> font/woff), so it was a no-op.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015DsoHbMN7kTTWwcVC6NNkg

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-3185 [FIX] Restore global Prism for prismjs add-ons (Prompt Studio detail + HITL blank page) (#2135)

* UN-3185 [FIX] Restore global Prism for prismjs add-ons (Prompt Studio detail + HITL blank page)

The Vite production build tree-shakes the bare `import "prismjs"` in
CombinedOutput.jsx (a side-effect-only import with no used bindings), so
nothing installs the global `Prism` that `prismjs/components/prism-json`
and the line-numbers plugin reference. Those add-ons then throw
`ReferenceError: Prism is not defined` at module evaluation, crashing the
Combined Output viewer shared by the Prompt Studio detail page and the
HITL / manual-review page (both render blank). The old CRA/webpack build
did not tree-shake it, so the regression only surfaces on the Vite build.

Add a dedicated prismSetup module that imports Prism core with a *used*
binding (survives tree-shaking) and pins it on globalThis, and import it
before the add-ons in CombinedOutput so the global is guaranteed present
by the time the add-on modules evaluate.

Verified against a production `vite build`: the emitted chunk now runs
`globalThis.Prism = <core>` immediately before `Prism.languages.json = ...`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* UN-3185 [FIX] Simplify globalThis guard in prismSetup (review)

Drop the always-true `typeof globalThis !== "undefined"` check — globalThis
is universally available in any ESM/Vite target. Addresses Greptile review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* UN-3185 [FIX] Install global Prism unconditionally + correct rationale (review)

Address PR review:
- silent-failure-hunter: a `!globalThis.Prism` guard could leave a different,
  pre-existing Prism in place, so the add-ons extend one instance while
  JsonView's highlightAll() reads another -> JSON silently unhighlighted.
  Assign unconditionally so the global is provably our core instance.
- comment-analyzer: the prior comment blamed evaluation order yet relied on it
  to justify the fix. Rewrite: relying on prismjs core's self-install is
  unreliable under code-splitting; the explicit globalThis assignment (from
  first-party code imported before the add-ons) is the deterministic fix.

Rebuilt: emitted chunk runs `globalThis.Prism = <core>` immediately before
`Prism.languages.json = ...`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* UN-3185 [FIX] Install global Prism eagerly at app entry (fixes HITL too)

Verified against the actual on-prem image (built with the manual-review
plugin): the per-component prismSetup import did NOT fix it. Because
manual-review's ResultEditor also imports `prismjs/components/prism-json`,
Rollup hoists that add-on into a SHARED lazy chunk (PdfViewer), separate
from CombinedOutput's chunk where prismSetup ran — with no ordering
guarantee between two lazy chunks, so the add-on still evaluated before the
global was installed. My local OSS build masked this: with no manual-review
plugin, prism-json wasn't shared and stayed in CombinedOutput's chunk.

Fix: import prismSetup EAGERLY from index.jsx so `globalThis.Prism` is
installed at bootstrap, before any lazy chunk (including the shared add-on
chunk) can load. Robust regardless of how Rollup hoists prism-json.

Reproduced the shared-chunk hoist locally (two independent lazy prism-json
importers) and confirmed: prism-json lands in its own lazy chunk while
globalThis.Prism stays in the eager entry <script type=module>, so the
global is always installed first. Fixes both the Prompt Studio detail page
and the HITL review page.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* UN-2265 [FEAT] Show the running platform version on the profile page (#2034)

* UN-2265 Display platform version in the frontend profile page

Bake the build VERSION into the frontend image as
UNSTRACT_APPS_VERSION (same pattern as backend.Dockerfile), surface it
through the runtime config injected at container start, and show it as
a 'Platform Version' field on the profile page. Production CI already
passes *.args.VERSION to all images via docker bake, so published
images carry the real version on every deployment target; the field is
hidden when no version is available (e.g. local npm start).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* UN-2265 Address review: empty VERSION default, escape value in runtime config

- ARG VERSION defaults to empty so images built without a version hide
  the field instead of showing 'dev'
- Escape backslashes/quotes before embedding the version in the
  generated JS

* UN-2265 [FIX] Source platform version from runtime env, not baked image

RC->stable promotion re-tags images (it does not rebuild), so a version baked
into the frontend image at build time would stay frozen at the rc tag after
promotion. Drop the frontend.Dockerfile ARG/ENV baking and instead supply
UNSTRACT_APPS_VERSION from docker-compose using ${VERSION} -- the same value
that already tags the image, so the displayed version always matches what runs.

Addresses review from @ritwik-g / @ejhari (version must come dynamically from
compose in OSS and the Helm chart in Enterprise).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* [MISC] Add 'auto' version bump to OSS create-release (#2136)

* [MISC] Add 'auto' version bump to OSS create-release

Adds an 'auto' choice (now the default) to create-release.yaml that picks
the OSS version bump from merged PR titles: a [FEAT]/[GATED-FEAT] PR merged
since the last release -> minor, otherwise patch. These are the only feature
PR types in the contribution guide, so this matches the documented SemVer
intent without any new labeling.

Details:
- 'auto' resolves in a new "Resolve auto bump" step (main mode only; hotfix
  lines stay patch-only). Fail-safe is always patch, so a compare/PR query
  hiccup never over-bumps the public version.
- Hotfix-mode input validation now accepts 'auto' (maps to patch).
- compute-version consumes the resolved bump; dry-run/final summaries show
  "auto -> minor/patch" for an explicit audit trail.
- 'auto' never selects major — that stays behind the confirm_major gate.

Validated by replaying the classifier over the last 13 real releases: 12/13
matched the human bump; the lone diff was a discretionary minor with no FEAT
PR (recoverable via manual minor override, which the notice points to).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [MISC] Address review: PR-read perms, patch fallback, truncation warning

- Add `pull-requests: read` to the job (the permissions block sets unlisted
  scopes to none, so `gh pr list` would 403 and 'auto' would silently always
  fall back to patch). [CodeRabbit]
- BUMP_TYPE falls back to 'patch' instead of 'auto' when resolved is empty,
  avoiding a latent "Unknown bump type: 'auto'" job failure. [Greptile]
- Surface a warning when the merged-PR query hits the 200-result cap instead
  of silently under-counting FEAT PRs. [Greptile]

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* [MISC] Harden auto-bump resolve step (review follow-ups)

From a multi-agent review of the auto-bump step:

- Guard the jq parses: malformed/non-array stdout now degrades to patch
  instead of crashing the job under set -e/pipefail (upholds the "never fail
  the release outright" invariant). Also validates TOTAL/FEAT_COUNT are numeric
  before arithmetic.
- Detect gh failure by exit status (if ! PR_JSON=$(...)) and surface captured
  stderr, so a permanent 403 (e.g. dropped pull-requests scope) is diagnosable
  rather than a cause-free warning that silently patches forever.
- Add '// empty' to the published_at lookup for consistency with get-latest,
  so a JSON null can't leak through as the literal "null".
- Only emit the truncation warning when no FEAT was found within the cap (the
  only case where truncation could change the outcome).
- Fix an inaccurate comment ("silently" -> "with a warning") and reword the
  self-referential permissions comment.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* UN-3648: Mark API deployment execution ERROR on synchronous staging failure (#2120)

* UN-3648: Mark API deployment execution ERROR on synchronous staging failure

When an API-deployment run failed synchronously at the "Staging files in
API storage" step (add_input_file_to_api_storage, before async dispatch),
the PENDING WorkflowExecution row was never marked ERROR, so the UI showed
the run as stuck/running forever and the real error wasn't surfaced.

Root cause: in api_v2/deployment_helper.py -> execute_workflow(), the
staging call sat outside the try/except. Only execute_workflow_async
failures were handled, and the DB row is marked ERROR by
execute_workflow_async internally -- a path a staging failure never reaches.

Fix:
- Move staging inside the try so synchronous failures are handled.
- In the except, explicitly call update_execution_err() to mark the row
  ERROR with the surfaced reason (the existing handler only did cleanup +
  built an error response, it never marked the DB row).

Add a regression test (sys.modules-stub style, no Django/DB) asserting a
staging exception marks the execution ERROR, releases the rate-limit slot,
and never reaches async dispatch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-3648: scope staging error-handling to pre-dispatch failures (review)

Address Greptile P1 + CodeRabbit Major on PR #2120:

- Give the synchronous staging call its own try/except instead of sharing the
  try with execute_workflow_async + post-dispatch processing. Error-marking
  (update_execution_err) now applies only to genuine pre-dispatch failures and
  can no longer overwrite an already-dispatched/completed execution's status.
- Isolate the update_execution_err call in its own try/except so a transient DB
  error while marking ERROR no longer skips slot release and storage cleanup
  (which were unconditional before this PR).
- Restore the dispatch/post-processing except to its original behaviour (slot
  release + storage cleanup only); dispatch failures are already marked ERROR
  internally by execute_workflow_async.

Add a regression test asserting cleanup still runs when update_execution_err
itself raises.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Drop ticket/incident references from in-code comments

Keep code comments focused on explaining the code; ticket and incident context
lives in the commit/PR history instead.

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* UN-3584 [FEAT] Restrict LLM adapter creation to org admins (controlled mode) (#2132)

* UN-3584 [FEAT] Restrict LLM adapter creation to org admins (controlled mode)

Add a per-org 'restrict_llm_adapter_creation' setting (default off). When an
org admin enables it, only organization admins may create LLM adapters:
non-admin create requests are rejected with 403 and a contact-admin message.
Other adapter types and the default-off behavior are unchanged.

- account_v2: new BooleanField on Organization + migration
- tenant_account_v2: admin-only GET/PATCH organization/settings endpoint
- adapter_processor_v2: enforce the gate in AdapterInstanceViewSet.create
- frontend: admin-only toggle in Platform Settings

* UN-3584 [FIX] Gate LLM-restriction toggle to enterprise builds (hide in OSS)

Org admin roles / user management don't exist in OSS, so the controlled-mode
toggle must not render there. Probe for an enterprise-only plugin (absent in
OSS) and show the toggle only on enterprise/cloud builds, mirroring the
plugin-gating idiom in SideNavBar. Backend is already OSS-safe: the flag
defaults off and the create gate never fires.

* UN-3584 [FIX] Address PR review: service-account bypass, modified_by audit, extract gate

- Bypass the LLM-creation gate for service accounts (platform API-key
  sessions), consistent with how the rest of the permission layer treats
  them (Greptile P1).
- Set Organization.modified_by on the settings PATCH so the audit field
  isn't left stale (Greptile P2).
- Extract the controlled-mode check into _enforce_llm_creation_restriction
  to bring AdapterInstanceViewSet.create back under the cognitive-complexity
  limit (SonarQube).

* UN-3584 [FIX] Bind adapter creation to request org (prevent payload org override)

AdapterInstanceSerializer exposes organization via fields=__all__, and
DefaultOrganizationMixin.save only fills it when None — so a payload-supplied
organization would persist. Pass organization=UserContext.get_organization()
to serializer.save() so the row is bound to the same request-scoped org the
controlled-mode check evaluates, closing a per-org restriction bypass
(CodeRabbit security finding).

* UN-3585 [FEAT] Restrict connector creation to org admins (controlled mode) (#2145)

* UN-3584 [FEAT] Restrict LLM adapter creation to org admins (controlled mode)

Add a per-org 'restrict_llm_adapter_creation' setting (default off). When an
org admin enables it, only organization admins may create LLM adapters:
non-admin create requests are rejected with 403 and a contact-admin message.
Other adapter types and the default-off behavior are unchanged.

- account_v2: new BooleanField on Organization + migration
- tenant_account_v2: admin-only GET/PATCH organization/settings endpoint
- adapter_processor_v2: enforce the gate in AdapterInstanceViewSet.create
- frontend: admin-only toggle in Platform Settings

* UN-3584 [FIX] Gate LLM-restriction toggle to enterprise builds (hide in OSS)

Org admin roles / user management don't exist in OSS, so the controlled-mode
toggle must not render there. Probe for an enterprise-only plugin (absent in
OSS) and show the toggle only on enterprise/cloud builds, mirroring the
plugin-gating idiom in SideNavBar. Backend is already OSS-safe: the flag
defaults off and the create gate never fires.

* UN-3584 [FIX] Address PR review: service-account bypass, modified_by audit, extract gate

- Bypass the LLM-creation gate for service accounts (platform API-key
  sessions), consistent with how the rest of the permission layer treats
  them (Greptile P1).
- Set Organization.modified_by on the settings PATCH so the audit field
  isn't left stale (Greptile P2).
- Extract the controlled-mode check into _enforce_llm_creation_restriction
  to bring AdapterInstanceViewSet.create back under the cognitive-complexity
  limit (SonarQube).

* UN-3584 [FIX] Bind adapter creation to request org (prevent payload org override)

AdapterInstanceSerializer exposes organization via fields=__all__, and
DefaultOrganizationMixin.save only fills it when None — so a payload-supplied
organization would persist. Pass organization=UserContext.get_organization()
to serializer.save() so the row is bound to the same request-scoped org the
controlled-mode check evaluates, closing a per-org restriction bypass
(CodeRabbit security finding).

* UN-3585 [FEAT] Restrict connector creation to org admins (controlled mode)

* UN-3585 [FIX] Address PR review: split settings validation from mutation; correct org-binding comment

* UN-3585 [FIX] Hoist connector admin check before payload validation (fail-fast)

* UN-3586 [FEAT] Allow platform API key rotation via API (#2133)

* UN-3586 [FEAT] Allow platform API key self-rotation via API

Operational automation (RLDatix) needs credential rotation through the API,
but the rotate endpoint was gated by IsOrganizationAdmin, which rejects
service accounts — so a platform API key could not rotate itself.

Add CanRotatePlatformApiKey for the rotate action: session callers still must
be org admins (may rotate any key in the org), while a platform API key
(bearer) may rotate ONLY its own key (pk == request.platform_api_key.id).
Cross-org access remains impossible (auth middleware + org-scoped queryset);
this only relaxes the intra-org gate to self-rotation. read keys still can't
POST (middleware), so only read_write/full_access keys reach rotate.

rotate returns the new key once via PlatformApiKeyDetailSerializer. No model
or migration change.

* UN-3586 [FIX] Move rotate self-check to has_object_permission (PR review)

Relocate the 'key may rotate only its own key' constraint from has_permission
(view-level, via view.kwargs[pk]) to has_object_permission, the idiomatic DRF
location for per-object access control — it receives the already-fetched obj.
has_permission stays as the coarse session-vs-key gate. Behavior is unchanged
(self-rotate 200, cross-key 403): this permission is only used by the rotate
detail action, which calls get_object() and so always triggers the
object-level check. (Greptile)

* UN-3586 [FIX] Allow key-based callers to rotate any key (drop self-only)

Empirically confirmed on staging that rotating a platform API key via the API
(bearer token) is blocked (403 'Only organization admins...') — the automation
path UN-3586 asks for. Enable it: a platform API key caller may rotate, same
as a session org admin. Drop the earlier self-only restriction (not a ticket
requirement) and the now-unneeded has_object_permission. Org scoping (auth
middleware + org-scoped queryset) still confines a key to its own org; read
keys still can't POST (middleware). rotate already returns the new key.

* UN-3586 [FIX] Require full_access key to rotate (close privilege escalation)

Greptile caught a real privilege escalation: after dropping self-only, a
read_write bearer key could rotate a full_access key and read its new secret
from the rotate response (rotate returns the new key), escalating read_write
-> full_access. Fix: key-based callers must present a full_access key to
rotate. read_write keys can no longer rotate; a full_access caller rotating
any key gains no privilege (already top tier) and matches what a session
admin can do. Session-admin rotate and org scoping unchanged.

* UN-3636 [MISC] Make the rig unit/integration CI job green on main (#2115)

* UN-3632 [FIX] Scope rig --fail-on-critical-gap to in-tier coverage so main runs green

The rig's unit/integration CI job had never passed on main. `--fail-on-critical-gap`
(passed only on the main push) failed the build on EVERY uncovered critical path,
including e2e-only paths run during the unit/integration tier and paths with no test
anywhere. Every group-level failure was already non-gating (optional groups, or exit
5 = no tests collected), so the gap gate was the sole cause of redness.

- Split critical-path gaps into in-scope vs out-of-scope (add `in_scope` to
  CriticalPathStatus; evaluate() already computed it). --fail-on-critical-gap now
  gates only on in-scope gaps — a declared in-tier covering group that didn't run
  green (real coverage regressed). Out-of-scope gaps (covered only by an unrun tier,
  or no declared coverage) are reported + logged but never gate that tier.
- Wire honest coverage that exists today: adapter-register-llm -> unit-sdk1. Gives
  the path teeth: if sdk1 regresses, it flips to an in-scope gap and fails.
- Drop unit-tool-registry group (component slated for removal; can't even collect).
- Delete 3 dead tests referencing removed code: core test_pandora_account.py
  (account_services removed), core test_pubsub_helper.py (LogHelper -> LogPublisher),
  platform-service test_auth_middleware.py (platform_service.main removed; also made
  live Postgres calls). unit-platform-service is green via its hermetic memory-leak
  test; unit-core has no valid tests left and skips as a placeholder.

New self-tests cover the in-scope/out-of-scope split at both evaluate() and cmd_run().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XJqp7xMdd1kjLUKbvrJsq4

* test: prune dead rig groups/paths, park deprecated services, wire backend tests

Follow-up cleanup on the rig manifests (UN-3636):

- Park unit-runner and unit-prompt-service (commented out, not run by
  default) with a TODO to delete when those services are removed — both are
  being decommissioned; no value testing components on their way out.
- Drop the unit-tool-registry NOTE block entirely.
- Prune 6 unit-backend paths that collect zero tests (account_v2,
  api_deployment_v2, connector_v2, file_management, project,
  tenant_account_v2 — dirs missing or empty); optional skip-if-missing was
  hiding them and implying coverage that was never written.
- Wire two real backend tests into unit-backend:
    * middleware/test_exception.py — 5 hermetic tests, pass now.
    * prompt_studio/prompt_studio_core_v2/tests — pins the executor
      _handle_ide_index async path (no prompt-service coupling); runs once
      unit-backend is un-gated, skips safely until then.
- Comment out the tool-sandbox-exec critical path (TODO: remove with
  tool-registry/runner) — its covering group unit-runner is now parked.

`python -m tests.rig validate` → OK (13 groups, 9 critical paths).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* fix: make rig editable-install survive uv run re-sync; drop phantom Django setting

unit-core ran 0 tests / 2 collection errors (ModuleNotFoundError: No module
named 'unstract'). Root cause: _prepare_group_env did `uv pip install -e .`
for install_editable groups, but _pytest_command runs `uv run`, which
re-syncs the venv every call and wipes that install before pytest imports the
package — the same hazard the code already flagged for plugins.

Inject the package via `uv run --with-editable <workdir>` (survives the
re-sync, same mechanism as the RIG_PYTEST_PLUGINS `--with` specs) and drop the
wiped `uv pip install -e .`. unit-core now 27 passed, 0 errors.

Also remove DJANGO_SETTINGS_MODULE from the repo-root [tool.pytest.ini_options]:
it's a pytest-django option that warns "Unknown config option" for every
non-Django group, points at a non-existent module (backend.settings.test_cases),
and Django settings don't belong at the polyglot repo root. The rig injects it
per-group via groups.yaml env for unit-backend only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: make unit-backend collect + run django_db tests in the rig

The unit-backend group pointed at a non-existent settings module
(backend.settings.test_cases) and ran without pytest-django, so the
DB-backed tests errored at collection (Django uninitialised) and the
django_db tests had no test-DB lifecycle. Make the group actually runnable:

- Add pytest-django to the backend test group (bootstraps Django before
  collection; provides the test-DB + django_db fixtures).
- Point DJANGO_SETTINGS_MODULE at the existing backend.settings.test and
  inject the import-time-required settings via the group env — base.py reads
  them before any dotenv load, so they must exist in the process env, not a
  settings module. ENCRYPTION_KEY is an all-zero (valid, zero-entropy) Fernet
  placeholder, not a real secret.
- Set DB_SCHEMA=public: the app's fixed schema doesn't exist in the fresh
  test DB and tenancy is row-level, so migrations run in public.
- Drop workflow_manager/endpoint_v2/tests from the wired paths: its
  destination-connector tests import the enterprise `plugins` package,
  absent in OSS.
- Add the missing utils/file_storage{,/helpers}/__init__.py so those
  modules import as a package under pytest.
- Stop test_build_index_payload's sys.modules stubs from leaking into
  sibling collection: record + restore the originals once the helper is
  imported (a stubbed account_v2.models was breaking other modules'
  real imports).

unit-backend now collects clean; 126 passed, 4 skipped. The remaining 6
failures (usage_v2 helper stubs, dashboard_metrics cleanup tasks) are
pre-existing test bugs tracked separately.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: fix two pre-existing backend test bugs exposed by the rig

dashboard_metrics: organization FK targets Organization's int PK, but the
tests passed a UUID string as organization_id, and verified rows through
the org-scoped default manager (empty without a UserContext). Create a
real Organization and read via _base_manager.

usage_v2: drop the fragile "stub usage_v2.models into sys.modules before
import" trick — under pytest-django Django imports the real module first,
so the stub never took and the helper hit the DB. Rebind the Usage symbol
the helper resolved instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: gate live connector integration tests behind credential env vars

The connector suite had 12 reds that needed real external services or
per-developer credentials no one has by default. Two were genuine bugs;
the rest are integration tests masquerading as unit tests.

Fixes (not credential-related):
- mariadb: assertion text drifted from the connector's actual message
  ("SSL SETTINGS", not "ssl-settings").
- sharepoint: skip test_json_schema_has_is_personal — is_personal is read
  from settings in code but was never exposed in json_schema.json (personal
  vs site is inferred from an empty site_url). Whether the schema should
  expose it is a product decision; tracked under UN-3414.

Gating (skipUnless, mirrors the existing SharePoint integration tests):
- filesystems (box, gdrive, minio, pcs, dropbox) already read creds from
  env; add the missing skip guard so they SKIP instead of failing.
- databases (mssql, mysql, postgresql, redshift, snowflake) had hardcoded
  personal creds (incl. a live-looking neon.tech URL and a Snowflake
  account) querying bespoke tables. Move creds to *_TEST_* env vars and
  skip unless provided, removing the secrets from the repo.

CI can run these by injecting the corresponding secrets as env vars in a
dedicated integration job; by default they skip cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: make unit-core and unit-connectors required rig groups

Both now run green and standalone (no external services; integration
tests skip cleanly when credentials are absent), so drop optional: true
to make them blocking merge gates per UN-3635.

unit-backend stays optional until the rig provisions a reachable DB_HOST
for it (UN-3636 follow-up).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* test: address PR review feedback on rig + connector test guards

- in_scope defaults False on CriticalPathStatus so a future evaluate()
  regression that forgets it under-gates (warning) rather than over-gates
  (spurious build block). [greptile]
- widen connector integration skip guards (redshift, snowflake, gdrive,
  minio, pcs) to require every env var the test hard-references, so a
  partially configured env skips cleanly instead of failing. [coderabbit]
- usage_v2 test_helper: swap Usage via an autouse monkeypatch fixture
  instead of a module-level rebind that leaks FakeUsage into later tests.
- build_index_payload test: evict the helper module from sys.modules after
  binding it, so later importers in the same process get a real copy.
- drop dead tox `runner` alias (its unit-runner group was removed).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: provision infra for integration tier; split DB/credential tests out of unit

Make `requires_services` actually provision instead of being cosmetic. The rig
now brings up testcontainers infra (Postgres/MinIO) for any runnable group that
declares `requires_services`, and injects connection env into the group's pytest
subprocess (Postgres URL -> discrete DB_* vars; MinIO endpoint/creds). Previously
django_db tests fell back to the compose hostname `backend-db-1`, unreachable
from host-side pytest, so unit-backend had to be `optional`.

Reclassify infra-dependent tests by the rig's own tier taxonomy (unit = no
external services, integration = real infra but not the full platform):

- backend: split unit-backend into pure `unit-backend` (gates unit tier, no
  infra) and `integration-backend` (django_db tests: dashboard_metrics +
  prompt_studio_registry_v2; provisioned Postgres; gates integration tier).
- connectors: marker-based split (tests are interleaved within files). Credential
  + MinIO tests marked `@pytest.mark.integration`; `unit-connectors` runs
  `-m "not integration"`, new `integration-connectors` runs `-m "integration"`.
  test_minio actually runs against provisioned MinIO; external-credential tests
  skip. Skip-guard test_http_fs (was hitting a live URL unguarded in CI).

Both groups are non-optional and gate their tiers. Unit tier stays infra-free.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: address PR review — wire provisioned Redis, mark http_fs integration, cut rig complexity

- integration-backend declares requires_services: [postgres, redis] but the rig
  only injected Postgres/MinIO env, so Redis-backed tests bypassed the
  testcontainer and hit localhost:6379. Inject REDIS_HOST/PORT +
  CELERY_BROKER_BASE_URL from the provisioned endpoint (CodeRabbit).
- test_http_fs was skip-guarded but unmarked, so the connector marker split
  (-m "not integration") could still run it in the unit tier. Mark the module
  integration (CodeRabbit).
- Extract _inject_infra_env and _pytest_base_cmd to bring both functions under
  SonarCloud's cognitive-complexity threshold. NOSONAR the test's DB_PASSWORD
  placeholder (not a real credential).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: use hostname not literal IP in redis-wiring test (Sonar hotspot)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: mark local testcontainers MinIO http endpoint NOSONAR

The MinIO endpoint is a throwaway testcontainer with no TLS, so http is
expected. Suppress the SonarCloud insecure-protocol hotspot that otherwise
blocks the quality gate on new code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: stub UserDefaultAdapter so prompt-studio build-index tests run

The module stubs adapter_processor_v2.models to import PromptStudioHelper
without the full Django app, but only provided AdapterInstance. The helper
also imports UserDefaultAdapter, so the import failed and all 4 tests in the
module self-skipped via the _IMPORT_ERROR guard. Add the missing stub so the
tests actually execute.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: drop prompt-service from test compose overlay

Removed from the e2e test overlay; the platform brought up for e2e no longer
provisions a standalone prompt-service.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: remove dead S3 smoke test and strip print() debug from connector tests

- test_miniofs: drop the permanently-skipped test_s3 (hardcoded AWS S3 smoke).
  MinIO/S3 is covered by test_minio (integration), the TestAccessFilteredS3
  unit tests, and the connectorkit registry check.
- database + filesystem tests: replace print()-in-loop debug with assertions
  (or drop redundant prints) so integration runs don't spam the logs.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: switch unit-backend to marker-based selection; classify integration tests

unit-backend was a hand-kept file allowlist that had to grow with every new
test dir. Collect the whole backend tree instead and let markers decide:
tests needing live infra carry `@pytest.mark.integration` and are excluded
from the unit tier via `-m "not integration"`.

- register the `integration` marker in backend/pyproject.toml
- mark the two DB-bound suites (dashboard_metrics, prompt_studio_registry_v2)
  that were previously grouped by path only
- add a conftest marking the endpoint_v2 destination-connector subtree
  integration (uses django.test.TestCase -> needs Postgres). Kept out of the
  gating integration-backend group for now: 3 postgres destination tests are
  pre-existing failures and need a skip-guard/fix before they can gate.
- remove the dead SharePoint `test_json_schema_has_is_personal` skip: the
  schema never exposes `is_personal`, so the test only ever asserted-then-
  skipped; drop it rather than carry a permanent skip.

Verified: unit tier 116 passed / 50 deselected; integration-backend collects
its marked suites; rig self-tests 54 passed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C5HQX5CSoMR6RzHtXcfwJt

* test: centralize DB-test marking; cover adapter-register-llm via integration API test

Replace the scattered integration-marking (per-file pytestmark, per-app
endpoint_v2 conftest) with a single backend/conftest.py hook that auto-marks
any Django TestCase/TransactionTestCase or django_db item as integration —
tests declare their DB need by how they're written, not a hand-kept marker.

Cover the adapter-register-llm critical path honestly: unit-sdk1 only
exercises the SDK provider classes, not the HTTP endpoint, so map it to a new
integration-backend APITestCase that POSTs /adapter/ (SDK context-window call
mocked, everything else real). Trim comments that would go stale.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: complete DB-test centralization; move DB-writer tests to integration tier

Follow-up completing 6d61a566, which staged only the adapter API test and the
deleted per-dir conftest, leaving the rest of the batch uncommitted.

- backend/conftest.py: central pytest_collection_modifyitems hook auto-marks
  every Django TestCase/TransactionTestCase/django_db test as `integration`, so
  unit-backend (-m "not integration") and integration-backend (-m integration)
  are exact complements. Drops now-redundant per-app pytestmark in
  dashboard_metrics and prompt_studio_registry_v2.
- critical_paths.yaml: adapter-register-llm now covered_by integration-backend
  (real API test), reverting the earlier unit-sdk1 placeholder mapping.
- groups.yaml: integration-backend gains the adapter API test and the
  destination-connectors DB-writer tests (BE orchestration over the connector
  lib — superset of the connector-lib DB tests). Adds WORKFLOW_EXECUTION_DIR_PREFIX
  to the shared backend test env (ExecutionFileHandler builds paths from it).
- destination-connector postgres test: read DB_SCHEMA (was hardcoded "test"),
  use a lowercase table name (connector lowercases on read-back), drop the
  error-record case (hits a latent product edge: data=None serialized as the
  string 'None' into a jsonb column).
- Delete 7 connector-lib DB tests (databases/test_*_db.py) — superseded by the
  backend DB-writer tests; keep test_sql_safety.py, filesystems, connectorkit.
- rig cli.py / critical_paths.py: comment + docstring cleanups.

Verified (testcontainers Postgres/Redis): unit-backend 116 passed,
integration-backend 24 passed / 26 skipped (external-DB engines skip w/o creds),
unit-connectors 53 passed, rig validate OK (15 groups, 9 paths).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test: switch integration-backend to marker-only selection; drop dead connector_v2 tests

- groups.yaml: integration-backend now selects paths ["."] with -m integration
  — the exact complement of unit-backend over the same tree. New backend DB
  tests auto-join the group with no manifest edit. Verified equivalent:
  collect-only shows the same 50/166 tests (116 deselected); full run against
  testcontainers Postgres/Redis gives the same 24 passed / 26 skipped as the
  hand-listed paths on CI.
- Delete backend/connector_v2/tests (connector_tests.py, conftest.py, fixture):
  written for the pre-v2 schema — the fixture loads removed models
  (account.org, account.user, project) so loaddata errors immediately, and the
  filename never matched pytest's test_*.py pattern, so these tests have not
  been collected anywhere. Same dead-test cleanup as the rest of this branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU

* docs: concise backend test-contribution steps in tests/README.md

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU

* test: rewrite deployment_helper staging tests with patch.object

The previous version stubbed cross-app imports in sys.modules (only when
absent) — written for a bare no-Django env. Under the rig, pytest-django
boots Django and the real modules are already imported, so the stubs were
skipped and the tests called reset_mock() on real classes (AttributeError).

Same control-flow assertions, now via mock.patch.multiple on the imported
module: no sys.modules mutation, no import-order dependence, safe in a
shared pytest session. Stays in the unit tier (no DB touched).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU

* test: drop sys.modules stub ceremony from build-index tests; subprocess flask check

- test_bui…
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