Skip to content

ENG-1656: Rename imported vault folders to owner-username-spaceName #1130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
trangdoan982 wants to merge 10 commits into
base: main
Choose a base branch
from
+149 −78
Open

ENG-1656: Rename imported vault folders to owner-username-spaceName #1130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1c3615e
ENG-1656: Rename imported vault folders to owner-username-spaceName
trangdoan982 Jun 18, 2026
2bd6a6d
Fix import folder rename edge cases and CI lint findings
trangdoan982 Jun 18, 2026
42425cf
remove tests
trangdoan982 Jun 18, 2026
aaf0cdf
cleanup
trangdoan982 Jun 18, 2026
7eb9501
simplify the flow
trangdoan982 Jun 18, 2026
159aaa8
bug fix
trangdoan982 Jun 21, 2026
72bf26d
remove auto-rename migration
trangdoan982 Jun 22, 2026
0803a25
address review: type guard, inline JSON.parse, explain trailing-comma…
trangdoan982 Jun 22, 2026
3ea5563
replace heuristic author resolution with Supabase space membership query
trangdoan982 Jun 22, 2026
598da9c
simplify owner name resolution: reuse fetchUserNames + authorId lookup
trangdoan982 Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 132 additions & 75 deletions apps/obsidian/src/utils/importFolderMetadata.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,51 @@ import type { ImportFolderMetadata } from "~/types";
const DG_METADATA_FILE = ".dg.metadata";
const IMPORT_ROOT = "import";

