Skip to content

Commit 0404d30

Browse files
hotlongCopilot
andcommitted
fix(preview): align commit-id length with publish (16 hex) + use $contains
Two bugs found during E2E smoke (cloud :4000 + preview :3100, real CRM project, real published artifact): 1. Host parser only matched 12-hex commit ids, but `publish` derives commitId via `sha256(artifact).slice(0,16)` (see routes/cloud.ts line 228). Result: every real preview URL was parsed as a branch slug, so the per-request HEAD revalidation flow tried to look up a 16-hex string as a branch name and 404'd. Fix: HEX12_RE -> COMMIT_HEX_RE (^[0-9a-f]{16}$). Test fixtures extended; doc comments aligned. 25 host-parser tests still pass. 2. Short-id lookup endpoint used { id: { $like: '...%' } } but the SQL driver supports $contains (LIKE %x%), not $like. Result: /cloud/projects-by-short-id/<8hex> always returned 404 even for valid prefixes. Fix: switch to $contains; the existing post-filter (id.replace(/-/g,'').startsWith(raw)) keeps the result exact. With limit:16 candidates per query the over-fetch is negligible. Test FakeDriver updated to match. Verified end-to-end (real cloud + preview): - GET /cloud/projects-by-short-id/67a5df1a -> 200 with full UUID - GET /api/v1/data/account on 3ec1ad2bd2b8613d--67a5df1a.localhost:3100 -> 200, 5 seeded records - POST /api/v1/data/account on same host -> 200, list now 6 - Same path on main--67a5df1a.localhost:3100 -> 200 (branch host) - Bad commit (deadbeefdeadbeef) -> 400 "artifact not found" - Browser navigation confirmed for both commit and branch hosts 121/121 service-cloud tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a6ed642 commit 0404d30

6 files changed

Lines changed: 54 additions & 46 deletions

File tree

