From 4ef3cca9fe8ba1f6acaff75956ad6b34a885641e Mon Sep 17 00:00:00 2001 From: David Moore Date: Tue, 6 Oct 2026 18:09:11 +1100 Subject: [PATCH] refactor: accept an invitation in one request --- .../contracts/src/http/groups/onboarding.ts | 12 +-- packages/contracts/src/membership.ts | 16 +++- packages/contracts/src/onboarding.ts | 6 -- .../workspaces/membership/membership.test.ts | 89 +++++++++++++++++ .../src/workspaces/membership/membership.ts | 81 +++++++++++++--- .../workspaces/onboarding/onboarding.test.ts | 45 +-------- .../src/workspaces/onboarding/onboarding.ts | 95 ++----------------- .../src/routes/onboarding/routes.test.ts | 25 +---- .../server/src/routes/onboarding/routes.ts | 18 +--- packages/web/src/router.tsx | 34 ++++--- packages/web/src/screens/Invite.stories.tsx | 11 +-- packages/web/src/screens/Invite.tsx | 78 +++++++-------- packages/web/src/shell/shell.test.tsx | 67 ++++++++++--- packages/web/src/test-api.tsx | 8 +- packages/web/src/test-client.ts | 1 - 15 files changed, 300 insertions(+), 286 deletions(-) diff --git a/packages/contracts/src/http/groups/onboarding.ts b/packages/contracts/src/http/groups/onboarding.ts index f1baecf5..5645a9f5 100644 --- a/packages/contracts/src/http/groups/onboarding.ts +++ b/packages/contracts/src/http/groups/onboarding.ts @@ -1,22 +1,14 @@ import { HttpApiEndpoint, HttpApiGroup } from "effect/unstable/httpapi"; -import { - completedInviteOnboardingSchema, - completeInviteOnboardingSchema, - completeOnboardingSchema, -} from "../../onboarding.ts"; +import { completeOnboardingSchema } from "../../onboarding.ts"; import { refused } from "../errors.ts"; import { Session } from "../middleware.ts"; -/** The signed-in person finishing setting up a workspace, or joining one by invitation. */ +/** The signed-in person finishing setting up a workspace. */ export class OnboardingApi extends HttpApiGroup.make("onboarding") .add( HttpApiEndpoint.post("complete", "/onboarding/complete", { payload: completeOnboardingSchema, error: refused, }), - HttpApiEndpoint.post("completeInvite", "/onboarding/complete-invite", { - payload: completeInviteOnboardingSchema, - success: completedInviteOnboardingSchema, - }), ) .middleware(Session) {} diff --git a/packages/contracts/src/membership.ts b/packages/contracts/src/membership.ts index 0e0d0daa..a1659f0e 100644 --- a/packages/contracts/src/membership.ts +++ b/packages/contracts/src/membership.ts @@ -70,10 +70,18 @@ export const newWorkspaceInvitationSchema = Schema.Struct({ }); export type NewWorkspaceInvitation = typeof newWorkspaceInvitationSchema.Type; -export const invitationPreviewSchema = Schema.Struct({ - workspaceName: Schema.String, - inviterName: Schema.String, -}); +/** + * An invitation as the person it was sent to sees it: one still to accept, or + * one they have accepted already, whose link leads back into its workspace. + */ +export const invitationPreviewSchema = Schema.Union([ + Schema.Struct({ + status: Schema.Literal("pending"), + workspaceName: Schema.String, + inviterName: Schema.String, + }), + Schema.Struct({ status: Schema.Literal("accepted"), workspaceId: uuidSchema }), +]); export type InvitationPreview = typeof invitationPreviewSchema.Type; export const acceptedInvitationSchema = Schema.Struct({ workspaceId: uuidSchema }); diff --git a/packages/contracts/src/onboarding.ts b/packages/contracts/src/onboarding.ts index 4fb20f19..7cbffb77 100644 --- a/packages/contracts/src/onboarding.ts +++ b/packages/contracts/src/onboarding.ts @@ -7,9 +7,3 @@ export const completeOnboardingSchema = Schema.Struct({ agentId: uuidSchema, }); export type CompleteOnboarding = typeof completeOnboardingSchema.Type; - -export const completeInviteOnboardingSchema = Schema.Struct({ invitationId: uuidSchema }); -export type CompleteInviteOnboarding = typeof completeInviteOnboardingSchema.Type; - -export const completedInviteOnboardingSchema = Schema.Struct({ workspaceId: uuidSchema }); -export type CompletedInviteOnboarding = typeof completedInviteOnboardingSchema.Type; diff --git a/packages/core/src/workspaces/membership/membership.test.ts b/packages/core/src/workspaces/membership/membership.test.ts index 56c55112..317fbdb3 100644 --- a/packages/core/src/workspaces/membership/membership.test.ts +++ b/packages/core/src/workspaces/membership/membership.test.ts @@ -5,10 +5,13 @@ import { Accounts } from "../../accounts/accounts.ts"; import { CurrentActor } from "../../authorization/current-actor.ts"; import { agent, + modelProvider, pod, podMember, + providerModel, searchProvider, user, + workspaceDefaultModel, workspaceInvite, workspaceMember, } from "../../database/schema.ts"; @@ -243,6 +246,7 @@ describe.skipIf(!process.env.DATABASE_URL)("Membership, against Postgres", () => }); expect(sent.at(-1)?.text).toContain(`${WEB_APP_URL}/invite/${invitation.id}`); expect(await run(bob.id, (m) => m.invitation({ invitationId: invitation.id }))).toEqual({ + status: "pending", workspaceName: "Nitric", inviterName: "Ada", }); @@ -260,6 +264,91 @@ describe.skipIf(!process.env.DATABASE_URL)("Membership, against Postgres", () => ).toHaveLength(1); }); + it("leads an invitee back in from a link already accepted, until they leave", async () => { + const { ada, workspace } = await workspaceOfAda(); + const bob = await person("Bob"); + const invitation = await run(ada.id, (m) => + m.invite({ workspace: workspace.id, invitation: { email: bob.email, role: "member" } }), + ); + if ("failed" in invitation) throw new Error(invitation.failed); + await run(bob.id, (m) => m.accept({ invitationId: invitation.id })); + + expect(await run(bob.id, (m) => m.invitation({ invitationId: invitation.id }))).toEqual({ + status: "accepted", + workspaceId: workspace.id, + }); + expect(await run(bob.id, (m) => m.accept({ invitationId: invitation.id }))).toEqual({ + workspaceId: workspace.id, + }); + expect(await run(bob.id, (m) => m.members({ workspace: workspace.id }))).toHaveLength(2); + + await run(bob.id, (m) => m.leave({ workspace: workspace.id })); + + expect(await run(bob.id, (m) => m.invitation({ invitationId: invitation.id }))).toEqual({ + failed: "ResourceHidden", + }); + expect(await run(bob.id, (m) => m.accept({ invitationId: invitation.id }))).toEqual({ + failed: "ResourceHidden", + }); + }); + + it("starts somebody joining on another model the workspace offers while it does not offer its default", async () => { + const { ada, workspace } = await workspaceOfAda(); + const bob = await person("Bob"); + const [provider] = await onDatabase((db) => + db + .insert(modelProvider) + .values({ + workspaceId: workspace.id, + name: "Models", + baseUrl: "https://models.example/v1", + apiFormat: "openai", + active: true, + }) + .returning(), + ); + if (!provider) throw new Error("could not create the model provider"); + await onDatabase((db) => + db.insert(providerModel).values( + [ + { modelId: "offered", enabled: true }, + { modelId: "switched-off", enabled: false }, + ].map((model) => ({ + ...model, + workspaceId: workspace.id, + providerId: provider.id, + source: "manual" as const, + })), + ), + ); + await onDatabase((db) => + db + .insert(workspaceDefaultModel) + .values({ workspaceId: workspace.id, modelId: "switched-off" }), + ); + const invitation = await run(ada.id, (m) => + m.invite({ workspace: workspace.id, invitation: { email: bob.email, role: "member" } }), + ); + if ("failed" in invitation) throw new Error(invitation.failed); + + await run(bob.id, (m) => m.accept({ invitationId: invitation.id })); + + const [assistant] = await onDatabase((db) => + db + .select({ model: agent.model }) + .from(agent) + .innerJoin(pod, eq(pod.id, agent.podId)) + .where( + and( + eq(pod.workspaceId, workspace.id), + eq(pod.ownerId, bob.id), + eq(agent.provisionedKey, "personal-assistant"), + ), + ), + ); + expect(assistant?.model).toBe("offered"); + }); + it("refuses an invitation to anybody but the address it was sent to", async () => { const { ada, workspace } = await workspaceOfAda(); const bob = await person("Bob"); diff --git a/packages/core/src/workspaces/membership/membership.ts b/packages/core/src/workspaces/membership/membership.ts index 51355ce5..e9bae2bd 100644 --- a/packages/core/src/workspaces/membership/membership.ts +++ b/packages/core/src/workspaces/membership/membership.ts @@ -12,7 +12,7 @@ import type { WorkspacePermissions, WorkspaceRole, } from "@sugabots/contracts"; -import { and, asc, eq, gt, sql } from "drizzle-orm"; +import { and, asc, eq, gt, or, sql } from "drizzle-orm"; import { Context, Data, DateTime, Duration, Effect, Layer } from "effect"; import { Accounts } from "../../accounts/accounts.ts"; import { @@ -40,6 +40,7 @@ import { user, workspace, workspaceInvite, workspaceMember } from "../../databas import { Email } from "../../email/email.ts"; import { Ids, isUuid } from "../../ids/ids.ts"; import { Installation } from "../../installation/installation.ts"; +import { offeredModels } from "../../providers/model-providers/model-provider-reads.ts"; import { ModelProviderRepository } from "../../providers/model-providers/model-provider-repository.ts"; import { SearchProviderRepository } from "../../providers/search-providers/search-provider-repository.ts"; import { type UserFacing, UserMessage } from "../../user-message.ts"; @@ -123,10 +124,18 @@ export interface Interface { readonly cancelInvitation: ( input: ForInvitation, ) => Effect.Effect; - /** For the person the invitation was sent to. */ + /** + * For the person the invitation was sent to: one still to accept, or one + * they have accepted and whose workspace they are still in. + */ readonly invitation: ( input: ForInvitation, ) => Effect.Effect; + /** + * Lets the invitee in, with their Personal pod. Accepting again, as a + * second tab or a reload does, answers with the workspace and changes + * nothing. + */ readonly accept: ( input: ForInvitation, ) => Effect.Effect< @@ -160,6 +169,25 @@ export const make = Effect.gen(function* () { const modelProviders = yield* ModelProviderRepository.Service; const ids = yield* Ids.Service; + /** + * The Personal pod of somebody joining, its assistant on the workspace's + * default model, or on another the workspace offers while it does not offer + * that one. + */ + const provisionJoining = (workspaceId: string, userId: string) => + Effect.gen(function* () { + const { models, defaultModel } = yield* offeredModels(workspaceId); + // The default goes unoffered while a failed test has its provider off. + const model = models.some((offered) => offered.modelId === defaultModel) + ? defaultModel + : models[0]?.modelId; + if (!model) return yield* personalPods.provision({ workspaceId, userId }); + return yield* personalPods.provisionWithModel({ workspaceId, userId, model }).pipe( + // Switched off since it was read: start on none, as while the workspace offers none. + Effect.catchTag("ModelNotEnabled", () => personalPods.provision({ workspaceId, userId })), + ); + }); + const sendInvitation = ( invitationId: string, to: string, @@ -556,7 +584,12 @@ export const make = Effect.gen(function* () { Effect.gen(function* () { const { userId } = yield* CurrentActor.Service; const invitation = yield* invitationFor(userId, input.invitationId); - return { workspaceName: invitation.workspaceName, inviterName: invitation.inviterName }; + if (invitation.status === "accepted") return invitation; + return { + status: "pending" as const, + workspaceName: invitation.workspaceName, + inviterName: invitation.inviterName, + }; }), ), @@ -567,6 +600,7 @@ export const make = Effect.gen(function* () { Effect.gen(function* () { const { userId } = yield* CurrentActor.Service; const invitation = yield* invitationFor(userId, input.invitationId); + if (invitation.status === "accepted") return { workspaceId: invitation.workspaceId }; if (accounts.requireEmailVerification && !invitation.inviteeVerified) { return yield* new EmailUnverified(); } @@ -586,7 +620,7 @@ export const make = Effect.gen(function* () { .where(eq(workspaceInvite.id, input.invitationId)); }), ); - yield* personalPods.provision({ workspaceId: invitation.workspaceId, userId }); + yield* provisionJoining(invitation.workspaceId, userId); return { workspaceId: invitation.workspaceId }; }), ), @@ -738,9 +772,11 @@ function deleteMember(memberId: string) { } /** - * A pending, unexpired invitation, locked for an enclosing transaction, when - * `userId` is the person it was sent to. Anything else is hidden, so a link - * cannot be probed. + * The invitation `invitationId`, locked for an enclosing transaction, when + * `userId` is the person it was sent to: pending and unexpired, or accepted + * while they are still in its workspace, so its link leads back in. Anything + * else is hidden, so a link cannot be probed, and leaving a workspace ends + * what its link can do. */ function invitationFor(userId: string, invitationId: string) { return Effect.gen(function* () { @@ -748,6 +784,8 @@ function invitationFor(userId: string, invitationId: string) { const [invitation] = yield* query((db) => db .select({ + status: workspaceInvite.status, + stillIn: workspaceMember.id, workspaceId: workspaceInvite.workspaceId, email: workspaceInvite.email, role: workspaceInvite.role, @@ -757,16 +795,27 @@ function invitationFor(userId: string, invitationId: string) { .from(workspaceInvite) .innerJoin(workspace, eq(workspace.id, workspaceInvite.workspaceId)) .innerJoin(user, eq(user.id, workspaceInvite.inviterId)) + .leftJoin( + workspaceMember, + and( + eq(workspaceMember.workspaceId, workspaceInvite.workspaceId), + eq(workspaceMember.userId, userId), + ), + ) .where( and( eq(workspaceInvite.id, invitationId), - eq(workspaceInvite.status, "pending"), - gt(workspaceInvite.expiresAt, now), + or( + and(eq(workspaceInvite.status, "pending"), gt(workspaceInvite.expiresAt, now)), + eq(workspaceInvite.status, "accepted"), + ), ), ) .for("update", { of: workspaceInvite }), ); - if (!invitation) return yield* new ResourceHidden({ resource: "invitation" }); + const hidden = new ResourceHidden({ resource: "invitation" }); + if (!invitation) return yield* hidden; + if (invitation.status === "accepted" && invitation.stillIn === null) return yield* hidden; const [invitee] = yield* query((db) => db .select({ email: user.email, emailVerified: user.emailVerified }) @@ -774,7 +823,17 @@ function invitationFor(userId: string, invitationId: string) { .where(eq(user.id, userId)), ); if (invitee?.email.toLowerCase() !== invitation.email) return yield* new NotTheInvitee(); - return { ...invitation, inviteeVerified: invitee.emailVerified }; + if (invitation.status === "accepted") { + return { status: "accepted" as const, workspaceId: invitation.workspaceId }; + } + return { + status: "pending" as const, + workspaceId: invitation.workspaceId, + role: invitation.role, + workspaceName: invitation.workspaceName, + inviterName: invitation.inviterName, + inviteeVerified: invitee.emailVerified, + }; }); } diff --git a/packages/core/src/workspaces/onboarding/onboarding.test.ts b/packages/core/src/workspaces/onboarding/onboarding.test.ts index a55a57f8..5477ecee 100644 --- a/packages/core/src/workspaces/onboarding/onboarding.test.ts +++ b/packages/core/src/workspaces/onboarding/onboarding.test.ts @@ -1,4 +1,4 @@ -import { and, eq } from "drizzle-orm"; +import { eq } from "drizzle-orm"; import { Effect } from "effect"; import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest"; import { ActionForbidden } from "../../authorization/access.ts"; @@ -10,7 +10,6 @@ import { user, workspace, workspaceDefaultModel, - workspaceInvite, workspaceMember, } from "../../database/schema.ts"; import { closeDatabase, onDatabase, runOnPostgres } from "../../database/testing.ts"; @@ -22,7 +21,6 @@ describe.skipIf(!process.env.DATABASE_URL)("onboarding, against Postgres", () => let workspaceId: string; let adminId: string; let memberId: string; - let memberEmail: string; let podId: string; let customAgentId: string; let systemAgentId: string; @@ -38,7 +36,6 @@ describe.skipIf(!process.env.DATABASE_URL)("onboarding, against Postgres", () => const as = (userId: string) => onPostgresAs(userId)({ complete: onboarding.complete, - completeAcceptedInvite: onboarding.completeAcceptedInvite, }); const complete = (userId: string, agentId: string) => as(userId).complete({ workspaceId, podId, agentId }); @@ -66,7 +63,6 @@ describe.skipIf(!process.env.DATABASE_URL)("onboarding, against Postgres", () => workspaceId = madeWorkspace.id; adminId = admin.id; memberId = member.id; - memberEmail = member.email; await onDatabase((db) => db.insert(workspaceMember).values([ { workspaceId, userId: adminId, role: "admin" }, @@ -203,43 +199,4 @@ describe.skipIf(!process.env.DATABASE_URL)("onboarding, against Postgres", () => ); await expect(complete(memberId, customAgentId)).rejects.toBeInstanceOf(ActionForbidden); }); - - it("completes an account from its accepted invitation", async () => { - // A workspace people are invited into has been set up, so it has a default. - await onDatabase((db) => - db.insert(workspaceDefaultModel).values({ workspaceId, modelId: "model" }), - ); - const [invitation] = await onDatabase((db) => - db - .insert(workspaceInvite) - .values({ - workspaceId, - email: memberEmail, - role: "member", - status: "accepted", - inviterId: adminId, - expiresAt: new Date(Date.now() + 60_000), - }) - .returning(), - ); - if (!invitation) throw new Error("could not create invitation"); - - expect(await as(memberId).completeAcceptedInvite({ invitationId: invitation.id })).toBe( - workspaceId, - ); - const [assistant] = await onDatabase((db) => - db - .select({ model: agent.model }) - .from(agent) - .innerJoin(pod, eq(pod.id, agent.podId)) - .where(and(eq(pod.ownerId, memberId), eq(agent.provisionedKey, "personal-assistant"))), - ); - expect(assistant?.model).toBe("model"); - }); - - it("refuses an invitation this account has not accepted", async () => { - await expect( - as(memberId).completeAcceptedInvite({ invitationId: crypto.randomUUID() }), - ).rejects.toBeInstanceOf(Onboarding.InvitationNotAccepted); - }); }); diff --git a/packages/core/src/workspaces/onboarding/onboarding.ts b/packages/core/src/workspaces/onboarding/onboarding.ts index e27f0527..54a50fef 100644 --- a/packages/core/src/workspaces/onboarding/onboarding.ts +++ b/packages/core/src/workspaces/onboarding/onboarding.ts @@ -4,30 +4,18 @@ import { and, eq, isNull } from "drizzle-orm"; import { Context, Data, DateTime, Effect, Layer } from "effect"; import type { AuthorizationDenied } from "../../authorization/access.ts"; import { Authorization } from "../../authorization/authorization.ts"; -import { CurrentActor } from "../../authorization/current-actor.ts"; +import type { CurrentActor } from "../../authorization/current-actor.ts"; import { query, serviceOperations, transaction } from "../../database/database.ts"; -import { - agent, - pod, - podMember, - user, - workspace, - workspaceInvite, - workspaceMember, -} from "../../database/schema.ts"; +import { agent, pod, podMember, workspace } from "../../database/schema.ts"; import { holdingModel } from "../../providers/model-providers/held-models.ts"; -import { offeredModels } from "../../providers/model-providers/model-provider-reads.ts"; import { ModelProviderRepository } from "../../providers/model-providers/model-provider-repository.ts"; import { type UserFacing, UserMessage } from "../../user-message.ts"; import { AgentRepository } from "../agents/agent-repository.ts"; -import { PersonalPods } from "../pods/personal-pods.ts"; /** - * The current actor finishing setting up a workspace, or joining one by - * invitation. - * - * Both are transactional and lock the row they check, so the check still holds - * when the writes that follow it land. + * The current actor finishing setting up a workspace. It is transactional and + * locks the rows it checks, so the check still holds when the writes that + * follow it land. */ export interface Interface { /** @@ -51,18 +39,6 @@ export interface Interface { AuthorizationDenied | NotReadyToFinish | NoModelChosen, CurrentActor.Service >; - /** - * Sets up an actor who joined through `invitationId`, pointing their - * Personal Assistant at the workspace's default model, or another it offers - * while it does not offer that one. Returns the workspace they joined. - */ - readonly completeAcceptedInvite: (input: { - invitationId: string; - }) => Effect.Effect< - string, - InvitationNotAccepted | ModelProviderRepository.ModelNotEnabled, - CurrentActor.Service - >; } export class Service extends Context.Service()("@sugabots/core/Onboarding") {} @@ -70,7 +46,6 @@ export class Service extends Context.Service()("@sugabots/co export const make = Effect.gen(function* () { const operation = yield* serviceOperations("Onboarding"); const authorization = yield* Authorization.Service; - const personalPods = yield* PersonalPods.Service; const modelProviders = yield* ModelProviderRepository.Service; const agents = yield* AgentRepository.Service; @@ -124,61 +99,13 @@ export const make = Effect.gen(function* () { }), ), ), - - completeAcceptedInvite: ({ invitationId }) => - operation( - "completeAcceptedInvite", - transaction( - Effect.gen(function* () { - const { userId } = yield* CurrentActor.Service; - const [accepted] = yield* query((db) => - db - .select({ workspaceId: workspaceInvite.workspaceId }) - .from(workspaceInvite) - .innerJoin(user, and(eq(user.id, userId), eq(user.email, workspaceInvite.email))) - .innerJoin( - workspaceMember, - and( - eq(workspaceMember.workspaceId, workspaceInvite.workspaceId), - eq(workspaceMember.userId, userId), - ), - ) - .where( - and(eq(workspaceInvite.id, invitationId), eq(workspaceInvite.status, "accepted")), - ) - .limit(1) - .for("update"), - ); - if (!accepted) { - return yield* new InvitationNotAccepted(); - } - const workspaceId = accepted.workspaceId; - const { models, defaultModel } = yield* offeredModels(workspaceId); - // The default goes unoffered while a failed test has its provider off. - const model = models.some((offered) => offered.modelId === defaultModel) - ? defaultModel - : models[0]?.modelId; - if (model) { - yield* personalPods.provisionWithModel({ workspaceId, userId, model }); - } else { - yield* personalPods.provision({ workspaceId, userId }); - } - return workspaceId; - }), - ), - ), }); }); export const layerNoDeps = Layer.effect(Service, make); export const layer = layerNoDeps.pipe( - Layer.provide([ - Authorization.layer, - PersonalPods.layer, - ModelProviderRepository.layer, - AgentRepository.layer, - ]), + Layer.provide([Authorization.layer, ModelProviderRepository.layer, AgentRepository.layer]), ); /** The pod and agent named are not ones this person may finish onboarding with. */ @@ -195,16 +122,6 @@ export class NoModelChosen extends Data.TaggedError("NoModelChosen") implements } } -/** No invitation this account accepted has that id. */ -export class InvitationNotAccepted - extends Data.TaggedError("InvitationNotAccepted") - implements UserFacing -{ - get userMessage() { - return UserMessage.of`The invitation has not been accepted by this account`; - } -} - /** Records that `workspaceId`'s setup is complete, unless it already was. */ export const recordSetupCompleted = (workspaceId: string) => Effect.flatMap(DateTime.nowAsDate, (now) => diff --git a/packages/server/src/routes/onboarding/routes.test.ts b/packages/server/src/routes/onboarding/routes.test.ts index 61597944..cba75005 100644 --- a/packages/server/src/routes/onboarding/routes.test.ts +++ b/packages/server/src/routes/onboarding/routes.test.ts @@ -1,8 +1,7 @@ -import { CurrentActor } from "@sugabots/core/authorization/current-actor"; import { unimplemented } from "@sugabots/core/testing"; import { Onboarding } from "@sugabots/core/workspaces/onboarding/onboarding"; import { Effect, Layer } from "effect"; -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import type { UserResolver } from "../../http/app.test-support.ts"; import { createTestApp, identifiedBy } from "../../http/app.test-support.ts"; @@ -10,7 +9,6 @@ const USER_ID = "0199a3a0-0000-7000-8000-0000000000ff"; const WORKSPACE_ID = "0199a3a0-0000-7000-8000-000000000001"; const POD_ID = "0199a3a0-0000-7000-8000-000000000002"; const AGENT_ID = "0199a3a0-0000-7000-8000-000000000003"; -const INVITATION_ID = "0199a3a0-0000-7000-8000-000000000004"; const resolveUser: UserResolver = async (headers) => headers.get("authorization") === "Bearer good-token" @@ -40,25 +38,4 @@ describe("onboarding routes", () => { message: "Finish creating your pod and agent first", }); }); - - it("returns the workspace belonging to an accepted invitation", async () => { - let askedAs: string | undefined; - const completeAcceptedInvite = vi.fn(() => - Effect.map(CurrentActor.Service, ({ userId }) => { - askedAs = userId; - return WORKSPACE_ID; - }), - ); - - const response = await app({ completeAcceptedInvite }).request("/onboarding/complete-invite", { - method: "POST", - headers: authorization, - body: JSON.stringify({ invitationId: INVITATION_ID }), - }); - - expect(response.status).toBe(200); - expect(await response.json()).toEqual({ workspaceId: WORKSPACE_ID }); - expect(completeAcceptedInvite).toHaveBeenCalledWith({ invitationId: INVITATION_ID }); - expect(askedAs).toBe(USER_ID); - }); }); diff --git a/packages/server/src/routes/onboarding/routes.ts b/packages/server/src/routes/onboarding/routes.ts index f169bbec..a01c7d58 100644 --- a/packages/server/src/routes/onboarding/routes.ts +++ b/packages/server/src/routes/onboarding/routes.ts @@ -6,21 +6,13 @@ import { asSessionUser } from "../../auth/middleware.ts"; import { ServerApi } from "../../http/api.ts"; import { asHttpError, refusals } from "../../http/errors.ts"; -/** The signed-in person finishing setting up a workspace, or joining one by invitation. */ +/** The signed-in person finishing setting up a workspace. */ export const onboardingRoutes = HttpApiBuilder.group(ServerApi, "onboarding", (handlers) => Effect.gen(function* () { const onboarding = yield* Onboarding.Service; - return handlers - .handle("complete", ({ payload }) => - onboarding.complete(payload).pipe(asSessionUser, asHttpError(onboardingErrors)), - ) - .handle("completeInvite", ({ payload }) => - onboarding.completeAcceptedInvite(payload).pipe( - Effect.map((workspaceId) => ({ workspaceId })), - asSessionUser, - asHttpError(onboardingErrors), - ), - ); + return handlers.handle("complete", ({ payload }) => + onboarding.complete(payload).pipe(asSessionUser, asHttpError(onboardingErrors)), + ); }), ); @@ -28,6 +20,4 @@ const onboardingErrors = { ...refusals, NotReadyToFinish: BadRequest, NoModelChosen: BadRequest, - InvitationNotAccepted: BadRequest, - ModelNotEnabled: BadRequest, }; diff --git a/packages/web/src/router.tsx b/packages/web/src/router.tsx index 9c7ed3bc..caa2a826 100644 --- a/packages/web/src/router.tsx +++ b/packages/web/src/router.tsx @@ -1,4 +1,4 @@ -import type { Agent, Pod, SessionUser, Workspace } from "@sugabots/contracts"; +import type { Agent, Pod, SessionUser, Workspace, WorkspacePermissions } from "@sugabots/contracts"; import { CONNECTION_SIGN_IN_RETURN_PATH } from "@sugabots/contracts"; import { type QueryClient, useQuery, useQueryClient } from "@tanstack/react-query"; import type { ParsedLocation, RouterHistory } from "@tanstack/react-router"; @@ -191,20 +191,22 @@ const indexRoute = createRoute({ }); /** - * Opens the chosen workspace if it is set up, and otherwise one that is. One - * still being set up opens when asked for by its address or the switcher, or - * here when there is no other. + * Opens the chosen workspace, unless that would take the caller into its setup + * and another is set up. One left part way then opens when asked for by its + * address or the switcher, or here when there is no other. */ function LandingRoute() { const { workspace: chosen, isPending, error, refetch } = useWorkspace(); const workspaces = useWorkspaces(); + const standing = useWorkspaceStanding(); if (isPending) return
; if (error) return ; if (!chosen) return ; - const landing = - chosen.setupCompletedAt !== null - ? chosen - : (workspaces.data?.find((one) => one.setupCompletedAt !== null) ?? chosen); + if (chosen.setupCompletedAt === null && standing.isPending) { + return
; + } + const setUp = workspaces.data?.find((one) => one.setupCompletedAt !== null); + const landing = setUp && opensIntoSetup(chosen, standing.data?.permissions) ? setUp : chosen; return ; } @@ -537,15 +539,25 @@ function ShellRoute() { if (standing.error) { return ; } - // Finishing setup takes `workspace.providers.manage`. - const mayFinishSetup = standing.data?.permissions.manageProviders; - if (mayFinishSetup) { + if (opensIntoSetup(workspace.workspace, standing.data?.permissions)) { return ; } } return ; } +/** + * Whether opening `workspace` takes the caller into its setup instead: it is + * still being set up, and they may finish it, which takes + * `workspace.providers.manage`. + */ +function opensIntoSetup( + workspace: Workspace, + permissions: WorkspacePermissions | undefined, +): boolean { + return workspace.setupCompletedAt === null && permissions?.manageProviders === true; +} + function RouteLoadFailure({ title, onRetry }: { title: string; onRetry: () => Promise }) { return (
diff --git a/packages/web/src/screens/Invite.stories.tsx b/packages/web/src/screens/Invite.stories.tsx index 79a50388..bd30e56f 100644 --- a/packages/web/src/screens/Invite.stories.tsx +++ b/packages/web/src/screens/Invite.stories.tsx @@ -8,17 +8,12 @@ import { Invite } from "./Invite.tsx"; /* * The other end of an invitation, once signed in: whose workspace it is and - * Accept, or why it will not work. It first tries to finish an invitation - * already accepted, which a fresh one refuses, then reads the invitation. + * Accept, or why it will not work. */ const API = import.meta.env.VITE_API_URL; const ID = "0199a3a0-0000-7000-8000-0000000000e1"; -const notYetAccepted = http.post(`${API}/onboarding/complete-invite`, () => - HttpResponse.json({ _tag: "BadRequest", message: "No accepted invitation" }, { status: 400 }), -); - function Cache({ children }: { children: ReactNode }) { const [client] = useState(() => createQueryClient()); return {children}; @@ -43,9 +38,8 @@ const meta = preview.meta({ export const Invited = meta.story({ beforeEach({ msw }) { msw.use( - notYetAccepted, http.get(`${API}/invitations/:invitationId`, () => - HttpResponse.json({ workspaceName: "Nitric", inviterName: "Jay Young" }), + HttpResponse.json({ status: "pending", workspaceName: "Nitric", inviterName: "Jay Young" }), ), ); }, @@ -61,7 +55,6 @@ export const Invited = meta.story({ export const WrongAccount = meta.story({ beforeEach({ msw }) { msw.use( - notYetAccepted, http.get(`${API}/invitations/:invitationId`, () => HttpResponse.json( { _tag: "Forbidden", message: "This invitation was sent to a different address" }, diff --git a/packages/web/src/screens/Invite.tsx b/packages/web/src/screens/Invite.tsx index ac94a578..e1d81400 100644 --- a/packages/web/src/screens/Invite.tsx +++ b/packages/web/src/screens/Invite.tsx @@ -4,7 +4,7 @@ import { Effect } from "effect"; import { useEffect, useEffectEvent, useState } from "react"; import { client } from "@/api.ts"; import { failureMessage } from "@/lib/failure.ts"; -import { chooseWorkspace } from "@/lib/workspace.ts"; +import { chooseWorkspace, workspacesQuery } from "@/lib/workspace.ts"; import { AuthLayout } from "@/screens/AuthLayout.tsx"; import { Alert } from "@/ui/alert.tsx"; import { Button } from "@/ui/button.tsx"; @@ -38,24 +38,23 @@ export function Invite({ id, onDone }: { id: string; onDone: () => Promise const [workspace, setWorkspace] = useState(); const [error, setError] = useState(); const [busy, setBusy] = useState(false); - const [accepted, setAccepted] = useState(false); - const resumeAcceptance = useEffectEvent(() => finishAcceptance(false)); + /** The workspace joined, once it has been, so continuing after a failure does not accept again. */ + const [joined, setJoined] = useState(); + const enterAccepted = useEffectEvent((workspaceId: string) => enter(workspaceId)); useEffect(() => { let current = true; - - void (async () => { - if (await resumeAcceptance()) return; - Effect.runPromise(client.api.workspaces.invitation({ params: { invitationId: id } })).then( - (invitation) => { - if (current) setWorkspace(invitation.workspaceName); - }, - (failure: unknown) => { - if (current) setError(invitationFailureMessage(failure)); - }, - ); - })(); - + Effect.runPromise(client.api.workspaces.invitation({ params: { invitationId: id } })).then( + (invitation) => { + if (!current) return; + // A link opened again after accepting leads back into the workspace. + if (invitation.status === "accepted") void enterAccepted(invitation.workspaceId); + else setWorkspace(invitation.workspaceName); + }, + (failure: unknown) => { + if (current) setError(invitationFailureMessage(failure)); + }, + ); return () => { current = false; }; @@ -64,51 +63,42 @@ export function Invite({ id, onDone }: { id: string; onDone: () => Promise async function accept() { setBusy(true); setError(undefined); + let workspaceId: string; try { - await Effect.runPromise( + ({ workspaceId } = await Effect.runPromise( client.api.workspaces.acceptInvitation({ params: { invitationId: id } }), - ); + )); } catch (failure) { setError(invitationFailureMessage(failure)); setBusy(false); return; } - - setAccepted(true); - await finishAcceptance(); + await enter(workspaceId); } - async function finishAcceptance(reportFailure = true): Promise { - let confirmed = false; + /** Opens the app on the workspace joined. */ + async function enter(workspaceId: string) { + setJoined(workspaceId); + setBusy(true); try { - const result = await Effect.runPromise( - client.api.onboarding.completeInvite({ payload: { invitationId: id } }), - ); - confirmed = true; - setAccepted(true); - chooseWorkspace(result.workspaceId); - await queries.invalidateQueries({ queryKey: ["workspaces"] }); + chooseWorkspace(workspaceId); + // Nothing here reads the list, but the page this leads to does, and + // it has to find the workspace just joined there to open it. + await queries.invalidateQueries({ queryKey: workspacesQuery.queryKey, refetchType: "all" }); await onDone(); - return true; } catch (failure) { - if (reportFailure || confirmed) { - setError( - `The invitation was accepted, but we could not continue. ${failureMessage(failure)}`, - ); - } + setError( + `The invitation was accepted, but we could not continue. ${failureMessage(failure)}`, + ); setBusy(false); - return confirmed; } } async function leave() { + if (joined) return enter(joined); setBusy(true); try { - if (accepted) { - await finishAcceptance(); - } else { - await onDone(); - } + await onDone(); } catch (failure) { setError(failureMessage(failure)); setBusy(false); @@ -117,7 +107,7 @@ export function Invite({ id, onDone }: { id: string; onDone: () => Promise if (error !== undefined) { return ( - + {error} ); diff --git a/packages/web/src/shell/shell.test.tsx b/packages/web/src/shell/shell.test.tsx index 9dc6c009..903c4802 100644 --- a/packages/web/src/shell/shell.test.tsx +++ b/packages/web/src/shell/shell.test.tsx @@ -6,7 +6,7 @@ import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-li import { Effect } from "effect"; import { afterEach, beforeEach, describe, expect, it, onTestFinished, vi } from "vitest"; import { createQueryClient } from "@/lib/query.ts"; -import { chooseWorkspace } from "@/lib/workspace.ts"; +import { chooseWorkspace, workspacesQuery } from "@/lib/workspace.ts"; import { createAppRouter } from "@/router.tsx"; import { agents, @@ -203,7 +203,7 @@ describe("the workspace choice", () => { client.api.agents.list.mockReturnValue( Effect.succeed([{ ...personalAssistant, model: "switched-off" }]), ); - const router = mount("/"); + const router = mount(`/${workspace.slug}/agents`); fireEvent.click(await screen.findByRole("button", { name: "Try again" })); expect(screen.queryByRole("navigation", { name: "Pods" })).toBeNull(); @@ -244,6 +244,17 @@ describe("the workspace choice", () => { expect(router.state.location.pathname).toMatch(/^\/suga\//); }); + it("opens an unfinished workspace from the root for somebody invited part way, who has their own", async () => { + apiAnswers({ role: "member" }); + const joined = { ...other, setupCompletedAt: null }; + client.api.workspaces.list.mockReturnValue(Effect.succeed([workspace, joined])); + chooseWorkspace(joined.id); + const router = mount("/"); + + await waitFor(() => expect(router.state.location.pathname).toMatch(/^\/nitric\//)); + expect(screen.queryByRole("heading", { name: "Connect a provider" })).toBeNull(); + }); + it("comes back to a workspace whose save a reload interrupted", async () => { client.api.workspaces.list.mockReturnValue(Effect.succeed([])); client.api.workspaces.create.mockReturnValue(Effect.never); @@ -921,10 +932,7 @@ describe("routes", () => { it("still honours the older ?invite= link shape", async () => { client.api.workspaces.invitation.mockReturnValue( - Effect.succeed({ - workspaceName: "Nitric", - inviterName: "Sam", - }), + Effect.succeed({ status: "pending", workspaceName: "Nitric", inviterName: "Sam" }), ); const router = mount("/?invite=an-invitation"); @@ -1039,12 +1047,8 @@ describe("routes", () => { it("does not retry an accepted invitation when continuing initially fails", async () => { client.api.workspaces.invitation.mockReturnValue( - Effect.succeed({ - workspaceName: "Nitric", - inviterName: "Sam", - }), + Effect.succeed({ status: "pending", workspaceName: "Nitric", inviterName: "Sam" }), ); - client.api.workspaces.acceptInvitation.mockReturnValue(Effect.succeed(undefined)); const refresh = vi .fn<() => Promise>() .mockRejectedValueOnce(new Error("Offline")) @@ -1065,9 +1069,44 @@ describe("routes", () => { expect(refresh).toHaveBeenCalledTimes(2); }); - it("resumes an accepted invitation after the page was reloaded", async () => { - client.api.onboarding.completeInvite.mockReturnValue( - Effect.succeed({ workspaceId: workspace.id }), + it("opens the workspace just joined, though the list the session started with lacks it", async () => { + const joined = { + id: "0199a3a0-0000-7000-8000-0000000000f9", + name: "Nitric", + slug: "nitric", + timeZone: "UTC", + setupCompletedAt: "2026-09-01T00:00:00.000Z", + }; + client.api.workspaces.invitation.mockReturnValue( + Effect.succeed({ status: "pending", workspaceName: "Nitric", inviterName: "Sam" }), + ); + client.api.workspaces.acceptInvitation.mockImplementation(() => { + client.api.workspaces.list.mockReturnValue(Effect.succeed([workspace, joined])); + return Effect.succeed({ workspaceId: joined.id }); + }); + // As main.tsx starts a session: with the workspaces /me answered with. + const queries = createQueryClient(); + queries.setQueryData(workspacesQuery.queryKey, [workspace]); + const router = createAppRouter({ + history: createMemoryHistory({ initialEntries: ["/invite/an-invitation"] }), + }); + render( + , + ); + + await screen.findByText("You have been invited to Nitric."); + fireEvent.click(screen.getByRole("button", { name: "Accept invite" })); + + await waitFor(() => expect(router.state.location.pathname).toMatch(/^\/nitric\//)); + }); + + it("leads back into the workspace from an invitation already accepted", async () => { + client.api.workspaces.invitation.mockReturnValue( + Effect.succeed({ status: "accepted", workspaceId: workspace.id }), ); const refresh = vi.fn().mockResolvedValue(undefined); const router = mount("/invite/an-invitation", sam, refresh); diff --git a/packages/web/src/test-api.tsx b/packages/web/src/test-api.tsx index 85dc04e2..886d4096 100644 --- a/packages/web/src/test-api.tsx +++ b/packages/web/src/test-api.tsx @@ -10,7 +10,7 @@ import { type SystemAgent, type WorkspaceRole, } from "@sugabots/contracts"; -import { BadRequest, NotFound } from "@sugabots/contracts/http"; +import { NotFound } from "@sugabots/contracts/http"; import { connectionWithTools, listedConnection, @@ -367,10 +367,8 @@ export function apiAnswers({ role = "admin" }: { role?: WorkspaceRole } = {}): v }, ]), ); - client.api.onboarding.completeInvite.mockImplementation(() => - client.api.workspaces.acceptInvitation.mock.calls.length > 0 - ? Effect.succeed({ workspaceId: WORKSPACE }) - : Effect.fail(new BadRequest({ message: "Pending" })), + client.api.workspaces.acceptInvitation.mockReturnValue( + Effect.succeed({ workspaceId: WORKSPACE }), ); const inPod = administersWorkspace(role) ? ADMIN_IN_POD diff --git a/packages/web/src/test-client.ts b/packages/web/src/test-client.ts index 4d075b8d..3e546bf0 100644 --- a/packages/web/src/test-client.ts +++ b/packages/web/src/test-client.ts @@ -61,7 +61,6 @@ export const client = { }, onboarding: { complete: vi.fn(), - completeInvite: vi.fn(), }, systemAgents: { list: vi.fn(),