diff --git a/module/services/frontend/AGENTS.md b/module/services/frontend/AGENTS.md index cd1e2aadd..3dabec42a 100644 --- a/module/services/frontend/AGENTS.md +++ b/module/services/frontend/AGENTS.md @@ -14,6 +14,20 @@ manifest — `id`, `nav`, `frontend.manifestUrl` + `exposedModule`, optional `src/solutions/registry.ts`, which then writes the frontend half **through the gateway** (`POST /solutions/_frontend`). +`sources` is the optional declaration of what a solution is **built on**: +`[{ provider: github, repo: owner/name, paths, ref, label }]`. `repo` is held +to `AddGitHubSourceRequest`'s own pattern, and the paths and ref to its bounds, +at the far end of the same journey — the declaration is submitted unedited by +the kit's ``, so a repository the connect RPC would refuse +is refused when it is *declared* rather than weeks later when somebody presses +Connect. A malformed declaration fails the registration whole, as a dashboard +or a surface does, and one repository declared twice is refused outright: +nothing downstream could say which of the two a card renders. It is a +statement, not an authority — declaring a source connects nothing and grants +no read of its contents. The solution page reads it in process and hands it to +the remote it mounts as `SolutionBinding.declaredSources`, so it is on neither +the public nor the internal HTTP projection and no new RPC exists to serve it. + The route **relays the registry's own answer**: `409` for a revision conflict, `403` when the id belongs to another publisher, `422` (`registration_rejected`) when the registry will not admit the manifest, `503` when the registry cannot be diff --git a/module/services/frontend/code/package-lock.json b/module/services/frontend/code/package-lock.json index 606968f29..8930678ff 100644 --- a/module/services/frontend/code/package-lock.json +++ b/module/services/frontend/code/package-lock.json @@ -14,8 +14,8 @@ "@base-ui/react": "^1.3.0", "@bufbuild/protobuf": "^2.11.0", "@codefly-dev/saas-sdk": "0.3.15", - "@codefly-dev/saas-ui": "0.11.4", - "@codefly-dev/ui": "0.11.4", + "@codefly-dev/saas-ui": "0.11.5", + "@codefly-dev/ui": "0.11.5", "@codefly/saas-plugin-contract": "2.3.0", "@codefly/saas-plugin-manifest": "0.1.0", "@codefly/saas-plugin-react": "0.4.1", @@ -15266,7 +15266,7 @@ }, "packages/codefly-ui": { "name": "@codefly-dev/ui", - "version": "0.11.4", + "version": "0.11.5", "license": "Apache-2.0", "dependencies": { "@base-ui/react": "^1.3.0", @@ -15418,7 +15418,7 @@ }, "packages/saas-ui": { "name": "@codefly-dev/saas-ui", - "version": "0.11.4", + "version": "0.11.5", "license": "Apache-2.0", "devDependencies": { "@bufbuild/protobuf": "^2.11.0", diff --git a/module/services/frontend/code/package.json b/module/services/frontend/code/package.json index e64b5826e..675b2aa80 100644 --- a/module/services/frontend/code/package.json +++ b/module/services/frontend/code/package.json @@ -37,8 +37,8 @@ "@base-ui/react": "^1.3.0", "@bufbuild/protobuf": "^2.11.0", "@codefly-dev/saas-sdk": "0.3.15", - "@codefly-dev/saas-ui": "0.11.4", - "@codefly-dev/ui": "0.11.4", + "@codefly-dev/saas-ui": "0.11.5", + "@codefly-dev/ui": "0.11.5", "@codefly/saas-plugin-contract": "2.3.0", "@codefly/saas-plugin-manifest": "0.1.0", "@codefly/saas-plugin-react": "0.4.1", diff --git a/module/services/frontend/code/packages/codefly-ui/README.md b/module/services/frontend/code/packages/codefly-ui/README.md index 697dccfa5..f7c35e303 100644 --- a/module/services/frontend/code/packages/codefly-ui/README.md +++ b/module/services/frontend/code/packages/codefly-ui/README.md @@ -51,6 +51,33 @@ and the v1 subset — is [CATALOG.md](./CATALOG.md). `allowImages`, and no `innerHTML` anywhere; colour only through the host's token utilities. +### Status, headers and empty states + +Three props exist because a consuming solution reported reaching past the kit +for them (the same report that asked for a standalone stylesheet, a `Chip` and +list primitives — those are **not** here yet): + +- `` prefixes a small filled circle in the variant's own colour. + It is `aria-hidden` and decorative: the badge's text is what a reader and a + screen reader get, so the dot adds a glanceable mark without making colour + the only channel carrying the meaning. Use it when several badges sit in one + row and have to be told apart at a glance. **There is no `success` or + `warning` variant**, because the appearance layer defines no such token — + `destructive` is the only semantic colour the skin ladder carries. A healthy + state therefore uses `default` (solid) against `outline` for "not started"; + giving status its own tones means adding appearance tokens, which is a design + decision for the skin's owner, not something a component may invent. +- `` puts a line under the title, matching `Section`'s prop + of the same name. Without it a caller needing one either mixes `CardRoot` + + `CardDescription` into a page of plain `Card`s, which is how a page stops + looking like one page, or drops the sentence into the body where it reads as + content and loses the `card-description` slot. +- `` takes nodes, not just a string, so an empty state + whose explanation lives elsewhere on the page can **point at it** — "the card + above says where this stands", with a link if there is one. `children` is the + action slot; a cross-reference is not an action, so prose there (or + hand-written outside the block) loses the slot and its width bound. + ## Entry points | Import | Contents | diff --git a/module/services/frontend/code/packages/codefly-ui/package.json b/module/services/frontend/code/packages/codefly-ui/package.json index d86b3edfc..759371743 100644 --- a/module/services/frontend/code/packages/codefly-ui/package.json +++ b/module/services/frontend/code/packages/codefly-ui/package.json @@ -1,6 +1,6 @@ { "name": "@codefly-dev/ui", - "version": "0.11.4", + "version": "0.11.5", "description": "The shared, versioned Codefly SaaS frontend kit: the plugin host and the tokens-as-data skin mechanism, standalone so the host and Module-Federation remotes import and dedupe one copy.", "repository": { "type": "git", diff --git a/module/services/frontend/code/packages/codefly-ui/src/__tests__/no-raw-type-primitives.test.ts b/module/services/frontend/code/packages/codefly-ui/src/__tests__/no-raw-type-primitives.test.ts index c878bddee..e03087adc 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/__tests__/no-raw-type-primitives.test.ts +++ b/module/services/frontend/code/packages/codefly-ui/src/__tests__/no-raw-type-primitives.test.ts @@ -75,7 +75,10 @@ const files = sourceFiles(srcDir); // when its file reaches zero. Anything else in that package is default-deny. const SAAS_UI_BASELINE: Record = { "datasources/collection-access.tsx": 2, - "datasources/connect-github-form.tsx": 16, + // Lowered from 16 when the credential block moved into + // `datasources/credential-mode.tsx`, which carries none: the shared copy + // names its slots, so the debt left behind shrank rather than moving. + "datasources/connect-github-form.tsx": 11, "datasources/datasources-panel.tsx": 17, }; const saasUiDir = resolve(srcDir, "../../saas-ui/src"); diff --git a/module/services/frontend/code/packages/codefly-ui/src/layout/badge.tsx b/module/services/frontend/code/packages/codefly-ui/src/layout/badge.tsx index 2d91847f1..65d9febb6 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/layout/badge.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/layout/badge.tsx @@ -27,17 +27,41 @@ const badgeVariants = cva( }, ); +/** + * `dot` prefixes a small filled circle that inherits the variant's text colour. + * + * It is decorative and `aria-hidden`: the badge's own text is what a reader and + * a screen reader get, so the dot adds a glanceable mark without adding a + * second, colour-only channel that carries meaning of its own. That is the + * point of keeping it here rather than letting each caller prepend its own — + * a hand-rolled dot is where a status ends up encoded in colour alone. + */ function Badge({ className, variant = "default", + dot = false, render, + children, ...props -}: useRender.ComponentProps<"span"> & VariantProps) { +}: useRender.ComponentProps<"span"> & + VariantProps & { dot?: boolean }) { return useRender({ defaultTagName: "span", props: mergeProps<"span">( { className: cn(badgeVariants({ variant }), className), + children: dot ? ( + <> + + {children} + + ) : ( + children + ), }, props, ), diff --git a/module/services/frontend/code/packages/codefly-ui/src/layout/card.tsx b/module/services/frontend/code/packages/codefly-ui/src/layout/card.tsx index 1a661192b..c2295e963 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/layout/card.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/layout/card.tsx @@ -10,6 +10,17 @@ import { CardRoot } from "./card-root.js"; export interface CardProps { title?: ReactNode; + /** + * A line under the title saying what this card is, matching `Section`'s + * prop of the same name. + * + * Without it a caller needing one either reaches for `CardRoot` + + * `CardDescription` — mixing two card shapes into one page, which is how a + * page stops looking like one page — or drops the sentence into the body as + * a muted paragraph, where it reads as content rather than as the card's + * own description and no longer carries the `card-description` slot. + */ + description?: ReactNode; /** Trailing controls rendered opposite the title (buttons, menus). */ actions?: ReactNode; children?: ReactNode; @@ -17,17 +28,41 @@ export interface CardProps { } /** A bordered surface with an optional title row and trailing actions. */ -export function Card({ title, actions, children, className }: CardProps) { +export function Card({ + title, + description, + actions, + children, + className, +}: CardProps) { + const hasHeader = title != null || description != null || actions != null; return ( - {(title || actions) && ( -
- {title &&

- {title} -

} - {actions &&
{actions}
} + {hasHeader && ( + // `items-start`, not `items-center`: with a description the heading + // block is two lines tall and centring would float the actions + // against the middle of it. Matches `Section`'s header. +
+
+ {title != null && ( +

+ {title} +

+ )} + {description != null && ( +

+ {description} +

+ )} +
+ {actions != null && ( +
{actions}
+ )}
)} {children} diff --git a/module/services/frontend/code/packages/codefly-ui/src/layout/empty-state.tsx b/module/services/frontend/code/packages/codefly-ui/src/layout/empty-state.tsx index dae1ac12a..d89afbd87 100644 --- a/module/services/frontend/code/packages/codefly-ui/src/layout/empty-state.tsx +++ b/module/services/frontend/code/packages/codefly-ui/src/layout/empty-state.tsx @@ -17,8 +17,20 @@ export interface EmptyStateProps { /** Leading glyph; defaults to a generic inbox so the block always has one. */ icon?: ReactNode; heading: ReactNode; + /** + * The line under the heading. Takes nodes, not just a string, so an empty + * state whose explanation lives elsewhere on the page can **point at it** + * — "the source card above says where this stands", with a link if there is + * one to give. That is what this prop is for; `children` below is the + * action slot, and a cross-reference is not an action, so putting a + * sentence there (or hand-writing one outside the block) loses the + * `empty-state-description` slot and its width bound. + */ description?: ReactNode; - /** Optional trailing content, e.g. a call-to-action button. */ + /** + * Optional trailing content: a call to action, i.e. something to press. + * Prose belongs in `description`. + */ children?: ReactNode; className?: string; variant?: "default" | "illustrated"; diff --git a/module/services/frontend/code/packages/saas-ui/README.md b/module/services/frontend/code/packages/saas-ui/README.md index c3a3ec783..6ad5c4f1a 100644 --- a/module/services/frontend/code/packages/saas-ui/README.md +++ b/module/services/frontend/code/packages/saas-ui/README.md @@ -29,14 +29,22 @@ from the package root and from `@codefly-dev/saas-ui/solution`, which imports React, Connect and the SDK's service descriptors but none of the datasource components: -- `SolutionBinding` — the backend half of the props the host injects into every - solution page (`solutionId`, `apiBase`, `getAccessToken`, `subscribeToken`, - `refreshAccessToken`, `authedFetch`). The host's `SolutionPageProps` extends - it, so a remote types its props against the one definition. `subscribeToken` +- `SolutionBinding` — the props the host injects into every solution page: the + backend half (`solutionId`, `apiBase`, `getAccessToken`, `subscribeToken`, + `refreshAccessToken`, `authedFetch`) plus `declaredSources`. The host's + `SolutionPageProps` extends it, so a remote types its props against the one + definition. `subscribeToken` is optional but a host that can notify should pass it: the getter stays stable while the token rotates underneath it, so without a subscription the kit re-reads on a short interval — one timer per observer, for as long as the page is open. +- `declaredSources: DeclaredSource[]` — what this solution declared it is built + on (`sources:` in its registration manifest), validated by the host at + registration and handed straight back, so the repository is stated once + rather than once in the manifest an operator reads and again in the bundle. + Feed an entry to ``. Optional, and absent from an older + host: treat absence as "the host does not tell me", never as "this solution + declares nothing". - `solutionFetch(binding, path, init)` / `solutionJson(binding, path, init)` — a request to the solution's own backend at `apiBase + path`, same-origin, through the host's `authedFetch` (refresh-then-retry on a 401) with the bearer @@ -151,6 +159,57 @@ components: GitHub's unauthenticated limit of 60 requests an hour per IP address. When the App is installed on that repository for the organization, the host uses the App instead. +- `` — one source a solution **declares** + it is built on, in one of three states. A solution built on one known + repository must not ask the person which repository: it declares it, and all + that is left to supply is the credential. + - **Set up** — nothing connected serves the declaration. The three credential + modes (App / public / PAT) and nothing else; `repo`, `paths` and `ref` come + from the declaration, are rendered as text, and are submitted unedited to + `AddGitHubSource`. There is no repository control at all, which is the + point: a field for it would let the person connect something other than + what the solution reads. The entries land in a collection named by the + declaration's `label`, falling back to the repository. + - **Connected** — the matched source is `ACTIVE`: its last ingest and commit, + how live updates reach it, the reconcile interval that bounds its + staleness, **Sync now**, and the same `renderSourceDetail` slot the panel + offers so the consumer can show the ingesting module's progress. + - **Error** — `DEGRADED` or `PAUSED`: the host's `status_reason` verbatim, + and Reconnect, which asks for the credential again (a replacement PAT, or + nothing on the App and public paths) and enqueues a sync. + There is no fourth state. A source whose status an **older host does not + report** (the field decodes to its proto default, which the gateway maps to + `unknown` rather than guessing) reads as **Connected** and says its state is + not reported: it matched the declaration, so it *is* connected, and calling + the absence of a status an error would paint a healthy source red with no + `status_reason` to show for it and offer to Reconnect — asking a person for a + credential again to fix nothing. Sync is still offered; Reconnect is not. + A viewer without `canManage` still sees the state and the reason, and is told + who acts; the host refuses the calls either way. The card takes no query or + auth context of its own — the same `client | gateway` binding + `DatasourcesPanel` takes — and adds **no RPC**: it reads `ListSources` for the + org and matches on `provider` + `repo` (case-insensitively, as GitHub treats + `owner/name`) plus `paths` when the declaration names any. + Two connected sources matching one declaration are **reported, never silently + picked**: they can disagree about credential, branch, scope and health, so + rendering the first would sync one and leave the other ingesting invisibly. + `matchDeclaredSources(declared, sources)` is that rule as a pure function, for + a consumer rendering the states in its own shell; `declaredCollectionLabel` + resolves where connected entries land. +- `CredentialMethodField` / `AccessTokenField` / `WebhookSecretField` / + `AppInstallPrompt` (with `CredentialMethod`, `credentialMethodFrom`, + `fieldErrorClass`) — the credential block `ConnectGitHubForm` and + `` both render, presentational and controlled, exported + so a consumer building its own shell around `matchDeclaredSources` can **ask + for the credential without copying the form**. Compose these; do not copy + `ConnectGitHubForm` — `solutions/README.md` refuses a copied capability + wrapped in a shared card, and these three modes are the whole of what a + person decides between, so two surfaces that word them differently teach two + different products. + Do not mount this and `` on the same page: both redeem the + GitHub App's single-use return state, and the loser reports a rejection for an + installation that in fact succeeded. - `createDatasourceClient({ apiBase, getAccessToken, refreshAccessToken })` — builds the gateway-bound `DatasourceClient` (with 401 refresh-and-retry) directly, for driving the hooks outside the panel. `datasourceClientOverTransport(transport)` does the same over a diff --git a/module/services/frontend/code/packages/saas-ui/package.json b/module/services/frontend/code/packages/saas-ui/package.json index 491745d99..0dcf98419 100644 --- a/module/services/frontend/code/packages/saas-ui/package.json +++ b/module/services/frontend/code/packages/saas-ui/package.json @@ -1,6 +1,6 @@ { "name": "@codefly-dev/saas-ui", - "version": "0.11.4", + "version": "0.11.5", "description": "Reusable SaaS-domain frontend components (datasources, \u2026) the portal and solutions both import, built on @codefly-dev/ui primitives and driven by an injected typed client.", "repository": { "type": "git", diff --git a/module/services/frontend/code/packages/saas-ui/src/__tests__/peer-docs.test.ts b/module/services/frontend/code/packages/saas-ui/src/__tests__/peer-docs.test.ts index 969d6ebef..ff74ce8b0 100644 --- a/module/services/frontend/code/packages/saas-ui/src/__tests__/peer-docs.test.ts +++ b/module/services/frontend/code/packages/saas-ui/src/__tests__/peer-docs.test.ts @@ -77,7 +77,12 @@ describe("the README install line matches peerDependencies", () => { }); describe("SaaS presentation composes shared controls", () => { - for (const file of ["connect-github-form.tsx", "datasources-panel.tsx"]) { + for (const file of [ + "connect-github-form.tsx", + "credential-mode.tsx", + "datasources-panel.tsx", + "declared-source-card.tsx", + ]) { it(file, () => { const source = readFileSync( join(packageRoot, "src/datasources", file), diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/__tests__/declared-source-card.test.tsx b/module/services/frontend/code/packages/saas-ui/src/datasources/__tests__/declared-source-card.test.tsx new file mode 100644 index 000000000..1e7da5075 --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/__tests__/declared-source-card.test.tsx @@ -0,0 +1,464 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import type { ReactElement } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { DeclaredSourceCard } from "../declared-source-card.js"; +import { + declaredCollectionLabel, + type DeclaredSource, + matchDeclaredSources, +} from "../declared-source.js"; +import type { DatasourceClient, DatasourceView } from "../types.js"; + +afterEach(cleanup); + +function renderWithClient(ui: ReactElement) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + {ui}, + ); +} + +function fakeClient( + overrides: Partial = {}, +): DatasourceClient { + return { + listSources: vi.fn(async () => [] as DatasourceView[]), + addGitHubSource: vi.fn(async () => {}), + syncSource: vi.fn(async () => "job-1"), + deleteSource: vi.fn(async () => {}), + ...overrides, + }; +} + +const declared: DeclaredSource = { + provider: "github", + repo: "example-org/handbook", + paths: ["proposals/"], + ref: "main", + label: "Proposals", +}; + +const connected: DatasourceView = { + id: "ds-1", + orgId: "org-1", + provider: "github", + repo: "example-org/handbook", + paths: ["proposals/"], + branch: "main", + boundaryNodeId: "11111111-1111-1111-1111-111111111111", + webhookConfigured: false, + liveDelivery: "app_webhook", + reconcileIntervalSeconds: 3600, + status: "active", + lastSyncedAt: undefined, + lastIngestedAt: "2026-10-02T09:30:00.000Z", + lastIngestedCommit: "abcdef1234567890", + createdAt: undefined, +}; + +describe("matchDeclaredSources", () => { + it("matches a repository however its owner and name are cased", () => { + // GitHub treats owner/name case-insensitively, so a source connected as + // Example-Org/Handbook IS the declared repository. Comparing literally + // would show the declaration as unconnected beside the source serving it. + expect( + matchDeclaredSources({ provider: "github", repo: "Example-Org/Handbook" }, [ + connected, + ]), + ).toHaveLength(1); + }); + + it("ignores paths when the declaration names none", () => { + expect( + matchDeclaredSources( + { provider: "github", repo: "example-org/handbook" }, + [{ ...connected, paths: ["anything/"] }], + ), + ).toHaveLength(1); + }); + + it("requires the same path set when the declaration names paths", () => { + expect( + matchDeclaredSources(declared, [{ ...connected, paths: ["docs/"] }]), + ).toHaveLength(0); + expect( + matchDeclaredSources({ ...declared, paths: ["a/", "b/"] }, [ + { ...connected, paths: ["b/", "a/"] }, + ]), + ).toHaveLength(1); + }); + + it("lands entries in the declared label, falling back to the repository", () => { + expect(declaredCollectionLabel(declared)).toBe("Proposals"); + expect( + declaredCollectionLabel({ provider: "github", repo: "example-org/handbook" }), + ).toBe("example-org/handbook"); + }); +}); + +describe("DeclaredSourceCard", () => { + it("asks only for the credential when nothing is connected yet", async () => { + const client = fakeClient(); + renderWithClient( + , + ); + + expect(await screen.findByText("Set up")).toBeTruthy(); + // The declaration is shown, as text — see "never offers the repository + // as a field" below for the half that matters. + expect(screen.getByText("example-org/handbook")).toBeTruthy(); + expect(screen.getByText("proposals/")).toBeTruthy(); + expect(screen.getByLabelText("Authentication")).toBeTruthy(); + }); + + it("connects the declared repository, paths and ref, not anything typed", async () => { + const client = fakeClient(); + renderWithClient( + , + ); + fireEvent.change(await screen.findByLabelText("Access token"), { + target: { value: "ghp_token" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Connect" })); + + await waitFor(() => expect(client.addGitHubSource).toHaveBeenCalled()); + expect(client.addGitHubSource).toHaveBeenCalledWith({ + orgId: "org-1", + repo: "example-org/handbook", + paths: ["proposals/"], + branch: "main", + targetCollection: "Proposals", + accessToken: "ghp_token", + webhookSecret: "", + }); + }); + + it("never offers the repository as a field", async () => { + const client = fakeClient(); + renderWithClient( + , + ); + + await screen.findByText("Set up"); + // The whole point of a declaration: the repository is not a question. + // A control for it — free text or a picker — would let the person + // connect something other than what the solution is built on. + expect(screen.queryByLabelText("Repository")).toBeNull(); + expect(screen.queryByRole("textbox")).toBeNull(); + expect( + screen + .getAllByRole("combobox") + .map((element) => (element as HTMLSelectElement).id), + ).toHaveLength(1); + }); + + it("tells a viewer who cannot manage who does, and offers nothing", async () => { + const client = fakeClient(); + renderWithClient( + , + ); + + expect(await screen.findByText("Set up")).toBeTruthy(); + expect( + screen.getByText(/An organization administrator connects it/), + ).toBeTruthy(); + expect(screen.queryByLabelText("Authentication")).toBeNull(); + expect(screen.queryByRole("button", { name: "Connect" })).toBeNull(); + }); + + it("reports a connected source's provenance and delivery, and syncs it", async () => { + const client = fakeClient({ + listSources: vi.fn(async () => [connected]), + }); + const onSyncEnqueued = vi.fn(); + renderWithClient( + , + ); + + expect(await screen.findByText("Connected")).toBeTruthy(); + expect(screen.getByText(/last ingest .* · abcdef1/)).toBeTruthy(); + expect( + screen.getByText(/On push, through the GitHub App/), + ).toBeTruthy(); + expect(screen.getByText(/reconciled every 1 hour/)).toBeTruthy(); + // Connected is not a state that asks for a credential again. + expect(screen.queryByLabelText("Authentication")).toBeNull(); + + fireEvent.click(screen.getByRole("button", { name: "Sync now" })); + await waitFor(() => + expect(client.syncSource).toHaveBeenCalledWith("org-1", "ds-1"), + ); + expect(onSyncEnqueued).toHaveBeenCalledWith("job-1"); + }); + + it("shows a degraded source's reason verbatim and reconnects with a new token", async () => { + const degraded: DatasourceView = { + ...connected, + status: "degraded", + statusReason: + "the stored credential was rejected by GitHub (401) on the last 3 attempts", + }; + const reconnectSource = vi.fn(async () => "job-2"); + const client = fakeClient({ + listSources: vi.fn(async () => [degraded]), + reconnectSource, + }); + renderWithClient( + , + ); + + expect(await screen.findByText("Error")).toBeTruthy(); + // The host writes status_reason from a closed set of named reasons, so + // it is rendered as it arrives rather than reworded into a guess. + expect( + screen.getByText( + "the stored credential was rejected by GitHub (401) on the last 3 attempts", + ), + ).toBeTruthy(); + + fireEvent.change(screen.getByLabelText("Access token"), { + target: { value: "ghp_replacement" }, + }); + fireEvent.click( + screen.getByRole("button", { name: "Reconnect and sync" }), + ); + await waitFor(() => + expect(reconnectSource).toHaveBeenCalledWith( + "org-1", + "ds-1", + "ghp_replacement", + ), + ); + // Reconnecting a declared source never re-asks for the repository. + expect(client.addGitHubSource).not.toHaveBeenCalled(); + }); + + it("leaves a degraded source to an administrator when the viewer is not one", async () => { + const client = fakeClient({ + listSources: vi.fn(async () => [ + { ...connected, status: "degraded" as const, statusReason: "paused" }, + ]), + }); + renderWithClient( + , + ); + + expect(await screen.findByText("Error")).toBeTruthy(); + expect( + screen.getByText("An organization administrator reconnects this source."), + ).toBeTruthy(); + expect(screen.queryByLabelText("Access token")).toBeNull(); + expect(screen.queryByRole("button", { name: /Reconnect/ })).toBeNull(); + }); + + it("tells the three states apart by shape, and says the repository was not chosen here", async () => { + // The consuming solution reported that a Connected badge on `secondary` + // reads neutral beside an outline "Set up", so a working source looks + // like one nobody has connected. Each state now carries a dot as well as + // a tone, and Connected no longer borrows `secondary`. + const client = fakeClient({ + listSources: vi.fn(async () => [connected]), + }); + const { container } = renderWithClient( + , + ); + + const badge = await screen.findByText("Connected"); + expect(badge.className).not.toMatch(/bg-secondary/); + expect(container.querySelector('[data-slot="badge-dot"]')).toBeTruthy(); + // The card's own description slot, not a muted paragraph in the body: + // the first thing to understand is that the repository below is not a + // choice being offered. + const description = container.querySelector( + '[data-slot="card-description"]', + ); + expect(description?.textContent).toBe( + "Declared by this solution, not chosen here.", + ); + }); + + it("does not call a source in error when the host did not report its state", async () => { + // "unknown" is what an OLDER host sends: the status field decodes to its + // proto default and the gateway maps it there rather than guessing. The + // source is connected; treating the absence of a status as DEGRADED + // would paint it red, with no status_reason to show for it, and offer to + // Reconnect — asking for a credential again to fix nothing. + const client = fakeClient({ + listSources: vi.fn(async () => [ + { ...connected, status: "unknown" as const }, + ]), + }); + renderWithClient( + , + ); + + expect(await screen.findByText("Connected")).toBeTruthy(); + expect(screen.queryByText("Error")).toBeNull(); + expect( + screen.getByText(/This host does not report this source's state/), + ).toBeTruthy(); + // Sync is offered — it works on any host — and Reconnect is not. + expect(screen.getByRole("button", { name: "Sync now" })).toBeTruthy(); + expect(screen.queryByRole("button", { name: /Reconnect/ })).toBeNull(); + expect(screen.queryByLabelText("Access token")).toBeNull(); + }); + + it("reports two matching sources instead of silently picking one", async () => { + // Two sources can legitimately exist for one repository — one through the + // App and one through a PAT, say — and they can disagree about branch, + // scope and health. Rendering the first would sync one and leave the + // other ingesting invisibly. + const second: DatasourceView = { + ...connected, + id: "ds-2", + branch: "release", + status: "degraded", + }; + const client = fakeClient({ + listSources: vi.fn(async () => [connected, second]), + }); + renderWithClient( + , + ); + + expect( + await screen.findByText("More than one source matches this declaration"), + ).toBeTruthy(); + expect(screen.getByText(/ds-1/)).toBeTruthy(); + expect(screen.getByText(/2 connected sources read/)).toBeTruthy(); + expect(screen.queryByRole("button", { name: "Sync now" })).toBeNull(); + expect(screen.queryByLabelText("Authentication")).toBeNull(); + }); + + describe("the GitHub App return leg", () => { + function landOn(search: string) { + window.history.replaceState(null, "", `/s/proposals${search}`); + } + afterEach(() => landOn("")); + + function appClient(overrides: Partial = {}) { + return fakeClient({ + beginGitHubAppSetup: vi.fn(async () => ({ + installUrl: "https://github.com/apps/example/installations/new", + state: "s1", + expiresAt: undefined, + })), + completeGitHubAppSetup: vi.fn(async () => ({ + installationId: "42", + repositories: [], + })), + ...overrides, + }); + } + + it("connects through the App with no token at all", async () => { + const client = appClient(); + renderWithClient( + , + ); + + // The App is the default where it is available, and it sends no + // credential: the host resolves the installation covering the repo. + expect(await screen.findByLabelText("Authentication")).toHaveProperty( + "value", + "app", + ); + expect(screen.queryByLabelText("Access token")).toBeNull(); + fireEvent.click(screen.getByRole("button", { name: "Connect" })); + + await waitFor(() => expect(client.addGitHubSource).toHaveBeenCalled()); + const input = vi.mocked(client.addGitHubSource).mock.calls[0]?.[0]; + expect(input?.repo).toBe("example-org/handbook"); + // Absent, not empty: the host reads a missing token as "resolve the + // installation covering this repository" (AddGitHubSourceRequest). + expect(input && "accessToken" in input).toBe(false); + }); + + it("redeems the state the redirect echoed back", async () => { + landOn("?installation_id=42&setup_action=install&state=s1&code=oauth-1"); + const client = appClient(); + renderWithClient( + , + ); + + await waitFor(() => + expect(client.completeGitHubAppSetup).toHaveBeenCalledWith( + "org-1", + "s1", + "42", + "oauth-1", + ), + ); + // And burns the single-use parameters out of the address bar. + expect(window.location.search).toBe(""); + }); + + it("says so when the viewer cannot redeem the return it just spent", async () => { + // The capture and the scrub run regardless of authority, because the + // credential canManage is read from may not have arrived on the load + // that follows the redirect. A viewer who really cannot manage has + // spent a single-use state on an installation standing at GitHub. + landOn("?installation_id=42&state=s1&code=oauth-1"); + const client = appClient(); + renderWithClient( + , + ); + + expect( + await screen.findByText( + "The installation finished, but it is not yours to connect", + ), + ).toBeTruthy(); + expect(client.completeGitHubAppSetup).not.toHaveBeenCalled(); + }); + }); + + it("says the state is unknown when the source list could not be read", async () => { + // An unreadable list is not an unconnected source: offering "Set up" + // here would invite a second connection of a repository that may well + // already be connected. + const client = fakeClient({ + listSources: vi.fn(async () => { + throw new Error("datasource service unavailable"); + }), + }); + renderWithClient( + , + ); + + expect( + await screen.findByText(/has not been reported as disconnected/), + ).toBeTruthy(); + expect(screen.queryByText("Set up")).toBeNull(); + expect(screen.queryByLabelText("Authentication")).toBeNull(); + }); +}); diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/connect-github-form.tsx b/module/services/frontend/code/packages/saas-ui/src/datasources/connect-github-form.tsx index 1c099187d..88f35524c 100644 --- a/module/services/frontend/code/packages/saas-ui/src/datasources/connect-github-form.tsx +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/connect-github-form.tsx @@ -1,11 +1,18 @@ "use client"; +import { Button, Input, Label, Textarea } from "@codefly-dev/ui/layout"; import { zodResolver } from "@hookform/resolvers/zod"; import { useId } from "react"; import { useForm, useWatch } from "react-hook-form"; +import { + AccessTokenField, + AppInstallPrompt, + CredentialMethodField, + fieldErrorClass as errorClass, + WebhookSecretField, +} from "./credential-mode.js"; import { type ConnectGitHubValues, connectGitHubSchema } from "./schema.js"; import type { CollectionAccessView, GitHubAppRepositoryView } from "./types.js"; -import { Button, Input, Label, Textarea } from "@codefly-dev/ui/layout"; interface ConnectGitHubFormProps { collections?: CollectionAccessView[]; @@ -28,8 +35,6 @@ interface ConnectGitHubFormProps { errorMessage?: string; } -const errorClass = "text-sm text-destructive"; - export function ConnectGitHubForm({ onSubmit, collections, @@ -103,77 +108,37 @@ export function ConnectGitHubForm({ className="space-y-4" noValidate > -
- - -

- {method === "app" - ? "Install the app on repositories you choose. Nothing to create, paste, or rotate." - : method === "public" - ? "No token needed, but GitHub allows only 60 unauthenticated requests an hour, so large repositories sync slowly — use the App or a token for those." - : "For an existing connection, or for development. Restrict the token to this repository with Contents: Read-only."} -

-
+ { + form.setValue("method", next); + // The App path can only connect what the installation + // grants, and the picker renders blank for anything else — + // leaving a repository typed on another path selected would + // submit a repository the reader cannot see chosen. + if ( + next === "app" && + !appRepositories?.some( + (candidate) => candidate.repo === form.getValues("repo"), + ) + ) { + form.setValue("repo", ""); + // The branch was filled in from the repository, so it + // describes one that is no longer selected. + form.setValue("branch", ""); + } + }} + /> {method === "app" && !appRepositories ? ( -
-

- Installing sends you to GitHub to pick the repositories this - organization may read. You come back here to choose the branch - and paths. -

- - {appSetupError && ( -

- {appSetupError} -

- )} -
+ ) : method === "app" ? (
@@ -384,58 +349,28 @@ export function ConnectGitHubForm({
{method === "pat" && ( -
- - -

- Use a fine-grained PAT restricted to this repository with - Contents: Read-only. Your organization may require approval or - SSO authorization. Repository and branch access are verified - before saving. -

- {errors.accessToken && ( -

- {errors.accessToken.message} -

- )} -
+ )} {/* A public repository is read with no credential, and a webhook needs administration of the repository — the host refuses a secret there and keeps the source current by periodic sync. */} {method !== "public" && ( -
- - -

- Enables live webhook ingestion. Add it later if you don't - have it yet. -

- {errors.webhookSecret && ( -

- {errors.webhookSecret.message} -

- )} -
+ )} {errorMessage && ( diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/credential-mode.tsx b/module/services/frontend/code/packages/saas-ui/src/datasources/credential-mode.tsx new file mode 100644 index 000000000..2bb9e3b11 --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/credential-mode.tsx @@ -0,0 +1,206 @@ +"use client"; + +import { Button, Input, Label } from "@codefly-dev/ui/layout"; +import type { ComponentProps, ReactNode } from "react"; + +/** + * How a GitHub source authenticates, and the fields each mode needs. + * + * Lifted out of `connect-github-form.tsx` so a surface that fixes the + * repository — ``, where the solution declares what it + * reads and the person supplies only the credential — asks for the credential + * in exactly the same words, with exactly the same three modes, without + * copying the form. A second copy of this choice is the failure mode worth + * naming: the App's "nothing to paste or rotate", the public path's 60 + * requests an hour, and the PAT's Contents: Read-only scope are the whole of + * what a person has to decide between, and two surfaces that say them + * differently teach two different products. + * + * Everything here is presentational and controlled. The form drives it through + * react-hook-form; the card drives it from `useState`. Neither mode's meaning + * lives here — the host decides what a missing token means (see + * `AddGitHubSourceRequest.access_token`). + */ +export type CredentialMethod = "app" | "public" | "pat"; + +/** The shared error style for a field message. */ +export const fieldErrorClass = "type-body text-destructive"; + +/** + * Narrows a ` + onChange(credentialMethodFrom(event.target.value, appAvailable)) + } + > + {appAvailable && } + + + +

{methodHelp[method]}

+
+ ); +} + +/** + * The leg that sends the browser to GitHub to install the App. + * + * `description` is the caller's, because the two surfaces promise different + * things on the way back: the form returns to a repository picker, the + * declared card returns to a repository it already knows. + */ +export function AppInstallPrompt({ + description, + onBeginAppSetup, + phase, + error, +}: { + description: ReactNode; + onBeginAppSetup: () => void; + /** Which leg of App setup is in flight, if either. */ + phase?: "beginning" | "completing"; + error?: string; +}) { + return ( +
+

{description}

+ + {error && ( +

+ {error} +

+ )} +
+ ); +} + +/** The fine-grained PAT, and what it must be scoped to. */ +export function AccessTokenField({ + id, + errorId, + error, + inputProps, +}: { + id: string; + errorId: string; + error?: string; + inputProps: ComponentProps; +}) { + return ( +
+ + +

+ Use a fine-grained PAT restricted to this repository with Contents: + Read-only. Your organization may require approval or SSO authorization. + Repository and branch access are verified before saving. +

+ {error && ( +

+ {error} +

+ )} +
+ ); +} + +/** + * The shared secret GitHub signs push deliveries with. + * + * Offered on the App and PAT paths only: a public repository is read with no + * credential and a webhook needs administration of the repository, so the host + * refuses a secret there and keeps the source current by periodic sync. The + * caller decides whether to render it; this component does not re-derive that. + */ +export function WebhookSecretField({ + id, + errorId, + error, + inputProps, +}: { + id: string; + errorId: string; + error?: string; + inputProps: ComponentProps; +}) { + return ( +
+ + +

+ Enables live webhook ingestion. Add it later if you don't have it + yet. +

+ {error && ( +

+ {error} +

+ )} +
+ ); +} diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/datasources-panel.tsx b/module/services/frontend/code/packages/saas-ui/src/datasources/datasources-panel.tsx index 2e47e7345..9ff43a617 100644 --- a/module/services/frontend/code/packages/saas-ui/src/datasources/datasources-panel.tsx +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/datasources-panel.tsx @@ -22,8 +22,6 @@ import { useLoadingPhase, } from "@codefly-dev/ui/layout"; -import { ConnectError } from "@connectrpc/connect"; - import { QueryClient, QueryClientProvider, @@ -40,7 +38,9 @@ import { } from "../solution/viewer.js"; import { CollectionGrants } from "./collection-access.js"; import { ConnectGitHubForm } from "./connect-github-form.js"; +import { messageOf } from "./errors.js"; import { createDatasourceClient, type GatewayBinding } from "./gateway.js"; +import { readAppSetupReturn, scrubAppSetupReturn } from "./github-app-setup.js"; import { useAccessibleScopes, useAddGitHubSource, @@ -61,6 +61,8 @@ import { cn, formatGrants, formatIngest, + formatLiveDelivery, + formatReconcileInterval, formatSyncedAt, parsePaths, shortBoundaryId, @@ -895,61 +897,12 @@ function PanelMessage({ ); } -/** - * The parameters GitHub appends to the App's configured setup URL when it sends - * the browser back: the installation it claims was installed, and the state we - * minted. Both are required — `setup_action` is deliberately not consulted, so - * an existing installation gaining repositories (`update`) lands here exactly as - * a first install does. - * - * Returns null under SSR, where the panel renders before any address exists. - */ -function readAppSetupReturn(): { - state: string; - installationId: string; - code: string; -} | null { - if (typeof window === "undefined") return null; - const params = new URLSearchParams(window.location.search); - const state = params.get("state"); - const installationId = params.get("installation_id"); - // `code` is deliberately not part of the trigger. It is absent when the App - // was registered without "Request user authorization (OAuth) during - // installation", and the host answers that with the error naming the setting - // — which an operator can act on, where ignoring the return says nothing. - return state && installationId - ? { state, installationId, code: params.get("code") ?? "" } - : null; -} - -function scrubAppSetupReturn(): void { - const params = new URLSearchParams(window.location.search); - for (const key of ["state", "installation_id", "setup_action", "code"]) - params.delete(key); - const query = params.toString(); - window.history.replaceState( - null, - "", - `${window.location.pathname}${query ? `?${query}` : ""}${window.location.hash}`, - ); -} - function without(set: ReadonlySet, id: string): ReadonlySet { const next = new Set(set); next.delete(id); return next; } -function messageOf(error: unknown): string { - const message = - error instanceof ConnectError - ? error.rawMessage - : error instanceof Error - ? error.message - : "unexpected error"; - return message.replace(/^rpc error: code = \w+ desc = /, ""); -} - /** * A row's clocks. At most one of them ticks for a given source: a github * source's ingest is advanced by the change-set compiler and never sets @@ -1004,12 +957,7 @@ function LiveDeliveryCell({ source }: { source: DatasourceView }) { ); } - const label = - source.liveDelivery === "source_webhook" - ? "On push, through this source's webhook" - : source.liveDelivery === "app_webhook" - ? "On push, through the GitHub App" - : "No live updates"; + const label = formatLiveDelivery(source.liveDelivery); return ( <>
{label}
@@ -1020,23 +968,6 @@ function LiveDeliveryCell({ source }: { source: DatasourceView }) { ); } -/** - * The reconcile schedule in words. Undefined when the host does not report it; - * an interval of 0 means the periodic reconcile is off for this source, which - * — with no live delivery — leaves "Sync now" as the only thing that ever - * refreshes it, and is worth saying rather than leaving blank. - */ -function formatReconcileInterval(seconds: number | undefined): string { - if (seconds === undefined) return ""; - if (seconds <= 0) return "No periodic reconcile; refreshed by Sync now only"; - if (seconds % 3600 === 0) { - const hours = seconds / 3600; - return `Otherwise reconciled every ${hours} ${hours === 1 ? "hour" : "hours"}`; - } - const minutes = Math.max(1, Math.round(seconds / 60)); - return `Otherwise reconciled every ${minutes} ${minutes === 1 ? "minute" : "minutes"}`; -} - /** * How each status that is not `active` presents. Active is deliberately absent: * it is the state of nearly every row, so badging it too would bury the states diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source-card.tsx b/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source-card.tsx new file mode 100644 index 000000000..8d7b82ec9 --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source-card.tsx @@ -0,0 +1,762 @@ +"use client"; + +import { + Badge, + Banner, + Button, + Card, + useLoadingPhase, +} from "@codefly-dev/ui/layout"; +import { + QueryClient, + QueryClientProvider, + useQuery, +} from "@tanstack/react-query"; +import { type ReactNode, useEffect, useId, useMemo, useState } from "react"; +import { + useAccessToken, + viewerAdministersOrganization, +} from "../solution/viewer.js"; +import { + AccessTokenField, + AppInstallPrompt, + type CredentialMethod, + CredentialMethodField, + fieldErrorClass, + WebhookSecretField, +} from "./credential-mode.js"; +import { + type DeclaredSource, + declaredCollectionLabel, + matchDeclaredSources, +} from "./declared-source.js"; +import { messageOf } from "./errors.js"; +import { createDatasourceClient, type GatewayBinding } from "./gateway.js"; +import { readAppSetupReturn, scrubAppSetupReturn } from "./github-app-setup.js"; +import { useAddGitHubSource, useListSources } from "./queries.js"; +import type { DatasourceClient, DatasourceView } from "./types.js"; +import { + cn, + formatIngest, + formatLiveDelivery, + formatReconcileInterval, + formatSyncedAt, +} from "./util.js"; + +interface DeclaredSourceCardBaseProps { + orgId: string; + /** + * The source the solution declares it is built on. Read, never edited: the + * repository, its paths and its ref are the solution's statement about + * itself, so this card renders them and submits them, and offers no control + * that could change them. + */ + declared: DeclaredSource; + /** + * Whether the viewer may connect, reconnect or sync — the host's + * organization-administrator tier. Without it the card still says what the + * source is and how it is doing, and offers nothing to do about it. The host + * refuses the calls either way. Default: with a `gateway`, read from the + * viewer's credential; with an injected `client`, true. + */ + canManage?: boolean; + /** Called with the durable job id after a sync is enqueued. */ + onSyncEnqueued?: (jobId: string) => void; + /** + * Renders beneath the connected source: the same extension point + * `DatasourcesPanel` offers, through which the consumer shows what the + * module that ingests this source's files has made of them. The host names + * no such module. + */ + renderSourceDetail?: (source: DatasourceView) => ReactNode; + className?: string; +} + +/** + * One declared source, in one card, in one of three states: **Set up**, + * **Connected**, **Error**. + * + * For a solution built on one known repository, the repository is not a + * question. The solution declares it (`sources:` in its registration manifest, + * handed back to the remote as `SolutionBinding.declaredSources`); the person + * supplies only the credential, in the same three modes the full connect form + * offers, because both render the kit's one credential block. + * + * It reads the organization's connected sources and matches the declaration + * against them. It adds no RPC of its own and takes no query or auth context: + * either drive it with an injected `client`, or hand it a `gateway` binding and + * it self-wires, exactly like `DatasourcesPanel`. + * + * Do not mount this and `DatasourcesPanel` on the same page: both redeem the + * GitHub App's single-use return state, and the loser reports a rejection for + * an installation that succeeded. + */ +export type DeclaredSourceCardProps = DeclaredSourceCardBaseProps & + ( + | { client: DatasourceClient; gateway?: never } + | { gateway: GatewayBinding; client?: never } + ); + +export function DeclaredSourceCard(props: DeclaredSourceCardProps) { + if (props.gateway) { + const { gateway, ...rest } = props; + return ; + } + const { client, ...rest } = props; + return ; +} + +function GatewayBoundCard({ + gateway, + ...rest +}: DeclaredSourceCardBaseProps & { gateway: GatewayBinding }) { + const { + apiBase, + getAccessToken, + refreshAccessToken, + contentResource, + subscribeToken, + } = gateway; + const token = useAccessToken(getAccessToken, subscribeToken); + const client = useMemo( + () => + createDatasourceClient({ + apiBase, + getAccessToken, + refreshAccessToken, + contentResource, + }), + [apiBase, getAccessToken, refreshAccessToken, contentResource], + ); + const [queryClient] = useState( + () => new QueryClient({ defaultOptions: { queries: { retry: 1 } } }), + ); + return ( + + + + ); +} + +function DeclaredSourceCardView({ + client, + orgId, + declared, + canManage = true, + onSyncEnqueued, + renderSourceDetail, + className, +}: DeclaredSourceCardBaseProps & { client: DatasourceClient }) { + const list = useListSources(client, orgId); + const { indicator, quiet } = useLoadingPhase(list.isLoading); + const matches = useMemo( + () => matchDeclaredSources(declared, list.data ?? []), + [declared, list.data], + ); + const source = matches.length === 1 ? matches[0] : undefined; + + const [syncing, setSyncing] = useState(false); + const [notice, setNotice] = useState(null); + const [actionError, setActionError] = useState(null); + + const beginAppSetup = client.beginGitHubAppSetup?.bind(client); + const completeAppSetup = client.completeGitHubAppSetup?.bind(client); + const reconnectSource = client.reconnectSource?.bind(client); + const appSetup = useAppSetupReturn(orgId, canManage, completeAppSetup); + const addMutation = useAddGitHubSource(client); + const [reconnectPending, setReconnectPending] = useState(false); + const [reconnectError, setReconnectError] = useState(); + + const handleSync = async () => { + if (!source) return; + setNotice(null); + setActionError(null); + setSyncing(true); + try { + const jobId = await client.syncSource(orgId, source.id); + setNotice( + "Sync queued. Ingestion runs in the background; content appears when it is ready.", + ); + onSyncEnqueued?.(jobId); + } catch (error) { + setActionError(`Couldn't sync ${source.repo}: ${messageOf(error)}`); + } finally { + setSyncing(false); + } + }; + + // "unknown" is not a fourth status, it is the ABSENCE of one: an older host + // leaves the field at its proto default and the gateway maps it there + // rather than guessing, exactly as it leaves `liveDelivery` undefined. A + // source that matched the declaration IS connected, so folding that + // absence into Error would paint a healthy source red, with no + // `status_reason` to show for it (an older host sends none either), and + // offer to Reconnect — asking a person for a credential again to fix + // nothing. It reads as Connected, and says the state is not reported. + const state: DeclaredSourceState = + matches.length > 1 + ? "ambiguous" + : !source + ? "setup" + : source.status === "active" || source.status === "unknown" + ? "connected" + : "error"; + + return ( + : null + } + > + + + {/* The capture and the scrub ran regardless of authority — they have + to, because the credential `canManage` is read from may not have + arrived yet on the load that follows the redirect. So a viewer who + really cannot manage has just spent a single-use state on an + installation that stands at GitHub, and saying nothing would leave + an ordinary card over a connect that never happened. Held until + the list has answered SUCCESSFULLY: a credential not yet read also + reads as "not an administrator", and a failed list proves nothing + and already shows its own error. */} + {appSetup.unredeemable && list.isSuccess && ( + + The GitHub App is installed. Only an organization administrator can + connect this source with it; ask one to open this page. The + installation itself is already in place. + + )} + + {/* Every state below is a claim about a read that ANSWERED, so the + body is gated on success rather than on `isLoading` being false. A + query disabled for want of an org id sits at `isPending` with + `fetchStatus: "idle"`, so `isLoading` is false and a branch keyed on + it falls straight through to "Set up" — offering a connect form for + a source nobody has looked for yet, which may already be connected. + The hook holds the indicator's timing; `quiet` is its pre-delay + window, which renders nothing rather than flashing. */} + {list.isError ? ( +

+ Couldn't read this organization's sources, so the state of + this one is unknown. It has not been reported as disconnected.{" "} + {messageOf(list.error)} +

+ ) : !list.isSuccess ? ( + quiet || !indicator ? null : ( +

+ Checking this source… +

+ ) + ) : state === "ambiguous" ? ( + + ) : state === "setup" ? ( + canManage ? ( + void appSetup.begin(beginAppSetup, orgId) + : undefined + } + onSubmit={(credential) => + addMutation.mutate({ + orgId, + repo: declared.repo, + paths: declared.paths ?? [], + branch: declared.ref ?? "", + targetCollection: declaredCollectionLabel(declared), + // Neither the App nor a public repository sends a token: + // the host resolves the installation, or confirms with + // GitHub that the repository is public. A public source + // takes no webhook either. + ...(credential.method === "pat" + ? { accessToken: credential.accessToken } + : {}), + webhookSecret: + credential.method === "public" + ? "" + : credential.webhookSecret, + }) + } + /> + ) : ( +

+ This source is not connected yet. An organization administrator + connects it; nothing is needed from you. +

+ ) + ) : source ? ( + <> + + {renderSourceDetail &&
{renderSourceDetail(source)}
} + {notice && ( + setNotice(null)} + dismissLabel="Dismiss the sync notice" + > + {notice} + + )} + {actionError && ( +

+ {actionError} +

+ )} + {canManage && state === "connected" && ( + + )} + {canManage && state === "error" && ( + void appSetup.begin(beginAppSetup, orgId) + : undefined + } + onSubmit={async (credential) => { + setReconnectPending(true); + setReconnectError(undefined); + setActionError(null); + const token = + credential.method === "pat" + ? credential.accessToken + : undefined; + try { + const jobId = reconnectSource + ? await reconnectSource(orgId, source.id, token) + : await client.syncSource(orgId, source.id, token); + setNotice( + "Reconnected. Sync queued; ingestion runs in the background.", + ); + onSyncEnqueued?.(jobId); + await list.refetch(); + } catch (error) { + setReconnectError(messageOf(error)); + } finally { + setReconnectPending(false); + } + }} + /> + )} + {!canManage && state === "error" && ( +

+ An organization administrator reconnects this source. +

+ )} + + ) : null} + + ); +} + +type DeclaredSourceState = "setup" | "connected" | "error" | "ambiguous"; + +/** + * The three states the declaration can be in, plus the one it must never + * silently resolve. "Set up" is a state, not an instruction: it is what the + * card says when nothing connected serves the declaration, whether or not the + * reader can do anything about it. + */ +function StateBadge({ + state, + source, +}: { + state: DeclaredSourceState; + source: DatasourceView | undefined; +}) { + // Every state carries a dot, so the three are told apart at a glance by + // shape as well as by colour. Connected does NOT use `secondary`: it reads + // as neutral beside an outline "Set up", which leaves a reader unable to + // tell a source that is working from one nobody has connected yet — the + // opposite of what this card exists to say. (The kit has no `success` + // token to tint it with; see the note in the PR.) + if (state === "connected") + return ( + + Connected + + ); + if (state === "setup") + return ( + + Set up + + ); + if (state === "ambiguous") + return ( + + More than one source + + ); + return ( + + Error{source?.status === "paused" ? " · paused" : ""} + + ); +} + +/** What the solution declared, as text. Never a control: it is not editable. */ +function Declaration({ declared }: { declared: DeclaredSource }) { + const paths = declared.paths ?? []; + return ( +
+
Repository
+
{declared.repo}
+
Paths
+
+ {paths.length ? paths.join(", ") : "The whole repository"} +
+
Branch
+
+ {declared.ref || "The repository default"} +
+
+ ); +} + +/** + * Two connected sources serve one declaration. Reported rather than resolved: + * picking one would show its status and sync its content while the other kept + * ingesting invisibly, and the two can disagree about everything that matters + * — credential, branch, scope, health. + */ +function AmbiguousMatches({ + declared, + matches, +}: { + declared: DeclaredSource; + matches: DatasourceView[]; +}) { + return ( + +

+ {`${matches.length} connected sources read `} + {declared.repo} + {`, so this card cannot say which one serves the solution. An organization administrator should remove the ones that do not, in Admin → Data sources.`} +

+ {/* Each one by its id: every other column can be identical, and the + id is what names the row to remove in Admin → Data sources. */} +
    + {matches.map((match) => ( +
  • + {`${match.id} · ${match.branch || "default branch"} · ${ + match.paths.length + ? match.paths.join(", ") + : "whole repository" + } · ${match.status}`} +
    • + ))} +
+ + ); +} + +/** How the connected source is doing, in the host's own words. */ +function SourceState({ source }: { source: DatasourceView }) { + const ingest = formatIngest(source.lastIngestedAt, source.lastIngestedCommit); + const delivery = formatLiveDelivery(source.liveDelivery); + const reconcile = formatReconcileInterval(source.reconcileIntervalSeconds); + return ( +
+ {/* At most one of the two clocks ticks for a given source: a github + source's ingest is advanced by the change-set compiler and never + sets lastSyncedAt. Showing "Never" above live provenance would tell + a reader a healthy source has never synced. */} +

{ingest ?? `Last sync ${formatSyncedAt(source.lastSyncedAt)}`}

+ {delivery === undefined ? ( +

+ This host does not report how live updates reach this source. +

+ ) : ( +

+ {delivery} + {reconcile ? ` · ${reconcile}` : ""} +

+ )} + {/* Verbatim. The host writes status_reason from a closed set of named + reasons — never the tenant, never raw provider error text — so it is + safe to render as it arrives, and rewording it would lose the one + thing a reader can act on. */} + {source.statusReason &&

{source.statusReason}

} + {source.status === "unknown" && ( +

+ This host does not report this source's state. It is connected; + whether its last pull succeeded is not something this page can say. +

+ )} + {source.status === "degraded" && ( +

+ Scheduled pulls have stopped; reconnecting retries once the cause is + fixed. Content already ingested stays readable. +

+ )} +
+ ); +} + +/** What the person supplies: the credential, and nothing else. */ +interface CredentialSubmission { + method: CredentialMethod; + accessToken: string; + webhookSecret: string; +} + +function CredentialForm({ + heading, + intro, + submitLabel, + pendingLabel, + pending, + error, + offerWebhookSecret = false, + appAvailable, + appDescription, + appInstalled, + appSetupPhase, + appSetupError, + onBeginAppSetup, + onSubmit, +}: { + heading: string; + intro: string; + submitLabel: string; + pendingLabel: string; + pending: boolean; + error?: string; + offerWebhookSecret?: boolean; + appAvailable: boolean; + appDescription: string; + appInstalled: boolean; + appSetupPhase?: "beginning" | "completing"; + appSetupError?: string; + onBeginAppSetup?: () => void; + onSubmit: (credential: CredentialSubmission) => void; +}) { + const fieldId = useId(); + const idFor = (name: string) => `${fieldId}-${name}`; + const [method, setMethod] = useState( + appAvailable ? "app" : "pat", + ); + const [accessToken, setAccessToken] = useState(""); + const [webhookSecret, setWebhookSecret] = useState(""); + // The host refuses a connect with no credential on the PAT path, so the + // button is not offered for a submission that cannot succeed. + const incomplete = method === "pat" && !accessToken.trim(); + + return ( +
{ + event.preventDefault(); + if (pending || incomplete) return; + onSubmit({ + method, + accessToken: accessToken.trim(), + webhookSecret: webhookSecret.trim(), + }); + }} + > +
+

{heading}

+

{intro}

+
+ + + + {method === "app" && onBeginAppSetup && ( + <> + + {appInstalled && ( +

+ The installation is in place. Connect to finish. +

+ )} +

+ If this organization already installed the App on this repository, + there is nothing to install — continue. +

+ + )} + + {method === "pat" && ( + setAccessToken(event.target.value), + }} + /> + )} + + {offerWebhookSecret && method !== "public" && ( + setWebhookSecret(event.target.value), + }} + /> + )} + + {error && ( +

+ {error} +

+ )} + + + + ); +} + +/** + * The GitHub App's return leg, redeemed once. + * + * Same shape as the panel's: the parameters are captured in a lazy initializer + * because they are a property of the address the page loaded with and the + * scrub below destroys them; redemption is a `useQuery` that never retries, + * because the state is single-use and a retry would report a rejection for an + * installation that in fact succeeded; and whether the viewer may redeem is + * decided every render rather than frozen, since the credential `canManage` is + * read from may not have arrived on the load that follows the redirect. + */ +function useAppSetupReturn( + orgId: string, + canManage: boolean, + completeAppSetup: DatasourceClient["completeGitHubAppSetup"] | undefined, +): { + installed: boolean; + /** A return leg this viewer may not redeem, so nothing on screen will. */ + unredeemable: boolean; + phase: "beginning" | "completing" | undefined; + error: string | undefined; + begin: ( + beginAppSetup: NonNullable, + orgId: string, + ) => Promise; +} { + const [appSetupReturn] = useState(() => { + if (!completeAppSetup) return null; + const params = readAppSetupReturn(); + return params && { ...params, orgId }; + }); + const [beginPending, setBeginPending] = useState(false); + const [beginError, setBeginError] = useState(); + const claimed = !!appSetupReturn && appSetupReturn.orgId === orgId; + const active = claimed && canManage; + const setup = useQuery({ + queryKey: [ + "github-app-setup", + appSetupReturn?.orgId, + appSetupReturn?.state, + ], + queryFn: () => + completeAppSetup!( + appSetupReturn!.orgId, + appSetupReturn!.state, + appSetupReturn!.installationId, + appSetupReturn!.code, + ), + enabled: active, + retry: false, + staleTime: Number.POSITIVE_INFINITY, + refetchOnMount: false, + refetchOnWindowFocus: false, + }); + useEffect(() => { + if (appSetupReturn) scrubAppSetupReturn(); + }, [appSetupReturn]); + + return { + installed: active && setup.isSuccess, + unredeemable: claimed && !canManage, + phase: beginPending + ? "beginning" + : active && setup.isFetching + ? "completing" + : undefined, + error: + beginError ?? + (active && setup.isError ? messageOf(setup.error) : undefined), + begin: async (beginAppSetup, targetOrgId) => { + setBeginError(undefined); + setBeginPending(true); + try { + const handle = await beginAppSetup(targetOrgId); + // Navigating away, so the pending flag is deliberately left set: the + // button must not re-enable under a browser that is already + // unloading. + window.location.assign(handle.installUrl); + } catch (error) { + setBeginError(messageOf(error)); + setBeginPending(false); + } + }, + }; +} diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source.ts b/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source.ts new file mode 100644 index 000000000..29a869840 --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source.ts @@ -0,0 +1,84 @@ +import type { DatasourceView } from "./types.js"; + +/** + * A source a solution declares it is built on, as its registration manifest + * states it (`sources:`) and as the host hands it back to the mounted remote + * (`SolutionBinding.declaredSources`). + * + * The point of a declaration is that the repository is NOT a question put to + * the person. A solution that reads one known repository already knows which + * one; all that is left to supply is the credential. So this type is read, it + * is never edited by a surface, and nothing in it is defaulted from what the + * person typed. + * + * It is not a permission and not a connection: declaring a source grants + * nothing and connects nothing. It is only the statement "this is the source + * my solution is built on", against which the org's connected sources can be + * matched. + */ +export interface DeclaredSource { + /** Only github today — the provider whose connector takes a repo + paths. */ + provider: "github"; + /** "owner/name", the same shape `AddGitHubSourceRequest.repo` takes. */ + repo: string; + /** Repository-relative path prefixes. Empty or absent means the whole repo. */ + paths?: string[]; + /** Git ref to read. Absent resolves to the repository's default branch. */ + ref?: string; + /** + * What to call this source where a person sees it, and — because a + * connected source has to land somewhere — the collection the host mints or + * reuses for its entries. Absent falls back to the repository name. + */ + label?: string; +} + +/** + * GitHub treats `owner/name` case-insensitively, so two sources whose repos + * differ only in case are one repository and must match each other. Comparing + * them literally would show a declaration as unconnected beside the very + * source that serves it. + */ +function sameRepo(a: string, b: string): boolean { + return a.toLowerCase() === b.toLowerCase(); +} + +/** Set equality over path prefixes; order is not meaningful in either place. */ +function samePaths(declared: string[], connected: string[]): boolean { + if (declared.length !== connected.length) return false; + const remaining = new Set(connected); + for (const path of declared) { + if (!remaining.delete(path)) return false; + } + return true; +} + +/** + * Every connected source that serves a declaration: same provider, same + * repository, and — only when the declaration names paths — the same set of + * path prefixes. A declaration that names no paths matches the repository + * however it was scoped, because it has asserted nothing about scope. + * + * Returns ALL of them rather than the first, deliberately. Two sources for one + * declared repository is a real state an organization can reach (one connected + * through the App, one through a PAT; one scoped to `docs/`, one to the whole + * repo), and picking one silently would show its status, sync its content and + * leave the other invisible — so a caller reports the ambiguity instead. + */ +export function matchDeclaredSources( + declared: DeclaredSource, + sources: readonly DatasourceView[], +): DatasourceView[] { + const paths = declared.paths ?? []; + return sources.filter( + (source) => + source.provider === declared.provider && + sameRepo(source.repo, declared.repo) && + (paths.length === 0 || samePaths(paths, source.paths)), + ); +} + +/** The collection a declared source's entries land in. */ +export function declaredCollectionLabel(declared: DeclaredSource): string { + return declared.label?.trim() || declared.repo; +} diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/errors.ts b/module/services/frontend/code/packages/saas-ui/src/datasources/errors.ts new file mode 100644 index 000000000..f036a4f76 --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/errors.ts @@ -0,0 +1,19 @@ +import { ConnectError } from "@connectrpc/connect"; + +/** + * A failed call in words a person can read: the host's own message, with the + * gRPC envelope (`rpc error: code = … desc = `) stripped. + * + * One definition, because every datasource surface renders the same failures — + * a refused credential, an unreachable host — and a second copy drifts into + * showing the raw envelope to half of them. + */ +export function messageOf(error: unknown): string { + const message = + error instanceof ConnectError + ? error.rawMessage + : error instanceof Error + ? error.message + : "unexpected error"; + return message.replace(/^rpc error: code = \w+ desc = /, ""); +} diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/github-app-setup.ts b/module/services/frontend/code/packages/saas-ui/src/datasources/github-app-setup.ts new file mode 100644 index 000000000..7888d1e58 --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/github-app-setup.ts @@ -0,0 +1,54 @@ +/** + * The return leg of a GitHub App installation, shared by every surface that + * can start one — `` and ``. + * + * Both halves belong together and to one definition: the state is redeemable + * exactly once, so a surface that read the parameters but scrubbed them + * differently (or not at all) would leave a single-use state and an + * authorization code in the address bar, in history and in same-origin + * referrers. + * + * Only one surface that redeems the return leg may be mounted on a page. Two + * would race for the same single-use state and one of them would report a + * rejection for an installation that in fact succeeded. + */ + +/** + * The parameters GitHub appends to the App's configured setup URL when it sends + * the browser back: the installation it claims was installed, and the state we + * minted. Both are required — `setup_action` is deliberately not consulted, so + * an existing installation gaining repositories (`update`) lands here exactly as + * a first install does. + * + * Returns null under SSR, where a surface renders before any address exists. + */ +export function readAppSetupReturn(): { + state: string; + installationId: string; + code: string; +} | null { + if (typeof window === "undefined") return null; + const params = new URLSearchParams(window.location.search); + const state = params.get("state"); + const installationId = params.get("installation_id"); + // `code` is deliberately not part of the trigger. It is absent when the App + // was registered without "Request user authorization (OAuth) during + // installation", and the host answers that with the error naming the setting + // — which an operator can act on, where ignoring the return says nothing. + return state && installationId + ? { state, installationId, code: params.get("code") ?? "" } + : null; +} + +/** Burns the single-use parameters out of the address bar, history and referrers. */ +export function scrubAppSetupReturn(): void { + const params = new URLSearchParams(window.location.search); + for (const key of ["state", "installation_id", "setup_action", "code"]) + params.delete(key); + const query = params.toString(); + window.history.replaceState( + null, + "", + `${window.location.pathname}${query ? `?${query}` : ""}${window.location.hash}`, + ); +} diff --git a/module/services/frontend/code/packages/saas-ui/src/datasources/util.ts b/module/services/frontend/code/packages/saas-ui/src/datasources/util.ts index 694075e3f..876e43029 100644 --- a/module/services/frontend/code/packages/saas-ui/src/datasources/util.ts +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/util.ts @@ -1,3 +1,5 @@ +import type { DatasourceLiveDeliveryName } from "./types.js"; + // A tiny classnames joiner, inlined so the kit carries no dependency (truthy // joining only, no tailwind-merge conflict resolution). export function cn(...parts: Array): string { @@ -79,3 +81,37 @@ export function formatGrants(actions: string[]): string { .map((action) => action.charAt(0).toUpperCase() + action.slice(1)) .join(" · "); } + +/** + * The reconcile schedule in words. Undefined when the host does not report it; + * an interval of 0 means the periodic reconcile is off for this source, which + * — with no live delivery — leaves "Sync now" as the only thing that ever + * refreshes it, and is worth saying rather than leaving blank. + */ +export function formatReconcileInterval(seconds: number | undefined): string { + if (seconds === undefined) return ""; + if (seconds <= 0) return "No periodic reconcile; refreshed by Sync now only"; + if (seconds % 3600 === 0) { + const hours = seconds / 3600; + return `Otherwise reconciled every ${hours} ${hours === 1 ? "hour" : "hours"}`; + } + const minutes = Math.max(1, Math.round(seconds / 60)); + return `Otherwise reconciled every ${minutes} ${minutes === 1 ? "minute" : "minutes"}`; +} + +/** + * How a change at the source reaches this deployment, in words. Undefined when + * the host reports nothing — an older host — where the honest answer is that + * nothing here knows, and a caller must say so rather than present "no live + * updates" as an answer it was never given. + */ +export function formatLiveDelivery( + delivery: DatasourceLiveDeliveryName | undefined, +): string | undefined { + if (delivery === undefined) return undefined; + return delivery === "source_webhook" + ? "On push, through this source's webhook" + : delivery === "app_webhook" + ? "On push, through the GitHub App" + : "No live updates"; +} diff --git a/module/services/frontend/code/packages/saas-ui/src/index.ts b/module/services/frontend/code/packages/saas-ui/src/index.ts index 989a900f5..abf894261 100644 --- a/module/services/frontend/code/packages/saas-ui/src/index.ts +++ b/module/services/frontend/code/packages/saas-ui/src/index.ts @@ -7,6 +7,31 @@ export { DatasourcesPanel, type DatasourcesPanelProps, } from "./datasources/datasources-panel.js"; +// The credential block, presentational and controlled, exported for the same +// reason `matchDeclaredSources` is: a consumer that renders the three states +// in its own shell must be able to ASK for the credential without copying +// `ConnectGitHubForm`. `solutions/README.md` refuses a copied capability in a +// shared card, so offering the matching rule while withholding the one control +// a caller cannot write for itself would leave the only documented path out of +// the whole card a path across that boundary. +export { + AccessTokenField, + AppInstallPrompt, + type CredentialMethod, + CredentialMethodField, + credentialMethodFrom, + fieldErrorClass, + WebhookSecretField, +} from "./datasources/credential-mode.js"; +export { + type DeclaredSource, + declaredCollectionLabel, + matchDeclaredSources, +} from "./datasources/declared-source.js"; +export { + DeclaredSourceCard, + type DeclaredSourceCardProps, +} from "./datasources/declared-source-card.js"; export { DatasourceAccountLinks, DatasourceDirectoryPanel, diff --git a/module/services/frontend/code/packages/saas-ui/src/solution/binding.ts b/module/services/frontend/code/packages/saas-ui/src/solution/binding.ts index 29240b761..9721afb15 100644 --- a/module/services/frontend/code/packages/saas-ui/src/solution/binding.ts +++ b/module/services/frontend/code/packages/saas-ui/src/solution/binding.ts @@ -1,8 +1,11 @@ +import type { DeclaredSource } from "../datasources/declared-source.js"; + /** - * What the host hands every solution remote it mounts — the part of - * `SolutionPageProps` a remote's own code reaches its backend with. The host's - * `SolutionPageProps` extends this interface, so the remote and the host read - * one definition rather than two copies that drift. + * What the host hands every solution remote it mounts — what a remote's own + * code reaches its backend with, plus the host's copy of what this solution + * declared about itself. The host's `SolutionPageProps` extends this + * interface, so the remote and the host read one definition rather than two + * copies that drift. * * Widening this is a minor of the host↔remote contract; making a field * required that a remote already receives optionally, or removing one, is a @@ -56,6 +59,24 @@ export interface SolutionBinding { input: RequestInfo | URL, init?: RequestInit, ) => Promise; + /** + * The sources this solution declared it is built on, as the host validated + * and stored them at registration (`sources:` in the registration + * manifest). The host hands the declaration straight back rather than + * making the remote restate it in code: one statement, validated once, + * where an operator reading the registration and the person reading the + * page see the same repository. + * + * A declaration is not a connection and not a permission. It says which + * repository the solution reads; whether the organization has connected it + * is answered by the host's own source list, which is what + * `` reads. + * + * Optional, and absent from an older host: a remote reading it must treat + * absence as "the host does not tell me", never as "this solution declares + * nothing". + */ + declaredSources?: DeclaredSource[]; } /** diff --git a/module/services/frontend/code/packages/saas-ui/src/solution/index.ts b/module/services/frontend/code/packages/saas-ui/src/solution/index.ts index aab2b3d7d..433c04c0c 100644 --- a/module/services/frontend/code/packages/saas-ui/src/solution/index.ts +++ b/module/services/frontend/code/packages/saas-ui/src/solution/index.ts @@ -1,3 +1,4 @@ +export type { DeclaredSource } from "../datasources/declared-source.js"; export { requestBinding, type SolutionBinding, diff --git a/module/services/frontend/code/packages/saas-ui/stories/datasources.stories.tsx b/module/services/frontend/code/packages/saas-ui/stories/datasources.stories.tsx index e7f768b60..242c22013 100644 --- a/module/services/frontend/code/packages/saas-ui/stories/datasources.stories.tsx +++ b/module/services/frontend/code/packages/saas-ui/stories/datasources.stories.tsx @@ -5,6 +5,8 @@ import { type DatasourceClient, DatasourcesPanel, type DatasourceView, + DeclaredSourceCard, + type DeclaredSource, describeSync, SourceExecutionRestricted, SourceSyncProgress, @@ -237,3 +239,77 @@ export const ConnectionPending = { export const ExecutionRestricted = { render: () => , }; + +/** + * A solution that declares the one repository it is built on: the repository + * is shown, never asked for, and the three states are the whole of what a + * person can be told about it. + */ +const declaredSource: DeclaredSource = { + provider: "github", + repo: "example-org/handbook", + paths: ["proposals/"], + ref: "main", + label: "Proposals", +}; +const declaredConnected: DatasourceView = { + ...source, + id: "example-declared-source", + repo: "example-org/handbook", + paths: ["proposals/"], + liveDelivery: "app_webhook", + reconcileIntervalSeconds: 3600, + lastIngestedAt: new Date(Date.now() - 42 * 60_000).toISOString(), + lastIngestedCommit: "9f2c1ab0d4e5", +}; +function DeclaredPreview({ + state, +}: { + state: "setup" | "connected" | "error"; +}) { + const [queryClient] = useState( + () => new QueryClient({ defaultOptions: { queries: { retry: false } } }), + ); + const [client] = useState(() => ({ + listSources: async () => + state === "setup" + ? [] + : [ + state === "error" + ? { + ...declaredConnected, + status: "degraded" as const, + statusReason: + "the stored credential was rejected by GitHub (401) on the last 3 attempts", + } + : declaredConnected, + ], + addGitHubSource: async () => { + throw new Error("Connection is unavailable in this preview."); + }, + syncSource: async () => { + throw new Error("Synchronization is unavailable in this preview."); + }, + deleteSource: async () => { + throw new Error("Deletion is unavailable in this preview."); + }, + })); + return ( + + + + ); +} +export const DeclaredSourceSetUp = { + render: () => , +}; +export const DeclaredSourceConnected = { + render: () => , +}; +export const DeclaredSourceError = { + render: () => , +}; diff --git a/module/services/frontend/code/src/app/(dashboard)/s/[solutionId]/page.tsx b/module/services/frontend/code/src/app/(dashboard)/s/[solutionId]/page.tsx index 2714926c6..3b268d04e 100644 --- a/module/services/frontend/code/src/app/(dashboard)/s/[solutionId]/page.tsx +++ b/module/services/frontend/code/src/app/(dashboard)/s/[solutionId]/page.tsx @@ -45,6 +45,15 @@ export default async function SolutionPage({ // services, e.g. the DatasourceService `` // calls), anything else reaches this solution's upstream. apiBase: solutionProxyBase(solution.id), + // What this solution declared it is built on, handed straight + // back to it. The remote could hard-code the same repository, + // but then two statements would have to agree: the one the host + // validated at registration and an operator can read, and the + // one compiled into the bundle. Returning the validated + // declaration keeps it to one. + ...(solution.sources?.length + ? { declaredSources: solution.sources } + : {}), }} /> ); diff --git a/module/services/frontend/code/src/solutions/__tests__/registry.test.ts b/module/services/frontend/code/src/solutions/__tests__/registry.test.ts index 6c6bd290d..4e3ac98e0 100644 --- a/module/services/frontend/code/src/solutions/__tests__/registry.test.ts +++ b/module/services/frontend/code/src/solutions/__tests__/registry.test.ts @@ -15,6 +15,7 @@ vi.mock("codefly", () => ({ getEndpoints, getWorkspaceSecret })); import { browserManifestUrl, + detailProjection, findSolution, loadSolutions, parseManifest, @@ -314,6 +315,126 @@ describe("parseManifest client surfaces", () => { }); }); +describe("parseManifest declared sources", () => { + function source(overrides: Record = {}) { + return { + provider: "github", + repo: "example-org/handbook", + paths: ["proposals/"], + ref: "main", + label: "Proposals", + ...overrides, + }; + } + + it("keeps a well-formed declaration", () => { + const parsed = parseManifest(baseManifest({ sources: [source()] })); + expect(parsed?.sources).toEqual([ + { + provider: "github", + repo: "example-org/handbook", + paths: ["proposals/"], + ref: "main", + label: "Proposals", + }, + ]); + }); + + it("keeps a declaration that names only a repository", () => { + const parsed = parseManifest( + baseManifest({ + sources: [{ provider: "github", repo: "example-org/handbook" }], + }), + ); + expect(parsed?.sources).toEqual([ + { provider: "github", repo: "example-org/handbook" }, + ]); + }); + + it("adds no key to the stored bytes of a manifest that declares none", () => { + // Same reason as the surfaces slot: the registry recognises a + // re-registration as a lease renewal by byte identity, so a slot + // materialised as [] would turn every heartbeat into a content change. + const parsed = parseManifest(baseManifest()); + expect(parsed).not.toBeNull(); + expect(JSON.stringify(parsed)).not.toContain("sources"); + }); + + it("refuses a repository the connect RPC would refuse", () => { + // The pattern is AddGitHubSourceRequest.repo's. Refusing it at + // registration is the whole value of declaring it: otherwise the error + // arrives weeks later, when somebody presses Connect, and names a field + // they cannot see. + for (const repo of [ + "example-org", + "example-org/handbook/extra", + "example org/handbook", + "https://github.com/example-org/handbook", + "", + ]) { + expect( + parseManifest(baseManifest({ sources: [source({ repo })] })), + repo, + ).toBeNull(); + } + }); + + it("refuses a malformed declaration whole rather than dropping it", () => { + // A solution that meant to declare the repository it is built on should + // learn its declaration is invalid, not silently lose it and show its + // users a card asking them to pick a repository. + const broken: Array> = [ + { provider: "gitlab", repo: "example-org/handbook" }, + source({ paths: "proposals/" }), + source({ paths: [""] }), + source({ paths: Array.from({ length: 65 }, (_, i) => `p${i}/`) }), + source({ paths: ["x".repeat(513)] }), + source({ ref: "" }), + source({ ref: "x".repeat(256) }), + source({ label: 7 }), + // Blank, not merely empty. A ref of " " clears the connect RPC's bounds + // (its branch has a maximum and no minimum), so accepting it here would + // move the failure to GitHub, against a branch of that name — which is + // the failure this validation exists to prevent. Same for a label that + // would silently fall back to the repository, and a path prefix that + // matches nothing. + source({ ref: " " }), + source({ label: " " }), + source({ paths: ["proposals/", " "] }), + ]; + for (const entry of broken) { + expect( + parseManifest(baseManifest({ sources: [entry] })), + JSON.stringify(entry), + ).toBeNull(); + } + expect(parseManifest(baseManifest({ sources: "proposals" }))).toBeNull(); + }); + + it("refuses a manifest that declares one repository twice", () => { + // Nothing downstream could say which of the two a card renders, and the + // contradiction is the manifest's own — so it is refused here rather than + // reported to a reader who cannot fix it. + expect( + parseManifest( + baseManifest({ + sources: [source(), source({ repo: "Example-Org/Handbook" })], + }), + ), + ).toBeNull(); + }); + + it("keeps the declaration off the internal detail projection", () => { + // Like the dashboard graph: the solution page reads it in process and + // hands it to the remote it mounts, so no HTTP reader spends the bytes. + const parsed = parseManifest(baseManifest({ sources: [source()] })); + expect(parsed).not.toBeNull(); + expect(JSON.stringify(detailProjection(parsed!))).not.toContain( + "handbook", + ); + }); +}); + describe("browserManifestUrl", () => { function withManifestUrl(manifestUrl: string) { const manifest = baseManifest(); diff --git a/module/services/frontend/code/src/solutions/registry.ts b/module/services/frontend/code/src/solutions/registry.ts index 40bc1b453..73637edeb 100644 --- a/module/services/frontend/code/src/solutions/registry.ts +++ b/module/services/frontend/code/src/solutions/registry.ts @@ -1,6 +1,7 @@ import "server-only"; import { assertDataGraph, type DataGraph } from "@codefly/saas-plugin-manifest"; +import type { DeclaredSource } from "@codefly-dev/saas-ui/solution"; import { getEndpoints, getWorkspaceSecret } from "codefly"; import { isStandingConditionMilestone } from "@/solutions/registration-log"; @@ -74,6 +75,24 @@ export interface SolutionManifest { * offers nothing outside the host. */ surfaces?: SolutionSurface[]; + /** + * The external sources this solution is built on, declared once at + * registration instead of asked of every person who opens it. + * + * A solution built on one known repository already knows which repository; + * putting that question to the tenant is how a shared source manager, or a + * copied connect form, ends up inside a solution. The host validates the + * declaration here, stores it with the registration, and hands it back to + * the mounted remote as `SolutionBinding.declaredSources` — so the kit's + * `` can ask only for the credential, and this issue + * adds no RPC. + * + * It is a statement, not an authority: declaring a source connects + * nothing, grants nothing, and gives the solution no read of its contents. + * Whether the organization has connected it is answered by the host's own + * `DatasourceService`, against the viewer's own permissions. + */ + sources?: DeclaredSource[]; } /** @@ -105,7 +124,10 @@ export interface SolutionSurface { export type SolutionNav = Pick; /** The internal detail projection (see detailProjection). */ -export type SolutionDetail = Omit; +export type SolutionDetail = Omit< + SolutionManifest, + "dashboard" | "surfaces" | "sources" +>; /** The per-client surface projection (see surfacesProjection). */ export interface SolutionClientSurfaces { @@ -929,6 +951,111 @@ function parseSurfaces(value: unknown): SolutionSurface[] | null | undefined { return surfaces; } +/** + * The repository shape `AddGitHubSourceRequest.repo` enforces. Restated here, + * at the other end of the same journey, because this manifest is stored at + * registration and read by a card that submits it unedited: a repository the + * connect RPC would refuse must be refused when it is DECLARED, not when + * somebody finally presses Connect weeks later and the error names a field + * they cannot see. + */ +const DECLARED_REPO = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; + +/** + * The declared sources, or null when any one of them is malformed — the whole + * registration then fails closed, as it does for a dashboard or a surface. A + * solution that meant to declare the repository it is built on should learn + * its declaration is invalid, not silently lose it and show its users a card + * asking them to pick a repository. + * + * Bounds mirror the connect RPC's (64 paths, 512 characters each, a 255- + * character ref) for the same reason the pattern does. + */ +function parseDeclaredSources( + value: unknown, +): DeclaredSource[] | null | undefined { + if (value === undefined) { + return undefined; + } + if (!Array.isArray(value)) { + return null; + } + const sources: DeclaredSource[] = []; + const seen = new Set(); + for (const entry of value) { + if (typeof entry !== "object" || entry === null) { + return null; + } + const candidate = entry as Record; + if ( + candidate.provider !== "github" || + typeof candidate.repo !== "string" || + candidate.repo.length > 255 || + !DECLARED_REPO.test(candidate.repo) + ) { + return null; + } + // Blank is refused rather than trimmed away, here and for the label and + // the paths below. A ref of " " satisfies the connect RPC's bounds (its + // branch has a maximum and no minimum), so it would be accepted at + // registration, submitted by the card, accepted by the host, and fail + // at GitHub against a branch of that name — the late failure this + // validation exists to prevent. A declaration that means "the default + // branch" omits the field; one that holds only spaces is a mistake, and + // a mistake in configuration is reported, not silently repaired. + if ( + candidate.ref !== undefined && + (typeof candidate.ref !== "string" || + candidate.ref.trim() === "" || + candidate.ref.length > 255) + ) { + return null; + } + if ( + candidate.label !== undefined && + (typeof candidate.label !== "string" || + candidate.label.trim() === "" || + candidate.label.length > 255) + ) { + return null; + } + let paths: string[] | undefined; + if (candidate.paths !== undefined) { + if ( + !Array.isArray(candidate.paths) || + candidate.paths.length > 64 || + candidate.paths.some( + (path) => + typeof path !== "string" || + path.trim() === "" || + path.length > 512, + ) + ) { + return null; + } + paths = [...(candidate.paths as string[])]; + } + // Two declarations of the same repository would leave a card with no way + // to say which one it renders — the same ambiguity the card refuses to + // resolve when an ORGANIZATION has connected a repository twice, except + // that here the solution's own manifest is the thing contradicting + // itself, so it is refused rather than reported. + const key = candidate.repo.toLowerCase(); + if (seen.has(key)) { + return null; + } + seen.add(key); + sources.push({ + provider: "github", + repo: candidate.repo, + ...(paths ? { paths } : {}), + ...(candidate.ref ? { ref: candidate.ref as string } : {}), + ...(candidate.label ? { label: candidate.label as string } : {}), + }); + } + return sources; +} + /** Minimal structural validation of a self-registration payload. */ export function parseManifest(value: unknown): SolutionManifest | null { if (typeof value !== "object" || value === null) { @@ -983,6 +1110,10 @@ export function parseManifest(value: unknown): SolutionManifest | null { if (surfaces === null) { return null; } + const sources = parseDeclaredSources(candidate.sources); + if (sources === null) { + return null; + } // A declared major must be a real major; an ABSENT one means "built against // the contract that existed when the field appeared", which is the only claim // the host can act on. @@ -1032,5 +1163,6 @@ export function parseManifest(value: unknown): SolutionManifest | null { }, dashboard, surfaces, + sources, }; } diff --git a/module/services/frontend/code/ui-inventory.json b/module/services/frontend/code/ui-inventory.json index 5023e20e8..c45bb2211 100644 --- a/module/services/frontend/code/ui-inventory.json +++ b/module/services/frontend/code/ui-inventory.json @@ -1250,6 +1250,19 @@ "coverage": "owner stories; browser/skin verification pending", "remaining": "Verify explorer rendering, interactive accessibility and visual baselines under external skins." }, + { + "source": "packages/saas-ui/src/datasources/credential-mode.tsx", + "imports": [ + "@codefly-dev/ui/layout", + "react" + ], + "owner": "@codefly-dev/saas-ui", + "disposition": "already canonical", + "reason": "The three credential modes a GitHub source can be connected with, and the fields each needs. Shared so the full connect form and the declared-source card ask the same question in the same words; a second copy would teach two products.", + "stories": [], + "coverage": "missing", + "remaining": "Exercised today only through its two consumers' stories (ConnectionValidation, DeclaredSourceSetUp). Add a story per mode once the explorer can drive a controlled select, then verify under external skins." + }, { "source": "packages/saas-ui/src/datasources/datasources-panel.tsx", "imports": [ @@ -1296,6 +1309,42 @@ "coverage": "owner stories; browser/skin verification pending", "remaining": "Verify explorer rendering, interactive accessibility and visual baselines under external skins." }, + { + "source": "packages/saas-ui/src/datasources/declared-source-card.tsx", + "imports": [ + "@codefly-dev/ui/layout", + "@tanstack/react-query", + "react", + "../solution/viewer.js", + "./credential-mode.js", + "./declared-source.js", + "./errors.js", + "./gateway.js", + "./github-app-setup.js", + "./queries.js", + "./types.js", + "./util.js" + ], + "owner": "@codefly-dev/saas-ui", + "disposition": "already canonical", + "reason": "One source a solution declares it is built on, in one of three states: Set up, Connected, Error. The repository is the solution's statement, never a question put to the person, so the card renders it and submits it and offers no control that could change it.", + "stories": [ + { + "file": "packages/saas-ui/stories/datasources.stories.tsx", + "export": "DeclaredSourceSetUp" + }, + { + "file": "packages/saas-ui/stories/datasources.stories.tsx", + "export": "DeclaredSourceConnected" + }, + { + "file": "packages/saas-ui/stories/datasources.stories.tsx", + "export": "DeclaredSourceError" + } + ], + "coverage": "owner stories and unit tests; browser/skin verification pending", + "remaining": "Verify explorer rendering and the three states under external skins, and the GitHub App return leg against a real installation." + }, { "source": "packages/saas-ui/src/datasources/directory.tsx", "imports": [