packages/services/service-cloud/src/preview/environment-registry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type IDataDriver = Contracts.IDataDriver;
3131
interface CompositeProject {
3232
id: string; // composite key: `${projectId}:${commitId}`
3333
projectId: string; // real project UUID
34-
commitId: string; // pinned 12-hex (or longer) commit id
34+
commitId: string; // pinned 16-hex commit id
3535
organization_id?: string;
3636
/** Set when the entry was resolved via a branch slug. */
3737
branchName?: string;

packages/services/service-cloud/src/preview/host-parser.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
* • `pidShort` is exactly 8 lowercase hex chars (= first 8 hex chars of
88
* a project UUID, dashes stripped).
99
* • `ref` is either:
10-
* - exactly 12 lowercase hex chars → a commit-pinned preview, or
10+
* - exactly 16 lowercase hex chars → a commit-pinned preview
11+
* (publish derives commitId = sha256(artifact).slice(0,16)), or
1112
* - a branch slug (matches BRANCH_SLUG_RE elsewhere) → branch-tracking.
1213
* • `<base>` is one of the configured preview base domains, e.g.
1314
* `preview.objectstack.ai` (prod) or `localhost[:port]` (dev,
@@ -24,18 +25,18 @@
2425
* pure string function so it can be unit-tested without a registry.
2526
*/
2627

27-
const HEX12_RE = /^[0-9a-f]{12}$/;
28+
const COMMIT_HEX_RE = /^[0-9a-f]{16}$/;
2829
const PID_SHORT_RE = /^[0-9a-f]{8}$/;
2930
/** Same shape as BRANCH_SLUG_RE in routes/branches.ts. Duplicated here to
3031
* avoid pulling that file into the parser's import graph. */
3132
const BRANCH_SLUG_RE = /^[a-z0-9][a-z0-9._/-]{0,62}$/;
3233

3334
export interface PreviewHost {
34-
/** 'commit' = ref is a 12-hex commit id; 'branch' = ref is a slug. */
35+
/** 'commit' = ref is a 16-hex commit id; 'branch' = ref is a slug. */
3536
kind: 'commit' | 'branch';
3637
/** First 8 hex chars of the project's UUID (no dashes). */
3738
pidShort: string;
38-
/** Either a 12-hex commit id or a branch slug. */
39+
/** Either a 16-hex commit id or a branch slug. */
3940
ref: string;
4041
}
4142

@@ -111,7 +112,7 @@ export function parsePreviewHost(host: string, config?: PreviewParseConfig): Pre
111112
if (!PID_SHORT_RE.test(pidShort)) return null;
112113
if (!ref) return null;
113114

114-
if (HEX12_RE.test(ref)) {
115+
if (COMMIT_HEX_RE.test(ref)) {
115116
return { kind: 'commit', pidShort, ref };
116117
}
117118
if (BRANCH_SLUG_RE.test(ref)) {

packages/services/service-cloud/src/routes/cloud.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,15 @@ export function registerCloudRoutes(server: IHttpServer, deps: RouteDeps): void
8282

8383
try {
8484
// The short id is a UUID prefix without dashes; the stored
85-
// `sys_project.id` is a canonical UUID *with* dashes. Use a
86-
// suffix-stripped LIKE: the first 8 hex chars of the UUID
87-
// are always the bytes before the first dash, so we can
88-
// match `${raw.slice(0,8)}%` and then post-filter exact.
85+
// `sys_project.id` is a canonical UUID *with* dashes. The first
86+
// 8 hex chars of the UUID are always the bytes before the first
87+
// dash. Most drivers expose `$contains` (LIKE %x%); we use that
88+
// to over-fetch a small candidate set, then post-filter to an
89+
// exact prefix match. UUIDs make collisions on 8 hex chars rare,
90+
// and we cap the candidate scan at 16 rows.
8991
const headHex = raw.slice(0, 8);
9092
const candidates = (await (driver.find as any)('sys_project', {
91-
where: { id: { $like: `${headHex}%` } },
93+
where: { id: { $contains: headHex } },
9294
limit: 16,
9395
})) as Array<{ id: string; organization_id?: string }>;
9496
const matches = candidates.filter((p) => p.id.replace(/-/g, '').toLowerCase().startsWith(raw));

packages/services/service-cloud/test/cloud-artifact-api-plugin.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,14 @@ class FakeDriver {
8989
const where = query?.where ?? {};
9090
if (table === 'sys_project') {
9191
return this.projects.filter((p) => Object.entries(where).every(([k, v]) => {
92-
if (v && typeof v === 'object' && '$like' in (v as any)) {
93-
const pattern = String((v as any).$like).replace(/%/g, '.*');
94-
return new RegExp(`^${pattern}$`).test(String((p as any)[k] ?? ''));
92+
if (v && typeof v === 'object') {
93+
if ('$like' in (v as any)) {
94+
const pattern = String((v as any).$like).replace(/%/g, '.*');
95+
return new RegExp(`^${pattern}$`).test(String((p as any)[k] ?? ''));
96+
}
97+
if ('$contains' in (v as any)) {
98+
return String((p as any)[k] ?? '').includes(String((v as any).$contains));
99+
}
95100
}
96101
return (p as any)[k] === v;
97102
})).slice(0, query?.limit ?? 100);

packages/services/service-cloud/test/preview-environment-registry.test.ts

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ describe('PreviewEnvironmentRegistry', () => {
6363
'7f3e9a01': { projectId: '7f3e9a01-1234-5678-9abc-def012345678' },
6464
});
6565
const reg = makeRegistry(client);
66-
const r = await reg.resolveByHostname('abc123def456--7f3e9a01.preview.objectstack.ai');
66+
const r = await reg.resolveByHostname('abc123def4567890--7f3e9a01.preview.objectstack.ai');
6767
expect(r).not.toBeNull();
68-
expect(r!.projectId).toBe('7f3e9a01-1234-5678-9abc-def012345678:abc123def456');
68+
expect(r!.projectId).toBe('7f3e9a01-1234-5678-9abc-def012345678:abc123def4567890');
6969
expect(client.lookups).toBe(1);
7070
expect(client.headLookups).toBe(0);
7171
expect(FakeDriver.instances).toBe(1);
@@ -76,7 +76,7 @@ describe('PreviewEnvironmentRegistry', () => {
7676
'7f3e9a01': { projectId: '7f3e9a01-1234-5678-9abc-def012345678' },
7777
});
7878
const reg = makeRegistry(client);
79-
const host = 'abc123def456--7f3e9a01.preview.objectstack.ai';
79+
const host = 'abc123def4567890--7f3e9a01.preview.objectstack.ai';
8080
await reg.resolveByHostname(host);
8181
await reg.resolveByHostname(host);
8282
await reg.resolveByHostname(host);
@@ -89,10 +89,10 @@ describe('PreviewEnvironmentRegistry', () => {
8989
'7f3e9a01': { projectId: '7f3e9a01-1234-5678-9abc-def012345678' },
9090
});
9191
const reg = makeRegistry(client);
92-
await reg.resolveByHostname('abc123def456--7f3e9a01.preview.objectstack.ai');
93-
const peek = reg.peekById('7f3e9a01-1234-5678-9abc-def012345678:abc123def456');
92+
await reg.resolveByHostname('abc123def4567890--7f3e9a01.preview.objectstack.ai');
93+
const peek = reg.peekById('7f3e9a01-1234-5678-9abc-def012345678:abc123def4567890');
9494
expect(peek).not.toBeNull();
95-
expect(peek!.project.commitId).toBe('abc123def456');
95+
expect(peek!.project.commitId).toBe('abc123def4567890');
9696
expect(peek!.project.projectId).toBe('7f3e9a01-1234-5678-9abc-def012345678');
9797
});
9898
});
@@ -101,18 +101,18 @@ describe('PreviewEnvironmentRegistry', () => {
101101
it('resolves a branch host using its current head', async () => {
102102
const client = new FakeClient(
103103
{ '7f3e9a01': { projectId: 'proj-1' } },
104-
{ 'proj-1': { main: { commitId: 'abc123def456' } } },
104+
{ 'proj-1': { main: { commitId: 'abc123def4567890' } } },
105105
);
106106
const reg = makeRegistry(client);
107107
const r = await reg.resolveByHostname('main--7f3e9a01.preview.objectstack.ai');
108-
expect(r!.projectId).toBe('proj-1:abc123def456');
108+
expect(r!.projectId).toBe('proj-1:abc123def4567890');
109109
expect(client.headLookups).toBe(1);
110110
});
111111

112112
it('re-checks branch head on every request (per-request semantics)', async () => {
113113
const client = new FakeClient(
114114
{ '7f3e9a01': { projectId: 'proj-1' } },
115-
{ 'proj-1': { main: { commitId: 'abc123def456' } } },
115+
{ 'proj-1': { main: { commitId: 'abc123def4567890' } } },
116116
);
117117
const reg = makeRegistry(client);
118118
const host = 'main--7f3e9a01.preview.objectstack.ai';
@@ -128,18 +128,18 @@ describe('PreviewEnvironmentRegistry', () => {
128128
it('evicts and rebuilds when branch head advances', async () => {
129129
const client = new FakeClient(
130130
{ '7f3e9a01': { projectId: 'proj-1' } },
131-
{ 'proj-1': { main: { commitId: 'aaaaaaaaaaaa' } } },
131+
{ 'proj-1': { main: { commitId: 'aaaaaaaaaaaaaaaa' } } },
132132
);
133133
const reg = makeRegistry(client);
134134
const host = 'main--7f3e9a01.preview.objectstack.ai';
135135
const first = await reg.resolveByHostname(host);
136-
expect(first!.projectId).toBe('proj-1:aaaaaaaaaaaa');
136+
expect(first!.projectId).toBe('proj-1:aaaaaaaaaaaaaaaa');
137137

138138
// Advance the head.
139-
client.branches['proj-1']!.main = { commitId: 'bbbbbbbbbbbb' };
139+
client.branches['proj-1']!.main = { commitId: 'bbbbbbbbbbbbbbbb' };
140140

141141
const second = await reg.resolveByHostname(host);
142-
expect(second!.projectId).toBe('proj-1:bbbbbbbbbbbb');
142+
expect(second!.projectId).toBe('proj-1:bbbbbbbbbbbbbbbb');
143143
expect(client.artifactInvalidations).toBeGreaterThanOrEqual(1);
144144
expect(FakeDriver.instances).toBe(2); // fresh driver for the new commit
145145
});
@@ -167,7 +167,7 @@ describe('PreviewEnvironmentRegistry', () => {
167167
it('singleflights concurrent first-resolve requests', async () => {
168168
const client = new FakeClient(
169169
{ '7f3e9a01': { projectId: 'proj-1' } },
170-
{ 'proj-1': { main: { commitId: 'aaaaaaaaaaaa' } } },
170+
{ 'proj-1': { main: { commitId: 'aaaaaaaaaaaaaaaa' } } },
171171
);
172172
const reg = makeRegistry(client);
173173
const host = 'main--7f3e9a01.preview.objectstack.ai';
@@ -176,9 +176,9 @@ describe('PreviewEnvironmentRegistry', () => {
176176
reg.resolveByHostname(host),
177177
reg.resolveByHostname(host),
178178
]);
179-
expect(a!.projectId).toBe('proj-1:aaaaaaaaaaaa');
180-
expect(b!.projectId).toBe('proj-1:aaaaaaaaaaaa');
181-
expect(c!.projectId).toBe('proj-1:aaaaaaaaaaaa');
179+
expect(a!.projectId).toBe('proj-1:aaaaaaaaaaaaaaaa');
180+
expect(b!.projectId).toBe('proj-1:aaaaaaaaaaaaaaaa');
181+
expect(c!.projectId).toBe('proj-1:aaaaaaaaaaaaaaaa');
182182
expect(client.lookups).toBe(1);
183183
// One head fetch during initial build, no per-request re-check
184184
// because all three calls dedupe through `pending`.
@@ -193,10 +193,10 @@ describe('PreviewEnvironmentRegistry', () => {
193193
'7f3e9a01': { projectId: 'proj-1' },
194194
});
195195
const reg = makeRegistry(client);
196-
await reg.resolveByHostname('abc123def456--7f3e9a01.localhost');
197-
expect(reg.peekById('proj-1:abc123def456')).not.toBeNull();
196+
await reg.resolveByHostname('abc123def4567890--7f3e9a01.localhost');
197+
expect(reg.peekById('proj-1:abc123def4567890')).not.toBeNull();
198198
reg.clear();
199-
expect(reg.peekById('proj-1:abc123def456')).toBeNull();
199+
expect(reg.peekById('proj-1:abc123def4567890')).toBeNull();
200200
});
201201
});
202202
});

packages/services/service-cloud/test/preview-host-parser.test.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,25 +4,25 @@ import { describe, it, expect } from 'vitest';
44
import { parsePreviewHost, projectIdToShort } from '../src/preview/host-parser.js';
55

66
describe('parsePreviewHost', () => {
7-
describe('commit-pinned (12 hex)', () => {
7+
describe('commit-pinned (16 hex)', () => {
88
it('parses prod hostname', () => {
9-
const r = parsePreviewHost('abc123def456--7f3e9a01.preview.objectstack.ai');
10-
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def456' });
9+
const r = parsePreviewHost('abc123def4567890--7f3e9a01.preview.objectstack.ai');
10+
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def4567890' });
1111
});
1212

1313
it('parses localhost hostname (RFC 6761)', () => {
14-
const r = parsePreviewHost('abc123def456--7f3e9a01.localhost');
15-
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def456' });
14+
const r = parsePreviewHost('abc123def4567890--7f3e9a01.localhost');
15+
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def4567890' });
1616
});
1717

1818
it('strips :port', () => {
19-
const r = parsePreviewHost('abc123def456--7f3e9a01.localhost:4100');
20-
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def456' });
19+
const r = parsePreviewHost('abc123def4567890--7f3e9a01.localhost:4100');
20+
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def4567890' });
2121
});
2222

2323
it('lowercases input', () => {
24-
const r = parsePreviewHost('ABC123DEF456--7F3E9A01.PREVIEW.OBJECTSTACK.AI');
25-
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def456' });
24+
const r = parsePreviewHost('ABC123DEF4567890--7F3E9A01.PREVIEW.OBJECTSTACK.AI');
25+
expect(r).toEqual({ kind: 'commit', pidShort: '7f3e9a01', ref: 'abc123def4567890' });
2626
});
2727
});
2828

@@ -102,16 +102,16 @@ describe('parsePreviewHost', () => {
102102
});
103103

104104
describe('commit/branch disambiguation', () => {
105-
it('exactly 12 hex → commit', () => {
106-
expect(parsePreviewHost('0123456789ab--7f3e9a01.localhost')?.kind).toBe('commit');
105+
it('exactly 16 hex → commit', () => {
106+
expect(parsePreviewHost('0123456789abcdef--7f3e9a01.localhost')?.kind).toBe('commit');
107107
});
108108

109109
it('11 hex → branch (slug regex still matches)', () => {
110110
expect(parsePreviewHost('0123456789a--7f3e9a01.localhost')?.kind).toBe('branch');
111111
});
112112

113113
it('13 hex → branch (slug regex still matches)', () => {
114-
expect(parsePreviewHost('0123456789abc--7f3e9a01.localhost')?.kind).toBe('branch');
114+
expect(parsePreviewHost('0123456789abcde--7f3e9a01.localhost')?.kind).toBe('branch');
115115
});
116116
});
117117
});

0 commit comments

Comments
 (0)