feat(ui): port the dev-platform operator SPA out of core web-ui (epic byte5ai/omadia#470 P2) - #3
Merged
Merged
Conversation
…yte5ai/omadia#470 P2) Twenty-eight source files that lived in `web-ui/app/admin/dev-platform/**` and `web-ui/app/_components/devjobs/**` now build as a standalone Vite/React 19 bundle in `packages/ui`, ship inside the plugin ZIP as `ui/`, and are served by core at `/p/<pluginId>/ui/` behind web-ui's `/plugin-ui/<pluginId>` host page. `web-ui` is untouched. Four screens: hub, job detail, repo detail, add-repo wizard. What replaced what - `next-intl` -> `src/lib/i18n.tsx`. 300 keys per locale, plain `{name}` interpolation, no ICU parser. The three ICU plurals were de-sugared to `{ one, other }` at extraction; en and de share the `n === 1` rule. - `next/link`, `next/navigation` -> `src/lib/router.tsx`, a hash router. The static route serves only the bundle root and real files, so a client route in the PATH would 404 on reload; a fragment never reaches the server. It also avoids baking a plugin id into the build, since the id comes from the install. - core's 4,827-line `app/_lib/api.ts` -> `src/lib/apiError.ts`. Exactly one name was imported from it. - `framer-motion` and `lucide-react` -> dropped. Both animate or size with classes the served vocabulary does not contain. Tailwind, constrained to what core actually serves The pages carried 334 arbitrary values (`text-[color:var(--fg-muted)]` and friends); the ZIP allowlist rejects that shape and a class core never saw renders unstyled rather than erroring. All 334 are gone. `Button` and `ConfirmDialog` were rewritten rather than ported for the same reason — core expresses every Button variant as an arbitrary value. `scripts/check-ui-vocabulary.mjs` gates the build with three checks: no emitted stylesheet, core's two ingest regexes verbatim, and a whitelist diff against `vocabulary/classes.txt` — 690 classes extracted from the generated stylesheet itself, not transcribed from the spec table. The diff is the half core has no counterpart for: ingest cannot see `bg-blue-500`, which is not an arbitrary value, merely a class that does not exist. Packaging and nav `ui/` joins `dist` and `migrations` in build-zip's REQUIRED_DIRS, with assertions that `ui/index.html` exists and that no stylesheet is present. The nav entry moves from `/admin/dev-platform` — a page core deletes in this epic — to `/plugin-ui/<id>`, percent-encoded because this plugin's id is scoped. Tests: 50, covering the four screens against fixtures in both locales, the theme attribute crossing the iframe boundary, the router, the i18n runtime, and the vocabulary gate (fixtures for `w-[137px]`, `[&>tr]:`, `bg-blue-500`). Two mutations were run against the suite and both were killed. Three core defects found and NOT fixable here — see docs/iframe-credentials.md 1. The host page's `sandbox` omits `allow-same-origin`, so every authenticated call from the frame is cross-origin with `Origin: null`. All four screens are data-driven; none can load data in a real browser until this is decided. 2. That page's plugin-id regex rejects scoped ids, though `manifestLoader` blesses them and this plugin's id is `@omadia/dev-platform`. 3. `plugin-ui.source.css` lines 341/346/353 use a top-level comma where `@source inline()` expands only braces, so `border`, `divide-*` and `transition*` emit NOTHING. With the base reset at `border: 0 solid`, `class="border border-border"` renders invisible — the exact silent failure the contract exists to prevent. `src/lib/cx.ts` works around it with the four directional utilities and a test pins the broken state so the workaround cannot rot.
Closed
7 tasks
Weegy
added a commit
to byte5ai/omadia
that referenced
this pull request
Aug 20, 2026
…me, scoped ids, emitted CSS (C8b) (#793) * fix(#470): make the C8 plugin-UI host actually work (C8b) Three defects found while porting the first real plugin SPA (byte5ai/omadia-dev-platform#3). Each failed silently rather than loudly, which is why C8 shipped green. 1. The iframe sandbox made every authenticated call cross-origin. `allow-same-origin` was withheld deliberately, so the document got an opaque origin: `fetch('/bot-api/...')` left with `Origin: null`, the `SameSite=Lax` session cookie was never attached, `EventSource` failed the same way and `localStorage` threw. A data-driven plugin UI opens every screen with a GET, so the host rendered a correctly-styled, correctly- themed shell showing an error state on all four screens. Neither repo's suite caught it: both stub `fetch`, and a stub has no origin. Decision: sandboxed but SAME-ORIGIN — `allow-same-origin allow-scripts allow-forms`. The sandbox bought no privilege it did not already have, as the plugin's server half runs in-process with the operator's authority; what confines the bundle is the response, not the attribute, and that CSP (`default-src 'none'`, `script-src 'self'`, `connect-src 'self'`, `frame-ancestors 'self'`, `base-uri 'none'`, `form-action 'none'`) is unchanged. `allow-popups` is dropped — nothing uses it. Rejected: a postMessage/CORS proxy (`Access-Control-Allow-Origin: null` matches every opaque origin, and the alternative loosens the session cookie to `SameSite=None` product-wide) and a separate origin (the clean answer, recorded as the upgrade path; needs a second hostname and TLS on every install target). Reasoning in plan.md §4.3a addendum. 2. The host page rejected every scoped plugin id. Its regex claimed to mirror `manifestLoader.ts` and omitted the optional `@scope/`, so every `@omadia/*` id — which is all of them — hit `notFound()`. The gate now lives in `web-ui/app/_lib/pluginId.ts` and a test reads `manifestLoader.ts` and asserts both the pattern and the 214-char cap are character-identical, turning the "mirrors" comment into a check. Traversal safety is asserted, not assumed. 3. `border`, `divide-*` and `transition*` emitted no CSS at all. `@source inline()` expands BRACES; a top-level comma is not a list separator, so `@source inline("border,border-{0,2,4}")` asked Tailwind for a class literally named `border,border-0` and produced nothing — while `plugin-ui-vocabulary.md` listed all three groups as available. Worse than a missing utility: Tailwind's reset is `border: 0 solid`, so `class="border border-border"` set a colour on a zero-width edge and rendered invisible. Fixed at source, artifact regenerated (69,559 → 72,659 B raw, 12,105 → 12,486 B gzip). `hover:`/`focus:` on `bg-transparent` were promised by the doc and not emitted either; the source line now declares them. Tests, all of which fail against the parent commit: - `pluginUiVocabulary.test.ts` checks parity in both directions — every class the source declares and every class the document promises must exist in the committed sheet. It refuses un-expandable notation rather than skipping it, so a promise cannot go unchecked; the doc's `xs…7xl` ellipsis is written out and the Responsive prose became a table for the same reason. - `PluginUiFrame.test.tsx` pins the sandbox attribute to its exact string and the src to core's own origin. - `pluginUiFrameCredentials.test.ts` proves the posture the decision rests on: the session cookie is `SameSite=Lax; Path=/; HttpOnly` and not `SameSite=None`; a same-origin request replaying it authenticates while the same request without it 401s. Core-decoupling ratchet held at 3300 — the tests use a neutral `@omadia/ example-ui` id rather than naming the package being extracted. * fix(#470): correct the C8b iframe trust-model rationale and put its premises under test The sandbox change in this branch is right and stays. Its stated reasoning was not, and a wrong rationale next to a security attribute is the exact defect class this PR exists to fix. Three corrections, all narrative plus tests — no behaviour change: 1. "It gives the plugin no privilege it does not already hold" was false. The server-side plugin contract is genuinely deny-by-default: pluginServiceGrants throws ServiceNotDeclaredError for undeclared services, pluginContext gates ctx.http / ctx.net / ctx.secrets / ctx.memory / ctx.llm / ctx.subAgent / ctx.knowledgeGraph / ctx.mcp / ctx.events.emit / ctx.flows on manifest permissions, operatorAuthAccessor exposes a verifier rather than a bearer, and CORE_RESERVED_ROOTS refuses /api/v1/admin to a plugin even on operator consent. A same-origin bundle riding the Path=/ admin session reaches all of it, and no CSRF layer exists to stop it. The decision is still correct, for a narrower reason now written down: plugin server code is loaded by a bare in-process dynamic import with no vm, worker, child process or permission wall, sharing globalThis and a process.env that holds the session signing key. Against a malicious plugin author the grant model was never a security boundary — it is a consent-and-contract seam — so withholding same-origin from the UI defended nothing while breaking every honest plugin. The residual cost is a different attacker: a compromised browser-only dependency or an XSS in the plugin UI, which now inherits the operator's admin surface. §4.3a says so plainly instead of claiming the trade is free. 2. "The sandbox is retained for what it still denies" was unenforceable. With allow-same-origin plus allow-scripts a bundle reaches window.frameElement, strips the attribute and reloads, regaining top-level navigation, downloads and modals. The attribute is an intent marker, not isolation, and the test that pins those tokens now says which of the two it is. 3. "ingest-scanned ZIP" overstated the scan. The only ingest pass over a UI bundle reads .js / .mjs for arbitrary Tailwind values, never opens ui/index.html, is bounded at 200 files / 8 MB, and does not run at all for built-in or local-dev catalog packages. Inline script, javascript: URLs and <base> are blocked by the runtime CSP, not by ingest. New middleware/test/pluginUiTrustModel.test.ts is the tripwire for the premise itself: it pins the bare in-process plugin load, keeps /api/v1/admin core-reserved with the accepted exposure recorded next to it, exercises the real ServiceNotDeclaredError gate, and pins the session JWT's role: 'admin' plus Path=/. Each failure message names the decision to reopen rather than the line that changed. A documented trade with no test is just a comment. pluginUiStaticServing.test.ts now table-drives every CONTENT_TYPES entry — Content-Type, nosniff, a non-empty CSP, the SVG sandboxing policy versus the standard one for the other ten extensions — plus the 304 branch. CSP was already branch-free in the handler but asserted on only three of eleven servable types. CONTENT_TYPES is exported so the table cannot drift from what it claims to cover. Verified: middleware plugin/auth/manifest surface 438/438; web-ui 778/778 across 91 files; web-ui typecheck, build and lint (0 errors, 47 pre-existing warnings); plugin-ui css drift up to date at 72,659 bytes; typecheck:test ratchet held at 406; core-decoupling ratchet held at 3300. Mutation-checked: removing /api/v1/admin from CORE_RESERVED_ROOTS, replacing the dynamic import with a vm-based load, and flipping role: 'admin' each redden one tripwire; deleting the CSP res.set reddens 15 of the table-driven cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Ports the Dev Platform's four operator screens out of omadia core's
web-uiinto a standalone Vite/React 19 SPA inpackages/ui. The bundle builds intopackages/plugin/ui/, ships inside the plugin ZIP, and is served by core at/p/<pluginId>/ui/behind the/plugin-ui/<pluginId>host page that C8 added.web-uiis untouched. This is the epic's biggest single risk, isolated in its own repo and its own package.#/#/?tab=#/jobs/<id>#/repos/<id>#/repos/newWhat replaced what
next-intlsrc/lib/i18n.tsx{name}interpolation, no ICU parser.next/link,next/navigationsrc/lib/router.tsx@/app/_lib/api(4,827 lines)src/lib/apiError.tsApiError.framer-motion,lucide-reactButton,ConfirmDialogbg-[color:var(--accent)]). Ingest rejects that shape;bg-accentresolves to the same variable.DevJobChatCardis deliberately not ported — it renders inside core's chat transcript, andplan.md§4.3 excludes the chat surface (H3, still undecided). The one five-line functionJobDetailScreenneeded from its state module is insrc/lib/gates.ts.Tailwind, constrained to what core actually serves
The ported pages carried 334 arbitrary values. All are gone. A class core never saw does not error — it renders unstyled, on the operator's screen and nowhere else — so
scripts/check-ui-vocabulary.mjsgates the build with three checks:.cssis absent from the ZIP allowlist and the static router's Content-Type table; that absence is the enforcement.vocabulary/classes.txt— 690 classes extracted from the generated stylesheet, not transcribed from the spec table. This is the half core has no counterpart for: ingest cannot seebg-blue-500, which is not an arbitrary value, merely a class that does not exist.Packaging and nav
ui/joinsdistandmigrationsin build-zip'sREQUIRED_DIRS, with assertions thatui/index.htmlexists and no stylesheet is present. Optional would mean "a build that forgotvite buildships silently, installs, activates, adds a nav entry, and 404s when the operator clicks it".The nav entry moves from
/admin/dev-platform— a page core deletes in this epic — to/plugin-ui/<id>, percent-encoded because this plugin's id is scoped.Verification
npm run typecheck, exit 0vite build;find -name '*.css'= 0check-ui-vocabulary.mjs, exit 0w-[137px],[&>tr]:…,bg-blue-500test/screens.test.tsxnpm run package+unzip -lnpm cireproduces all of itnode_modules50 tests.
package-lock.jsonis regenerated — it did not contain the new workspace, which would have failednpm ciin CI.Full write-up in
docs/iframe-credentials.md. All three fail silently.1. The host page's sandbox makes every authenticated call cross-origin.
PluginUiFrame.tsxsetssandbox="allow-scripts allow-forms allow-popups"— noallow-same-origin— so the document has an opaque origin. Everyfetchleaves withOrigin: null,credentials: 'include'cannot attach the session cookie, andEventSource(withCredentials)has the same problem. All four screens are data-driven. The bundle is correct; the boundary is not, and no screen loads data in a real browser until this trust-model decision is made. Tests do not show it because they stubfetch, which has no origin.2. The host page rejects every scoped plugin id — including this one. Its regex is
/^[a-z0-9](?:[a-z0-9._-]{0,62}[a-z0-9])?$/and its comment claims it "mirrors the plugin-id charset gate inmanifestLoader". It does not:manifestLoader.ts:182is/^(?:@[a-z0-9][a-z0-9._-]*\/)?[a-z0-9][a-z0-9._-]*$/, where the scope is optional but blessed. This plugin's id is@omadia/dev-platform. One-line fix in core.3.
borderrenders invisible.plugin-ui.source.csslines 341/346/353 use a top-level comma where@source inline()expands only braces:Verified against the committed artifact: no
.border,.divide-yor.transitionrule exists. Tailwind's base reset isborder: 0 solid, soclass="border border-border"— 27 occurrences in the ported pages — sets a colour on a zero-width border and renders invisible.plugin-ui-vocabulary.mdlists all three groups as available, so the doc and the artifact disagree.src/lib/cx.tsexportsBORDER = 'border-t border-r border-b border-l'(all four are emitted, 1px each) andtest/vocabulary.test.tspins the broken state, so regenerating the vocabulary after the core fix fails that test and prompts the workaround to collapse back to'border'rather than rotting.Fix in core:
Test plan
npm run typecheck— exit 0npm run build— exit 0, zero.cssemittednpm run test -w packages/ui— 50/50npm run lint:vocabulary— exit 0npm run package -w packages/plugin— ZIP containsui/index.html, zero stylesheetsdata-themeand fakingborderinto the vocabulary both turn the suite redSupersedes #2, which accidentally carried two unpushed P4 commits from a concurrent agent sharing the same local checkout. This branch is a clean cherry-pick onto
main— one commit, P2 only.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.