Skip to content

Commit 95c280e

Browse files
committed
fix(daemon): resolve an unbound project against the caller's current workspace
Every project must resolve to a workspace; when the project itself has no binding, the workspace is the one the caller is currently acting in. There is no other candidate. `GET /api/projects/:id/workspace-scope` read only the persisted `workspace_projects` row and the membership directory — never the request's `x-od-workspace-*` identity — so a project with no row answered `unbound` for every caller forever. `unbound` makes `projectWorkspaceScopeAuthorizesAmr` false, which disabled the chat composer's send button for an Open Design Cloud run on that project permanently, with nothing the user could do to clear it. #6178 wrote user-facing copy for that state instead of fixing it (reverted in #6184). `resolveProjectWorkspaceScopeForCaller` wraps the existing resolver instead of branching inside it, so `resolveProjectWorkspaceScope` stays byte-identical and the bound path is provably untouched. The fallback re-enters that resolver with a synthetic binding naming the caller's own workspace, so the resulting `workspaceMemberId` comes from the membership directory and never from the request header: a caller can only select among workspaces their signed-in identity is genuinely an active member of, and anything the directory cannot confirm degrades back to `unbound` rather than to `unavailable` on a workspace the project was never bound to. Two cases keep today's behavior, both pinned by spec: unavailable — all three of its return sites. A project pinned to workspace X read by a member of Y answers X. The scope carries `workspaceMemberId`, the wallet that pays for the project's runs; resolving to Y would bill Y for X's project. Reachable because `GET /api/projects/:id` has no workspace gate, so a deep link or a workspace switch lands there. no caller identity — signed out, or a plain curl, has no current workspace to fall back to, and inventing one is worse than answering "none". The fallback deliberately does not persist a binding. `reconcileUnboundProjectBeforeMutation` was the preferred shape going in and is wrong here: its own docblock is explicit that a passive read must not hand out ownership just because it ran first, and this endpoint is a GET. Writing a row from it would let whichever workspace opened the project first claim authorship on no user intent, and would race two clients in different workspaces.
1 parent 613cd51 commit 95c280e

4 files changed

Lines changed: 483 additions & 3 deletions

File tree

apps/daemon/src/collab/project-workspace-scope.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,52 @@ export function resolveProjectWorkspaceScope(input: {
8383
context: { ...context, workspaceType: 'personal' },
8484
};
8585
}
86+
87+
/**
88+
* Resolve one project's workspace scope for a SPECIFIC caller.
89+
*
90+
* Every project resolves to a workspace, and when the project itself does not
91+
* name one the answer is the workspace the caller is currently acting in —
92+
* there is nothing else an unbound project could sensibly belong to. Answering
93+
* `unbound` instead makes `projectWorkspaceScopeAuthorizesAmr` false, which
94+
* disables the chat composer's send button for an AMR run on that project
95+
* permanently, with nothing the user can do to clear it.
96+
*
97+
* A project that IS bound is resolved by {@link resolveProjectWorkspaceScope}
98+
* alone and the caller's own workspace is irrelevant to it. That asymmetry is
99+
* the billing-integrity rule, not an oversight: the scope carries
100+
* `workspaceMemberId`, which is the wallet that pays for the project's runs, so
101+
* a project pinned to workspace X read by a member of Y must answer X — or
102+
* `unavailable` — and never Y.
103+
*
104+
* The fallback resolves THROUGH the membership directory, so the
105+
* `workspaceMemberId` it hands back is always B's for that workspace and never
106+
* the request header's. A caller can therefore only select among workspaces
107+
* their signed-in identity is genuinely an active member of. Anything the
108+
* directory cannot confirm — B unreachable, the claimed workspace not an active
109+
* membership, or no caller identity at all — stays `unbound`: reporting "no
110+
* workspace" is strictly better than inventing a billing subject.
111+
*/
112+
export function resolveProjectWorkspaceScopeForCaller(input: {
113+
projectId: string;
114+
binding: ProjectWorkspaceBinding | null | undefined;
115+
directory: WorkspaceDirectoryFetchResult;
116+
/** The caller's current workspace, or null when the request carries no workspace identity. */
117+
callerWorkspaceId: string | null | undefined;
118+
}): ProjectWorkspaceScope {
119+
const bound = resolveProjectWorkspaceScope(input);
120+
if (bound.kind !== 'unbound') return bound;
121+
const callerWorkspaceId =
122+
typeof input.callerWorkspaceId === 'string' ? input.callerWorkspaceId.trim() : '';
123+
if (!callerWorkspaceId) return bound;
124+
const fallback = resolveProjectWorkspaceScope({
125+
projectId: input.projectId,
126+
// An unbound project is a private local draft. It is not shared with the
127+
// team even when the workspace hosting it is one — the same `personal`
128+
// visibility every other "claim this orphan into the current workspace"
129+
// path writes (`ensureWorkspaceProjection`, `bindDuplicateIntoRequestWorkspace`).
130+
binding: { workspaceId: callerWorkspaceId, visibility: 'personal' },
131+
directory: input.directory,
132+
});
133+
return fallback.kind === 'personal' || fallback.kind === 'team' ? fallback : bound;
134+
}

