Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion apps/obsidian/src/components/ImportNodesModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const ImportNodesContent = ({ plugin, onClose }: ImportNodesModalProps) => {
getSpaceUris(client, uniqueSpaceIds),
]);

// Populate plugin settings with current space names so they stay up to date
// Keep spaceNames in settings up to date for UI display (formatImportSource reads it)
if (uniqueSpaceIds.length > 0) {
if (!plugin.settings.spaceNames) plugin.settings.spaceNames = {};

Expand Down
5 changes: 5 additions & 0 deletions apps/obsidian/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
migrateFrontmatterRelationsToRelationsJson,
mergeAllRelationsJsonToRoot,
} from "~/utils/relationsStore";
import { migrateImportFolderMetadata } from "./utils/importFolderMetadata";

export default class DiscourseGraphPlugin extends Plugin {
settings: Settings = { ...DEFAULT_SETTINGS };
Expand All @@ -57,6 +58,10 @@ export default class DiscourseGraphPlugin extends Plugin {
console.error("Failed to migrate frontmatter relations:", error);
});

await migrateImportFolderMetadata(this).catch((error) => {
console.error("Failed to migrate import folder metadata:", error);
});

if (this.settings.syncModeEnabled === true) {
void initializeSupabaseSync(this).catch((error) => {
console.error("Failed to initialize Supabase sync:", error);
Expand Down
6 changes: 6 additions & 0 deletions apps/obsidian/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,10 @@ export type GroupWithNodes = {
nodes: ImportableNode[];
};

export type ImportFolderMetadata = {
spaceUri: string;
spaceName: string;
userName?: string;
};

export const VIEW_TYPE_DISCOURSE_CONTEXT = "discourse-context-view";
222 changes: 222 additions & 0 deletions apps/obsidian/src/utils/importFolderMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
import { DataAdapter } from "obsidian";
import type DiscourseGraphPlugin from "~/index";
import type { ImportFolderMetadata } from "~/types";

const DG_METADATA_FILE = ".dg.metadata";
const IMPORT_ROOT = "import";

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

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

const readImportFolderMetadata = async (
adapter: DataAdapter,
folderPath: string,
): Promise<ImportFolderMetadata | null> => {
const metadataPath = `${folderPath}/${DG_METADATA_FILE}`;
try {
const exists = await adapter.exists(metadataPath);
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;
} catch {
return null;
}
};

const writeImportFolderMetadata = async ({
adapter,
folderPath,
metadata,
}: {
adapter: DataAdapter;
folderPath: string;
metadata: ImportFolderMetadata;
}): Promise<void> => {
const metadataPath = `${folderPath}/${DG_METADATA_FILE}`;
await adapter.write(metadataPath, JSON.stringify(metadata, null, 2));
};

const resolveMetadataDuplicate = async ({
adapter,
existingFolderPath,
newFolderPath,
}: {
adapter: DataAdapter;
existingFolderPath: string;
newFolderPath: string;
}): Promise<string> => {
const existingMetadataPath = `${existingFolderPath}/${DG_METADATA_FILE}`;
const newMetadataPath = `${newFolderPath}/${DG_METADATA_FILE}`;

const existingStat = await adapter.stat(existingMetadataPath);
const newStat = await adapter.stat(newMetadataPath);

const newIsNewer =
existingStat && newStat && existingStat.mtime < newStat.mtime;
if (newIsNewer) {
await adapter.remove(existingMetadataPath);
return newFolderPath;
}

await adapter.remove(newMetadataPath);
return existingFolderPath;
};

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

const importExists = await adapter.exists(IMPORT_ROOT);
if (!importExists) return map;

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

for (const folderPath of folders) {
const metadata = await readImportFolderMetadata(adapter, folderPath);
if (!metadata) 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);
}
}

return map;
};

export const resolveFolderForSpaceUri = async ({
adapter,
spaceUri,
spaceName,
}: {
adapter: DataAdapter;
spaceUri: string;
spaceName: 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) {
await writeImportFolderMetadata({
adapter,
folderPath,
metadata: { ...existingMetadata, spaceName },
});
}
return 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);

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;
}
}
}

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

let newPath: string;
if (desiredExists) {
// The existing folder has a different spaceUri (would have been returned above otherwise)
newPath = `${IMPORT_ROOT}/${sanitized}-${generateShortId()}`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 No existence check for randomly generated collision-avoidance folder path

When resolveFolderForSpaceUri detects a name collision at step 3 (line 169), it appends a random 6-char suffix via generateShortId() but never checks whether the resulting path already exists. While the collision probability is very low (~1 in 2.2 billion), if a collision occurs, adapter.mkdir may be a no-op on an existing folder, and writeImportFolderMetadata at line 180 would overwrite that folder's .dg.metadata, effectively stealing it from another space.

Suggested change
newPath = `${IMPORT_ROOT}/${sanitized}-${generateShortId()}`;
newPath = `${IMPORT_ROOT}/${sanitized}-${generateShortId()}`;
while (await adapter.exists(newPath)) {
newPath = `${IMPORT_ROOT}/${sanitized}-${generateShortId()}`;
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

} else {
newPath = desiredPath;
}

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

return newPath;
};

