Skip to content

Commit 4f96ff8

Browse files
committed
fix(service-storage): the commit and chunked-completion doors answer an organization change with 409, not an outage 500
An uploader whose active organization changed after starting an upload names a row the doors' scoped by-id write cannot reach. The engine's miss came back as 500 INTERNAL carrying the store's "Restore the data engine" text. Both doors now ask the store (organizationOutOfWriteReach) before any write and answer 409 RESOURCE_CONFLICT naming the change, with one operator warning naming the cause. The engine-absent stand-in does not scope and keeps its answers. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1e5d322 commit 4f96ff8

7 files changed

Lines changed: 732 additions & 3 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/service-storage': patch
3+
---
4+
5+
fix(service-storage): finishing an upload after switching active organization answers `409 RESOURCE_CONFLICT` naming the change, not a `500` that reads as a data-engine outage
6+
7+
Clause-②: no
8+
9+
`POST /api/v1/storage/upload/complete` and `POST /api/v1/storage/upload/chunked/:uploadId/complete` write by id under the caller's active organization, and that write is scoped to it. When the uploader's active organization is no longer the one the upload was started in, the write cannot reach the upload's row. Both doors used to answer that with `500 INTERNAL` and a message diagnosing a data-engine outage ("Restore the data engine …").
10+
11+
- **Now:** before writing anything, each door answers `409 RESOURCE_CONFLICT` in the standard error envelope. The message says the upload was started in a different organization than the active one, and that switching back finishes it. The server log carries one warning naming the upload, the organization it was started in and the active one.
12+
- **Unchanged:** which organization an upload belongs to (switching back finishes it there); a commit or completion from the starting organization; a real data-engine failure, which still answers `500` with its consequence and fix; the uploader-only rule, which runs first, so a caller who did not start the upload is still refused `403 PERMISSION_DENIED` and learns nothing about organizations; uploads stamped with no organization; routes with no session resolver; a store with no data engine wired; and the progress door.
13+
14+
What changes for you: when either door answers `409 RESOURCE_CONFLICT`, switch the active organization back to the one the upload was started in and retry the same call.
Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,254 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #22175 — an uploader whose ACTIVE organization changed after starting an
5+
* upload, measured on a BOOTED organization wall.
6+
*
7+
* The commit door (`POST …/upload/complete`) and the chunked-completion door
8+
* (`POST …/upload/chunked/:uploadId/complete`) write by id, and those writes
9+
* are scoped to the caller's active organization. A row stamped for the
10+
* organization the upload started in is out of that scope's reach once the
11+
* uploader switches, and the doors used to answer the scoped miss as
12+
* `500 INTERNAL` with a message diagnosing a data-engine outage. They now ask
13+
* the store whether that write can reach the row (`organizationOutOfWriteReach`)
14+
* before any write and answer `409 RESOURCE_CONFLICT`, in the ADR-0112
15+
* envelope, with a message that names the organization change. Which
16+
* organization an upload belongs to is NOT changed: switching back finishes it,
17+
* from where it started.
18+
*
19+
* Same boot as `storage-upload-door-ownership.wall.test.ts` (and the same
20+
* reason this file lives in this package: the wall is raised by THIS
21+
* package's `OrganizationsPlugin`, which ADR-0132 forbids every other
22+
* workspace package from mounting). One caller: the seeded platform owner,
23+
* who holds two organizations and switches between them through the
24+
* product's own `set-active` door. Nothing is stubbed.
25+
*/
26+
27+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
28+
import { mkdtempSync, rmSync } from 'node:fs';
29+
import { tmpdir } from 'node:os';
30+
import { join } from 'node:path';
31+
import { bootStack, type VerifyStack } from '@objectstack/verify';
32+
import { StorageServicePlugin } from '@objectstack/service-storage';
33+
import { OrganizationsPlugin } from './organizations-plugin.js';
34+
35+
const APP = {
36+
manifest: {
37+
id: 'com.example.uploadorgchange',
38+
namespace: 'uploadorgchange',
39+
version: '0.0.1',
40+
type: 'app',
41+
name: 'Upload Organization Change Fixture',
42+
},
43+
objects: [],
44+
};
45+
46+
const SYS = { context: { isSystem: true } } as const;
47+
48+
interface Answer {
49+
status: number;
50+
body: any;
51+
}
52+
53+
describe('the upload doors after the uploader switches organization, on a booted wall (#22175)', () => {
54+
const restoreEnv: Array<[string, string | undefined]> = [];
55+
let rootDir: string;
56+
let stack: VerifyStack;
57+
let ql: any;
58+
/** The seeded platform owner — the uploader throughout. */
59+
let uploader: string;
60+
/** The organization every upload below is started in. */
61+
let startOrg: string;
62+
/** The uploader's second organization — where they switch to. */
63+
let otherOrg: string;
64+
65+
const answer = async (res: Response): Promise<Answer> => ({
66+
status: res.status,
67+
body: await res.json().catch(() => undefined),
68+
});
69+
const activeOrganization = async (): Promise<string | null> => {
70+
const res = await stack.apiAs(uploader, 'GET', '/auth/get-session');
71+
return ((await res.json()) as any)?.session?.activeOrganizationId ?? null;
72+
};
73+
const switchTo = async (organizationId: string): Promise<void> => {
74+
const res = await stack.apiAs(uploader, 'POST', '/auth/organization/set-active', { organizationId });
75+
expect(res.status).toBe(200);
76+
expect(await activeOrganization()).toBe(organizationId);
77+
};
78+
const stored = (object: string, id: string) => ql.findOne(object, { where: { id } }, SYS);
79+
80+
const presign = async (): Promise<string> => {
81+
const res = await answer(
82+
await stack.apiAs(uploader, 'POST', '/storage/upload/presigned', {
83+
filename: 'org-change-plan.txt',
84+
mimeType: 'text/plain',
85+
size: 5,
86+
}),
87+
);
88+
expect(res.status).toBe(200);
89+
return res.body.data.fileId;
90+
};
91+
const startChunked = async (): Promise<{ uploadId: string; fileId: string }> => {
92+
const res = await answer(
93+
await stack.apiAs(uploader, 'POST', '/storage/upload/chunked', {
94+
filename: 'org-change-chunked.bin',
95+
mimeType: 'application/octet-stream',
96+
totalSize: 10,
97+
}),
98+
);
99+
expect(res.status).toBe(200);
100+
return { uploadId: res.body.data.uploadId, fileId: res.body.data.fileId };
101+
};
102+
103+
/**
104+
* The organization-change answer: a 4xx in the declared envelope, its code,
105+
* and a message that names the change — never the `500` an engine outage
106+
* answers, nor that outage's prescription.
107+
*/
108+
const expectOrganizationChanged = (res: Answer, label: string) => {
109+
const seen = `${label}: ${JSON.stringify(res.body)}`;
110+
expect(res.status, seen).toBe(409);
111+
expect(res.body?.success, seen).toBe(false);
112+
expect(res.body?.error?.code, seen).toBe('RESOURCE_CONFLICT');
113+
expect(res.body?.data, seen).toBeUndefined();
114+
expect(res.body?.error?.message, seen).toMatch(/started in a different organization/);
115+
expect(res.body?.error?.message, seen).not.toMatch(/data engine/);
116+
};
117+
118+
beforeAll(async () => {
119+
for (const [name, value] of [
120+
['OS_TENANCY_POSTURE', 'isolated'],
121+
// A walled deployment must declare what a new user joins, or this
122+
// package's own boot gate refuses it.
123+
['OS_AUTH_MEMBERSHIP_POLICY', 'invite-only'],
124+
] as const) {
125+
restoreEnv.push([name, process.env[name]]);
126+
process.env[name] = value;
127+
}
128+
rootDir = mkdtempSync(join(tmpdir(), 'upload-org-change-'));
129+
stack = await bootStack(APP as never, {
130+
extraPlugins: [
131+
new OrganizationsPlugin(),
132+
new StorageServicePlugin({ adapter: 'local', local: { rootDir }, bindToSettings: false }),
133+
],
134+
});
135+
ql = await stack.kernel.getServiceAsync('objectql');
136+
137+
uploader = await stack.signIn();
138+
startOrg = (await activeOrganization())!;
139+
const created = await answer(
140+
await stack.apiAs(uploader, 'POST', '/auth/organization/create', {
141+
name: 'Upload Org Change Second',
142+
slug: 'upload-org-change-second',
143+
}),
144+
);
145+
expect(created.status).toBe(200);
146+
otherOrg = created.body.id;
147+
await switchTo(startOrg);
148+
}, 120_000);
149+
150+
afterAll(async () => {
151+
await stack?.stop();
152+
if (rootDir) rmSync(rootDir, { recursive: true, force: true });
153+
for (const [name, value] of restoreEnv.reverse()) {
154+
if (value === undefined) delete process.env[name];
155+
else process.env[name] = value;
156+
}
157+
});
158+
159+
it('PREMISE: the wall is up, the uploader holds two organizations, and uploads are stamped where they start', async () => {
160+
const tenancy: any = await stack.kernel.getServiceAsync('tenancy');
161+
expect(tenancy.posture).toBe('isolated');
162+
expect(tenancy.isolationActive).toBe(true);
163+
expect(tenancy.degraded).toBe(false);
164+
165+
expect(startOrg).toBeTruthy();
166+
expect(otherOrg).toBeTruthy();
167+
expect(otherOrg).not.toBe(startOrg);
168+
169+
await switchTo(startOrg);
170+
const fileId = await presign();
171+
expect((await stored('sys_file', fileId)).organization_id).toBe(startOrg);
172+
const { uploadId } = await startChunked();
173+
expect((await stored('sys_upload_session', uploadId)).organization_id).toBe(startOrg);
174+
});
175+
176+
describe('the commit door', () => {
177+
const commit = (fileId: string, eTag: string) =>
178+
stack.apiAs(uploader, 'POST', '/storage/upload/complete', { fileId, eTag });
179+
180+
it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; the row is unchanged; switching back commits it', async () => {
181+
await switchTo(startOrg);
182+
const fileId = await presign();
183+
const before = await stored('sys_file', fileId);
184+
185+
await switchTo(otherOrg);
186+
expectOrganizationChanged(await answer(await commit(fileId, 'etag-elsewhere')), 'commit after switching');
187+
expect(await stored('sys_file', fileId)).toEqual(before);
188+
189+
// The upload still belongs where it started, and finishes there.
190+
await switchTo(startOrg);
191+
const home = await answer(await commit(fileId, 'etag-home'));
192+
expect(home.status, JSON.stringify(home.body)).toBe(200);
193+
const row = await stored('sys_file', fileId);
194+
expect(row.status).toBe('committed');
195+
expect(row.etag).toBe('etag-home');
196+
expect(row.organization_id).toBe(startOrg);
197+
});
198+
199+
it('CONTROL — SAME ORGANIZATION: the uploader commits exactly as before', async () => {
200+
await switchTo(startOrg);
201+
const fileId = await presign();
202+
const own = await answer(await commit(fileId, 'etag-same'));
203+
expect(own.status, JSON.stringify(own.body)).toBe(200);
204+
expect(own.body.data.fileId).toBe(fileId);
205+
expect((await stored('sys_file', fileId)).status).toBe('committed');
206+
});
207+
});
208+
209+
describe('the chunked-completion door', () => {
210+
const complete = (uploadId: string) =>
211+
stack.apiAs(uploader, 'POST', `/storage/upload/chunked/${uploadId}/complete`, { parts: [] });
212+
213+
it('ORGANIZATION CHANGED: 409 RESOURCE_CONFLICT naming the change, not 500; neither row moves; switching back completes it', async () => {
214+
await switchTo(startOrg);
215+
const { uploadId, fileId } = await startChunked();
216+
const sessionBefore = await stored('sys_upload_session', uploadId);
217+
const fileBefore = await stored('sys_file', fileId);
218+
219+
await switchTo(otherOrg);
220+
expectOrganizationChanged(await answer(await complete(uploadId)), 'chunked completion after switching');
221+
expect(await stored('sys_upload_session', uploadId)).toEqual(sessionBefore);
222+
expect(await stored('sys_file', fileId)).toEqual(fileBefore);
223+
224+
await switchTo(startOrg);
225+
const home = await answer(await complete(uploadId));
226+
expect(home.status, JSON.stringify(home.body)).toBe(200);
227+
expect((await stored('sys_upload_session', uploadId)).status).toBe('completed');
228+
const file = await stored('sys_file', fileId);
229+
expect(file.status).toBe('committed');
230+
expect(file.organization_id).toBe(startOrg);
231+
});
232+
233+
it('CONTROL — SAME ORGANIZATION: the uploader completes exactly as before', async () => {
234+
await switchTo(startOrg);
235+
const { uploadId, fileId } = await startChunked();
236+
const own = await answer(await complete(uploadId));
237+
expect(own.status, JSON.stringify(own.body)).toBe(200);
238+
expect(own.body.data.fileId).toBe(fileId);
239+
expect((await stored('sys_upload_session', uploadId)).status).toBe('completed');
240+
expect((await stored('sys_file', fileId)).status).toBe('committed');
241+
});
242+
});
243+
244+
it('UNCHANGED — the progress door still answers the uploader after the switch (a read; no write is due)', async () => {
245+
await switchTo(startOrg);
246+
const { uploadId, fileId } = await startChunked();
247+
await switchTo(otherOrg);
248+
const progress = await answer(await stack.apiAs(uploader, 'GET', `/storage/upload/chunked/${uploadId}/progress`));
249+
expect(progress.status, JSON.stringify(progress.body)).toBe(200);
250+
expect(progress.body.data.fileId).toBe(fileId);
251+
expect(progress.body.data.status).toBe('in_progress');
252+
await switchTo(startOrg);
253+
});
254+
});

