[athul-rs]

What

• Plugin manifest, slots and registry for Frontend v1 (P0-T07): a plugin declares the routes and slot fillers it adds in plugins/<id>/manifest.ts; shell and pages render <Slot id="…"> from @v1/lib/plugins.
• plugins/index.ts discovers manifests with import.meta.glob, keeps those for the current VITE_DISTRIBUTION, and builds the registry. PluginProvider hands the slot renderer to lib/ through context, so lib/ keeps importing only lib/.
• A build check (scripts/v1-plugins.mjs) fails the build when two plugins fill a single-filler slot or claim the same route.
• .gitignore keeps overlaid cloud plugins out of OSS commits; the frontend Docker build passes VITE_DISTRIBUTION through.

Why

• P0-T07 (UN-4286): cloud-only v1 features (plan and billing, product switcher, API Hub) plug into shared v1 screens without OSS code naming them.
• Stacked on the v1 scaffold (UN-4270 [FEAT] v1 scaffold, strict TypeScript and import boundary #2380); it follows the scaffold's folder rules and lint.

How

• A slot with no filler renders its fallback, so OSS shows nothing where a cloud plugin would sit. A filler that throws is caught per slot and reported, and the rest of the page renders.
• skipLibCheck in src/v1/tsconfig.json now comes from the scaffold (UN-4270 [FEAT] v1 scaffold, strict TypeScript and import boundary #2380, 664d1cf), which settled the @types/chai / vitest containSubset clash. This branch merged the scaffold and takes its tsconfig, so the earlier temporary commit no longer changes anything. The scaffold's new rule against dynamic imports it cannot analyse is met by missing-import.test.ts (a template whose fixed prefix names the folder).

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

• No. Everything is under frontend/src/v1/, which no v0 route renders. Outside it: a .gitignore rule for frontend/src/v1/plugins/*, a VITE_DISTRIBUTION build arg (defaults to OSS behaviour when unset) and a Vite plugin that only inspects src/v1/plugins.

Database Migrations

• None.

Env Config

• VITE_DISTRIBUTION (build time, optional). Unset means OSS.

Relevant Docs

• frontend/src/v1/README.md, section "Plugins".

Related Issues or PRs

• UN-4286 (P0-T07), epic UN-4257. Base: UN-4270 [FEAT] v1 scaffold, strict TypeScript and import boundary #2380 (UN-4270 scaffold). Cloud overlay: Zipstack/unstract-cloud#1896 (UN-4285). Next in the stack: UN-4290 [FEAT] Add the v1 account menu and organisation switcher #2395 (UN-4290).

Dependencies Versions

• None.

Notes on Testing

• bun run typecheck:v1, lint:v1, test:v1: clean; 20 vitest tests and 50 node tests (the scaffold's fixtures plus this PR's build-check and CLI tests).
• The cloud overlay tests (unstract-cloud#1896) pass 7/7 against this branch. The check runs under Bun with no dependencies installed, as the cloud CI overlay step needs.
• Frontend CI (typecheck, vitest, biome) runs once this PR targets main, i.e. after UN-4270 [FEAT] v1 scaffold, strict TypeScript and import boundary #2380 merges. The commands above were run locally.
• The fixture tests cover the build check: duplicate route, two fillers on a single slot, a manifest for another distribution.

Screenshots

None: no UI.

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

[greptile-apps]

via Greptile

Confidence Score: 5/5

[Medium impact] The reviewed changes appear safe to merge within the documented scope.

Summary

Adds plugin manifests, distribution filtering, slot rendering, and a build-time conflict check.

• V1 slots keep optional plugin content separate from the rest of the page.
• The v1 registry exposes routes and slot fillers from distribution-specific plugin manifests.
• Vite builds stop when a plugin manifest conflicts with the v1 app.

_{Reviews (12) · Last reviewed commit: "UN-4286 [FIX] Address review: check mani..." · Reviewed by Greptile}

[ritwik-g]

Thanks, the contract is clean and the cloud overlay (unstract-cloud#1896) fits it: check(dir), the matrix fields, the distribution names and the folder = id rule all match. I also merged this branch into current main locally: the only conflict is the test:v1 line in frontend/package.json (keep both unit-gate.fixtures.mjs and scripts/v1-plugins.fixtures.mjs), and main's new v1 checks (lint with warnings as errors, typecheck, separation, bundle, seam) and all 177 node fixtures pass on the merged tree.

Requesting changes for two spec rules from P0-T07 that are not met yet (OSS route clashes, reporting through the notification contract), a retry loop on failed chunk loads, the dev image never getting VITE_DISTRIBUTION, and untested distribution logic. Details inline.

Not tied to a line

• Stale references to the removed plugins/manifest.ts: the coverage comment in frontend/vitest.v1.config.mts:42-44 still calls it "OSS's placeholder plugins/manifest.ts" (it is now the loader index.ts), and main's frontend/test-fixtures/v1-boundary/git-case.sh:20 still writes the old !frontend/src/v1/plugins/manifest.ts ignore rule. Worth updating both when resolving the conflict with main. (Fixtures still pass.)
• Acceptance "An OSS build renders an OSS page with empty slots; a cloud build fills them" can't be shown yet since no v1 page or provider exists. Could the Jira ticket say which task carries it?

[athul-rs]

Review body: b3a667a updates the coverage comment in vitest.v1.config.mts and the ignore rule in test-fixtures/v1-boundary/git-case.sh (!frontend/src/v1/plugins/index.ts). The "OSS page with empty slots, cloud build fills them" acceptance can be shown once P2-T01 mounts the shell with PluginProvider and P2-T13's product switcher fills shell.product-switcher. That belongs with P2-T01, which wires PluginProvider.

[ritwik-g]

Re-reviewed b3a667a: 7 of the 9 threads are resolved, and the new guards are covered by tests. Two are partly done (route matching should use patternsOverlap, and README.md:188 still contradicts the matrix sentence), and there is one new issue inline on Slot.tsx:119.

[vishnuszipstack]

Review of PR 2394 — UN-4286 Add the v1 plugin manifest, slots and build-time plugin check

Mode: INITIAL, at the PR's current head. Reviewed head 5cc28d787200f53196f14c238ef894310dbffb39, merge base ed416ff66d99d5bda904d1036a37a2482657361d, base freshened (base_fetch: ok), 23 files, 1460 lines (matches GitHub). Rubric: unstract plugin v2.1.80.
How this was done: a full fan-out (code reviewer, silent-failure hunter, type design, comments, lifecycle and a whole-diff cold read) ran at an earlier head c015c2537. Before posting, every finding was re-checked against this head. Findings already fixed, already raised by another reviewer, or wrong on a second read were dropped (17 dropped: 10 already raised by ritwik-g and fixed in b3a667a or 5cc28d7, 2 more fixed, 5 withdrawn). The author's own commits since c015c2537 were reviewed for new defects, and the tests were re-run here.
Security deep-dive: degraded — the vendored security prompt ran once at this head, its filter step inline instead of as independent passes; no findings at confidence >= 8.

Verdict — APPROVE WITH NITS

Summary — Critical: 0 · High: 0 · Medium: 1 · Low: 8 · Lenses run: 19/19 (2 not covered, 4 N/A; see checklist).

The review rounds' fixes hold, including the overlap matching and per-error reporting in 5cc28d7. One Medium is left: nothing checks the shape of a cloud manifest. The root tsconfig.json excludes src/v1, the build check strips types, and assertDeclared looks only at ids, distributions and slot keys. So a cloud manifest with an undefined slot, a non-function load or a route without path passes both repos and fails only at render or routing time. One Low is new in 5cc28d7 (plugin route paths skip patternProblem). The rest are comment accuracy, an undeclared esbuild, a hand-copied distribution list, and fixture hygiene. Tests at this head, from frontend/: the changed vitest files pass (4 files, 31/31), the node fixtures pass (183/183), and so does the Bun branch of the build check (21/21). typecheck:v1, lint:v1 and biome ci on the 17 changed files all exit 0; biome's 4 warnings and 3 infos are in vite.config.js, and only one of those lines is new here. The full test:v1, vite build and real-browser chunk retries were not run.

Unanchored findings

• PR description is out of date (Low). The description says the scaffold's dynamic-import rule "is met by missing-import.test.ts", but 365c924 replaced that file with the resolver fixture in boundary.fixtures.mjs. It also still describes an "earlier temporary commit" for skipLibCheck, and gives "20 vitest tests and 50 node tests" from before the review rounds. It says the fixture tests cover a "duplicate route", but the plugin-vs-plugin route case is tested only in registry.test.ts; the fixtures cover plugin-vs-OSS routes. Please refresh the Testing notes against the current head.

Findings

Posted inline on the diff; index:

• 1. [Medium] [Lens 7] — Cloud manifests are never type-checked, and the registry does not check their shape — frontend/src/v1/lib/plugins/registry.ts:32
• 2. [Low] [Lens 3] — Plugin route paths skip patternProblem, so the overlap check can miss clashes — frontend/src/v1/lib/plugins/registry.ts:87
• 3. [Low] [Lens 10] — Manifest load errors lose their cause and always blame import type — frontend/scripts/v1-plugins.mjs:65
• 4. [Low] [Lens 16] — The contract comments say pages never import a plugin; the boundary check allows it — frontend/src/v1/lib/plugins/index.tsx:2
• 5. [Low] [Lens 16] — plugins/index.ts header still says every import error is reported — frontend/src/v1/plugins/index.ts:2
• 6. [Low] [Lens 16] — jest-dom.d.ts names v0's setup file — frontend/src/v1/test/jest-dom.d.ts:1
• 7. [Low] [Lens 2] — The distribution list is copied by hand, though the route registry already exports it — frontend/src/v1/plugins/index.ts:22
• 8. [Low] [Lens 14] — esbuild is used at build time but not declared — frontend/scripts/v1-plugins.mjs:35
• 9. [Low] [Lens 13] — Fixture uses a fixed shared tmp path and the bare CLI run reads the real plugins folder — frontend/scripts/v1-plugins.fixtures.mjs:62

Lens checklist

1. Spec & intent — Clean
2. Architectural fit & precedent — See findings #7
3. Correctness & edge cases — See findings #2
4. Security — Not covered — the security deep-dive ran degraded (single pass, filters inline) and found nothing at confidence >= 8, which is not a clean bill
5. Data integrity & migrations — N/A — no persisted data or schema
6. Concurrency — Clean
7. API & contract compatibility — See findings #1
8. Reliability & resilience — Clean
9. Performance & cost — Clean
10. Observability — See findings #3
11. Operational safety — Clean
12. LLM/agent-specific — N/A — no model, prompt or agent path
13. Testing — See findings #9
14. Dependencies & build — See findings #8
15. Code quality — Clean
16. Doc & comment accuracy — See findings #4, #5, #6
17. Cross-document rule consistency — Not covered — merge-result sweep unavailable: git 2.34 here, the script needs 2.38
18. On-prem deployment experience — N/A — no chart, values or on-prem build path touched
19. Scaling & workload lifecycle — N/A — no worker or scaling config

[vishnuszipstack]

1. [Medium] [Lens 7] — Cloud manifests are never type-checked, and the registry does not check their shape

• Location: frontend/src/v1/lib/plugins/registry.ts:L32-L55
• Only satisfies PluginManifest under tsc enforces the shape of a manifest: each slot value and route load is a loader function, and each route has a string path. The build check strips types (esbuild transform under Node, a direct import under Bun). The root frontend/tsconfig.json excludes src/v1, so cloud CI's bun run typecheck never sees the overlaid manifests. In OSS, typecheck:v1 covers src/v1/plugins, but that folder is empty here. assertDeclared checks ids, distributions and slot keys only. A cloud manifest with slots: { "shell.product-switcher": undefined }, a non-function load, or a route without path therefore passes both repos. It fails only at render or routing time. An explicit undefined slot also claims the slot in collect, so a real filler for it fails as a conflict. Either have the overlay run tsc -p src/v1/tsconfig.json after copying and say so in the README, or add typeof load === "function" and typeof path === "string" checks to assertDeclared.
• Verification: VERIFIED — read at this head: frontend/tsconfig.json excludes src/v1, and assertDeclared throws only on ids, distributions and slot keys

[athul-rs]

Fixed in 736790646. The registry now rejects a slot value that is not a function (an explicit undefined included), a route load that is not a function and a route without a string path, naming the plugin and the slot or route; tests cover each case in registry.test.ts and in the build-check fixtures.

[vishnuszipstack]

2. [Low] [Lens 3] — Plugin route paths skip patternProblem, so the overlap check can miss clashes

• Location: frontend/src/v1/lib/plugins/registry.ts:L87-L98
• patternsOverlap is exact only for patterns that pass patternProblem: absolute paths, static segments, :params and a final *. validate.ts runs that check on every page route. Plugin routes go straight into overlaps(path, route.path) with no check. A plugin route /:orgName/v1-demo/:id? is compared segment by segment. Its :id? meets the end of /:orgName/v1-demo, patternsOverlap returns false, and the build passes. React Router still matches /acme/v1-demo for both routes, so the plugin silently shadows the OSS page or is shadowed by it. A path without a leading / or with a mid-path * also gets in. The README now promises the check catches "two routes the router would match for the same URL". Reject a plugin route whose path fails the registry's pattern rule. Pass the validator in next to overlaps (lib/ cannot import app/), and add a fixture for :id?.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. buildRegistry now takes { ossRoutes, overlaps, patternProblem }, both callers pass the route registry's patternProblem, and a plugin route it refuses fails the build; fixtures cover /:orgName/v1-demo/:id? and a path without a leading /.

[vishnuszipstack]

3. [Low] [Lens 10] — Manifest load errors lose their cause and always blame import type

• Location: frontend/scripts/v1-plugins.mjs:L65-L72
• The catch in load() throws a new Error that keeps only err.message. The stack, esbuild's location frames and the cause are dropped, and the CLI handler (L148-L151) prints only err.message again. Every failure also gets the hint "A manifest may only import type": a syntax error, a TypeError thrown while the manifest evaluates, or esbuild failing to import under Node. A developer with a typo on line 40 is steered toward import rules. Pass { cause: err }, and add the hint only for module-resolution failures (ERR_MODULE_NOT_FOUND, "Cannot find module", esbuild's "Could not resolve"). In the CLI catch, print err.stack or err.cause as well.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. The wrapped error keeps the original as cause, the import type hint appears only for module-resolution failures, and the CLI prints the cause with its stack; a syntax-error fixture checks that it gets no hint.

[vishnuszipstack]

4. [Low] [Lens 16] — The contract comments say pages never import a plugin; the boundary check allows it

• Location: frontend/src/v1/lib/plugins/index.tsx:L2-L4
• This header and the README Plugins paragraph (README.md:L181) say pages "never import a plugin" and call @v1/plugins "the only folder that may see plugin manifests". CONTRACT.pages in scripts/check-v1.mjs lists "plugins", and the README import table lets pages/ import plugins/. app, entry and test may import everything. So neither sentence is enforced. A page that reaches into plugins/<id>/ passes lint:v1, and a reader who trusts the comment assumes it would be caught. The README folder table (README.md:176, outside this diff) also still says plugins/ holds "only a placeholder", though it now ships the loader index.ts. Either drop plugins from CONTRACT.pages and the table so the prose becomes true, or remove the two claims and update the table row.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. Both claims are removed (the lib/plugins/index.tsx header and the README Plugins paragraph), and the README row for plugins/ now says it holds the loader index.ts.

[vishnuszipstack]

5. [Low] [Lens 16] — plugins/index.ts header still says every import error is reported

• Location: frontend/src/v1/plugins/index.ts:L2-L5
• The header says "every import error left is a real failure and is reported". Manifests load through an eager import.meta.glob, so a broken manifest import fails the build or module evaluation and never reaches reportPluginError. Only filler load and render errors caught by SlotBoundary are reported. Narrow it to "a filler that fails to load or render is reported", or drop the clause. The makeSlot docstring already describes this correctly.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. The header now says a filler that fails to load or render is reported.

[vishnuszipstack]

6. [Low] [Lens 16] — jest-dom.d.ts names v0's setup file

• Location: frontend/src/v1/test/jest-dom.d.ts:L1
• The comment says src/setupTests.js registers the matchers at run time. That is v0's setup file (vitest.config.mjs). v1 tests run src/v1/test/setup.ts (the setupFiles in vitest.v1.config.mts), which imports @testing-library/jest-dom/vitest. Point the comment at src/v1/test/setup.ts.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. The comment now points at src/v1/test/setup.ts.

[vishnuszipstack]

7. [Low] [Lens 2] — The distribution list is copied by hand, though the route registry already exports it

• Location: frontend/src/v1/plugins/index.ts:L22
• DISTRIBUTIONS here is typed readonly Distribution[], so it still type-checks if a distribution is added to PLUGIN_DISTRIBUTIONS and missed here. The same list is written out again in vite.config.js (with a "mirrors" comment) and in the matrix keys in scripts/v1-plugins.mjs:L112-L116. If a new distribution is missed in one copy, VITE_DISTRIBUTION=<new> builds but then throws at module load, or the matrix leaves out a key the cloud overlay indexes (matrix.distributions[distribution]). app/registry/types.ts already exports DISTRIBUTIONS as a const tuple. Import it here, and build the matrix keys from ["oss", ...PLUGIN_DISTRIBUTIONS].
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. plugins/index.ts and the matrix keys in scripts/v1-plugins.mjs now come from DISTRIBUTIONS in app/registry/types.ts. vite.config.js keeps its own list because the config loads synchronously, and a new fixture checks that it accepts every distribution the matrix lists.

[vishnuszipstack]

8. [Low] [Lens 14] — esbuild is used at build time but not declared

• Location: frontend/scripts/v1-plugins.mjs:L35-L45
• Under Node, vite build runs this check, which imports esbuild. Since 5cc28d7 the check also uses esbuild's build (bundling) API, not just transform. esbuild is not in frontend/package.json. It resolves only because it is hoisted as Vite's dependency. An isolated or non-hoisted install would fail every build in buildStart. Add esbuild to devDependencies, pinned to Vite's range.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. esbuild is a devDependency at Vite's range (^0.27.0), with a one-line bun.lock change.

[vishnuszipstack]

9. [Low] [Lens 13] — Fixture uses a fixed shared tmp path and the bare CLI run reads the real plugins folder

• Location: frontend/scripts/v1-plugins.fixtures.mjs:L62
• The "no plugins directory" case passes os.tmpdir()/v1-plugins-none. That path stays absent only by convention: if a leftover or a concurrent run creates it, the test fails. None of the mkdtemp directories the other cases create is removed. The bare CLI case (L243-L248) runs without --dir, so it checks the repo's own src/v1/plugins. In a cloud-overlaid checkout, any broken overlaid manifest fails this OSS fixture. Use a fresh mkdtemp path, removed afterwards, for the "none" case, and clean up the temp dirs. Point the bare run at a fixture through an env override or cwd.
• Verification: VERIFIED — read at this head

[athul-rs]

Fixed in 736790646. Every fixture directory is a fresh mkdtemp removed after the run, including the "no plugins directory" case. The bare CLI run reads a fixture through a new V1_PLUGINS_DIR override instead of the repository's src/v1/plugins.

[github-actions]

Frontend Lint Report (Biome)

✅ All checks passed! No linting or formatting issues found.

[sonarqubecloud]

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Errors	Skipped	Duration (s)
⚪	e2e-agent-kv	e2e	0	0	0	1	0.1
✅	e2e-api-deployment	e2e	3	0	0	0	18.1
✅	e2e-coowners	e2e	1	0	0	0	1.1
✅	e2e-etl	e2e	1	0	0	0	10.1
✅	e2e-login	e2e	2	0	0	0	1.0
✅	e2e-prompt-studio	e2e	1	0	0	0	10.6
✅	e2e-smoke	e2e	2	0	0	0	0.8
✅	e2e-workflow	e2e	1	0	0	0	18.0
✅	frontend	unit	673	0	0	0	13.1
✅	frontend-v1-unit	unit	698	0	0	1	20.5
✅	integration-backend	integration	1823	0	0	26	79.5
✅	integration-connectors	integration	1	0	0	7	7.3
✅	integration-workers	integration	181	0	0	1	44.8
❌	ui	e2e	0	1	0	0	0.0
✅	unit-agent-kv-schema	unit	143	0	0	0	1.1
✅	unit-backend	unit	2191	0	0	1	41.4
✅	unit-connectors	unit	119	0	0	0	7.5
✅	unit-core	unit	289	0	0	0	2.0
✅	unit-filesystem	unit	1	0	0	0	2.7
✅	unit-platform-service	unit	22	0	0	0	1.8
✅	unit-rig	unit	139	0	0	0	3.5
✅	unit-runner	unit	12	0	0	0	1.9
✅	unit-sdk1	unit	1358	0	0	0	29.5
✅	unit-tool-sidecar	unit	8	0	0	0	1.1
✅	unit-workers	unit	1675	0	0	2	105.8
✅	unit-x2text-service	unit	116	0	0	0	2.5
	TOTAL		9460	1	0	39	425.8

Critical paths

⚠️ Critical paths not yet covered

• workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)

✅ Covered critical paths

• auth-login — covered by e2e-login
• adapter-register-llm — covered by integration-backend
• workflow-author — covered by integration-backend
• co-owner-manage — covered by integration-backend, e2e-coowners
• workflow-create-execute — covered by e2e-workflow
• api-deployment-provision — covered by integration-backend
• api-deployment-auth — covered by integration-backend
• api-deployment-run — covered by e2e-api-deployment
• mcp-server-auth — covered by integration-backend
• mcp-platform-auth — covered by integration-backend
• platform-key-whoami — covered by integration-backend
• prompt-studio-author — covered by integration-backend
• prompt-studio-fetch-response — covered by e2e-prompt-studio
• connector-register-test — covered by integration-backend
• pipeline-etl-execute — covered by e2e-etl
• usage-aggregate-read — covered by integration-backend
• usage-token-tracking — covered by e2e-api-deployment
• callback-result-delivery — covered by e2e-api-deployment

[ritwik-g]

All threads resolved, and each fix is covered by a test. Approving. Thanks, Athul.