Skip to content

Commit 3217440

Browse files
committed
fix(kiloclaw): preserve legacy routing after user ID migration
1 parent 0d2140f commit 3217440

File tree

11 files changed

+541
-144
lines changed

11 files changed

+541
-144
lines changed

services/kiloclaw/src/db/index.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,14 @@ export async function getActiveInstance(db: WorkerDb, userId: string) {
6969
organization_id: kiloclaw_instances.organization_id,
7070
})
7171
.from(kiloclaw_instances)
72-
.where(and(eq(kiloclaw_instances.user_id, userId), isNull(kiloclaw_instances.destroyed_at)))
72+
.where(
73+
and(
74+
eq(kiloclaw_instances.user_id, userId),
75+
isNull(kiloclaw_instances.organization_id),
76+
isNull(kiloclaw_instances.destroyed_at)
77+
)
78+
)
79+
.orderBy(kiloclaw_instances.created_at)
7380
.limit(1)
7481
.then(rows => rows[0] ?? null);
7582

services/kiloclaw/src/durable-objects/kiloclaw-instance/index.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import {
6969
markRestartSuccessful,
7070
} from './reconcile';
7171
import { restoreFromPostgres, markDestroyedInPostgresHelper } from './postgres';
72+
import { legacyDoKeysForIdentity } from '../../lib/instance-routing';
7273
import {
7374
beginUnexpectedStopRecovery,
7475
cleanupPendingRecoveryVolumeIfNeeded,
@@ -1440,22 +1441,23 @@ export class KiloClawInstance extends DurableObject<KiloClawEnv> {
14401441
instanceId: registryInstanceId,
14411442
});
14421443
} else {
1443-
// Legacy: find active entry by doKey=userId
1444+
const legacyDoKeys = legacyDoKeysForIdentity(preDestroyUserId, preDestroySandboxId);
14441445
const entries = await registryStub.listInstances(registryKey);
1445-
const legacyEntry = entries.find(e => e.doKey === preDestroyUserId);
1446+
const legacyEntry = entries.find(e => legacyDoKeys.includes(e.doKey));
14461447
if (legacyEntry) {
14471448
await registryStub.destroyInstance(registryKey, legacyEntry.instanceId);
14481449
console.log('[DO] Registry entry destroyed on finalization (legacy):', {
14491450
registryKey,
14501451
instanceId: legacyEntry.instanceId,
1451-
doKey: preDestroyUserId,
1452+
doKeysTried: legacyDoKeys,
1453+
matchedDoKey: legacyEntry.doKey,
14521454
});
14531455
} else {
14541456
console.log(
14551457
'[DO] Registry cleanup: no active entry found (already cleaned or never existed):',
14561458
{
14571459
registryKey,
1458-
doKey: preDestroyUserId,
1460+
doKeysTried: legacyDoKeys,
14591461
activeEntryCount: entries.length,
14601462
}
14611463
);
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { fallbackAppNameForRestore } from './postgres';
3+
import { sandboxIdFromUserId } from '../../auth/sandbox-id';
4+
import { appNameFromUserId, appNameFromInstanceId } from '../../fly/apps';
5+
6+
describe('fallbackAppNameForRestore', () => {
7+
it('keeps migrated legacy sandboxes on the acct-* naming path', async () => {
8+
const legacyUserId = 'oauth/google:117453785559478190551';
9+
const migratedUserId = '199e2b19-aa40-488d-9442-9a18a620ba68';
10+
11+
await expect(
12+
fallbackAppNameForRestore(migratedUserId, sandboxIdFromUserId(legacyUserId))
13+
).resolves.toBe(await appNameFromUserId(legacyUserId));
14+
});
15+
16+
it('keeps ki_ sandboxes on the inst-* naming path', async () => {
17+
const instanceId = '11111111-1111-4111-8111-111111111111';
18+
19+
await expect(
20+
fallbackAppNameForRestore(
21+
'199e2b19-aa40-488d-9442-9a18a620ba68',
22+
'ki_11111111111141118111111111111111'
23+
)
24+
).resolves.toBe(await appNameFromInstanceId(instanceId));
25+
});
26+
});

services/kiloclaw/src/durable-objects/kiloclaw-instance/postgres.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,24 @@ import { getAppKey, getFlyConfig } from './types';
1212
import { storageUpdate } from './state';
1313
import { attemptMetadataRecovery } from './reconcile';
1414
import { doError, doWarn, toLoggable, createReconcileContext } from './log';
15+
import { isInstanceKeyedSandboxId } from '@kilocode/worker-utils/instance-id';
1516

1617
type RestoreOpts = {
1718
/** If the DO has a stored sandboxId, use it for precise lookup. */
1819
sandboxId?: string | null;
1920
};
2021

22+
export async function fallbackAppNameForRestore(
23+
userId: string,
24+
sandboxId: string,
25+
prefix?: string
26+
): Promise<string> {
27+
const appKey = getAppKey({ userId, sandboxId });
28+
return isInstanceKeyedSandboxId(sandboxId)
29+
? appNameFromInstanceId(appKey, prefix)
30+
: appNameFromUserId(appKey, prefix);
31+
}
32+
2133
/**
2234
* Restore DO state from Postgres backup if SQLite was wiped.
2335
*
@@ -63,10 +75,7 @@ export async function restoreFromPostgres(
6375
const appKey = getAppKey({ userId, sandboxId: instance.sandboxId });
6476
const appStub = env.KILOCLAW_APP.get(env.KILOCLAW_APP.idFromName(appKey));
6577
const prefix = env.WORKER_ENV === 'development' ? 'dev' : undefined;
66-
const isInstanceKeyed = appKey !== userId;
67-
const fallbackAppName = isInstanceKeyed
68-
? await appNameFromInstanceId(appKey, prefix)
69-
: await appNameFromUserId(userId, prefix);
78+
const fallbackAppName = await fallbackAppNameForRestore(userId, instance.sandboxId, prefix);
7079
const recoveredAppName = (await appStub.getAppName()) ?? fallbackAppName;
7180

7281
await ctx.storage.put(
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { getAppKey } from './types';
3+
import { sandboxIdFromUserId } from '../../auth/sandbox-id';
4+
5+
describe('getAppKey', () => {
6+
it('derives the legacy app owner key from sandboxId instead of migrated userId', () => {
7+
const legacyUserId = 'oauth/google:117453785559478190551';
8+
9+
expect(
10+
getAppKey({
11+
userId: '199e2b19-aa40-488d-9442-9a18a620ba68',
12+
sandboxId: sandboxIdFromUserId(legacyUserId),
13+
})
14+
).toBe(legacyUserId);
15+
});
16+
17+
it('keeps instance-keyed sandboxes on the instanceId owner key', () => {
18+
expect(
19+
getAppKey({
20+
userId: '199e2b19-aa40-488d-9442-9a18a620ba68',
21+
sandboxId: 'ki_11111111111141118111111111111111',
22+
})
23+
).toBe('11111111-1111-4111-8111-111111111111');
24+
});
25+
});

services/kiloclaw/src/durable-objects/kiloclaw-instance/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { KiloClawEnv } from '../../types';
22
import type { GoogleCredentials, PersistedState, MachineSize } from '../../schemas/instance-config';
33
import type { FlyClientConfig } from '../../fly/client';
4+
import { userIdFromSandboxId } from '../../auth/sandbox-id';
45
import {
56
isInstanceKeyedSandboxId,
67
instanceIdFromSandboxId,
@@ -136,6 +137,14 @@ export function getAppKey(state: { userId: string | null; sandboxId: string | nu
136137
if (state.sandboxId && isInstanceKeyedSandboxId(state.sandboxId)) {
137138
return instanceIdFromSandboxId(state.sandboxId);
138139
}
140+
if (state.sandboxId) {
141+
try {
142+
return userIdFromSandboxId(state.sandboxId);
143+
} catch {
144+
// Older tests and malformed legacy state can still carry placeholder
145+
// sandboxIds that are not reversible base64url encodings.
146+
}
147+
}
139148
if (state.userId) return state.userId;
140149
throw new Error('Cannot derive app key: no sandboxId or userId');
141150
}

services/kiloclaw/src/durable-objects/kiloclaw-registry.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,8 @@ import { eq, isNull, and } from 'drizzle-orm';
55
import migrations from '../../drizzle/migrations';
66
import { registryInstances } from '../db/sqlite-schema';
77
import { getWorkerDb, getActiveInstance } from '../db';
8-
import {
9-
isInstanceKeyedSandboxId,
10-
instanceIdFromSandboxId,
11-
} from '@kilocode/worker-utils/instance-id';
128
import type { KiloClawEnv } from '../types';
9+
import { doKeyFromActiveInstance } from '../lib/instance-routing';
1310

1411
export type RegistryEntry = {
1512
instanceId: string;
@@ -209,13 +206,7 @@ export class KiloClawRegistry extends DurableObject<KiloClawEnv> {
209206
const instance = await getActiveInstance(db, userId);
210207

211208
if (instance) {
212-
// Backfill registry entry from Postgres row.
213-
// Derive doKey from the row's sandboxId format:
214-
// - ki_ prefix → instance-keyed DO at idFromName(instanceId)
215-
// - base64url → legacy DO at idFromName(userId)
216-
const doKey = isInstanceKeyedSandboxId(instance.sandboxId)
217-
? instanceIdFromSandboxId(instance.sandboxId)
218-
: userId;
209+
const doKey = doKeyFromActiveInstance(instance);
219210
this.db
220211
.insert(registryInstances)
221212
.values({

services/kiloclaw/src/index.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { sandboxIdFromUserId } from './auth/sandbox-id';
2525
import { InstanceIdParam } from './schemas/instance-config';
2626
import { isValidInstanceId } from '@kilocode/worker-utils/instance-id';
2727
import { registerVersionIfNeeded } from './lib/image-version';
28+
import { resolveDoKeyForUser } from './lib/instance-routing';
2829
import { startingUpPage } from './pages/starting-up';
2930
import { buildForwardHeaders } from './utils/proxy-headers';
3031
import { KILOCLAW_ACTIVE_INSTANCE_COOKIE } from './config';
@@ -349,16 +350,21 @@ async function resolveRegistryEntry(c: Context<AppEnv>) {
349350
const stub = c.env.KILOCLAW_INSTANCE.get(c.env.KILOCLAW_INSTANCE.idFromName(entry.doKey));
350351
return { stub, entry };
351352
} catch (err) {
352-
// Registry DO failed. Fall back to the legacy userId-keyed DO.
353-
// Only preserves access for legacy instances (doKey = userId).
354-
// For instance-keyed DOs (doKey = instanceId), this hits the wrong
355-
// (empty) DO — the user sees "not provisioned" until the registry
356-
// recovers. Acceptable: a broken registry is transient, and silently
357-
// routing to the wrong DO would be worse.
358-
console.error('[PROXY] Registry lookup failed, falling back to legacy DO:', err);
359-
const stub = c.env.KILOCLAW_INSTANCE.get(c.env.KILOCLAW_INSTANCE.idFromName(userId));
353+
console.error(
354+
'[PROXY] Registry lookup failed, falling back to Postgres-backed DO lookup:',
355+
err
356+
);
357+
const fallbackDoKey =
358+
(await resolveDoKeyForUser(c.env.HYPERDRIVE?.connectionString, userId).catch(fallbackErr => {
359+
console.error(
360+
'[PROXY] Postgres-backed DO lookup failed, falling back to userId:',
361+
fallbackErr
362+
);
363+
return null;
364+
})) ?? userId;
365+
const stub = c.env.KILOCLAW_INSTANCE.get(c.env.KILOCLAW_INSTANCE.idFromName(fallbackDoKey));
360366
const fallbackEntry: RegistryEntry = {
361-
doKey: userId,
367+
doKey: fallbackDoKey,
362368
instanceId: '',
363369
assignedUserId: userId,
364370
createdAt: '',
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import { getActiveInstance, getWorkerDb } from '../db';
2+
import { userIdFromSandboxId } from '../auth/sandbox-id';
3+
import {
4+
instanceIdFromSandboxId,
5+
isInstanceKeyedSandboxId,
6+
} from '@kilocode/worker-utils/instance-id';
7+
8+
type ActiveInstanceIdentity = {
9+
id: string;
10+
sandboxId: string;
11+
};
12+
13+
export function legacyDoKeysForIdentity(userId: string, sandboxId: string): string[] {
14+
const keys = new Set<string>([userId]);
15+
16+
if (!isInstanceKeyedSandboxId(sandboxId)) {
17+
try {
18+
keys.add(userIdFromSandboxId(sandboxId));
19+
} catch {
20+
// Placeholder sandboxIds can exist in old tests or malformed state.
21+
}
22+
}
23+
24+
return [...keys];
25+
}
26+
27+
export function doKeyFromActiveInstance(instance: ActiveInstanceIdentity): string {
28+
return isInstanceKeyedSandboxId(instance.sandboxId)
29+
? instanceIdFromSandboxId(instance.sandboxId)
30+
: userIdFromSandboxId(instance.sandboxId);
31+
}
32+
33+
export async function resolveDoKeyForUser(
34+
connectionString: string | undefined,
35+
userId: string
36+
): Promise<string | null> {
37+
if (!connectionString) return null;
38+
39+
const instance = await getActiveInstance(getWorkerDb(connectionString), userId);
40+
if (!instance) return null;
41+
42+
return doKeyFromActiveInstance(instance);
43+
}

0 commit comments

Comments
 (0)