‎packages/services/service-storage/src/error-envelope.conformance.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,31 @@ describe('storage error envelope (#3675)', () => {
222222
return drive(routes, 'POST', `${BASE}/upload/complete`, { body: { fileId: 'f-owned' } });
223223
},
224224
},
225+
{
226+
// #22175: the uploader, acting in another organization than the one the
227+
// upload was started in.
228+
name: 'committing an upload after switching active organization',
229+
status: 409,
230+
code: 'RESOURCE_CONFLICT',
231+
run: async () => {
232+
// An engine-backed store (the stand-in does not scope its writes, so
233+
// it never answers this); the door refuses before any write, so the
234+
// engine only ever answers the by-id read.
235+
const row = {
236+
id: 'f-started',
237+
key: 'user/f-started.bin',
238+
name: 'f.bin',
239+
status: 'pending',
240+
owner_id: 'u-uploader',
241+
organization_id: 'org-start',
242+
};
243+
const store = new StorageMetadataStore({ findOne: async () => ({ ...row }) } as any);
244+
const routes = mount(await tmpAdapter(), store, {
245+
resolveSession: async () => ({ userId: 'u-uploader', organizationId: 'org-elsewhere' }),
246+
});
247+
return drive(routes, 'POST', `${BASE}/upload/complete`, { body: { fileId: 'f-started' } });
248+
},
249+
},
225250
{
226251
name: 'anonymous upload when a session resolver is wired',
227252
status: 401,

‎packages/services/service-storage/src/metadata-store.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import type { IDataEngine } from '@objectstack/spec/contracts';
2222
import {
2323
StorageMetadataStore,
2424
StorageMetadataStoreError,
25+
organizationOutOfWriteReach,
2526
type FileRecord,
2627
type UploadSessionRecord,
2728
} from './metadata-store.js';
@@ -359,3 +360,47 @@ describe('StorageMetadataStore: engine wired and failing (#5216)', () => {
359360
);
360361
});
361362
});
363+
364+
// ---------------------------------------------------------------------------
365+
// 4. #22175 — does the scoped by-id write reach the row? Asked before writing.
366+
// ---------------------------------------------------------------------------
367+
368+
describe('organizationOutOfWriteReach (#22175)', () => {
369+
const inOrgA = { organization_id: 'org_a' };
370+
371+
it('names the starting organization when the acting one differs, on an engine-backed store', () => {
372+
const store = new StorageMetadataStore(createFakeEngine());
373+
expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: 'org_b' })).toBe('org_a');
374+
});
375+
376+
it('answers null when the row is in reach: same organization', () => {
377+
const store = new StorageMetadataStore(createFakeEngine());
378+
expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: 'org_a' })).toBeNull();
379+
});
380+
381+
it('answers null when no organization is threaded — the write is unscoped', () => {
382+
const store = new StorageMetadataStore(createFakeEngine());
383+
expect(organizationOutOfWriteReach(store, inOrgA)).toBeNull();
384+
expect(organizationOutOfWriteReach(store, inOrgA, {})).toBeNull();
385+
expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: '' })).toBeNull();
386+
expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: null })).toBeNull();
387+
});
388+
389+
it('answers null for a row stamped with no organization — it stays in reach', () => {
390+
const store = new StorageMetadataStore(createFakeEngine());
391+
for (const row of [{}, { organization_id: null }, { organization_id: '' }]) {
392+
expect(organizationOutOfWriteReach(store, row, { organizationId: 'org_b' })).toBeNull();
393+
}
394+
});
395+
396+
it('answers null for a missing row — there is nothing for it to refuse', () => {
397+
const store = new StorageMetadataStore(createFakeEngine());
398+
expect(organizationOutOfWriteReach(store, null, { organizationId: 'org_b' })).toBeNull();
399+
expect(organizationOutOfWriteReach(store, undefined, { organizationId: 'org_b' })).toBeNull();
400+
});
401+
402+
it('answers null on the engine-absent stand-in, which does not scope its writes', () => {
403+
const store = new StorageMetadataStore(null);
404+
expect(organizationOutOfWriteReach(store, inOrgA, { organizationId: 'org_b' })).toBeNull();
405+
});
406+
});

0 commit comments

Comments
 (0)