Workspace Switcher, Workspace Area, and Workspace Creation on a Brain-owned session - #352
aimeritething wants to merge 17 commits into
Conversation
…kie (AIM-444) Brain no longer takes credentials from the Desktop SDK. `POST /api/session` reads the shared `sealos_auth_token` cookie on the server and exchanges it with Desktop (`regionToken` → `namespace/list` → `namespace/switch` ∥ `auth/info`), rewrites the kubeconfig context namespace to Desktop's current Workspace, and answers Brain's own zod-validated session shape. Desktop's "HTTP 200 + body.code" envelope is translated into real statuses (401 / 409 / 502 / 504); nothing logs a token. Client side, the three credentials plus the Workspace list, current Workspace, and user live only in Jotai atoms. `SessionBootstrap` replaces `AuthBootstrap` / `SealosSdkBootstrap`: the SDK reading layer returns only `nsid` (and language / host domain), never credentials. Workspace-management fetchers go through `createSessionFetch`, which attaches `X-Sealos-Region-Token` and runs the 401 two-step (silent re-exchange, one retry, then the click-through "Session expired" overlay that hands `window.top` to Desktop `/signin`). Every credential-keyed SWR key is now built in `features/session/swr-keys.ts`, with a test that walks them all. Local development runs the real path against a staging Desktop via `DESKTOP_API_BASE_URL` + `DEV_GLOBAL_TOKEN` (dev builds only); the self-signed pair (`NEXT_PUBLIC_DEV_ENCODED_KUBECONFIG`, `NEXT_PUBLIC_DEV_APP_TOKEN`, the mint script, `hasDevCredentialBypass()`) is gone. A session dev-mock (Owner / Manager / Developer / Personal-only) answers `/api/session` from fixtures when selected. The chart derives `DESKTOP_API_BASE_URL` to the in-cluster Desktop Service and documents Desktop's `allowedOrigins` requirement. Docs: CONTEXT.md (Workspace, Workspace Role, Workspace Area, Managed Workspace, Workspace Invite Link, Brain Session, Workspace Switcher, Workspace Creation), ADR-0083 (Proposed), ADR-0059 status note, ADR index. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- memoize useSessionCredentials so hook dependencies stay stable (the
GitHub auth hook was re-subscribing every render)
- render the generic session error (409 / 502 / 504) as an overlay with
a reload, beside the session-expired overlay
- map only regionToken's 401 to session-expired; a 401 on a token Desktop
just minted is a Desktop anomaly (502)
- pick the Personal Workspace by the list's nstype before the token claim
- answer 400 for a body that is not JSON instead of treating it as {}
- fall back to the referrer origin for the Desktop sign-in target when the
host config never answered inside the iframe
- session dev-mock: an unknown nsid lands in Personal, like the real path
- share one cookie-header parser between the login cookie and dev-mock
cookies; build region-token headers on a Headers instance; drop the
unused establish dependency and type exports; document the smoke
script's kubeconfig variable
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-445) The App Sidebar's top becomes the brand slot plus the Workspace Switcher (spec AIM-443 §C, §H.2, §J.1, §B.2 `list`): - Brand row: the logo slot is the only collapse / expand control. It crossfades into the PanelLeft glyph while the pointer is over the sidebar or the button has focus (200ms `--ease-out-strong`, opacity / scale / blur; opacity only under reduced motion). One element in both states: `data-slot`, `aria-label`, and `aria-expanded` flip, so focus stays put. No wordmark, no separate collapse button, tooltip only in the rail. - Workspace Switcher row (`data-slot="app-sidebar-workspace"`): square avatar (`WorkspaceAvatar` gains `square`), name, plan badge or PAYG, ⇕; the rail keeps the avatar and still opens the popover. The popover shows the current Workspace card, "Switch to" (Personal first, role + plan per row), New Workspace (`/billing?mode=create`), Manage Workspaces (`/workspace/<uid>`, return address recorded). Switching hands the top window to Desktop's `?openapp=system-brain?<path>?<query> &workspaceUid=` deep link; the Billing and Workspace Areas keep their page, everywhere else lands on `/project`. Outside the Desktop iframe the rows are disabled with a notice. - Data: the list comes from the session atoms and stays fresh through `GET /api/workspace/list` (route table + on-disk guard test, session dev-mock answers it); plan names come from the new Brain route `GET /api/billing/workspace-plans`, read per Workspace with the internal JWT (account-service allows any member to read). - The plan badge and subscription hint move from the account row to the Switcher row (two-line state on payment-due / cancelling). - `WorkspaceSubscriptionRole` retired: the notification read dispatch and Billing's `canManage` (Owner only) read the session's Workspace Role. - `/project/<uid>` guard: a loaded list without the Project replaces the route with `/project` and toasts; nothing is judged while loading. - Billing's return-route module generalised into an area return route shared with the Workspace Area. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- The switch deep link takes its cloud domain only from the SDK host config (spec §C.6): the referrer fallback let the "Switch to" rows run under the local Dev Bridge, whose origin is not Desktop. Inside Desktop before the host config answers the rows wait with their own notice. - The `/project/<uid>` guard revalidates the Project list once before leaving, so a Project created in another tab is never bounced by the stale SWR cache. - The Workspace routes share the session's error codes for the codes they have in common; the plan-name rule is one module used by the route handler and the billing Dev Mock; `blur-none` replaces `blur-[0px]`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Workspace Area lands at `/workspace` and `/workspace/<uid>` (spec
AIM-443 §D.1–D.5, §D.9, §E, §B.2 `details`), rendering every control
its gates allow while the write operations wait for the next ticket:
- Area shell: the Billing Area's title bar, close button, and aside are
extracted into `AreaShell`, parameterized by icon, title, close label,
return-route reader, and aside width; Billing composes it unchanged.
- Routes: `/workspace` replaces itself with the current Workspace's
uid; a uid outside the list falls back to it with a notice. The close
button returns to the recorded entry point, home on a direct entry.
- `POST /api/workspace/details { uid }`: Desktop `namespace/details`
through the Desktop client, answered as `{ workspace, members }` in
Brain's shape with Desktop's envelope codes translated; joined the
route table, its on-disk guard, and the session dev-mock (members per
scenario, 404 for a uid outside the list).
- Gating (`workspace-gating-core`): a pure module mirroring Desktop's
`vaildManage` — role gates hide, state gates disable with a reason
(delete / leave the current Workspace, transfer with nobody to
transfer to), Personal never deleted or transferred, Owner never
removed or self-removed, roles offered never Owner.
- UI: the 240px list (Desktop order, current dot, role, Create row),
the detail header (square avatar, plan badge, Current, role line,
copyable id, Leave or the Owner's ⋯ menu with reasons on a second
line), and the Members panel (You badge, alias subline and pencil,
role select or text, Joined, remove icon; the action column only
when someone is removable; no Status column).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ea (AIM-446) - The ⋯ menu stays disabled until the members landed, so transfer's member-count gate is never guessed (was assumed transferable). - Desktop epoch timestamps become ISO text (were stringified numbers that rendered "-" in Joined). - One PlanSlot / workspaceRoleLabel / planNameFor shared by the Switcher and the area; an id absent from the plans record means "unknown" in both (the area showed PAYG). - Member gates computed once per row; copy-id and alias controls are AppButtons; "Members · N" carries its separator; comments on the Personal-invite gate, Desktop's `details` isPersonal, and the §D.5 column widths. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner actions, member management, and the Workspace Invite Link take
effect in the Workspace Area (spec AIM-443 §B.2, §D.6–D.8, §E, §F).
Server: seven write routes join the route table — rename, delete,
invite-link (answers `{ code }`), member/remove, member/role,
member/alias, transfer — each validating its body with a shared schema
(roles never Owner, alias trimmed and capped at 128, name at 32),
requiring `X-Sealos-Region-Token`, calling Desktop through the auth API
module, and translating Desktop's envelope codes into Brain's own
statuses and error codes. The dev-mock fixtures become mutable per
scenario so every write lands in memory and shows on the next read.
Client: the detail header's ⋯ menu and Leave button and the members
table's role select, alias pencil, remove icon, and Invite member button
open the dialogs the spec's three confirmation tiers call for — typed
name for delete and transfer (with "You will become a Developer"), plain
confirmation for remove and leave, direct effect for rename, alias, and
role. After a write the list and members are re-read (the fresh list is
written back to the session atoms), a delete or leave moves the page to
the current Workspace, and a 403 / 404 re-reads, re-gates, and says the
permissions changed. The invite dialog builds Desktop's
`/WorkspaceInvite/?code=` link on the cloud domain and copies it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…(AIM-447) - The "departed" mark that suppresses the list fallback after a delete or leave is spent the one time it is met, so a later visit to that uid gets the ordinary redirect and notice again. - Dialogs ignore Escape and the overlay while a write is in flight, as Cancel already did, so the outcome lands in a mounted dialog. - The invite dialog says so when the clipboard refused, instead of showing the link silently; the transfer dialog no longer claims billing follows. - Rename explains the 32-character cap inline rather than failing with the generic notice on a longer Desktop-made name. - One `memberDisplayName` next to the member type replaces four copies; `inviteRoleOptions` and `ASSIGNABLE_ROLES` are typed as assignable roles, dropping a cast; unused request types and a duplicated success constant go; the confirm field uses colour tokens; the dev-mock answers 404 for an unknown Workspace on every write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`/billing?mode=create` opens the Billing Area's creation mode beside
`mode=upgrade` (spec AIM-443 §G): the Workspace Switcher's New Workspace
row and the Workspace Area's Create Workspace row already led here.
Client: a creation dialog with the name field and the Plan Picker on one
screen — every paid plan reads Subscribe, Free stays out — that is never
gated by the current Workspace's role or lifecycle. Picking a plan checks
the name inline (required, trimmed, at most 32 characters, no
case-insensitive duplicate of a Workspace the session lists) and stacks a
confirmation over the picker; confirming runs Brain's two steps and hands
the top window to Stripe Checkout as a whole-page hop, recording which
Workspace this tab is creating. Desktop's 409 lands on the field; a
failed second step becomes the offer to retry the payment or leave the
created Workspace Pay-As-You-Go for now. On the Stripe return the
recorded creation words the congratulations as "Workspace created", and
the Billing Area's return route is forgotten — it named the Workspace
the creation left — so close goes home.
Server: `POST /api/billing/workspace-create` authorizes like every
Billing route, asks Desktop `namespace/create { teamName, userType:
subscription }` with the raw app token, then account-service's pay as
Brain (`operator: created`, `payApp: system-brain`). A taken name is a
409 with a code; other Desktop failures translate without their text; a
failed Step 2 answers 200 with `payment.status = failed`, and
`/workspace-create/retry-payment` redoes Step 2 alone. Both join the
route table and the dev-mock, where the name picks the branch.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The confirm and retry buttons stay in their submitting state once the top window is handed to Stripe, so a second press during the unload cannot create a second Workspace. Only a creation's Stripe return forgets the Billing Area's return route — the page reads the pending-creation record, not the mere `stripeState` — so a plan change's top-level return still closes to where the user came from. The payment terms admit Stripe only: a balance payment would settle without a redirect, which the routes read as a failed payment and offer to retry. Cleanups: the two handlers share one authorize-then-parse preamble, the handler's payment fields derive from the zod schema instead of mirroring it, the client passes its credentials through, and the dev-mock's payment helper names what makes it fail. The retry route documents that account-service, not Brain, vouches for the Workspace it subscribes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@cursoragent review this |
|
Posted the review on PR #352: a summary comment plus eight line comments. The session model is the right one (server cookie exchange, in-memory credentials, Desktop handoff for switch). Before merge I’d close: CSRF on Invite still mints a Desktop code before the origin is known, and Manager→Manager is schema-legal even though the UI forbids it. Keep the PR draft until the Stripe create→return walk lands.   |
Review (AIM-444–448)The session model is the right one: Brain exchanges the Desktop login cookie on the server, holds credentials only in page memory, switches by handing Should fix
Looks good
Still uncheckedThe PR's own Stripe create → Checkout → "Workspace created" walk. I would keep this draft until that lands, and until the session CSRF / wrong-Workspace fallbacks above are closed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| export const workspaceInviteLinkRequestSchema = z.object({ | ||
| role: assignableRoleSchema, | ||
| uid: workspaceUidSchema, | ||
| }); |
There was a problem hiding this comment.
The schema comment says it closes holes Desktop leaves open (never Owner). inviteRoleOptions already limits Managers to Developer, but this still accepts role: "Manager" for any caller, the handler does not look up the actor, and Desktop's getInviteCode only rejects Owner.
A Manager can mint a Manager link through the API / mock. Resolve the actor in uid and reject role: "Manager" unless they are Owner — same class of hole already closed for Owner. Add a handler/mock test for Manager + role: "Manager" → 403.
There was a problem hiding this comment.
Closed in 36fbfe59. The handler looks up the actor on namespace/list for body.uid and 403s unless Owner; list miss and list failure do not mint. Handler + mock tests cover Manager + role: "Manager" → 403. The schema still accepts Manager on the wire so an Owner can mint it — that is the right split.
| const createLink = async () => { | ||
| const code = await onCreateLink(role); | ||
| if (code == null) { | ||
| return; | ||
| } | ||
| const url = workspaceInviteUrl({ cloudDomain, code }); | ||
| if (url == null) { | ||
| toast(INVITE_LINK_NO_DESKTOP_NOTICE); | ||
| return; |
There was a problem hiding this comment.
Desktop upserts invite codes on {inviter, workspaceUid, role} — a new code invalidates the previous one. This still POSTs first; if cloudDomain is empty the URL cannot be built, the toast fires, and the working link is gone.
If workspaceInviteUrl({ cloudDomain, code: "x" }) would be null, refuse before onCreateLink. Also: the role select stays live while pending, so an in-flight Copy can setLink a URL for the role the user already switched away from — disable the select (or ignore stale replies) while the mint is in flight.
There was a problem hiding this comment.
The empty-domain POST is closed in 36fbfe59 (refuse before onCreateLink; role select disabled={pending}). Remaining: useDesktopCloudDomain fills an empty SDK domain with the kubeconfig apiserver host, so this check almost never fires and the copied URL is still not Desktop — see the new comment on workspace-area.tsx.
| const created = await desktop.desktop.namespaceCreate( | ||
| appTokenFromRequest(request), | ||
| body.name | ||
| ); | ||
| if (!created.ok) { | ||
| log( | ||
| "Desktop workspace creation failed", | ||
| desktopFailureLogFields(created) | ||
| ); | ||
| return desktopCreateFailureResponse(created); | ||
| } |
There was a problem hiding this comment.
If Desktop created the Workspace and then this call timed out (30s) or returned a malformed envelope, Brain answers 504/502 with no workspace id. The client stays on confirm ("could not be created"). Same-name retry is Desktop 409 → picker "taken" for a Workspace the user now owns and cannot pay for.
Once Desktop has succeeded, always return { workspace, payment } (wrap Step 2 so a pay throw cannot hide the id). On 409 after a create attempt, refresh the list and offer Retry payment when the caller already owns that name as PAYG.
Related: planName is any non-empty string (workspace-creation-schema.ts). The picker hides Free; a crafted POST does not. account-service's $0 created path can succeed with no redirectUrl, which this handler maps to payment.failed. Reject unpriced/Free on the route; if pay returns success with no URL, treat as settled, not failed.
There was a problem hiding this comment.
Server path closed in 92e9f852: pay throw returns { workspace, payment: failed }; Free/unpriced 400; catalog unread 502; success with no URL is settled. Remaining: 409 recovery matches any Owner name (including Personal), not PAYG — see the new comment on the dialog.
| function referrerOrigin(): string | null { | ||
| try { | ||
| const referrer = document.referrer.trim(); | ||
| return referrer === "" ? null : new URL(referrer).origin; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** Copy for the generic session error (spec §A.3): the code, never Desktop text. */ | ||
| function sessionErrorDescription(code: string): string { | ||
| if (code === "workspace_not_inited") { | ||
| return "Your Sealos account has no Workspace in this region yet. Open Sealos Desktop to finish setting it up, then reload."; | ||
| } | ||
| if (code === "desktop_timeout") { | ||
| return "Sealos Desktop did not answer in time. Reload to try again."; | ||
| } | ||
| return "Brain could not establish a session with Sealos Desktop. Reload to try again."; | ||
| } | ||
|
|
||
| /** | ||
| * The session overlays (spec §A.3, §A.8). "Session expired" shows when the | ||
| * login cookie itself is stale — the session's own 401, or a second 401 | ||
| * after a silent re-exchange. It is click-through by design: a cross-origin | ||
| * frame cannot navigate its top window without a user gesture, so the | ||
| * button hands `window.top` to Desktop's sign-in page. Outside the Desktop | ||
| * iframe (local development, where `DEV_GLOBAL_TOKEN` stands in for the | ||
| * cookie) there is no Desktop to go to, so the button reloads once the | ||
| * token is refreshed. Any other establish failure shows the generic session | ||
| * error with a reload. | ||
| */ | ||
| export function SessionExpiredOverlay() { | ||
| const status = useAtomValue(sessionStatusAtom); | ||
| const desktopDomain = useAtomValue(desktopDomainAtom); | ||
| const inIframe = isInsideDesktopIframe(); | ||
| const signinUrl = inIframe | ||
| ? (desktopSigninUrl(desktopDomain) ?? | ||
| desktopSigninUrl(referrerOrigin() ?? "")) | ||
| : null; |
There was a problem hiding this comment.
document.referrer is not Desktop. isInsideDesktopIframe() is true for any parent, and there is no frame-ancestors policy. If the SDK host config never answers (typical in a hostile iframe), "Sign in again" assigns window.top to {referrer}/signin.
Only build a sign-in URL from the SDK cloud.domain. If it is missing, reload (or show copy with no navigation).
There was a problem hiding this comment.
Closed in 35c62c0b. Sign-in URL is only desktopSigninUrl(desktopDomainAtom) from the SDK host config; without a domain the button reloads. document.referrer is gone.
| if (verifiedMissing !== projectId) { | ||
| let cancelled = false; | ||
| refreshProjects() | ||
| .catch(() => undefined) | ||
| .then(() => { | ||
| if (!cancelled) { | ||
| setVerifiedMissing(projectId); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The comment says: revalidate once, and only leave if the fresh list still lacks the Project. .catch(() => undefined).then(() => setVerifiedMissing(projectId)) confirms on failure too. A 401/5xx/offline mutate then replace("/project") using the stale leave verdict — the case this extra round-trip was added to prevent.
Only set verifiedMissing when refresh succeeds and the resolved list still lacks the id. On failure, leave the guard in place and do not toast. The unit tests never reject refreshProjects, so they lock the happy path.
There was a problem hiding this comment.
Failure path closed in 36fbfe59 (reject → no bounce, no toast; test now actually rejects). Remaining: any defined mutate() result still confirms, without inspecting fresh.projects — see the new comment on this file.
| const result = await establishSession(store, { | ||
| nsid: shell?.nsid ?? null, | ||
| }); |
There was a problem hiding this comment.
null nsid means Personal, with no not_member toast. Inside the Desktop iframe a timed-out / empty getSession() is not "no shell" — Desktop's chrome can still be on a Team Workspace while Brain mints a Personal kubeconfig and the user creates resources in the wrong place.
If isInsideDesktopIframe() and shell is null, treat it as a session error (generic overlay / retry), not as Personal. Local-dev (no iframe) can keep the current fallback.
There was a problem hiding this comment.
Closed in 35c62c0b. Inside the iframe a missed SDK handshake sets desktop_unavailable and does not POST /api/session. Local-dev (no iframe) still lands in Personal. Locked by session-bootstrap.shell-miss.test.tsx.
| const closeStage = () => { | ||
| if (submitting) { | ||
| return; | ||
| } | ||
| // Leaving the failed-payment offer leaves the created Workspace as is | ||
| // (spec §G.6): the whole dialog closes rather than returning to a | ||
| // picker that would create a second one. | ||
| if (stage.kind === "payment-failed") { | ||
| onOpenChange(false); | ||
| return; | ||
| } | ||
| setError(null); | ||
| setStage({ kind: "pick" }); | ||
| }; |
There was a problem hiding this comment.
After Step 1 succeeded and Step 2 failed, Later just closes. existingWorkspaceNames and the Switcher both read workspacesAtom / SWR with revalidateOnFocus: false. The unpaid Workspace is invisible; a second create with a new name goes through. Spec's "PAYG, subscribe like any other" assumes it is findable.
After any successful Step 1 (including payment-failed), mutate /api/workspace/list and write workspacesAtom, same as Workspace Area writes (useWorkspaceRefresh).
There was a problem hiding this comment.
Closed on the 200-create path in 92e9f852. After started / settled / failed the dialog calls useWorkspaceRefresh (mutate /api/workspace/list + workspacesAtom), same as Workspace Area writes. Remaining: the dialog does not reset on close, so Later still leaves the payment-failed stage mounted — see the new comment on this file.
| /** The request body: absent or blank means `{}`; anything else must be JSON. */ | ||
| async function requestPayload( | ||
| request: Request | ||
| ): Promise<{ payload: unknown } | { invalid: true }> { | ||
| const text = (await request.text().catch(() => null))?.trim() ?? ""; | ||
| if (text === "") { | ||
| return { payload: {} }; | ||
| } | ||
| try { | ||
| return { payload: JSON.parse(text) }; | ||
| } catch { | ||
| return { invalid: true }; | ||
| } | ||
| } | ||
|
|
||
| export function createSessionHandler( | ||
| dependencies: SessionHandlerDependencies = {} | ||
| ): (request: Request) => Promise<Response> { | ||
| const env = dependencies.env ?? process.env; | ||
| const log: SessionLog = | ||
| dependencies.log ?? | ||
| ((message, fields) => console.warn(`[session] ${message}`, fields)); | ||
|
|
||
| return async function handler(request: Request): Promise<Response> { | ||
| const body = await requestPayload(request); | ||
| const parsed = | ||
| "invalid" in body | ||
| ? null | ||
| : sessionRequestSchema.safeParse(body.payload ?? {}); | ||
| if (parsed == null || !parsed.success) { | ||
| return errorResponse(SESSION_ERROR_CODES.invalidRequest, 400); | ||
| } |
There was a problem hiding this comment.
Cookie-authenticated POST with no Origin check, and any body is JSON-parsed (so Content-Type: text/plain still works). A same-site sibling on *.<cloudDomain> can CSRF { "nsid": "ns-…" } and trigger Desktop namespace/switch — the mutation ADR-0083 refuses to let Brain do in-place because it reloads every tab.
CSRF cannot read the session JSON (no CORS ACAO). It can still switch the user's Workspace.
Reject unless Origin is Brain's own origin, and require Content-Type: application/json so a simple request cannot carry nsid.
There was a problem hiding this comment.
The sibling text/plain+nsid switch is closed in 35c62c0b (foreign Origin 403, non-JSON body 400). Remaining: Origin: null and scheme-ignoring Host match are still treated as Brain — see the new comment on originAllowed.
- POST /api/session refuses a foreign Origin (403 session_forbidden) and
a JSON body that does not travel as application/json (400): a sibling
page on the shared cloud domain could POST { nsid } as text/plain and
trigger Desktop's namespace/switch — the mutation ADR-0083 refuses to
perform in place — while being unable to read the answer.
- The expired overlay's "Sign in again" no longer falls back to the
embedding page's referrer: without a frame-ancestors policy that page
is not proven Desktop, and the button could aim window.top at it. The
sign-in URL comes only from the SDK host config; without a domain the
button reloads.
- Inside the Desktop iframe a missed SDK handshake raises the generic
session error instead of silently landing in Personal: Desktop's shell
is the only source of the current Workspace, and a guessed Personal
kubeconfig would mint credentials for the wrong Workspace. Outside an
iframe (local development) Personal remains the landing.
… bounce
- The invite dialog refuses before the POST when the Desktop domain is
unknown: Desktop upserts the code per { inviter, workspace, role }, so
minting without a domain to build the link on would replace a working
link with one the user cannot see. The role select is disabled while a
mint is in flight, so an in-flight Copy can no longer setLink a URL for
a role the user switched away from.
- A Manager invite link now requires an Owner actor, proven server-side
against namespace/list — Desktop's getInviteCode rejects Owner but lets
a Manager mint a Manager link, the same class of hole the schema's
never-Owner rule closes. The dev-mock fixture enforces the same rule.
- The /project/<uid> guard only confirms its leave verdict when the
revalidation came back with a list that still lacks the Project: a
failed refresh (401, 5xx, offline) keeps the guard standing instead of
bouncing a Project that may exist — the case the extra round-trip was
added to prevent.
…h holes - Both creation routes validate planName against account-service's plan catalog and refuse a plan it does not price (400) — the picker hides Free, but a crafted POST did not — and fail closed (502) when the catalog cannot be read, before Desktop creates anything. - A pay answer that succeeds without a checkout URL (account-service's balance-style path, which the terms rule out) is read as settled, not failed: the schema gains a settled variant and the page closes as done instead of offering a retry for a payment that exists. - A pay call that throws after Desktop created the Workspace answers 200 with payment failed, so the Workspace id never disappears behind an error the page cannot act on. - A 409 on a name the actor already owns — Desktop created it but the answer never landed — re-reads the session list and offers Retry payment instead of the taken-name verdict that stranded a Workspace the user could not pay for. - Any create response carrying a Workspace refreshes /api/workspace/list into the session atoms (the duplicate check and the Switcher read it), so a payment-failed "Later" leaves the unpaid Workspace visible and a second create under a new name no longer slips past the check. Also applies the repo formatter's pass over the touched files.
|
@cursoragent review again |
|
Posted the follow-up on PR #352: a summary plus six line comments, and replies on the eight previous threads. The three Still open: the creation dialog does not reset on close (Later / settled leave it stuck); the project guard still does not inspect the resolved list; |
Review again (after the three
|
| if (payment.status === "settled") { | ||
| // Paid without a checkout hop (the terms rule the path out; read | ||
| // it as done): close rather than offer a payment that exists. | ||
| handedOff = true; | ||
| onOpenChange(false); | ||
| return; | ||
| } | ||
| setStage({ | ||
| error: payment.error, | ||
| kind: "payment-failed", | ||
| plan, | ||
| workspace, | ||
| }); | ||
| } catch (cause) { | ||
| if (cause instanceof WorkspaceNameConflictError) { | ||
| // A 409 on a name this dialog just submitted can be Desktop having | ||
| // created the Workspace while its answer never landed (a timeout, | ||
| // a malformed envelope). Re-read the list: if the actor now owns | ||
| // the name, offer the payment retry instead of "taken" — the user | ||
| // already owns a Workspace they cannot pay for otherwise. | ||
| await refreshWorkspaces({ details: false }).catch(() => undefined); | ||
| const owned = store | ||
| .get(workspacesAtom) | ||
| .find( | ||
| (candidate) => | ||
| candidate.role === "Owner" && | ||
| candidate.name.trim().toLowerCase() === trimmedName.toLowerCase() | ||
| ); | ||
| if (owned != null) { | ||
| setStage({ | ||
| error: | ||
| "The Workspace was created, but its payment was never started.", | ||
| kind: "payment-failed", | ||
| plan, | ||
| workspace: { | ||
| id: owned.id, | ||
| name: owned.name, | ||
| uid: owned.uid, | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| setTakenNames((names) => [...names, trimmedName]); | ||
| setNameIssue("duplicate"); | ||
| setStage({ kind: "pick" }); | ||
| return; | ||
| } | ||
| setError(errorDescription(cause, "The Workspace could not be created.")); | ||
| } finally { | ||
| if (!handedOff) { | ||
| setSubmitting(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
BillingPlanWorkflow always mounts this dialog (open only hides it). settled sets handedOff and returns without setSubmitting(false); Later leaves stage.kind === "payment-failed". Reopening New Workspace on the same Billing Plan tree then either sticks Create & Pay on "Creating…" (Cancel is a no-op while submitting) or immediately restacks Workspace created / Retry payment.
handedOff is the right freeze for a real window.top hop. It is the wrong freeze for close. Reset submitting / stage / name when open becomes false. Tests never reopen after Later or settled.
There was a problem hiding this comment.
Closed in 19988f31. When open turns false the dialog resets stage, name, and submitting. Reopen tests cover Later (no stacked offer) and settled (not stuck on "Creating…"). handedOff still freezes settled until the parent flips open; the effect then clears it — the user-visible leak is gone.
| } catch (cause) { | ||
| if (cause instanceof WorkspaceNameConflictError) { | ||
| // A 409 on a name this dialog just submitted can be Desktop having | ||
| // created the Workspace while its answer never landed (a timeout, | ||
| // a malformed envelope). Re-read the list: if the actor now owns | ||
| // the name, offer the payment retry instead of "taken" — the user | ||
| // already owns a Workspace they cannot pay for otherwise. | ||
| await refreshWorkspaces({ details: false }).catch(() => undefined); | ||
| const owned = store | ||
| .get(workspacesAtom) | ||
| .find( | ||
| (candidate) => | ||
| candidate.role === "Owner" && | ||
| candidate.name.trim().toLowerCase() === trimmedName.toLowerCase() | ||
| ); | ||
| if (owned != null) { | ||
| setStage({ | ||
| error: | ||
| "The Workspace was created, but its payment was never started.", | ||
| kind: "payment-failed", | ||
| plan, | ||
| workspace: { | ||
| id: owned.id, | ||
| name: owned.name, | ||
| uid: owned.uid, | ||
| }, | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The required 409 recovery was "the caller already owns that name as PAYG." This matches any role === "Owner", including Personal and an already-subscribed Team of the same name.
A Personal collision would offer Retry payment (operator: "created") on the Personal Workspace. A 409 whose list lag misses the new row still pushes the name into takenNames, so this session cannot pay for a Workspace the user now owns.
Require !isPersonal (and plan null if you can see it). Do not add to takenNames until ownership is ruled out — retry the list once, same as a 504 after an unobserved Desktop create.
There was a problem hiding this comment.
Closed for Personal in 19988f31: match is !isPersonal && role === "Owner". Personal-name collision stays a field verdict; nothing is pushed into takenNames. Still no PAYG/subscription check on Retry — a subscribed Team of the same name would still get operator: "created". Follow-up, not blocking.
| function useDesktopCloudDomain(): string { | ||
| const desktopDomain = useAtomValue(desktopDomainAtom).trim(); | ||
| const kubeconfig = useAtomValue(kubeconfigAtom); | ||
| return useMemo( | ||
| () => | ||
| desktopDomain === "" | ||
| ? routingDomainFromKubeconfig(kubeconfig) | ||
| : desktopDomain, | ||
| [desktopDomain, kubeconfig] | ||
| ); |
There was a problem hiding this comment.
The invite dialog now refuses before POST when workspaceInviteUrl would be null — but this hook almost never hands it an empty domain. Empty SDK cloud.domain becomes routingDomainFromKubeconfig: the apiserver hostname (apiserver.test in fixtures; often kubernetes.default.svc in cluster), truncated to a 63-char label.
The dialog then mints (and invalidates the previous {inviter, workspace, role} code) and copies https://<apiserver>/WorkspaceInvite/?code=…. The Switcher already waits for a real Desktop origin (desktop-pending). Invite should too: no SDK/dev Desktop domain → don't mint. Do not treat the kubeconfig server URL as Desktop.
There was a problem hiding this comment.
Closed in d5f2dbc4. useDesktopCloudDomain is desktopDomainAtom only. The kubeconfig apiserver host no longer yields a link; the area test asserts no mint.
| refreshProjects() | ||
| .then((fresh) => { | ||
| if (!cancelled && fresh !== undefined) { | ||
| setRevalidatedFor(projectId); | ||
| } | ||
| }) | ||
| .catch(() => undefined); |
There was a problem hiding this comment.
The failure path is closed (reject → no revalidatedFor, no toast). The required rule is not: any defined mutate() result confirms, and the bounce still keys off hook decision (states.projects).
Production refreshProjects is SWR mutate and resolves { projects: BrainProject[] }. A successful revalidation whose payload contains the id can still replace("/project") if that snapshot has not painted yet — the case the extra round-trip was added to prevent.
Parse fresh.projects[].id; set the flag only if that list is present and still lacks projectId. Unparseable / undefined = failure: do not bounce, do not toast. The tests still resolve explorer rows and update hook state before resolve, so they lock the happy path.
There was a problem hiding this comment.
Closed in d5f2dbc4. The guard parses the raw /api/projects payload and confirms only when that list still lacks the id. A payload that carries the id holds even before the rendered snapshot paints; reject confirms nothing. Tests now resolve { projects: [...] } and cover the before-paint case.
| function originAllowed(request: Request): boolean { | ||
| const origin = request.headers.get("origin")?.trim() ?? ""; | ||
| if (origin === "" || origin === "null") { | ||
| return true; | ||
| } | ||
| const host = request.headers.get("host")?.trim() ?? ""; | ||
| try { | ||
| const parsed = new URL(origin); | ||
| if (host !== "" && parsed.host === host) { | ||
| return true; | ||
| } | ||
| return parsed.origin === new URL(request.url).origin; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
The sibling text/plain+nsid switch is closed when the browser sends a foreign Origin. This still allowlists two values that are not Brain:
Origin: null— a browser (sandboxed iframe, some 307s), not the smoke script. Reject it.parsed.host === host— scheme is ignored, sohttp://brain.examplematches an HTTPS app.
Empty Origin for a non-browser client is a documented tradeoff; null is not. Happy-path tests omit Origin and lock the empty-Origin allow; there is no Origin: null case.
There was a problem hiding this comment.
Closed in 24ba9ec6. Origin: null fails the URL parse and is refused (tested). Host match requires HTTPS; HTTP is allowed only outside production. Empty Origin remains the documented smoke-script hole.
| if (refresh?.key !== key) { | ||
| // Read once per arrival, alongside the refresh: a creation's record is | ||
| // spent on the first read. Its recorded return route belongs to the | ||
| // Workspace the creation left (spec §G.5), so close returns home; a | ||
| // plan change came back to the same Workspace and keeps its own. | ||
| const created = consumePendingWorkspaceCreation(stripeReturn.workspaceId); | ||
| if (created) { | ||
| clearBillingReturnRoute(); | ||
| } |
There was a problem hiding this comment.
consumePendingWorkspaceCreation is keyed only on workspaceId and runs only when stripeState === "success" (apps/ui/src/app/billing/page.tsx). Cancel / abandon leaves sessionStorage["billing-workspace-creation"].
CONTEXT's model is: the Workspace exists before payment, so an abandoned Checkout is PAYG and "subscribes like any other." The next Plan-change return for that same Workspace then reads as a creation ("Workspace created" + forgotten Billing entry point).
Record { workspaceId, payId } (or consume on any Stripe return for that Workspace, including cancel). readBillingReturnRoute currently treats any stripeState + matching id as a creation landing and mutates storage inside getSnapshot — do not clear on cancel, and do not mutate from a snapshot read.
There was a problem hiding this comment.
Mostly closed in 19988f31: the record carries payId; any success or cancel for that Workspace spends it; cancel keeps the entry point; "created" requires a pay-id match when one was recorded; readBillingReturnRoute is a pure read.
Residual: pending.payId == null is still a wildcard match (this file and readBillingReturnRoute). Fail closed — "created" only when both pay ids are non-empty and equal. See the new comment.
…n route `Origin: null` is a browser (a sandboxed frame, some redirects), not a missing header — it now fails the origin parse and is refused, where an absent Origin (the smoke script, non-browser clients) still passes the content-type gate. The Host match no longer ignores scheme: an Origin must name this app over HTTPS, or over HTTP only outside production (local development), so http://<host> cannot stand in for the HTTPS app.
… links on Desktop only
- The /project/<uid> guard now inspects the revalidation's payload: only
a raw /api/projects answer that came back as a list still lacking the
Project confirms the leave verdict. A payload that carries the Project
holds the guard even before the rendered snapshot paints — the case
the extra round-trip was added to prevent — and an unusable shape
confirms nothing.
- The Workspace Area builds its invite links on the SDK host config's
cloud domain only; the kubeconfig's apiserver host is not Desktop, and
a link built on it both points nowhere and invalidates the previous
{inviter, workspace, role} code. An empty SDK domain now really means
"cannot build links yet" — the dialog refuses to mint — the same gate
the Switcher applies.
…ecord on any Stripe return; scope 409 recovery to Team Workspaces - The creation dialog resets its stage, name, and submitting state the moment `open` turns false: the workflow keeps it mounted, so a reopen after Later must not restack the failed-payment offer, and a reopen after a settled creation must not inherit the submitting freeze that exists for a real top-window Stripe hop. - The pending-creation record now carries the checkout's payId, and any Stripe return for the recorded Workspace — success or cancel — spends it: an abandoned Checkout can no longer reword a later plan change for that Workspace as a creation. A success return concludes "created" only when the recorded payId matches; a cancel keeps the recorded entry point (the user continues where they were) and draws no conclusion. readBillingReturnRoute is now a pure read — the workflow's return effect does the consuming and the entry-point voiding, never a snapshot-style read. - The 409 recovery matches only a Team Workspace the actor owns; a name shared with the Personal Workspace reads as taken on the field, never as a Retry-payment offer on the Personal one. The name stays retriable: nothing is pushed into a session-local taken list, so a list that has not caught up shows "taken" and the next attempt re-runs the ownership recovery.
|
The six open findings from the follow-up review are addressed in three commits (
All focused suites pass per file (14 files, 173 tests), plus @cursoragent review again |
|
@cursoragent review again |
|
Posted the follow-up on PR #352. The six findings from last time are closed in One residual, not blocking: I would not merge until the Stripe create → Checkout → “Workspace created” walk lands; that box is still unchecked. |
Review again (HEAD
|
| const pending = readPendingWorkspaceCreation(); | ||
| const recordedHere = | ||
| pending != null && pending.workspaceId === stripeReturn.workspaceId; | ||
| const created = | ||
| recordedHere && | ||
| (pending?.payId == null || pending.payId === stripeReturn.payId); |
There was a problem hiding this comment.
pending.payId == null is still treated as a match. If the recorded checkout had no payID (account-service omitted it, "" stored then read as null, or a legacy string record), a later Plan-change success for that Workspace still concludes "Workspace created" and clearBillingReturnRoute().
That is the abandoned-creation miswording this change was meant to kill, with a hole left for "Desktop didn't carry a pay id." Fail closed: "created" only when pending.payId is a non-empty string and equals the return payId. Spend the record either way.
The creation-landing test records without a pay id (recordPendingWorkspaceCreation("ns-new00001")), so it currently locks the wildcard. Record "payment-1" there.
The creation conclusion required the recorded pay id to match only when one was recorded; a record without one (Desktop's checkout answer omitted it, or a legacy plain-string record) matched any success return for that Workspace — the abandoned-creation miswording this record exists to prevent, left open for "no pay id". "Created" now requires both pay ids to be non-empty and equal, in the workflow's return effect and in readBillingReturnRoute; the record is spent either way. The creation-landing test now records with its pay id instead of locking the wildcard, and both levels gained a no-pay-id fail-closed case.
3 participants
Brain owns its session and, on top of it, lets users see, switch, manage, and create Workspaces without leaving for Desktop's Team Center. Five steps, one branch, each step a
featcommit followed by afixcommit that addresses its code review.What changes
Brain Session (AIM-444).
POST /api/sessionexchanges the Desktop login cookie server-side (regionToken→namespace/list→namespace/switch∥auth/info) and answers Brain's own zod-validated session; Desktop's "HTTP 200 +body.code" envelope becomes real statuses. Credentials live only in Jotai atoms; Workspace-management fetchers attachX-Sealos-Region-Tokenand run the 401 two-step. The self-signed dev credentials are gone; local dev runs the real path against a staging Desktop (DESKTOP_API_BASE_URL,DEV_GLOBAL_TOKEN). ADR-0083 (Proposed) records the model.App Sidebar brand row and Workspace Switcher (AIM-445). The logo slot is the only collapse/expand control. The Switcher row shows the current Workspace (square avatar, name, plan badge or PAYG) and opens a popover with the current card, "Switch to", New Workspace, and Manage Workspaces. Switching hands the top window to Desktop's
?openapp=system-brain…&workspaceUid=deep link. Billing'scanManageand the notification read dispatch read the session's Workspace Role;/project/<uid>gains a guard.Workspace Area, read-only (AIM-446).
/workspace/<uid>: the Workspace list beside the managed Workspace's detail (header, members). Gating is a pure module mirroring Desktop'svaildManage. The Billing Area's title bar becomes the sharedAreaShell.Workspace Area writes (AIM-447). Rename, delete, invite link, member remove / role / alias, transfer: seven routes in the route table, each validating with a shared schema (roles never Owner), calling Desktop through the auth API module, translating envelope codes into Brain's own. Three confirmation tiers in the UI; after a write the list and members are re-read.
Workspace Creation (AIM-448).
/billing?mode=createopens the Billing Area's creation mode: name field plus Plan Picker on one screen, paid plans only, never gated by the current Workspace's role or lifecycle.POST /api/billing/workspace-createasks Desktopnamespace/createwith the raw app token, then starts the first payment through account-service as Brain (operator: created,payApp: system-brain); a taken name is a 409 on the field, a failed second step is an outcome with "Retry payment" / "Later" (…/retry-paymentredoes Step 2 alone). The Stripe hop is a whole-page top-window redirect; on return the congratulations read "Workspace created" and the Billing Area's return route is forgotten, since it named the Workspace the user left.Every new route joins its route table with an on-disk guard test and has dev-mock answers.
Deployment notes
DESKTOP_API_BASE_URL; the chart derives it to the in-cluster Desktop Service when left empty.allowedOriginsmust include Brain's domain (chart README).NEXT_PUBLIC_DEV_ENCODED_KUBECONFIG,NEXT_PUBLIC_DEV_APP_TOKEN, the mint script.Verification
bun typecheck,bun checkpass.apps/uisuite in onebun testprocess trips Bun's "test() inside another test()" limitation and anext/navigationmock leak between files; a baseline run atmainshows the same failing files and error kinds.🤖 Generated with Claude Code