apps/daemon/src/routes/project/index.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ import {
8787
type WorkspaceResourceMutationCapability,
8888
} from '../../collab/workspace-resource-mutation.js';
8989
import type { WorkspaceContextProvider } from '../../collab/workspace-context.js';
90-
import { resolveProjectWorkspaceScope } from '../../collab/project-workspace-scope.js';
90+
import { resolveProjectWorkspaceScopeForCaller } from '../../collab/project-workspace-scope.js';
9191
import {
9292
authorizeCreatedProjectWorkspace,
9393
bindCreatedProjectToWorkspace,
@@ -3431,10 +3431,17 @@ export function registerProjectRoutes(app: Express, ctx: RegisterProjectRoutesDe
34313431
(): WorkspaceDirectoryFetchResult => ({ ok: false, items: [] }),
34323432
)
34333433
: { ok: false, items: [] };
3434-
const scope = resolveProjectWorkspaceScope({
3434+
// A project with no binding of its own resolves to the workspace THIS
3435+
// caller is acting in, read the same sanctioned way every neighbouring
3436+
// handler reads it. A bound project ignores the caller entirely — see
3437+
// `resolveProjectWorkspaceScopeForCaller`.
3438+
const callerCtx = workspaceProjectContextFromRequest(req);
3439+
const scope = resolveProjectWorkspaceScopeForCaller({
34353440
projectId: project.id,
34363441
binding,
34373442
directory,
3443+
callerWorkspaceId:
3444+
callerCtx === null || callerCtx === 'missing' ? null : callerCtx.workspaceId,
34383445
});
34393446
/** @type {import('@open-design/contracts').ProjectWorkspaceScopeResponse} */
34403447
const body = { scope };

apps/daemon/tests/collab/project-workspace-scope.test.ts

Lines changed: 121 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { describe, expect, it } from 'vitest';
22

3-
import { resolveProjectWorkspaceScope } from '../../src/collab/project-workspace-scope.js';
3+
import {
4+
resolveProjectWorkspaceScope,
5+
resolveProjectWorkspaceScopeForCaller,
6+
} from '../../src/collab/project-workspace-scope.js';
47

58
const directoryItems = [
69
{
@@ -137,3 +140,120 @@ describe('resolveProjectWorkspaceScope', () => {
137140
});
138141
});
139142
});
143+
144+
describe('resolveProjectWorkspaceScopeForCaller', () => {
145+
it('resolves an unbound project against the caller current workspace', () => {
146+
const scope = resolveProjectWorkspaceScopeForCaller({
147+
projectId: 'project-unbound',
148+
binding: null,
149+
directory: { ok: true, items: directoryItems },
150+
callerWorkspaceId: 'workspace-b',
151+
});
152+
153+
expect(scope).toMatchObject({
154+
kind: 'team',
155+
projectId: 'project-unbound',
156+
workspaceId: 'workspace-b',
157+
// An unbound project is a private local draft even inside a team.
158+
visibility: 'personal',
159+
context: {
160+
workspaceId: 'workspace-b',
161+
workspaceType: 'team',
162+
workspaceMemberId: 'member-b',
163+
},
164+
});
165+
});
166+
167+
it('never lets the caller workspace override a project already pinned elsewhere', () => {
168+
const scope = resolveProjectWorkspaceScopeForCaller({
169+
projectId: 'project-pinned',
170+
binding: { workspaceId: 'workspace-gone', visibility: 'team' },
171+
directory: { ok: true, items: directoryItems },
172+
callerWorkspaceId: 'workspace-b',
173+
});
174+
175+
// `workspaceMemberId` is the billing subject, so borrowing the caller's
176+
// would bill their wallet for another workspace's project.
177+
expect(scope).toEqual({
178+
kind: 'unavailable',
179+
projectId: 'project-pinned',
180+
workspaceId: 'workspace-gone',
181+
visibility: 'team',
182+
context: null,
183+
});
184+
});
185+
186+
it('leaves a pinned project unavailable when the directory could not be read either', () => {
187+
const scope = resolveProjectWorkspaceScopeForCaller({
188+
projectId: 'project-pinned',
189+
binding: { workspaceId: 'workspace-b', visibility: 'personal' },
190+
directory: { ok: false, items: [] },
191+
callerWorkspaceId: 'workspace-b',
192+
});
193+
194+
// The caller names the very workspace the project is pinned to, but nothing
195+
// confirmed it. `unavailable` keeps its exact pre-existing behavior; the
196+
// caller's claim is not a substitute for the membership directory.
197+
expect(scope).toEqual({
198+
kind: 'unavailable',
199+
projectId: 'project-pinned',
200+
workspaceId: 'workspace-b',
201+
visibility: 'personal',
202+
context: null,
203+
});
204+
});
205+
206+
it('keeps an unbound project unbound when the caller has no workspace identity', () => {
207+
const scope = resolveProjectWorkspaceScopeForCaller({
208+
projectId: 'project-anonymous',
209+
binding: null,
210+
directory: { ok: true, items: directoryItems },
211+
callerWorkspaceId: null,
212+
});
213+
214+
expect(scope).toEqual({
215+
kind: 'unbound',
216+
projectId: 'project-anonymous',
217+
workspaceId: null,
218+
context: null,
219+
});
220+
});
221+
222+
it('keeps an unbound project unbound when the claimed workspace is not an active membership', () => {
223+
const unconfirmed = resolveProjectWorkspaceScopeForCaller({
224+
projectId: 'project-unconfirmed',
225+
binding: null,
226+
directory: { ok: true, items: directoryItems },
227+
callerWorkspaceId: 'workspace-not-mine',
228+
});
229+
const outage = resolveProjectWorkspaceScopeForCaller({
230+
projectId: 'project-outage',
231+
binding: null,
232+
directory: { ok: false, items: [] },
233+
callerWorkspaceId: 'workspace-b',
234+
});
235+
236+
// The fallback resolves THROUGH the directory, so the member id it returns
237+
// is always B's and never the request header's. An unconfirmable claim must
238+
// degrade to "no workspace", never to `unavailable` on a workspace the
239+
// project was never bound to.
240+
expect(unconfirmed.kind).toBe('unbound');
241+
expect(unconfirmed.workspaceId).toBeNull();
242+
expect(outage.kind).toBe('unbound');
243+
expect(outage.workspaceId).toBeNull();
244+
});
245+
246+
it('keeps an unbound project unbound when the caller membership is revoked', () => {
247+
const scope = resolveProjectWorkspaceScopeForCaller({
248+
projectId: 'project-revoked',
249+
binding: null,
250+
directory: {
251+
ok: true,
252+
items: [{ ...directoryItems[0]!, memberStatus: 'removed' as const }],
253+
},
254+
callerWorkspaceId: 'workspace-b',
255+
});
256+
257+
expect(scope.kind).toBe('unbound');
258+
});
259+
});

0 commit comments

Comments
 (0)