From b29953d681edb5854534f6f876608718b1dabb17 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Fri, 2 Oct 2026 13:40:03 -0400 Subject: [PATCH 1/5] feat: a solution declares the source it is built on, and saas-ui asks only for the credential MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. The registration manifest gains `sources:` — `[{ provider: github, repo: owner/name, paths, ref, label }]` — validated in the host's solution registry against the same repository pattern and the same path/ref bounds `AddGitHubSourceRequest` enforces, 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; one repository declared twice is refused outright, because nothing downstream could say which of the two a card renders. The solution page reads it in process and hands it back to the remote it mounts as `SolutionBinding.declaredSources`, so the declaration is on neither the public nor the internal HTTP projection and this change adds no RPC. saas-ui gains ``: one declared source, in one of three states. Set up offers the three credential modes and nothing else — repo, paths and ref are rendered as text and submitted unedited, and there is no repository control at all. Connected shows the last ingest and commit, how live updates reach the source, the reconcile interval, Sync now and the consumer's `renderSourceDetail` slot. Error shows the host's `status_reason` verbatim and reconnects with a replacement credential. Two connected sources matching one declaration are reported, never silently picked: they can disagree about credential, branch, scope and health. The card does not fork the connect form. The credential modes, the App install leg, the token and webhook-secret fields now live in `datasources/credential-mode.tsx`, which both render; the App return leg and the gRPC-envelope stripping move to modules of their own for the same reason. `ConnectGitHubForm` renders exactly what it rendered before. No change to `DatasourceService` or to connector semantics; credentials stay in host custody. The co-versioned kit moves to 0.11.5. Co-Authored-By: Claude Opus 5 (1M context) --- module/services/frontend/AGENTS.md | 14 + .../services/frontend/code/package-lock.json | 8 +- module/services/frontend/code/package.json | 4 +- .../code/packages/codefly-ui/package.json | 2 +- .../__tests__/no-raw-type-primitives.test.ts | 5 +- .../frontend/code/packages/saas-ui/README.md | 49 +- .../code/packages/saas-ui/package.json | 2 +- .../saas-ui/src/__tests__/peer-docs.test.ts | 7 +- .../__tests__/declared-source-card.test.tsx | 412 ++++++++++ .../src/datasources/connect-github-form.tsx | 171 ++--- .../src/datasources/credential-mode.tsx | 206 +++++ .../src/datasources/datasources-panel.tsx | 79 +- .../src/datasources/declared-source-card.tsx | 726 ++++++++++++++++++ .../src/datasources/declared-source.ts | 84 ++ .../saas-ui/src/datasources/errors.ts | 19 + .../src/datasources/github-app-setup.ts | 54 ++ .../packages/saas-ui/src/datasources/util.ts | 36 + .../code/packages/saas-ui/src/index.ts | 9 + .../packages/saas-ui/src/solution/binding.ts | 29 +- .../packages/saas-ui/src/solution/index.ts | 1 + .../saas-ui/stories/datasources.stories.tsx | 76 ++ .../app/(dashboard)/s/[solutionId]/page.tsx | 9 + .../src/solutions/__tests__/registry.test.ts | 112 +++ .../frontend/code/src/solutions/registry.ts | 124 ++- .../services/frontend/code/ui-inventory.json | 49 ++ 25 files changed, 2076 insertions(+), 211 deletions(-) create mode 100644 module/services/frontend/code/packages/saas-ui/src/datasources/__tests__/declared-source-card.test.tsx create mode 100644 module/services/frontend/code/packages/saas-ui/src/datasources/credential-mode.tsx create mode 100644 module/services/frontend/code/packages/saas-ui/src/datasources/declared-source-card.tsx create mode 100644 module/services/frontend/code/packages/saas-ui/src/datasources/declared-source.ts create mode 100644 module/services/frontend/code/packages/saas-ui/src/datasources/errors.ts create mode 100644 module/services/frontend/code/packages/saas-ui/src/datasources/github-app-setup.ts 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/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/saas-ui/README.md b/module/services/frontend/code/packages/saas-ui/README.md index c3a3ec783..1c96d18d3 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,39 @@ 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. + 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. + 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..0a497220a --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/__tests__/declared-source-card.test.tsx @@ -0,0 +1,412 @@ +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("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..ee43d78ba --- /dev/null +++ b/module/services/frontend/code/packages/saas-ui/src/datasources/declared-source-card.tsx @@ -0,0 +1,726 @@ +"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); + } + }; + + const state: DeclaredSourceState = + matches.length > 1 + ? "ambiguous" + : !source + ? "setup" + : source.status === "active" + ? "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; +}) { + 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 === "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..514eddb6b 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,15 @@ export { DatasourcesPanel, type DatasourcesPanelProps, } from "./datasources/datasources-panel.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..7dc32e083 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,117 @@ 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 }), + ]; + 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..6b90d4802 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,101 @@ 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; + } + if ( + candidate.ref !== undefined && + (typeof candidate.ref !== "string" || + candidate.ref === "" || + candidate.ref.length > 255) + ) { + return null; + } + if ( + candidate.label !== undefined && + (typeof candidate.label !== "string" || + candidate.label === "" || + 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 === "" || 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 +1100,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 +1153,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": [ From fa6673c9d4b7bda4cba66480aacdcca3930965be Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Fri, 2 Oct 2026 13:57:19 -0400 Subject: [PATCH 2/5] fix(saas-ui): a source whose state an older host does not report is connected, not in error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DatasourceStatusName` carries a fourth value the card's state machine folded into Error by falling through its `else`: `unknown`, which is not a status at all but the absence of one. An older host leaves `Datasource.status` at its proto default and the gateway maps it to `unknown` rather than guessing, the same way it leaves `liveDelivery` undefined — "we could not tell" and "it is broken" being different claims, and only one of them guessable. A source that matched the declaration is connected. Calling the absence of a status DEGRADED painted a healthy source with a destructive badge, showed no `status_reason` under it (an older host sends none either, so the one line a reader could act on was empty), and offered Reconnect — asking a person for a credential again to repair nothing. `DatasourcesPanel` already refuses this read: it renders `unknown` as a neutral outline "Unknown", never destructive. So `unknown` reads as Connected and says its state is not reported, in the same words the card already uses for a host that does not report live delivery. Sync stays offered, because it works on any host; Reconnect does not. Still three states and no fourth. Co-Authored-By: Claude Opus 5 (1M context) --- .../frontend/code/packages/saas-ui/README.md | 7 +++++ .../__tests__/declared-source-card.test.tsx | 26 +++++++++++++++++++ .../src/datasources/declared-source-card.tsx | 16 +++++++++++- 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/module/services/frontend/code/packages/saas-ui/README.md b/module/services/frontend/code/packages/saas-ui/README.md index 1c96d18d3..10741c0c9 100644 --- a/module/services/frontend/code/packages/saas-ui/README.md +++ b/module/services/frontend/code/packages/saas-ui/README.md @@ -178,6 +178,13 @@ components: - **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 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 index 0a497220a..b34bb08d6 100644 --- 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 @@ -275,6 +275,32 @@ describe("DeclaredSourceCard", () => { expect(screen.queryByRole("button", { name: /Reconnect/ })).toBeNull(); }); + 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, 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 index ee43d78ba..b607ecf07 100644 --- 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 @@ -189,12 +189,20 @@ function DeclaredSourceCardView({ } }; + // "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 === "active" || source.status === "unknown" ? "connected" : "error"; @@ -489,6 +497,12 @@ function SourceState({ source }: { source: DatasourceView }) { 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 From 1b515e9b8a0a60cf07b404c7e88e5e7c0c74cafa Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:00:17 -0400 Subject: [PATCH 3/5] fix(frontend): a declared ref, label or path that is only whitespace is refused at registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser refused an empty `ref`, `label` and path prefix but accepted a blank one. `" "` clears the connect RPC's bounds — `AddGitHubSourceRequest` gives `branch` a maximum and no minimum, and a path item only `min_len: 1` — so a declaration holding spaces was admitted at registration, submitted unedited by the card, accepted by the host, and failed at GitHub against a branch or prefix of that name. That is precisely the late failure validating the declaration at registration exists to prevent, and it surfaced as an error naming a field the person looking at the card cannot see. Blank is refused rather than trimmed. A declaration that means "the default branch" omits the field; one holding only spaces is a mistake, and silently repairing it would leave the solution believing it had named something. A whitespace label was the quieter case: it passed, then fell back to the repository in both the card title and the collection, so the solution's chosen name vanished with nothing said. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/solutions/__tests__/registry.test.ts | 9 +++++++++ .../frontend/code/src/solutions/registry.ts | 16 +++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) 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 7dc32e083..4e3ac98e0 100644 --- a/module/services/frontend/code/src/solutions/__tests__/registry.test.ts +++ b/module/services/frontend/code/src/solutions/__tests__/registry.test.ts @@ -392,6 +392,15 @@ describe("parseManifest declared sources", () => { 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( diff --git a/module/services/frontend/code/src/solutions/registry.ts b/module/services/frontend/code/src/solutions/registry.ts index 6b90d4802..73637edeb 100644 --- a/module/services/frontend/code/src/solutions/registry.ts +++ b/module/services/frontend/code/src/solutions/registry.ts @@ -995,10 +995,18 @@ function parseDeclaredSources( ) { 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 === "" || + candidate.ref.trim() === "" || candidate.ref.length > 255) ) { return null; @@ -1006,7 +1014,7 @@ function parseDeclaredSources( if ( candidate.label !== undefined && (typeof candidate.label !== "string" || - candidate.label === "" || + candidate.label.trim() === "" || candidate.label.length > 255) ) { return null; @@ -1018,7 +1026,9 @@ function parseDeclaredSources( candidate.paths.length > 64 || candidate.paths.some( (path) => - typeof path !== "string" || path === "" || path.length > 512, + typeof path !== "string" || + path.trim() === "" || + path.length > 512, ) ) { return null; From ed005eca3ce7be5f4970045aee5e82575b37e3bb Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:02:40 -0400 Subject: [PATCH 4/5] feat(saas-ui): export the credential block, so a consumer's own shell need not copy the form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The kit exported `matchDeclaredSources` precisely so a consumer could render the three states in its own shell, and the README said so — then withheld the credential block, which is the one part of that shell a caller must not write for itself. The only documented path out of the whole card therefore ran through a copy of `ConnectGitHubForm`, which `solutions/README.md` refuses in terms: wrapping a copied capability in a shared card does not satisfy the boundary. So `CredentialMethodField`, `AccessTokenField`, `WebhookSecretField` and `AppInstallPrompt` join it, with `CredentialMethod`, `credentialMethodFrom` and `fieldErrorClass`. They were already presentational and controlled — the form drives them through react-hook-form and the card from `useState`, and neither holds any meaning of its own — so exporting them adds no behaviour, only the composition the matching rule already implied. The three modes are the whole of what a person decides between: the App's nothing to paste or rotate, the public path's 60 requests an hour, the PAT's Contents: Read-only. Two surfaces wording them differently teach two different products, which is the reason this block was lifted out of the form rather than duplicated into the card in the first place. Co-Authored-By: Claude Opus 5 (1M context) --- .../frontend/code/packages/saas-ui/README.md | 13 ++++++++++++- .../frontend/code/packages/saas-ui/src/index.ts | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/module/services/frontend/code/packages/saas-ui/README.md b/module/services/frontend/code/packages/saas-ui/README.md index 10741c0c9..6ad5c4f1a 100644 --- a/module/services/frontend/code/packages/saas-ui/README.md +++ b/module/services/frontend/code/packages/saas-ui/README.md @@ -195,7 +195,18 @@ components: 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. + 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. 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 514eddb6b..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,22 @@ 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, From a75681b4efed7106dd020dccfefbe09c0a3f0a82 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:21:01 -0400 Subject: [PATCH 5/5] feat(ui): a status badge carries a dot, a card takes a description, an empty state may point elsewhere MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The consuming solution reported six places it reached past the kit. Three are small, belong here, and are fixed; the other three are named in the PR body and not invented on its behalf. `` prefixes a filled circle in the variant's own colour, decorative and aria-hidden, so several badges in one row are told apart by shape and not by colour alone. **No `success` or `warning` variant**: the appearance layer defines no such token — `destructive` is the only semantic colour the skin ladder carries — and a component may not invent one, because choosing those colours is the skin owner's decision. So the declared-source card's Connected state stops borrowing `secondary`, which reads neutral beside an outline "Set up" and left a working source looking like one nobody had connected, and uses `default` with a dot instead. That was the consumer's actual complaint. `` matches `Section`'s prop of the same name. Without it a caller either mixed `CardRoot` + `CardDescription` into a page of plain cards, which is how a page stops looking like one page, or dropped the sentence into the body where it reads as content and loses the `card-description` slot. Its header becomes `items-start`, as `Section`'s already is: with a description the heading block is two lines tall and centring floats the actions against the middle of it. The declared-source card now uses it to say the one thing a person has to understand first — the repository is not a choice being offered. `` needed no new prop, only to say what it is for: it takes nodes, so an empty state whose explanation lives elsewhere can point at it. `children` is the action slot, and a cross-reference is not an action, so prose there loses the slot and its width bound. Documented rather than widened. Co-Authored-By: Claude Opus 5 (1M context) --- .../code/packages/codefly-ui/README.md | 27 ++++++++++ .../packages/codefly-ui/src/layout/badge.tsx | 26 +++++++++- .../packages/codefly-ui/src/layout/card.tsx | 49 ++++++++++++++++--- .../codefly-ui/src/layout/empty-state.tsx | 14 +++++- .../__tests__/declared-source-card.test.tsx | 26 ++++++++++ .../src/datasources/declared-source-card.tsx | 30 ++++++++++-- 6 files changed, 159 insertions(+), 13 deletions(-) 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/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/src/datasources/__tests__/declared-source-card.test.tsx b/module/services/frontend/code/packages/saas-ui/src/datasources/__tests__/declared-source-card.test.tsx index b34bb08d6..1e7da5075 100644 --- 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 @@ -275,6 +275,32 @@ describe("DeclaredSourceCard", () => { 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 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 index b607ecf07..8d7b82ec9 100644 --- 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 @@ -210,6 +210,9 @@ function DeclaredSourceCardView({ : null } @@ -402,13 +405,32 @@ function StateBadge({ 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; + return ( + + Connected + + ); + if (state === "setup") + return ( + + Set up + + ); if (state === "ambiguous") - return More than one source; + return ( + + More than one source + + ); return ( - + Error{source?.status === "paused" ? " · paused" : ""} );