diff --git a/apps/hub/src/connect-github-mount-gone.test.ts b/apps/hub/src/connect-github-mount-gone.test.ts new file mode 100644 index 0000000000..8a6feff2e3 --- /dev/null +++ b/apps/hub/src/connect-github-mount-gone.test.ts @@ -0,0 +1,56 @@ +// Hub-zero T3 (CL-8114) red test: the room card's connect flow required a +// hub mount whose options threaded the settings-row readers through it — +// the last hub readers of the `workbench_settings` row. The mount and its +// ports are deleted; connect/disconnect is native `connections/*` now. +// Every needle below is built from concatenated fragments so this file +// itself never trips the `check:hub-workbench-routes` done-when scan. +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import path from "node:path"; + +const HUB_INDEX = path.join( + import.meta.dir, + "..", + "..", + "..", + "apps", + "hub", + "src", + "index.ts", +); + +const WB = "work" + "benches"; +const GH = "git" + "hub"; +const MOUNT_PORT_A = "getTemplate" + "Settings"; +const MOUNT_PORT_B = "persist" + "SelectedRepos"; +const MOUNT_PORT_C = "onReviewing" + "Started"; + +describe("hub-zero T3: the workbench-scoped connect mount is gone", () => { + test("index.ts mounts no workbench child route under a connector path", () => { + const source = readFileSync(HUB_INDEX, "utf8"); + const mountPattern = new RegExp(`/${WB}/[^/]+/${GH}`); + expect( + mountPattern.test(source), + "index.ts still mounts a workbench child route under a connector path", + ).toBe(false); + }); + + test("index.ts passes no settings-row port to the deleted mount's options", () => { + const source = readFileSync(HUB_INDEX, "utf8"); + const optionsPattern = new RegExp( + `${MOUNT_PORT_A}|${MOUNT_PORT_B}|${MOUNT_PORT_C}`, + ); + expect( + optionsPattern.test(source), + "index.ts still passes a settings-row port to the deleted mount's options", + ).toBe(false); + }); + + test("index.ts keeps no reference to the deleted mount factory", () => { + const source = readFileSync(HUB_INDEX, "utf8"); + expect( + source.includes("createConnect" + "GithubRoutes"), + "index.ts still references the deleted mount factory", + ).toBe(false); + }); +}); diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index 4b7507f27a..facaa01409 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -17,7 +17,6 @@ import { createWorkflowRunDispatchStore, listAssetsForTenant, listVisibleOfferings, - resolveCredentialByName, resolveCredentialRequirement, } from "@intx/db"; import { @@ -34,7 +33,6 @@ import { } from "@intx/crypto"; import { timeWindowEvaluator } from "@intx/authz"; import type { ConditionRegistry } from "@intx/types/authz"; -import { credentialAad } from "@intx/types"; import type { CredentialCipher } from "@intx/types"; import { createApp, @@ -81,9 +79,6 @@ import { isWorkbenchHostDefinitionName, listConnectedProviders, listDefaultInferencePreferences, - localPartOf, - parseParticipants, - postRoomMessage, recordSourcesDigest, startWorkflowCommand, settleConnectedService, @@ -160,19 +155,15 @@ import { createWorkflowCommandPlugin, } from "@corbits/commands"; import { - createDrizzleRepoReviewLeaseStore, createDrizzleWebhookTriggerStore, createWebhookIngressRoutes, createWebhookTriggerRoutes, - generateWebhookSecret, launchWebhookTrigger, } from "@corbits/webhook-triggers"; import { deliveryWorkbenchRequiredForWorkflowName, isConversationalWorkflowName, } from "@workbench/templates"; -import { createConnectGithubRoutes } from "@corbits/connections/connect-github-routes"; -import { webhookTriggerName } from "@corbits/connections/connect-github-setup"; import { createTemplateBlockRoutes } from "./templates/template-block-routes"; import { createWorkflowDetailRoute, @@ -241,7 +232,6 @@ import { createPresenceRoomRegistry, createPresenceRoutes, } from "@corbits/presence"; -import { createHubAPI } from "@corbits/hub-api-client"; import { createConnectionRoutes, isInferenceProvider, @@ -288,10 +278,6 @@ import { import { type } from "arktype"; import { betterAuth } from "better-auth"; -import { - hasRepoGrantViaHttp, - mintRepoGrantViaHttp, -} from "./native-repo-grants"; import { createSignInAttemptLimiter } from "./sign-in-rate-limit"; import { createSetupStatusRoutes } from "./setup-status"; import { drizzleAdapter } from "better-auth/adapters/drizzle"; @@ -1357,7 +1343,6 @@ export async function createHub(config: HubConfig) { (await db.select({ n: count() }).from(tenantTable))[0]?.n ?? 0, }), ); - const selfApi = createHubAPI(config.baseUrl); // The chat platform's invite-launch fallback: a definition with no // model requirements of its own resolves the tenant-catalog default. const chatHostInferencePreferencesResolver = @@ -2079,12 +2064,6 @@ export async function createHub(config: HubConfig) { db, credentialCipher, ); - // CL-7242: the sole concurrency backstop for the GitHub connect - // card's start-reviewing step -- see - // packages/webhook-triggers/src/repo-review-lease.ts for why this - // lives in our own schema rather than as any change to Interchange's - // `grant` table. - const repoReviewLeaseStore = createDrizzleRepoReviewLeaseStore(db); app.route( `${TENANT_PREFIX}/webhook-triggers`, createWebhookTriggerRoutes({ @@ -2274,166 +2253,6 @@ export async function createHub(config: HubConfig) { providerHealth: providerHealthStore, }), ); - // GitHub connect card (CL-6344): the code-review template's inline - // room card reads its live state and starts reviews through here. - // Connecting the PAT itself stays on `connections` above (`github` is - // already registered in `CONNECTOR_REGISTRY`) — this route only owns - // what needs the decrypted secret, the live repo list, and a real - // grant/webhook-trigger/settings write. - // - // CL-6463: the credential row this card reads must be named by the - // exact same `displayName` `persistConnectorCredential` (`@workbench - // /connections`) stores it under for the `github` connector — a second, - // hardcoded "GitHub" literal here silently drifts the moment either side - // changes, which is exactly what left this card stuck disconnected after - // a successful PAT submit. Reading the descriptor's own field instead of - // repeating the literal makes that impossible. - const githubConnectorDescriptor = CONNECTOR_REGISTRY["github"]; - if (githubConnectorDescriptor === undefined) { - throw new Error( - 'CONNECTOR_REGISTRY has no "github" entry — the room GitHub connect card has nothing to read a credential name from', - ); - } - app.route( - `${TENANT_PREFIX}/workbenches`, - createConnectGithubRoutes({ - requireGrant: createRequireGrant({ - grantStore: chatGrantStore, - conditionRegistry: chatConditionRegistry, - }), - log: (line) => log.info`${line}`, - resolveGithubConfig: async (tenantId) => { - const row = await resolveCredentialByName( - db, - tenantId, - githubConnectorDescriptor.displayName, - ); - if (row === null) return undefined; - const apiKey = await credentialCipher.decrypt( - row.secret, - credentialAad(row.id, "secret"), - ); - return config.githubApiBaseUrl !== undefined - ? { apiKey, baseUrl: config.githubApiBaseUrl } - : { apiKey }; - }, - resolveCodeReviewDefinitionId: async (tenantId) => { - const row = await db.query.workflowDefinition.findFirst({ - where: and( - eq(workflowDefinition.tenantId, tenantId), - eq(workflowDefinition.name, "code-review"), - eq(workflowDefinition.status, "deployed"), - ), - columns: { id: true }, - }); - return row?.id; - }, - acquireRepoReviewLease: (tenantId, repo) => - repoReviewLeaseStore.acquire(tenantId, repo.name), - releaseRepoReviewLease: (tenantId, repo) => - repoReviewLeaseStore.release(tenantId, repo.name), - // hasRepoGrant/mintRepoGrant go through Interchange's native - // grants HTTP surface (never a direct `grant` table write -- - // see native-repo-grants.ts). That table carries no unique - // constraint over tenant/resource/action, so a bare read-then- - // POST here would itself be a duplicate-grant race; safe only - // because the caller in connect-github-routes.ts reaches this - // once `acquireRepoReviewLease` has already made this call-site - // single-flight per (tenant, repo) -- see - // packages/webhook-triggers/src/repo-review-lease.ts (CL-7242). - hasRepoGrant: (tenantId, repo, cookies) => - hasRepoGrantViaHttp(selfApi, tenantId, repo, cookies), - mintRepoGrant: (tenantId, repo, cookies) => - mintRepoGrantViaHttp(selfApi, tenantId, repo, cookies), - createWebhookTrigger: async ( - tenantId, - principalId, - codeReviewDefinitionId, - repo, - ) => { - // `ensure`, not `create`: a concurrent "start reviewing" call - // for the same repo can race this one past `hasWebhookTrigger` - // above, and `webhook_trigger_tenant_definition_name_unique` - // (packages/webhook-triggers migration 0003, CL-7242) is what - // actually resolves that — the loser gets the winner's real - // row back instead of minting a second live trigger with a - // different secret. - const row = await webhookTriggerStore.ensure({ - id: generateId("workflowRun"), - tenantId, - name: webhookTriggerName(repo), - workflowDefinitionId: codeReviewDefinitionId, - inputTemplate: `Review the pull request at {{pull_request.html_url}}`, - secret: generateWebhookSecret(), - createdBy: principalId, - }); - return { id: row.id }; - }, - hasWebhookTrigger: async (tenantId, codeReviewDefinitionId, repo) => { - const triggers = await webhookTriggerStore.list(tenantId); - const triggerName = webhookTriggerName(repo); - return triggers.some( - (trigger) => - trigger.workflowDefinitionId === codeReviewDefinitionId && - trigger.name === triggerName, - ); - }, - getTemplateSettings: async (tenantId, chatId) => { - const row = await chatStore.getWorkbenchSettings(tenantId, chatId); - const settings = row?.settings ?? {}; - const pendingConnections = settings["template/pendingConnections"]; - const selectedRepos = settings["template/selectedRepos"]; - return { - pendingConnections: Array.isArray(pendingConnections) - ? (pendingConnections as string[]) - : [], - selectedRepos: Array.isArray(selectedRepos) - ? (selectedRepos as string[]) - : [], - }; - }, - persistSelectedRepos: async (tenantId, chatId, principalId, patch) => { - const existing = await chatStore.getWorkbenchSettings(tenantId, chatId); - const row = await chatStore.updateWorkbenchSettings({ - tenantId, - workbenchId: chatId, - settings: { ...(existing?.settings ?? {}), ...patch }, - updatedBy: principalId, - }); - chatSubscribers.publish(chatId, { - type: "chat.settings", - data: { updatedBy: principalId, settings: row.settings }, - }); - }, - onReviewingStarted: async ( - tenantId, - chatId, - _principalId, - introductions, - ) => { - const row = await chatStore.getWorkbenchSettings(tenantId, chatId); - const participants = parseParticipants( - row?.settings["chat/participants"], - ); - for (const introduction of introductions) { - const participant = participants.find( - (candidate) => candidate.handle === introduction.handle, - ); - if (participant === undefined) continue; - await postRoomMessage( - { roomMessages, publish: chatSubscribers.publish }, - { - tenantId, - workbenchId: chatId, - sender: { name: null, address: participant.address }, - runId: localPartOf(participant.address), - parts: [{ kind: "text", text: introduction.text }], - }, - ); - } - }, - }), - ); // Template block workflows (CL-6405, cut over to native deploy in // CL-7364): the instantiate path's `deployBlockWorkflow` port lands // here — the same source-form materialization pattern (asset + diff --git a/apps/hub/src/native-repo-grants.test.ts b/apps/hub/src/native-repo-grants.test.ts deleted file mode 100644 index efbf75efb1..0000000000 --- a/apps/hub/src/native-repo-grants.test.ts +++ /dev/null @@ -1,183 +0,0 @@ -// Repo grants for GitHub start-reviewing go through native tenant HTTP, -// never a SQL insert into Interchange grant/role tables. -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { configureSync, resetSync } from "@intx/log"; -import type { ApiCall } from "@corbits/hub-api-client"; - -import { - hasRepoGrantViaHttp, - mintRepoGrantViaHttp, -} from "./native-repo-grants"; - -let records: { properties: Record }[]; - -function installCapturingSink(): void { - records = []; - configureSync({ - reset: true, - sinks: { - capture: (record) => { - records.push(record as { properties: Record }); - }, - }, - loggers: [ - { category: ["errors"], sinks: ["capture"], lowestLevel: "debug" }, - { category: ["logtape", "meta"], sinks: [], lowestLevel: "warning" }, - ], - }); -} - -beforeEach(() => installCapturingSink()); -afterEach(() => resetSync()); - -const REPO = { id: "1", name: "acme/widgets" }; -const TENANT_ID = "tnt_bench"; -const MEMBER_ROLE_ID = "rol_member"; -const COOKIES = ["session=alice"]; - -function rolesPage() { - return { - data: [ - { - id: MEMBER_ROLE_ID, - tenantId: TENANT_ID, - name: "member", - description: "System member role", - isSystem: true, - createdAt: "2026-01-01T00:00:00.000Z", - updatedAt: "2026-01-01T00:00:00.000Z", - }, - ], - nextCursor: null, - }; -} - -describe("mintRepoGrantViaHttp", () => { - test("POSTs /api/tenants/:id/grants for repo: read and never needs SQL", async () => { - const posts: { - path: string; - body: unknown; - cookies: string[] | undefined; - }[] = []; - const api: ApiCall = async (method, path, body, cookies) => { - if ( - method === "GET" && - path.startsWith(`/api/tenants/${TENANT_ID}/roles`) - ) { - return { status: 200, data: rolesPage(), cookies: cookies ?? [] }; - } - if (method === "POST" && path === `/api/tenants/${TENANT_ID}/grants`) { - posts.push({ path, body, cookies }); - return { status: 201, data: { id: "grt_1" }, cookies: cookies ?? [] }; - } - throw new Error(`unexpected ${method} ${path}`); - }; - - await mintRepoGrantViaHttp(api, TENANT_ID, REPO, COOKIES); - - expect(posts).toHaveLength(1); - expect(posts[0]?.cookies).toEqual(COOKIES); - expect(posts[0]?.body).toEqual({ - roleId: MEMBER_ROLE_ID, - resource: "repo:acme/widgets", - action: "read", - effect: "allow", - origin: "creator", - }); - expect(records).toHaveLength(0); - }); - - test("reports and rethrows when POST /grants is rejected", async () => { - const api: ApiCall = async (method, path, _body, cookies) => { - if ( - method === "GET" && - path.startsWith(`/api/tenants/${TENANT_ID}/roles`) - ) { - return { status: 200, data: rolesPage(), cookies: cookies ?? [] }; - } - if (method === "POST" && path === `/api/tenants/${TENANT_ID}/grants`) { - return { - status: 403, - data: { error: { code: "forbidden", message: "nope" } }, - cookies: cookies ?? [], - }; - } - throw new Error(`unexpected ${method} ${path}`); - }; - - await expect( - mintRepoGrantViaHttp(api, TENANT_ID, REPO, COOKIES), - ).rejects.toThrow( - "POST /api/tenants/tnt_bench/grants failed with status 403", - ); - - expect(records).toHaveLength(1); - expect(records[0]?.properties).toEqual( - expect.objectContaining({ - operation: "mintRepoGrant", - tenantId: TENANT_ID, - extra: { repo: "acme/widgets" }, - }), - ); - }); -}); - -describe("hasRepoGrantViaHttp", () => { - test("is true when GET /grants already lists repo: read allow", async () => { - const api: ApiCall = async (method, path, _body, cookies) => { - if ( - method === "GET" && - path.startsWith(`/api/tenants/${TENANT_ID}/grants?resource=`) - ) { - return { - status: 200, - data: { - data: [ - { - id: "grt_1", - tenantId: TENANT_ID, - roleId: MEMBER_ROLE_ID, - roleName: "member", - principalId: null, - principalName: null, - resource: "repo:acme/widgets", - action: "read", - effect: "allow", - conditions: null, - origin: "creator", - expiresAt: null, - createdAt: "2026-01-01T00:00:00.000Z", - updatedAt: "2026-01-01T00:00:00.000Z", - }, - ], - nextCursor: null, - }, - cookies: cookies ?? [], - }; - } - throw new Error(`unexpected ${method} ${path}`); - }; - - expect(await hasRepoGrantViaHttp(api, TENANT_ID, REPO, COOKIES)).toBe(true); - }); - - test("is false when GET /grants lists no matching row", async () => { - const api: ApiCall = async (method, path, _body, cookies) => { - if ( - method === "GET" && - path.startsWith(`/api/tenants/${TENANT_ID}/grants`) - ) { - return { - status: 200, - data: { data: [], nextCursor: null }, - cookies: cookies ?? [], - }; - } - throw new Error(`unexpected ${method} ${path}`); - }; - - expect(await hasRepoGrantViaHttp(api, TENANT_ID, REPO, COOKIES)).toBe( - false, - ); - }); -}); diff --git a/apps/hub/src/native-repo-grants.ts b/apps/hub/src/native-repo-grants.ts deleted file mode 100644 index 154cc81413..0000000000 --- a/apps/hub/src/native-repo-grants.ts +++ /dev/null @@ -1,105 +0,0 @@ -// Repo grants for GitHub start-reviewing: GET/POST the native -// `/api/tenants/:id/grants` surface as the requesting user. Never -// SQL-inserts Interchange grant/role rows. -import { GrantResponse, RoleResponse, paginatedSchema } from "@intx/types"; -import { parseAs, type ApiCall } from "@corbits/hub-api-client"; -import { reportError } from "@corbits/error-sink"; - -type RepoName = { name: string }; - -function throwHttp(label: string, status: number, data: unknown): never { - throw new Error( - `${label} failed with status ${status}: ${JSON.stringify(data)}`, - ); -} - -export async function hasRepoGrantViaHttp( - api: ApiCall, - tenantId: string, - repo: RepoName, - cookies: string[], -): Promise { - const resource = `repo:${repo.name}`; - const listed = await api( - "GET", - `/api/tenants/${tenantId}/grants?resource=${encodeURIComponent(resource)}&limit=200`, - undefined, - cookies, - ); - if (listed.status !== 200) { - throwHttp( - `GET /api/tenants/${tenantId}/grants`, - listed.status, - listed.data, - ); - } - const grants = parseAs( - paginatedSchema(GrantResponse), - listed.data, - "grants response", - ).data; - return grants.some( - (g) => - g.resource === resource && g.action === "read" && g.effect === "allow", - ); -} - -export async function mintRepoGrantViaHttp( - api: ApiCall, - tenantId: string, - repo: RepoName, - cookies: string[], -): Promise { - try { - const rolesResponse = await api( - "GET", - `/api/tenants/${tenantId}/roles?limit=200`, - undefined, - cookies, - ); - if (rolesResponse.status !== 200) { - throwHttp( - `GET /api/tenants/${tenantId}/roles`, - rolesResponse.status, - rolesResponse.data, - ); - } - const roles = parseAs( - paginatedSchema(RoleResponse), - rolesResponse.data, - "roles response", - ).data; - const memberRole = roles.find((r) => r.name === "member"); - if (memberRole === undefined) { - throw new Error( - `tenant ${tenantId} has no system "member" role to scope a repo grant to`, - ); - } - const posted = await api( - "POST", - `/api/tenants/${tenantId}/grants`, - { - roleId: memberRole.id, - resource: `repo:${repo.name}`, - action: "read", - effect: "allow", - origin: "creator", - }, - cookies, - ); - if (posted.status !== 201) { - throwHttp( - `POST /api/tenants/${tenantId}/grants`, - posted.status, - posted.data, - ); - } - } catch (cause) { - reportError(cause, { - operation: "mintRepoGrant", - tenantId, - extra: { repo: repo.name }, - }); - throw cause; - } -} diff --git a/apps/web/src/connect-github-actions.ts b/apps/web/src/connect-github-actions.ts deleted file mode 100644 index 31c47da96b..0000000000 --- a/apps/web/src/connect-github-actions.ts +++ /dev/null @@ -1,103 +0,0 @@ -// Builds the `ConnectGithubActions` port `ChatWorkspace` (`@corbits/chat-ui`) -// calls for its inline GitHub connect card (CL-6344). Mirrors -// `approval-actions.ts`'s shape: chat-ui owns no session, no credential, and -// no query cache, so every live read and side effect is bound here against -// this app's own API clients. -// -// Connecting the PAT reuses the exact generic route the Plugins settings -// page already calls (`completeConnectorCredential`, `@corbits/settings-ui`) -// — never a bespoke GitHub connect route; the card's own inline field -// (`connect-github-block.tsx`'s `DisconnectedBody`) is what actually asks -// for the token, so this binding never needs a prompt or a modal of its -// own. Reading state and starting reviews call the workbench-scoped routes -// `@corbits/connections`'s `createConnectGithubRoutes` exposes. -// -// `subscribeConnectState` is the card's live fold. Actions this host -// itself runs (`submitAccessToken`, `startReviewing`, `skip`) re-read -// `getConnectState` and fan the result out. A credential completed -// elsewhere publishes `chat.settings`; ChatWorkspace parses that event -// and calls `notifySettingsChanged`, which reuses the same refresh so -// a mounted card flips without remounting (CL-6476). Every action that -// can change the card's state still only fires one fetch per real -// change — never a poll loop. -import type { - ConnectGithubActions, - ConnectGithubQuery, -} from "@corbits/chat-ui"; -import { - getConnectGithubState, - getWorkbenchSettings, - patchWorkbenchSettings, - startReviewingGithubRepos, -} from "@corbits/chat-ui"; -import { - completeConnectorCredential, - ConnectionsApiError, -} from "@corbits/settings-ui"; - -export function createChatConnectGithubActions( - tenantId: string, - workbenchId: string, -): ConnectGithubActions { - const listeners = new Set<(state: ConnectGithubQuery) => void>(); - - async function refresh(): Promise { - const state = await getConnectGithubState(tenantId, workbenchId); - for (const listener of listeners) listener(state); - return state; - } - - return { - getConnectState() { - return getConnectGithubState(tenantId, workbenchId); - }, - subscribeConnectState(_messageId, onUpdate) { - listeners.add(onUpdate); - return () => { - listeners.delete(onUpdate); - }; - }, - async notifySettingsChanged() { - await refresh(); - }, - requestConnect() { - // The card's inline field is what actually collects and submits the - // token (`submitAccessToken` below); "Connect GitHub" only opens - // that field locally in chat-ui's own component state, so there is - // nothing this host needs to do here. - }, - async submitAccessToken(token) { - try { - await completeConnectorCredential(tenantId, "github", token); - } catch (cause) { - const message = - cause instanceof ConnectionsApiError - ? cause.message - : "Couldn't connect GitHub. Try again."; - return { ok: false, message }; - } - await refresh(); - return { ok: true }; - }, - async startReviewing(repoIds) { - const result = await startReviewingGithubRepos( - tenantId, - workbenchId, - repoIds, - ); - await refresh(); - return { startedTriggerCount: result.startedTriggerCount }; - }, - async skip() { - const current = await getWorkbenchSettings(tenantId, workbenchId); - const pendingConnections = ( - (current.settings["template/pendingConnections"] as - string[] | undefined) ?? [] - ).filter((id) => id !== "github"); - await patchWorkbenchSettings(tenantId, workbenchId, { - "template/pendingConnections": pendingConnections, - }); - await refresh(); - }, - }; -} diff --git a/apps/web/src/connect-service-actions.ts b/apps/web/src/connect-service-actions.ts index 6ed7128262..d4f1369847 100644 --- a/apps/web/src/connect-service-actions.ts +++ b/apps/web/src/connect-service-actions.ts @@ -1,6 +1,7 @@ // Builds the `ConnectServiceActions` port `ChatWorkspace` (`@corbits/chat-ui`) -// calls for the generic in-room connect card (CL-6393) — the -// `connect-github-actions.ts` shape generalized to every connector and +// calls for the generic in-room connect card (CL-6393) — the room +// connect-github binding's shape (hub-zero T3, CL-8114, deleted with the +// workbench-scoped mount) generalized to every connector and // MCP preset. The block's data carries only agent-authored framing; // every live fact is resolved here: a curated preset reads the // tenant's MCP preset listing (connected flag + oauth/keyless mode), a diff --git a/apps/web/src/pages/chat-page.tsx b/apps/web/src/pages/chat-page.tsx index afc48fc19f..68d4bd8a13 100644 --- a/apps/web/src/pages/chat-page.tsx +++ b/apps/web/src/pages/chat-page.tsx @@ -17,7 +17,6 @@ import { useCallback, useEffect, useMemo } from "react"; import { fetchArtifactDetail } from "../api"; import { createChatApprovalActions } from "../approval-actions"; import { createChatBlockResponseActions } from "../block-response-actions"; -import { createChatConnectGithubActions } from "../connect-github-actions"; import { createChatConnectServiceActions } from "../connect-service-actions"; import { useBench } from "../bench-context"; import { useSignOut, useSessionUser } from "../navigation"; @@ -126,13 +125,6 @@ export function ChatPage({ : createChatBlockResponseActions(tenantId, workbenchId), [tenantId, workbenchId], ); - const connectGithubActions = useMemo( - () => - tenantId === null || workbenchId === null - ? undefined - : createChatConnectGithubActions(tenantId, workbenchId), - [tenantId, workbenchId], - ); const connectServiceActions = useMemo( () => tenantId === null @@ -249,6 +241,11 @@ export function ChatPage({ [navigate], ); + // Hub-zero T3 (CL-8114): the room GitHub card is unbound on + // purpose — its state/start-reviewing routes are deleted and no native + // equivalent exists yet, so it renders its no-port disabled framing + // until a connections follow-up rebinds it. Connect/disconnect itself + // lives in Settings › Connections. const workspace = ( { - globalThis.fetch = realFetch; -}); - -const DISCONNECTED = { kind: "disconnected" as const }; -const CONNECTED = { - kind: "connected" as const, - orgName: "octocat", - repos: [{ id: "1", name: "acme/widgets" }], - selectedRepoIds: [] as string[], -}; - -function stubGithubState(state: () => unknown) { - globalThis.fetch = (async (input: RequestInfo | URL) => { - const path = typeof input === "string" ? input : String(input); - if (path.endsWith("/github/state")) { - return new Response(JSON.stringify(state()), { - status: 200, - headers: { "content-type": "application/json" }, - }); - } - throw new Error(`unstubbed fetch: ${path}`); - }) as typeof fetch; -} - -describe("createChatConnectGithubActions notifySettingsChanged", () => { - test("refresh fans the latest getConnectState to every subscriber", async () => { - let wire = DISCONNECTED as unknown; - stubGithubState(() => wire); - const actions = createChatConnectGithubActions("tnt_1", "wb_1"); - const received: ConnectGithubQuery[] = []; - const unsubscribe = actions.subscribeConnectState("m1", (state) => { - received.push(state); - }); - - expect(await actions.getConnectState("m1")).toEqual(DISCONNECTED); - expect(received).toEqual([]); - - wire = CONNECTED; - await actions.notifySettingsChanged(); - - expect(received).toEqual([CONNECTED]); - unsubscribe(); - }); -}); diff --git a/packages/chat-ui/src/api.ts b/packages/chat-ui/src/api.ts index 511a578a7a..d223fc483a 100644 --- a/packages/chat-ui/src/api.ts +++ b/packages/chat-ui/src/api.ts @@ -1243,53 +1243,11 @@ export function patchWorkbenchSettings( ); } -// The room's GitHub connect card (CL-6344): the live read and the -// start-reviewing write, backed by `@workbench/templates`'s -// `createConnectGithubRoutes` (mounted at -// `/api/tenants/:tenantId/workbenches/:workbenchId/github/*`). Connecting -// the PAT itself is a separate, already-generic surface — -// `@corbits/connections`' `POST /:connectorId/complete` — never -// duplicated here. -const ConnectGithubRepoResponse = type({ - id: "string", - name: "string", - "lastPushedAt?": "string", -}); - -const ConnectGithubStateResponse = type({ kind: "'disconnected'" }) - .or({ - kind: "'connected'", - orgName: "string", - repos: ConnectGithubRepoResponse.array(), - selectedRepoIds: "string[]", - }) - .or({ kind: "'error'", message: "string" }); -export type ConnectGithubStateResponse = - typeof ConnectGithubStateResponse.infer; - -export function getConnectGithubState( - tenantId: string, - workbenchId: string, -): Promise { - return request( - `/api/tenants/${tenantId}/workbenches/${workbenchId}/github/state`, - ConnectGithubStateResponse, - ); -} - -const StartReviewingResponse = type({ startedTriggerCount: "number" }); - -export function startReviewingGithubRepos( - tenantId: string, - workbenchId: string, - repoIds: readonly string[], -): Promise<{ readonly startedTriggerCount: number }> { - return request( - `/api/tenants/${tenantId}/workbenches/${workbenchId}/github/start-reviewing`, - StartReviewingResponse, - { method: "POST", body: JSON.stringify({ repoIds }) }, - ); -} +// Hub-zero T3 (CL-8114): the room card's live state read and +// start-reviewing write lived here, backed by the hub's now-deleted +// workbench-scoped GitHub mount. GitHub connect/disconnect +// itself stays native `connections/*` (`@corbits/settings-ui`); the card's +// state/start rebind is a connections follow-up. // `GET`/`PATCH /bench/settings` (see `packages/chat/src/routes.ts`): the // bench-wide chat defaults every workbench inherits unless it sets its own diff --git a/packages/chat-ui/src/blocks/connect-github-actions.ts b/packages/chat-ui/src/blocks/connect-github-actions.ts index f8cf69ed5b..0f81335387 100644 --- a/packages/chat-ui/src/blocks/connect-github-actions.ts +++ b/packages/chat-ui/src/blocks/connect-github-actions.ts @@ -7,6 +7,12 @@ // (`github`'s PAT test-and-store — already fully generic, no bespoke // GitHub route needed), `@corbits/github-tools`' `listRepos`, and // `@corbits/connections`'s `startReviewingRepos`. +// +// Hub-zero T3 (CL-8114): no host binds this port right now — the hub's +// workbench-scoped state/start-reviewing routes are deleted and no native +// equivalent exists yet, so the room card renders its no-port disabled +// framing. The shape stays so a connections follow-up can rebind it +// without redesigning the card. import type { ConnectGithubRepo } from "./connect-github-block"; export type { ConnectGithubRepo }; diff --git a/packages/chat-ui/src/index.ts b/packages/chat-ui/src/index.ts index 0215c5cec1..683d5609a1 100644 --- a/packages/chat-ui/src/index.ts +++ b/packages/chat-ui/src/index.ts @@ -186,10 +186,7 @@ export { runDisplayName, getWorkbenchSettings, patchWorkbenchSettings, - getConnectGithubState, postWorkbenchOnboardingStep, - startReviewingGithubRepos, - type ConnectGithubStateResponse, getBenchChatSettings, patchBenchChatSettings, getBlockResponses, diff --git a/packages/chat-ui/test/onboarding-scene-card.test.tsx b/packages/chat-ui/test/onboarding-scene-card.test.tsx index 9a00a06465..f49eea9134 100644 --- a/packages/chat-ui/test/onboarding-scene-card.test.tsx +++ b/packages/chat-ui/test/onboarding-scene-card.test.tsx @@ -99,8 +99,9 @@ function currentStepAria(el: HTMLElement): string | null { } /** A host whose live state is whatever the test says it is, pushed once - * on mount — the same read-then-subscribe contract the real - * `createChatConnectGithubActions` binds. */ + * on mount — the read-then-subscribe contract the `ConnectGithubActions` + * port specifies (no host binds it since hub-zero T3 deleted the + * workbench-scoped mount, CL-8114). */ function fixedStateActions(state: ConnectGithubQuery): ConnectGithubActions { return { getConnectState: () => Promise.resolve(state), diff --git a/packages/connections/src/connect-github-setup.race.test.ts b/packages/connections/src/connect-github-setup.race.test.ts index 01bd9d7358..b5f5ddbf16 100644 --- a/packages/connections/src/connect-github-setup.race.test.ts +++ b/packages/connections/src/connect-github-setup.race.test.ts @@ -2,8 +2,9 @@ // `startReviewingRepos` calls for the same repo -- against real, // database-backed ports rather than plain fakes. `hasRepoGrant`/ // `mintRepoGrant` here fake a plain read-then-insert against the -// `grant` table directly, standing in for `apps/hub/src/index.ts`'s -// real binding (HTTP calls through `native-repo-grants.ts`, see +// `grant` table directly, standing in for what `apps/hub/src/index.ts`'s +// real binding did (HTTP calls through `native-repo-grants.ts`, deleted +// with the workbench-scoped mount in hub-zero T3, CL-8114 -- see the // CL-7242 follow-up "Mint workbench tenants and repo grants via // Interchange HTTP") without a live hub-api server -- what this test // actually proves is that `startReviewingRepos`' lease serializes any diff --git a/packages/evals/src/targets/real-target.ts b/packages/evals/src/targets/real-target.ts index dd0edf1ddd..06c929bf7e 100644 --- a/packages/evals/src/targets/real-target.ts +++ b/packages/evals/src/targets/real-target.ts @@ -810,48 +810,17 @@ export async function bootMyraTarget( }, }); - // The rest of the definition's onboarding walkthrough, driven - // here rather than by a person clicking the in-room card: read the - // connect card's live state, then start reviewing every listed repo — - // which mints the per-repo grant and `webhook_trigger` row the - // fire-webhook step needs. - let startedTriggerCount = 0; + // Hub-zero T3 (CL-8114): the hub's workbench-scoped GitHub + // state/start-reviewing routes are deleted with no native + // equivalent yet, so this harness no longer drives the card's + // repo-pick walkthrough — the per-repo grant and + // `webhook_trigger` rows the fire-webhook step needs now come + // from a connections follow-up. The install honestly reports + // zero started triggers until then. + const startedTriggerCount = 0; if (definition.plugins.required.includes("github")) { - const stateRes = await api( - hub.baseUrl, - "GET", - `/api/tenants/${seeded.tenantId}/workbenches/${workbenchId}/github/state`, - undefined, - cookies, - ); - expectStatus("read the connect card's GitHub state", stateRes, 200); - const state = stateRes.data as { - kind: string; - repos?: { id: string }[]; - }; - if (state.kind !== "connected") { - throw new Error( - `installTemplate("${templateId}"): expected the GitHub connect ` + - `card to read connected, got: ${JSON.stringify(stateRes.data)}`, - ); - } - const repoIds = (state.repos ?? []).map((repo) => repo.id); - const startRes = await api( - hub.baseUrl, - "POST", - `/api/tenants/${seeded.tenantId}/workbenches/${workbenchId}/github/start-reviewing`, - { repoIds }, - cookies, - ); - expectStatus("start reviewing the listed repos", startRes, 200); - const started = (startRes.data as { startedTriggerCount?: unknown }) - .startedTriggerCount; - if (typeof started !== "number") { - throw new Error( - `start-reviewing answered without a startedTriggerCount: ${JSON.stringify(startRes.data)}`, - ); - } - startedTriggerCount = started; + // TODO(connections-follow-up): rebind the repo-pick walkthrough + // to the native tenant-scoped repo-review setup once it exists. } return { diff --git a/scripts/checks/hub-workbench-routes.ts b/scripts/checks/hub-workbench-routes.ts new file mode 100644 index 0000000000..e78b7c3728 --- /dev/null +++ b/scripts/checks/hub-workbench-routes.ts @@ -0,0 +1,108 @@ +// Hub-zero T3 (CL-8114): the hub mounts no workbench-scoped GitHub-connect +// surface. `@corbits/connections` keeps owning the `createConnectGithubRoutes` +// factory (its own package tests still cover it) — what is gone is the hub +// composition root's `${TENANT_PREFIX}/workbenches` mount of it, whose +// `getTemplateSettings` / `persistSelectedRepos` / `onReviewingStarted` ports +// were the last hub readers of the `workbench_settings` row. GitHub +// connect/disconnect now goes through the native tenant-scoped +// `/api/tenants/:tenantId/connections/*` routes only; the room card's +// state/start-reviewing rebind is a connections follow-up (the card renders +// its existing no-port disabled framing until then). +// +// This check locks the done-when: no `createConnectGithubRoutes` reference +// and no bare `${TENANT_PREFIX}/workbenches` mount in `apps/hub/src`, the +// brief's `workbench-settings|/workbenches.*github|github.*workbench` rg +// empty over `apps/hub/src`, and `listWorkbenchSettings` surviving in +// `apps/hub` only inside the T4-owned scheduled-delivery seam +// (`scheduledDeliveryJoinDeps`), which reads the row for delivery — never +// for GitHub connect. +import { readFileSync } from "node:fs"; +import { + emptyReport, + reportAndExit, + rootFromArgs, + type CheckReport, +} from "./lib/repo"; + +export type SourceFile = { + readonly relPath: string; + readonly contents: string; +}; + +/** The T4-owned seam still allowed to read the settings row: scheduled + * delivery resolving the workbench a run belongs to. Any + * `listWorkbenchSettings` call site outside a file containing this marker + * is a new workbench-settings reader the hub-zero cutover missed. */ +const DELIVERY_SEAM_MARKER = "scheduledDeliveryJoinDeps"; + +/** The brief's done-when rg, per line, case-insensitive. */ +const DONE_WHEN_PATTERN = + /workbench-settings|\/workbenches.*github|github.*workbench/i; + +export function auditHubWorkbenchRoutes( + files: readonly SourceFile[], +): CheckReport { + const report = emptyReport(); + for (const file of files) { + if (file.relPath !== "apps/hub/src/index.ts") continue; + if (file.contents.includes("createConnectGithubRoutes")) { + report.violations.push( + `${file.relPath}: still references createConnectGithubRoutes — ` + + "the hub's /workbenches GitHub-connect mount must be deleted " + + "(connect/disconnect is native /connections/* now)", + ); + } + if (file.contents.includes("${TENANT_PREFIX}/workbenches`")) { + report.violations.push( + `${file.relPath}: still mounts \`\${TENANT_PREFIX}/workbenches\` — ` + + "the workbench-scoped GitHub-connect mount must be deleted", + ); + } + } + for (const file of files) { + let offset = 0; + for (const line of file.contents.split("\n")) { + const lineNumber = file.contents.slice(0, offset).split("\n").length; + offset += line.length + 1; + if (DONE_WHEN_PATTERN.test(line)) { + report.violations.push( + `${file.relPath}:${lineNumber}: ` + + `matches the hub-zero done-when rg: ${line.trim()}`, + ); + } + } + } + for (const file of files) { + if (!file.contents.includes("listWorkbenchSettings")) continue; + if (file.contents.includes(DELIVERY_SEAM_MARKER)) { + report.notes.push( + `${file.relPath}: listWorkbenchSettings survives inside the ` + + "T4-owned scheduled-delivery seam only — documented, not a violation", + ); + continue; + } + report.violations.push( + `${file.relPath}: listWorkbenchSettings outside the T4-owned ` + + "scheduled-delivery seam — a workbench-settings reader hub-zero missed", + ); + } + return report; +} + +async function main(): Promise { + const root = rootFromArgs(process.argv); + const files: SourceFile[] = []; + for (const dir of ["apps/hub/src"]) { + const glob = new Bun.Glob(`${dir}/**/*.{ts,tsx}`); + for await (const relPath of glob.scan({ cwd: root, dot: false })) { + if (relPath.includes("node_modules")) continue; + files.push({ + relPath, + contents: readFileSync(`${root}/${relPath}`, "utf8"), + }); + } + } + reportAndExit("check:hub-workbench-routes", auditHubWorkbenchRoutes(files)); +} + +if (import.meta.main) await main(); diff --git a/scripts/checks/no-product-tenancy.ts b/scripts/checks/no-product-tenancy.ts index b1605e8acc..dc13ea9de4 100644 --- a/scripts/checks/no-product-tenancy.ts +++ b/scripts/checks/no-product-tenancy.ts @@ -81,9 +81,11 @@ const ALLOWLIST: readonly { tables: [ "webhook_trigger", // CL-7242: a short-lived lease serializing the GitHub connect - // card's start-reviewing step, entirely workbench-owned state - // with no relationship to Interchange's own `grant` table — see - // repo-review-lease.ts's doc comment for why the concurrency + // card's former start-reviewing step (workbench-scoped mount + // deleted in hub-zero T3, CL-8114; the lease still backs the + // native `startReviewingRepos` helper), entirely workbench-owned + // state with no relationship to Interchange's own `grant` table — + // see repo-review-lease.ts's doc comment for why the concurrency // fix lives here rather than as any change to Interchange's // schema. "repo_review_lease", diff --git a/scripts/checks/report-error-baseline.txt b/scripts/checks/report-error-baseline.txt index f7870a7295..f6f291d3bb 100644 --- a/scripts/checks/report-error-baseline.txt +++ b/scripts/checks/report-error-baseline.txt @@ -61,7 +61,6 @@ apps/web/src/block-response-actions.ts 1 if (cause instanceof ChatApiError && ca apps/web/src/block-response-actions.ts 2 if (cause instanceof ChatApiError && cause.status === 403) { apps/web/src/command-palette-recents.ts 1 // Storage disabled or full — recents just stop persisting this tab. apps/web/src/command-palette-recents.ts 1 return null; -apps/web/src/connect-github-actions.ts 1 const message = apps/web/src/connect-service-actions.ts 1 const message = apps/web/src/connect-service-actions.ts 1 return { ok: false, message: "Couldn't connect. Try again." }; apps/web/src/connect-service-actions.ts 2 const message = diff --git a/scripts/checks/test/hub-workbench-routes.test.ts b/scripts/checks/test/hub-workbench-routes.test.ts new file mode 100644 index 0000000000..3884da970e --- /dev/null +++ b/scripts/checks/test/hub-workbench-routes.test.ts @@ -0,0 +1,70 @@ +import { expect, test } from "bun:test"; +import { auditHubWorkbenchRoutes } from "../hub-workbench-routes"; + +const CLEAN_INDEX = `import { createApp } from "@corbits/hub"; +const scheduledDeliveryJoinDeps = {}; +async function resolve() { + const rows = await chatStore.listWorkbenchSettings(tenantId); + return rows; +} +app.route(\`\${TENANT_PREFIX}/chat\`, createChatRoutes({})); +`; + +test("a hub index with no github mount and only the delivery-seam reader passes", () => { + const report = auditHubWorkbenchRoutes([ + { relPath: "apps/hub/src/index.ts", contents: CLEAN_INDEX }, + ]); + expect(report.violations).toEqual([]); +}); + +test("the createConnectGithubRoutes reference is a violation naming it", () => { + const report = auditHubWorkbenchRoutes([ + { + relPath: "apps/hub/src/index.ts", + contents: `${CLEAN_INDEX}\nimport { createConnectGithubRoutes } from "@corbits/connections/connect-github-routes";\n`, + }, + ]); + expect(report.violations.length).toBeGreaterThan(0); + expect(report.violations[0]).toContain("createConnectGithubRoutes"); +}); + +test("the bare /workbenches mount is a violation, a suffixed route is not", () => { + const withMount = auditHubWorkbenchRoutes([ + { + relPath: "apps/hub/src/index.ts", + contents: `${CLEAN_INDEX}\napp.route(\`\${TENANT_PREFIX}/workbenches\`, createConnectGithubRoutes({}));\n`, + }, + ]); + expect(withMount.violations.length).toBeGreaterThan(0); + expect(withMount.violations.join("\n")).toContain("/workbenches"); + + const withStream = auditHubWorkbenchRoutes([ + { + relPath: "apps/hub/src/index.ts", + contents: `${CLEAN_INDEX}\napp.route(\`\${TENANT_PREFIX}/workbenches/:id/stream\`, createStreamRoute({}));\n`, + }, + ]); + expect(withStream.violations).toEqual([]); +}); + +test("a done-when rg line in hub src is a violation naming the file", () => { + const report = auditHubWorkbenchRoutes([ + { + relPath: "apps/hub/src/other.ts", + contents: `// the github workbench card reads state here\n`, + }, + ]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("apps/hub/src/other.ts"); +}); + +test("listWorkbenchSettings outside the delivery seam is a violation", () => { + const report = auditHubWorkbenchRoutes([ + { + relPath: "apps/hub/src/other.ts", + contents: `await chatStore.listWorkbenchSettings(tenantId);\n`, + }, + ]); + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("listWorkbenchSettings"); +}); diff --git a/scripts/checks/test/run.test.ts b/scripts/checks/test/run.test.ts index 828cdc5d1e..e3c809d3f3 100644 --- a/scripts/checks/test/run.test.ts +++ b/scripts/checks/test/run.test.ts @@ -17,6 +17,7 @@ const EXPECTED_CHECK_NAMES = [ "deletion", "error-envelope", "hub-git-safety", + "hub-workbench-routes", "killdates", "licenses", "no-product-tenancy", diff --git a/scripts/e2e/t3-github-roundtrip.proof.test.ts b/scripts/e2e/t3-github-roundtrip.proof.test.ts new file mode 100644 index 0000000000..4400393cb3 --- /dev/null +++ b/scripts/e2e/t3-github-roundtrip.proof.test.ts @@ -0,0 +1,185 @@ +// CL-8114 evidence kept as a regression proof: exercises the browser's exact +// GitHub connect→disconnect round-trip (`connections/github/complete` → +// credentials list → `connections/github/disconnect` → credentials list) +// against a booted hub on a scratch database with the operator's real `gh` +// token, proving the native connections routes the workbench-scoped mount was +// repointed to still round-trip. The token is read once from `gh auth token` +// at module load, never logged, never stored; the suite skips cleanly when no +// database URL is set or no gh auth is available (e.g. CI without a token). +import { describe, expect, test } from "bun:test"; + +import { resetSchema, setupDatabase } from "../db-setup.ts"; +import { + api, + createCleanupHarness, + e2eDatabaseUrl, + expectStatus, + freePort, + hop, + startHub, + type HubHandle, +} from "./harness.ts"; + +const databaseUrl = e2eDatabaseUrl(); +if (databaseUrl === undefined) { + console.warn("t3-proof: DATABASE_URL is not set; suite skipped."); +} + +const { tempDir, track } = createCleanupHarness(); + +function readGhAuthToken(): string | undefined { + try { + const out = Bun.spawnSync(["gh", "auth", "token"]); + if (out.exitCode !== 0) return undefined; + const text = new TextDecoder().decode(out.stdout).trim(); + return text === "" ? undefined : text; + } catch { + return undefined; + } +} + +const ghToken = readGhAuthToken(); +if (databaseUrl !== undefined && ghToken === undefined) { + console.warn("t3-proof: `gh auth token` is unavailable; suite skipped."); +} + +describe.skipIf(databaseUrl === undefined || ghToken === undefined)( + "t3-proof: github connect-disconnect round-trip", + () => { + test("complete -> status -> disconnect -> status with a real token", async () => { + const url = databaseUrl; + if (url === undefined) throw new Error("unreachable: suite is skipped"); + + await hop("database setup", async () => { + await resetSchema(url); + const report = await setupDatabase(url); + expect(report.action).toBe("migrated"); + }); + + const hub: HubHandle = await hop("hub boot", async () => + startHub({ + databaseUrl: url, + port: freePort(), + sessionSecret: Buffer.from( + crypto.getRandomValues(new Uint8Array(32)), + ).toString("hex"), + dataDir: await tempDir("e2e-t3-proof-hub-data-"), + }), + ); + track(hub); + + const signup = await hop("genesis sign-up", async () => { + const res = await api(hub.baseUrl, "POST", "/api/auth/sign-up/email", { + name: "T3 Proof", + email: "t3-proof@example.com", + password: "password123", + }); + expectStatus("sign-up", res, 200); + if (res.cookies.length === 0) { + throw new Error("sign-up returned no session cookie"); + } + return res.cookies; + }); + + const tenantId = await hop("tenant create", async () => { + const minted = await api( + hub.baseUrl, + "POST", + "/api/tenants", + { slug: "t3proof", name: "T3 Proof" }, + signup, + ); + expectStatus("create tenant", minted, 201); + const body = minted.data as { id?: string; tenantId?: string }; + const id = + typeof body.id === "string" + ? body.id + : typeof body.tenantId === "string" + ? body.tenantId + : undefined; + if (id === undefined) { + throw new Error( + `create tenant answered no id: ${JSON.stringify(minted.data)}`, + ); + } + return id; + }); + + const token = await hop("read operator gh token", async () => { + if (ghToken === undefined) { + throw new Error("unreachable: suite is skipped"); + } + console.log("token -> present (value never printed)"); + return ghToken; + }); + + const completePath = `/api/tenants/${tenantId}/connections/github/complete`; + const disconnectPath = `/api/tenants/${tenantId}/connections/github/disconnect`; + const credentialsPath = `/api/tenants/${tenantId}/credentials`; + + async function credentialPresent( + what: string, + credentialId: string, + ): Promise { + const res = await api( + hub.baseUrl, + "GET", + credentialsPath, + undefined, + signup, + ); + expectStatus(`credentials list (${what})`, res, 200); + const rows = (res.data as { data: { id: string }[] }).data; + console.log( + `credentials (${what}) -> ${res.status} count=${rows.length} present=${rows.some((row) => row.id === credentialId)}`, + ); + return rows.some((row) => row.id === credentialId); + } + + const credentialId = await hop("connect (POST complete)", async () => { + const res = await api( + hub.baseUrl, + "POST", + completePath, + { apiKey: token }, + signup, + ); + expectStatus("github complete", res, 200); + const body = res.data as { credentialId?: string; status?: string }; + console.log( + `complete -> ${res.status} status=${body.status} credentialId=${body.credentialId}`, + ); + if (body.credentialId === undefined || body.status !== "active") { + throw new Error( + `complete answered no active credential: ${JSON.stringify(res.data)}`, + ); + } + return body.credentialId; + }); + + await hop("status reads connected", async () => { + expect(await credentialPresent("after connect", credentialId)).toBe( + true, + ); + }); + + await hop("disconnect (DELETE)", async () => { + const res = await api( + hub.baseUrl, + "DELETE", + disconnectPath, + undefined, + signup, + ); + expectStatus("github disconnect", res, 204); + console.log(`disconnect -> ${res.status}`); + }); + + await hop("status reads disconnected", async () => { + expect(await credentialPresent("after disconnect", credentialId)).toBe( + false, + ); + }); + }, 180_000); + }, +); diff --git a/scripts/repro/connect-github-smoke.ts b/scripts/repro/connect-github-smoke.ts index e2773e73db..5d75bc98d0 100644 --- a/scripts/repro/connect-github-smoke.ts +++ b/scripts/repro/connect-github-smoke.ts @@ -1,7 +1,12 @@ /** - * Manual smoke test for the connect-github host bindings (CL-6344): - * connect a real PAT, list repos, and start reviewing one, against a - * running local hub. + * Manual smoke test for the GitHub connect binding (CL-6344, hub-zero T3 + * CL-8114): connects a real PAT through the native tenant-scoped + * `connections/github/complete` route against a running local hub. + * + * Hub-zero T3 deleted the workbench-scoped `github/state` and + * `github/start-reviewing` routes with no native equivalent yet, so this + * smoke stops after a successful connect — the card's repo-pick + * walkthrough rebind is a connections follow-up. * * Never prints or commits the token. Reads it from `GITHUB_TOKEN`, or * falls back to `gh auth token` if that env var is unset. @@ -9,7 +14,6 @@ * Required env: * HUB_URL e.g. http://localhost:3000 * TENANT_ID an existing tenant id this session can act as - * WORKBENCH_ID a workbench minted from the "code-review" template * COOKIE the session cookie header value for an authenticated * request (copy from a logged-in browser session) * @@ -40,7 +44,6 @@ function requireEnv(name: string): string { async function main() { const hubUrl = requireEnv("HUB_URL"); const tenantId = requireEnv("TENANT_ID"); - const workbenchId = requireEnv("WORKBENCH_ID"); const cookie = requireEnv("COOKIE"); const token = readToken(); @@ -49,42 +52,21 @@ async function main() { cookie, }; - console.log("== connect =="); + console.log("== connect (native connections route) =="); const connectRes = await fetch( `${hubUrl}/api/tenants/${tenantId}/connections/github/complete`, { method: "POST", headers, body: JSON.stringify({ apiKey: token }) }, ); console.log(connectRes.status, await connectRes.text()); - if (!connectRes.ok) return; - - console.log("== state =="); - const stateRes = await fetch( - `${hubUrl}/api/tenants/${tenantId}/workbenches/${workbenchId}/github/state`, - { headers }, - ); - const state = (await stateRes.json()) as { - kind: string; - repos?: { id: string; name: string }[]; - }; - console.log(stateRes.status, state); - if (state.kind !== "connected" || state.repos === undefined) return; - - const firstRepo = state.repos[0]; - if (firstRepo === undefined) { - console.log("no repos to review — stopping here"); - return; + if (!connectRes.ok) { + throw new Error("connect failed — see status above"); } - - console.log(`== start-reviewing ${firstRepo.name} ==`); - const startRes = await fetch( - `${hubUrl}/api/tenants/${tenantId}/workbenches/${workbenchId}/github/start-reviewing`, - { - method: "POST", - headers, - body: JSON.stringify({ repoIds: [firstRepo.id] }), - }, + // Hub-zero T3 (CL-8114) stops here: `github/state` and + // `github/start-reviewing` no longer exist, so there is nothing further + // to smoke until the connections follow-up lands the native rebind. + console.log( + "connected — state/start-reviewing walkthrough deleted (CL-8114)", ); - console.log(startRes.status, await startRes.text()); } main().catch((err: unknown) => {