const sanitizeFileName = (fileName: string): string => {
const sanitizeImportFolderName = (fileName: string): string => {
return fileName
.replace(/[<>:"/\\|?*]/g, "")
.replace(/\s+/g, " ")
.trim();
};

const buildImportFolderBasename = (
userName: string,
spaceName: string,
): string => {
return sanitizeImportFolderName(`${userName}-${spaceName}`);
};

const generateShortId = (): string => Math.random().toString(36).slice(2, 8);

const getImportFolderBasename = (folderPath: string): string =>
folderPath.split("/").pop() ?? "";

const isImportFolderMetadata = (
value: unknown,
): value is ImportFolderMetadata => {
if (typeof value !== "object" || value === null) return false;
const v = value as Record<string, unknown>;
return typeof v.spaceUri === "string" && typeof v.spaceName === "string";
};

const parseImportFolderMetadataRaw = (
raw: string,
): ImportFolderMetadata | null => {
let parsed: unknown;
try {
parsed = JSON.parse(raw);
} catch {
try {
// Tolerate trailing commas in case the file was hand-edited outside the plugin.
parsed = JSON.parse(raw.replace(/,\s*([\]}])/g, "$1"));
} catch {
return null;
}
}

return isImportFolderMetadata(parsed) ? parsed : null;
};

const readImportFolderMetadata = async (
adapter: DataAdapter,
folderPath: string,
Expand All @@ -24,18 +60,7 @@ const readImportFolderMetadata = async (
if (!exists) return null;

const raw = await adapter.read(metadataPath);
const parsed: unknown = JSON.parse(raw);

if (
parsed !== null &&
typeof parsed === "object" &&
"spaceUri" in parsed &&
typeof (parsed as Record<string, unknown>).spaceUri === "string"
) {
return parsed as ImportFolderMetadata;
}

return null;
return parseImportFolderMetadataRaw(raw);
} catch {
return null;
}
Expand Down Expand Up @@ -80,107 +105,139 @@ const resolveMetadataDuplicate = async ({
return existingFolderPath;
};

const buildSpaceUriToFolderMap = async (
adapter: DataAdapter,
): Promise<Map<string, string>> => {
const map = new Map<string, string>();

const findImportFolderBySpaceUri = async ({
adapter,
spaceUri,
}: {
adapter: DataAdapter;
spaceUri: string;
}): Promise<{ folderPath: string; metadata: ImportFolderMetadata } | null> => {
const importExists = await adapter.exists(IMPORT_ROOT);
if (!importExists) return map;
if (!importExists) return null;

@maparent maparent Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's the right call, but flagging that means we're not resilient to the user renaming the folder.


const { folders } = await adapter.list(IMPORT_ROOT);

let keptFolderPath: string | null = null;

for (const folderPath of folders) {
const metadata = await readImportFolderMetadata(adapter, folderPath);
if (!metadata) continue;
if (metadata?.spaceUri !== spaceUri) continue;

if (map.has(metadata.spaceUri)) {
const existingPath = map.get(metadata.spaceUri)!;
const keptPath = await resolveMetadataDuplicate({
adapter,
existingFolderPath: existingPath,
newFolderPath: folderPath,
});
map.set(metadata.spaceUri, keptPath);
} else {
map.set(metadata.spaceUri, folderPath);
if (keptFolderPath === null) {
keptFolderPath = folderPath;
continue;
}

keptFolderPath = await resolveMetadataDuplicate({
adapter,
existingFolderPath: keptFolderPath,
newFolderPath: folderPath,
});
}

if (!keptFolderPath) return null;

const metadata = await readImportFolderMetadata(adapter, keptFolderPath);
if (!metadata) return null;

return { folderPath: keptFolderPath, metadata };
};

const resolveUniqueImportFolderPath = async ({
adapter,
desiredBasename,
spaceUri,
}: {
adapter: DataAdapter;
desiredBasename: string;
spaceUri: string;
}): Promise<string> => {
let basename = desiredBasename;
let path = `${IMPORT_ROOT}/${basename}`;

while (await adapter.exists(path)) {
const existingMetadata = await readImportFolderMetadata(adapter, path);
if (existingMetadata?.spaceUri === spaceUri) {
return path;
}
basename = `${desiredBasename}-${generateShortId()}`;
path = `${IMPORT_ROOT}/${basename}`;
}

return map;
return path;
};

export const resolveFolderForSpaceUri = async ({
adapter,
spaceUri,
spaceName,
ownerUserName,
}: {
adapter: DataAdapter;
spaceUri: string;
spaceName: string;
ownerUserName?: string;
}): Promise<string> => {
const spaceUriToFolder = await buildSpaceUriToFolderMap(adapter);

// 1. Exact spaceUri match
if (spaceUriToFolder.has(spaceUri)) {
const folderPath = spaceUriToFolder.get(spaceUri)!;
const existingMetadata = await readImportFolderMetadata(
adapter,
folderPath,
);
if (existingMetadata && existingMetadata.spaceName !== spaceName) {
const existingFolder = await findImportFolderBySpaceUri({
adapter,
spaceUri,
});
if (existingFolder) {
if (existingFolder.metadata.spaceName !== spaceName) {
await writeImportFolderMetadata({
adapter,
folderPath,
metadata: { ...existingMetadata, spaceName },
folderPath: existingFolder.folderPath,
metadata: { ...existingFolder.metadata, spaceName },
});
}
return folderPath;
return existingFolder.folderPath;
}

// 2. Fallback: scan for a folder whose basename matches the sanitized spaceName
// but has no metadata yet
const { folders } = (await adapter.exists(IMPORT_ROOT))
? await adapter.list(IMPORT_ROOT)
: { folders: [] };

const sanitized = sanitizeFileName(spaceName);
const sanitizedSpaceName = sanitizeImportFolderName(spaceName);

for (const folderPath of folders) {
const basename = folderPath.split("/").pop();
if (basename === sanitized) {
const existingMetadata = await readImportFolderMetadata(
adapter,
folderPath,
);
if (!existingMetadata) {
await writeImportFolderMetadata({
adapter,
folderPath,
metadata: { spaceUri, spaceName },
});
return folderPath;
}
}
}
if (getImportFolderBasename(folderPath) !== sanitizedSpaceName) continue;

// 3. Create a new folder, handling name collisions
const desiredPath = `${IMPORT_ROOT}/${sanitized}`;
const desiredExists = await adapter.exists(desiredPath);
const existingMetadata = await readImportFolderMetadata(
adapter,
folderPath,
);
if (existingMetadata) continue;

let newPath: string;
if (desiredExists) {
// The existing folder has a different spaceUri (would have been returned above otherwise)
newPath = `${IMPORT_ROOT}/${sanitized}-${generateShortId()}`;
} else {
newPath = desiredPath;
await writeImportFolderMetadata({
adapter,
folderPath,
metadata: {
spaceUri,
spaceName,
...(ownerUserName ? { userName: ownerUserName } : {}),
},
});
return folderPath;
}

const desiredBasename = ownerUserName
? buildImportFolderBasename(ownerUserName, spaceName)
: sanitizedSpaceName;
const newPath = await resolveUniqueImportFolderPath({
adapter,
desiredBasename,
spaceUri,
});

await adapter.mkdir(newPath);
await writeImportFolderMetadata({
adapter,
folderPath: newPath,
metadata: { spaceUri, spaceName },
metadata: {
spaceUri,
spaceName,
...(ownerUserName ? { userName: ownerUserName } : {}),
},
});

return newPath;
Expand All @@ -202,7 +259,7 @@ export const migrateImportFolderMetadata = async (
const spaceNames = plugin.settings.spaceNames ?? {};
const nameToSpaceUris = new Map<string, Set<string>>();
for (const [spaceUri, name] of Object.entries(spaceNames)) {
const sanitized = sanitizeFileName(name);
const sanitized = sanitizeImportFolderName(name);
const existing = nameToSpaceUris.get(sanitized);
if (existing) {
existing.add(spaceUri);
Expand All @@ -219,7 +276,7 @@ export const migrateImportFolderMetadata = async (
const metadataExists = await adapter.exists(metadataPath);
if (metadataExists) continue;

const basename = folderPath.split("/").pop() ?? "";
const basename = getImportFolderBasename(folderPath);
const spaceUris = nameToSpaceUris.get(basename);

if (spaceUris?.size === 1) {
Expand Down
20 changes: 17 additions & 3 deletions apps/obsidian/src/utils/importNodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,16 @@ export const fetchUserNames = async (
await plugin.saveSettings();
};

const resolveOwnerUserName = (
nodes: ImportableNode[],
plugin: DiscourseGraphPlugin,
): string | undefined => {
const authorId = nodes.find((n) => n.authorId !== undefined)?.authorId;

@mdroidian mdroidian Jun 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think deriving ownerUserName from the first imported node’s authorId matches the intent here. This gives us “an author of one selected node,” not the username/account associated with the source space. If the selected nodes are authored by multiple people, or the first selected node was authored by someone other than the vault account, the import folder could be named after the wrong person.

For Obsidian, the configured username is uploaded as PlatformAccount.name via create_account_in_space, and that account is linked to the vault’s space through LocalAccess(space_id, account_id). Since this flow already has the source spaceId, could we derive the username from that space/account relationship instead? For example, query LocalAccess for this spaceId, join to my_accounts, prefer the relevant agent_type === "person" account, and fall back to spaceName if the lookup is unavailable or ambiguous.

return authorId !== undefined

@maparent maparent Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a concern, let me know if I'm missing something. We are looking at first author; but in obsidian first author is the default author, which is created in src/utils/supabaseContext.ts:101, and unless we've set a username in settings, the default name is the same as the vault name (L80). So we'd basically repeat the vault name in most cases. Can we detect that case, and consider that the author name is undefined if it's identical to the vault name?

? (plugin.settings.userNames ?? {})[authorId]
: undefined;
};

export const fetchNodeContent = async ({
client,
spaceId,
Expand Down Expand Up @@ -1318,9 +1328,11 @@ export const importSelectedNodes = async ({
nodesBySpace.get(node.spaceId)!.push(node);
}

const spaceUris = await getSpaceUris(client, [...nodesBySpace.keys()]);
const spaceNames = await getSpaceNameFromIds(client, [
...nodesBySpace.keys(),
const spaceIdList = [...nodesBySpace.keys()];
const [spaceUris, spaceNames] = await Promise.all([
getSpaceUris(client, spaceIdList),
getSpaceNameFromIds(client, spaceIdList),
fetchUserNames(plugin, client),
]);

// Process each space
Expand All @@ -1336,10 +1348,12 @@ export const importSelectedNodes = async ({
}

const spaceName = spaceNames.get(spaceId) ?? `space-${spaceId}`;
const ownerUserName = resolveOwnerUserName(nodes, plugin);
const importFolderPath = await resolveFolderForSpaceUri({
adapter: plugin.app.vault.adapter,
spaceUri,
spaceName,
ownerUserName,
});

// Process each node in this space
Expand Down