Skip to content

Commit fbb8f1d

Browse files
committed
test: close pr86 greptile follow-ups
1 parent ea3b559 commit fbb8f1d

3 files changed

Lines changed: 42 additions & 24 deletions

File tree

lib/storage.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -943,15 +943,6 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 {
943943
return { version: 1, accounts: [] };
944944
}
945945

946-
// Flagged storage must keep sibling workspace entries separate when they share an
947-
// organization but have different accountIds, so this key is more specific than
948-
// the normal account identity collapse used in active storage.
949-
const getFlaggedIdentityKey = (account: {
950-
organizationId?: string;
951-
accountId?: string;
952-
refreshToken: string;
953-
}): string => getWorkspaceIdentityKey(account);
954-
955946
const byIdentityKey = new Map<string, FlaggedAccountMetadataV1>();
956947
for (const rawAccount of data.accounts) {
957948
if (!isRecord(rawAccount)) continue;
@@ -1031,7 +1022,9 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 {
10311022
flaggedReason: typeof rawAccount.flaggedReason === "string" ? rawAccount.flaggedReason : undefined,
10321023
lastError: typeof rawAccount.lastError === "string" ? rawAccount.lastError : undefined,
10331024
};
1034-
byIdentityKey.set(getFlaggedIdentityKey(normalized), normalized);
1025+
// Keep flagged dedup aligned with active cleanup so sibling workspaces only
1026+
// collapse when they resolve to the same shared workspace identity.
1027+
byIdentityKey.set(getWorkspaceIdentityKey(normalized), normalized);
10351028
}
10361029

10371030
return {

test/index.test.ts

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,10 @@ const mockSaveFlaggedAccounts = vi.fn(async (nextStorage: typeof mockFlaggedStor
272272
await persistMockFlaggedStorage(nextStorage);
273273
});
274274

275-
vi.mock("../lib/storage.js", () => ({
275+
vi.mock("../lib/storage.js", async () => {
276+
const actual = await vi.importActual<typeof import("../lib/storage.js")>("../lib/storage.js");
277+
278+
return {
276279
getStoragePath: () => "/mock/path/accounts.json",
277280
loadAccounts: vi.fn(async () => cloneMockStorage()),
278281
saveAccounts: vi.fn(async (nextStorage: typeof mockStorage) => {
@@ -311,18 +314,7 @@ vi.mock("../lib/storage.js", () => ({
311314
previewImportAccounts: vi.fn(async () => ({ imported: 2, skipped: 1, total: 5 })),
312315
createTimestampedBackupPath: vi.fn((prefix?: string) => `/tmp/${prefix ?? "codex-backup"}-20260101-000000.json`),
313316
loadFlaggedAccounts: vi.fn(async () => cloneMockFlaggedStorage()),
314-
getWorkspaceIdentityKey: vi.fn((account: { organizationId?: string; accountId?: string; refreshToken: string }) => {
315-
const organizationId = account.organizationId?.trim();
316-
const accountId = account.accountId?.trim();
317-
const refreshToken = account.refreshToken.trim();
318-
if (organizationId) {
319-
return accountId
320-
? `organizationId:${organizationId}|accountId:${accountId}`
321-
: `organizationId:${organizationId}`;
322-
}
323-
if (accountId) return `accountId:${accountId}`;
324-
return `refreshToken:${refreshToken}`;
325-
}),
317+
getWorkspaceIdentityKey: vi.fn(actual.getWorkspaceIdentityKey),
326318
saveFlaggedAccounts: mockSaveFlaggedAccounts,
327319
withFlaggedAccountStorageTransaction: vi.fn(
328320
async <T>(
@@ -347,7 +339,8 @@ vi.mock("../lib/storage.js", () => ({
347339
}
348340
},
349341
formatStorageErrorHint: () => "Check file permissions",
350-
}));
342+
};
343+
});
351344

352345
vi.mock("../lib/accounts.js", () => {
353346
class MockAccountManager {

test/storage.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
importAccounts,
2121
previewImportAccounts,
2222
createTimestampedBackupPath,
23+
getWorkspaceIdentityKey,
2324
withAccountStorageTransaction,
2425
withFlaggedAccountStorageTransaction,
2526
} from "../lib/storage.js";
@@ -30,6 +31,37 @@ import {
3031
// But Task 0 says: "Tests should fail initially (RED phase)"
3132

3233
describe("storage", () => {
34+
describe("getWorkspaceIdentityKey", () => {
35+
it("uses organizationId and accountId when both are present", () => {
36+
expect(
37+
getWorkspaceIdentityKey({
38+
organizationId: " org-shared ",
39+
accountId: " workspace-a ",
40+
refreshToken: " refresh-a ",
41+
}),
42+
).toBe("organizationId:org-shared|accountId:workspace-a");
43+
});
44+
45+
it("falls back to accountId when organizationId is missing", () => {
46+
expect(
47+
getWorkspaceIdentityKey({
48+
accountId: " workspace-only ",
49+
refreshToken: " refresh-b ",
50+
}),
51+
).toBe("accountId:workspace-only");
52+
});
53+
54+
it("falls back to refreshToken when workspace ids are missing", () => {
55+
expect(
56+
getWorkspaceIdentityKey({
57+
organizationId: " ",
58+
accountId: "",
59+
refreshToken: " refresh-c ",
60+
}),
61+
).toBe("refreshToken:refresh-c");
62+
});
63+
});
64+
3365
describe("deduplication", () => {
3466
it("remaps activeIndex after deduplication using active account key", () => {
3567
const now = Date.now();

0 commit comments

Comments
 (0)