export const migrateImportFolderMetadata = async (
plugin: DiscourseGraphPlugin,
): Promise<void> => {
const adapter = plugin.app.vault.adapter;

const importExists = await adapter.exists(IMPORT_ROOT);
if (!importExists) return;

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

// Invert spaceNames: Record<spaceUri, spaceName> → Map<spaceName, spaceUri>
const spaceNames = plugin.settings.spaceNames ?? {};
const nameToSpaceUri = new Map<string, string>();
for (const [spaceUri, name] of Object.entries(spaceNames)) {
nameToSpaceUri.set(sanitizeFileName(name), spaceUri);
}
Comment thread
trangdoan982 marked this conversation as resolved.
Comment on lines +201 to +204
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Migration silently assigns wrong spaceUri when multiple spaces share the same sanitized name

In migrateImportFolderMetadata, the nameToSpaceUri map is built by iterating Object.entries(spaceNames) and keying on sanitizeFileName(name). If two different spaceUris map to the same sanitized name (e.g., "obsidian:abc" → "My Space" and "obsidian:def" → "My Space"), the last entry silently overwrites the first. The import folder then gets tagged with the wrong spaceUri. Future imports from the overwritten spaceUri will fail to match the existing folder (step 1 of resolveFolderForSpaceUri at importFolderMetadata.ts:125) and will create a new folder (step 3), leaving old imported files in a folder tagged with a different space's URI.

Possible fix: detect collisions and skip ambiguous folders

Before assigning metadata, check if the sanitized name has already been used. If a collision is detected in the inverted map, skip migration for that folder and log a warning, rather than silently assigning the wrong URI.

Prompt for agents
In migrateImportFolderMetadata, the nameToSpaceUri Map is built from Object.entries(spaceNames), keying on sanitizeFileName(name). When two different spaceUris map to the same sanitized name, only the last one survives in the Map, causing the folder to be tagged with the wrong spaceUri.

To fix this: when building the nameToSpaceUri Map (lines 201-204 in importFolderMetadata.ts), detect collisions. If a sanitized name is already present in the Map, either:
1. Remove that key from the Map entirely (skip ambiguous migrations), or
2. Store a Set of spaceUris per name, and in the folder-matching loop (line 212), only write metadata when the mapping is unambiguous (Set has exactly one entry).

A log warning when a collision is detected would help users diagnose the issue.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


for (const folderPath of folders) {
const metadataPath = `${folderPath}/${DG_METADATA_FILE}`;
const metadataExists = await adapter.exists(metadataPath);
if (metadataExists) continue;

const basename = folderPath.split("/").pop() ?? "";
const spaceUri = nameToSpaceUri.get(basename);

if (spaceUri) {
await writeImportFolderMetadata({
adapter,
folderPath,
metadata: { spaceUri, spaceName: spaceNames[spaceUri] ?? basename },
});
}
}
};
26 changes: 13 additions & 13 deletions apps/obsidian/src/utils/importNodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
importRelationsForImportedNodes,
type RemoteRelationInstance,
} from "./importRelations";
import { resolveFolderForSpaceUri } from "./importFolderMetadata";

export const getAvailableGroupIds = async (
client: DGSupabaseClient,
Expand Down Expand Up @@ -754,15 +755,15 @@ const importAssetsForNode = async ({
client,
spaceId,
nodeInstanceId,
spaceName,
importBasePath,
targetMarkdownFile,
originalNodePath,
}: {
plugin: DiscourseGraphPlugin;
client: DGSupabaseClient;
spaceId: number;
nodeInstanceId: string;
spaceName: string;
importBasePath: string;
targetMarkdownFile: TFile;
/** Source vault path of the note (e.g. from Content metadata filePath). Used to place assets under import/{space}/ relative to note. */
originalNodePath?: string;
Expand Down Expand Up @@ -794,8 +795,6 @@ const importAssetsForNode = async ({
return { success: true, pathMapping, errors };
}

const importBasePath = `import/${sanitizeFileName(spaceName)}`;

// Get existing asset mappings from frontmatter
const cache = plugin.app.metadataCache.getFileCache(targetMarkdownFile);
const frontmatter = (cache?.frontmatter as Record<string, unknown>) || {};
Expand Down Expand Up @@ -1223,11 +1222,12 @@ export const importSelectedNodes = async ({
}

const spaceUris = await getSpaceUris(client, [...nodesBySpace.keys()]);
const spaceNames = await getSpaceNameFromIds(client, [
...nodesBySpace.keys(),
]);

// Process each space
for (const [spaceId, nodes] of nodesBySpace.entries()) {
const spaceName = await getSpaceNameFromId(client, spaceId);
const importFolderPath = `import/${sanitizeFileName(spaceName)}`;
const spaceUri = spaceUris.get(spaceId);
if (!spaceUri) {
for (const _node of nodes) {
Expand All @@ -1238,12 +1238,12 @@ export const importSelectedNodes = async ({
continue;
}

// Ensure the import folder exists
const folderExists =
await plugin.app.vault.adapter.exists(importFolderPath);
if (!folderExists) {
await plugin.app.vault.createFolder(importFolderPath);
}
const spaceName = spaceNames.get(spaceId) ?? `space-${spaceId}`;
const importFolderPath = await resolveFolderForSpaceUri({
adapter: plugin.app.vault.adapter,
spaceUri,
spaceName,
});

// Process each node in this space
for (const node of nodes) {
Expand Down Expand Up @@ -1344,7 +1344,7 @@ export const importSelectedNodes = async ({
client,
spaceId,
nodeInstanceId: node.nodeInstanceId,
spaceName,
importBasePath: importFolderPath,
targetMarkdownFile: processedFile,
originalNodePath,
});
Expand Down
Loading